wuchong commented on a change in pull request #15332:
URL: https://github.com/apache/flink/pull/15332#discussion_r600628674
##########
File path:
flink-table/flink-sql-client/src/main/java/org/apache/flink/table/client/cli/CliClient.java
##########
@@ -682,6 +697,22 @@ private void printInfo(String message) {
terminal.flush();
}
+ private void printWarning(String message) {
+ terminal.writer().println(CliStrings.messageWarning(message).toAnsi());
+ terminal.flush();
+ }
+
+ private void printRemovedAndDeprecatedKeyMessage(String key) {
Review comment:
`printRemovedOrDeprecatedKeyMessage`
##########
File path:
flink-table/flink-sql-client/src/test/java/org/apache/flink/table/client/gateway/context/SessionContextTest.java
##########
@@ -79,78 +66,120 @@ public void testSetAndResetOption() throws Exception {
sessionContext.set(NAME.key(), "test");
// runtime config from flink-conf
sessionContext.set(OBJECT_REUSE.key(), "false");
+ Assert.assertEquals("hive",
getConfiguration(sessionContext).getString(TABLE_SQL_DIALECT));
+ Assert.assertEquals(128,
getConfiguration(sessionContext).getInteger(MAX_PARALLELISM));
+ Assert.assertEquals("test",
getConfiguration(sessionContext).getString(NAME));
+
Assert.assertFalse(getConfiguration(sessionContext).getBoolean(OBJECT_REUSE));
+
+ sessionContext.reset();
Assert.assertEquals(
- "hive",
- sessionContext
- .getExecutionContext()
- .getTableEnvironment()
- .getConfig()
- .getConfiguration()
- .getString(TABLE_SQL_DIALECT));
+ "default",
getConfiguration(sessionContext).getString(TABLE_SQL_DIALECT));
+ Assert.assertNull(getConfiguration(sessionContext).get(NAME));
+ // The value of MAX_PARALLELISM in DEFAULTS_ENVIRONMENT_FILE is 16
+ Assert.assertEquals(16,
getConfiguration(sessionContext).getInteger(MAX_PARALLELISM));
+ Assert.assertNull(getConfiguration(sessionContext).getString(NAME,
null));
+ // The value of OBJECT_REUSE in origin configuration is true
+
Assert.assertTrue(getConfiguration(sessionContext).getBoolean(OBJECT_REUSE));
+ }
+
+ @Test
+ public void testSetAndResetKeyInYamlKey() throws Exception {
+ SessionContext sessionContext = createSessionContext();
+ sessionContext.set("execution.max-table-result-rows", "200000");
+ sessionContext.set("execution.result-mode", "table");
+
Assert.assertEquals(
- 128,
- sessionContext
- .getExecutionContext()
- .getTableEnvironment()
- .getConfig()
- .getConfiguration()
- .getInteger(MAX_PARALLELISM));
+ "200000",
getMap(sessionContext).get("execution.max-table-result-rows"));
+
+ Assert.assertEquals("table",
getMap(sessionContext).get("execution.result-mode"));
+
+ sessionContext.reset("execution.result-mode");
+ Assert.assertEquals("changelog",
getMap(sessionContext).get("execution.result-mode"));
+ // no reset this key execution.max-table-result-rows
Assert.assertEquals(
- "test",
- sessionContext
- .getExecutionContext()
- .getTableEnvironment()
- .getConfig()
- .getConfiguration()
- .getString(NAME));
- Assert.assertFalse(
- sessionContext
- .getExecutionContext()
- .getTableEnvironment()
- .getConfig()
- .getConfiguration()
- .getBoolean(OBJECT_REUSE));
+ "200000",
getMap(sessionContext).get("execution.max-table-result-rows"));
- sessionContext.reset();
+ // reset option for deprecated key
+ sessionContext.reset("sql-client.execution.max-table-result.rows");
Assert.assertEquals(
- "default",
- sessionContext
- .getExecutionContext()
- .getTableEnvironment()
- .getConfig()
- .getConfiguration()
- .getString(TABLE_SQL_DIALECT));
+ "100",
getMap(sessionContext).get("sql-client.execution.max-table-result.rows"));
+ Assert.assertEquals("100",
getMap(sessionContext).get("execution.max-table-result-rows"));
+ }
+
+ @Test
+ public void testSetAndResetKeyInConfigOptions() throws Exception {
+ SessionContext sessionContext = createSessionContext();
+ // table config option
+ sessionContext.set(TABLE_SQL_DIALECT.key(), "hive");
+ // runtime config option and Yaml specified value
+ sessionContext.set(MAX_PARALLELISM.key(), "128");
+ // runtime config option and doesn't have default value
+ sessionContext.set(NAME.key(), "test");
+ // runtime config from flink-conf
+ sessionContext.set(OBJECT_REUSE.key(), "false");
+
+ Assert.assertEquals("hive",
getConfiguration(sessionContext).getString(TABLE_SQL_DIALECT));
+ Assert.assertEquals(128,
getConfiguration(sessionContext).getInteger(MAX_PARALLELISM));
+ Assert.assertEquals("test",
getConfiguration(sessionContext).getString(NAME));
+
Assert.assertFalse(getConfiguration(sessionContext).getBoolean(OBJECT_REUSE));
+
+ sessionContext.reset(TABLE_SQL_DIALECT.key());
+ Assert.assertEquals(
+ "default",
getConfiguration(sessionContext).getString(TABLE_SQL_DIALECT));
+
+ sessionContext.reset(MAX_PARALLELISM.key());
+ Assert.assertEquals(16,
getConfiguration(sessionContext).getInteger(MAX_PARALLELISM));
+
+ sessionContext.reset(NAME.key());
+ Assert.assertNull(getConfiguration(sessionContext).get(NAME));
+
+ sessionContext.reset(OBJECT_REUSE.key());
+
Assert.assertTrue(getConfiguration(sessionContext).getBoolean(OBJECT_REUSE));
+ }
+
+ @Test
+ public void testSetWithConfigOptionAndResetWithYamlKey() throws Exception {
+ SessionContext sessionContext = createSessionContext();
+ // runtime config option and has deprecated key
+ sessionContext.set(TABLE_PLANNER.key(), "blink");
+ Assert.assertEquals(
+ "blink",
getConfiguration(sessionContext).get(TABLE_PLANNER).name().toLowerCase());
+
+ sessionContext.reset(TABLE_PLANNER.key());
+ Assert.assertEquals(
+ "old",
getConfiguration(sessionContext).get(TABLE_PLANNER).name().toLowerCase());
+ Assert.assertEquals("old",
getMap(sessionContext).get("execution.planner").toLowerCase());
+ }
+
+ @Test
+ public void testSetAndResetKeyNotInYaml() throws Exception {
+ SessionContext sessionContext = createSessionContext();
+ // other property not in yaml and flink-conf
+ sessionContext.set("aa", "11");
+ sessionContext.set("bb", "22");
+
+ Assert.assertEquals(
+ "11",
+ getConfiguration(sessionContext)
+
.getString(ConfigOptions.key("aa").stringType().noDefaultValue()));
Review comment:
Can simply use `getMap` in this test?
##########
File path:
flink-table/flink-sql-client/src/test/java/org/apache/flink/table/client/gateway/context/SessionContextTest.java
##########
@@ -79,78 +66,120 @@ public void testSetAndResetOption() throws Exception {
sessionContext.set(NAME.key(), "test");
// runtime config from flink-conf
sessionContext.set(OBJECT_REUSE.key(), "false");
+ Assert.assertEquals("hive",
getConfiguration(sessionContext).getString(TABLE_SQL_DIALECT));
+ Assert.assertEquals(128,
getConfiguration(sessionContext).getInteger(MAX_PARALLELISM));
+ Assert.assertEquals("test",
getConfiguration(sessionContext).getString(NAME));
+
Assert.assertFalse(getConfiguration(sessionContext).getBoolean(OBJECT_REUSE));
+
+ sessionContext.reset();
Assert.assertEquals(
- "hive",
- sessionContext
- .getExecutionContext()
- .getTableEnvironment()
- .getConfig()
- .getConfiguration()
- .getString(TABLE_SQL_DIALECT));
+ "default",
getConfiguration(sessionContext).getString(TABLE_SQL_DIALECT));
+ Assert.assertNull(getConfiguration(sessionContext).get(NAME));
+ // The value of MAX_PARALLELISM in DEFAULTS_ENVIRONMENT_FILE is 16
+ Assert.assertEquals(16,
getConfiguration(sessionContext).getInteger(MAX_PARALLELISM));
+ Assert.assertNull(getConfiguration(sessionContext).getString(NAME,
null));
+ // The value of OBJECT_REUSE in origin configuration is true
+
Assert.assertTrue(getConfiguration(sessionContext).getBoolean(OBJECT_REUSE));
+ }
+
+ @Test
+ public void testSetAndResetKeyInYamlKey() throws Exception {
+ SessionContext sessionContext = createSessionContext();
+ sessionContext.set("execution.max-table-result-rows", "200000");
+ sessionContext.set("execution.result-mode", "table");
+
Assert.assertEquals(
- 128,
- sessionContext
- .getExecutionContext()
- .getTableEnvironment()
- .getConfig()
- .getConfiguration()
- .getInteger(MAX_PARALLELISM));
+ "200000",
getMap(sessionContext).get("execution.max-table-result-rows"));
+
+ Assert.assertEquals("table",
getMap(sessionContext).get("execution.result-mode"));
+
+ sessionContext.reset("execution.result-mode");
+ Assert.assertEquals("changelog",
getMap(sessionContext).get("execution.result-mode"));
+ // no reset this key execution.max-table-result-rows
Assert.assertEquals(
- "test",
- sessionContext
- .getExecutionContext()
- .getTableEnvironment()
- .getConfig()
- .getConfiguration()
- .getString(NAME));
- Assert.assertFalse(
- sessionContext
- .getExecutionContext()
- .getTableEnvironment()
- .getConfig()
- .getConfiguration()
- .getBoolean(OBJECT_REUSE));
+ "200000",
getMap(sessionContext).get("execution.max-table-result-rows"));
- sessionContext.reset();
+ // reset option for deprecated key
+ sessionContext.reset("sql-client.execution.max-table-result.rows");
Assert.assertEquals(
- "default",
- sessionContext
- .getExecutionContext()
- .getTableEnvironment()
- .getConfig()
- .getConfiguration()
- .getString(TABLE_SQL_DIALECT));
+ "100",
getMap(sessionContext).get("sql-client.execution.max-table-result.rows"));
+ Assert.assertEquals("100",
getMap(sessionContext).get("execution.max-table-result-rows"));
+ }
+
+ @Test
+ public void testSetAndResetKeyInConfigOptions() throws Exception {
+ SessionContext sessionContext = createSessionContext();
+ // table config option
+ sessionContext.set(TABLE_SQL_DIALECT.key(), "hive");
+ // runtime config option and Yaml specified value
+ sessionContext.set(MAX_PARALLELISM.key(), "128");
+ // runtime config option and doesn't have default value
+ sessionContext.set(NAME.key(), "test");
+ // runtime config from flink-conf
+ sessionContext.set(OBJECT_REUSE.key(), "false");
+
+ Assert.assertEquals("hive",
getConfiguration(sessionContext).getString(TABLE_SQL_DIALECT));
+ Assert.assertEquals(128,
getConfiguration(sessionContext).getInteger(MAX_PARALLELISM));
+ Assert.assertEquals("test",
getConfiguration(sessionContext).getString(NAME));
+
Assert.assertFalse(getConfiguration(sessionContext).getBoolean(OBJECT_REUSE));
+
+ sessionContext.reset(TABLE_SQL_DIALECT.key());
+ Assert.assertEquals(
+ "default",
getConfiguration(sessionContext).getString(TABLE_SQL_DIALECT));
+
+ sessionContext.reset(MAX_PARALLELISM.key());
+ Assert.assertEquals(16,
getConfiguration(sessionContext).getInteger(MAX_PARALLELISM));
Review comment:
nit: would be better to import the static method and use `assertEquals`
directly in the tests.
##########
File path:
flink-table/flink-sql-client/src/test/java/org/apache/flink/table/client/gateway/context/SessionContextTest.java
##########
@@ -177,4 +206,16 @@ private SessionContext createSessionContext() throws
Exception {
Collections.singletonList(new DefaultCLI()));
return SessionContext.create(defaultContext, "test-session");
}
+
+ private Map<String, String> getMap(SessionContext context) {
Review comment:
==> `getConfigurationMap` would be more specific.
--
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]
