Re: [PR] [CALCITE-6221] JDBC adapter generates invalid query when the same table is joined multiple times [calcite]
YiwenWu commented on code in PR #3664:
URL: https://github.com/apache/calcite/pull/3664#discussion_r1475652612
##
core/src/main/java/org/apache/calcite/rel/rel2sql/RelToSqlConverter.java:
##
@@ -437,11 +437,43 @@ public Result visit(Filter e) {
final Result x = visitInput(e, 0, Clause.WHERE);
parseCorrelTable(e, x);
final Builder builder = x.builder(e);
+ if (input instanceof Join) {
+final Context context = x.qualifiedContext();
+final ImmutableList.Builder selectList =
ImmutableList.builder();
+// Fieldnames are unique since they are created by
SqlValidatorUtil.deriveJoinRowType()
+final List uniqueFieldNames =
input.getRowType().getFieldNames();
+boolean selectListRequired = false;
+for (int i = 0; i < context.fieldCount; i++) {
+ final SqlNode field = context.field(i);
+ final String fieldName = uniqueFieldNames.get(i);
+ if (fieldWasRenamedByJoinDueToNameClash(field, fieldName)) {
+selectListRequired = true;
+ }
+ selectList.add(
+ SqlStdOperatorTable.AS.createCall(POS, field,
+ new SqlIdentifier(fieldName, POS)));
+}
+if (selectListRequired) {
+ builder.setSelect(new SqlNodeList(selectList.build(), POS));
+}
+ }
builder.setWhere(builder.context.toSql(null, e.getCondition()));
return builder.result();
}
}
+ private static boolean fieldWasRenamedByJoinDueToNameClash(SqlNode field,
String fieldName) {
+if (!(field instanceof SqlIdentifier)) {
Review Comment:
Whether this method can satisfy？
```
private static boolean fieldRequiresAlias(Result x) {
List uniqueList = new ArrayList();
for(SqlNode node : x.qualifiedContext().fieldList()) {
if(node instance SqlIdentifier) {
SqlIdentifier field = (SqlIdentifier) node;
if (uniqueList.contains(last(field.names))) {
return true;
}
uniqueList.add(last(field.names));
}
}
return 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-6221] JDBC adapter generates invalid query when the same table is joined multiple times [calcite]
sonarcloud[bot] commented on PR #3664: URL: https://github.com/apache/calcite/pull/3664#issuecomment-1923021602 ## [](https://sonarcloud.io/dashboard?id=apache_calcite=3664) **Quality Gate passed** Kudos, no new issues were introduced! [0 New issues](https://sonarcloud.io/project/issues?id=apache_calcite=3664=false=true) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3664=false=true) [88.6% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite=3664=new_coverage=list) [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite=3664=new_duplicated_lines_density=list) [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_calcite=3664) -- 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-6221] JDBC adapter generates invalid query when the same table is joined multiple times [calcite]
kramerul commented on code in PR #3664:
URL: https://github.com/apache/calcite/pull/3664#discussion_r1475585120
##
core/src/main/java/org/apache/calcite/rel/rel2sql/RelToSqlConverter.java:
##
@@ -437,11 +437,43 @@ public Result visit(Filter e) {
final Result x = visitInput(e, 0, Clause.WHERE);
parseCorrelTable(e, x);
final Builder builder = x.builder(e);
+ if (input instanceof Join) {
+final Context context = x.qualifiedContext();
+final ImmutableList.Builder selectList =
ImmutableList.builder();
+// Fieldnames are unique since they are created by
SqlValidatorUtil.deriveJoinRowType()
+final List uniqueFieldNames =
input.getRowType().getFieldNames();
+boolean selectListRequired = false;
+for (int i = 0; i < context.fieldCount; i++) {
+ final SqlNode field = context.field(i);
+ final String fieldName = uniqueFieldNames.get(i);
+ if (fieldWasRenamedByJoinDueToNameClash(field, fieldName)) {
+selectListRequired = true;
+ }
+ selectList.add(
+ SqlStdOperatorTable.AS.createCall(POS, field,
+ new SqlIdentifier(fieldName, POS)));
+}
+if (selectListRequired) {
+ builder.setSelect(new SqlNodeList(selectList.build(), POS));
+}
+ }
builder.setWhere(builder.context.toSql(null, e.getCondition()));
return builder.result();
}
}
+ private static boolean fieldWasRenamedByJoinDueToNameClash(SqlNode field,
String fieldName) {
+if (!(field instanceof SqlIdentifier)) {
Review Comment:
I also thought about simplifying this on base of
`Result.qualifiedContext().fieldList()`. But I had no idea how this could be
achieved.
--
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-6221] JDBC adapter generates invalid query when the same table is joined multiple times [calcite]
kramerul commented on PR #3664: URL: https://github.com/apache/calcite/pull/3664#issuecomment-1922916619 I'm not so deep into Calcite that I would be able to say if there are other cases. As far as I know, this is the only case because * columns are only renamed in the `Join` class * the generation of selected columns in SQL is only missing for `Filter` -- 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-6221] JDBC adapter generates invalid query when the same table is joined multiple times [calcite]
kramerul commented on code in PR #3664:
URL: https://github.com/apache/calcite/pull/3664#discussion_r1475583359
##
core/src/main/java/org/apache/calcite/rel/rel2sql/RelToSqlConverter.java:
##
@@ -437,11 +437,43 @@ public Result visit(Filter e) {
final Result x = visitInput(e, 0, Clause.WHERE);
parseCorrelTable(e, x);
final Builder builder = x.builder(e);
+ if (input instanceof Join) {
+final Context context = x.qualifiedContext();
+final ImmutableList.Builder selectList =
ImmutableList.builder();
+// Fieldnames are unique since they are created by
SqlValidatorUtil.deriveJoinRowType()
+final List uniqueFieldNames =
input.getRowType().getFieldNames();
+boolean selectListRequired = false;
+for (int i = 0; i < context.fieldCount; i++) {
+ final SqlNode field = context.field(i);
+ final String fieldName = uniqueFieldNames.get(i);
+ if (fieldWasRenamedByJoinDueToNameClash(field, fieldName)) {
+selectListRequired = true;
+ }
+ selectList.add(
+ SqlStdOperatorTable.AS.createCall(POS, field,
+ new SqlIdentifier(fieldName, POS)));
+}
+if (selectListRequired) {
+ builder.setSelect(new SqlNodeList(selectList.build(), POS));
+}
+ }
builder.setWhere(builder.context.toSql(null, e.getCondition()));
return builder.result();
}
}
+ private static boolean fieldWasRenamedByJoinDueToNameClash(SqlNode field,
String fieldName) {
+if (!(field instanceof SqlIdentifier)) {
+ return true;
+}
+SqlIdentifier identifier = (SqlIdentifier) field;
+if (identifier.isStar()) {
+ return true;
+}
+List names = identifier.names;
+return !names.get(names.size() - 1).equals(fieldName);
Review Comment:
I changed 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-6236] EnumerableBatchNestedLoopJoin uses wrong row count for cost calculation [calcite]
YiwenWu commented on code in PR #3661:
URL: https://github.com/apache/calcite/pull/3661#discussion_r1474392304
##
core/src/main/java/org/apache/calcite/rel/metadata/RelMdRowCount.java:
##
@@ -181,6 +182,11 @@ public Double getRowCount(Calc rel, RelMetadataQuery mq) {
return mq.getRowCount(rel.getInput());
}
+ // Ensures that EnumerableBatchNestedLoopJoin has the same rowCount as the
join that originated it
+ public @Nullable Double getRowCount(EnumerableBatchNestedLoopJoin join,
RelMetadataQuery mq) {
Review Comment:
How can we ensure that the original join has the same statistics as the
EnumerableBatchNestedLoopJoin？
since the EnumerableBatchNestedLoopJoin has new Filter conditions
--
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-6236] EnumerableBatchNestedLoopJoin uses wrong row count for cost calculation [calcite]
YiwenWu commented on code in PR #3661:
URL: https://github.com/apache/calcite/pull/3661#discussion_r1474392304
##
core/src/main/java/org/apache/calcite/rel/metadata/RelMdRowCount.java:
##
@@ -181,6 +182,11 @@ public Double getRowCount(Calc rel, RelMetadataQuery mq) {
return mq.getRowCount(rel.getInput());
}
+ // Ensures that EnumerableBatchNestedLoopJoin has the same rowCount as the
join that originated it
+ public @Nullable Double getRowCount(EnumerableBatchNestedLoopJoin join,
RelMetadataQuery mq) {
Review Comment:
How to ensure that the original join has the same statistics as the
EnumerableBatchNestedLoopJoin？
since the EnumerableBatchNestedLoopJoin has new Filter conditions
--
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-6221] JDBC adapter generates invalid query when the same table is joined multiple times [calcite]
YiwenWu commented on code in PR #3664:
URL: https://github.com/apache/calcite/pull/3664#discussion_r1475566245
##
core/src/main/java/org/apache/calcite/rel/rel2sql/RelToSqlConverter.java:
##
@@ -437,11 +437,43 @@ public Result visit(Filter e) {
final Result x = visitInput(e, 0, Clause.WHERE);
parseCorrelTable(e, x);
final Builder builder = x.builder(e);
+ if (input instanceof Join) {
+final Context context = x.qualifiedContext();
+final ImmutableList.Builder selectList =
ImmutableList.builder();
+// Fieldnames are unique since they are created by
SqlValidatorUtil.deriveJoinRowType()
+final List uniqueFieldNames =
input.getRowType().getFieldNames();
+boolean selectListRequired = false;
+for (int i = 0; i < context.fieldCount; i++) {
+ final SqlNode field = context.field(i);
+ final String fieldName = uniqueFieldNames.get(i);
+ if (fieldWasRenamedByJoinDueToNameClash(field, fieldName)) {
+selectListRequired = true;
+ }
+ selectList.add(
+ SqlStdOperatorTable.AS.createCall(POS, field,
+ new SqlIdentifier(fieldName, POS)));
+}
+if (selectListRequired) {
+ builder.setSelect(new SqlNodeList(selectList.build(), POS));
+}
+ }
builder.setWhere(builder.context.toSql(null, e.getCondition()));
return builder.result();
}
}
+ private static boolean fieldWasRenamedByJoinDueToNameClash(SqlNode field,
String fieldName) {
+if (!(field instanceof SqlIdentifier)) {
Review Comment:
context(AliasContext) field is only allowed to return SqlIdentifier.
and Is there any other simpler way to determine whether rename is needed?
can we identify the Duplicate name by only
Result.qualifiedContext().fieldList()
--
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-6221] JDBC adapter generates invalid query when the same table is joined multiple times [calcite]
YiwenWu commented on code in PR #3664:
URL: https://github.com/apache/calcite/pull/3664#discussion_r1475566245
##
core/src/main/java/org/apache/calcite/rel/rel2sql/RelToSqlConverter.java:
##
@@ -437,11 +437,43 @@ public Result visit(Filter e) {
final Result x = visitInput(e, 0, Clause.WHERE);
parseCorrelTable(e, x);
final Builder builder = x.builder(e);
+ if (input instanceof Join) {
+final Context context = x.qualifiedContext();
+final ImmutableList.Builder selectList =
ImmutableList.builder();
+// Fieldnames are unique since they are created by
SqlValidatorUtil.deriveJoinRowType()
+final List uniqueFieldNames =
input.getRowType().getFieldNames();
+boolean selectListRequired = false;
+for (int i = 0; i < context.fieldCount; i++) {
+ final SqlNode field = context.field(i);
+ final String fieldName = uniqueFieldNames.get(i);
+ if (fieldWasRenamedByJoinDueToNameClash(field, fieldName)) {
+selectListRequired = true;
+ }
+ selectList.add(
+ SqlStdOperatorTable.AS.createCall(POS, field,
+ new SqlIdentifier(fieldName, POS)));
+}
+if (selectListRequired) {
+ builder.setSelect(new SqlNodeList(selectList.build(), POS));
+}
+ }
builder.setWhere(builder.context.toSql(null, e.getCondition()));
return builder.result();
}
}
+ private static boolean fieldWasRenamedByJoinDueToNameClash(SqlNode field,
String fieldName) {
+if (!(field instanceof SqlIdentifier)) {
Review Comment:
context(AliasContext) field is only allowed to return SqlIdentifier.
and Is there any other simpler way to determine whether rename is needed?
can we identify the Duplicate name by Result.qualifiedContext().fieldList()
##
core/src/main/java/org/apache/calcite/rel/rel2sql/RelToSqlConverter.java:
##
@@ -437,11 +437,43 @@ public Result visit(Filter e) {
final Result x = visitInput(e, 0, Clause.WHERE);
parseCorrelTable(e, x);
final Builder builder = x.builder(e);
+ if (input instanceof Join) {
+final Context context = x.qualifiedContext();
+final ImmutableList.Builder selectList =
ImmutableList.builder();
+// Fieldnames are unique since they are created by
SqlValidatorUtil.deriveJoinRowType()
+final List uniqueFieldNames =
input.getRowType().getFieldNames();
+boolean selectListRequired = false;
+for (int i = 0; i < context.fieldCount; i++) {
+ final SqlNode field = context.field(i);
+ final String fieldName = uniqueFieldNames.get(i);
+ if (fieldWasRenamedByJoinDueToNameClash(field, fieldName)) {
+selectListRequired = true;
+ }
+ selectList.add(
+ SqlStdOperatorTable.AS.createCall(POS, field,
+ new SqlIdentifier(fieldName, POS)));
+}
+if (selectListRequired) {
+ builder.setSelect(new SqlNodeList(selectList.build(), POS));
+}
+ }
builder.setWhere(builder.context.toSql(null, e.getCondition()));
return builder.result();
}
}
+ private static boolean fieldWasRenamedByJoinDueToNameClash(SqlNode field,
String fieldName) {
+if (!(field instanceof SqlIdentifier)) {
+ return true;
+}
+SqlIdentifier identifier = (SqlIdentifier) field;
+if (identifier.isStar()) {
+ return true;
+}
+List names = identifier.names;
+return !names.get(names.size() - 1).equals(fieldName);
Review Comment:
can be simplified to `last(names)equals(fieldName)`
--
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-6221] JDBC adapter generates invalid query when the same table is joined multiple times [calcite]
sonarcloud[bot] commented on PR #3664: URL: https://github.com/apache/calcite/pull/3664#issuecomment-1922883755 ## [](https://sonarcloud.io/dashboard?id=apache_calcite=3664) **Quality Gate passed** Kudos, no new issues were introduced! [0 New issues](https://sonarcloud.io/project/issues?id=apache_calcite=3664=false=true) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3664=false=true) [88.6% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite=3664=new_coverage=list) [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite=3664=new_duplicated_lines_density=list) [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_calcite=3664) -- 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-6221] JDBC adapter generates invalid query when the same table is joined multiple times [calcite]
kramerul commented on code in PR #3664:
URL: https://github.com/apache/calcite/pull/3664#discussion_r147280
##
core/src/main/java/org/apache/calcite/rel/rel2sql/RelToSqlConverter.java:
##
@@ -437,11 +437,43 @@ public Result visit(Filter e) {
final Result x = visitInput(e, 0, Clause.WHERE);
parseCorrelTable(e, x);
final Builder builder = x.builder(e);
+ if (input instanceof Join) {
+final Context context = x.qualifiedContext();
+final ImmutableList.Builder selectList =
ImmutableList.builder();
+// Fieldnames are unique since they are created by
SqlValidatorUtil.deriveJoinRowType()
+final List uniqueFieldNames =
input.getRowType().getFieldNames();
+boolean selectListRequired = false;
+for (int i = 0; i < context.fieldCount; i++) {
+ final SqlNode field = context.field(i);
+ final String fieldName = uniqueFieldNames.get(i);
+ if (fieldRequiresAlias(field, fieldName)) {
+selectListRequired = true;
+ }
+ selectList.add(
+ SqlStdOperatorTable.AS.createCall(POS, field,
+ new SqlIdentifier(fieldName, POS)));
+}
+if (selectListRequired) {
+ builder.setSelect(new SqlNodeList(selectList.build(), POS));
+}
+ }
builder.setWhere(builder.context.toSql(null, e.getCondition()));
return builder.result();
}
}
+ private static boolean fieldRequiresAlias(SqlNode field, String fieldName) {
+if (!(field instanceof SqlIdentifier)) {
+ return true;
+}
+SqlIdentifier identifier = (SqlIdentifier) field;
+if (identifier.isStar()) {
+ return true;
+}
+List names = identifier.names;
+return !names.get(names.size() - 1).equals(fieldName);
Review Comment:
I renamed this function to `fieldWasRenamedByJoinDueToNameClash`
--
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-6218] RelToSqlConverter fails to convert correlated lateral joins [calcite]
JiajunBernoulli commented on PR #3656: URL: https://github.com/apache/calcite/pull/3656#issuecomment-1922650184 I noticed that we are still discussing in Jira, so marked 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-2067] RexLiteral cannot represent accurately floating point values, including NaN, Infinity [calcite]
mihaibudiu commented on PR #3663: URL: https://github.com/apache/calcite/pull/3663#issuecomment-1922637894 @julianhyde I hope to have addressed your comments. This is a breaking change in some respect; one can see how the Druid Adapter code had to be modified to accommodate this change. But I think it fixes a genuine bug. It doesn't solve the problem of FP literals completely, but it enables more programs to produce correct results than before. -- 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-2067] RexLiteral cannot represent accurately floating point values, including NaN, Infinity [calcite]
mihaibudiu commented on code in PR #3663: URL: https://github.com/apache/calcite/pull/3663#discussion_r1475414494 ## core/src/main/java/org/apache/calcite/rex/RexLiteral.java: ## @@ -325,19 +324,18 @@ public static boolean valueMatchesType( } // fall through case DECIMAL: +case BIGINT: + return value instanceof BigDecimal; case DOUBLE: case FLOAT: case REAL: -case BIGINT: - return value instanceof BigDecimal; + return value instanceof BigDecimal || value instanceof Double; Review Comment: I have removed the choice here too. -- 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-2067] RexLiteral cannot represent accurately floating point values, including NaN, Infinity [calcite]
mihaibudiu commented on code in PR #3663:
URL: https://github.com/apache/calcite/pull/3663#discussion_r1475414363
##
core/src/main/java/org/apache/calcite/rex/RexBuilder.java:
##
@@ -1110,6 +1110,14 @@ public RexLiteral makeApproxLiteral(BigDecimal bd) {
return makeApproxLiteral(bd,
typeFactory.createSqlType(SqlTypeName.DOUBLE));
}
+ /**
Review Comment:
I have deleted the method
--
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-2067] RexLiteral cannot represent accurately floating point values, including NaN, Infinity [calcite]
sonarcloud[bot] commented on PR #3663: URL: https://github.com/apache/calcite/pull/3663#issuecomment-1922604110 ## [](https://sonarcloud.io/dashboard?id=apache_calcite=3663) **Quality Gate passed** The SonarCloud Quality Gate passed, but some issues were introduced. [3 New issues](https://sonarcloud.io/project/issues?id=apache_calcite=3663=false=true) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3663=false=true) [70.6% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite=3663=new_coverage=list) [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite=3663=new_duplicated_lines_density=list) [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_calcite=3663) -- 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-5647] Use mq.getRowCount(rel) instead of rel.estimateRowCount(mq) [calcite]
sonarcloud[bot] commented on PR #3632: URL: https://github.com/apache/calcite/pull/3632#issuecomment-1922050930 ## [](https://sonarcloud.io/dashboard?id=apache_calcite=3632) **Quality Gate passed** Kudos, no new issues were introduced! [0 New issues](https://sonarcloud.io/project/issues?id=apache_calcite=3632=false=true) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3632=false=true) [100.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite=3632=new_coverage=list) [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite=3632=new_duplicated_lines_density=list) [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_calcite=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-6221] JDBC adapter generates invalid query when the same table is joined multiple times [calcite]
mihaibudiu commented on code in PR #3664:
URL: https://github.com/apache/calcite/pull/3664#discussion_r1474977545
##
core/src/main/java/org/apache/calcite/rel/rel2sql/RelToSqlConverter.java:
##
@@ -437,11 +437,43 @@ public Result visit(Filter e) {
final Result x = visitInput(e, 0, Clause.WHERE);
parseCorrelTable(e, x);
final Builder builder = x.builder(e);
+ if (input instanceof Join) {
+final Context context = x.qualifiedContext();
+final ImmutableList.Builder selectList =
ImmutableList.builder();
+// Fieldnames are unique since they are created by
SqlValidatorUtil.deriveJoinRowType()
+final List uniqueFieldNames =
input.getRowType().getFieldNames();
+boolean selectListRequired = false;
+for (int i = 0; i < context.fieldCount; i++) {
+ final SqlNode field = context.field(i);
+ final String fieldName = uniqueFieldNames.get(i);
+ if (fieldRequiresAlias(field, fieldName)) {
+selectListRequired = true;
+ }
+ selectList.add(
+ SqlStdOperatorTable.AS.createCall(POS, field,
+ new SqlIdentifier(fieldName, POS)));
+}
+if (selectListRequired) {
+ builder.setSelect(new SqlNodeList(selectList.build(), POS));
+}
+ }
builder.setWhere(builder.context.toSql(null, e.getCondition()));
return builder.result();
}
}
+ private static boolean fieldRequiresAlias(SqlNode field, String fieldName) {
+if (!(field instanceof SqlIdentifier)) {
+ return true;
+}
+SqlIdentifier identifier = (SqlIdentifier) field;
+if (identifier.isStar()) {
+ return true;
+}
+List names = identifier.names;
+return !names.get(names.size() - 1).equals(fieldName);
Review Comment:
This is the inverse of what I expected.
Maybe you need to document this function if this is correct.
--
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]
mihaibudiu commented on code in PR #3648:
URL: https://github.com/apache/calcite/pull/3648#discussion_r1474973925
##
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##
@@ -6188,6 +6188,67 @@ void checkRegexpExtract(SqlOperatorFixture f0,
FunctionAlias functionAlias) {
f.checkNull("log(10, cast(null as real))");
}
+ /** Test case for
+ * https://issues.apache.org/jira/browse/CALCITE-6224;>[CALCITE-6224]
+ * Add LOG@ function (enabled in MYSQL, Spark library). */
+ @Test void testLog2Func() {
+final SqlOperatorFixture f0 = fixture();
+final Consumer consumer = f -> {
+ f.setFor(SqlLibraryOperators.LOG2);
+ f.checkScalarApprox("log2(2)", "DOUBLE NOT NULL",
+ isWithin(1.0, 0.01));
+ f.checkScalarApprox("log2(4)", "DOUBLE NOT NULL",
+ isWithin(2.0, 0.01));
+ f.checkScalarApprox("log2(65536)", "DOUBLE NOT NULL",
+ isWithin(16.0, 0.01));
+ f.checkScalarApprox("log2(-2)", "DOUBLE NOT NULL",
+ "NaN");
+ f.checkScalarApprox("log2(2/3)", "DOUBLE NOT NULL",
+ "-Infinity");
+ f.checkScalarApprox("log2(2.2)", "DOUBLE NOT NULL",
+ "1.1375035237499351");
+ f.checkScalarApprox("log2(0.5)", "DOUBLE NOT NULL",
+ "-1.0");
+ f.checkScalarApprox("log2(3)", "DOUBLE NOT NULL",
+ isWithin(1.5849625007211563, 0.01));
+ f.checkNull("log2(cast(null as real))");
+};
+f0.forEachLibrary(list(SqlLibrary.MYSQL, SqlLibrary.SPARK), consumer);
+ }
+
+ /** Test case for
+ * https://issues.apache.org/jira/browse/CALCITE-6232;>[CALCITE-6232]
+ * Using fractions in LOG function does not return correct results. */
+ @Test void testLogFuncByConvert() {
+// The fractional conversion of the Log function is reserved only for
integer bits
+final SqlOperatorFixture f0 = Fixtures.forOperators(true);
+f0.setFor(SqlLibraryOperators.LOG, VmName.EXPAND);
+final SqlOperatorFixture f = f0.withLibrary(SqlLibrary.BIG_QUERY);
+f.checkScalarApprox("log(2/3, 2)", "DOUBLE NOT NULL",
Review Comment:
you have to write tests with illegal and NULL values for all combinations of
arguments
--
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-5647] Use mq.getRowCount(rel) instead of rel.estimateRowCount(mq) [calcite]
jduo commented on PR #3632: URL: https://github.com/apache/calcite/pull/3632#issuecomment-1921996618 @rubenada , I've made the commit and PR have the same title. Should the JIRA have the same title now? It states the problem whereas the commit/PR describe what changed. -- 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-2067] RexLiteral cannot represent accurately floating point values, including NaN, Infinity [calcite]
sonarcloud[bot] commented on PR #3663: URL: https://github.com/apache/calcite/pull/3663#issuecomment-1921959086 ## [](https://sonarcloud.io/dashboard?id=apache_calcite=3663) **Quality Gate passed** The SonarCloud Quality Gate passed, but some issues were introduced. [2 New issues](https://sonarcloud.io/project/issues?id=apache_calcite=3663=false=true) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3663=false=true) [58.5% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite=3663=new_coverage=list) [25.7% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite=3663=new_duplicated_lines_density=list) [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_calcite=3663) -- 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]
tanclary commented on code in PR #3648:
URL: https://github.com/apache/calcite/pull/3648#discussion_r1474830504
##
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##
@@ -6188,6 +6188,65 @@ void checkRegexpExtract(SqlOperatorFixture f0,
FunctionAlias functionAlias) {
f.checkNull("log(10, cast(null as real))");
}
+ /** Test case for
+ * https://issues.apache.org/jira/browse/CALCITE-6224;>[CALCITE-6224]
+ * Add LOG@ function (enabled in MYSQL, Spark library). */
+ @Test void testLog2Func() {
+final SqlOperatorFixture f0 = fixture();
+final Consumer consumer = f -> {
+ f.setFor(SqlLibraryOperators.LOG2);
+ f.checkScalarApprox("log2(2)", "DOUBLE NOT NULL",
+ isWithin(1.0, 0.01));
+ f.checkScalarApprox("log2(4)", "DOUBLE NOT NULL",
+ isWithin(2.0, 0.01));
+ f.checkScalarApprox("log2(65536)", "DOUBLE NOT NULL",
+ isWithin(16.0, 0.01));
+ f.checkScalarApprox("log2(-2)", "DOUBLE NOT NULL",
+ "NaN");
+ f.checkScalarApprox("log2(2/3)", "DOUBLE NOT NULL",
+ "-Infinity");
+ f.checkScalarApprox("log2(2.2)", "DOUBLE NOT NULL",
+ "1.1375035237499351");
+ f.checkScalarApprox("log2(0.5)", "DOUBLE NOT NULL",
+ "-1.0");
+ f.checkScalarApprox("log2(3)", "DOUBLE NOT NULL",
+ isWithin(1.5849625007211563, 0.01));
+ f.checkNull("log2(cast(null as real))");
+};
+f0.forEachLibrary(list(SqlLibrary.MYSQL, SqlLibrary.SPARK), consumer);
+ }
+
+ /** Test case for
+ * https://issues.apache.org/jira/browse/CALCITE-6232;>[CALCITE-6232]
+ * Using fractions in LOG function does not return correct results. */
+ @Test void testLogFuncByConvert() {
+// The fractional conversion of the Log function is reserved only for
integer bits
+final SqlOperatorFixture f0 = Fixtures.forOperators(true);
+f0.setFor(SqlLibraryOperators.LOG, VmName.EXPAND);
+final SqlOperatorFixture f = f0.withLibrary(SqlLibrary.BIG_QUERY);
+f.checkScalarApprox("log(2/3, 2)", "DOUBLE NOT NULL",
+ "-Infinity");
+f.checkScalarApprox("log(0,2)", "DOUBLE NOT NULL",
+ "-Infinity");
+f.checkScalarApprox("log(1,2)", "DOUBLE NOT NULL",
+"0.0");
+f.checkScalarApprox("log(4/3,2)", "DOUBLE NOT NULL",
+"0.0");
+f.checkScalarApprox("log(5/3,2)", "DOUBLE NOT NULL",
+"0.0");
+f.checkScalarApprox("log(2/3, 10)", "DOUBLE NOT NULL",
+ "-Infinity");
+f.checkScalarApprox("log(0,10)", "DOUBLE NOT NULL",
+ "-Infinity");
+f.checkScalarApprox("log(1,10)", "DOUBLE NOT NULL",
+"0.0");
+f.checkScalarApprox("log(4/3,10)", "DOUBLE NOT NULL",
Review Comment:
What happens if you use the decimal form like `.5` instead of `1/2`?
--
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]
caicancai commented on code in PR #3648:
URL: https://github.com/apache/calcite/pull/3648#discussion_r1474711660
##
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##
@@ -6188,6 +6188,65 @@ void checkRegexpExtract(SqlOperatorFixture f0,
FunctionAlias functionAlias) {
f.checkNull("log(10, cast(null as real))");
}
+ /** Test case for
+ * https://issues.apache.org/jira/browse/CALCITE-6224;>[CALCITE-6224]
+ * Add LOG@ function (enabled in MYSQL, Spark library). */
+ @Test void testLog2Func() {
+final SqlOperatorFixture f0 = fixture();
+final Consumer consumer = f -> {
+ f.setFor(SqlLibraryOperators.LOG2);
+ f.checkScalarApprox("log2(2)", "DOUBLE NOT NULL",
+ isWithin(1.0, 0.01));
+ f.checkScalarApprox("log2(4)", "DOUBLE NOT NULL",
+ isWithin(2.0, 0.01));
+ f.checkScalarApprox("log2(65536)", "DOUBLE NOT NULL",
+ isWithin(16.0, 0.01));
+ f.checkScalarApprox("log2(-2)", "DOUBLE NOT NULL",
+ "NaN");
+ f.checkScalarApprox("log2(2/3)", "DOUBLE NOT NULL",
+ "-Infinity");
+ f.checkScalarApprox("log2(2.2)", "DOUBLE NOT NULL",
+ "1.1375035237499351");
+ f.checkScalarApprox("log2(0.5)", "DOUBLE NOT NULL",
+ "-1.0");
+ f.checkScalarApprox("log2(3)", "DOUBLE NOT NULL",
+ isWithin(1.5849625007211563, 0.01));
+ f.checkNull("log2(cast(null as real))");
+};
+f0.forEachLibrary(list(SqlLibrary.MYSQL, SqlLibrary.SPARK), consumer);
+ }
+
+ /** Test case for
+ * https://issues.apache.org/jira/browse/CALCITE-6232;>[CALCITE-6232]
+ * Using fractions in LOG function does not return correct results. */
+ @Test void testLogFuncByConvert() {
+// The fractional conversion of the Log function is reserved only for
integer bits
+final SqlOperatorFixture f0 = Fixtures.forOperators(true);
+f0.setFor(SqlLibraryOperators.LOG, VmName.EXPAND);
+final SqlOperatorFixture f = f0.withLibrary(SqlLibrary.BIG_QUERY);
+f.checkScalarApprox("log(2/3, 2)", "DOUBLE NOT NULL",
+ "-Infinity");
+f.checkScalarApprox("log(0,2)", "DOUBLE NOT NULL",
+ "-Infinity");
+f.checkScalarApprox("log(1,2)", "DOUBLE NOT NULL",
+"0.0");
+f.checkScalarApprox("log(4/3,2)", "DOUBLE NOT NULL",
+"0.0");
+f.checkScalarApprox("log(5/3,2)", "DOUBLE NOT NULL",
+"0.0");
+f.checkScalarApprox("log(2/3, 10)", "DOUBLE NOT NULL",
+ "-Infinity");
+f.checkScalarApprox("log(0,10)", "DOUBLE NOT NULL",
+ "-Infinity");
+f.checkScalarApprox("log(1,10)", "DOUBLE NOT NULL",
+"0.0");
+f.checkScalarApprox("log(4/3,10)", "DOUBLE NOT NULL",
Review Comment:
> Why don't we fix the bug? Do you have an updated JIRA case for it?
I don't know if this is true.
https://issues.apache.org/jira/browse/CALCITE-6232
--
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-6234] Add Test on SqlOperatorTest for format_date function(enable Bigquery and pg) [calcite]
caicancai commented on PR #3659: URL: https://github.com/apache/calcite/pull/3659#issuecomment-1921654956 > Thanks for the clarification @caicancai about the related PR > > I'm a bit confused, because the Jira title & description (and the PR title and commit message) talk about `format_date` function, but in fact the new tests involve `to_char`. May I suggest as commit message (and Jira title and PR title) something along the lines: `[CALCITE-6234] Add tests on SqlOperatorTest for to_char function` ? OK, thanks. Please forgive my unclear description -- 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]
caicancai commented on code in PR #3648:
URL: https://github.com/apache/calcite/pull/3648#discussion_r1474701648
##
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##
@@ -6188,6 +6188,65 @@ void checkRegexpExtract(SqlOperatorFixture f0,
FunctionAlias functionAlias) {
f.checkNull("log(10, cast(null as real))");
}
+ /** Test case for
+ * https://issues.apache.org/jira/browse/CALCITE-6224;>[CALCITE-6224]
+ * Add LOG@ function (enabled in MYSQL, Spark library). */
+ @Test void testLog2Func() {
+final SqlOperatorFixture f0 = fixture();
+final Consumer consumer = f -> {
+ f.setFor(SqlLibraryOperators.LOG2);
+ f.checkScalarApprox("log2(2)", "DOUBLE NOT NULL",
+ isWithin(1.0, 0.01));
+ f.checkScalarApprox("log2(4)", "DOUBLE NOT NULL",
+ isWithin(2.0, 0.01));
+ f.checkScalarApprox("log2(65536)", "DOUBLE NOT NULL",
+ isWithin(16.0, 0.01));
+ f.checkScalarApprox("log2(-2)", "DOUBLE NOT NULL",
+ "NaN");
+ f.checkScalarApprox("log2(2/3)", "DOUBLE NOT NULL",
+ "-Infinity");
+ f.checkScalarApprox("log2(2.2)", "DOUBLE NOT NULL",
+ "1.1375035237499351");
+ f.checkScalarApprox("log2(0.5)", "DOUBLE NOT NULL",
+ "-1.0");
+ f.checkScalarApprox("log2(3)", "DOUBLE NOT NULL",
+ isWithin(1.5849625007211563, 0.01));
+ f.checkNull("log2(cast(null as real))");
+};
+f0.forEachLibrary(list(SqlLibrary.MYSQL, SqlLibrary.SPARK), consumer);
+ }
+
+ /** Test case for
+ * https://issues.apache.org/jira/browse/CALCITE-6232;>[CALCITE-6232]
+ * Using fractions in LOG function does not return correct results. */
+ @Test void testLogFuncByConvert() {
+// The fractional conversion of the Log function is reserved only for
integer bits
+final SqlOperatorFixture f0 = Fixtures.forOperators(true);
+f0.setFor(SqlLibraryOperators.LOG, VmName.EXPAND);
+final SqlOperatorFixture f = f0.withLibrary(SqlLibrary.BIG_QUERY);
+f.checkScalarApprox("log(2/3, 2)", "DOUBLE NOT NULL",
+ "-Infinity");
+f.checkScalarApprox("log(0,2)", "DOUBLE NOT NULL",
+ "-Infinity");
+f.checkScalarApprox("log(1,2)", "DOUBLE NOT NULL",
+"0.0");
+f.checkScalarApprox("log(4/3,2)", "DOUBLE NOT NULL",
+"0.0");
+f.checkScalarApprox("log(5/3,2)", "DOUBLE NOT NULL",
+"0.0");
+f.checkScalarApprox("log(2/3, 10)", "DOUBLE NOT NULL",
+ "-Infinity");
+f.checkScalarApprox("log(0,10)", "DOUBLE NOT NULL",
+ "-Infinity");
+f.checkScalarApprox("log(1,10)", "DOUBLE NOT NULL",
+"0.0");
+f.checkScalarApprox("log(4/3,10)", "DOUBLE NOT NULL",
Review Comment:
ok, this may take a while for me because i am learning this
--
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]
tanclary commented on code in PR #3648:
URL: https://github.com/apache/calcite/pull/3648#discussion_r1474685867
##
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##
@@ -6188,6 +6188,65 @@ void checkRegexpExtract(SqlOperatorFixture f0,
FunctionAlias functionAlias) {
f.checkNull("log(10, cast(null as real))");
}
+ /** Test case for
+ * https://issues.apache.org/jira/browse/CALCITE-6224;>[CALCITE-6224]
+ * Add LOG@ function (enabled in MYSQL, Spark library). */
+ @Test void testLog2Func() {
+final SqlOperatorFixture f0 = fixture();
+final Consumer consumer = f -> {
+ f.setFor(SqlLibraryOperators.LOG2);
+ f.checkScalarApprox("log2(2)", "DOUBLE NOT NULL",
+ isWithin(1.0, 0.01));
+ f.checkScalarApprox("log2(4)", "DOUBLE NOT NULL",
+ isWithin(2.0, 0.01));
+ f.checkScalarApprox("log2(65536)", "DOUBLE NOT NULL",
+ isWithin(16.0, 0.01));
+ f.checkScalarApprox("log2(-2)", "DOUBLE NOT NULL",
+ "NaN");
+ f.checkScalarApprox("log2(2/3)", "DOUBLE NOT NULL",
+ "-Infinity");
+ f.checkScalarApprox("log2(2.2)", "DOUBLE NOT NULL",
+ "1.1375035237499351");
+ f.checkScalarApprox("log2(0.5)", "DOUBLE NOT NULL",
+ "-1.0");
+ f.checkScalarApprox("log2(3)", "DOUBLE NOT NULL",
+ isWithin(1.5849625007211563, 0.01));
+ f.checkNull("log2(cast(null as real))");
+};
+f0.forEachLibrary(list(SqlLibrary.MYSQL, SqlLibrary.SPARK), consumer);
+ }
+
+ /** Test case for
+ * https://issues.apache.org/jira/browse/CALCITE-6232;>[CALCITE-6232]
+ * Using fractions in LOG function does not return correct results. */
+ @Test void testLogFuncByConvert() {
+// The fractional conversion of the Log function is reserved only for
integer bits
+final SqlOperatorFixture f0 = Fixtures.forOperators(true);
+f0.setFor(SqlLibraryOperators.LOG, VmName.EXPAND);
+final SqlOperatorFixture f = f0.withLibrary(SqlLibrary.BIG_QUERY);
+f.checkScalarApprox("log(2/3, 2)", "DOUBLE NOT NULL",
+ "-Infinity");
+f.checkScalarApprox("log(0,2)", "DOUBLE NOT NULL",
+ "-Infinity");
+f.checkScalarApprox("log(1,2)", "DOUBLE NOT NULL",
+"0.0");
+f.checkScalarApprox("log(4/3,2)", "DOUBLE NOT NULL",
+"0.0");
+f.checkScalarApprox("log(5/3,2)", "DOUBLE NOT NULL",
+"0.0");
+f.checkScalarApprox("log(2/3, 10)", "DOUBLE NOT NULL",
+ "-Infinity");
+f.checkScalarApprox("log(0,10)", "DOUBLE NOT NULL",
+ "-Infinity");
+f.checkScalarApprox("log(1,10)", "DOUBLE NOT NULL",
+"0.0");
+f.checkScalarApprox("log(4/3,10)", "DOUBLE NOT NULL",
Review Comment:
Why don't we fix the bug? Do you have an updated JIRA case for 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-6234] Add Test on SqlOperatorTest for format_date function(enable Bigquery and pg) [calcite]
rubenada commented on PR #3659: URL: https://github.com/apache/calcite/pull/3659#issuecomment-1921625030 Thanks for the clarification @caicancai about the related PR I'm a bit confused, because the Jira title & description (and the PR title and commit message) talk about `format_date` function, but in fact the new tests involve `to_char`. May I suggest as commit message (and Jira title and PR title) something along the lines: `[CALCITE-6234] Add tests on SqlOperatorTest for to_char function` ? -- 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-6234] Add Test on SqlOperatorTest for format_date function(enable Bigquery and pg) [calcite]
caicancai commented on PR #3659: URL: https://github.com/apache/calcite/pull/3659#issuecomment-1921560147 At that time I forgot to add tests in sqloperatortest -- 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-6234] Add Test on SqlOperatorTest for format_date function(enable Bigquery and pg) [calcite]
caicancai commented on PR #3659: URL: https://github.com/apache/calcite/pull/3659#issuecomment-1921558777 > > I have to admit that this is a problem left by my previous PR, and I'm sorry for that. > > What previous PR? Is [CALCITE-6234](https://issues.apache.org/jira/browse/CALCITE-6234) related to another Jira? In which case, could you please link both? Thank you for your reply https://github.com/apache/calcite/pull/3642 -- 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-6221] JDBC adapter generates invalid query when the same table is joined multiple times [calcite]
sonarcloud[bot] commented on PR #3664: URL: https://github.com/apache/calcite/pull/3664#issuecomment-1921528712 ## [](https://sonarcloud.io/dashboard?id=apache_calcite=3664) **Quality Gate passed** The SonarCloud Quality Gate passed, but some issues were introduced. [2 New issues](https://sonarcloud.io/project/issues?id=apache_calcite=3664=false=true) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3664=false=true) [88.6% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite=3664=new_coverage=list) [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite=3664=new_duplicated_lines_density=list) [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_calcite=3664) -- 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-5647 Use mq.getRowCount(rel) instead of rel.estimateRowCount(mq) [calcite]
rubenada commented on PR #3632: URL: https://github.com/apache/calcite/pull/3632#issuecomment-1921499432 LGTM. @jduo , Just to avoid confusion, would it be possible to align commit message with PR title and Jira title? -- 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-6221] JDBC adapter generates invalid query when the same table is joined multiple times [calcite]
kramerul opened a new pull request, #3664: URL: https://github.com/apache/calcite/pull/3664 (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-6236] EnumerableBatchNestedLoopJoin uses wrong row count for cost calculation [calcite]
zabetak commented on code in PR #3661:
URL: https://github.com/apache/calcite/pull/3661#discussion_r1474526470
##
core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableBatchNestedLoopJoin.java:
##
@@ -55,6 +55,7 @@
public class EnumerableBatchNestedLoopJoin extends Join implements
EnumerableRel {
private final ImmutableBitSet requiredColumns;
+ private final Join originalJoin;
Review Comment:
Keeping a RelNode in another RelNode is a strange pattern that we shall
better avoid. Other than that adding attributes may necessitate touching the
digest which may further complicate the situation.
--
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-6234] Add Test on SqlOperatorTest for format_date function(enable Bigquery and pg) [calcite]
rubenada commented on PR #3659: URL: https://github.com/apache/calcite/pull/3659#issuecomment-1921402124 > I have to admit that this is a problem left by my previous PR, and I'm sorry for that. What previous PR? Is [CALCITE-6234](https://issues.apache.org/jira/browse/CALCITE-6234) related to another Jira? In which case, could you please link both? -- 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-6221] JDBC adapter generates invalid query when the same table is joined multiple times [calcite]
kramerul closed pull request #3658: [CALCITE-6221] JDBC adapter generates invalid query when the same table is joined multiple times URL: https://github.com/apache/calcite/pull/3658 -- 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-6221] JDBC adapter generates invalid query when the same table is joined multiple times [calcite]
kramerul commented on PR #3658: URL: https://github.com/apache/calcite/pull/3658#issuecomment-1921384505 Closing this PR in favor of a new 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
Re: [PR] [CALCITE-6236] EnumerableBatchNestedLoopJoin uses wrong row count for cost calculation [calcite]
rubenada commented on code in PR #3661:
URL: https://github.com/apache/calcite/pull/3661#discussion_r1474483811
##
core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableBatchNestedLoopJoin.java:
##
@@ -63,9 +64,11 @@ protected EnumerableBatchNestedLoopJoin(
RexNode condition,
Set variablesSet,
ImmutableBitSet requiredColumns,
- JoinRelType joinType) {
+ JoinRelType joinType,
+ Join originalJoin) {
super(cluster, traits, ImmutableList.of(), left, right, condition,
variablesSet, joinType);
this.requiredColumns = requiredColumns;
+this.originalJoin = originalJoin;
}
public static EnumerableBatchNestedLoopJoin create(
Review Comment:
Ok @JiajunBernoulli .
I have committed the comment on the next RN about the breaking change.
When the PR gets merged, I will mention it also in the Jira.
--
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]
caicancai commented on PR #3648: URL: https://github.com/apache/calcite/pull/3648#issuecomment-1921316637 @mihaibudiu Maybe I need your opinion if you have time, thank you -- 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-6236] EnumerableBatchNestedLoopJoin uses wrong row count for cost calculation [calcite]
JiajunBernoulli commented on code in PR #3661:
URL: https://github.com/apache/calcite/pull/3661#discussion_r1474422218
##
core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableBatchNestedLoopJoin.java:
##
@@ -63,9 +64,11 @@ protected EnumerableBatchNestedLoopJoin(
RexNode condition,
Set variablesSet,
ImmutableBitSet requiredColumns,
- JoinRelType joinType) {
+ JoinRelType joinType,
+ Join originalJoin) {
super(cluster, traits, ImmutableList.of(), left, right, condition,
variablesSet, joinType);
this.requiredColumns = requiredColumns;
+this.originalJoin = originalJoin;
}
public static EnumerableBatchNestedLoopJoin create(
Review Comment:
Second is ok.
Let's emphasize this change in JIRA.
--
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]
caicancai commented on code in PR #3648:
URL: https://github.com/apache/calcite/pull/3648#discussion_r1472236067
##
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##
@@ -6188,6 +6188,65 @@ void checkRegexpExtract(SqlOperatorFixture f0,
FunctionAlias functionAlias) {
f.checkNull("log(10, cast(null as real))");
}
+ /** Test case for
+ * https://issues.apache.org/jira/browse/CALCITE-6224;>[CALCITE-6224]
+ * Add LOG@ function (enabled in MYSQL, Spark library). */
+ @Test void testLog2Func() {
+final SqlOperatorFixture f0 = fixture();
+final Consumer consumer = f -> {
+ f.setFor(SqlLibraryOperators.LOG2);
+ f.checkScalarApprox("log2(2)", "DOUBLE NOT NULL",
+ isWithin(1.0, 0.01));
+ f.checkScalarApprox("log2(4)", "DOUBLE NOT NULL",
+ isWithin(2.0, 0.01));
+ f.checkScalarApprox("log2(65536)", "DOUBLE NOT NULL",
+ isWithin(16.0, 0.01));
+ f.checkScalarApprox("log2(-2)", "DOUBLE NOT NULL",
+ "NaN");
+ f.checkScalarApprox("log2(2/3)", "DOUBLE NOT NULL",
+ "-Infinity");
+ f.checkScalarApprox("log2(2.2)", "DOUBLE NOT NULL",
+ "1.1375035237499351");
+ f.checkScalarApprox("log2(0.5)", "DOUBLE NOT NULL",
+ "-1.0");
+ f.checkScalarApprox("log2(3)", "DOUBLE NOT NULL",
+ isWithin(1.5849625007211563, 0.01));
+ f.checkNull("log2(cast(null as real))");
+};
+f0.forEachLibrary(list(SqlLibrary.MYSQL, SqlLibrary.SPARK), consumer);
+ }
+
+ /** Test case for
+ * https://issues.apache.org/jira/browse/CALCITE-6232;>[CALCITE-6232]
+ * Using fractions in LOG function does not return correct results. */
+ @Test void testLogFuncByConvert() {
+// The fractional conversion of the Log function is reserved only for
integer bits
+final SqlOperatorFixture f0 = Fixtures.forOperators(true);
+f0.setFor(SqlLibraryOperators.LOG, VmName.EXPAND);
+final SqlOperatorFixture f = f0.withLibrary(SqlLibrary.BIG_QUERY);
+f.checkScalarApprox("log(2/3, 2)", "DOUBLE NOT NULL",
+ "-Infinity");
+f.checkScalarApprox("log(0,2)", "DOUBLE NOT NULL",
+ "-Infinity");
+f.checkScalarApprox("log(1,2)", "DOUBLE NOT NULL",
+"0.0");
+f.checkScalarApprox("log(4/3,2)", "DOUBLE NOT NULL",
+"0.0");
+f.checkScalarApprox("log(5/3,2)", "DOUBLE NOT NULL",
+"0.0");
+f.checkScalarApprox("log(2/3, 10)", "DOUBLE NOT NULL",
+ "-Infinity");
+f.checkScalarApprox("log(0,10)", "DOUBLE NOT NULL",
+ "-Infinity");
+f.checkScalarApprox("log(1,10)", "DOUBLE NOT NULL",
+"0.0");
+f.checkScalarApprox("log(4/3,10)", "DOUBLE NOT NULL",
Review Comment:
@tanclary I'm trying to fix it, but I'm sure it won't last long. I think we
have to be honest and tell the user about this bug. Now maybe our difficulty is
how to tell the user about this bug, should I add a TODO tag
--
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
