[SPARK-58260][SQL] Add config to render Hive DDL in SHOW CREATE TABLE for Hive tables#57429
[SPARK-58260][SQL] Add config to render Hive DDL in SHOW CREATE TABLE for Hive tables#57429pan3793 wants to merge 2 commits into
Conversation
| case ShowCreateTable(ResolvedV1TableOrViewIdentifier(ident), asSerde, output) if asSerde => | ||
| case ShowCreateTable(ResolvedV1TableOrViewIdentifier(ident), asSerde, output) if asSerde || | ||
| (conf.hiveTableShowCreateTableAsSerde && | ||
| DDLUtils.isHiveTable(catalogManager.v1SessionCatalog.getTableRawMetadata(ident))) => |
There was a problem hiding this comment.
Can we reuse the metadata already in the resolved plan instead of re-fetching it ?
There was a problem hiding this comment.
Good point, done. The guard now matches ResolvedTable(catalog, _, t: V1Table, _) directly and reuses t.catalogTable instead of re-fetching via getTableRawMetadata(ident) — same pattern as the existing SHOW CREATE TABLE case just below.
The view handling is split into its own case since ResolvedV1TableOrViewIdentifier covered both: views keep the explicit AS SERDE routing, and do not need the config check because isHiveTable is always false for them (provider is None — temp views are built without one, and HiveExternalCatalog.restoreTableMetadata skips restoreHiveSerdeTable for view-like tables). Views also never go through the Hive-to-Spark DDL conversion this config bypasses, so plain SHOW CREATE TABLE on a view is unchanged either way.
Existing ShowCreateTableSuite (core v1/v2 + Hive) and view suites (PersistedViewTestSuite, temp view suites, HiveSQLViewSuite) all pass.
|
CI failure is unrelated, will be fixed by #57431 |
viirya
left a comment
viirya
left a comment
There was a problem hiding this comment.
The implementation is clean and I verified it end to end. Splitting the old ResolvedV1TableOrViewIdentifier case into a ResolvedTable(V1Table) case plus a ResolvedViewIdentifier case is exactly equivalent for the asSerde = true path (ResolvedV1TableOrViewIdentifier is defined as the union of ResolvedV1TableIdentifier and ResolvedViewIdentifier, and ResolvedV1TableIdentifier is ResolvedTable(V1Table) if supportsV1Command), so there's no regression — the new case just adds the config-gated Hive extension on top, and its ordering ahead of the later Hive-table->ShowCreateTableCommand case is correct. Reusing t.catalogTable instead of re-fetching addresses the earlier review nicely. The NOT_APPLICABLE binding policy is the right call (the config only affects display, never the resolution of a view/UDF body), and the guard's isHiveTable gate is consistent with the command's own isDatasourceTable check. Default-off means no behavior change, and the tests cover the meaningful cases (Hive table matches AS SERDE, data-source table and view unaffected, RCFILE and transactional tables render instead of erroring).
One thing worth surfacing — a direction/consensus point, not a code issue. The "plain SHOW CREATE TABLE always emits Spark DDL" behavior this config relaxes was a deliberate design decision in SPARK-27946 (#24938), not an oversight. In that PR @gatorsmile explicitly steered it: "let us make it more aggressive here… always show how to create Spark native tables if possible. This will further simplify the migration from Hive to Spark. To the existing Spark users who prefer keeping Hive serde formats, we can introduce a new option AS SERDE…" (gengliangwang +1'd it). The specific failure modes this PR cites were also intentional at the time: the transactional-table error was added on purpose in response to a review question, and cloud-fan explicitly asked that the unsupported-serde error (e.g. RCFILE) guide users to run SHOW CREATE TABLE AS SERDE rather than convert. So AS SERDE was designed as the intended escape hatch for exactly these cases.
This PR doesn't overturn that design — it keeps the default and just turns "type AS SERDE every time" into a session-level toggle. I don't have concerns with the PR itself, but since the original direction had strong committer consensus, it'd be good to (a) note this SPARK-27946 context in the PR description, and (b) have @cloud-fan / @gengliangwang confirm they're on board with providing a config shortcut, since both were deeply involved in that original decision.
ulysses-you
left a comment
ulysses-you
left a comment
There was a problem hiding this comment.
LGTM, thank you @pan3793
|
thank you, @viirya, I updated the PR description to mention the background. cc @cloud-fan @gengliangwang, are you fine with adding this config? |
…ching from catalog
|
Rebased to fix the compile error. |
3 participants
What changes were proposed in this pull request?
Add a new config
spark.sql.hive.showCreateTableAsSerde(defaultfalse). When enabled,plain
SHOW CREATE TABLEon a Hive serde table routes to the existingShowCreateTableAsSerdeCommandand renders Hive DDL, instead of converting to Spark DDL.Why are the changes needed?
We get some reports from customers: they are confused that
CREATE TABLE ... STORED AS ParquetbutSHOW CREATE TABLE ...displaysCREATE TABLE ... USING Parquet.Background: SPARK-27946 (#24938) deliberately changed plain
SHOW CREATE TABLEto always emit Spark DDL, to push and simplify migration from Hive to Spark.AS SERDEwas introduced in the same PR as the intended escape hatch for users who still need Hive DDL, and the failure modes were intentional: transactional Hive tables error out by design, and tables whose serde has no Spark data source mapping (e.g. RCFILE) are meant to be served bySHOW CREATE TABLE AS SERDE.This PR keeps that direction intact: the default behavior is unchanged, and
AS SERDEremains the per-query escape hatch. It only adds a session-level toggle that turns "appendAS SERDEevery time" into a config, which helps users and integrations (e.g. BI tools, metastore sync jobs) that cannot easily rewrite the SQL they issue.Does this PR introduce any user-facing change?
Yes. New config
spark.sql.hive.showCreateTableAsSerde, defaultfalse(no behaviorchange). When
true,SHOW CREATE TABLEon Hive serde tables returns Hive DDL; datasource tables and views are unaffected.
How was this patch tested?
Added tests to
o.a.s.sql.hive.execution.command.ShowCreateTableSuitecovering: Hivetable (matches
AS SERDEoutput), data source table and view (unaffected), RCFILE andtransactional Hive tables (Hive DDL rendered instead of erroring).
Was this patch authored or co-authored using generative AI tooling?
Generated-by: KIMI K3