vladanvasi-db commented on code in PR #60169:
URL: https://github.com/apache/spark/pull/60169#discussion_r4491696083
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/KeepLegacyOutputs.scala:
##########
@@ -33,9 +34,7 @@ object KeepLegacyOutputs extends Rule[LogicalPlan] {
plan.resolveOperatorsUpWithPruning(
_.containsPattern(COMMAND)) {
case s: ShowTables =>
- assert(s.output.length == 3)
- val newOutput = s.output.head.withName("database") +: s.output.tail
- s.copy(output = newOutput)
+ keepLegacyShowTablesOutput(s)
Review Comment:
@mihailoale-db This should remain refactor-only. I rechecked all seven
changed files at `6f25862f28c`, including an independent review, and found no
change to the existing analyzer behavior.
For this hunk, the [original `apply` already checked
`LEGACY_KEEP_COMMAND_OUTPUT_SCHEMA` before entering the
traversal](https://github.com/apache/spark/blob/352ac9bf22806d6aa2047bf3c62096697d0afeb1/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/KeepLegacyOutputs.scala#L29-L38):
- With the flag **false**, `apply` returns the original plan before reaching
this case, both before and after the extraction.
- With the flag **true**, the helper performs the same three-column
assertion, first-attribute `withName("database")`, and `ShowTables.copy` as the
original branch. The remaining output attributes are unchanged, and `withName`
preserves the first attribute's ExprId, qualifier, type, nullability, and
metadata.
The additional flag check inside the helper is redundant at this existing
production call site. Both checks use the same active `SQLConf`, and this path
introduces no configuration update or scope change between them. The check
gives the extracted helper the same configuration contract when called
independently.
I also compared the namespace-resolution extraction and all four SHOW
lowering branches with the base: metadata is still loaded only on request;
namespace validation still precedes the SHOW TABLES V1 guard; SHOW TABLE
EXTENDED retains its existing lowering and legacy-schema guard; and
ViewCatalog/V2/unresolved handling is unchanged. The existing rule traversal
continues to supply origin and tag propagation.
The recorded validation passed **273 selected tests** and all four
Catalyst/SQL production/test style checks. The focused flag matrix covers both
legacy-schema settings crossed with both V1-command settings and checks output
ExprIds. SQL/Hive tests used the temporary Snowflake test-driver exclusion
documented in the PR; the dependency files are unchanged.
Please point me to the input or configuration you believe changes behavior
so I can check that case directly.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
