[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.
   see 
https://github.com/apache/calcite/commit/7be30db36d449e0a7fcc76b7d4647e141f4bc72d#diff-70d5035797baebe157bab8cbf860e456R415



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] 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 process it. 
   Sounds rare but fare, I will add it in the release notes.



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] 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?



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] 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 `call.transformTo`, which can be added in next version.
   
   > Also please make sure to include this in the release note as a potential 
breaking change.
   
   What do you think it will break?
   



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] 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 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