You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@calcite.apache.org by GitBox <gi...@apache.org> on 2021/09/14 12:50:38 UTC
[GitHub] [calcite] YuKongEr opened a new pull request #2516: [CALCITE-4772]PushProjector assigns incorrect field alias
YuKongEr opened a new pull request #2516:
URL: https://github.com/apache/calcite/pull/2516
For [CALCITE-4772](https://issues.apache.org/jira/projects/CALCITE/issues/CALCITE-4772?filter=allissues)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] YuKongEr commented on pull request #2516: [CALCITE-4772]PushProjector assigns incorrect field alias
Posted by GitBox <gi...@apache.org>.
YuKongEr commented on pull request #2516:
URL: https://github.com/apache/calcite/pull/2516#issuecomment-923929854
@zabetak thanks, i will fix up
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] xy2953396112 commented on pull request #2516: [CALCITE-4772]PushProjector assigns incorrect field alias
Posted by GitBox <gi...@apache.org>.
xy2953396112 commented on pull request #2516:
URL: https://github.com/apache/calcite/pull/2516#issuecomment-924854922
Please fix CI failures.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] xy2953396112 commented on pull request #2516: [CALCITE-4772]PushProjector assigns incorrect field alias
Posted by GitBox <gi...@apache.org>.
xy2953396112 commented on pull request #2516:
URL: https://github.com/apache/calcite/pull/2516#issuecomment-924854409
Could you update the title `PushProjector should retain alias when handling RexCall`?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] YuKongEr commented on pull request #2516: [CALCITE-4772]PushProjector assigns incorrect field alias
Posted by GitBox <gi...@apache.org>.
YuKongEr commented on pull request #2516:
URL: https://github.com/apache/calcite/pull/2516#issuecomment-924058380
@rubenada oh, get 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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] zabetak commented on pull request #2516: [CALCITE-4772]PushProjector assigns incorrect field alias
Posted by GitBox <gi...@apache.org>.
zabetak commented on pull request #2516:
URL: https://github.com/apache/calcite/pull/2516#issuecomment-923919315
Hey @YuKongEr there are some CI failures. Can you please fix those?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] zabetak commented on a change in pull request #2516: [CALCITE-4772]PushProjector assigns incorrect field alias
Posted by GitBox <gi...@apache.org>.
zabetak commented on a change in pull request #2516:
URL: https://github.com/apache/calcite/pull/2516#discussion_r713819847
##########
File path: core/src/main/java/org/apache/calcite/rel/rules/PushProjector.java
##########
@@ -562,18 +562,30 @@ public Project createProjectRefsAndExprs(
new RexUtil.FixNullabilityShuttle(
projChild.getCluster().getRexBuilder(), typeList);
newExpr = newExpr.accept(fixer);
-
+ String alias = SqlUtil.deriveAliasFromOrdinal(preserveExpOrdinal++);
+ RelDataTypeField relDataTypeField = getRelDataTypeField(projExpr);
+ if (relDataTypeField != null) {
+ alias = relDataTypeField.getName();
+ }
newProjects.add(
- Pair.of(
- newExpr,
- SqlUtil.deriveAliasFromOrdinal(preserveExpOrdinal++)));
+ Pair.of(newExpr, alias));
}
return (Project) relBuilder.push(projChild)
.projectNamed(Pair.left(newProjects), Pair.right(newProjects), true)
.build();
}
+ private @Nullable RelDataTypeField getRelDataTypeField(RexNode originRexNode) {
Review comment:
Rename method to `findOriginalFieldName`?
##########
File path: core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java
##########
@@ -3481,6 +3481,21 @@ private void checkLiteral2(String expression, String expected) {
sql(sql).optimize(rules, null).ok(expected);
}
+ @Test void testMultiplicationKeepAliased() {
Review comment:
Rename to: `testMultiplicationRetainsExplicitAlias`
##########
File path: core/src/main/java/org/apache/calcite/rel/rules/PushProjector.java
##########
@@ -562,18 +562,30 @@ public Project createProjectRefsAndExprs(
new RexUtil.FixNullabilityShuttle(
projChild.getCluster().getRexBuilder(), typeList);
newExpr = newExpr.accept(fixer);
-
+ String alias = SqlUtil.deriveAliasFromOrdinal(preserveExpOrdinal++);
+ RelDataTypeField relDataTypeField = getRelDataTypeField(projExpr);
+ if (relDataTypeField != null) {
+ alias = relDataTypeField.getName();
+ }
newProjects.add(
- Pair.of(
- newExpr,
- SqlUtil.deriveAliasFromOrdinal(preserveExpOrdinal++)));
+ Pair.of(newExpr, alias));
Review comment:
How about the following refactoring?
```
final String originalFieldName = findOriginalFieldName(projExpr);
final String newAlias;
if(originalFieldName != null) {
newAlias = originalFieldName;
} else {
newAlias = SqlUtil.deriveAliasFromOrdinal(preserveExpOrdinal);
}
newProjects.add(Pair.of(newExpr, newAlias));
preserveExpOrdinal++;
```
##########
File path: core/src/main/java/org/apache/calcite/rel/rules/PushProjector.java
##########
@@ -562,18 +562,30 @@ public Project createProjectRefsAndExprs(
new RexUtil.FixNullabilityShuttle(
projChild.getCluster().getRexBuilder(), typeList);
newExpr = newExpr.accept(fixer);
-
+ String alias = SqlUtil.deriveAliasFromOrdinal(preserveExpOrdinal++);
+ RelDataTypeField relDataTypeField = getRelDataTypeField(projExpr);
+ if (relDataTypeField != null) {
+ alias = relDataTypeField.getName();
+ }
newProjects.add(
- Pair.of(
- newExpr,
- SqlUtil.deriveAliasFromOrdinal(preserveExpOrdinal++)));
+ Pair.of(newExpr, alias));
}
return (Project) relBuilder.push(projChild)
.projectNamed(Pair.left(newProjects), Pair.right(newProjects), true)
.build();
}
+ private @Nullable RelDataTypeField getRelDataTypeField(RexNode originRexNode) {
+ if (origProj == null) {
+ return null;
+ }
+ int idx = origProj.getProjects().indexOf(originRexNode);
+ if (idx < 0) {
+ return null;
+ }
+ return origProj.getRowType().getFieldList().get(idx);
Review comment:
Return the name of the field directly?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] YuKongEr commented on pull request #2516: [CALCITE-4772]PushProjector should retain alias when handling RexCall
Posted by GitBox <gi...@apache.org>.
YuKongEr commented on pull request #2516:
URL: https://github.com/apache/calcite/pull/2516#issuecomment-924856198
@xy2953396112 done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] rubenada commented on pull request #2516: [CALCITE-4772]PushProjector assigns incorrect field alias
Posted by GitBox <gi...@apache.org>.
rubenada commented on pull request #2516:
URL: https://github.com/apache/calcite/pull/2516#issuecomment-923971263
@YuKongEr there seems to be a CheckerFramework error.
Tip: if I am not mistaken, you can run CheckerFramework locally (to verify before commit/push) via `gradlew build -x test -PenableCheckerframework`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] xy2953396112 commented on pull request #2516: [CALCITE-4772]PushProjector assigns incorrect field alias
Posted by GitBox <gi...@apache.org>.
xy2953396112 commented on pull request #2516:
URL: https://github.com/apache/calcite/pull/2516#issuecomment-924854600
> Overall LGTM, only some minor comments.
+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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] zabetak closed pull request #2516: [CALCITE-4772]PushProjector should retain alias when handling RexCall
Posted by GitBox <gi...@apache.org>.
zabetak closed pull request #2516:
URL: https://github.com/apache/calcite/pull/2516
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org