twalthr commented on a change in pull request #15018:
URL: https://github.com/apache/flink/pull/15018#discussion_r583537116



##########
File path: 
flink-table/flink-table-api-java/src/main/java/org/apache/flink/table/api/TableConfig.java
##########
@@ -75,7 +78,17 @@
     private MathContext decimalContext = MathContext.DECIMAL128;
 
     /** A configuration object to hold all key/value configuration. */
-    private Configuration configuration = new Configuration();
+    private Configuration configuration;
+
+    /** Create TableConfig with default configuration. */
+    public TableConfig() {
+        this.configuration = new TableConfiguration();

Review comment:
       Having `TableConfig` and `TableConfiguration` can be confusing for users.

##########
File path: 
flink-core/src/main/java/org/apache/flink/configuration/Configuration.java
##########
@@ -604,6 +604,9 @@ public void addAllToProperties(Properties props) {
     public void addAll(Configuration other) {
         synchronized (this.confData) {
             synchronized (other.confData) {
+                for (String key : other.keySet()) {

Review comment:
       This class is used by a lot of Flink components has been proven to be 
relatively mature. I would like to avoid changes here.

##########
File path: 
flink-table/flink-table-api-java/src/main/java/org/apache/flink/table/api/TableConfig.java
##########
@@ -343,4 +356,10 @@ public void addJobParameter(String key, String value) {
     public static TableConfig getDefault() {
         return new TableConfig();
     }
+
+    public static TableConfig getBatchTableConfig() {

Review comment:
       We are introducing a lot of public API methods for which we need to 
provide backwards compatibility across releases. This can simply be done in a 
private method in `BatchTableEnvironment`.

##########
File path: 
flink-table/flink-table-api-java/src/main/java/org/apache/flink/table/api/EnvironmentSettings.java
##########
@@ -121,6 +177,13 @@ public boolean isStreamingMode() {
         return isStreamingMode;
     }
 
+    /** Tells if the {@link TableEnvironment} should work in the blink planner 
or old planner. */
+    public boolean isBlinkPlanner() {

Review comment:
       package private?




----------------------------------------------------------------
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]


Reply via email to