[GitHub] [calcite] wangxlong commented on a change in pull request #1878: [CALCITE-3782]Bitwise agg operator Bit_And, Bit_OR and Bit_XOR support binary and varbinary type

2020-05-14 Thread GitBox
wangxlong commented on a change in pull request #1878: URL: https://github.com/apache/calcite/pull/1878#discussion_r424891765 ## File path: core/src/main/java/org/apache/calcite/runtime/BitwiseFunctions.java ## @@ -0,0 +1,124 @@ +/* + * Licensed to the Apache Software

[calcite] branch master updated: [CALCITE-3979] Simplification might have removed CAST expression(s) incorrectly

2020-05-14 Thread kgyrtkirk
This is an automated email from the ASF dual-hosted git repository. kgyrtkirk pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/calcite.git The following commit(s) were added to refs/heads/master by this push: new ffa22f7 [CALCITE-3979] Simplification

[GitHub] [calcite] kgyrtkirk merged pull request #1969: [CALCITE-3979] Simplification might have removed CAST expressi…

2020-05-14 Thread GitBox
kgyrtkirk merged pull request #1969: URL: https://github.com/apache/calcite/pull/1969 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

[GitHub] [calcite] DonnyZone commented on pull request #1974: [CALCITE-3985] Simplify grouped window function in parser

2020-05-14 Thread GitBox
DonnyZone commented on pull request #1974: URL: https://github.com/apache/calcite/pull/1974#issuecomment-628411750 @amaliujia To avoid conflicts, I'd like to merge it after 1.23.0's release. Nitpicking: we'd better remove the period at the end of the commit message.

[GitHub] [calcite] jinxing64 commented on pull request #1560: [CALCITE-3478] Restructure tests for materialized views

2020-05-14 Thread GitBox
jinxing64 commented on pull request #1560: URL: https://github.com/apache/calcite/pull/1560#issuecomment-628439046 Since @hsyuan gives +1 for this change. I will add a LGTM-will-merge-soon tag. Welcome further comments and I will update ASAP before merging.

[GitHub] [calcite] hsyuan closed pull request #1976: [CALCITE-3997] Logical rules matched with physical operators but failed to handle traits

2020-05-14 Thread GitBox
hsyuan closed pull request #1976: URL: https://github.com/apache/calcite/pull/1976 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

[calcite] branch master updated: [CALCITE-3997] Logical rules matched with physical operators but failed to handle traits

2020-05-14 Thread hyuan
This is an automated email from the ASF dual-hosted git repository. hyuan pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/calcite.git The following commit(s) were added to refs/heads/master by this push: new 7be30db [CALCITE-3997] Logical rules matched

[GitHub] [calcite] hsyuan commented on pull request #1976: [CALCITE-3997] Logical rules matched with physical operators but failed to handle traits

2020-05-14 Thread GitBox
hsyuan commented on pull request #1976: URL: https://github.com/apache/calcite/pull/1976#issuecomment-628834771 The only possible case is the user's special rule creates a physical operator that is a sub-class of Enumerable operator, at the same use Calcite's built-in logical rule to

[GitHub] [calcite] xndai commented on pull request #1976: [CALCITE-3997] Logical rules matched with physical operators but failed to handle traits

2020-05-14 Thread GitBox
xndai commented on pull request #1976: URL: https://github.com/apache/calcite/pull/1976#issuecomment-628816637 If one physical node is created by a rule, rather than being converted from a logical node, then there's no logical counterpart and won't be processed by your transformation

[GitHub] [calcite] hsyuan commented on pull request #1976: [CALCITE-3997] Logical rules matched with physical operators but failed to handle traits

2020-05-14 Thread GitBox
hsyuan commented on pull request #1976: URL: https://github.com/apache/calcite/pull/1976#issuecomment-628858500 The restriction only applies operator that implements `PhysicalNode`, which is a newly introduced interface, no one has the chance to implement it until 1.23.0 is released.

[GitHub] [calcite] hsyuan closed pull request #1958: [CALCITE-3968] Disable JoinPushThroughJoinRule.right by default

2020-05-14 Thread GitBox
hsyuan closed pull request #1958: URL: https://github.com/apache/calcite/pull/1958 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

[GitHub] [calcite] xndai commented on pull request #1976: [CALCITE-3997] Logical rules matched with physical operators but failed to handle traits

2020-05-14 Thread GitBox
xndai commented on pull request #1976: URL: https://github.com/apache/calcite/pull/1976#issuecomment-628856256 Not necessarily a sub-class of Enumerable. It could be a sub-class of the base RelNode, such as Project or Aggregate.

[GitHub] [calcite] hsyuan commented on pull request #1958: [CALCITE-3968] Disable JoinPushThroughJoinRule.right by default

2020-05-14 Thread GitBox
hsyuan commented on pull request #1958: URL: https://github.com/apache/calcite/pull/1958#issuecomment-628887415 Closing this PR, since this has been superseded by https://github.com/apache/calcite/pull/1976. This is an

[GitHub] [calcite] hsyuan commented on pull request #1976: [CALCITE-3997] Logical rules matched with physical operators but failed to handle traits

2020-05-14 Thread GitBox
hsyuan commented on pull request #1976: URL: https://github.com/apache/calcite/pull/1976#issuecomment-628802576 If it can match physical operators, that means it is already matches with logical operators, it just generates redundant operators. What scenario will it break?

[GitHub] [calcite] hsyuan commented on a change in pull request #1973: [CALCITE-3993] Add utility methods to RelTrait, RelDistribution and RelCollation

2020-05-14 Thread GitBox
hsyuan commented on a change in pull request #1973: URL: https://github.com/apache/calcite/pull/1973#discussion_r424842177 ## File path: core/src/main/java/org/apache/calcite/plan/RelTraitSet.java ## @@ -417,13 +420,34 @@ public int size() { * @return true if traits are

[GitHub] [calcite] eolivelli commented on pull request #1976: [CALCITE-3997] Logical rules matched with physical operators but failed to handle traits

2020-05-14 Thread GitBox
eolivelli commented on pull request #1976: URL: https://github.com/apache/calcite/pull/1976#issuecomment-628737545 @hsyuan it looks like you modified 92 files... IMHO it would be better to narrow down the patch to the minimal set of changes, in order to prevent conflicts with other

[GitHub] [calcite] eolivelli commented on pull request #1976: [CALCITE-3997] Logical rules matched with physical operators but failed to handle traits

2020-05-14 Thread GitBox
eolivelli commented on pull request #1976: URL: https://github.com/apache/calcite/pull/1976#issuecomment-628737002 @hsyuan thank you ! @vlsi Do you know how to build locally this branch and put the jars into local Maven repository ? this way I can test this branch against my

[GitHub] [calcite] eolivelli commented on pull request #1976: [CALCITE-3997] Logical rules matched with physical operators but failed to handle traits

2020-05-14 Thread GitBox
eolivelli commented on pull request #1976: URL: https://github.com/apache/calcite/pull/1976#issuecomment-628738883 btw I see now that you added a marker interface. It is a big change. But I don't have enough context in order to give a suggestion

[GitHub] [calcite] hsyuan commented on pull request #1976: [CALCITE-3997] Logical rules matched with physical operators but failed to handle traits

2020-05-14 Thread GitBox
hsyuan commented on pull request #1976: URL: https://github.com/apache/calcite/pull/1976#issuecomment-628739137 There won't be conflicts. It is the thorough fix. This is an automated message from the Apache Git Service. To

[GitHub] [calcite] xndai commented on pull request #1976: [CALCITE-3997] Logical rules matched with physical operators but failed to handle traits

2020-05-14 Thread GitBox
xndai commented on pull request #1976: URL: https://github.com/apache/calcite/pull/1976#issuecomment-628783691 How do you guarantee only logical nodes are generated in transformation rule? Also please make sure to include this in the release note as a potential breaking change.

[GitHub] [calcite] hsyuan commented on pull request #1976: [CALCITE-3997] Logical rules matched with physical operators but failed to handle traits

2020-05-14 Thread GitBox
hsyuan commented on pull request #1976: URL: https://github.com/apache/calcite/pull/1976#issuecomment-628792593 > How do you guarantee only logical nodes are generated in transformation rule? It is allowed, but not encouraged. Just a contract, unless we add a check inside

[GitHub] [calcite] xndai commented on pull request #1976: [CALCITE-3997] Logical rules matched with physical operators but failed to handle traits

2020-05-14 Thread GitBox
xndai commented on pull request #1976: URL: https://github.com/apache/calcite/pull/1976#issuecomment-628798575 If this's something we believe should be enforced, why don't we just add such check? There are a number of rules you can be applied to both logical and physical nodes