[SPARK-58423][SQL] Raise MISSING_ATTRIBUTES for missing input after single-pass hidden-output insertion - #57637
Conversation
…ingle-pass hidden-output insertion
cloud-fan
left a comment
There was a problem hiding this comment.
0 blocking, 0 non-blocking, 1 nit.
The analyzer behavior change is sound; one broken Scaladoc reference remains.
Nits: 1 minor item (see inline comments).
Verification
I traced both analyzer paths to the shared missingAttributesError builder and verified that the new check only observes missingInput after expansion. When missingInput is empty, it returns the same expanded operator; when nonempty, it raises the intended MISSING_ATTRIBUTES subclass based on whether the child has a same-name attribute.
…ingle-pass hidden-output insertion ### What changes were proposed in this pull request? A clause like `ORDER BY` / `WHERE` / `HAVING` / `QUALIFY` / `DISTRIBUTE BY` can reference a column not in its operator's output. The single-pass resolver resolves such a column from hidden output and appends it to the `Project` / `Aggregate` / `Window` below (in `ResolvesNameByHiddenOutput.expandOperatorsOutputList`) so the clause can reference it. That append never checked whether the child actually produces the column. When the child drops it (for example `PIVOT` / `UNPIVOT`), the operator ends up referencing an attribute its child does not output. The fixed-point analyzer rejects this with a clean `MISSING_ATTRIBUTES` error, but the single-pass resolver hit a failed assert in the resolution validator and reported `INTERNAL_ERROR`. This PR runs a missing-input check right after `expandOperatorsOutputList` builds the operator, so the single-pass resolver raises the same `MISSING_ATTRIBUTES` error. The error is built by a shared `QueryCompilationErrors.missingAttributesError` used by both the single-pass call sites and the fixed-point `CheckAnalysis`. ### Why are the changes needed? An invalid query surfaced as `INTERNAL_ERROR` under the single-pass resolver instead of the clean `MISSING_ATTRIBUTES` the fixed-point analyzer already reports. This aligns the two. ### Does this PR introduce _any_ user-facing change? Yes. For the affected invalid queries, the single-pass resolver now reports `MISSING_ATTRIBUTES` instead of `INTERNAL_ERROR`. Valid queries are unaffected. ### How was this patch tested? New tests in `DataFramePivotSuite`, `DatasetUnpivotSuite`, and `BinBySuite` assert `MISSING_ATTRIBUTES` for a sort on a column dropped or re-output by `PIVOT` / `UNPIVOT` / `BIN BY`. A new `DataFrameAnalyzerTestGapsSuite` test confirms the check does not fire on a valid query. Existing `AnalysisErrorSuite` coverage confirms the shared error message is unchanged. ### Was this patch authored or co-authored using generative AI tooling? Generated-by: Claude Code (Anthropic) Closes #57637 from vranes/missing-attributes-at-hidden-output-insertion. Authored-by: Nikolina Vraneš <nikolina.vranes@databricks.com> Signed-off-by: Wenchen Fan <wenchen@databricks.com> (cherry picked from commit b14a426) Signed-off-by: Wenchen Fan <wenchen@databricks.com>
…ingle-pass hidden-output insertion ### What changes were proposed in this pull request? A clause like `ORDER BY` / `WHERE` / `HAVING` / `QUALIFY` / `DISTRIBUTE BY` can reference a column not in its operator's output. The single-pass resolver resolves such a column from hidden output and appends it to the `Project` / `Aggregate` / `Window` below (in `ResolvesNameByHiddenOutput.expandOperatorsOutputList`) so the clause can reference it. That append never checked whether the child actually produces the column. When the child drops it (for example `PIVOT` / `UNPIVOT`), the operator ends up referencing an attribute its child does not output. The fixed-point analyzer rejects this with a clean `MISSING_ATTRIBUTES` error, but the single-pass resolver hit a failed assert in the resolution validator and reported `INTERNAL_ERROR`. This PR runs a missing-input check right after `expandOperatorsOutputList` builds the operator, so the single-pass resolver raises the same `MISSING_ATTRIBUTES` error. The error is built by a shared `QueryCompilationErrors.missingAttributesError` used by both the single-pass call sites and the fixed-point `CheckAnalysis`. ### Why are the changes needed? An invalid query surfaced as `INTERNAL_ERROR` under the single-pass resolver instead of the clean `MISSING_ATTRIBUTES` the fixed-point analyzer already reports. This aligns the two. ### Does this PR introduce _any_ user-facing change? Yes. For the affected invalid queries, the single-pass resolver now reports `MISSING_ATTRIBUTES` instead of `INTERNAL_ERROR`. Valid queries are unaffected. ### How was this patch tested? New tests in `DataFramePivotSuite`, `DatasetUnpivotSuite`, and `BinBySuite` assert `MISSING_ATTRIBUTES` for a sort on a column dropped or re-output by `PIVOT` / `UNPIVOT` / `BIN BY`. A new `DataFrameAnalyzerTestGapsSuite` test confirms the check does not fire on a valid query. Existing `AnalysisErrorSuite` coverage confirms the shared error message is unchanged. ### Was this patch authored or co-authored using generative AI tooling? Generated-by: Claude Code (Anthropic) Closes #57637 from vranes/missing-attributes-at-hidden-output-insertion. Authored-by: Nikolina Vraneš <nikolina.vranes@databricks.com> Signed-off-by: Wenchen Fan <wenchen@databricks.com> (cherry picked from commit b14a426) Signed-off-by: Wenchen Fan <wenchen@databricks.com>
HyukjinKwon
left a comment
There was a problem hiding this comment.
0 blocking, 0 non-blocking, 0 nits.
Clean analyzer robustness fix with a tidy error-consolidation; single-pass and fixed-point stay in parity.
Verification
Confirmed checkMissingInput uses the standard operator.missingInput after hidden-output expansion and that all three paths route through the one shared error helper, so messages/conditions are identical by construction; the single-pass private duplicate is removed. Pivot/Unpivot/BinBy suites exercise the new error.
Scanner note: the contract_claim_verifier scanner could not complete on this PR (an infrastructure hang, unrelated to the change), so its automated behavioral/type-contract pass was skipped; the text_quality and local_efficiency scanners ran clean, and the contract/error-parity claims were verified manually here instead.
What changes were proposed in this pull request?
A clause like
ORDER BY/WHERE/HAVING/QUALIFY/DISTRIBUTE BYcan reference a column not in its operator's output. The single-pass resolver resolves such a column from hidden output and appends it to theProject/Aggregate/Windowbelow (inResolvesNameByHiddenOutput.expandOperatorsOutputList) so the clause can reference it.That append never checked whether the child actually produces the column. When the child drops it (for example
PIVOT/UNPIVOT), the operator ends up referencing an attribute its child does not output. The fixed-point analyzer rejects this with a cleanMISSING_ATTRIBUTESerror, but the single-pass resolver hit a failed assert in the resolution validator and reportedINTERNAL_ERROR.This PR runs a missing-input check right after
expandOperatorsOutputListbuilds the operator, so the single-pass resolver raises the sameMISSING_ATTRIBUTESerror. The error is built by a sharedQueryCompilationErrors.missingAttributesErrorused by both the single-pass call sites and the fixed-pointCheckAnalysis.Why are the changes needed?
An invalid query surfaced as
INTERNAL_ERRORunder the single-pass resolver instead of the cleanMISSING_ATTRIBUTESthe fixed-point analyzer already reports. This aligns the two.Does this PR introduce any user-facing change?
Yes. For the affected invalid queries, the single-pass resolver now reports
MISSING_ATTRIBUTESinstead ofINTERNAL_ERROR. Valid queries are unaffected.How was this patch tested?
New tests in
DataFramePivotSuite,DatasetUnpivotSuite, andBinBySuiteassertMISSING_ATTRIBUTESfor a sort on a column dropped or re-output byPIVOT/UNPIVOT/BIN BY. A newDataFrameAnalyzerTestGapsSuitetest confirms the check does not fire on a valid query. ExistingAnalysisErrorSuitecoverage confirms the shared error message is unchanged.Was this patch authored or co-authored using generative AI tooling?
Generated-by: Claude Code (Anthropic)