Skip to content

[SPARK-57524][SQL] Correct SHOW TABLE EXTENDED property formatting#56587

Open
huangxiaopingRD wants to merge 1 commit into
apache:masterfrom
huangxiaopingRD:SPARK-57524
Open

[SPARK-57524][SQL] Correct SHOW TABLE EXTENDED property formatting#56587
huangxiaopingRD wants to merge 1 commit into
apache:masterfrom
huangxiaopingRD:SPARK-57524

Conversation

@huangxiaopingRD

Copy link
Copy Markdown
Contributor

What changes were proposed in this pull request?

The purpose of this change is to fix how SHOW TABLE EXTENDED formats table properties for v2 tables.

Why are the changes needed?

Fix a bug.Before this change, the code converted the redacted property entries into a single string too early, and the later mkString call operated on that string character by character. As a result, the Table Properties output could be formatted incorrectly. This update keeps the properties as a list of key=value entries until the final formatting step, so the command produces the expected bracketed, comma-separated property list.

Does this PR introduce any user-facing change?

Yes.
Execute show table extended:
show table extended in paimon.dev_xxx like 'test_show_table_extended';

Before this PR, the output result is:
image

After this PR:
image

How was this patch tested?

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

No

@MaxGekk MaxGekk left a comment

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.

0 blocking, 2 non-blocking, 0 nits.
A correct, minimal bug fix. The root cause diagnosis is right: properties was collapsed into a single String by the early .mkString("[", ",", "]"), so the later properties.mkString(...) iterated that string character-by-character. Keeping it a List[String] until the final mkString is the right fix, and it matches the existing getTablePartitionDetails pattern in the same file.

Correctness (1)

  • ShowTablesSuiteBase.scala:424: no regression test pins this fix. The shared replace() helper masks the value wholesale (case s"Table Properties:$_" => "Table Properties: <table properties>"), so every show table extended assertion compares the masked string — the pre-fix garbage output would have masked to the same <table properties> and passed, and a future regression would stay invisible too. Please add a v2 ShowTablesSuite case that creates a table with TBLPROPERTIES ('p1'='v1','p2'='v2') and asserts the unmasked Table Properties: [p1=v1, p2=v2] line (or stop masking Table Properties for the v2 suite).

Suggestions (1)

  • ShowTablesExtendedExec.scala:105 (pre-existing, out of scope): the if (!table.properties().isEmpty) guard tests the raw properties while the displayed list is filtered, so an all-reserved-properties table prints Table Properties: [].

PR description suggestions

  • Document: "How was this patch tested?" is empty — please state whether a test was added (and if not, why).

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