GitHub user OjashKush added a comment to the discussion: Native SQL DDL support for Hudi table creation across engines (Trino, Presto etc.)
Thanks. **Point 1: you were right, and my descriptor was diverging in ways that break interop.** I went looking for the differences and found them. I had been building to `ResourceHudiTablesInitializer` — a minimal test double — rather than to `HMSDDLExecutor`, which is the code that actually writes the descriptor. Consequences: - I omit the Spark datasource table properties entirely (`spark.sql.sources.provider`, `spark.sql.sources.schema.numParts`/`part.N`, `numPartCols`/`partCol.N`), and the serde properties alongside them (`path`, `hoodie.query.as.ro.table`). Since Spark SQL dispatches on `provider`, a table I create almost certainly wouldn't be recognised as a Hudi datasource table — which defeats the cross-engine goal the feature exists for. - I put `serialization.format=1` in table parameters; `HMSDDLExecutor:133` puts it in serde properties. So my descriptor isn't even a faithful subset. So this isn't a hypothetical maintenance worry — my implementation had already drifted, after I'd read `HiveSyncTool` and believed I'd matched it. Concrete evidence for your concern. **But the shared layer you're describing partly exists already, and is callable from here.** `SparkDataSourceTableUtils` in `hudi-sync-common` — already a compile dependency of `hudi-trino` — imports only `org.apache.hudi.common.*` and `java.util.*`. No Spark, no Hadoop, no Hive. It produces exactly what I was missing: - `getSparkTableProperties(partitionNames, sparkVersion, schemaLengthThreshold, HoodieSchema, includeFieldDocs)` → provider, create version, the Spark-reconstructible schema parts, partition columns - `getSparkSerdeProperties(readAsOptimized, basePath)` → `path`, `hoodie.query.as.ro.table` I've verified it runs from the connector — a test asserts `provider=hudi`, `partCol.0=city`, and that the reassembled schema parts contain `_hoodie_commit_time`, `id`, `city`. It's directly linked, not reflectively loaded, so there's no classloader question. Its Spark-compatibility is also already covered: `TestSparkSchemaUtils` round-trips the hand-rolled JSON through Spark's own `StructType.fromJson` and compares canonically against a Spark-built `StructType`, across primitives, timestamp micros/millis/local, date, time, enum, UUID, nested structs and arrays. So consuming it from Trino inherits that guarantee rather than adding a new surface to keep in sync. **And your "separate metadata-building paths" is literally true — there are two, one an explicit fork.** - `SparkDataSourceTableUtils.getSparkTableProperties` — `hudi-sync-common`, engine-neutral, used by hive-sync (and adb-sync, datahub-sync, and the Flink catalog) - `CreateHoodieTableCommand.tableMetaToTableProps` — `hudi-spark-common`, used by Spark SQL, with a comment at line 298 reading "This code is forked from `org.apache.spark.sql.hive.HiveExternalCatalog#tableMetaToTableProps`" Same key set; the forked one additionally handles bucket specs. And `HIVE_SYNC_AS_DATA_SOURCE_TABLE` defaults to `true` (`HiveSyncConfigHolder:98-101`), so hive-sync emits these by default — both paths already target the same contract by two independent implementations. I'm planning to emit them unconditionally rather than mirroring the `sync_as_datasource` knob, since Trino has no reason to offer the choice; flagging that as a deliberate difference in behaviour surface. So the duplicated remainder is narrower than it looked. The Spark-property construction — the largest and most error-prone part, and the part I got wrong by omission — doesn't need extracting; it's already shared and Spark-verified. What's left Trino-specific is the API translation you said each integration would do anyway: column types via `HiveTypeTranslator`, `StorageFormat`, the EXTERNAL flag, the partition split, into `io.trino.metastore.Table`. The genuinely duplicated remainder is the assembly decisions, where Spark SQL and hive-sync already diverge (Spark's own Hive client vs `HMSDDLExecutor`) — so if you want that extracted, it's a smaller change than it first looked, and I'm happy to shape my code so it's a move rather than a rewrite. **Point 2** New tables pinned to version 10; `register_table` reads and preserves the existing version, with a test registering a v8 fixture and asserting it stays v8. **Point 3: adopting the five cases.** This reverses what I had — I'd followed Iceberg's `dropTable`, which deletes data. Switching to: explicit location → external → ordinary `DROP TABLE` preserves filesystem data; omitted location → managed → `DROP TABLE` deletes; `register_table` registers external; PURGE deferred. On case 4 — managed tables with no location — it's implementable via `io.trino.metastore.Database.getLocation()`, deriving `<schemaLocation>/<tableName>` the way Delta Lake does, with no new config. One caveat: it requires the schema to have a location, and Hudi catalogs often don't (the connector's own test query runner creates a location-less database). So it'll work where the schema has a location and error clearly otherwise, matching Delta — but the error path will be the common one in tests. **Two questions** `unregister_table`'s original rationale is gone. I'd justified it as the non-destructive path, but under external semantics ordinary `DROP TABLE` already is. Two narrower justifications survive: detaching a managed table without deleting data, and removing a catalog entry the connector can't load at all — `getTableHandle` calls `HudiTableTypeUtils.fromInputFormat`, which throws on an unrecognised input format, so a mis-registered table may not be droppable via `DROP TABLE`. That second one looks like a real recovery gap. Still want it, on those grounds? Separately, I hit a pre-existing hole while wiring table initialization. `TrinoStorageConfiguration` already declares `HudiTrinoStorage` as `HOODIE_STORAGE_CLASS` — Hudi's pluggable-storage extension point — but `HudiTrinoStorage` had no `(StoragePath, StorageConfiguration)` constructor, so any `hudi-common` path resolving storage from configuration failed with "Unable to create HudiTrinoStorage". Trino is the first non-Hadoop writer, so nothing had exercised it. The fix is contained in `hudi-trino` (add the constructor, have the configuration carry the session's `TrinoFileSystem`) — verified against an unmodified `hudi-common`, with a test that pins the specific claim by asserting the failure surfaces from inside that constructor. It's independent of the DDL work. Want it as its own PR ahead of this one? **One toolchain note.** I'm on JDK 25 for this, matching your original guidance. Worth flagging that the module's enforcer accepts `[25,)` and JDK 26 builds and tests it cleanly (`--release 25` is supported), while `bootstrap_trino.sh` gates on major version exactly 25 — so a contributor on 26 can build and test the connector but not re-bootstrap Trino at a new `trino.sha`. Might be worth relaxing that gate to `>=25` if CI ever moves. **Two things I can't verify from this repo, and won't claim:** whether Spark's Hive client sets `EXTERNAL=TRUE` from `CatalogTableType.EXTERNAL`, and whether Spark actually reads a Trino-created table. Matching what hive-sync writes by default is the best available proxy, not proof. Both close only via the cross-engine E2E (`ITTestTrino*`, real HDFS + metastore), so I'm treating Spark-reads-a-Trino-created-table as the load-bearing test on this PR's critical path rather than a follow-up — and it's the one I'd most want your eyes on. GitHub link: https://github.com/apache/hudi/discussions/19484#discussioncomment-18366003 ---- This is an automatically sent email for [email protected]. To unsubscribe, please send an email to: [email protected]
