sv3ndk commented on a change in pull request #16245:
URL: https://github.com/apache/flink/pull/16245#discussion_r658738756
##########
File path:
flink-table/flink-sql-client/src/main/java/org/apache/flink/table/client/cli/CliChangelogResultView.java
##########
@@ -85,16 +94,6 @@ public CliChangelogResultView(CliClient client,
ResultDescriptor resultDescripto
return Arrays.copyOfRange(resultRow, 1, resultRow.length);
}
- @Override
- protected int computeColumnWidth(int idx) {
Review comment:
Hi @pensz .
The PR is 200 lines indeed. About half of it is documentation though, and
the previous behavior of column width was implemented differently for all 3
modes, so one factor that made the PR grow is my change to make them all depend
on the same logic.
The `computeColumnWidth(int idx)` you refer to would be called for every
row, which would be marginally slower I think. In this PR the width are now
initialized only once, in the constructor, which seems a natural place for
performing initialization.
IMHO, adding methods like `protected int computeColumnWidth(int idx)` adds
complexity, since it contributes to making `CliResultView` more different than
the tableau mode, and it adds a degree of freedom to the code which might not
add that much value, since using the same column width policy everywhere
increase the user experience I think.
--
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]
