Skip to content

[SPARK-56652][SQL] Always emit RELY/NORELY in DESCRIBE EXTENDED constraint output#55590

Closed
yyanyy wants to merge 2 commits into
apache:masterfrom
yyanyy:desc-extended-rely-norely
Closed

[SPARK-56652][SQL] Always emit RELY/NORELY in DESCRIBE EXTENDED constraint output#55590
yyanyy wants to merge 2 commits into
apache:masterfrom
yyanyy:desc-extended-rely-norely

Conversation

@yyanyy
Copy link
Copy Markdown
Contributor

@yyanyy yyanyy commented Apr 28, 2026

What changes were proposed in this pull request?

BaseConstraint.toDescription() now always emits RELY|NORELY, matching toDDL() used by SHOW CREATE TABLE. Previously, toDescription() skipped the NORELY token when rely=false, so the same constraint rendered differently across the two surfaces.

After this change, the invariant holds:
toDDL is "CONSTRAINT " + name + " " + toDescription.

Why are the changes needed?

After SPARK-52141 (DESCRIBE EXTENDED) and SPARK-52142 (SHOW CREATE TABLE) landed, the two surfaces render the same constraint asymmetrically:

State DESCRIBE EXTENDED (before) SHOW CREATE TABLE
PK default PRIMARY KEY (a) NOT ENFORCED PRIMARY KEY (a) NOT ENFORCED NORELY
CHECK default CHECK (...) ENFORCED CHECK (...) ENFORCED NORELY

The original review on PR #51577 considered skipping both NOT ENFORCED and NORELY when default, but only NORELY was skipped because ENFORCED's default is per-subclass and not visible to BaseConstraint. The result is a confusing experience: SHOW CREATE TABLE reports the rely state explicitly while DESCRIBE EXTENDED hides it for the default value.

Does this PR introduce any user-facing change?

Slight output change in DESCRIBE EXTENDED command output. DESCRIBE EXTENDED now emits NORELY for constraints whose rely value is the default. PRIMARY KEY / UNIQUE / FOREIGN KEY / CHECK constraints all show the full ENFORCED|NOT ENFORCED RELY|NORELY suffix, matching SHOW CREATE TABLE.

How was this patch tested?

Updated golden strings in the v2 DescribeTableSuite "desc table constraints" test. Ran:

build/sbt 'sql/testOnly
org.apache.spark.sql.execution.command.v2.DescribeTableSuite
org.apache.spark.sql.execution.command.v2.ShowCreateTableSuite'

41 tests pass.

Was this patch authored or co-authored using generative AI tooling?

Claude Opus 4.7 

…raint output

### What changes were proposed in this pull request?

`BaseConstraint.toDescription()` now always emits `RELY|NORELY`,
matching `toDDL()` used by SHOW CREATE TABLE. Previously,
`toDescription()` skipped the `NORELY` token when `rely=false`, so the
same constraint rendered differently across the two surfaces.

After this change, the invariant holds:
`toDDL` is `"CONSTRAINT " + name + " " + toDescription`.

### Why are the changes needed?

After SPARK-52141 (DESCRIBE EXTENDED) and SPARK-52142 (SHOW CREATE
TABLE) landed, the two surfaces render the same constraint
asymmetrically:

| State        | DESCRIBE EXTENDED (before)        | SHOW CREATE TABLE                   |
|--------------|-----------------------------------|-------------------------------------|
| PK default   | PRIMARY KEY (a) NOT ENFORCED      | PRIMARY KEY (a) NOT ENFORCED NORELY |
| CHECK default| CHECK (...) ENFORCED              | CHECK (...) ENFORCED NORELY         |

The original review on PR apache#51577 considered skipping both
`NOT ENFORCED` and `NORELY` when default, but only `NORELY` was
skipped because `ENFORCED`'s default is per-subclass and not visible
to `BaseConstraint`. The result is a confusing experience: SHOW CREATE
TABLE reports the rely state explicitly while DESCRIBE EXTENDED hides
it for the default value.

### Does this PR introduce any user-facing change?

Yes. DESCRIBE EXTENDED now emits `NORELY` for constraints whose `rely`
value is the default. PRIMARY KEY / UNIQUE / FOREIGN KEY / CHECK
constraints all show the full `ENFORCED|NOT ENFORCED RELY|NORELY`
suffix, matching SHOW CREATE TABLE.

### How was this patch tested?

Updated golden strings in the v2 `DescribeTableSuite` "desc table
constraints" test. Ran:

  build/sbt 'sql/testOnly \
    org.apache.spark.sql.execution.command.v2.DescribeTableSuite \
    org.apache.spark.sql.execution.command.v2.ShowCreateTableSuite'

41 tests pass.

### Was this patch authored or co-authored using generative AI tooling?

No.
@yyanyy yyanyy changed the title [SPARK-XXXXX][SQL] Always emit RELY/NORELY in DESCRIBE EXTENDED constraint output [SPARK-56652][SQL] Always emit RELY/NORELY in DESCRIBE EXTENDED constraint output Apr 28, 2026
Copy link
Copy Markdown
Member

@gengliangwang gengliangwang left a comment

Choose a reason for hiding this comment

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

The change is minimal and correct, and the PR description accurately characterizes the user-visible alignment with SHOW CREATE TABLE. One small suggestion inline.

@@ -74,13 +74,11 @@ public String toDDL() {
}

public String toDescription() {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Since this PR establishes the invariant toDDL == "CONSTRAINT " + name + " " + toDescription, consider implementing it structurally so the two methods can't drift apart, e.g. simplify toDDL above to:

@Override
public String toDDL() {
  // The validation status is not included in the DDL output as it's not part of
  // the Spark SQL syntax for constraints.
  return "CONSTRAINT " + name + " " + toDescription();
}

This also removes the duplicated String.format template.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

good point, updated, thanks!

Per Gengliang's review on PR apache#55590, encode the invariant
`toDDL == "CONSTRAINT " + name + " " + toDescription` structurally
so the two methods can't drift apart. Removes the duplicated
String.format template in toDDL.

Behavior is unchanged; existing golden strings in ConstraintSuite,
v2/ShowCreateTableSuite, and v2/DescribeTableSuite still hold.
@gengliangwang
Copy link
Copy Markdown
Member

Thanks,merging to master

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants