[ 
https://issues.apache.org/jira/browse/CALCITE-3183?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16882387#comment-16882387
 ] 

Juhwan Kim commented on CALCITE-3183:
-------------------------------------

[~julianhyde] Thanks for the review. I addressed them in the new PR. One 
additional thing I found was that, since builder simplifies condition before 
passing it to the constructor, trimming result from this could be different in 
some cases, which caused two test failures(one in misc.iq and one in 
GeodeAllDataTypesTest). I don't think there would be problems with this, so I 
updated expected result for those two failures. If this could be problematic, 
then, I think we could add a builder method that directly creates filter using 
a given condition.

> Trimming method for Filter rel uses wrong traitSet 
> ---------------------------------------------------
>
>                 Key: CALCITE-3183
>                 URL: https://issues.apache.org/jira/browse/CALCITE-3183
>             Project: Calcite
>          Issue Type: Bug
>            Reporter: Juhwan Kim
>            Assignee: Juhwan Kim
>            Priority: Major
>              Labels: pull-request-available
>          Time Spent: 1h 40m
>  Remaining Estimate: 0h
>
> It seems like there is a bug here: 
> https://github.com/apache/calcite/blob/e8d598a434e8dbadaf756f8c57c748f4d7e16fdf/core/src/main/java/org/apache/calcite/sql2rel/RelFieldTrimmer.java#L487.
> Unlike other trimming methods, filter trim function copies the current filter 
> rel and directly pushes it to the builder instead of calling factory method 
> for filter rel. The problem with the current code is that it uses the same 
> traitSet even though it would no longer be valid after trimming its input. 
> For example, fields in collation might have been updated after trimming. We 
> should reflect this change when creating a new rel.
>  



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

Reply via email to