twalthr commented on code in PR #28290:
URL: https://github.com/apache/flink/pull/28290#discussion_r3450747566


##########
flink-table/flink-table-planner/src/main/java/org/apache/calcite/rex/RexBuilder.java:
##########
@@ -86,10 +87,6 @@
  * Factory for row expressions.
  *
  * <p>Some common literal values (NULL, TRUE, FALSE, 0, 1, '') are cached.
- *
- * <p>FLINK modifications (backport of CALCITE-6764): Lines 237, 243, 247, 251 
~ 255
- *
- * <p>FLINK modifications (backport of FLINK-39945): Lines 1270-1284

Review Comment:
   Shouldn't these comments be preserved?



##########
flink-table/flink-table-planner/src/test/resources/org/apache/flink/table/planner/plan/batch/sql/DynamicFunctionPlanTest.xml:
##########
@@ -34,10 +34,11 @@ LogicalAggregate(group=[{0, 1}], EXPR$2=[SUM($2)], 
EXPR$3=[COUNT()])
     </Resource>
     <Resource name="optimized exec plan">
       <![CDATA[
-HashAggregate(isMerge=[false], groupBy=[cat], auxGrouping=[gmt_date], 
select=[cat, gmt_date, SUM(cnt) AS EXPR$2, COUNT(*) AS EXPR$3])
-+- Exchange(distribution=[hash[cat]])
-   +- Calc(select=[cat, gmt_date, cnt], where=[(gmt_date = CURRENT_DATE())])
-      +- TableSourceScan(table=[[default_catalog, default_database, src, 
filter=[], project=[cat, gmt_date, cnt], metadata=[]]], fields=[cat, gmt_date, 
cnt])
+Calc(select=[cat, CAST(CURRENT_DATE() AS DATE) AS gmt_date, EXPR$2, EXPR$3])

Review Comment:
   This optimization is a bit fishy. Will it also be this way for streaming? 
For batch I guess it reduces traffic during aggregation. But for streaming it 
will return results that are not so easy to understand, the aggregation will 
happen on a different date than the one that is shown in the output.



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to