[GitHub] [calcite] tisonkun edited a comment on pull request #2411: Add generic info to Map & Array annotation
tisonkun edited a comment on pull request #2411: URL: https://github.com/apache/calcite/pull/2411#issuecomment-848410856 @rubenada @zabetak @amaliujia thanks for your time! > Just out of curiosity: what is the benefit to have generic info? @amaliujia I think adding generic parameter when there should be one is a general best practice. It is useful as linter in this case. I created this pull request on a day reading the source code and linter complain. The community may or may not reject such pr and require to be included in a larger one, but I prefer we accept valid contributions at any level. If you turn a blind eye to generic parameter missing, it is possible we are gonna run into a case where Apache Hudi is, that it is hard to fix missing generic parameters that really hurt. -- 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [calcite] tisonkun commented on pull request #2411: Add generic info to Map & Array annotation
tisonkun commented on pull request #2411: URL: https://github.com/apache/calcite/pull/2411#issuecomment-848410856 @rubenada @zabetak @amaliujia thanks for your time! > Just out of curiosity: what is the benefit to have generic info? @amaliujia I think adding generic parameter when there should be one is a general best practice. It is useful as linter in this case. I created this pull request on a day reading the source code and linter complain. The community may or may not reject such pr and require to be included in a larger one, but I prefer we accept valid contributions at any level. If you turn a blind eye to generic parameter missing, it is possible we run into a case where Apache Hudi is, that it is hard to fix missing generic parameters that really hurt. -- 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [calcite] jinxing64 closed pull request #1468: [CALCITE-3366] RelDecorrelator supports Union
jinxing64 closed pull request #1468: URL: https://github.com/apache/calcite/pull/1468 -- 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [calcite] amaliujia commented on a change in pull request #2411: Add generic info to Map & Array annotation
amaliujia commented on a change in pull request #2411:
URL: https://github.com/apache/calcite/pull/2411#discussion_r639136087
##
File path: core/src/main/java/org/apache/calcite/adapter/java/Map.java
##
@@ -16,23 +16,22 @@
*/
package org.apache.calcite.adapter.java;
+import java.lang.annotation.ElementType;
import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;
import java.lang.annotation.Target;
-import static java.lang.annotation.ElementType.FIELD;
-
/**
* Annotation that indicates that a field is a map type.
*/
-@Target({FIELD })
+@Target(ElementType.FIELD)
@Retention(RetentionPolicy.RUNTIME)
public @interface Map {
/** Key type. */
- Class key();
+ Class key();
Review comment:
What is changed after having `Class`?
--
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [calcite] amaliujia commented on pull request #2411: Add generic info to Map & Array annotation
amaliujia commented on pull request #2411: URL: https://github.com/apache/calcite/pull/2411#issuecomment-848213486 Just out of curiosity: what is the benefit to have generic info? -- 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [calcite] jamesstarr commented on pull request #2385: [CALCITE-1045] Supporting SubQueryRemoveRule for Joins
jamesstarr commented on pull request #2385: URL: https://github.com/apache/calcite/pull/2385#issuecomment-848194654 @Zabetak, This PR is focused on handling left joins with correlated queries in ON clauses. Joins with correlated queries are rewritten too as join with the right side contain filter node and correlate node to handle the join's filter in SubQueryRemovelRule. The filter is checked in a loop for correlation ID to handle multiple subqueries. Right and full outer joins are not rewritten because they would logically be incorrect. Shifter, a util class in RelBuilder, was moved to RelOptUtil so the existing logic could be used in SubQueryRemoveRule. RelBuilder also now only considers a join correlated if an ID is provided and actually referenced is found on the right side to the ID. -- 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [calcite] zabetak commented on pull request #2385: [CALCITE-1045] Supporting SubQueryRemoveRule for Joins
zabetak commented on pull request #2385: URL: https://github.com/apache/calcite/pull/2385#issuecomment-847990347 Hey @jamesstarr , thanks for working on this. Going quickly over the changeset I get the impression that the PR tries to achieve multiple things. Can you highlight/outline the goals of this PR either here or under CALCITE-1045? -- 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [calcite] zabetak closed pull request #2217: CALCITE-4340 WIP (James Starr)
zabetak closed pull request #2217: URL: https://github.com/apache/calcite/pull/2217 -- 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [calcite] zabetak commented on pull request #2217: CALCITE-4340 WIP (James Starr)
zabetak commented on pull request #2217: URL: https://github.com/apache/calcite/pull/2217#issuecomment-847970264 I assume that https://github.com/apache/calcite/pull/2364 supersedes this one so I am closing it. @jamesstarr if that's not the case please re-open. -- 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [calcite] scrozon commented on a change in pull request #2417: [CALCITE-4617] Wrong offset when SortJoinTransposeRule pushes a sort node with an offset
scrozon commented on a change in pull request #2417: URL: https://github.com/apache/calcite/pull/2417#discussion_r638839071 ## File path: core/src/test/resources/org/apache/calcite/test/RelOptRulesTest.xml ## @@ -11751,7 +11751,7 @@ LogicalProject(DEPTNO=[$0], EMPNO=[$2])
[GitHub] [calcite] hannerwang commented on pull request #2408: [CALCITE-4512] Optimize when GROUP BY or HAVING alias should be substituted with select item
hannerwang commented on pull request #2408: URL: https://github.com/apache/calcite/pull/2408#issuecomment-847900410 > The commit message not in the proper way. Yes, I think the Jira title can't summarize what the commit has complete, so I leave a comment at first. -- 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [calcite] rubenada commented on a change in pull request #2417: [CALCITE-4617] Wrong offset when SortJoinTransposeRule pushes a sort node with an offset
rubenada commented on a change in pull request #2417: URL: https://github.com/apache/calcite/pull/2417#discussion_r638801193 ## File path: core/src/test/resources/org/apache/calcite/test/RelOptRulesTest.xml ## @@ -11751,7 +11751,7 @@ LogicalProject(DEPTNO=[$0], EMPNO=[$2])
[GitHub] [calcite] rubenada commented on a change in pull request #2417: [CALCITE-4617] Wrong offset when SortJoinTransposeRule pushes a sort node with an offset
rubenada commented on a change in pull request #2417: URL: https://github.com/apache/calcite/pull/2417#discussion_r638801193 ## File path: core/src/test/resources/org/apache/calcite/test/RelOptRulesTest.xml ## @@ -11751,7 +11751,7 @@ LogicalProject(DEPTNO=[$0], EMPNO=[$2])
[GitHub] [calcite] scrozon commented on a change in pull request #2417: [CALCITE-4617] Wrong offset when SortJoinTransposeRule pushes a sort node with an offset
scrozon commented on a change in pull request #2417: URL: https://github.com/apache/calcite/pull/2417#discussion_r638763790 ## File path: core/src/test/resources/org/apache/calcite/test/RelOptRulesTest.xml ## @@ -11751,7 +11751,7 @@ LogicalProject(DEPTNO=[$0], EMPNO=[$2])
[GitHub] [calcite] rubenada commented on pull request #2411: Add generic info to Map & Array annotation
rubenada commented on pull request #2411: URL: https://github.com/apache/calcite/pull/2411#issuecomment-847844526 Thanks for the confirmation @zabetak -- 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [calcite] rubenada opened a new pull request #2418: [CALCITE-4621] SemiJoinRule throws AssertionError on ANTI join
rubenada opened a new pull request #2418: URL: https://github.com/apache/calcite/pull/2418 Jira: [CALCITE-4621](https://issues.apache.org/jira/browse/CALCITE-4621) -- 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [calcite] zabetak commented on pull request #2411: Add generic info to Map & Array annotation
zabetak commented on pull request #2411: URL: https://github.com/apache/calcite/pull/2411#issuecomment-847821775 @rubenada changes with small impact to end-users can go in without 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [calcite] rubenada commented on pull request #2411: Add generic info to Map & Array annotation
rubenada commented on pull request #2411: URL: https://github.com/apache/calcite/pull/2411#issuecomment-847740202 This patch could be merged, but there is no associated Jira. Since it is a small change, perhaps we could make an exception, what do you think @zabetak ? -- 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [calcite] Aaaaaaron commented on pull request #2408: [CALCITE-4512] Optimize when GROUP BY or HAVING alias should be substituted with select item
Aaron commented on pull request #2408: URL: https://github.com/apache/calcite/pull/2408#issuecomment-847583880 The commit message not in the proper way. -- 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
