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/08/19 18:37:32 UTC
[GitHub] [calcite] yingyuwang opened a new pull request #2502: [CALCITE-2736] ReduceExpressionsRule never reduces dynamic expressions but this should be configurable (Yingyu Wanag)
yingyuwang opened a new pull request #2502:
URL: https://github.com/apache/calcite/pull/2502
1. Add new config option treatDynamicCallsAsNonConstant.
2. Update ReducibleExprLocator.analyzeCall() method to also rely on
treatDynamicCallsAsNonConstant option when determining whether to
reduce a dynamic 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-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] asfgit closed pull request #2502: [CALCITE-2736] ReduceExpressionsRule never reduces dynamic expressions but this should be configurable (Yingyu Wang)
Posted by GitBox <gi...@apache.org>.
asfgit closed pull request #2502:
URL: https://github.com/apache/calcite/pull/2502
--
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] yingyuwang commented on a change in pull request #2502: [CALCITE-2736] ReduceExpressionsRule never reduces dynamic expressions but this should be configurable (Yingyu Wang)
Posted by GitBox <gi...@apache.org>.
yingyuwang commented on a change in pull request #2502:
URL: https://github.com/apache/calcite/pull/2502#discussion_r693074381
##########
File path: core/src/main/java/org/apache/calcite/rel/rules/ReduceExpressionsRule.java
##########
@@ -1173,6 +1180,14 @@ private void analyzeCall(RexCall call, Constancy callConstancy) {
/** Sets {@link #matchNullability()}. */
Config withMatchNullability(boolean matchNullability);
+ /** Whether to treat dynamic functions as non-constants. */
+ @ImmutableBeans.Property
+ @ImmutableBeans.BooleanDefault(true)
+ boolean treatDynamicCallsAsNonConstant();
Review comment:
Agreed. I prefer not using negative naming too. I've renamed it to `treatDynamicCallsAsConstants` as suggested.
--
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 #2502: [CALCITE-2736] ReduceExpressionsRule never reduces dynamic expressions but this should be configurable (Yingyu Wang)
Posted by GitBox <gi...@apache.org>.
xy2953396112 commented on pull request #2502:
URL: https://github.com/apache/calcite/pull/2502#issuecomment-903038783
please squash commits.
--
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] vlsi commented on a change in pull request #2502: [CALCITE-2736] ReduceExpressionsRule never reduces dynamic expressions but this should be configurable (Yingyu Wang)
Posted by GitBox <gi...@apache.org>.
vlsi commented on a change in pull request #2502:
URL: https://github.com/apache/calcite/pull/2502#discussion_r692491232
##########
File path: core/src/main/java/org/apache/calcite/rel/rules/ReduceExpressionsRule.java
##########
@@ -1173,6 +1180,14 @@ private void analyzeCall(RexCall call, Constancy callConstancy) {
/** Sets {@link #matchNullability()}. */
Config withMatchNullability(boolean matchNullability);
+ /** Whether to treat dynamic functions as non-constants. */
+ @ImmutableBeans.Property
+ @ImmutableBeans.BooleanDefault(true)
+ boolean treatDynamicCallsAsNonConstant();
Review comment:
What if the naming was `treatDynamicCallsAsConstants` with the default value of `false`?
It looks like `not` adds complexity like "oh, what else could it be besides constants?"
--
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] yingyuwang commented on pull request #2502: [CALCITE-2736] ReduceExpressionsRule never reduces dynamic expressions but this should be configurable (Yingyu Wang)
Posted by GitBox <gi...@apache.org>.
yingyuwang commented on pull request #2502:
URL: https://github.com/apache/calcite/pull/2502#issuecomment-904136870
> please squash commits.
I cleaned up my branch commit history by reset the branch head to upstream/master and replayed the changes on this branch.
--
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