[GitHub] [calcite] sonarcloud[bot] commented on pull request #3212: [CALCITE-5710] Add ARRAY_MAX, ARRAY_MIN function (enabled in Spark li…
sonarcloud[bot] commented on PR #3212: URL: https://github.com/apache/calcite/pull/3212#issuecomment-1573168602 Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_calcite=3212) [](https://sonarcloud.io/project/issues?id=apache_calcite=3212=false=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite=3212=false=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite=3212=false=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite=3212=false=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_calcite=3212=false=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite=3212=false=VULNERABILITY) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3212=false=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3212=false=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3212=false=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/issues?id=apache_calcite=3212=false=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_calcite=3212=false=CODE_SMELL) [3 Code Smells](https://sonarcloud.io/project/issues?id=apache_calcite=3212=false=CODE_SMELL) [](https://sonarcloud.io/component_measures?id=apache_calcite=3212=new_coverage=list) [97.2% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite=3212=new_coverage=list) [](https://sonarcloud.io/component_measures?id=apache_calcite=3212=new_duplicated_lines_density=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite=3212=new_duplicated_lines_density=list) -- 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-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [calcite] sonarcloud[bot] commented on pull request #3212: [CALCITE-5710] Add ARRAY_MAX, ARRAY_MIN function (enabled in Spark li…
sonarcloud[bot] commented on PR #3212: URL: https://github.com/apache/calcite/pull/3212#issuecomment-1573112867 Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_calcite=3212) [](https://sonarcloud.io/project/issues?id=apache_calcite=3212=false=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite=3212=false=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite=3212=false=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite=3212=false=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_calcite=3212=false=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite=3212=false=VULNERABILITY) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3212=false=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3212=false=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3212=false=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/issues?id=apache_calcite=3212=false=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_calcite=3212=false=CODE_SMELL) [10 Code Smells](https://sonarcloud.io/project/issues?id=apache_calcite=3212=false=CODE_SMELL) [](https://sonarcloud.io/component_measures?id=apache_calcite=3212=new_coverage=list) [97.1% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite=3212=new_coverage=list) [](https://sonarcloud.io/component_measures?id=apache_calcite=3212=new_duplicated_lines_density=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite=3212=new_duplicated_lines_density=list) -- 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-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [calcite] sonarcloud[bot] commented on pull request #3212: [CALCITE-5710] Add ARRAY_MAX, ARRAY_MIN function (enabled in Spark li…
sonarcloud[bot] commented on PR #3212: URL: https://github.com/apache/calcite/pull/3212#issuecomment-157329 Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_calcite=3212) [](https://sonarcloud.io/project/issues?id=apache_calcite=3212=false=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite=3212=false=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite=3212=false=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite=3212=false=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_calcite=3212=false=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite=3212=false=VULNERABILITY) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3212=false=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3212=false=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3212=false=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/issues?id=apache_calcite=3212=false=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_calcite=3212=false=CODE_SMELL) [10 Code Smells](https://sonarcloud.io/project/issues?id=apache_calcite=3212=false=CODE_SMELL) [](https://sonarcloud.io/component_measures?id=apache_calcite=3212=new_coverage=list) [97.1% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite=3212=new_coverage=list) [](https://sonarcloud.io/component_measures?id=apache_calcite=3212=new_duplicated_lines_density=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite=3212=new_duplicated_lines_density=list) -- 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-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [calcite] liuyongvs commented on a diff in pull request #3212: [CALCITE-5710] Add ARRAY_MAX, ARRAY_MIN function (enabled in Spark li…
liuyongvs commented on code in PR #3212:
URL: https://github.com/apache/calcite/pull/3212#discussion_r1213866726
##
core/src/main/java/org/apache/calcite/util/BuiltInMethod.java:
##
@@ -629,6 +629,8 @@ public enum BuiltInMethod {
SUBMULTISET_OF(SqlFunctions.class, "submultisetOf", Collection.class,
Collection.class),
ARRAY_DISTINCT(SqlFunctions.class, "distinct", List.class),
+ ARRAY_MAX(SqlFunctions.class, "max", List.class),
Review Comment:
@JiajunBernoulli i does that because i refer to array_reverse implmented by
snuyanzin, the function also names reverse. so i do it too. i modify all,
contains ARRAY_REVERSE
--
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-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [calcite] liuyongvs commented on pull request #3212: [CALCITE-5710] Add ARRAY_MAX, ARRAY_MIN function (enabled in Spark li…
liuyongvs commented on PR #3212: URL: https://github.com/apache/calcite/pull/3212#issuecomment-1573106351 @JiajunBernoulli fix all your reviews and squash the commits -- 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-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [calcite] liuyongvs commented on a diff in pull request #3212: [CALCITE-5710] Add ARRAY_MAX, ARRAY_MIN function (enabled in Spark li…
liuyongvs commented on code in PR #3212:
URL: https://github.com/apache/calcite/pull/3212#discussion_r1213866726
##
core/src/main/java/org/apache/calcite/util/BuiltInMethod.java:
##
@@ -629,6 +629,8 @@ public enum BuiltInMethod {
SUBMULTISET_OF(SqlFunctions.class, "submultisetOf", Collection.class,
Collection.class),
ARRAY_DISTINCT(SqlFunctions.class, "distinct", List.class),
+ ARRAY_MAX(SqlFunctions.class, "max", List.class),
Review Comment:
i does that because i refer to array_reverse implmented by snuyanzin, the
function also names reverse. so i do it too @JiajunBernoulli do you think it is
need? i can modify all
--
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-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [calcite] liuyongvs commented on a diff in pull request #3212: [CALCITE-5710] Add ARRAY_MAX, ARRAY_MIN function (enabled in Spark li…
liuyongvs commented on code in PR #3212:
URL: https://github.com/apache/calcite/pull/3212#discussion_r1213864303
##
core/src/main/java/org/apache/calcite/sql/type/ReturnTypes.java:
##
@@ -500,7 +515,7 @@ public static SqlCall stripSeparator(SqlCall call) {
* Returns the element type of a MULTISET.
*/
public static final SqlReturnTypeInference MULTISET_ELEMENT_NULLABLE =
- MULTISET.andThen(SqlTypeTransforms.TO_MULTISET_ELEMENT_TYPE);
+ MULTISET.andThen(SqlTypeTransforms.TO_COLLECTION_ELEMENT_TYPE);
Review Comment:
@JiajunBernoulli i modify the name because it can be use not only multiset
type, but also array type
--
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-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [calcite] JiajunBernoulli commented on a diff in pull request #3212: [CALCITE-5710] Add ARRAY_MAX, ARRAY_MIN function (enabled in Spark li…
JiajunBernoulli commented on code in PR #3212:
URL: https://github.com/apache/calcite/pull/3212#discussion_r1213835525
##
core/src/main/java/org/apache/calcite/util/BuiltInMethod.java:
##
@@ -629,6 +629,8 @@ public enum BuiltInMethod {
SUBMULTISET_OF(SqlFunctions.class, "submultisetOf", Collection.class,
Collection.class),
ARRAY_DISTINCT(SqlFunctions.class, "distinct", List.class),
+ ARRAY_MAX(SqlFunctions.class, "max", List.class),
Review Comment:
Oh, I just realized now that this name is not suitable.
`MULTISET_UNION_ALL` named as `multisetUnionAll`
ARRAY_MAX should be `arrayMax`, not `max`.
Please change other function names together `ARRAY_REVERSE`, `ARRAY_REPEAT`,
`ARRAY_DISTINCT`.
--
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-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [calcite] JiajunBernoulli commented on a diff in pull request #3212: [CALCITE-5710] Add ARRAY_MAX, ARRAY_MIN function (enabled in Spark li…
JiajunBernoulli commented on code in PR #3212:
URL: https://github.com/apache/calcite/pull/3212#discussion_r1213832112
##
core/src/main/java/org/apache/calcite/sql/type/ReturnTypes.java:
##
@@ -500,7 +515,7 @@ public static SqlCall stripSeparator(SqlCall call) {
* Returns the element type of a MULTISET.
*/
public static final SqlReturnTypeInference MULTISET_ELEMENT_NULLABLE =
- MULTISET.andThen(SqlTypeTransforms.TO_MULTISET_ELEMENT_TYPE);
+ MULTISET.andThen(SqlTypeTransforms.TO_COLLECTION_ELEMENT_TYPE);
Review Comment:
Do you think SEQ or LIST better than COLLECTION?
--
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-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[calcite] branch main updated: [CALCITE-5723] Oracle dialect generates SQL that cannot be recognized by lower version Oracle Server(<12) when unparsing OffsetFetch
This is an automated email from the ASF dual-hosted git repository.
jiajunxie pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/calcite.git
The following commit(s) were added to refs/heads/main by this push:
new d98f96d830 [CALCITE-5723] Oracle dialect generates SQL that cannot be
recognized by lower version Oracle Server(<12) when unparsing OffsetFetch
d98f96d830 is described below
commit d98f96d830e2114aff4166476ebedf6bd82a4c4e
Author: ILuffZhe
AuthorDate: Thu May 25 20:41:28 2023 +0800
[CALCITE-5723] Oracle dialect generates SQL that cannot be recognized by
lower version Oracle Server(<12) when unparsing OffsetFetch
---
.../calcite/sql/dialect/OracleSqlDialect.java | 12 +
.../calcite/rel/rel2sql/RelToSqlConverterTest.java | 29 +-
2 files changed, 40 insertions(+), 1 deletion(-)
diff --git
a/core/src/main/java/org/apache/calcite/sql/dialect/OracleSqlDialect.java
b/core/src/main/java/org/apache/calcite/sql/dialect/OracleSqlDialect.java
index fbd9aef93b..e99ab2a44f 100644
--- a/core/src/main/java/org/apache/calcite/sql/dialect/OracleSqlDialect.java
+++ b/core/src/main/java/org/apache/calcite/sql/dialect/OracleSqlDialect.java
@@ -69,9 +69,12 @@ public class OracleSqlDialect extends SqlDialect {
public static final SqlDialect DEFAULT = new
OracleSqlDialect(DEFAULT_CONTEXT);
+ private final int majorVersion;
+
/** Creates an OracleSqlDialect. */
public OracleSqlDialect(Context context) {
super(context);
+this.majorVersion = context.databaseMajorVersion();
}
@Override public boolean supportsApproxCountDistinct() {
@@ -188,4 +191,13 @@ public class OracleSqlDialect extends SqlDialect {
}
}
}
+
+ @Override public void unparseOffsetFetch(SqlWriter writer, @Nullable SqlNode
offset,
+ @Nullable SqlNode fetch) {
+// majorVersion in SqlDialect.EMPTY_CONTEXT is -1 by default
+if (this.majorVersion != -1 && this.majorVersion < 12) {
+ throw new RuntimeException("Lower Oracle version(<12) doesn't support
offset/fetch syntax!");
+}
+super.unparseOffsetFetch(writer, offset, fetch);
+ }
}
diff --git
a/core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java
b/core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java
index dc6658162c..412ddb4c16 100644
---
a/core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java
+++
b/core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java
@@ -6866,6 +6866,22 @@ class RelToSqlConverterTest {
.withCalcite().ok(expectedCalciteX);
}
+ /** Test case for
+ * https://issues.apache.org/jira/browse/CALCITE-5723;>[CALCITE-5723]
+ * Oracle dialect generates SQL that cannot be recognized by lower version
+ * Oracle Server(<12) when unparsing OffsetFetch. */
+ @Test void testFetchOffsetOracle() {
+String query = "SELECT \"department_id\" FROM \"employee\" LIMIT 2 OFFSET
1";
+String expected = "SELECT \"department_id\"\n"
++ "FROM \"foodmart\".\"employee\"\n"
++ "OFFSET 1 ROWS\n"
++ "FETCH NEXT 2 ROWS ONLY";
+sql(query)
+.withOracle().ok(expected)
+.withOracle(19).ok(expected)
+.withOracle(11).throws_("Lower Oracle version(<12) doesn't support
offset/fetch syntax!");
+ }
+
/** Test case for
* https://issues.apache.org/jira/browse/CALCITE-5265;>[CALCITE-5265]
* JDBC adapter sometimes adds unnecessary parentheses around SELECT in
INSERT. */
@@ -7122,7 +7138,18 @@ class RelToSqlConverterTest {
}
Sql withOracle() {
- return dialect(DatabaseProduct.ORACLE.getDialect());
+ return withOracle(12);
+}
+
+Sql withOracle(int majorVersion) {
+ final SqlDialect oracleDialect = DatabaseProduct.ORACLE.getDialect();
+ return dialect(
+ new OracleSqlDialect(OracleSqlDialect.DEFAULT_CONTEXT
+ .withDatabaseProduct(DatabaseProduct.ORACLE)
+ .withDatabaseMajorVersion(majorVersion)
+ .withIdentifierQuoteString(oracleDialect.quoteIdentifier("")
+ .substring(0, 1))
+ .withNullCollation(oracleDialect.getNullCollation(;
}
Sql withPostgresql() {
[GitHub] [calcite] JiajunBernoulli merged pull request #3225: [CALCITE-5723] Oracle dialect generates SQL that cannot be recognized by lower version Oracle Server(<12) when unparsing OffsetFetch
JiajunBernoulli merged PR #3225: URL: https://github.com/apache/calcite/pull/3225 -- 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-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [calcite] sonarcloud[bot] commented on pull request #3054: (do not check in)
sonarcloud[bot] commented on PR #3054: URL: https://github.com/apache/calcite/pull/3054#issuecomment-1572950445 Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_calcite=3054) [](https://sonarcloud.io/project/issues?id=apache_calcite=3054=false=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite=3054=false=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite=3054=false=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite=3054=false=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_calcite=3054=false=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite=3054=false=VULNERABILITY) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3054=false=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3054=false=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3054=false=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/issues?id=apache_calcite=3054=false=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_calcite=3054=false=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_calcite=3054=false=CODE_SMELL) [](https://sonarcloud.io/component_measures?id=apache_calcite=3054=new_coverage=list) [100.0% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite=3054=new_coverage=list) [](https://sonarcloud.io/component_measures?id=apache_calcite=3054=new_duplicated_lines_density=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite=3054=new_duplicated_lines_density=list) -- 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-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [calcite] sonarcloud[bot] commented on pull request #3236: [CALCITE-5738] Add SORT_ARRAY function (enabled in Spark library)
sonarcloud[bot] commented on PR #3236: URL: https://github.com/apache/calcite/pull/3236#issuecomment-1572938505 Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_calcite=3236) [](https://sonarcloud.io/project/issues?id=apache_calcite=3236=false=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite=3236=false=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite=3236=false=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite=3236=false=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_calcite=3236=false=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite=3236=false=VULNERABILITY) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3236=false=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3236=false=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3236=false=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/issues?id=apache_calcite=3236=false=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_calcite=3236=false=CODE_SMELL) [4 Code Smells](https://sonarcloud.io/project/issues?id=apache_calcite=3236=false=CODE_SMELL) [](https://sonarcloud.io/component_measures?id=apache_calcite=3236=new_coverage=list) [89.0% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite=3236=new_coverage=list) [](https://sonarcloud.io/component_measures?id=apache_calcite=3236=new_duplicated_lines_density=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite=3236=new_duplicated_lines_density=list) -- 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-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [calcite] liuyongvs commented on a diff in pull request #3236: [CALCITE-5738] Add SORT_ARRAY function (enabled in Spark library)
liuyongvs commented on code in PR #3236:
URL: https://github.com/apache/calcite/pull/3236#discussion_r1213775021
##
core/src/main/java/org/apache/calcite/sql/type/OperandTypes.java:
##
@@ -496,6 +496,71 @@ public static SqlOperandTypeChecker variadic(
public static final SqlSingleOperandTypeChecker LITERAL =
new LiteralOperandTypeChecker(false);
+ /**
+ * Operand type-checking strategy type must be a boolean non-NULL literal.
+ */
+ public static final SqlSingleOperandTypeChecker BOOLEAN_LITERAL =
+ new FamilyOperandTypeChecker(ImmutableList.of(SqlTypeFamily.BOOLEAN),
+ i -> false) {
+@Override public boolean checkSingleOperandType(
+SqlCallBinding callBinding,
+SqlNode operand,
+int iFormalOperand,
+boolean throwOnFailure) {
+ if (!LITERAL.checkSingleOperandType(
+ callBinding,
+ operand,
+ iFormalOperand,
+ throwOnFailure)) {
+return false;
+ }
+
+ if (!super.checkSingleOperandType(
+ callBinding,
+ operand,
+ iFormalOperand,
+ throwOnFailure)) {
+return false;
+ }
+
+ final SqlLiteral arg = (SqlLiteral) operand;
+ final boolean isBooleanLiteral =
+ SqlLiteral.valueMatchesType(arg.getValue(), SqlTypeName.BOOLEAN);
+
+ if (!isBooleanLiteral) {
+if (throwOnFailure) {
+ throw callBinding.newError(
+ RESOURCE.argumentMustBeBooleanLiteral(
+ callBinding.getOperator().getName()));
+}
+return false;
+ }
+ return true;
+}
+ };
+
Review Comment:
@tanclary it is need, you can look the unit test i add , which will both
fail in spark
f.checkFails("^sort_array(array[2, null, 1], cast(1 as boolean))^",
"Argument to function 'SORT_ARRAY' must be a literal", false);
and sort_array(array[2, null, 1], b) , b is column
--
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-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [calcite] liuyongvs commented on pull request #3236: [CALCITE-5738] Add SORT_ARRAY function (enabled in Spark library)
liuyongvs commented on PR #3236: URL: https://github.com/apache/calcite/pull/3236#issuecomment-1572926486 hi @tanclary thanks for your review, i have fixed all your reviews -- 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-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [calcite] liuyongvs commented on a diff in pull request #3236: [CALCITE-5738] Add SORT_ARRAY function (enabled in Spark library)
liuyongvs commented on code in PR #3236:
URL: https://github.com/apache/calcite/pull/3236#discussion_r1213775021
##
core/src/main/java/org/apache/calcite/sql/type/OperandTypes.java:
##
@@ -496,6 +496,71 @@ public static SqlOperandTypeChecker variadic(
public static final SqlSingleOperandTypeChecker LITERAL =
new LiteralOperandTypeChecker(false);
+ /**
+ * Operand type-checking strategy type must be a boolean non-NULL literal.
+ */
+ public static final SqlSingleOperandTypeChecker BOOLEAN_LITERAL =
+ new FamilyOperandTypeChecker(ImmutableList.of(SqlTypeFamily.BOOLEAN),
+ i -> false) {
+@Override public boolean checkSingleOperandType(
+SqlCallBinding callBinding,
+SqlNode operand,
+int iFormalOperand,
+boolean throwOnFailure) {
+ if (!LITERAL.checkSingleOperandType(
+ callBinding,
+ operand,
+ iFormalOperand,
+ throwOnFailure)) {
+return false;
+ }
+
+ if (!super.checkSingleOperandType(
+ callBinding,
+ operand,
+ iFormalOperand,
+ throwOnFailure)) {
+return false;
+ }
+
+ final SqlLiteral arg = (SqlLiteral) operand;
+ final boolean isBooleanLiteral =
+ SqlLiteral.valueMatchesType(arg.getValue(), SqlTypeName.BOOLEAN);
+
+ if (!isBooleanLiteral) {
+if (throwOnFailure) {
+ throw callBinding.newError(
+ RESOURCE.argumentMustBeBooleanLiteral(
+ callBinding.getOperator().getName()));
+}
+return false;
+ }
+ return true;
+}
+ };
+
Review Comment:
@tanclary it is need, you can look the unit test i add , which will fail in
spark
f.checkFails("^sort_array(array[2, null, 1], cast(1 as boolean))^",
"Argument to function 'SORT_ARRAY' must be a literal", false);
and sort_array(array[2, null, 1], b) , b is column
##
core/src/main/java/org/apache/calcite/util/BuiltInMethod.java:
##
@@ -308,6 +308,7 @@ public enum BuiltInMethod {
COLLECTIONS_REVERSE_ORDER(Collections.class, "reverseOrder"),
COLLECTIONS_EMPTY_LIST(Collections.class, "emptyList"),
COLLECTIONS_SINGLETON_LIST(Collections.class, "singletonList", Object.class),
+ COLLECTIONS_SORT(Collections.class, "sort", List.class, Comparator.class),
Review Comment:
it doesn't need, i will remove it
##
site/_docs/reference.md:
##
@@ -2658,6 +2658,7 @@ BigQuery's type system uses confusingly different names
for types and functions:
| s | ARRAY_REPEAT(element, count) | Returns the array
containing element count times.
| b | ARRAY_REVERSE(array) | Reverses elements of
*array*
| s | ARRAY_SIZE(array) | Synonym for
`CARDINALITY`
+| s | SORT_ARRAY(array [, ascendingOrder]) | Sorts the input array
in ascending or descending order according to the natural ordering of the array
elements. Null elements will be placed at the beginning of the returned array
in ascending order or at the end of the returned array in descending order
Review Comment:
add in the doc @tanclary
--
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-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [calcite] mihaibudiu commented on pull request #3145: [CALCITE-5615] Program to run SQL Logic Tests for Calcite
mihaibudiu commented on PR #3145: URL: https://github.com/apache/calcite/pull/3145#issuecomment-1572795640 It took about 6 minutes to run 10K tests using CalciteExecutor, so by extrapolation it would take about 2 days to run all tests. Memory consumption increases slightly with time. The failures are saved in a list which keeps growing. I am not sure whether there could be leaks somewhere else. -- 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-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [calcite] tanclary commented on a diff in pull request #3236: [CALCITE-5738] Add SORT_ARRAY function (enabled in Spark library)
tanclary commented on code in PR #3236:
URL: https://github.com/apache/calcite/pull/3236#discussion_r1213435538
##
core/src/main/java/org/apache/calcite/adapter/enumerable/RexImpTable.java:
##
@@ -3061,6 +3063,28 @@ private static class ReinterpretImplementor extends
AbstractRexCallImplementor {
}
}
+ /** Implementor for sort_array. */
+ private static class SortArrayImplementor extends AbstractRexCallImplementor
{
+SortArrayImplementor() {
+ super("sort_array", NullPolicy.STRICT, false);
+}
+
+@Override Expression implementSafe(RexToLixTranslator translator,
+RexCall call, List argValueList) {
+ if (argValueList.size() == 2) {
+return Expressions.call(
+BuiltInMethod.SORT_ARRAY.method,
+argValueList.get(0),
Review Comment:
Here is an example: you call the method and pass just the entire
`argValueList`
https://github.com/apache/calcite/blob/main/core/src/main/java/org/apache/calcite/adapter/enumerable/RexImpTable.java#L3884
The function takes two arguments:
https://github.com/apache/calcite/blob/main/core/src/main/java/org/apache/calcite/util/BuiltInMethod.java#L449
But the argValueList has two elements so this matches.
--
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-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [calcite] tanclary commented on a diff in pull request #3236: [CALCITE-5738] Add SORT_ARRAY function (enabled in Spark library)
tanclary commented on code in PR #3236:
URL: https://github.com/apache/calcite/pull/3236#discussion_r1213433111
##
core/src/main/java/org/apache/calcite/adapter/enumerable/RexImpTable.java:
##
@@ -3061,6 +3063,28 @@ private static class ReinterpretImplementor extends
AbstractRexCallImplementor {
}
}
+ /** Implementor for sort_array. */
+ private static class SortArrayImplementor extends AbstractRexCallImplementor
{
+SortArrayImplementor() {
+ super("sort_array", NullPolicy.STRICT, false);
+}
+
+@Override Expression implementSafe(RexToLixTranslator translator,
+RexCall call, List argValueList) {
+ if (argValueList.size() == 2) {
+return Expressions.call(
+BuiltInMethod.SORT_ARRAY.method,
+argValueList.get(0),
Review Comment:
Yes but you don't need to explicitly retrieve each element, you can just
pass in the list of arguments. Give it a try.
--
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-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [calcite] MasseGuillaume commented on a diff in pull request #3236: [CALCITE-5738] Add SORT_ARRAY function (enabled in Spark library)
MasseGuillaume commented on code in PR #3236:
URL: https://github.com/apache/calcite/pull/3236#discussion_r1213430245
##
core/src/main/java/org/apache/calcite/adapter/enumerable/RexImpTable.java:
##
@@ -3061,6 +3063,28 @@ private static class ReinterpretImplementor extends
AbstractRexCallImplementor {
}
}
+ /** Implementor for sort_array. */
+ private static class SortArrayImplementor extends AbstractRexCallImplementor
{
+SortArrayImplementor() {
+ super("sort_array", NullPolicy.STRICT, false);
+}
+
+@Override Expression implementSafe(RexToLixTranslator translator,
+RexCall call, List argValueList) {
+ if (argValueList.size() == 2) {
+return Expressions.call(
+BuiltInMethod.SORT_ARRAY.method,
+argValueList.get(0),
Review Comment:
https://github.com/apache/calcite/pull/3236/files#diff-23301236e7a9c0e772620a2093cce6a979d7703c511f2417973fae0b26ee62b3R3891
```
list: argValueList.get(0)
ordering: argValueList.get(1)
```
--
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-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [calcite] tanclary commented on a diff in pull request #3236: [CALCITE-5738] Add SORT_ARRAY function (enabled in Spark library)
tanclary commented on code in PR #3236: URL: https://github.com/apache/calcite/pull/3236#discussion_r1213408832 ## site/_docs/reference.md: ## @@ -2658,6 +2658,7 @@ BigQuery's type system uses confusingly different names for types and functions: | s | ARRAY_REPEAT(element, count) | Returns the array containing element count times. | b | ARRAY_REVERSE(array) | Reverses elements of *array* | s | ARRAY_SIZE(array) | Synonym for `CARDINALITY` +| s | SORT_ARRAY(array [, ascendingOrder]) | Sorts the input array in ascending or descending order according to the natural ordering of the array elements. Null elements will be placed at the beginning of the returned array in ascending order or at the end of the returned array in descending order Review Comment: array should be surrounded with asterisks, Also I don't know if it's clear from this what the default behavior is if `ascendingOrder` is not specified. I think this could be made a little more clear. -- 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-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [calcite] tanclary commented on a diff in pull request #3236: [CALCITE-5738] Add SORT_ARRAY function (enabled in Spark library)
tanclary commented on code in PR #3236:
URL: https://github.com/apache/calcite/pull/3236#discussion_r1213406654
##
core/src/main/java/org/apache/calcite/util/BuiltInMethod.java:
##
@@ -308,6 +308,7 @@ public enum BuiltInMethod {
COLLECTIONS_REVERSE_ORDER(Collections.class, "reverseOrder"),
COLLECTIONS_EMPTY_LIST(Collections.class, "emptyList"),
COLLECTIONS_SINGLETON_LIST(Collections.class, "singletonList", Object.class),
+ COLLECTIONS_SORT(Collections.class, "sort", List.class, Comparator.class),
Review Comment:
Why do you need both `COLLECTIONS_SORT` and `SORT_ARRAY`? From what I can
see COLLECTIONS_SORT never gets used. Let me know if I'm misunderstanding.
##
site/_docs/reference.md:
##
@@ -2658,6 +2658,7 @@ BigQuery's type system uses confusingly different names
for types and functions:
| s | ARRAY_REPEAT(element, count) | Returns the array
containing element count times.
| b | ARRAY_REVERSE(array) | Reverses elements of
*array*
| s | ARRAY_SIZE(array) | Synonym for
`CARDINALITY`
+| s | SORT_ARRAY(array [, ascendingOrder]) | Sorts the input array
in ascending or descending order according to the natural ordering of the array
elements. Null elements will be placed at the beginning of the returned array
in ascending order or at the end of the returned array in descending order
Review Comment:
array should be *array*, Also I don't know if it's clear from this what the
default behavior is if `ascendingOrder` is not specified. I think this could be
made a little more clear.
##
core/src/main/java/org/apache/calcite/adapter/enumerable/RexImpTable.java:
##
@@ -3061,6 +3063,28 @@ private static class ReinterpretImplementor extends
AbstractRexCallImplementor {
}
}
+ /** Implementor for sort_array. */
+ private static class SortArrayImplementor extends AbstractRexCallImplementor
{
+SortArrayImplementor() {
+ super("sort_array", NullPolicy.STRICT, false);
+}
+
+@Override Expression implementSafe(RexToLixTranslator translator,
+RexCall call, List argValueList) {
+ if (argValueList.size() == 2) {
+return Expressions.call(
+BuiltInMethod.SORT_ARRAY.method,
+argValueList.get(0),
Review Comment:
Pretty sure if you're just passing the whole argValueList you don't need to
retrieve each element individually, you can just do
`.call(BuiltInMethod.SORT_ARRAY.method, argValueList);` The
`LogicalNotImplementor` and `LogImplementor` are a couple of examples.
##
core/src/main/java/org/apache/calcite/sql/type/OperandTypes.java:
##
@@ -496,6 +496,71 @@ public static SqlOperandTypeChecker variadic(
public static final SqlSingleOperandTypeChecker LITERAL =
new LiteralOperandTypeChecker(false);
+ /**
+ * Operand type-checking strategy type must be a boolean non-NULL literal.
+ */
+ public static final SqlSingleOperandTypeChecker BOOLEAN_LITERAL =
+ new FamilyOperandTypeChecker(ImmutableList.of(SqlTypeFamily.BOOLEAN),
+ i -> false) {
+@Override public boolean checkSingleOperandType(
+SqlCallBinding callBinding,
+SqlNode operand,
+int iFormalOperand,
+boolean throwOnFailure) {
+ if (!LITERAL.checkSingleOperandType(
+ callBinding,
+ operand,
+ iFormalOperand,
+ throwOnFailure)) {
+return false;
+ }
+
+ if (!super.checkSingleOperandType(
+ callBinding,
+ operand,
+ iFormalOperand,
+ throwOnFailure)) {
+return false;
+ }
+
+ final SqlLiteral arg = (SqlLiteral) operand;
+ final boolean isBooleanLiteral =
+ SqlLiteral.valueMatchesType(arg.getValue(), SqlTypeName.BOOLEAN);
+
+ if (!isBooleanLiteral) {
+if (throwOnFailure) {
+ throw callBinding.newError(
+ RESOURCE.argumentMustBeBooleanLiteral(
+ callBinding.getOperator().getName()));
+}
+return false;
+ }
+ return true;
+}
+ };
+
Review Comment:
There's already a `BOOLEAN` entry in `OperandTypes`, what about this
function necessitates the need for `BOOLEAN_LITERAL`?
--
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-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [calcite] snuyanzin commented on a diff in pull request #3113: [CALCITE-5567] Enable jdk19 in ci
snuyanzin commented on code in PR #3113:
URL: https://github.com/apache/calcite/pull/3113#discussion_r1213265269
##
core/src/main/java/org/apache/calcite/util/Util.java:
##
@@ -1717,16 +1719,14 @@ private static void appendPosixTime(StringBuilder buf,
int millis) {
* @return Java locale object
*/
public static Locale parseLocale(String localeString) {
-String[] strings = localeString.split("_");
-switch (strings.length) {
-case 1:
- return new Locale(strings[0]);
-case 2:
- return new Locale(strings[0], strings[1]);
-case 3:
- return new Locale(strings[0], strings[1], strings[2]);
-default:
- throw new AssertionError("bad locale string '" + localeString + "'");
+if (localeString.isEmpty()) {
+ return Locale.ROOT;
Review Comment:
if we are using `Locale.Builder.setLanguageTag` as was suggested in other
comment https://github.com/apache/calcite/pull/3113/files#r1212934318 then yes.
The issue with `Locale.Builder.setLanguageTag` is that it does not allow
empty `lcoalStr` and fails.
I mentioned it in same thread, probably should do it here as well
--
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-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [calcite] snuyanzin commented on a diff in pull request #3113: [CALCITE-5567] Enable jdk19 in ci
snuyanzin commented on code in PR #3113:
URL: https://github.com/apache/calcite/pull/3113#discussion_r1213265269
##
core/src/main/java/org/apache/calcite/util/Util.java:
##
@@ -1717,16 +1719,14 @@ private static void appendPosixTime(StringBuilder buf,
int millis) {
* @return Java locale object
*/
public static Locale parseLocale(String localeString) {
-String[] strings = localeString.split("_");
-switch (strings.length) {
-case 1:
- return new Locale(strings[0]);
-case 2:
- return new Locale(strings[0], strings[1]);
-case 3:
- return new Locale(strings[0], strings[1], strings[2]);
-default:
- throw new AssertionError("bad locale string '" + localeString + "'");
+if (localeString.isEmpty()) {
+ return Locale.ROOT;
Review Comment:
if we are using `Locale.Builder.setLanguageTag` as was suggested in other
comment https://github.com/apache/calcite/pull/3113/files#r1212934318 then yes.
The issue with `Locale.Builder.setLanguageTag` is that it does not allow
empty `lcoalStr` and fails
I mentioned it in same tread, probably should do it here as well
--
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-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [calcite] rubenada commented on a diff in pull request #3113: [CALCITE-5567] Enable jdk19 in ci
rubenada commented on code in PR #3113:
URL: https://github.com/apache/calcite/pull/3113#discussion_r1213257423
##
core/src/main/java/org/apache/calcite/util/Util.java:
##
@@ -1717,16 +1719,14 @@ private static void appendPosixTime(StringBuilder buf,
int millis) {
* @return Java locale object
*/
public static Locale parseLocale(String localeString) {
-String[] strings = localeString.split("_");
-switch (strings.length) {
-case 1:
- return new Locale(strings[0]);
-case 2:
- return new Locale(strings[0], strings[1]);
-case 3:
- return new Locale(strings[0], strings[1], strings[2]);
-default:
- throw new AssertionError("bad locale string '" + localeString + "'");
+if (localeString.isEmpty()) {
+ return Locale.ROOT;
Review Comment:
Is it required to introduce this fallback mechanism to Locale.ROOT?
--
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-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [calcite] rubenada commented on a diff in pull request #3113: [CALCITE-5567] Enable jdk19 in ci
rubenada commented on code in PR #3113:
URL: https://github.com/apache/calcite/pull/3113#discussion_r1213256693
##
site/_docs/history.md:
##
@@ -43,6 +43,10 @@ z.
Breaking Changes
{: #breaking-1-35-0}
+The way of Locale parsing changed within [https://issues.apache.org/jira/browse/CALCITE-5567;>CALCITE-5567]
+Now locales are parsed according to IETF BCP 47 language tag string.
+This leads to the fact that some locales not satisfying IETF BCP 47 language
tag string.
Review Comment:
This sentence sounds a bit confusing or unfinished, wdyt?
--
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-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [calcite] sonarcloud[bot] commented on pull request #3113: [CALCITE-5567] Enable jdk19 in ci
sonarcloud[bot] commented on PR #3113: URL: https://github.com/apache/calcite/pull/3113#issuecomment-1572028969 Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_calcite=3113) [](https://sonarcloud.io/project/issues?id=apache_calcite=3113=false=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite=3113=false=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite=3113=false=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite=3113=false=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_calcite=3113=false=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite=3113=false=VULNERABILITY) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3113=false=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3113=false=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3113=false=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/issues?id=apache_calcite=3113=false=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_calcite=3113=false=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_calcite=3113=false=CODE_SMELL) [](https://sonarcloud.io/component_measures?id=apache_calcite=3113=new_coverage=list) [65.2% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite=3113=new_coverage=list) [](https://sonarcloud.io/component_measures?id=apache_calcite=3113=new_duplicated_lines_density=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite=3113=new_duplicated_lines_density=list) -- 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-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [calcite] snuyanzin commented on a diff in pull request #3113: [CALCITE-5567] Enable jdk19 in ci
snuyanzin commented on code in PR #3113:
URL: https://github.com/apache/calcite/pull/3113#discussion_r1213124528
##
core/src/main/java/org/apache/calcite/sql/parser/SqlParserUtil.java:
##
@@ -738,17 +741,7 @@ public static ParsedCollation parseCollation(String in) {
}
Charset charset = SqlUtil.getCharset(charsetStr);
-String[] localeParts = localeStr.split("_");
-Locale locale;
-if (1 == localeParts.length) {
- locale = new Locale(localeParts[0]);
-} else if (2 == localeParts.length) {
- locale = new Locale(localeParts[0], localeParts[1]);
-} else if (3 == localeParts.length) {
- locale = new Locale(localeParts[0], localeParts[1], localeParts[2]);
-} else {
- throw RESOURCE.illegalLocaleFormat(localeStr).ex();
Review Comment:
I'm ok with `Locale.Builder.setLanguageTag`
the only thing we should probably handle is that current behavior: in case
of empty `localeString` it returns `Locale.ROOT` while
`Locale.Builder.setLanguageTag` fails
Also added actual `localeStr` in exception message since
`IllformedLocaleException` throwing by `setLamguageTag` does not contain it
--
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-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [calcite] sonarcloud[bot] commented on pull request #3228: [CALCITE-5730] Initial null values can be dropped by EnumerableLimitSort with offset
sonarcloud[bot] commented on PR #3228: URL: https://github.com/apache/calcite/pull/3228#issuecomment-1571675075 Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_calcite=3228) [](https://sonarcloud.io/project/issues?id=apache_calcite=3228=false=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite=3228=false=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite=3228=false=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite=3228=false=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_calcite=3228=false=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite=3228=false=VULNERABILITY) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3228=false=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3228=false=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3228=false=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/issues?id=apache_calcite=3228=false=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_calcite=3228=false=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_calcite=3228=false=CODE_SMELL) [](https://sonarcloud.io/component_measures?id=apache_calcite=3228=new_coverage=list) [75.0% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite=3228=new_coverage=list) [](https://sonarcloud.io/component_measures?id=apache_calcite=3228=new_duplicated_lines_density=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite=3228=new_duplicated_lines_density=list) -- 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-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [calcite] gf2121 commented on pull request #3228: [CALCITE-5730] Initial null values can be dropped by EnumerableLimitSort with offset
gf2121 commented on PR #3228: URL: https://github.com/apache/calcite/pull/3228#issuecomment-1571654729 @rubenada No problem! I've squashed the commits. -- 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-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [calcite] rubenada commented on pull request #3228: [CALCITE-5730] Initial null values can be dropped by EnumerableLimitSort with offset
rubenada commented on PR #3228: URL: https://github.com/apache/calcite/pull/3228#issuecomment-1571617897 Thanks @gf2121 ! Sorry, it was actually my fault, I had included that comment as part of a review... and I did not publish it :sweat: , so it appeared as "pending" on my side (and it was not public for the rest). I apologize for the confusion. Changes look good. If no further comment arrives, I'll proceed with the merge in 24h. Please squash commits when you have a bit 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-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [calcite] sonarcloud[bot] commented on pull request #3228: [CALCITE-5730] Initial null values can be dropped by EnumerableLimitSort with offset
sonarcloud[bot] commented on PR #3228: URL: https://github.com/apache/calcite/pull/3228#issuecomment-1571612926 Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_calcite=3228) [](https://sonarcloud.io/project/issues?id=apache_calcite=3228=false=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite=3228=false=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite=3228=false=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite=3228=false=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_calcite=3228=false=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite=3228=false=VULNERABILITY) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3228=false=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3228=false=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3228=false=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/issues?id=apache_calcite=3228=false=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_calcite=3228=false=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_calcite=3228=false=CODE_SMELL) [](https://sonarcloud.io/component_measures?id=apache_calcite=3228=new_coverage=list) [75.0% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite=3228=new_coverage=list) [](https://sonarcloud.io/component_measures?id=apache_calcite=3228=new_duplicated_lines_density=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite=3228=new_duplicated_lines_density=list) -- 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-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [calcite] gf2121 commented on pull request #3228: [CALCITE-5730] Initial null values can be dropped by EnumerableLimitSort with offset
gf2121 commented on PR #3228: URL: https://github.com/apache/calcite/pull/3228#issuecomment-1571584494 > Minor request: could you make the non-bug-related tests a bit more varied: different offset/limit values (not always 1), use asc/desc on order by, use different columns (not always commission+salary) @rubenada Sorry, for some reason I could not see this comment on github. Just got that from your latest comment. I have added a new commit to fix it. will squash commits when ready to merge :) -- 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-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [calcite] rubenada commented on pull request #3228: [CALCITE-5730] Initial null values can be dropped by EnumerableLimitSort with offset
rubenada commented on PR #3228: URL: https://github.com/apache/calcite/pull/3228#issuecomment-1571490053 Thanks for your review, @thomasrebele ! And no problem at all, I also missed this during the review of EnumerableLimitSort. . @gf2121 what's the status here? Do you have time for the final minor changes that I requested on the new test class? > Minor request: could you make the non-bug-related tests a bit more varied: different offset/limit values (not always 1), use asc/desc on order by, use different columns (not always commission+salary) -- 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-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [calcite] luoyuxia commented on a diff in pull request #3187: [CALCITE-5688] Support TRUNCATE TABLE DDL statement in Babel parser
luoyuxia commented on code in PR #3187:
URL: https://github.com/apache/calcite/pull/3187#discussion_r1212661901
##
core/src/main/java/org/apache/calcite/sql/SqlKind.java:
##
@@ -1171,7 +1174,7 @@ public enum SqlKind {
public static final EnumSet DDL =
EnumSet.of(COMMIT, ROLLBACK, ALTER_SESSION,
CREATE_SCHEMA, CREATE_FOREIGN_SCHEMA, DROP_SCHEMA,
- CREATE_TABLE, ALTER_TABLE, DROP_TABLE,
+ CREATE_TABLE, ALTER_TABLE, DROP_TABLE, TRUNCATE_TABLE,
Review Comment:
Not very sure why truncate table is kind of ddl statement.
--
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-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
