[GitHub] [calcite] JiajunBernoulli commented on a diff in pull request #3395: [CALCITE-5948] Explicit casting should be made if the type of an element in ARRAY/MAP not equals with the derived componen
JiajunBernoulli commented on code in PR #3395: URL: https://github.com/apache/calcite/pull/3395#discussion_r1331032658 ## core/src/test/resources/sql/misc.iq: ## @@ -2167,12 +2167,12 @@ select array[1,null,2] as a from (values (1)); values array['a',null,'bcd'], array['efgh']; -++ -| EXPR$0 | -++ -| [a, null, bcd] | -| [efgh] | -++ ++--+ +| EXPR$0 | ++--+ +| [a , null, bcd] | Review Comment: Ok, Thank you for your clear explanation. -- 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 #3418: [CALCITE-5994] Add optimization rule to remove Sort when its input's row number is less or equal to one
sonarcloud[bot] commented on PR #3418: URL: https://github.com/apache/calcite/pull/3418#issuecomment-1726977927 Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_calcite=3418) [](https://sonarcloud.io/project/issues?id=apache_calcite=3418=false=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite=3418=false=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite=3418=false=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite=3418=false=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_calcite=3418=false=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite=3418=false=VULNERABILITY) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3418=false=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3418=false=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3418=false=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/issues?id=apache_calcite=3418=false=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_calcite=3418=false=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_calcite=3418=false=CODE_SMELL) [](https://sonarcloud.io/component_measures?id=apache_calcite=3418=new_coverage=list) [87.0% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite=3418=new_coverage=list) [](https://sonarcloud.io/component_measures?id=apache_calcite=3418=new_duplicated_lines_density=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite=3418=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] LakeShen commented on pull request #3418: [CALCITE-5994] Add optimization rule to remove Sort when its input's row number is less or equal to one
LakeShen commented on PR #3418: URL: https://github.com/apache/calcite/pull/3418#issuecomment-1726951953 > Thanks @LakeShen , lgtm. Could you please squash commits into a single one? I'll merge the PR in the coming days if no further remark appears. Hi @rubenada ,I have squashed the commits,If you have time, please help me merge this pr, thank you so much. -- 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] LakeShen commented on a diff in pull request #3418: [CALCITE-5994] Add optimization rule to remove Sort when its input's row number is less or equal to one
LakeShen commented on code in PR #3418:
URL: https://github.com/apache/calcite/pull/3418#discussion_r1330971288
##
core/src/main/java/org/apache/calcite/rel/rules/SortRemoveRedundantRule.java:
##
@@ -0,0 +1,77 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.calcite.rel.rules;
+
+import org.apache.calcite.plan.RelOptRuleCall;
+import org.apache.calcite.plan.RelRule;
+import org.apache.calcite.rel.core.Sort;
+
+import org.immutables.value.Value;
+
+/**
+ * Planner rule that removes
+ * the redundant {@link org.apache.calcite.rel.core.Sort} if its input
+ * max row number is less than or equal to one.
+ *
+ * For example:
+ * {@code
+ * select max(totalprice) from orders order by 1}
+ *
+ *
+ * could be converted to
+ * {@code
+ * select max(totalprice) from orders}
+ *
+ *
+ * @see CoreRules#SORT_REMOVE_REDUNDANT
+ */
+@Value.Enclosing
+public class SortRemoveRedundantRule
+extends RelRule
+implements TransformationRule {
+ protected SortRemoveRedundantRule(final SortRemoveRedundantRule.Config
config) {
+super(config);
+ }
+
+ @Override public void onMatch(final RelOptRuleCall call) {
+final Sort sort = call.rel(0);
+if (sort.offset != null || sort.fetch != null) {
Review Comment:
> Could we also support explicit OFFSET and LIMIT when sort's input max row
count is less or equal than 1? I think it is also useful.
Hi @herunkang2018 ,thank you for your review suggestions.
We could remove the Limit if input's row number less or equal to Limit fetch
,and could also redundant topN when input row number less or equal to 1.
I plan to contribute these optimizations in the future.
--
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] LakeShen commented on a diff in pull request #3418: [CALCITE-5994] Add optimization rule to remove Sort when its input's row number is less or equal to one
LakeShen commented on code in PR #3418:
URL: https://github.com/apache/calcite/pull/3418#discussion_r1330968837
##
core/src/main/java/org/apache/calcite/rel/rules/SortRemoveRedundantRule.java:
##
@@ -0,0 +1,77 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.calcite.rel.rules;
+
+import org.apache.calcite.plan.RelOptRuleCall;
+import org.apache.calcite.plan.RelRule;
+import org.apache.calcite.rel.core.Sort;
+
+import org.immutables.value.Value;
+
+/**
+ * Planner rule that removes
+ * the redundant {@link org.apache.calcite.rel.core.Sort} if its input
+ * max row number is less than or equal to one.
+ *
+ * For example:
+ * {@code
+ * select max(totalprice) from orders order by 1}
+ *
+ *
+ * could be converted to
+ * {@code
+ * select max(totalprice) from orders}
+ *
+ *
+ * @see CoreRules#SORT_REMOVE_REDUNDANT
+ */
+@Value.Enclosing
+public class SortRemoveRedundantRule
+extends RelRule
+implements TransformationRule {
+ protected SortRemoveRedundantRule(final SortRemoveRedundantRule.Config
config) {
+super(config);
+ }
+
+ @Override public void onMatch(final RelOptRuleCall call) {
+final Sort sort = call.rel(0);
+if (sort.offset != null || sort.fetch != null) {
+ // Don't remove sort if would also remove OFFSET or LIMIT.
Review Comment:
Yes, I will take 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
[calcite] branch main updated: [CALCITE-5997] OFFSET operator is incorrectly unparsed
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 9ce7077bc8 [CALCITE-5997] OFFSET operator is incorrectly unparsed
9ce7077bc8 is described below
commit 9ce7077bc8353e21e71ddb9f4a6a3025422ea8c4
Author: Mihai Budiu
AuthorDate: Mon Sep 18 15:09:32 2023 -0700
[CALCITE-5997] OFFSET operator is incorrectly unparsed
Signed-off-by: Mihai Budiu
---
.../apache/calcite/sql/fun/SqlItemOperator.java| 8 +-
.../apache/calcite/sql/parser/SqlParserTest.java | 31 ++
2 files changed, 38 insertions(+), 1 deletion(-)
diff --git a/core/src/main/java/org/apache/calcite/sql/fun/SqlItemOperator.java
b/core/src/main/java/org/apache/calcite/sql/fun/SqlItemOperator.java
index 627115f5a3..4f5371c9fc 100644
--- a/core/src/main/java/org/apache/calcite/sql/fun/SqlItemOperator.java
+++ b/core/src/main/java/org/apache/calcite/sql/fun/SqlItemOperator.java
@@ -78,7 +78,13 @@ public class SqlItemOperator extends SqlSpecialOperator {
SqlWriter writer, SqlCall call, int leftPrec, int rightPrec) {
call.operand(0).unparse(writer, leftPrec, 0);
final SqlWriter.Frame frame = writer.startList("[", "]");
-call.operand(1).unparse(writer, 0, 0);
+if (!this.getName().equals("ITEM")) {
+ final SqlWriter.Frame offsetFrame = writer.startFunCall(this.getName());
+ call.operand(1).unparse(writer, 0, 0);
+ writer.endFunCall(offsetFrame);
+} else {
+ call.operand(1).unparse(writer, 0, 0);
+}
writer.endList(frame);
}
diff --git
a/testkit/src/main/java/org/apache/calcite/sql/parser/SqlParserTest.java
b/testkit/src/main/java/org/apache/calcite/sql/parser/SqlParserTest.java
index 60606f58ac..edda041414 100644
--- a/testkit/src/main/java/org/apache/calcite/sql/parser/SqlParserTest.java
+++ b/testkit/src/main/java/org/apache/calcite/sql/parser/SqlParserTest.java
@@ -725,6 +725,37 @@ public class SqlParserTest {
+ ".*");
}
+ /** Test case for https://issues.apache.org/jira/browse/CALCITE-5997;>[CALCITE-5997]
+ * OFFSET operator is incorrectly unparsed. */
+ @Test void testOffset() {
+sql("SELECT ARRAY[2,4,6][2]")
+.ok("SELECT (ARRAY[2, 4, 6])[2]");
+sql("SELECT ARRAY[2,4,6][ORDINAL(2)]")
+.withDialect(BIG_QUERY)
+.ok("SELECT (ARRAY[2, 4, 6])[ORDINAL(2)]");
+sql("SELECT ARRAY[2,4,6][OFFSET(2)]")
+.withDialect(BIG_QUERY)
+.ok("SELECT (ARRAY[2, 4, 6])[OFFSET(2)]");
+sql("SELECT ARRAY[2,4,6][SAFE_OFFSET(2)]")
+.withDialect(BIG_QUERY)
+.ok("SELECT (ARRAY[2, 4, 6])[SAFE_OFFSET(2)]");
+sql("SELECT ARRAY[2,4,6][SAFE_ORDINAL(2)]")
+.withDialect(BIG_QUERY)
+.ok("SELECT (ARRAY[2, 4, 6])[SAFE_ORDINAL(2)]");
+
+// All these tests work without BIG_QUERY as well.
+// The SQL parser accepts this syntax, so we need to be
+// able to unparse it into something.
+sql("SELECT ARRAY[2,4,6][ORDINAL(2)]")
+.ok("SELECT (ARRAY[2, 4, 6])[ORDINAL(2)]");
+sql("SELECT ARRAY[2,4,6][OFFSET(2)]")
+.ok("SELECT (ARRAY[2, 4, 6])[OFFSET(2)]");
+sql("SELECT ARRAY[2,4,6][SAFE_OFFSET(2)]")
+.ok("SELECT (ARRAY[2, 4, 6])[SAFE_OFFSET(2)]");
+sql("SELECT ARRAY[2,4,6][SAFE_ORDINAL(2)]")
+.ok("SELECT (ARRAY[2, 4, 6])[SAFE_ORDINAL(2)]");
+ }
+
@Test void testInvalidToken() {
// Causes problems to the test infrastructure because the token mgr
// throws a java.lang.Error. The usual case is that the parser throws
[GitHub] [calcite] JiajunBernoulli merged pull request #3423: [CALCITE-5997] OFFSET operator is incorrectly unparsed
JiajunBernoulli merged PR #3423: URL: https://github.com/apache/calcite/pull/3423 -- 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 #3317: [CALCITE-5826] Add FIND_IN_SET function (enabled in Hive and Spark library)
sonarcloud[bot] commented on PR #3317: URL: https://github.com/apache/calcite/pull/3317#issuecomment-1726729404 Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_calcite=3317) [](https://sonarcloud.io/project/issues?id=apache_calcite=3317=false=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite=3317=false=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite=3317=false=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite=3317=false=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_calcite=3317=false=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite=3317=false=VULNERABILITY) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3317=false=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3317=false=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3317=false=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/issues?id=apache_calcite=3317=false=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_calcite=3317=false=CODE_SMELL) [2 Code Smells](https://sonarcloud.io/project/issues?id=apache_calcite=3317=false=CODE_SMELL) [](https://sonarcloud.io/component_measures?id=apache_calcite=3317=new_coverage=list) [98.0% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite=3317=new_coverage=list) [](https://sonarcloud.io/component_measures?id=apache_calcite=3317=new_duplicated_lines_density=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite=3317=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] herunkang2018 commented on a diff in pull request #3418: [CALCITE-5994] Add optimization rule to remove Sort when its input's row number is less or equal to one
herunkang2018 commented on code in PR #3418:
URL: https://github.com/apache/calcite/pull/3418#discussion_r1330838164
##
core/src/main/java/org/apache/calcite/rel/rules/SortRemoveRedundantRule.java:
##
@@ -0,0 +1,77 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.calcite.rel.rules;
+
+import org.apache.calcite.plan.RelOptRuleCall;
+import org.apache.calcite.plan.RelRule;
+import org.apache.calcite.rel.core.Sort;
+
+import org.immutables.value.Value;
+
+/**
+ * Planner rule that removes
+ * the redundant {@link org.apache.calcite.rel.core.Sort} if its input
+ * max row number is less than or equal to one.
+ *
+ * For example:
+ * {@code
+ * select max(totalprice) from orders order by 1}
+ *
+ *
+ * could be converted to
+ * {@code
+ * select max(totalprice) from orders}
+ *
+ *
+ * @see CoreRules#SORT_REMOVE_REDUNDANT
+ */
+@Value.Enclosing
+public class SortRemoveRedundantRule
+extends RelRule
+implements TransformationRule {
+ protected SortRemoveRedundantRule(final SortRemoveRedundantRule.Config
config) {
+super(config);
+ }
+
+ @Override public void onMatch(final RelOptRuleCall call) {
+final Sort sort = call.rel(0);
+if (sort.offset != null || sort.fetch != null) {
Review Comment:
Could we also support explicit OFFSET and LIMIT when sort's input max row
count is less or equal than 1? I think it is also useful.
--
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] herunkang2018 commented on a diff in pull request #3418: [CALCITE-5994] Add optimization rule to remove Sort when its input's row number is less or equal to one
herunkang2018 commented on code in PR #3418:
URL: https://github.com/apache/calcite/pull/3418#discussion_r1330831977
##
core/src/main/java/org/apache/calcite/rel/rules/SortRemoveRedundantRule.java:
##
@@ -0,0 +1,77 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.calcite.rel.rules;
+
+import org.apache.calcite.plan.RelOptRuleCall;
+import org.apache.calcite.plan.RelRule;
+import org.apache.calcite.rel.core.Sort;
+
+import org.immutables.value.Value;
+
+/**
+ * Planner rule that removes
+ * the redundant {@link org.apache.calcite.rel.core.Sort} if its input
+ * max row number is less than or equal to one.
+ *
+ * For example:
+ * {@code
+ * select max(totalprice) from orders order by 1}
+ *
+ *
+ * could be converted to
+ * {@code
+ * select max(totalprice) from orders}
+ *
+ *
+ * @see CoreRules#SORT_REMOVE_REDUNDANT
+ */
+@Value.Enclosing
+public class SortRemoveRedundantRule
+extends RelRule
+implements TransformationRule {
+ protected SortRemoveRedundantRule(final SortRemoveRedundantRule.Config
config) {
+super(config);
+ }
+
+ @Override public void onMatch(final RelOptRuleCall call) {
+final Sort sort = call.rel(0);
+if (sort.offset != null || sort.fetch != null) {
+ // Don't remove sort if would also remove OFFSET or LIMIT.
Review Comment:
Minor comment, this description is not clear. I would use 'Don't remove sort
if it has explicit OFFSET and LIMIT'.
--
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 #3317: [CALCITE-5826] Add FIND_IN_SET function (enabled in Hive and Spark library)
sonarcloud[bot] commented on PR #3317: URL: https://github.com/apache/calcite/pull/3317#issuecomment-1726720147 Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_calcite=3317) [](https://sonarcloud.io/project/issues?id=apache_calcite=3317=false=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite=3317=false=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite=3317=false=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite=3317=false=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_calcite=3317=false=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite=3317=false=VULNERABILITY) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3317=false=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3317=false=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3317=false=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/issues?id=apache_calcite=3317=false=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_calcite=3317=false=CODE_SMELL) [2 Code Smells](https://sonarcloud.io/project/issues?id=apache_calcite=3317=false=CODE_SMELL) [](https://sonarcloud.io/component_measures?id=apache_calcite=3317=new_coverage=list) [98.0% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite=3317=new_coverage=list) [](https://sonarcloud.io/component_measures?id=apache_calcite=3317=new_duplicated_lines_density=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite=3317=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 #3421: [CALCITE-5982] Allow overloading the created enumerable in CalciteMetaImpl when calling getTables() or getColumns()
sonarcloud[bot] commented on PR #3421: URL: https://github.com/apache/calcite/pull/3421#issuecomment-1726692918 Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_calcite=3421) [](https://sonarcloud.io/project/issues?id=apache_calcite=3421=false=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite=3421=false=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite=3421=false=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite=3421=false=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_calcite=3421=false=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite=3421=false=VULNERABILITY) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3421=false=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3421=false=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3421=false=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/issues?id=apache_calcite=3421=false=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_calcite=3421=false=CODE_SMELL) [5 Code Smells](https://sonarcloud.io/project/issues?id=apache_calcite=3421=false=CODE_SMELL) [](https://sonarcloud.io/component_measures?id=apache_calcite=3421=new_coverage=list) [83.3% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite=3421=new_coverage=list) [](https://sonarcloud.io/component_measures?id=apache_calcite=3421=new_duplicated_lines_density=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite=3421=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
[calcite] branch main updated: [CALCITE-5961] Type inference of ARRAY_COMPACT is incorrect
This is an automated email from the ASF dual-hosted git repository.
xiong 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 abb3e5be43 [CALCITE-5961] Type inference of ARRAY_COMPACT is incorrect
abb3e5be43 is described below
commit abb3e5be43c9d030bea6489cc8ee42a2905fc4e8
Author: Ran Tao
AuthorDate: Thu Aug 31 20:18:12 2023 +0800
[CALCITE-5961] Type inference of ARRAY_COMPACT is incorrect
---
.../org/apache/calcite/sql/fun/SqlLibraryOperators.java | 17 -
.../java/org/apache/calcite/test/SqlOperatorTest.java | 10 +++---
2 files changed, 23 insertions(+), 4 deletions(-)
diff --git
a/core/src/main/java/org/apache/calcite/sql/fun/SqlLibraryOperators.java
b/core/src/main/java/org/apache/calcite/sql/fun/SqlLibraryOperators.java
index f850ab1cc0..2835184788 100644
--- a/core/src/main/java/org/apache/calcite/sql/fun/SqlLibraryOperators.java
+++ b/core/src/main/java/org/apache/calcite/sql/fun/SqlLibraryOperators.java
@@ -1093,11 +1093,26 @@ public abstract class SqlLibraryOperators {
SqlLibraryOperators::arrayAppendPrependReturnType,
OperandTypes.ARRAY_ELEMENT);
+ @SuppressWarnings("argument.type.incompatible")
+ private static RelDataType arrayCompactReturnType(SqlOperatorBinding
opBinding) {
+final RelDataType arrayType = opBinding.collectOperandTypes().get(0);
+if (arrayType.getSqlTypeName() == SqlTypeName.NULL) {
+ return arrayType;
+}
+RelDataType type = arrayType.getComponentType();
+// force set nullable=false, and there are no side effects for 'NULL' type
+if (type != null && type.isNullable()) {
+ type = opBinding.getTypeFactory().createTypeWithNullability(type, false);
+}
+requireNonNull(type, "inferred array element type");
+return SqlTypeUtil.createArrayType(opBinding.getTypeFactory(), type,
arrayType.isNullable());
+ }
+
/** The "ARRAY_COMPACT(array)" function. */
@LibraryOperator(libraries = {SPARK})
public static final SqlFunction ARRAY_COMPACT =
SqlBasicFunction.create(SqlKind.ARRAY_COMPACT,
- ReturnTypes.ARG0_NULLABLE,
+ SqlLibraryOperators::arrayCompactReturnType,
OperandTypes.ARRAY);
/** The "ARRAY_CONCAT(array [, array]*)" function. */
diff --git a/testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java
b/testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java
index e4b774ac59..f322eb6b9c 100644
--- a/testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java
+++ b/testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java
@@ -5970,11 +5970,15 @@ public class SqlOperatorTest {
final SqlOperatorFixture f = f0.withLibrary(SqlLibrary.SPARK);
f.checkScalar("array_compact(array[null, 1, null, 2])", "[1, 2]",
-"INTEGER ARRAY NOT NULL");
+"INTEGER NOT NULL ARRAY NOT NULL");
+f.checkScalar("array_compact(array[1, 2])", "[1, 2]",
+"INTEGER NOT NULL ARRAY NOT NULL");
+f.checkScalar("array_compact(array[null, 'hello', null, 'world'])",
"[hello, world]",
+"CHAR(5) NOT NULL ARRAY NOT NULL");
+f.checkScalar("array_compact(array['hello', 'world'])", "[hello, world]",
+"CHAR(5) NOT NULL ARRAY NOT NULL");
f.checkScalar("array_compact(array[null])", "[]",
"NULL ARRAY NOT NULL");
-f.checkScalar("array_compact(array(null))", "[]",
-"NULL ARRAY NOT NULL");
f.checkScalar("array_compact(array())", "[]",
"UNKNOWN NOT NULL ARRAY NOT NULL");
f.checkNull("array_compact(null)");
[GitHub] [calcite] NobiGo merged pull request #3405: [CALCITE-5961] Type inference of ARRAY_COMPACT is incorrect
NobiGo merged PR #3405: URL: https://github.com/apache/calcite/pull/3405 -- 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 pull request #3421: [CALCITE-5982] Allow overloading the created enumerable in CalciteMetaImpl when calling getTables() or getColumns()
tanclary commented on PR #3421: URL: https://github.com/apache/calcite/pull/3421#issuecomment-1726617642 I think some of the code smells on this might be helpful suggestions. Like marking some of the variables as final or protected. Of course maybe they're irrelevant. -- 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 #3421: [CALCITE-5982] Allow overloading the created enumerable in CalciteMetaImpl when calling getTables() or getColumns()
sonarcloud[bot] commented on PR #3421: URL: https://github.com/apache/calcite/pull/3421#issuecomment-1726615819 Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_calcite=3421) [](https://sonarcloud.io/project/issues?id=apache_calcite=3421=false=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite=3421=false=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite=3421=false=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite=3421=false=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_calcite=3421=false=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite=3421=false=VULNERABILITY) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3421=false=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3421=false=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3421=false=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/issues?id=apache_calcite=3421=false=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_calcite=3421=false=CODE_SMELL) [14 Code Smells](https://sonarcloud.io/project/issues?id=apache_calcite=3421=false=CODE_SMELL) [](https://sonarcloud.io/component_measures?id=apache_calcite=3421=new_coverage=list) [83.3% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite=3421=new_coverage=list) [](https://sonarcloud.io/component_measures?id=apache_calcite=3421=new_duplicated_lines_density=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite=3421=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 #3317: [CALCITE-5826] Add FIND_IN_SET function (enabled in Hive and Spark library)
sonarcloud[bot] commented on PR #3317: URL: https://github.com/apache/calcite/pull/3317#issuecomment-1725927416 Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_calcite=3317) [](https://sonarcloud.io/project/issues?id=apache_calcite=3317=false=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite=3317=false=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite=3317=false=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite=3317=false=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_calcite=3317=false=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite=3317=false=VULNERABILITY) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3317=false=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3317=false=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3317=false=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/issues?id=apache_calcite=3317=false=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_calcite=3317=false=CODE_SMELL) [2 Code Smells](https://sonarcloud.io/project/issues?id=apache_calcite=3317=false=CODE_SMELL) [](https://sonarcloud.io/component_measures?id=apache_calcite=3317=new_coverage=list) [98.0% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite=3317=new_coverage=list) [](https://sonarcloud.io/component_measures?id=apache_calcite=3317=new_duplicated_lines_density=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite=3317=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 #3417: [CALCITE-5993] Add CODE_POINTS_TO_STRING, TO_CODE_POINTS function (en…
sonarcloud[bot] commented on PR #3417: URL: https://github.com/apache/calcite/pull/3417#issuecomment-1725904459 Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_calcite=3417) [](https://sonarcloud.io/project/issues?id=apache_calcite=3417=false=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite=3417=false=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite=3417=false=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite=3417=false=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_calcite=3417=false=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite=3417=false=VULNERABILITY) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3417=false=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3417=false=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3417=false=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/issues?id=apache_calcite=3417=false=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_calcite=3417=false=CODE_SMELL) [3 Code Smells](https://sonarcloud.io/project/issues?id=apache_calcite=3417=false=CODE_SMELL) [](https://sonarcloud.io/component_measures?id=apache_calcite=3417=new_coverage=list) [97.9% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite=3417=new_coverage=list) [](https://sonarcloud.io/component_measures?id=apache_calcite=3417=new_duplicated_lines_density=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite=3417=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 #3317: [CALCITE-5826] Add FIND_IN_SET function (enabled in Hive and Spark library)
sonarcloud[bot] commented on PR #3317: URL: https://github.com/apache/calcite/pull/3317#issuecomment-1725897584 Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_calcite=3317) [](https://sonarcloud.io/project/issues?id=apache_calcite=3317=false=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite=3317=false=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite=3317=false=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite=3317=false=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_calcite=3317=false=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite=3317=false=VULNERABILITY) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3317=false=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3317=false=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3317=false=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/issues?id=apache_calcite=3317=false=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_calcite=3317=false=CODE_SMELL) [2 Code Smells](https://sonarcloud.io/project/issues?id=apache_calcite=3317=false=CODE_SMELL) [](https://sonarcloud.io/component_measures?id=apache_calcite=3317=new_coverage=list) [98.0% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite=3317=new_coverage=list) [](https://sonarcloud.io/component_measures?id=apache_calcite=3317=new_duplicated_lines_density=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite=3317=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] macroguo-ghy commented on a diff in pull request #3417: [CALCITE-5993] Add CODE_POINTS_TO_STRING, TO_CODE_POINTS function (en…
macroguo-ghy commented on code in PR #3417:
URL: https://github.com/apache/calcite/pull/3417#discussion_r1330318436
##
core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java:
##
@@ -1130,14 +1130,70 @@ public static String charFromUtf8(int n) {
assert codePoint instanceof Number;
long cp = ((Number) codePoint).longValue();
if (cp < 0 || cp > 255) {
-throw RESOURCE.inputArgumentsOfCodePointsToBytesOutOfRange(cp).ex();
+throw RESOURCE.inputArgumentsOfFunctionOutOfRange(
+"CODE_POINTS_TO_BYTES", cp, "[0, 255]").ex();
}
bytes[i] = (byte) cp;
}
return new ByteString(bytes);
}
+ /**
+ * SQL CODE_POINTS_TO_STRING function.
Review Comment:
Fixed.
--
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 #3418: [CALCITE-5994] Add optimization rule to remove Sort when its input's row number is less or equal to one
rubenada commented on PR #3418: URL: https://github.com/apache/calcite/pull/3418#issuecomment-1725832445 Thanks @LakeShen , lgtm. Could you please squash commits into a single one? I'll merge the PR in the coming days if no further remark appears. -- 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 #3318: [CALCITE-5825] Add URL_ENCODE and URL_DECODE function (enabled in Spark library)
sonarcloud[bot] commented on PR #3318: URL: https://github.com/apache/calcite/pull/3318#issuecomment-1725819792 Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_calcite=3318) [](https://sonarcloud.io/project/issues?id=apache_calcite=3318=false=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite=3318=false=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite=3318=false=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite=3318=false=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_calcite=3318=false=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite=3318=false=VULNERABILITY) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3318=false=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3318=false=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3318=false=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/issues?id=apache_calcite=3318=false=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_calcite=3318=false=CODE_SMELL) [2 Code Smells](https://sonarcloud.io/project/issues?id=apache_calcite=3318=false=CODE_SMELL) [](https://sonarcloud.io/component_measures?id=apache_calcite=3318=new_coverage=list) [89.2% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite=3318=new_coverage=list) [](https://sonarcloud.io/component_measures?id=apache_calcite=3318=new_duplicated_lines_density=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite=3318=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] LakeShen commented on pull request #3379: [CALCITE-5927] LoptOptimizeJoinRule miss removable self-join when col…
LakeShen commented on PR #3379: URL: https://github.com/apache/calcite/pull/3379#issuecomment-1725769734 Hi @wq352400523 ,you should rebase the latest code of main branch,after that,your pr's commits and file changed would be clean. -- 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 #3418: [CALCITE-5994] Add optimization rule to remove Sort when its input's row number is less or equal to one
rubenada commented on code in PR #3418:
URL: https://github.com/apache/calcite/pull/3418#discussion_r1330175194
##
core/src/main/java/org/apache/calcite/rel/rules/CoreRules.java:
##
@@ -715,6 +715,11 @@ private CoreRules() {}
public static final SortRemoveConstantKeysRule SORT_REMOVE_CONSTANT_KEYS =
SortRemoveConstantKeysRule.Config.DEFAULT.toRule();
+ /** Rule that removes redundant {@link Sort} if its input max row number
+ * is less than or equal one. */
+ public static final RemoveRedundantSort REMOVE_REDUNDANT_SORT =
Review Comment:
Ok, sounds good !
--
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] LakeShen commented on pull request #3418: [CALCITE-5994] Add optimization rule to remove Sort when its input's row number is less or equal to one
LakeShen commented on PR #3418: URL: https://github.com/apache/calcite/pull/3418#issuecomment-1725509678 > Overall looks good @LakeShen , I left some minor comments. Also, I think the Jira/PR title (and hence the commit message and tests javadocs) could be improved (IMO it should mention the word "rule"). What about something like "Add optimization rule to remove Sort when its input's row number is less or equal to one" ? Hi @rubenada ,thank you for your review suggestions. I have modified according to your suggestions. If you have time, please help me review it again -- 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] chucheng92 commented on a diff in pull request #3395: [CALCITE-5948] Explicit casting should be made if the type of an element in ARRAY/MAP not equals with the derived component typ
chucheng92 commented on code in PR #3395: URL: https://github.com/apache/calcite/pull/3395#discussion_r132782 ## core/src/test/resources/sql/misc.iq: ## @@ -2167,12 +2167,12 @@ select array[1,null,2] as a from (values (1)); values array['a',null,'bcd'], array['efgh']; -++ -| EXPR$0 | -++ -| [a, null, bcd] | -| [efgh] | -++ ++--+ +| EXPR$0 | ++--+ +| [a , null, bcd] | Review Comment: @JiajunBernoulli hi, Jiajun. If you have time, could you help to review it again? ## core/src/test/resources/sql/misc.iq: ## @@ -2167,12 +2167,12 @@ select array[1,null,2] as a from (values (1)); values array['a',null,'bcd'], array['efgh']; -++ -| EXPR$0 | -++ -| [a, null, bcd] | -| [efgh] | -++ ++--+ +| EXPR$0 | ++--+ +| [a , null, bcd] | Review Comment: @JiajunBernoulli hi, Jiajun. If you have time, could you help to review it again? thanks -- 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 #3418: [CALCITE-5994] Add optimization rule to remove Sort when its input's row number is less or equal to one
sonarcloud[bot] commented on PR #3418: URL: https://github.com/apache/calcite/pull/3418#issuecomment-1725360946 Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_calcite=3418) [](https://sonarcloud.io/project/issues?id=apache_calcite=3418=false=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite=3418=false=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite=3418=false=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite=3418=false=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_calcite=3418=false=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite=3418=false=VULNERABILITY) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3418=false=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3418=false=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3418=false=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/issues?id=apache_calcite=3418=false=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_calcite=3418=false=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_calcite=3418=false=CODE_SMELL) [](https://sonarcloud.io/component_measures?id=apache_calcite=3418=new_coverage=list) [87.0% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite=3418=new_coverage=list) [](https://sonarcloud.io/component_measures?id=apache_calcite=3418=new_duplicated_lines_density=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite=3418=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-avatica] zstan closed pull request #225: [CALCITE-5957] Date representation string without precede 0 is acceptable
zstan closed pull request #225: [CALCITE-5957] Date representation string without precede 0 is acceptable URL: https://github.com/apache/calcite-avatica/pull/225 -- 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] LakeShen commented on pull request #3418: [CALCITE-5994] Add the optimize for removing the redundant order by when its source row number is less than or equal 1
LakeShen commented on PR #3418: URL: https://github.com/apache/calcite/pull/3418#issuecomment-1725303854 > Add optimization rule to remove Sort when its input's row number is less or equal to one Yes, it makes sense for me,I will take 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] LakeShen commented on a diff in pull request #3418: [CALCITE-5994] Add the optimize for removing the redundant order by when its source row number is less than or equal 1
LakeShen commented on code in PR #3418:
URL: https://github.com/apache/calcite/pull/3418#discussion_r1329956216
##
core/src/main/java/org/apache/calcite/rel/rules/CoreRules.java:
##
@@ -715,6 +715,11 @@ private CoreRules() {}
public static final SortRemoveConstantKeysRule SORT_REMOVE_CONSTANT_KEYS =
SortRemoveConstantKeysRule.Config.DEFAULT.toRule();
+ /** Rule that removes redundant {@link Sort} if its input max row number
+ * is less than or equal one. */
+ public static final RemoveRedundantSort REMOVE_REDUNDANT_SORT =
Review Comment:
Hi @rubenada ,thank you very much for your review.
My idea was to change the name to SortRemoveRedundantRule,I also plan to add
more optimization logic to this rule in the future,such as redundant
limit,redundant topn
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] LakeShen commented on a diff in pull request #3418: [CALCITE-5994] Add the optimize for removing the redundant order by when its source row number is less than or equal 1
LakeShen commented on code in PR #3418:
URL: https://github.com/apache/calcite/pull/3418#discussion_r1329950552
##
core/src/main/java/org/apache/calcite/rel/rules/CoreRules.java:
##
@@ -715,6 +715,11 @@ private CoreRules() {}
public static final SortRemoveConstantKeysRule SORT_REMOVE_CONSTANT_KEYS =
SortRemoveConstantKeysRule.Config.DEFAULT.toRule();
+ /** Rule that removes redundant {@link Sort} if its input max row number
+ * is less than or equal one. */
Review Comment:
Yes,it makes sense for me
--
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 #3418: [CALCITE-5994] Add the optimize for removing the redundant order by when its source row number is less than or equal 1
rubenada commented on code in PR #3418:
URL: https://github.com/apache/calcite/pull/3418#discussion_r1329887547
##
core/src/main/java/org/apache/calcite/rel/rules/CoreRules.java:
##
@@ -715,6 +715,11 @@ private CoreRules() {}
public static final SortRemoveConstantKeysRule SORT_REMOVE_CONSTANT_KEYS =
SortRemoveConstantKeysRule.Config.DEFAULT.toRule();
+ /** Rule that removes redundant {@link Sort} if its input max row number
+ * is less than or equal one. */
+ public static final RemoveRedundantSort REMOVE_REDUNDANT_SORT =
Review Comment:
We already have SortRemoveRule (removes a Sort if its input is already
sorted) and SortRemoveConstantKeysRule (removes keys from Sort if that are
constant, or removes the entire Sort if all keys are constant).
To be consistent, I think the new rule should keep this "name pattern". What
about something along the lines "SortRemoveSingleRowRule", or something like
that?
UPDATE: on a second thought, perhaps this new rule should not be on a
dedicated class, bur rather inside PruneEmptyRules (like
SORT_FETCH_ZERO_INSTANCE)? Not sure about 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] rubenada commented on pull request #3418: [CALCITE-5994] Add the optimize for removing the redundant order by when its source row number is less than or equal 1
rubenada commented on PR #3418: URL: https://github.com/apache/calcite/pull/3418#issuecomment-1725222676 Overall looks good @LakeShen , I left some minor comments. Also, I think the Jira/PR title (and hence the commit message and tests javadocs) could be improved (IMO it should mention the word "rule"). What about something like "Add optimization rule to remove Sort when its input's row number is less or equal to one" ? -- 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 #3418: [CALCITE-5994] Add the optimize for removing the redundant order by when its source row number is less than or equal 1
rubenada commented on code in PR #3418:
URL: https://github.com/apache/calcite/pull/3418#discussion_r1329887547
##
core/src/main/java/org/apache/calcite/rel/rules/CoreRules.java:
##
@@ -715,6 +715,11 @@ private CoreRules() {}
public static final SortRemoveConstantKeysRule SORT_REMOVE_CONSTANT_KEYS =
SortRemoveConstantKeysRule.Config.DEFAULT.toRule();
+ /** Rule that removes redundant {@link Sort} if its input max row number
+ * is less than or equal one. */
+ public static final RemoveRedundantSort REMOVE_REDUNDANT_SORT =
Review Comment:
We already have SortRemoveRule (removes a Sort if its input is already
sorted) and SortRemoveConstantKeysRule (removes keys from Sort if that are
constant, or removes the entire Sort if all keys are constant).
To be consistent, I think the new rule should keep this "name pattern". What
about something along the lines "SortRemoveSingleRowRule", or something like
that?
--
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 #3418: [CALCITE-5994] Add the optimize for removing the redundant order by when its source row number is less than or equal 1
rubenada commented on code in PR #3418:
URL: https://github.com/apache/calcite/pull/3418#discussion_r1329885385
##
core/src/main/java/org/apache/calcite/rel/rules/CoreRules.java:
##
@@ -715,6 +715,11 @@ private CoreRules() {}
public static final SortRemoveConstantKeysRule SORT_REMOVE_CONSTANT_KEYS =
SortRemoveConstantKeysRule.Config.DEFAULT.toRule();
+ /** Rule that removes redundant {@link Sort} if its input max row number
+ * is less than or equal one. */
Review Comment:
nitipck: "... or equal **to** one"
--
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 #3432: [CALCITE-5995] add cache to dejsonize function in JsonFunctions
sonarcloud[bot] commented on PR #3432: URL: https://github.com/apache/calcite/pull/3432#issuecomment-1725084537 Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_calcite=3432) [](https://sonarcloud.io/project/issues?id=apache_calcite=3432=false=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite=3432=false=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite=3432=false=BUG) [](https://sonarcloud.io/project/issues?id=apache_calcite=3432=false=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_calcite=3432=false=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite=3432=false=VULNERABILITY) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3432=false=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3432=false=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3432=false=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/issues?id=apache_calcite=3432=false=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_calcite=3432=false=CODE_SMELL) [9 Code Smells](https://sonarcloud.io/project/issues?id=apache_calcite=3432=false=CODE_SMELL) [](https://sonarcloud.io/component_measures?id=apache_calcite=3432=new_coverage=list) [100.0% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite=3432=new_coverage=list) [](https://sonarcloud.io/component_measures?id=apache_calcite=3432=new_duplicated_lines_density=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite=3432=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
