QuakeWang commented on code in PR #444:
URL: https://github.com/apache/paimon-rust/pull/444#discussion_r3518084706
##########
crates/integrations/datafusion/src/table/mod.rs:
##########
@@ -89,6 +97,119 @@ impl PaimonTableProvider {
}
}
+/// Build a `CREATE TABLE` DDL string for a Paimon table.
+///
+/// Mirrors the syntax accepted by `SQLContext::handle_create_table`:
+/// `CREATE TABLE <db>.<table> (<col> <type>, ..., PRIMARY KEY (...))
[PARTITIONED BY (...)] [WITH ('k'='v', ...)]`.
+fn build_table_definition(table: &Table) -> String {
+ let identifier = table.identifier();
+ let schema = table.schema();
+ let mut ddl = String::new();
+ let _ = write!(
+ ddl,
+ "CREATE TABLE {}.{} (",
+ identifier.database(),
+ identifier.object()
+ );
+
+ for (i, field) in schema.fields().iter().enumerate() {
+ if i > 0 {
+ ddl.push_str(", ");
+ }
+ let _ = write!(
Review Comment:
This drops column nullability. For example, a table created with `payload
BLOB NOT NULL` is rendered as `payload BLOB`, and replaying the DDL changes the
schema. `data_type_to_sql` should preserve `NOT NULL` where the Paimon type is
non-nullable.
##########
crates/integrations/datafusion/src/table/mod.rs:
##########
@@ -89,6 +97,119 @@ impl PaimonTableProvider {
}
}
+/// Build a `CREATE TABLE` DDL string for a Paimon table.
+///
+/// Mirrors the syntax accepted by `SQLContext::handle_create_table`:
+/// `CREATE TABLE <db>.<table> (<col> <type>, ..., PRIMARY KEY (...))
[PARTITIONED BY (...)] [WITH ('k'='v', ...)]`.
+fn build_table_definition(table: &Table) -> String {
+ let identifier = table.identifier();
+ let schema = table.schema();
+ let mut ddl = String::new();
+ let _ = write!(
+ ddl,
+ "CREATE TABLE {}.{} (",
+ identifier.database(),
+ identifier.object()
+ );
+
+ for (i, field) in schema.fields().iter().enumerate() {
+ if i > 0 {
+ ddl.push_str(", ");
+ }
+ let _ = write!(
+ ddl,
+ "{} {}",
+ field.name(),
+ data_type_to_sql(field.data_type())
+ );
+ }
+
+ let pks = schema.primary_keys();
+ if !pks.is_empty() {
+ ddl.push_str(", PRIMARY KEY (");
+ for (i, pk) in pks.iter().enumerate() {
+ if i > 0 {
+ ddl.push_str(", ");
+ }
+ let _ = write!(ddl, "{}", pk);
+ }
+ ddl.push(')');
+ }
+ ddl.push(')');
+
+ let partition_keys = schema.partition_keys();
+ if !partition_keys.is_empty() {
+ ddl.push_str(" PARTITIONED BY (");
+ for (i, pk) in partition_keys.iter().enumerate() {
+ if i > 0 {
+ ddl.push_str(", ");
+ }
+ let _ = write!(ddl, "{}", pk);
+ }
+ ddl.push(')');
+ }
+
+ let options = schema.options();
+ if !options.is_empty() {
+ ddl.push_str(" WITH (");
+ for (i, (k, v)) in options.iter().enumerate() {
+ if i > 0 {
+ ddl.push_str(", ");
+ }
+ let _ = write!(ddl, "'{}' = '{}'", k, v);
+ }
+ ddl.push(')');
+ }
+
+ ddl
+}
+
+/// Render a Paimon [`DataType`] as a SQL type string matching the syntax
+/// accepted by paimon-rust's `CREATE TABLE` parser.
+fn data_type_to_sql(data_type: &DataType) -> String {
+ match data_type {
+ DataType::Boolean(_) => "BOOLEAN".to_string(),
+ DataType::TinyInt(_) => "TINYINT".to_string(),
+ DataType::SmallInt(_) => "SMALLINT".to_string(),
+ DataType::Int(_) => "INT".to_string(),
+ DataType::BigInt(_) => "BIGINT".to_string(),
+ DataType::Decimal(t) => format!("DECIMAL({}, {})", t.precision(),
t.scale()),
+ DataType::Double(_) => "DOUBLE".to_string(),
+ DataType::Float(_) => "FLOAT".to_string(),
+ DataType::Binary(t) => format!("BINARY({})", t.length()),
+ DataType::VarBinary(t) => format!("VARBINARY({})", t.length()),
+ DataType::Blob(_) => "BLOB".to_string(),
+ DataType::Char(t) => format!("CHAR({})", t.length()),
+ DataType::VarChar(t) => format!("VARCHAR({})", t.length()),
+ DataType::Date(_) => "DATE".to_string(),
+ DataType::Time(t) => format!("TIME({})", t.precision()),
+ DataType::Timestamp(t) => format!("TIMESTAMP({})", t.precision()),
+ DataType::LocalZonedTimestamp(t) => {
+ format!("TIMESTAMP_LTZ({})", t.precision())
+ }
+ DataType::Array(t) => format!("ARRAY<{}>",
data_type_to_sql(t.element_type())),
+ DataType::Map(t) => format!(
+ "MAP<{}, {}>",
+ data_type_to_sql(t.key_type()),
+ data_type_to_sql(t.value_type())
+ ),
+ DataType::Multiset(t) => format!("MULTISET<{}>",
data_type_to_sql(t.element_type())),
+ DataType::Row(t) => {
Review Comment:
ROW is not the syntax accepted by the current CREATE TABLE path.
`SQLContext` maps `SqlType::Struct` to Paimon Row, so this should emit
`STRUCT<field type, ...>` instead of `ROW<field: type, ...>`, otherwise
row/struct tables cannot round-trip.
##########
crates/integrations/datafusion/tests/sql_context_tests.rs:
##########
@@ -1262,3 +1262,163 @@ async fn
test_multiple_temporary_views_in_same_database() {
.sum::<usize>();
assert_eq!(rows2, 3);
}
+
+// ======================= SHOW CREATE TABLE =======================
+
+/// Collect the `definition` column from `SHOW CREATE TABLE` output as a
String.
+async fn collect_definition(sql_context: &SQLContext, table_ref: &str) ->
String {
+ let rows = sql_context
+ .sql(&format!("SHOW CREATE TABLE {}", table_ref))
+ .await
+ .expect("SHOW CREATE TABLE should plan")
+ .collect()
+ .await
+ .expect("SHOW CREATE TABLE should execute");
+ assert_eq!(
+ rows.len(),
+ 1,
+ "SHOW CREATE TABLE should return exactly one row"
+ );
+ let row = &rows[0];
+ assert_eq!(
+ row.num_rows(),
+ 1,
+ "SHOW CREATE TABLE should return exactly one row"
+ );
+ let val = row.column(3); // definition is the 4th column
+ let def = val
+ .as_any()
+ .downcast_ref::<datafusion::arrow::array::StringArray>()
+ .expect("definition column should be a StringArray")
+ .value(0);
+ def.to_string()
+}
+
+#[tokio::test]
+async fn test_show_create_table_simple() {
+ let (_tmp, catalog) = create_test_env();
+ let sql_context = create_sql_context(catalog).await;
+
+ sql_context
+ .sql("CREATE SCHEMA paimon.test_db")
+ .await
+ .expect("CREATE SCHEMA should succeed");
+ sql_context
+ .sql("CREATE TABLE paimon.test_db.t (id INT, name VARCHAR(100))")
+ .await
+ .expect("CREATE TABLE should succeed");
+
+ let definition = collect_definition(&sql_context,
"paimon.test_db.t").await;
+ assert!(
+ definition.contains("CREATE TABLE test_db.t"),
+ "definition should start with CREATE TABLE test_db.t, got:
{definition}"
+ );
+ assert!(
+ definition.contains("id INT"),
+ "definition should contain `id INT`, got: {definition}"
+ );
+ assert!(
+ definition.contains("name VARCHAR("),
+ "definition should contain `name VARCHAR(...)`, got: {definition}"
+ );
+}
+
+#[tokio::test]
+async fn test_show_create_table_with_primary_key() {
+ let (_tmp, catalog) = create_test_env();
+ let sql_context = create_sql_context(catalog).await;
+
+ sql_context
+ .sql("CREATE SCHEMA paimon.test_db")
+ .await
+ .expect("CREATE SCHEMA should succeed");
+ sql_context
+ .sql("CREATE TABLE paimon.test_db.t (id INT NOT NULL, name VARCHAR,
PRIMARY KEY (id))")
+ .await
+ .expect("CREATE TABLE should succeed");
+
+ let definition = collect_definition(&sql_context,
"paimon.test_db.t").await;
+ assert!(
+ definition.contains("PRIMARY KEY (id)"),
+ "definition should contain PRIMARY KEY (id), got: {definition}"
+ );
+}
+
+#[tokio::test]
+async fn test_show_create_table_with_partition_and_options() {
+ let (_tmp, catalog) = create_test_env();
+ let sql_context = create_sql_context(catalog).await;
+
+ sql_context
+ .sql("CREATE SCHEMA paimon.test_db")
+ .await
+ .expect("CREATE SCHEMA should succeed");
+ sql_context
+ .sql(
+ "CREATE TABLE paimon.test_db.t (id INT, name VARCHAR, pt INT) \
+ PARTITIONED BY (pt) WITH ('bucket' = '4', 'file.format' =
'parquet')",
+ )
+ .await
+ .expect("CREATE TABLE should succeed");
+
+ let definition = collect_definition(&sql_context,
"paimon.test_db.t").await;
+ assert!(
+ definition.contains("PARTITIONED BY (pt)"),
+ "definition should contain PARTITIONED BY (pt), got: {definition}"
+ );
+ assert!(
+ definition.contains("'bucket' = '4'"),
+ "definition should contain bucket option, got: {definition}"
+ );
+ assert!(
+ definition.contains("'file.format' = 'parquet'"),
+ "definition should contain file.format option, got: {definition}"
+ );
+}
+
+#[tokio::test]
+async fn test_show_create_table_various_types() {
+ let (_tmp, catalog) = create_test_env();
+ let sql_context = create_sql_context(catalog).await;
+
+ sql_context
+ .sql("CREATE SCHEMA paimon.test_db")
+ .await
+ .expect("CREATE SCHEMA should succeed");
+ sql_context
+ .sql(
+ "CREATE TABLE paimon.test_db.t (\
+ a BOOLEAN, \
+ b TINYINT, \
+ c SMALLINT, \
+ d BIGINT, \
+ e DECIMAL(10, 2), \
+ f DOUBLE, \
+ g FLOAT, \
+ h DATE, \
+ i TIMESTAMP(3), \
+ j BLOB) \
+ WITH ('data-evolution.enabled' = 'true')",
+ )
+ .await
+ .expect("CREATE TABLE should succeed");
+
+ let definition = collect_definition(&sql_context,
"paimon.test_db.t").await;
Review Comment:
These tests only check substrings. Please add a round-trip assertion that
executes the returned definition, especially for NOT NULL, MAP, and STRUCT
columns.
##########
crates/integrations/datafusion/src/table/mod.rs:
##########
@@ -89,6 +97,119 @@ impl PaimonTableProvider {
}
}
+/// Build a `CREATE TABLE` DDL string for a Paimon table.
+///
+/// Mirrors the syntax accepted by `SQLContext::handle_create_table`:
+/// `CREATE TABLE <db>.<table> (<col> <type>, ..., PRIMARY KEY (...))
[PARTITIONED BY (...)] [WITH ('k'='v', ...)]`.
+fn build_table_definition(table: &Table) -> String {
+ let identifier = table.identifier();
+ let schema = table.schema();
+ let mut ddl = String::new();
+ let _ = write!(
+ ddl,
+ "CREATE TABLE {}.{} (",
+ identifier.database(),
+ identifier.object()
+ );
+
+ for (i, field) in schema.fields().iter().enumerate() {
+ if i > 0 {
+ ddl.push_str(", ");
+ }
+ let _ = write!(
+ ddl,
+ "{} {}",
+ field.name(),
+ data_type_to_sql(field.data_type())
+ );
+ }
+
+ let pks = schema.primary_keys();
+ if !pks.is_empty() {
+ ddl.push_str(", PRIMARY KEY (");
+ for (i, pk) in pks.iter().enumerate() {
+ if i > 0 {
+ ddl.push_str(", ");
+ }
+ let _ = write!(ddl, "{}", pk);
+ }
+ ddl.push(')');
+ }
+ ddl.push(')');
+
+ let partition_keys = schema.partition_keys();
+ if !partition_keys.is_empty() {
+ ddl.push_str(" PARTITIONED BY (");
+ for (i, pk) in partition_keys.iter().enumerate() {
+ if i > 0 {
+ ddl.push_str(", ");
+ }
+ let _ = write!(ddl, "{}", pk);
+ }
+ ddl.push(')');
+ }
+
+ let options = schema.options();
+ if !options.is_empty() {
+ ddl.push_str(" WITH (");
+ for (i, (k, v)) in options.iter().enumerate() {
+ if i > 0 {
+ ddl.push_str(", ");
+ }
+ let _ = write!(ddl, "'{}' = '{}'", k, v);
+ }
+ ddl.push(')');
+ }
+
+ ddl
+}
+
+/// Render a Paimon [`DataType`] as a SQL type string matching the syntax
+/// accepted by paimon-rust's `CREATE TABLE` parser.
+fn data_type_to_sql(data_type: &DataType) -> String {
+ match data_type {
+ DataType::Boolean(_) => "BOOLEAN".to_string(),
+ DataType::TinyInt(_) => "TINYINT".to_string(),
+ DataType::SmallInt(_) => "SMALLINT".to_string(),
+ DataType::Int(_) => "INT".to_string(),
+ DataType::BigInt(_) => "BIGINT".to_string(),
+ DataType::Decimal(t) => format!("DECIMAL({}, {})", t.precision(),
t.scale()),
+ DataType::Double(_) => "DOUBLE".to_string(),
+ DataType::Float(_) => "FLOAT".to_string(),
+ DataType::Binary(t) => format!("BINARY({})", t.length()),
+ DataType::VarBinary(t) => format!("VARBINARY({})", t.length()),
+ DataType::Blob(_) => "BLOB".to_string(),
+ DataType::Char(t) => format!("CHAR({})", t.length()),
+ DataType::VarChar(t) => format!("VARCHAR({})", t.length()),
+ DataType::Date(_) => "DATE".to_string(),
+ DataType::Time(t) => format!("TIME({})", t.precision()),
+ DataType::Timestamp(t) => format!("TIMESTAMP({})", t.precision()),
+ DataType::LocalZonedTimestamp(t) => {
+ format!("TIMESTAMP_LTZ({})", t.precision())
+ }
+ DataType::Array(t) => format!("ARRAY<{}>",
data_type_to_sql(t.element_type())),
+ DataType::Map(t) => format!(
Review Comment:
The generated MAP syntax does not match what `SQLContext` currently parses.
`sqlparser` accepts `Map(key_type, value_type)` here, while this emits
`MAP<key_type, value_type>`, so `SHOW CREATE TABLE` returns non-replayable DDL
for map columns.
--
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]
