shyjsarah commented on code in PR #444:
URL: https://github.com/apache/paimon-rust/pull/444#discussion_r3526250398


##########
crates/integrations/datafusion/src/table/mod.rs:
##########
@@ -89,6 +97,131 @@ 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 {}.{} (",

Review Comment:
   Fixed in 78c15c0. `SHOW CREATE TABLE` now quotes database/table names, 
column names, primary key columns, partition columns, and nested STRUCT field 
names, escaping embedded double quotes through the standard doubled-quote form.
   
   I also added 
`test_show_create_table_round_trip_with_quoted_identifiers_and_options`, which 
covers reserved-word identifiers such as `"select"`, `"order"`, `"group"`, and 
nested `"from"`, then re-executes the generated DDL and checks schema 
equivalence.



##########
crates/integrations/datafusion/src/table/mod.rs:
##########
@@ -89,6 +97,131 @@ 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(", ");
+        }
+        // `NOT NULL` is a column constraint; render it here at the column
+        // level rather than inside nested type arguments (see 
`data_type_to_sql`).
+        let ty = data_type_to_sql(field.data_type());
+        if field.data_type().is_nullable() {
+            let _ = write!(ddl, "{} {}", field.name(), ty);
+        } else {
+            let _ = write!(ddl, "{} {} NOT NULL", field.name(), ty);
+        }
+    }
+
+    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.
+///
+/// `NOT NULL` is a column constraint, not a type modifier — it is only valid
+/// at the top of a column definition, not nested inside `MAP`, `ARRAY`, or
+/// `STRUCT` arguments. Callers that render a column should append `NOT NULL`
+/// themselves when the field is non-nullable; recursive calls below must not.
+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()),

Review Comment:
   Fixed in 78c15c0. `TIMESTAMP_LTZ` is now rendered as `TIMESTAMP(p) WITH TIME 
ZONE`, which the current converter already maps back to `LocalZonedTimestamp`.
   
   For `TIME`, `MULTISET`, and `VECTOR`, `SHOW CREATE TABLE` now returns an 
explicit unsupported error instead of emitting non-replayable DDL. I added 
`test_show_create_table_rejects_non_round_trippable_types` to cover those cases.



-- 
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]

Reply via email to