[CALCITE-7345] Quidem test support for Field Trimmer #4708
Conversation
| int thresholdValue = ((BigDecimal) value).intValue(); | ||
| closer.add(Prepare.THREAD_INSUBQUERY_THRESHOLD.push(thresholdValue)); | ||
| } | ||
| if (propertyName.equals("trimfields")) { |
There was a problem hiding this comment.
what I don't understand is "at which point during compilation is field trimming executed"?
The result will depend on the optimizations that are executed before and after.
There was a problem hiding this comment.
The field trimmer is executed in two phases: the first one occurs after the SQL is converted into a Rel, and the second one takes place during the optimization of the RelNode.
xiedeyantu
left a comment
xiedeyantu
left a comment
There was a problem hiding this comment.
Look fine, only one small problem remains.
| if (propertyName.equals("trimfields")) { | ||
| final boolean b = value instanceof Boolean | ||
| && (Boolean) value; | ||
| closer.add( |
There was a problem hiding this comment.
It might be the same issue as with Mihai: will there be two places in the entire process where Trimer is performed?
There was a problem hiding this comment.
Yes. The field trimmer occurs twice.
There was a problem hiding this comment.
I have a question that might not be directly related to the PR: why is column pruning necessary in both phases?
There was a problem hiding this comment.
The first fields trimmer occurs after the SQL is converted into a RelNode. At this stage, the RelNode should be simplified before optimization, making subsequent optimization plans more lightweight. The second fields trimmer happens during or after optimization—during this phase, certain columns may no longer be needed. For example, a condition like (age <> 5 OR age = 5) can be directly simplified to True, allowing the age column to be eliminated.
There was a problem hiding this comment.
Thank you for your detailed explanation. We'll wait and see if @mihaibudiu has any further comments; if not, I think you can merge them anytime.
|
3 participants
No description provided.