Unwrap year in comparison also for IN predicate#18092
Unwrap year in comparison also for IN predicate#18092findepi merged 1 commit intotrinodb:masterfrom kabunchi:unwrap-in
Conversation
There was a problem hiding this comment.
@ebyhr I am not sure I need this line. please advise.
There was a problem hiding this comment.
i think it would be better to default-rewrite (recursive) first, then to extract the attributes, just in case they got rewritten or simplified (don't think it matters currently, matter of style/future-proofing)
InPredicate inPredicate = treeRewriter.defaultRewrite(node, null);
Expression value = inPredicate.getValue();
Expression valueList = inPredicate.getValueList();
There was a problem hiding this comment.
we probably want similar change for UnwrapDateTruncInComparison and UnwrapCastInComparison?
There was a problem hiding this comment.
use ImmutableList.Builder
(per style, but also to avoid copy when creating new LogicalExpression)
There was a problem hiding this comment.
i think it would be better to default-rewrite (recursive) first, then to extract the attributes, just in case they got rewritten or simplified (don't think it matters currently, matter of style/future-proofing)
InPredicate inPredicate = treeRewriter.defaultRewrite(node, null);
Expression value = inPredicate.getValue();
Expression valueList = inPredicate.getValueList();
please avoid mixing changes with rebase. it's hard to notice what chaned. |
There was a problem hiding this comment.
| Expression value = node.getValue(); | |
| Expression valueList = node.getValueList(); | |
| Expression value = inPredicate.getValue(); | |
| Expression valueList = inPredicate.getValueList(); |
There was a problem hiding this comment.
done. I wasn't sure that the returned expression from defaultRewriter must be an InPredicate so I also added a validation.
There was a problem hiding this comment.
- avoid abbreviations ("curr")
comparisonExpressionsListBuilderis a long name and the fact it's listbuilder is known from its typeexpressionsListcan be inlined
| List<Expression> expressionsList = inListExpression.getValues(); | |
| ImmutableList.Builder<Expression> comparisonExpressionsListBuilder = ImmutableList.builder(); | |
| for (Expression currExpression : expressionsList) { | |
| ImmutableList.Builder<Expression> comparisonExpressions = ImmutableList.builderWithExpectedSize(inListExpression.getValues().size()); | |
| for (Expression rightExpression : inListExpression.getValues()) { | |
| ComparisonExpression comparisonExpression = new ComparisonExpression(EQUAL, value, rightExpression); |
core/trino-main/src/test/java/io/trino/sql/planner/TestUnwrapYearInComparison.java
Outdated
Show resolved
Hide resolved
|
@kabunchi thanks, will merge when the build passes |
4 participants
Description
Additional context and related issues
Release notes
(*) This is not user-visible or docs only and no release notes are required.
( ) Release notes are required, please propose a release note for me.
( ) Release notes are required, with the following suggested text: