fsk119 commented on code in PR #23809:
URL: https://github.com/apache/flink/pull/23809#discussion_r1406993946
##########
flink-table/flink-sql-client/src/main/java/org/apache/flink/table/client/cli/CliTableauResultView.java:
##########
@@ -128,8 +128,13 @@ private void printBatchResults(AtomicInteger
receivedRowCount) {
resultDescriptor.getRowDataStringConverter(),
resultDescriptor.maxColumnWidth(),
false,
- false);
- style.print(resultRows.iterator(), terminal.writer());
+ false,
+ resultDescriptor.isPrintQueryTimeCost());
+ style.print(resultRows.iterator(), terminal.writer(),
getQueryBeginTime());
+ }
+
+ long getQueryBeginTime() {
+ return System.currentTimeMillis();
Review Comment:
If we get the time here, I think we don't take the submission time into the
consideration.
##########
flink-table/flink-table-common/src/main/java/org/apache/flink/table/utils/print/TableauStyle.java:
##########
@@ -92,12 +95,14 @@ public final class TableauStyle implements PrintStyle {
int[] columnWidths,
int maxColumnWidth,
boolean printNullAsEmpty,
- boolean printRowKind) {
+ boolean printRowKind,
+ boolean printQueryTimeCost) {
Review Comment:
`printQueryTimeCost` relies on the input parameter of the `print` method. So
I just think whether it's better don't introduce this parameter here?
##########
flink-table/flink-table-common/src/main/java/org/apache/flink/table/utils/print/TableauStyle.java:
##########
@@ -117,6 +122,10 @@ public final class TableauStyle implements PrintStyle {
@Override
public void print(Iterator<RowData> it, PrintWriter printWriter) {
+ print(it, printWriter, -1);
Review Comment:
If -1 is a magic number, it's better to introduce a special variable to
represent its meaning.
##########
flink-table/flink-table-api-java/src/main/java/org/apache/flink/table/api/config/TableConfigOptions.java:
##########
@@ -125,6 +126,14 @@ private TableConfigOptions() {}
+ "This only applies to columns with
variable-length types (e.g. CHAR, VARCHAR, STRING) in the streaming mode. "
+ "Fixed-length types are printed in the
batch mode using a deterministic column width.");
+ @Documentation.TableOption(execMode = Documentation.ExecMode.BATCH)
+ public static final ConfigOption<Boolean> DISPLAY_QUERY_TIME_COST =
+ ConfigOptions.key("table.display.query-time-cost")
Review Comment:
I think we introduce an option that works for the sql client only? If so, I
perfer to move it into the sql client options right now.
BTW, I think it also influence other SQL statement behaviour. What about
`sql-client.display.print-time-cost`?
##########
flink-table/flink-table-api-java/src/main/java/org/apache/flink/table/api/config/TableConfigOptions.java:
##########
@@ -125,6 +126,14 @@ private TableConfigOptions() {}
+ "This only applies to columns with
variable-length types (e.g. CHAR, VARCHAR, STRING) in the streaming mode. "
+ "Fixed-length types are printed in the
batch mode using a deterministic column width.");
+ @Documentation.TableOption(execMode = Documentation.ExecMode.BATCH)
+ public static final ConfigOption<Boolean> DISPLAY_QUERY_TIME_COST =
+ ConfigOptions.key("table.display.query-time-cost")
+ .booleanType()
+ .defaultValue(false)
Review Comment:
I think we mark the default value true here. Becaue Hive or Presto both
prints time cost by default.
You can refer to
[hive](https://github.com/apache/hive/blob/master/beeline/src/java/org/apache/hive/beeline/BeeLineOpts.java#L86)
for more details
##########
flink-table/flink-table-common/src/main/java/org/apache/flink/table/utils/print/TableauStyle.java:
##########
@@ -117,6 +122,10 @@ public final class TableauStyle implements PrintStyle {
@Override
public void print(Iterator<RowData> it, PrintWriter printWriter) {
+ print(it, printWriter, -1);
+ }
+
+ public void print(Iterator<RowData> it, PrintWriter printWriter, long
queryBeginTime) {
Review Comment:
Do we need to modify this here. Please take a look at
`CliTableauResultView`. SQL Client controls the print style by itself in
streaming mode.
--
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: issues-unsubscr...@flink.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
