You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@calcite.apache.org by "ILuffZhe (via GitHub)" <gi...@apache.org> on 2025/07/08 11:52:16 UTC
[PR] [CALCITE-815] Add an option to allow empty strings to represent null values [calcite]
ILuffZhe opened a new pull request, #4462:
URL: https://github.com/apache/calcite/pull/4462
The following functions will return null if the result is an empty string under Oracle semantics:
- concat (||), UPPER, LOWER, TRIM, OVERLAY, SUBSTRING, INITCAP, REPLACE
- JDBC functions: INSERT, LCASE, UCASE
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-815] Add an option to allow empty strings to represent null values [calcite]
Posted by "ILuffZhe (via GitHub)" <gi...@apache.org>.
ILuffZhe commented on code in PR #4462:
URL: https://github.com/apache/calcite/pull/4462#discussion_r2193696450
##########
core/src/main/java/org/apache/calcite/sql/validate/SqlConformance.java:
##########
@@ -662,4 +662,18 @@ enum SelectAliasLookup {
* True when the unsigned versions of integer types are supported.
*/
boolean supportsUnsignedTypes();
+
+ /**
+ * Whether to convert empty string to null..
+ *
+ * <p>Consider the {@code SUBSTRING} operator. SUBSTRING('abc', 0, 0) will
+ * return null in Oracle, and return an empty string in other databases
+ * like MySQL, PostgreSQL, SQL Server that support SUBSTRING function.
+ *
+ * <p>Among the built-in conformance levels, true in
+ * {@link SqlConformanceEnum#ORACLE_10},
+ * {@link SqlConformanceEnum#ORACLE_12};
+ * false otherwise.
+ */
+ boolean emptyStringIsNull();
Review Comment:
This is discussed in JIRA, I think emptyStringIsNull is OK.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-815] Add an option to allow empty strings to represent null values [calcite]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #4462:
URL: https://github.com/apache/calcite/pull/4462#issuecomment-3170029001
This pull request has been marked as stale due to 30 days of inactivity. It will be closed in 90 days if no further activity occurs. If you think that’s incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the dev@calcite.apache.org list. Thank you for your contributions.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-815] Add an option to allow empty strings to represent null values [calcite]
Posted by "ILuffZhe (via GitHub)" <gi...@apache.org>.
ILuffZhe commented on code in PR #4462:
URL: https://github.com/apache/calcite/pull/4462#discussion_r2194741064
##########
core/src/main/java/org/apache/calcite/sql2rel/StandardConvertletTable.java:
##########
@@ -1952,10 +1960,32 @@ private static class TrimConvertlet implements SqlRexConvertlet {
@Override public RexNode convertCall(SqlRexContext cx, SqlCall call) {
final RexBuilder rexBuilder = cx.getRexBuilder();
+ final SqlParserPos pos = call.getParserPosition();
+ final RelDataTypeFactory typeFactory =
+ cx.getValidator().getTypeFactory();
final RexNode operand =
cx.convertExpression(call.getOperandList().get(0));
- return rexBuilder.makeCall(call.getParserPosition(), SqlStdOperatorTable.TRIM,
- rexBuilder.makeFlag(flag), rexBuilder.makeLiteral(" "), operand);
+ RexNode rawCall =
+ rexBuilder.makeCall(pos, SqlStdOperatorTable.TRIM, rexBuilder.makeFlag(flag),
+ rexBuilder.makeLiteral(" "), operand);
+ SqlConformance conformance = cx.getValidator().config().conformance();
+ if (conformance.emptyStringIsNull()) {
+ // Translate
+ // LTRIM/RTRIM(operand0[,operand1,...])
+ //
+ // to the following if we want Oracle semantics
+ // CASE
+ // WHEN LTRIM/RTRIM(operand0[,operand1,...]) = ''
+ // THEN NULL
+ // ELSE LTRIM/RTRIM(operand0[,operand1,...])
+ // END
+ return rexBuilder.makeCall(pos, SqlStdOperatorTable.CASE,
+ rexBuilder.makeCall(pos, SqlStdOperatorTable.EQUALS, rawCall,
+ rexBuilder.makeLiteral("")),
+ rexBuilder.makeNullLiteral(typeFactory.createSqlType(SqlTypeName.NULL)),
Review Comment:
Done, NULL type now is consistent with call's.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-815] Add an option to allow empty strings to represent null values [calcite]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] closed pull request #4462: [CALCITE-815] Add an option to allow empty strings to represent null values
URL: https://github.com/apache/calcite/pull/4462
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-815] Add an option to allow empty strings to represent null values [calcite]
Posted by "ILuffZhe (via GitHub)" <gi...@apache.org>.
ILuffZhe commented on code in PR #4462:
URL: https://github.com/apache/calcite/pull/4462#discussion_r2193699819
##########
core/src/main/java/org/apache/calcite/sql/fun/SqlLibraryOperators.java:
##########
@@ -346,7 +346,7 @@ private static RelDataType deriveTypePad(SqlOperatorBinding binding, RelDataType
@LibraryOperator(libraries = {BIG_QUERY, ORACLE, POSTGRESQL, SPARK, HIVE})
public static final SqlFunction LTRIM =
SqlBasicFunction.create(SqlKind.LTRIM,
- ReturnTypes.ARG0.andThen(SqlTypeTransforms.TO_NULLABLE)
+ ReturnTypes.ARG0.andThen(SqlTypeTransforms.FORCE_NULLABLE)
Review Comment:
Yes, this makes the functions return null even if no argements is null.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-815] Add an option to allow empty strings to represent null values [calcite]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #4462:
URL: https://github.com/apache/calcite/pull/4462#issuecomment-3882036632
This pull request has been marked as stale due to 30 days of inactivity. It will be closed in 90 days if no further activity occurs. If you think that’s incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the dev@calcite.apache.org list. Thank you for your contributions.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-815] Add an option to allow empty strings to represent null values [calcite]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #4462:
URL: https://github.com/apache/calcite/pull/4462#issuecomment-3048993853
## [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=4462) **Quality Gate passed**
Issues
 [11 New issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=4462&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=4462&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=4462&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [96.1% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=4462&metric=new_coverage&view=list)
 [5.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=4462&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=4462)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-815] Add an option to allow empty strings to represent null values [calcite]
Posted by "ILuffZhe (via GitHub)" <gi...@apache.org>.
ILuffZhe commented on code in PR #4462:
URL: https://github.com/apache/calcite/pull/4462#discussion_r2194740220
##########
core/src/main/java/org/apache/calcite/sql2rel/StandardConvertletTable.java:
##########
@@ -1952,10 +1960,32 @@ private static class TrimConvertlet implements SqlRexConvertlet {
@Override public RexNode convertCall(SqlRexContext cx, SqlCall call) {
final RexBuilder rexBuilder = cx.getRexBuilder();
+ final SqlParserPos pos = call.getParserPosition();
+ final RelDataTypeFactory typeFactory =
+ cx.getValidator().getTypeFactory();
final RexNode operand =
cx.convertExpression(call.getOperandList().get(0));
- return rexBuilder.makeCall(call.getParserPosition(), SqlStdOperatorTable.TRIM,
- rexBuilder.makeFlag(flag), rexBuilder.makeLiteral(" "), operand);
+ RexNode rawCall =
+ rexBuilder.makeCall(pos, SqlStdOperatorTable.TRIM, rexBuilder.makeFlag(flag),
+ rexBuilder.makeLiteral(" "), operand);
+ SqlConformance conformance = cx.getValidator().config().conformance();
+ if (conformance.emptyStringIsNull()) {
+ // Translate
+ // LTRIM/RTRIM(operand0[,operand1,...])
+ //
+ // to the following if we want Oracle semantics
Review Comment:
> Here, we convert LTRIM(A) to the following expression: CASE WHEN LTRIM(A) = '' THEN NULL ELSE LTRIM(A) END. LTRIM(A) is calculated multiple times. If LTRIM(A) is a non-deterministic function, it may lead to an uncontrolled final result.
Nondeterministic is not allowed during runtime, I've added some tests and throw exception when it occurs.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-815] Add an option to allow empty strings to represent null values [calcite]
Posted by "NobiGo (via GitHub)" <gi...@apache.org>.
NobiGo commented on code in PR #4462:
URL: https://github.com/apache/calcite/pull/4462#discussion_r2193697860
##########
core/src/main/java/org/apache/calcite/sql/validate/SqlConformance.java:
##########
@@ -662,4 +662,18 @@ enum SelectAliasLookup {
* True when the unsigned versions of integer types are supported.
*/
boolean supportsUnsignedTypes();
+
+ /**
+ * Whether to convert empty string to null..
+ *
+ * <p>Consider the {@code SUBSTRING} operator. SUBSTRING('abc', 0, 0) will
+ * return null in Oracle, and return an empty string in other databases
+ * like MySQL, PostgreSQL, SQL Server that support SUBSTRING function.
+ *
+ * <p>Among the built-in conformance levels, true in
+ * {@link SqlConformanceEnum#ORACLE_10},
+ * {@link SqlConformanceEnum#ORACLE_12};
+ * false otherwise.
+ */
+ boolean emptyStringIsNull();
Review Comment:
Ok
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-815] Add an option to allow empty strings to represent null values [calcite]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #4462:
URL: https://github.com/apache/calcite/pull/4462#issuecomment-4427410863
This pull request has been closed due to lack of activity. If you think that is incorrect, or the pull request requires review, you can revive the PR at any time.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-815] Add an option to allow empty strings to represent null values [calcite]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #4462:
URL: https://github.com/apache/calcite/pull/4462#issuecomment-3052289111
## [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=4462) **Quality Gate passed**
Issues
 [13 New issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=4462&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=4462&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=4462&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [95.1% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=4462&metric=new_coverage&view=list)
 [5.8% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=4462&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=4462)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-815] Add an option to allow empty strings to represent null values [calcite]
Posted by "ILuffZhe (via GitHub)" <gi...@apache.org>.
ILuffZhe commented on code in PR #4462:
URL: https://github.com/apache/calcite/pull/4462#discussion_r2192316603
##########
core/src/main/java/org/apache/calcite/sql/validate/SqlConformance.java:
##########
@@ -662,4 +662,18 @@ enum SelectAliasLookup {
* True when the unsigned versions of integer types are supported.
*/
boolean supportsUnsignedTypes();
+
+ /**
+ * Whether to convert empty string to null..
Review Comment:
I'll remove the redundant .
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-815] Add an option to allow empty strings to represent null values [calcite]
Posted by "NobiGo (via GitHub)" <gi...@apache.org>.
NobiGo commented on code in PR #4462:
URL: https://github.com/apache/calcite/pull/4462#discussion_r2193679882
##########
core/src/main/java/org/apache/calcite/sql/fun/SqlLibraryOperators.java:
##########
@@ -346,7 +346,7 @@ private static RelDataType deriveTypePad(SqlOperatorBinding binding, RelDataType
@LibraryOperator(libraries = {BIG_QUERY, ORACLE, POSTGRESQL, SPARK, HIVE})
public static final SqlFunction LTRIM =
SqlBasicFunction.create(SqlKind.LTRIM,
- ReturnTypes.ARG0.andThen(SqlTypeTransforms.TO_NULLABLE)
+ ReturnTypes.ARG0.andThen(SqlTypeTransforms.FORCE_NULLABLE)
Review Comment:
Is it only possible to change this in the Oracle mode here?
##########
core/src/main/java/org/apache/calcite/sql/validate/SqlConformance.java:
##########
@@ -662,4 +662,18 @@ enum SelectAliasLookup {
* True when the unsigned versions of integer types are supported.
*/
boolean supportsUnsignedTypes();
+
+ /**
+ * Whether to convert empty string to null..
+ *
+ * <p>Consider the {@code SUBSTRING} operator. SUBSTRING('abc', 0, 0) will
+ * return null in Oracle, and return an empty string in other databases
+ * like MySQL, PostgreSQL, SQL Server that support SUBSTRING function.
+ *
+ * <p>Among the built-in conformance levels, true in
+ * {@link SqlConformanceEnum#ORACLE_10},
+ * {@link SqlConformanceEnum#ORACLE_12};
+ * false otherwise.
+ */
+ boolean emptyStringIsNull();
Review Comment:
`treatEmptyStringAsNull` ?
##########
core/src/main/java/org/apache/calcite/sql2rel/StandardConvertletTable.java:
##########
@@ -1952,10 +1960,32 @@ private static class TrimConvertlet implements SqlRexConvertlet {
@Override public RexNode convertCall(SqlRexContext cx, SqlCall call) {
final RexBuilder rexBuilder = cx.getRexBuilder();
+ final SqlParserPos pos = call.getParserPosition();
+ final RelDataTypeFactory typeFactory =
+ cx.getValidator().getTypeFactory();
final RexNode operand =
cx.convertExpression(call.getOperandList().get(0));
- return rexBuilder.makeCall(call.getParserPosition(), SqlStdOperatorTable.TRIM,
- rexBuilder.makeFlag(flag), rexBuilder.makeLiteral(" "), operand);
+ RexNode rawCall =
+ rexBuilder.makeCall(pos, SqlStdOperatorTable.TRIM, rexBuilder.makeFlag(flag),
+ rexBuilder.makeLiteral(" "), operand);
+ SqlConformance conformance = cx.getValidator().config().conformance();
+ if (conformance.emptyStringIsNull()) {
+ // Translate
+ // LTRIM/RTRIM(operand0[,operand1,...])
+ //
+ // to the following if we want Oracle semantics
+ // CASE
+ // WHEN LTRIM/RTRIM(operand0[,operand1,...]) = ''
+ // THEN NULL
+ // ELSE LTRIM/RTRIM(operand0[,operand1,...])
+ // END
+ return rexBuilder.makeCall(pos, SqlStdOperatorTable.CASE,
+ rexBuilder.makeCall(pos, SqlStdOperatorTable.EQUALS, rawCall,
+ rexBuilder.makeLiteral("")),
+ rexBuilder.makeNullLiteral(typeFactory.createSqlType(SqlTypeName.NULL)),
Review Comment:
The NULL type here should be the same as that of LTRIM(A)
##########
core/src/main/java/org/apache/calcite/sql2rel/StandardConvertletTable.java:
##########
@@ -1952,10 +1960,32 @@ private static class TrimConvertlet implements SqlRexConvertlet {
@Override public RexNode convertCall(SqlRexContext cx, SqlCall call) {
final RexBuilder rexBuilder = cx.getRexBuilder();
+ final SqlParserPos pos = call.getParserPosition();
+ final RelDataTypeFactory typeFactory =
+ cx.getValidator().getTypeFactory();
final RexNode operand =
cx.convertExpression(call.getOperandList().get(0));
- return rexBuilder.makeCall(call.getParserPosition(), SqlStdOperatorTable.TRIM,
- rexBuilder.makeFlag(flag), rexBuilder.makeLiteral(" "), operand);
+ RexNode rawCall =
+ rexBuilder.makeCall(pos, SqlStdOperatorTable.TRIM, rexBuilder.makeFlag(flag),
+ rexBuilder.makeLiteral(" "), operand);
+ SqlConformance conformance = cx.getValidator().config().conformance();
+ if (conformance.emptyStringIsNull()) {
+ // Translate
+ // LTRIM/RTRIM(operand0[,operand1,...])
+ //
+ // to the following if we want Oracle semantics
Review Comment:
Here, we convert LTRIM(A) to the following expression: CASE WHEN LTRIM(A) = '' THEN NULL ELSE LTRIM(A) END. LTRIM(A) is calculated multiple times. If LTRIM(A) is a non-deterministic function, it may lead to an uncontrolled final result.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-815] Add an option to allow empty strings to represent null values [calcite]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #4462:
URL: https://github.com/apache/calcite/pull/4462#issuecomment-3445639464
This pull request has been marked as stale due to 30 days of inactivity. It will be closed in 90 days if no further activity occurs. If you think that’s incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the dev@calcite.apache.org list. Thank you for your contributions.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org