Skip to content

Conversation

XuechunHHH
Copy link
Contributor

@XuechunHHH XuechunHHH commented Oct 1, 2025

Relevant Issues

Description

  • Add UPPER, LOWER and TRIM functions support on CHAR/VARCHAR types.
  • Formalize Concat operator length/type handling for string types.
  • Update submodule.

Other Information

  • Updated Unreleased Section in CHANGELOG: YES
  • Any backward-incompatible changes? NO
  • Any new external dependencies? NO
  • Do your changes comply with the contributing and code style guidelines? YES

License Information

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Copy link

github-actions bot commented Oct 1, 2025

CROSS-COMMIT-REPORT-PARTIQLCORE ❌

BASE (154B9B4) TARGET (7A62BED) +/-
% Passing 94.65% 93.36% -1.29% ⭕
Passing 6440 6501 61 ✅
Failing 176 276 100 ⭕
Ignored 188 186 -2 ✅
Total Tests 6804 6963 159 ✅

Testing Details

Result Details

  • ❌ REGRESSION DETECTED. See Now Failing/Ignored Tests. ❌
  • New passing tests: 255
  • New failing tests: 27
  • New ignored tests: 0
  • Passing in both: 6246
  • Failing in both: 170
  • Ignored in both: 186
  • PASSING in BASE but now FAILING in TARGET: 81
  • PASSING in BASE but now IGNORED in TARGET: 0
  • FAILING in BASE but now PASSING in TARGET: 2
  • IGNORED in BASE but now PASSING in TARGET: 0

New Tests Added

The complete list can be found in GitHub CI summary, either from Step Summary or in the Artifact.

Now FAILING Tests ❌

The complete list can be found in GitHub CI summary, either from Step Summary or in the Artifact.

Now Passing Tests

The following 2 test(s) were previously FAILING in BASE but are now PASSING in TARGET. Before merging, confirm they are intended to pass:

Click here to see
  1. repeated field on struct is ambiguous{identifier:"REPEATED",cn:9,bn:"REPEATED"}, compileOption: STRICT
  2. repeated field on struct is ambiguous{identifier:" "repeated" ",cn:10,bn:"repeated"}, compileOption: STRICT

CROSS-COMMIT-REPORT-PARTIQLEXTENDED ✅

BASE (154B9B4) TARGET (7A62BED) +/-
% Passing 34.25% 35.22% 0.97% ✅
Passing 670 696 26 ✅
Failing 1286 1280 -6 ✅
Ignored 0 0 0 ✅
Total Tests 1956 1976 20 ✅

Testing Details

Result Details

  • New passing tests: 34
  • New failing tests: 0
  • New ignored tests: 0
  • Passing in both: 662
  • Failing in both: 1280
  • Ignored in both: 0
  • PASSING in BASE but now FAILING in TARGET: 0
  • PASSING in BASE but now IGNORED in TARGET: 0
  • FAILING in BASE but now PASSING in TARGET: 0
  • IGNORED in BASE but now PASSING in TARGET: 0

New Tests Added

The complete list can be found in GitHub CI summary, either from Step Summary or in the Artifact.

@XuechunHHH
Copy link
Contributor Author

The cross-commit report failure stems from the update of conformance test submodule. Run the conformance test locally, the failing and passing tests are exactly the same as the main branch.

@XuechunHHH XuechunHHH closed this Oct 6, 2025
@XuechunHHH XuechunHHH reopened this Oct 7, 2025
@XuechunHHH XuechunHHH changed the title add: temporary work around string functions support for CHAR/VARCHAR add: UPPER, LOWER and TRIM support for CHAR/VARCHAR types Oct 8, 2025
CHANGELOG.md Outdated
## [Unreleased](https://TODO.com) - YYYY-MM-DD

### Added
- `UPPER`, `LOWER`, `TRIM` function support for `CHAR/VARCHAR` types
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

self-review: Since the length handling should also be done at planning time, add the real function support instead of only siganatures.

@xd1313113
Copy link
Contributor

The cross-commit report failure stems from the update of conformance test submodule. Run the conformance test locally, the failing and passing tests are exactly the same as the main branch.

You probably need a rebase

@XuechunHHH
Copy link
Contributor Author

XuechunHHH commented Oct 8, 2025

The cross-commit report failure stems from the update of conformance test submodule. Run the conformance test locally, the failing and passing tests are exactly the same as the main branch.

You probably need a rebase

Previously, our submodule partiql-tests target is https://github.com/partiql/partiql-tests/tree/c40ef9738ed812ac8322c8c63aa3cbe3e2fee18f.
In this PR, update it to https://github.com/partiql/partiql-tests/tree/67d22dcd5a7eeaaa2624be9d1bb3da6a8b9faf0a.

@xd1313113
Copy link
Contributor

The cross-commit report failure stems from the update of conformance test submodule. Run the conformance test locally, the failing and passing tests are exactly the same as the main branch.

You probably need a rebase

Previously, our submodule partiql-tests target is https://github.com/partiql/partiql-tests/tree/c40ef9738ed812ac8322c8c63aa3cbe3e2fee18f. In this PR, update it to https://github.com/partiql/partiql-tests/tree/67d22dcd5a7eeaaa2624be9d1bb3da6a8b9faf0a.

PLK has been updated to latest https://github.com/partiql/partiql-tests/tree/67d22dcd5a7eeaaa2624be9d1bb3da6a8b9faf0a by my last PR.
And it is interesting that you did not get a merge conflicts. If you rebase, you will get a clean conformance report.

# Conflicts:
#	CHANGELOG.md
@alancai98
Copy link
Member

alancai98 commented Oct 8, 2025

The cross-commit report failure stems from the update of conformance test submodule. Run the conformance test locally, the failing and passing tests are exactly the same as the main branch.

You probably need a rebase

Previously, our submodule partiql-tests target is partiql/partiql-tests@c40ef97. In this PR, update it to partiql/partiql-tests@67d22dc.

PLK has been updated to latest partiql/partiql-tests@67d22dc by my last PR. And it is interesting that you did not get a merge conflicts. If you rebase, you will get a clean conformance report.

The submodule currently in main is pointing to https://github.com/partiql/partiql-tests/tree/c40ef9738ed812ac8322c8c63aa3cbe3e2fee18f, which is not updated. It's correct in this PR https://github.com/partiql/partiql-lang-kotlin/pull/1839/files#diff-9f1f22f097a05c1bf7e176ec44837633ab6cd0b2afec6cf9743dccc4904f21e6.

In the future, I think we should generally update the conformance submodule in its own PR so we can distinguish conformance report changes more easily.

@xd1313113
Copy link
Contributor

The cross-commit report failure stems from the update of conformance test submodule. Run the conformance test locally, the failing and passing tests are exactly the same as the main branch.

You probably need a rebase

Previously, our submodule partiql-tests target is partiql/partiql-tests@c40ef97. In this PR, update it to partiql/partiql-tests@67d22dc.

PLK has been updated to latest partiql/partiql-tests@67d22dc by my last PR. And it is interesting that you did not get a merge conflicts. If you rebase, you will get a clean conformance report.

The submodule currently in main is pointing to https://github.com/partiql/partiql-tests/tree/c40ef9738ed812ac8322c8c63aa3cbe3e2fee18f, which is not updated. It's correct in this PR https://github.com/partiql/partiql-lang-kotlin/pull/1839/files#diff-9f1f22f097a05c1bf7e176ec44837633ab6cd0b2afec6cf9743dccc4904f21e6.

In the future, I think we should generally update the conformance submodule in its own PR so we can distinguish conformance report changes more easily.

Synced Xuechun offline. The conformance submodule was updated to latest by my PR 6792e1d,
but was reverted in John's PR yesterday accidentally. 154b9b4

It looks PR author is not aware of submodule update and may revert submodule unexpectedly as git add . will add reverted submodule automatically.

@XuechunHHH XuechunHHH self-assigned this Oct 13, 2025
@XuechunHHH XuechunHHH requested a review from alancai98 October 13, 2025 21:12
@@ -1,56 +1,100 @@
// ktlint-disable filename
@file:Suppress("ClassName")

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@XuechunHHH XuechunHHH merged commit ddb9a95 into main Oct 13, 2025
17 checks passed
@XuechunHHH XuechunHHH deleted the add--string-fn-support-on-char branch October 13, 2025 21:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants