You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by GitBox <gi...@apache.org> on 2021/01/06 06:24:50 UTC
[GitHub] [flink] wenlong88 commented on a change in pull request #14562: [FLINK-20738][table-planner-blink] Separate the implementation of batch group aggregate nodes
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:
users@infra.apache.org