Re: [PR] [CALCITE-6251] innerEnumerator in EnumerableDefaults::correlateBatchJoin is not closed [calcite]
rubenada commented on code in PR #3675:
URL: https://github.com/apache/calcite/pull/3675#discussion_r1482547365
##
linq4j/src/main/java/org/apache/calcite/linq4j/EnumerableDefaults.java:
##
@@ -1799,11 +1801,16 @@ private boolean innerHasNext() {
i = -1;
}
- @Override public void close() {
-outerEnumerator.close();
+ private void closeInner() {
Review Comment:
Interesting point.
Maybe I'm wrong, but my understanding is that our enumerators are not
thread-safe, so if a 3rd party uses them on a multi-threaded environment, they
would need to handle synchronization on their end.
--
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
Re: [PR] [CALCITE-6251] innerEnumerator in EnumerableDefaults::correlateBatchJoin is not closed [calcite]
JiajunBernoulli commented on code in PR #3675:
URL: https://github.com/apache/calcite/pull/3675#discussion_r1482360787
##
linq4j/src/main/java/org/apache/calcite/linq4j/EnumerableDefaults.java:
##
@@ -1799,11 +1801,16 @@ private boolean innerHasNext() {
i = -1;
}
- @Override public void close() {
-outerEnumerator.close();
+ private void closeInner() {
Review Comment:
Should we use `synchronized`?
--
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
Re: [PR] [CALCITE-6214] Remove DISTINCT in aggregate function if field is unique [calcite]
sonarcloud[bot] commented on PR #3641: URL: https://github.com/apache/calcite/pull/3641#issuecomment-1933291861 ## [](https://sonarcloud.io/dashboard?id=apache_calcite=3641) **Quality Gate passed** Kudos, no new issues were introduced! [0 New issues](https://sonarcloud.io/project/issues?id=apache_calcite=3641=false=true) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3641=false=true) [96.3% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite=3641=new_coverage=list) [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite=3641=new_duplicated_lines_density=list) [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_calcite=3641) -- 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
Re: [PR] [CALCITE-6214] Remove DISTINCT in aggregate function if field is unique [calcite]
JiajunBernoulli commented on PR #3641:
URL: https://github.com/apache/calcite/pull/3641#issuecomment-1933283166
> Another question I have is whether this should be as you wrote it in
RelBuilder, or it should be a rewrite rule in the optimizer. It is a matter of
design choice rather than of correctness.
Thank you for your review.
1. RelBuilder optimization can reuse subexpressions.
- Using RelBuilder
```
LogicalProject(DEPTNO=[$0], CDS=[$1], CS=[$2], SDS=[$3], SS=[$3]) -- SDS is
same as SS
LogicalAggregate(group=[{0}], CDS=[COUNT($1)], CS=[COUNT()], SDS=[SUM($1)])
LogicalAggregate(group=[{0, 1}])
LogicalProject(DEPTNO=[$7], SAL=[$5])
LogicalTableScan(table=[[CATALOG, SALES, EMP]])
```
- Using Rule
```
LogicalProject(DEPTNO=[$0], CDS=[$1], CS=[$2], SDS=[$3], SS=[$4]) -- SDS is
same as SS
LogicalAggregate(group=[{0}], CDS=[COUNT($1)], CS=[COUNT()],
SDS=[SUM($1)], SS=[SUM($1)])
LogicalAggregate(group=[{0, 1}])
LogicalProject(DEPTNO=[$7], SAL=[$5])
LogicalTableScan(table=[[CATALOG, SALES, EMP]])
```
We need other rules to remove same function.
2. RelBuilder is easier to use than Rule.
- RelBuilder: `withRedundantDistinct(flag)` to enable or disable.
- Rule: Add or remove rule in programs.
--
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
Re: [PR] [CALCITE-6214] Remove DISTINCT in aggregate function if field is unique [calcite]
JiajunBernoulli commented on code in PR #3641: URL: https://github.com/apache/calcite/pull/3641#discussion_r1482348351 ## core/src/main/java/org/apache/calcite/tools/RelBuilder.java: ## @@ -2525,6 +2529,29 @@ private RelBuilder aggregate_(GroupKeyImpl groupKey, return project(projects.transform((i, name) -> aliasMaybe(field(i), name))); } + /** + * Removed redundant distinct if an input is already unique. Review Comment: Renamed to `removeRedundantAggregateDistinct`. -- 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
Re: [PR] [CALCITE-6214] Remove DISTINCT in aggregate function if field is unique [calcite]
JiajunBernoulli commented on code in PR #3641:
URL: https://github.com/apache/calcite/pull/3641#discussion_r1482348168
##
core/src/main/java/org/apache/calcite/tools/RelBuilder.java:
##
@@ -4902,6 +4929,18 @@ public interface Config {
/** Sets {@link #convertCorrelateToJoin()}. */
Config withConvertCorrelateToJoin(boolean convertCorrelateToJoin);
+
+/** Whether to save the distinct if we know that the input is
+ * already unique; default true. */
+@Value.Default
+default boolean redundantDistinct() {
Review Comment:
Good catch.
I changed default value, now we can set `true` to optimize. (Default is
`false`)
--
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
Re: [PR] [CALCITE-6214] Remove DISTINCT in aggregate function if field is unique [calcite]
JiajunBernoulli commented on code in PR #3641:
URL: https://github.com/apache/calcite/pull/3641#discussion_r1482347625
##
core/src/test/resources/org/apache/calcite/test/SqlToRelConverterTest.xml:
##
@@ -6343,6 +6343,161 @@ LogicalAggregate(group=[{0}], CNT=[COUNT()])
LogicalFilter(condition=[>(ITEM($0, 'N_NATIONKEY'), 5)])
LogicalProject(**=[$0])
LogicalTableScan(table=[[CATALOG, SALES, NATION]])
+]]>
+
+
+
Review Comment:
Forgot to delete.
Removed 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
Re: [PR] [CALCITE-6214] Remove DISTINCT in aggregate function if field is unique [calcite]
JiajunBernoulli commented on code in PR #3641:
URL: https://github.com/apache/calcite/pull/3641#discussion_r1482347529
##
core/src/test/resources/org/apache/calcite/test/SqlToRelConverterTest.xml:
##
@@ -6343,6 +6343,161 @@ LogicalAggregate(group=[{0}], CNT=[COUNT()])
LogicalFilter(condition=[>(ITEM($0, 'N_NATIONKEY'), 5)])
LogicalProject(**=[$0])
LogicalTableScan(table=[[CATALOG, SALES, NATION]])
+]]>
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
Re: [PR] [CALCITE-6238] Exception while evaluating ROUND/TRUNCATE functions [calcite]
sonarcloud[bot] commented on PR #3672: URL: https://github.com/apache/calcite/pull/3672#issuecomment-1933263478 ## [](https://sonarcloud.io/dashboard?id=apache_calcite=3672) **Quality Gate passed** The SonarCloud Quality Gate passed, but some issues were introduced. [1 New issue](https://sonarcloud.io/project/issues?id=apache_calcite=3672=false=true) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3672=false=true) [100.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite=3672=new_coverage=list) [9.5% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite=3672=new_duplicated_lines_density=list) [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_calcite=3672) -- 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
Re: [PR] [CALCITE-6214] Remove DISTINCT in aggregate function if field is unique [calcite]
mihaibudiu commented on code in PR #3641:
URL: https://github.com/apache/calcite/pull/3641#discussion_r1482331799
##
core/src/test/resources/org/apache/calcite/test/SqlToRelConverterTest.xml:
##
@@ -6343,6 +6343,161 @@ LogicalAggregate(group=[{0}], CNT=[COUNT()])
LogicalFilter(condition=[>(ITEM($0, 'N_NATIONKEY'), 5)])
LogicalProject(**=[$0])
LogicalTableScan(table=[[CATALOG, SALES, NATION]])
+]]>
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
Re: [PR] [CALCITE-6214] Remove DISTINCT in aggregate function if field is unique [calcite]
JiajunBernoulli commented on code in PR #3641:
URL: https://github.com/apache/calcite/pull/3641#discussion_r1482328835
##
core/src/test/resources/org/apache/calcite/test/SqlToRelConverterTest.xml:
##
@@ -6343,6 +6343,161 @@ LogicalAggregate(group=[{0}], CNT=[COUNT()])
LogicalFilter(condition=[>(ITEM($0, 'N_NATIONKEY'), 5)])
LogicalProject(**=[$0])
LogicalTableScan(table=[[CATALOG, SALES, NATION]])
+]]>
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
Re: [PR] [CALCITE-6238] Exception while evaluating ROUND/TRUNCATE functions [calcite]
mihaibudiu commented on code in PR #3672:
URL: https://github.com/apache/calcite/pull/3672#discussion_r1482322829
##
core/src/main/java/org/apache/calcite/sql/fun/SqlStdOperatorTable.java:
##
@@ -1802,11 +1802,11 @@ public class SqlStdOperatorTable extends
ReflectiveSqlOperatorTable {
OperandTypes.NUMERIC,
SqlFunctionCategory.NUMERIC);
- /** The {@code TRUNCATE(numeric [, numeric])} function. */
+ /** The {@code TRUNCATE(numeric [, integer])} function. */
public static final SqlBasicFunction TRUNCATE =
SqlBasicFunction.create("TRUNCATE",
ReturnTypes.ARG0_NULLABLE,
- OperandTypes.NUMERIC_OPTIONAL_INTEGER,
+
OperandTypes.NUMERIC.or(OperandTypes.NUMERIC_INT32.apply("TRUNCATE")),
Review Comment:
I have renamed the JIRA case and the PR, I will fix the commit message when
I squash.
I have added the suggested new unit test.
--
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
Re: [PR] [CALCITE-6249] RelNode::estimatedRowCount should not be used in computeSelfCost [calcite]
JiajunBernoulli commented on code in PR #3674:
URL: https://github.com/apache/calcite/pull/3674#discussion_r1482320473
##
core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableBatchNestedLoopJoin.java:
##
@@ -124,8 +124,8 @@ public static EnumerableBatchNestedLoopJoin create(
final RelMetadataQuery mq) {
double rowCount = mq.getRowCount(this);
-final double rightRowCount = right.estimateRowCount(mq);
Review Comment:
+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
(calcite) branch main updated: [CALCITE-6228] ELEMENT function infers incorrect return type
This is an automated email from the ASF dual-hosted git repository.
mbudiu 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 f837ffa877 [CALCITE-6228] ELEMENT function infers incorrect return type
f837ffa877 is described below
commit f837ffa877c5b7f795fd175532f6ec9db4440533
Author: Mihai Budiu
AuthorDate: Wed Feb 7 06:41:23 2024 -0800
[CALCITE-6228] ELEMENT function infers incorrect return type
Signed-off-by: Mihai Budiu
---
.../org/apache/calcite/sql/fun/SqlStdOperatorTable.java| 2 +-
.../main/java/org/apache/calcite/sql/type/ReturnTypes.java | 7 ---
.../java/org/apache/calcite/test/SqlValidatorTest.java | 14 +++---
.../main/java/org/apache/calcite/test/SqlOperatorTest.java | 2 +-
4 files changed, 13 insertions(+), 12 deletions(-)
diff --git
a/core/src/main/java/org/apache/calcite/sql/fun/SqlStdOperatorTable.java
b/core/src/main/java/org/apache/calcite/sql/fun/SqlStdOperatorTable.java
index 4b07e5ad01..16151967a3 100644
--- a/core/src/main/java/org/apache/calcite/sql/fun/SqlStdOperatorTable.java
+++ b/core/src/main/java/org/apache/calcite/sql/fun/SqlStdOperatorTable.java
@@ -2103,7 +2103,7 @@ public class SqlStdOperatorTable extends
ReflectiveSqlOperatorTable {
*/
public static final SqlFunction ELEMENT =
SqlBasicFunction.create("ELEMENT",
- ReturnTypes.MULTISET_ELEMENT_NULLABLE,
+ ReturnTypes.MULTISET_ELEMENT_FORCE_NULLABLE,
OperandTypes.COLLECTION);
/**
diff --git a/core/src/main/java/org/apache/calcite/sql/type/ReturnTypes.java
b/core/src/main/java/org/apache/calcite/sql/type/ReturnTypes.java
index 5924f001d0..f8696893b0 100644
--- a/core/src/main/java/org/apache/calcite/sql/type/ReturnTypes.java
+++ b/core/src/main/java/org/apache/calcite/sql/type/ReturnTypes.java
@@ -615,10 +615,11 @@ public abstract class ReturnTypes {
ARG0.andThen(SqlTypeTransforms.TO_MULTISET);
/**
- * Returns the element type of a MULTISET.
+ * Returns the element type of a MULTISET, with nullability enforced.
*/
- public static final SqlReturnTypeInference MULTISET_ELEMENT_NULLABLE =
- MULTISET.andThen(SqlTypeTransforms.TO_COLLECTION_ELEMENT_TYPE);
+ public static final SqlReturnTypeInference MULTISET_ELEMENT_FORCE_NULLABLE =
+ MULTISET.andThen(SqlTypeTransforms.TO_COLLECTION_ELEMENT_TYPE)
+ .andThen(SqlTypeTransforms.FORCE_NULLABLE);
/**
* Same as {@link #MULTISET} but returns with nullability if any of the
diff --git a/core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java
b/core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java
index 8fead2ce72..f888d4e3ea 100644
--- a/core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java
+++ b/core/src/test/java/org/apache/calcite/test/SqlValidatorTest.java
@@ -1925,17 +1925,17 @@ public class SqlValidatorTest extends
SqlValidatorTestCase {
@Test void testElement() {
expr("element(multiset[1])")
-.columnType("INTEGER NOT NULL");
+.columnType("INTEGER");
expr("1.0+element(multiset[1])")
-.columnType("DECIMAL(12, 1) NOT NULL");
+.columnType("DECIMAL(12, 1)");
expr("element(multiset['1'])")
-.columnType("CHAR(1) NOT NULL");
+.columnType("CHAR(1)");
expr("element(multiset[1e-2])")
-.columnType("DOUBLE NOT NULL");
+.columnType("DOUBLE");
expr("element(multiset[multiset[cast(null as tinyint)]])")
-.columnType("TINYINT MULTISET NOT NULL");
-// Test case for https://issues.apache.org/jira/projects/CALCITE/issues/CALCITE-6227;>
-// ELEMENT(NULL) causes an assertion failure.
+.columnType("TINYINT MULTISET");
+// Test case for
https://issues.apache.org/jira/projects/CALCITE/issues/CALCITE-6227
+// ELEMENT(NULL) causes an assertion failure.
expr("element(null)")
.columnType("NULL");
}
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 badde6f889..4b4e8fd4b8 100644
--- a/testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java
+++ b/testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java
@@ -9816,7 +9816,7 @@ public class SqlOperatorTest {
@Test void testElementFunc() {
final SqlOperatorFixture f = fixture();
f.setFor(SqlStdOperatorTable.ELEMENT, VM_FENNEL, VM_JAVA);
-f.checkString("element(multiset['abc'])", "abc", "CHAR(3) NOT NULL");
+f.checkString("element(multiset['abc'])", "abc", "CHAR(3)");
f.checkNull("element(multiset[cast(null as integer)])");
}
Re: [PR] [CALCITE-6228] ELEMENT function infers incorrect return type [calcite]
mihaibudiu merged PR #3650: URL: https://github.com/apache/calcite/pull/3650 -- 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
Re: [PR] [CALCITE-6032] Multilevel correlated query is failing in RelDecorrela… [calcite]
HanumathRao commented on PR #3640: URL: https://github.com/apache/calcite/pull/3640#issuecomment-1932905283 > @HanumathRao could you please elaborate why we need in this PR the changes in `CassandraSchema.java` and `Prepare.java`? Is is related to the decorrelation problem, or could they be part of a separate issue? 1. Changes to Prepare.java file, Yes in a subtle way. The trimUnusedField ,in the prepareSql function call path, is called conditionally based on the configHolder's isTrimUnusedFields flag and due to this it results in trimUnusedFields getting called before decorrelateQuery is called. However, the same functionality in PlannerImpl.rel code path (most calcite users seem to use it), trimUnusedFields is set to false and this makes sure the decorrelateQuery is called before the trimUnusedFields. In both the cases the RelFieldTrimmer program is called in the optimize phase of the query, so essentially fields will get trimmed, but in the case of Prepare (which I believe is less used by users) doing so will result in a tree which will cause other issues due to lack of comprehensive handling of correlate subqueries. Essentially I am trying to make both the paths to behave same. 2. The changes in CassandraSchema.java are done because earlier CassandraSchema Test code loaded the materializations using TRIMMED Hook. As this code path is no more available, I changed it to load on CONVERTED Hook. This way the materializations are available for the optimizations, otherwise the materializations are not loaded and MaterializedView optimization is skipped. Hope the above description made it clear. Please let me know if you have any further questions/suggestions. -- 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
Re: [PR] [CALCITE-6228] ELEMENT function infers incorrect return type [calcite]
sonarcloud[bot] commented on PR #3650: URL: https://github.com/apache/calcite/pull/3650#issuecomment-1932806091 ## [](https://sonarcloud.io/dashboard?id=apache_calcite=3650) **Quality Gate passed** The SonarCloud Quality Gate passed, but some issues were introduced. [1 New issue](https://sonarcloud.io/project/issues?id=apache_calcite=3650=false=true) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3650=false=true) [100.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite=3650=new_coverage=list) [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite=3650=new_duplicated_lines_density=list) [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_calcite=3650) -- 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
Re: [PR] [CALCITE-6249] RelNode::estimatedRowCount should not be used in computeSelfCost [calcite]
tanclary commented on code in PR #3674:
URL: https://github.com/apache/calcite/pull/3674#discussion_r1482010981
##
core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableBatchNestedLoopJoin.java:
##
@@ -124,8 +124,8 @@ public static EnumerableBatchNestedLoopJoin create(
final RelMetadataQuery mq) {
double rowCount = mq.getRowCount(this);
-final double rightRowCount = right.estimateRowCount(mq);
Review Comment:
Can we please write a test for this? If it shouldn't be used then a test
would be helpful to show why, and how this commit corrects 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
Re: [PR] [CALCITE-6228] ELEMENT function infers incorrect return type [calcite]
mihaibudiu commented on code in PR #3650:
URL: https://github.com/apache/calcite/pull/3650#discussion_r1481995618
##
core/src/main/java/org/apache/calcite/sql/type/ReturnTypes.java:
##
@@ -620,6 +620,13 @@ public static SqlCall stripSeparator(SqlCall call) {
public static final SqlReturnTypeInference MULTISET_ELEMENT_NULLABLE =
Review Comment:
I love removing code, I will do that right away.
I will squash the commits optimistically, assuming that it can be merged
after 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
Re: [PR] [CALCITE-6249] RelNode::estimatedRowCount should not be used in computeSelfCost [calcite]
mihaibudiu commented on PR #3674: URL: https://github.com/apache/calcite/pull/3674#issuecomment-1932740859 This looks fine, but no test is affected, so it's hard to gauge the actual impact of this change. -- 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
Re: [PR] [CALCITE-6250] Add hints with restrictions to MongoDB adapter documentation [calcite]
mihaibudiu commented on PR #3676: URL: https://github.com/apache/calcite/pull/3676#issuecomment-1932637903 The rule is for the PR name, the JIRA issue, and the commit message to be the same. -- 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
Re: [PR] [CALCITE-6032] Multilevel correlated query is failing in RelDecorrela… [calcite]
rubenada commented on PR #3640: URL: https://github.com/apache/calcite/pull/3640#issuecomment-1932326838 @HanumathRao could you please elaborate why we need in this PR the changes in `CassandraSchema.java` and `Prepare.java`? Is is related to the decorrelation problem, or could they be part of a separate issue? -- 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
Re: [PR] [CALCITE-6251] innerEnumerator in EnumerableDefaults::correlateBatchJoin is not closed [calcite]
rubenada commented on PR #3675: URL: https://github.com/apache/calcite/pull/3675#issuecomment-1932186954 LGTM -- 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
Re: [PR] [CALCITE-6250] Add hints with restrictions to MongoDB adapter documentation [calcite]
sonarcloud[bot] commented on PR #3676: URL: https://github.com/apache/calcite/pull/3676#issuecomment-1932129472 ## [](https://sonarcloud.io/dashboard?id=apache_calcite=3676) **Quality Gate passed** Kudos, no new issues were introduced! [0 New issues](https://sonarcloud.io/project/issues?id=apache_calcite=3676=false=true) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3676=false=true) No data about Coverage No data about Duplication [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_calcite=3676) -- 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
Re: [PR] [CALCITE-6251] innerEnumerator in EnumerableDefaults::correlateBatchJoin is not closed [calcite]
sonarcloud[bot] commented on PR #3675: URL: https://github.com/apache/calcite/pull/3675#issuecomment-1932096483 ## [](https://sonarcloud.io/dashboard?id=apache_calcite=3675) **Quality Gate passed** The SonarCloud Quality Gate passed, but some issues were introduced. [1 New issue](https://sonarcloud.io/project/issues?id=apache_calcite=3675=false=true) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3675=false=true) [100.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite=3675=new_coverage=list) [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite=3675=new_duplicated_lines_density=list) [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_calcite=3675) -- 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
Re: [PR] [CALCITE-6251] innerEnumerator in EnumerableDefaults::correlateBatchJoin is not closed [calcite]
sonarcloud[bot] commented on PR #3675: URL: https://github.com/apache/calcite/pull/3675#issuecomment-1932076911 ## [](https://sonarcloud.io/dashboard?id=apache_calcite=3675) **Quality Gate passed** Kudos, no new issues were introduced! [0 New issues](https://sonarcloud.io/project/issues?id=apache_calcite=3675=false=true) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3675=false=true) [100.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite=3675=new_coverage=list) [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite=3675=new_duplicated_lines_density=list) [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_calcite=3675) -- 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
Re: [PR] [CALCITE-6251] innerEnumerator in EnumerableDefaults::correlateBatchJoin is not closed [calcite]
kramerul commented on PR #3675: URL: https://github.com/apache/calcite/pull/3675#issuecomment-1932059769 I had to fix also another issue with `@Nullable` -- 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
Re: [PR] [CALCITE-6251] innerEnumerator in EnumerableDefaults::correlateBatchJoin is not closed [calcite]
rubenada commented on PR #3675: URL: https://github.com/apache/calcite/pull/3675#issuecomment-1932044193 @kramerul CI is failing due to commit message: ``` <[invalid git log message '[CALCITE-6251] innerEnumerator in EnumerableDefaults::correlateBatchJoin is not closed'; Message must start with upper-case letter]> ``` I think the simplest would be to amend the commit message and start with uppercase: `[CALCITE-6251] InnerEnumerator in ...` -- 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
[PR] [CALCITE-6251] innerEnumerator in EnumerableDefaults::correlateBatchJoin is not closed [calcite]
kramerul opened a new pull request, #3675: URL: https://github.com/apache/calcite/pull/3675 (no comment) -- 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
Re: [PR] [CALCITE-6239] Add a postgis dialect that supports ST functions [calcite]
sonarcloud[bot] commented on PR #3668: URL: https://github.com/apache/calcite/pull/3668#issuecomment-1931862374 ## [](https://sonarcloud.io/dashboard?id=apache_calcite=3668) **Quality Gate passed** The SonarCloud Quality Gate passed, but some issues were introduced. [3 New issues](https://sonarcloud.io/project/issues?id=apache_calcite=3668=false=true) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3668=false=true) [60.2% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite=3668=new_coverage=list) [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite=3668=new_duplicated_lines_density=list) [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_calcite=3668) -- 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
Re: [PR] [CALCITE-6238] Exception while evaluating ROUND function [calcite]
rubenada commented on code in PR #3672:
URL: https://github.com/apache/calcite/pull/3672#discussion_r1481305092
##
core/src/main/java/org/apache/calcite/sql/fun/SqlStdOperatorTable.java:
##
@@ -1802,11 +1802,11 @@ public class SqlStdOperatorTable extends
ReflectiveSqlOperatorTable {
OperandTypes.NUMERIC,
SqlFunctionCategory.NUMERIC);
- /** The {@code TRUNCATE(numeric [, numeric])} function. */
+ /** The {@code TRUNCATE(numeric [, integer])} function. */
public static final SqlBasicFunction TRUNCATE =
SqlBasicFunction.create("TRUNCATE",
ReturnTypes.ARG0_NULLABLE,
- OperandTypes.NUMERIC_OPTIONAL_INTEGER,
+
OperandTypes.NUMERIC.or(OperandTypes.NUMERIC_INT32.apply("TRUNCATE")),
Review Comment:
I understand that TRUNCATE suffers from the same issue as ROUND. Perhaps we
should enlarge the Jira title (and therefore the commit message) to reflect
this, e.g. `[CALCITE-6238] Exception while evaluating ROUND and TRUNCATE
functions when the second parameter is not an integer`.
Also we should add a new unit test with TRUNCATE, analogous to the new
`round(42, CAST(2 as BIGINT))` test in `SqlOperatorTest.java`.
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
Re: [PR] [CALCITE-6228] ELEMENT function infers incorrect return type [calcite]
rubenada commented on code in PR #3650:
URL: https://github.com/apache/calcite/pull/3650#discussion_r1481299196
##
core/src/main/java/org/apache/calcite/sql/type/ReturnTypes.java:
##
@@ -620,6 +620,13 @@ public static SqlCall stripSeparator(SqlCall call) {
public static final SqlReturnTypeInference MULTISET_ELEMENT_NULLABLE =
Review Comment:
My two cents: in this particular case, I'd vote for removing the (now
unused) `MULTISET_ELEMENT_NULLABLE`.
It is true that it might be used by a 3rd party library, but since the name
was misleading, I think that those potential 3rd party libraries could be
suffering from the same issue as CALCITE-6228, so by having a compilation
problem after the next Calcite upgrade, they'd be force to analyze their usage
of `MULTISET_ELEMENT_NULLABLE` and maybe switch to the new
`MULTISET_ELEMENT_FORCE_NULLABLE` (or define their own `SqlReturnTypeInference`
if they choose so).
--
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
Re: [PR] [CALCITE-6249] RelNode::estimatedRowCount should not be used in computeSelfCost [calcite]
rubenada commented on PR #3674: URL: https://github.com/apache/calcite/pull/3674#issuecomment-1931738219 LGTM -- 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
Re: [PR] [CALCITE-6228] ELEMENT function infers incorrect return type [calcite]
YiwenWu commented on code in PR #3650:
URL: https://github.com/apache/calcite/pull/3650#discussion_r1481239292
##
core/src/main/java/org/apache/calcite/sql/type/ReturnTypes.java:
##
@@ -620,6 +620,13 @@ public static SqlCall stripSeparator(SqlCall call) {
public static final SqlReturnTypeInference MULTISET_ELEMENT_NULLABLE =
Review Comment:
This is a minor suggestion that can be ignored.
I'm not sure about the removal policy either. Not deleting can avoid
version upgrade issues, but maintenance may be lengthy.
--
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
Re: [PR] [CALCITE-6249] RelNode::estimatedRowCount should not be used in computeSelfCost [calcite]
sonarcloud[bot] commented on PR #3674: URL: https://github.com/apache/calcite/pull/3674#issuecomment-1931597625 ## [](https://sonarcloud.io/dashboard?id=apache_calcite=3674) **Quality Gate passed** Kudos, no new issues were introduced! [0 New issues](https://sonarcloud.io/project/issues?id=apache_calcite=3674=false=true) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3674=false=true) [78.6% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite=3674=new_coverage=list) [28.6% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite=3674=new_duplicated_lines_density=list) [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_calcite=3674) -- 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-5647] RelMdPopulationSize should use mq.getRowCount(rel) instead of rel.estimateRowCount(mq)
This is an automated email from the ASF dual-hosted git repository.
rubenql 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 f7069cc524 [CALCITE-5647] RelMdPopulationSize should use
mq.getRowCount(rel) instead of rel.estimateRowCount(mq)
f7069cc524 is described below
commit f7069cc5245c22f816c565669f52b4f30b046f4d
Author: Adam Kennedy
AuthorDate: Fri Apr 14 15:40:06 2023 -0700
[CALCITE-5647] RelMdPopulationSize should use mq.getRowCount(rel) instead
of rel.estimateRowCount(mq)
Use RelMetadataQuery#getRowCount() instead of estimateRowCount()
when calculating RelMdPopulatioSize#getRowCount()
---
.../calcite/rel/metadata/RelMdPopulationSize.java | 2 +-
.../org/apache/calcite/test/RelMetadataTest.java | 39 ++
2 files changed, 40 insertions(+), 1 deletion(-)
diff --git
a/core/src/main/java/org/apache/calcite/rel/metadata/RelMdPopulationSize.java
b/core/src/main/java/org/apache/calcite/rel/metadata/RelMdPopulationSize.java
index 963c3bac5f..bbd4c0fa82 100644
---
a/core/src/main/java/org/apache/calcite/rel/metadata/RelMdPopulationSize.java
+++
b/core/src/main/java/org/apache/calcite/rel/metadata/RelMdPopulationSize.java
@@ -113,7 +113,7 @@ public class RelMdPopulationSize
public Double getPopulationSize(Values rel, RelMetadataQuery mq,
ImmutableBitSet groupKey) {
// assume half the rows are duplicates
-return rel.estimateRowCount(mq) / 2;
+return mq.getRowCount(rel) / 2;
}
public @Nullable Double getPopulationSize(Project rel, RelMetadataQuery mq,
diff --git a/core/src/test/java/org/apache/calcite/test/RelMetadataTest.java
b/core/src/test/java/org/apache/calcite/test/RelMetadataTest.java
index 24155414e6..db1f65c242 100644
--- a/core/src/test/java/org/apache/calcite/test/RelMetadataTest.java
+++ b/core/src/test/java/org/apache/calcite/test/RelMetadataTest.java
@@ -15,6 +15,7 @@
* limitations under the License.
*/
package org.apache.calcite.test;
+
import org.apache.calcite.adapter.enumerable.EnumerableConvention;
import org.apache.calcite.adapter.enumerable.EnumerableLimit;
import org.apache.calcite.adapter.enumerable.EnumerableMergeJoin;
@@ -76,6 +77,7 @@ import
org.apache.calcite.rel.metadata.ReflectiveRelMetadataProvider;
import org.apache.calcite.rel.metadata.RelColumnOrigin;
import org.apache.calcite.rel.metadata.RelMdCollation;
import org.apache.calcite.rel.metadata.RelMdColumnUniqueness;
+import org.apache.calcite.rel.metadata.RelMdPopulationSize;
import org.apache.calcite.rel.metadata.RelMdUtil;
import org.apache.calcite.rel.metadata.RelMetadataProvider;
import org.apache.calcite.rel.metadata.RelMetadataQuery;
@@ -3639,6 +3641,43 @@ public class RelMetadataTest {
is(mq.getPopulationSize(rel, bitSetOf(0;
}
+ /**
+ * Test that RelMdPopulationSize is calculated based on the
RelMetadataQuery#getRowCount().
+ *
+ * @see https://issues.apache.org/jira/browse/CALCITE-5647;>[CALCITE-5647]
+ */
+ @Test public void testPopulationSizeFromValues() {
+final String sql = "values(1,2,3),(1,2,3),(1,2,3),(1,2,3)";
+final RelNode rel = sql(sql).toRel();
+assertThat(rel, instanceOf(Values.class));
+
+RelMetadataProvider provider = RelMdPopulationSize.SOURCE;
+
+List> handlers =
+provider.handlers(BuiltInMetadata.PopulationSize.Handler.class);
+
+// The population size is calculated to be half the row count. (The
assumption is that half
+// the rows are duplicated.) With the default handler it should evaluate
to 2 since there
+// are 4 rows.
+RelMdPopulationSize populationSize = (RelMdPopulationSize) handlers.get(0);
+Double popSize =
+populationSize.getPopulationSize((Values) rel,
rel.getCluster().getMetadataQuery(),
+bitSetOf(0, 1, 2));
+assertEquals(2.0, popSize);
+
+// If we use a custom RelMetadataQuery and override the row count, the
population size
+// should be half the reported row count. In this case we will have the
RelMetadataQuery say
+// the row count is 12 for testing purposes, so we should expect a
population size of 6.
+RelMetadataQuery customQuery = new RelMetadataQuery() {
+ @Override public Double getRowCount(RelNode rel) {
+return 12.0;
+ }
+};
+
+popSize = populationSize.getPopulationSize((Values) rel, customQuery,
bitSetOf(0, 1, 2));
+assertEquals(6.0, popSize);
+ }
+
private static final SqlOperator NONDETERMINISTIC_OP =
SqlBasicFunction.create("NDC", ReturnTypes.BOOLEAN,
OperandTypes.VARIADIC)
.withDeterministic(false);
Re: [PR] [CALCITE-5647] RelMdPopulationSize should use mq.getRowCount(rel) instead of rel.estimateRowCount(mq) [calcite]
rubenada merged PR #3632: URL: https://github.com/apache/calcite/pull/3632 -- 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
Re: [PR] [CALCITE-6224] Add LOG2 function (enabled in Mysql, Spark library) [calcite]
sonarcloud[bot] commented on PR #3648: URL: https://github.com/apache/calcite/pull/3648#issuecomment-1931497457 ## [](https://sonarcloud.io/dashboard?id=apache_calcite=3648) **Quality Gate passed** The SonarCloud Quality Gate passed, but some issues were introduced. [1 New issue](https://sonarcloud.io/project/issues?id=apache_calcite=3648=false=true) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3648=false=true) [100.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite=3648=new_coverage=list) [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite=3648=new_duplicated_lines_density=list) [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_calcite=3648) -- 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
