Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

support create table like in flink catalog #12199

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

swapna267
Copy link

Creation of dynamic Iceberg table in Flink Catalog using the underlying physical Iceberg table using LIKE clause.

Currently (without the changes in PR), create table in flink catalog works by configuring flink connector as described in,
flink-connector

But that needs user to provide the schema for the table. A way to tackle that is to do create table LIKE using below DDL.

CREATE TABLE table_wm (
      eventTS AS CAST(t1 AS TIMESTAMP(3)),
      WATERMARK FOR eventTS AS SOURCE_WATERMARK()
) WITH (
  'connector'='iceberg',
  'catalog-name'='iceberg_catalog',
  'catalog-database'='testdb',
  'catalog-table'='t'
) LIKE iceberg_catalog.testdb.t;

Options like connector, catalog-name, catalog-database, catalog-table need to be duplicated as Iceberg FlinkCatalog doesn't return any catalog related properties during getTable. This PR addresses the issue by including these properties when getTable is called , which will be used by Flink when creating table in Flink Catalog.

Previous discussion related to feature is in PR, #12116 .

@github-actions github-actions bot added the flink label Feb 7, 2025
FlinkCreateTableOptions.CATALOG_TABLE.key(), tablePath.getObjectName());
catalogAndTableProps.put("connector", FlinkDynamicTableFactory.FACTORY_IDENTIFIER);
catalogAndTableProps.putAll(table.properties());
return toCatalogTableWithProps(table, catalogAndTableProps);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you help me understand why is the table properties needed to be added here?
We also send the table as a parameter. Wouldn't it be enough?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just thinking out loud:

  • Maybe the code would be easier to read if we send only the catalogProps to the toCatalogTableWithProps and create a merged map when calling the Flink method
  • This is somewhat suboptimal as we create an extra map

Even if we decide to follow your approach, the parameter name of the method should reflect that at the declaration of toCatalogTableWithProps, and maybe some comments or javadoc should be nice there for future generations 😉

WDYT?

@@ -188,6 +188,23 @@ public void testCreateTableLike() throws TableNotExistException {
.isEqualTo(TableSchema.builder().field("id", DataTypes.BIGINT()).build());
}

@TestTemplate
public void testCreateTableLikeInFlinkCatalog() throws TableNotExistException {
sql("CREATE TABLE tl(id BIGINT)");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: extra space in the sql

@@ -384,13 +395,6 @@ public void renameTable(ObjectPath tablePath, String newTableName, boolean ignor
@Override
public void createTable(ObjectPath tablePath, CatalogBaseTable table, boolean ignoreIfExists)
throws CatalogException, TableAlreadyExistException {
if (Objects.equals(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we have a way to check if we can get the catalog properties in another way if they are not directly set by the table options?

@@ -188,6 +188,23 @@ public void testCreateTableLike() throws TableNotExistException {
.isEqualTo(TableSchema.builder().field("id", DataTypes.BIGINT()).build());
}

@TestTemplate
public void testCreateTableLikeInFlinkCatalog() throws TableNotExistException {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a simple way to check whether the create table like command is working even when the source table is not using the default catalog.

I'm a bit concerned that this test would pass if we simply use the default catalog for the new table instead copying the values from the source table.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants