wenlong88 commented on a change in pull request #14562:
URL: https://github.com/apache/flink/pull/14562#discussion_r552391242
##########
File path:
flink-table/flink-table-planner-blink/src/main/scala/org/apache/flink/table/planner/plan/metadata/FlinkRelMdModifiedMonotonicity.scala
##########
@@ -352,7 +352,7 @@ class FlinkRelMdModifiedMonotonicity private extends
MetadataHandler[ModifiedMon
// if group by an update field or group by a field mono is null, just
return null
if (inputMonotonicity == null ||
- grouping.exists(e => inputMonotonicity.fieldMonotonicities(e) !=
CONSTANT)) {
+ grouping.exists(e => inputMonotonicity.fieldMonotonicities(e) !=
CONSTANT)) {
Review comment:
the old format is better
##########
File path:
flink-table/flink-table-planner-blink/src/main/scala/org/apache/flink/table/planner/plan/metadata/FlinkRelMdModifiedMonotonicity.scala
##########
@@ -373,7 +373,7 @@ class FlinkRelMdModifiedMonotonicity private extends
MetadataHandler[ModifiedMon
// if group by a update field or group by a field mono is null, just
return null
if (inputMonotonicity == null ||
- grouping.exists(e => inputMonotonicity.fieldMonotonicities(e) !=
CONSTANT)) {
+ grouping.exists(e => inputMonotonicity.fieldMonotonicities(e) !=
CONSTANT)) {
Review comment:
ditto
##########
File path:
flink-table/flink-table-planner-blink/src/main/scala/org/apache/flink/table/planner/plan/metadata/FlinkRelMdModifiedMonotonicity.scala
##########
@@ -398,7 +398,7 @@ class FlinkRelMdModifiedMonotonicity private extends
MetadataHandler[ModifiedMon
val childMono =
inputMonotonicity.fieldMonotonicities(aggCall.getArgList.head)
val currentMono = fieldMonotonicities(index)
if (childMono != currentMono &&
- !aggCall.getAggregation.isInstanceOf[SqlCountAggFunction]) {
+ !aggCall.getAggregation.isInstanceOf[SqlCountAggFunction]) {
Review comment:
ditto
##########
File path:
flink-table/flink-table-planner-blink/src/main/java/org/apache/flink/table/planner/plan/nodes/exec/utils/ExecNodeUtil.java
##########
@@ -35,6 +38,11 @@
/** An Utility class that helps translating {@link ExecNode} to {@link
Transformation}. */
public class ExecNodeUtil {
+ /** Return bytes size for given option in {@link TableConfig}. */
+ public static long getMemorySize(TableConfig tableConfig,
ConfigOption<String> option) {
+ return
MemorySize.parse(tableConfig.getConfiguration().getString(option)).getBytes();
Review comment:
ConfigOption supports type of MemorySize, can we just use
it?,ref:FRAMEWORK_HEAP_MEMORY
----------------------------------------------------------------
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]
