[CALCITE-5241] Implement CHAR function #2878
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
julianhyde
reviewed
Aug 24, 2022
| @@ -462,6 +462,16 @@ public static ByteString right(ByteString s, int n) { | |||
| return s.substring(len - n); | |||
| } | |||
|
|
|||
| /** | |||
| * SQL CHAR(int) function. | |||
| */ | |||
There was a problem hiding this comment.
explain the difference between this and chr. It isn't int vs long.
There was a problem hiding this comment.
also make the javadoc consistent with other javadoc
There was a problem hiding this comment.
Yes, it should be bigint.
| OperandTypes.INTEGER, | ||
| SqlFunctionCategory.STRING); | ||
|
|
||
| @LibraryOperator(libraries = {ORACLE, POSTGRESQL}) |
There was a problem hiding this comment.
CHR and CHAR need javadoc
There was a problem hiding this comment.
Added javadoc.
| */ | ||
| public static final SqlReturnTypeInference CHAR_FORCE_NULLABLE = | ||
| CHAR.andThen(SqlTypeTransforms.FORCE_NULLABLE); | ||
|
|
There was a problem hiding this comment.
not 'nullable Char', but 'nullable CHAR(1)'
There was a problem hiding this comment.
Changed to it.
| @@ -1492,9 +1492,6 @@ protected static Calendar getCalendarNotTooNear(int timeUnit) { | |||
| f.checkScalar("{fn ASCII('ABC')}", "65", "INTEGER NOT NULL"); | |||
| f.checkNull("{fn ASCII(cast(null as varchar(1)))}"); | |||
|
|
|||
| if (false) { | |||
| f.checkScalar("{fn CHAR(code)}", null, ""); | |||
| } | |||
There was a problem hiding this comment.
Why remove this test? The goal of the change is to implement JDBC '{fn CHAR}'
There was a problem hiding this comment.
Sorry, my mistake.
| @@ -339,6 +339,7 @@ public enum BuiltInMethod { | |||
| UPPER(SqlFunctions.class, "upper", String.class), | |||
| LOWER(SqlFunctions.class, "lower", String.class), | |||
| ASCII(SqlFunctions.class, "ascii", String.class), | |||
| CHAR(SqlFunctions.class, "charN", int.class), | |||
| REPEAT(SqlFunctions.class, "repeat", String.class, int.class), | |||
There was a problem hiding this comment.
should this be CHAR(SqlFunctions.class, "charN", long.class)?
and don't you also need CHR(SqlFunctions.class, "chr", long.class)?
There was a problem hiding this comment.
Thank you for your careful review, I changed to them.
julianhyde
added a commit
to julianhyde/calcite
that referenced
this pull request
Sep 8, 2022
Rename methods in SqlFunctions to make semantics clearer.
CHAR is semi-strict (sometimes returns null if argument is not null).
Improve descriptions of CHAR, CHR, {fn CHAR}.
Close apache#2878
julianhyde
pushed a commit
to julianhyde/calcite
that referenced
this pull request
Sep 8, 2022
… '{fn CHAR(n)}'
Close apache#2878
kennethmhc
pushed a commit
to kennethmhc/calcite
that referenced
this pull request
Sep 15, 2022
… '{fn CHAR(n)}'
Close apache#2878
tanclary
pushed a commit
to tanclary/calcite
that referenced
this pull request
Nov 11, 2022
… '{fn CHAR(n)}'
Close apache#2878
tanclary
pushed a commit
to tanclary/calcite
that referenced
this pull request
Nov 28, 2022
… '{fn CHAR(n)}'
Close apache#2878
gleonSun
pushed a commit
to gleonSun/calcite
that referenced
this pull request
Apr 10, 2023
… '{fn CHAR(n)}'
Close apache#2878
gleonSun
added a commit
to gleonSun/calcite
that referenced
this pull request
Jun 12, 2023
For SDK dialect conversion Calcite 1.30 changed Rexcall operator from SqlCaseOperator to SqlPostfixOperator in RelOptUtil#isDistinctFromInternal Calcite 1.30 remove EquiJoinInfo and NonEquiJoinInfo, but we need rexbuilder Calcite 1.30 implement CHAR function Fallback the logic of the simplification condition to version 1.16, otherwise it may lead to a failure to prune partitions Revert "Fallback the logic of the simplification condition to version 1.16, otherwise it may lead to a failure to prune partitions" This reverts commit 27f1a28. [Follow up] Fallback the logic of the simplification condition to version 1.16, otherwise it may lead to a failure to prune partitions Refer to KE-36291 for an adapted method to simplify the filter condition Refer to AL-5295 fix the ut about CharNColumnTest#testCharNColumn Calcite 1.30 fix about SumCaseWhenFunctionRule and CountDistinctCaseWhenFunctionRule [Follow up] Fix ProjectRel replacement and old agg replacement caused by RelBuilder#aggregate method Roll back Calcite's computeDigest and simplifyCase logic from previous versions to ensure correct matching of Kylin models [Follow up] Fix the way digest is calculated, delete the previous logic Fix exception in Calcite 1.30 where data type conversion affected logical plan changes Fix with Sort optimize and Trim function Revert "Calcite 1.30 implement CHAR function" This reverts commit 0e1f6b0. [CALCITE-5241] Implement CHAR function for MySQL and Spark, also JDBC '{fn CHAR(n)}' Close apache#2878 Debug for Calcite deploy Calcite 1.30 don't keep the precision of BigDecimal, add scale to fix exception Fix SqlBasicCall's Deep Copy Logic Raises Rule Optimization Exception and TDVT test Change kap-external-guava20 to kylin-external-guava30 and package name Remove the default Unicode operation on the quoteStringLiteral function in Calcite 1.30
gleonSun
added a commit
to gleonSun/calcite
that referenced
this pull request
Jun 12, 2023
For SDK dialect conversion Calcite 1.30 changed Rexcall operator from SqlCaseOperator to SqlPostfixOperator in RelOptUtil#isDistinctFromInternal Calcite 1.30 remove EquiJoinInfo and NonEquiJoinInfo, but we need rexbuilder Calcite 1.30 implement CHAR function Fallback the logic of the simplification condition to version 1.16, otherwise it may lead to a failure to prune partitions Revert "Fallback the logic of the simplification condition to version 1.16, otherwise it may lead to a failure to prune partitions" This reverts commit 27f1a28. [Follow up] Fallback the logic of the simplification condition to version 1.16, otherwise it may lead to a failure to prune partitions Refer to KE-36291 for an adapted method to simplify the filter condition Refer to AL-5295 fix the ut about CharNColumnTest#testCharNColumn Calcite 1.30 fix about SumCaseWhenFunctionRule and CountDistinctCaseWhenFunctionRule [Follow up] Fix ProjectRel replacement and old agg replacement caused by RelBuilder#aggregate method Roll back Calcite's computeDigest and simplifyCase logic from previous versions to ensure correct matching of Kylin models [Follow up] Fix the way digest is calculated, delete the previous logic Fix exception in Calcite 1.30 where data type conversion affected logical plan changes Fix with Sort optimize and Trim function Revert "Calcite 1.30 implement CHAR function" This reverts commit 0e1f6b0. [CALCITE-5241] Implement CHAR function for MySQL and Spark, also JDBC '{fn CHAR(n)}' Close apache#2878 Debug for Calcite deploy Calcite 1.30 don't keep the precision of BigDecimal, add scale to fix exception Fix SqlBasicCall's Deep Copy Logic Raises Rule Optimization Exception and TDVT test Change kap-external-guava20 to kylin-external-guava30 and package name Remove the default Unicode operation on the quoteStringLiteral function in Calcite 1.30
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Notes:
char(n)is different fromchr(n)when n large than 256.chr(n)be added in https://issues.apache.org/jira/browse/CALCITE-2510