add: `UPPER`, `LOWER` and `TRIM` support for `CHAR/VARCHAR` types #1839

XuechunHHH · 2025-10-01T21:51:53Z

Relevant Issues

Related to Issue Add string functions support for CHAR/VARCHAR #1838

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.

github-actions · 2025-10-01T21:58:31Z

CROSS-COMMIT-REPORT-PARTIQLCORE ❌

	BASE (154B9B4)	TARGET (BF59CE2)	+/-
% Passing	94.65%	93.80%	-0.85% ⭕
Passing	6440	6531	91 ✅
Failing	176	246	70 ⭕
Ignored	188	186	-2 ✅
Total Tests	6804	6963	159 ✅

Testing Details

Base Commit: 154b9b4
Target Commit: bf59ce2
Java Version: 17.0.16

Result Details

❌ REGRESSION DETECTED. See Now Failing/Ignored Tests. ❌
New passing tests: 255
New failing tests: 27
New ignored tests: 0
Passing in both: 6276
Failing in both: 170
Ignored in both: 186
PASSING in BASE but now FAILING in TARGET: 51
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

repeated field on struct is ambiguous{identifier:"REPEATED",cn:9,bn:"REPEATED"}, compileOption: STRICT
repeated field on struct is ambiguous{identifier:" "repeated" ",cn:10,bn:"repeated"}, compileOption: STRICT

CROSS-COMMIT-REPORT-PARTIQLEXTENDED ✅

	BASE (154B9B4)	TARGET (BF59CE2)	+/-
% 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

Base Commit: 154b9b4
Target Commit: bf59ce2
Java Version: 17.0.16

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 · 2025-10-01T22:10:14Z

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.

partiql-spi/src/main/kotlin/org/partiql/spi/function/builtins/FnLower.kt

XuechunHHH · 2025-10-08T17:20:16Z

CHANGELOG.md

 ## [Unreleased](https://TODO.com) - YYYY-MM-DD

 ### Added
+- `UPPER`, `LOWER`, `TRIM` function support for `CHAR/VARCHAR` types


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

partiql-spi/src/main/kotlin/org/partiql/spi/function/builtins/FnConcat.kt

xd1313113 · 2025-10-08T17:32:59Z

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 · 2025-10-08T18:07:57Z

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 · 2025-10-08T18:18:59Z

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

partiql-planner/src/test/kotlin/org/partiql/planner/internal/typer/functions/UpperTest.kt

lucaxchen · 2025-10-08T18:25:46Z

partiql-spi/src/main/kotlin/org/partiql/spi/function/builtins/FnLower.kt

+            ) { args ->
+                val string = args[0].bytes.toString(Charsets.UTF_8)
+                val result = string.lowercase()
+                Datum.clob(result.toByteArray())


out of curious, in Datum, we treat PType.CLOB as a byte array. I might be wrong here, but I think it should be just text by the SQL spec.

We currently treat both CLOBand BLOB as byte[] in Datum. See

partiql-lang-kotlin/partiql-spi/src/main/java/org/partiql/spi/value/Datum.java

Lines 723 to 763 in 154b9b4

static Datum clob(@NotNull byte[] value) throws PRuntimeException {

// TODO: Check size of value

return clob(value, Integer.MAX_VALUE);

}

/**

* @param value the backing value

* @param length the length of the clob to coerce the value to

* @return a value of type {@link PType#CLOB} with the default length

* @throws PRuntimeException if the value could not fit into the requested length, or if the requested length is not allowed ({@link org.partiql.spi.errors.PError#INTERNAL_ERROR})

*/

@NotNull

static Datum clob(@NotNull byte[] value, int length) throws PRuntimeException {

// TODO: Check size of value

return new DatumBytes(value, PType.clob(length));

}

// BYTE STRINGS

/**

* @param value the backing value

* @return a value of type {@link PType#BLOB} with the default length

* @throws PRuntimeException if the value could not fit into the default length, or if the requested length is not allowed ({@link org.partiql.spi.errors.PError#INTERNAL_ERROR})

*/

@NotNull

static Datum blob(@NotNull byte[] value) throws PRuntimeException {

// TODO: Check size

return new DatumBytes(value, PType.blob(Integer.MAX_VALUE));

}

/**

* @param value the backing value

* @param length the length of the clob to coerce the value to

* @return a value of type {@link PType#BLOB} with the default length

* @throws PRuntimeException if the value could not fit into the requested length, or if the length is not valid ({@link org.partiql.spi.errors.PError#INTERNAL_ERROR})

*/

@NotNull

static Datum blob(@NotNull byte[] value, int length) throws PRuntimeException {

// TODO: Check size

return new DatumBytes(value, PType.blob(length));

}

Thanks for the call out. Agree that CLOB should be test-based instead. Both PType and Datum need to be formalized further.

partiql-planner/src/test/kotlin/org/partiql/planner/internal/typer/functions/TrimTest.kt

partiql-spi/src/main/kotlin/org/partiql/spi/function/builtins/FnTrim.kt

alancai98 · 2025-10-08T20:43:27Z

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 · 2025-10-08T21:08:03Z

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.

partiql-spi/src/main/kotlin/org/partiql/spi/function/builtins/FnLower.kt

partiql-spi/src/main/kotlin/org/partiql/spi/function/builtins/FnTrim.kt

partiql-spi/src/main/kotlin/org/partiql/spi/function/builtins/FnConcat.kt

partiql-planner/src/test/kotlin/org/partiql/planner/internal/typer/functions/LowerTest.kt

partiql-spi/src/main/kotlin/org/partiql/spi/function/builtins/FnConcat.kt

partiql-spi/src/main/kotlin/org/partiql/spi/function/Builtins.kt

partiql-spi/src/main/kotlin/org/partiql/spi/function/builtins/FnConcat.kt

partiql-spi/src/main/kotlin/org/partiql/spi/function/builtins/FnUpper.kt

XuechunHHH · 2025-10-13T16:56:20Z

CHANGELOG.md


 ### Changed
+- Formalize `UPPER`, `LOWER`, `TRIM` and `CONCAT` length and type handling on string types
+- `CLOB` now supports length parameters with `CLOB(n)` syntax


Self-review: Added support for max_length parameters in the CLOB type, which is CLOB(n) instead of CLOB now. This change aligns our syntax with the SQL-99 specification, which defines CLOB as optionally accepting a length parameter:

<character string type> ::= CHARACTER [ <left paren> <length> <right paren> ] ... | CLOB [ <left paren> <large object length> <right paren> ]

XuechunHHH · 2025-10-13T17:01:56Z

partiql-planner/src/test/kotlin/org/partiql/planner/util/Utils.kt

    PType.varchar(256), // TODO: Length
    PType.string(),
-    PType.clob(Int.MAX_VALUE), // TODO: Length
+    PType.clob(256), // TODO: Length


self-review: This change avoid corner test (overflow) in planner. Previously CLOB actually had fixed param max_length = Int.MAX_VALUE in the implementation.

work around string functions support for char/varchar

1e08443

XuechunHHH requested review from alancai98 and xd1313113 October 1, 2025 22:12

alancai98 reviewed Oct 1, 2025

View reviewed changes

partiql-spi/src/main/kotlin/org/partiql/spi/function/builtins/FnLower.kt Outdated Show resolved Hide resolved

alancai98 reviewed Oct 1, 2025

View reviewed changes

partiql-spi/src/main/kotlin/org/partiql/spi/function/builtins/FnLower.kt Outdated Show resolved Hide resolved

XuechunHHH closed this Oct 6, 2025

XuechunHHH reopened this Oct 7, 2025

XuechunHHH and others added 3 commits October 7, 2025 23:17

Dynamic type resolve for upper/lower

b82d886

add trim

52cc57d

Merge branch 'main' into add--string-fn-support-on-char

58c346b

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

formalize concat

769b46a

XuechunHHH commented Oct 8, 2025

View reviewed changes

XuechunHHH requested review from alancai98 and lucaxchen October 8, 2025 17:25

XuechunHHH added 2 commits October 8, 2025 11:31

add trim

aea18f8

# Conflicts: # CHANGELOG.md

update CHANGELOG

6333695

lucaxchen reviewed Oct 8, 2025

View reviewed changes

update tests names

a03a903

alancai98 reviewed Oct 8, 2025

View reviewed changes

xd1313113 reviewed Oct 9, 2025

View reviewed changes

partiql-spi/src/main/kotlin/org/partiql/spi/function/builtins/FnConcat.kt Outdated Show resolved Hide resolved

xd1313113 reviewed Oct 9, 2025

View reviewed changes

partiql-spi/src/main/kotlin/org/partiql/spi/function/builtins/FnUpper.kt Outdated Show resolved Hide resolved

formalize acc. sql2023

37fe13c

update changelog

5a958c8

XuechunHHH commented Oct 13, 2025

View reviewed changes

XuechunHHH requested review from alancai98, lucaxchen and xd1313113 October 13, 2025 17:02

	static Datum clob(@NotNull byte[] value) throws PRuntimeException {
	// TODO: Check size of value
	return clob(value, Integer.MAX_VALUE);
	}

	/**
	* @param value the backing value
	* @param length the length of the clob to coerce the value to
	* @return a value of type {@link PType#CLOB} with the default length
	* @throws PRuntimeException if the value could not fit into the requested length, or if the requested length is not allowed ({@link org.partiql.spi.errors.PError#INTERNAL_ERROR})
	*/
	@NotNull
	static Datum clob(@NotNull byte[] value, int length) throws PRuntimeException {
	// TODO: Check size of value
	return new DatumBytes(value, PType.clob(length));
	}

	// BYTE STRINGS

	/**
	* @param value the backing value
	* @return a value of type {@link PType#BLOB} with the default length
	* @throws PRuntimeException if the value could not fit into the default length, or if the requested length is not allowed ({@link org.partiql.spi.errors.PError#INTERNAL_ERROR})
	*/
	@NotNull
	static Datum blob(@NotNull byte[] value) throws PRuntimeException {
	// TODO: Check size
	return new DatumBytes(value, PType.blob(Integer.MAX_VALUE));
	}

	/**
	* @param value the backing value
	* @param length the length of the clob to coerce the value to
	* @return a value of type {@link PType#BLOB} with the default length
	* @throws PRuntimeException if the value could not fit into the requested length, or if the length is not valid ({@link org.partiql.spi.errors.PError#INTERNAL_ERROR})
	*/
	@NotNull
	static Datum blob(@NotNull byte[] value, int length) throws PRuntimeException {
	// TODO: Check size
	return new DatumBytes(value, PType.blob(length));
	}

add: UPPER, LOWER and TRIM support for CHAR/VARCHAR types #1839

Are you sure you want to change the base?

add: UPPER, LOWER and TRIM support for CHAR/VARCHAR types #1839

Conversation

XuechunHHH commented Oct 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Relevant Issues

Description

Other Information

License Information

Uh oh!

github-actions bot commented Oct 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

CROSS-COMMIT-REPORT-PARTIQLCORE ❌

Testing Details

Result Details

New Tests Added

Now FAILING Tests ❌

Now Passing Tests

CROSS-COMMIT-REPORT-PARTIQLEXTENDED ✅

Testing Details

Result Details

New Tests Added

Uh oh!

XuechunHHH commented Oct 1, 2025

Uh oh!

Uh oh!

Uh oh!

XuechunHHH Oct 8, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

xd1313113 commented Oct 8, 2025

Uh oh!

XuechunHHH commented Oct 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

xd1313113 commented Oct 8, 2025

Uh oh!

Uh oh!

lucaxchen Oct 8, 2025

Choose a reason for hiding this comment

Uh oh!

XuechunHHH Oct 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

alancai98 commented Oct 8, 2025 • edited by xd1313113 Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

xd1313113 commented Oct 8, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

XuechunHHH Oct 13, 2025

Choose a reason for hiding this comment

Uh oh!

XuechunHHH Oct 13, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

add: `UPPER`, `LOWER` and `TRIM` support for `CHAR/VARCHAR` types #1839

add: `UPPER`, `LOWER` and `TRIM` support for `CHAR/VARCHAR` types #1839

XuechunHHH commented Oct 1, 2025 •

edited

Loading

github-actions bot commented Oct 1, 2025 •

edited

Loading

XuechunHHH commented Oct 8, 2025 •

edited

Loading

XuechunHHH Oct 8, 2025 •

edited

Loading

alancai98 commented Oct 8, 2025 •

edited by xd1313113

Loading