fsk119 commented on code in PR #23809:
URL: https://github.com/apache/flink/pull/23809#discussion_r1408879750
##########
flink-table/flink-sql-client/src/main/java/org/apache/flink/table/client/cli/CliTableauResultView.java:
##########
@@ -128,8 +146,9 @@ private void printBatchResults(AtomicInteger
receivedRowCount) {
resultDescriptor.getRowDataStringConverter(),
resultDescriptor.maxColumnWidth(),
false,
- false);
- style.print(resultRows.iterator(), terminal.writer());
+ false,
Review Comment:
Can we do like this? The following modification doesn't influence the table
api behaviour and we can control the timeout by ourself. `TableauStyle` convert
the RowData to String[] twice in the `print` method.
```
private void printBatchResults(AtomicInteger receivedRowCount) {
final List<RowData> resultRows = waitBatchResults();
receivedRowCount.addAndGet(resultRows.size());
if (resultRows.isEmpty()) {
terminal.writer()
.println(
"Empty set."
+ calculateTimeCostInPrintFormat(
queryBeginTime,
System.currentTimeMillis()));
terminal.flush();
return;
}
TableauStyle style =
PrintStyle.tableauWithDataInferredColumnWidths(
resultDescriptor.getResultSchema(),
resultDescriptor.getRowDataStringConverter(),
resultDescriptor.maxColumnWidth(),
false,
false,
resultDescriptor.isPrintQueryTimeCost());
List<String[]> content =
resultRows.stream().map(style::rowFieldsToString).collect(Collectors.toList());
// infer column width from the actual content
style.inferColumnWidth(content);
// print filed names
style.printBorderLine(terminal.writer());
style.printColumnNamesTableauRow(terminal.writer());
style.printBorderLine(terminal.writer());
terminal.flush();
for (String[] resultRow : content) {
if (Thread.currentThread().isInterrupted()) {
return;
}
style.printTableauRow(resultRow, terminal.writer());
}
String rowTerm = getRowTerm(receivedRowCount);
terminal.writer()
.println(
"Received a total of "
+ receivedRowCount
+ " "
+ rowTerm
+ calculateTimeCostInPrintFormat(
queryBeginTime,
System.currentTimeMillis()));
}
```
##########
flink-table/flink-sql-client/src/main/java/org/apache/flink/table/client/cli/CliTableauResultView.java:
##########
@@ -42,27 +42,45 @@
/** Print result in tableau mode. */
public class CliTableauResultView implements AutoCloseable {
+ public static final long DEFAULT_QUERY_BEGIN_TIME = -1;
+
private final Terminal terminal;
private final ResultDescriptor resultDescriptor;
private final ChangelogResult collectResult;
private final ExecutorService displayResultExecutorService;
+ private final long queryBeginTime;
+
public CliTableauResultView(final Terminal terminal, final
ResultDescriptor resultDescriptor) {
Review Comment:
Do we need this? Actually, no one uses it and it is a internal api.
##########
flink-table/flink-sql-client/src/main/java/org/apache/flink/table/client/config/SqlClientOptions.java:
##########
@@ -70,6 +70,14 @@ private SqlClientOptions() {}
+ "This only applies to columns with
variable-length types (e.g. CHAR, VARCHAR, STRING) in streaming mode. "
+ "Fixed-length types and all types in
batch mode are printed using a deterministic column width.");
+ @Documentation.TableOption(execMode = Documentation.ExecMode.BATCH)
+ public static final ConfigOption<Boolean> DISPLAY_QUERY_TIME_COST =
Review Comment:
It seems the option only works for the batch mode. I just wonder whether we
should print the time cost for streaming mode.
##########
flink-table/flink-sql-client/src/main/java/org/apache/flink/table/client/cli/CliTableauResultView.java:
##########
@@ -42,27 +42,45 @@
/** Print result in tableau mode. */
public class CliTableauResultView implements AutoCloseable {
+ public static final long DEFAULT_QUERY_BEGIN_TIME = -1;
Review Comment:
nit: -1L
--
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
