beyond1920 commented on a change in pull request #15997: URL: https://github.com/apache/flink/pull/15997#discussion_r640272825
########## File path: flink-table/flink-table-planner-blink/src/test/resources/org/apache/flink/table/planner/plan/rules/physical/stream/WatermarkAssignerChangelogNormalizeTransposeRuleTest.xml ########## @@ -0,0 +1,154 @@ +<?xml version="1.0" ?> +<!-- +Licensed to the Apache Software Foundation (ASF) under one or more +contributor license agreements. See the NOTICE file distributed with +this work for additional information regarding copyright ownership. +The ASF licenses this file to you under the Apache License, Version 2.0 +(the "License"); you may not use this file except in compliance with +the License. You may obtain a copy of the License at + +http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +--> +<Root> + <TestCase name="testNothingHappenWithCalc"> + <Resource name="sql"> + <![CDATA[ +SELECT +TUMBLE_START(currency_time, INTERVAL '5' SECOND) as w_start, +TUMBLE_END(currency_time, INTERVAL '5' SECOND) as w_end, +MAX(rate) AS max_rate +FROM src_with_computed_column +GROUP BY TUMBLE(currency_time, INTERVAL '5' SECOND) +]]> + </Resource> + <Resource name="ast"> + <![CDATA[ +LogicalProject(w_start=[TUMBLE_START($0)], w_end=[TUMBLE_END($0)], max_rate=[$1]) ++- LogicalAggregate(group=[{0}], max_rate=[MAX($1)]) + +- LogicalProject($f0=[$TUMBLE($4, 5000:INTERVAL SECOND)], rate=[$2]) + +- LogicalWatermarkAssigner(rowtime=[currency_time], watermark=[-($4, 5000:INTERVAL SECOND)]) + +- LogicalProject(currency=[$0], currency_no=[$1], rate=[$2], c=[$3], currency_time=[TO_TIMESTAMP($3)]) + +- LogicalTableScan(table=[[default_catalog, default_database, src_with_computed_column]]) +]]> + </Resource> + <Resource name="optimized rel plan"> + <![CDATA[ +Calc(select=[w$start AS w_start, w$end AS w_end, max_rate], changelogMode=[I]) ++- GroupWindowAggregate(window=[TumblingGroupWindow('w$, currency_time, 5000)], properties=[w$start, w$end, w$rowtime, w$proctime], select=[MAX(rate) AS max_rate, start('w$) AS w$start, end('w$) AS w$end, rowtime('w$) AS w$rowtime, proctime('w$) AS w$proctime], changelogMode=[I]) + +- Exchange(distribution=[single], changelogMode=[I,UB,UA,D]) + +- WatermarkAssigner(rowtime=[currency_time], watermark=[-(currency_time, 5000:INTERVAL SECOND)], changelogMode=[I,UB,UA,D]) Review comment: The original fix logical is : 1. If shuffle keys of Exchange are all kept in the output rowType of Calc, push down Calc and WatermarkAssigner as a whole 2. else if if row time field comes from input directly instead of computed column, push down Watermark alone 3. else do nothing After discuss with @godfreyhe , we should push down watermarkAssigner no matter in any case. So we decide to updates the 3 from `do nothing ` to a. Creates a new Calc with complete shuffle key and new watermark assigner b. push them down as a whole. And after `exchange` c. add a new Calc to remove shuffle key added in step a -- 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: [email protected]
