twalthr commented on code in PR #25704:
URL: https://github.com/apache/flink/pull/25704#discussion_r1860917076
##########
flink-table/flink-table-api-java/src/main/java/org/apache/flink/table/api/TableEnvironment.java:
##########
@@ -745,6 +745,35 @@ void createTemporarySystemFunction(
*/
void createTable(String path, TableDescriptor descriptor);
+ /**
+ * Registers the given {@link TableDescriptor} as a catalog table.
+ *
+ * <p>The {@link TableDescriptor descriptor} is converted into a {@link
CatalogTable} and stored
+ * in the catalog.
+ *
+ * <p>If the table should not be permanently stored in a catalog, use
{@link
+ * #createTemporaryTable(String, TableDescriptor)} instead.
+ *
+ * <p>Examples:
+ *
+ * <pre>{@code
+ * tEnv.createTable("MyTable", TableDescriptor.forConnector("datagen")
+ * .schema(Schema.newBuilder()
+ * .column("f0", DataTypes.STRING())
+ * .build())
+ * .option(DataGenOptions.ROWS_PER_SECOND, 10)
+ * .option("fields.f0.kind", "random")
+ * .build());
+ * }</pre>
+ *
+ * @param path The path under which the table will be registered. See also
the {@link
+ * TableEnvironment} class description for the format of the path.
+ * @param descriptor Template for creating a {@link CatalogTable} instance.
+ * @param ignoreIfExists If a table exists under the given path and this
flag is set, no
+ * operation is executed. An exception is thrown otherwise.
+ */
+ void createTable(String path, TableDescriptor descriptor, boolean
ignoreIfExists);
Review Comment:
can you check whether Python already supports `createTable` with
TableDescriptors. if yes, we should also add the method there. check
`flink/flink-python/pyflink/table/table_environment.py`
##########
flink-table/flink-table-api-java/src/test/java/org/apache/flink/table/api/TableEnvironmentTest.java:
##########
@@ -100,6 +101,55 @@ void testCreateTableFromDescriptor() throws Exception {
.contains(entry("connector", "fake"), entry("a", "Test"));
}
+ @Test
+ void testCreateTableIfNotExistsFromDescriptor() throws Exception {
+ final TableEnvironmentMock tEnv =
TableEnvironmentMock.getStreamingInstance();
+ final String catalog = tEnv.getCurrentCatalog();
+ final String database = tEnv.getCurrentDatabase();
+
+ final Schema schema = Schema.newBuilder().column("f0",
DataTypes.INT()).build();
+ tEnv.createTable(
+ "T",
+
TableDescriptor.forConnector("fake").schema(schema).option("a", "Test").build(),
+ true);
+
+ final ObjectPath objectPath = new ObjectPath(database, "T");
+ assertThat(
+ tEnv.getCatalog(catalog)
+ .orElseThrow(AssertionError::new)
+ .tableExists(objectPath))
+ .isTrue();
+
+ final CatalogBaseTable catalogTable =
+
tEnv.getCatalog(catalog).orElseThrow(AssertionError::new).getTable(objectPath);
+ assertThat(catalogTable).isInstanceOf(CatalogTable.class);
+ assertThat(catalogTable.getUnresolvedSchema()).isEqualTo(schema);
+ assertThat(catalogTable.getOptions())
+ .contains(entry("connector", "fake"), entry("a", "Test"));
+
+ assertThatNoException()
+ .isThrownBy(
+ () ->
+ tEnv.createTable(
+ "T",
+ TableDescriptor.forConnector("fake")
+ .schema(schema)
+ .option("a", "Test")
+ .build(),
+ true));
+
+ assertThatThrownBy(
+ () ->
+ tEnv.createTable(
+ "T",
+ TableDescriptor.forConnector("fake")
+ .schema(schema)
+ .option("a", "Test")
+ .build(),
+ false))
+ .isInstanceOf(ValidationException.class);
Review Comment:
ideally, we should also check the error message. at least parts of it.
`hasMessageContaining` could do the job?
##########
flink-table/flink-table-api-java/src/main/java/org/apache/flink/table/api/TableEnvironment.java:
##########
@@ -745,6 +745,35 @@ void createTemporarySystemFunction(
*/
void createTable(String path, TableDescriptor descriptor);
+ /**
+ * Registers the given {@link TableDescriptor} as a catalog table.
+ *
+ * <p>The {@link TableDescriptor descriptor} is converted into a {@link
CatalogTable} and stored
+ * in the catalog.
+ *
+ * <p>If the table should not be permanently stored in a catalog, use
{@link
+ * #createTemporaryTable(String, TableDescriptor)} instead.
Review Comment:
we should also support the boolean flag for `createTemporaryTable` and link
it here
##########
flink-table/flink-table-api-java/src/test/java/org/apache/flink/table/api/TableEnvironmentTest.java:
##########
@@ -100,6 +101,55 @@ void testCreateTableFromDescriptor() throws Exception {
.contains(entry("connector", "fake"), entry("a", "Test"));
}
+ @Test
+ void testCreateTableIfNotExistsFromDescriptor() throws Exception {
+ final TableEnvironmentMock tEnv =
TableEnvironmentMock.getStreamingInstance();
+ final String catalog = tEnv.getCurrentCatalog();
+ final String database = tEnv.getCurrentDatabase();
+
+ final Schema schema = Schema.newBuilder().column("f0",
DataTypes.INT()).build();
+ tEnv.createTable(
+ "T",
+
TableDescriptor.forConnector("fake").schema(schema).option("a", "Test").build(),
+ true);
+
+ final ObjectPath objectPath = new ObjectPath(database, "T");
+ assertThat(
+ tEnv.getCatalog(catalog)
+ .orElseThrow(AssertionError::new)
+ .tableExists(objectPath))
+ .isTrue();
+
+ final CatalogBaseTable catalogTable =
+
tEnv.getCatalog(catalog).orElseThrow(AssertionError::new).getTable(objectPath);
+ assertThat(catalogTable).isInstanceOf(CatalogTable.class);
+ assertThat(catalogTable.getUnresolvedSchema()).isEqualTo(schema);
+ assertThat(catalogTable.getOptions())
+ .contains(entry("connector", "fake"), entry("a", "Test"));
Review Comment:
this code is duplicated with the test above. create a private helper method
e.g. `assertTableFromDescriptor(String name)`
--
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]
