lihaosky commented on code in PR #28790:
URL: https://github.com/apache/flink/pull/28790#discussion_r4517268635
##########
flink-table/flink-table-planner/src/test/java/org/apache/flink/table/planner/runtime/common/sql/ConnectionITCase.java:
##########
@@ -38,14 +47,34 @@
import static org.assertj.core.api.Assertions.assertThatThrownBy;
import static org.assertj.core.api.Assertions.entry;
-/** IT case for CREATE CONNECTION statement. */
-class CreateConnectionITCase extends BatchTestBase {
+/** IT cases for connection statements. */
+@ExtendWith(ParameterizedTestExtension.class)
+class ConnectionITCase {
Review Comment:
**[blocking]** This fails `ITCASE_USE_MINICLUSTER`. The rename dropped
`extends BatchTestBase`, which registered the MiniCluster. `SinkReuseITCase` in
this package extends `AbstractTestBase`:
```suggestion
class ConnectionITCase extends AbstractTestBase {
```
(plus `import org.apache.flink.test.util.AbstractTestBase;`). Please confirm
the ITCases pass in both modes afterwards.
##########
flink-table/flink-table-planner/src/test/java/org/apache/flink/table/planner/runtime/common/sql/ConnectionITCase.java:
##########
@@ -38,14 +47,34 @@
import static org.assertj.core.api.Assertions.assertThatThrownBy;
import static org.assertj.core.api.Assertions.entry;
-/** IT case for CREATE CONNECTION statement. */
-class CreateConnectionITCase extends BatchTestBase {
+/** IT cases for connection statements. */
+@ExtendWith(ParameterizedTestExtension.class)
+class ConnectionITCase {
- @Test
+ @Parameter public boolean isBatch;
+
+ @Parameters(name = "isBatch: {0}")
+ public static List<Boolean> parameters() {
+ return List.of(true, false);
+ }
+
+ private TableEnvironment tEnv;
+
+ @BeforeEach
+ void setup() {
+ tEnv =
+ TestingTableEnvironment.create(
+ isBatch
+ ? EnvironmentSettings.inBatchMode()
+ : EnvironmentSettings.inStreamingMode(),
+ null,
+ TableConfig.getDefault());
+ }
+
+ @TestTemplate
void testCreateTemporaryConnection() {
- tEnv().executeSql(
- "CREATE TEMPORARY CONNECTION my_conn COMMENT 'hi
there' "
- + "WITH ('k' = 'v')");
+ tEnv.executeSql(
+ "CREATE TEMPORARY CONNECTION my_conn COMMENT 'hi there' " +
"WITH ('k' = 'v')");
Review Comment:
**[nit]** Leftover concatenation from the reformat. It fits on one line:
```suggestion
tEnv.executeSql("CREATE TEMPORARY CONNECTION my_conn COMMENT 'hi
there' WITH ('k' = 'v')");
```
##########
flink-table/flink-table-planner/src/test/java/org/apache/flink/table/planner/runtime/common/sql/ConnectionITCase.java:
##########
@@ -161,12 +190,69 @@ void testShowCreateSecretOnlyTemporaryConnection() {
.containsOnly(entry("type",
"default")));
}
- @Test
- void testDescribeTemporaryConnection() {
- tEnv().executeSql(
- "CREATE TEMPORARY CONNECTION my_conn COMMENT 'hi
there' "
- + "WITH ('type' = 'default', 'k' = 'v',
'comment' = 'option comment', "
- + "'password' = 'super-secret')");
+ @TestTemplate
+ void testShowConnections() throws Exception {
+ ObjectIdentifier identifier = connectionIdentifier("permanent_conn");
+ catalogManager()
+ .getCatalog(identifier.getCatalogName())
+ .orElseThrow()
+ .createConnection(
+ identifier.toObjectPath(),
+ CatalogConnection.of(Map.of("k", "v"), null),
+ false);
+ tEnv.executeSql("CREATE TEMPORARY CONNECTION b_conn WITH ('k' = 'v')");
+ tEnv.executeSql("CREATE TEMPORARY CONNECTION a_conn WITH ('k' = 'v')");
+
+ assertThat(collectRows("SHOW CONNECTIONS"))
+ .containsExactly(Row.of("a_conn"), Row.of("b_conn"),
Row.of("permanent_conn"));
+ assertThat(collectRows("SHOW CONNECTIONS FROM " +
identifier.getDatabaseName()))
+ .containsExactly(Row.of("a_conn"), Row.of("b_conn"),
Row.of("permanent_conn"));
+ }
+
+ @TestTemplate
+ void testShowConnectionsInDatabase() throws Exception {
Review Comment:
**[suggestion]** Could we cover the error paths too?
- `SHOW CONNECTIONS FROM missing_db`
- `SHOW CONNECTIONS IN missing_cat.db`: from reading the code, this surfaces
a raw `CatalogNotExistException`, not the `ValidationException` that `SHOW
MODELS` gives. I haven't run it. If that's right, worth aligning.
- `SHOW CONNECTIONS FROM a.b.c` (the converter's format error)
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
