cloud-fan commented on code in PR #59694:
URL: https://github.com/apache/spark/pull/59694#discussion_r4348848340
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/ColumnResolutionHelper.scala:
##########
@@ -688,8 +689,13 @@ trait ColumnResolutionHelper extends Logging with
DataTypeErrorsBase {
// via `Project.hiddenOutputTag`). We accept that here but tag the
candidate
// as `hidden` so the top-level merge in `resolveDataFrameColumn` can
prefer
// a regular (p.output) match over hidden (p.metadataOutput) ones.
+ //
+ // A candidate is hidden only if it is not visible via `p.output`. An
attribute
Review Comment:
**Nit (P3):** This paragraph says a candidate "is hidden only if it is not
visible via `p.output`" and that a re-selected hidden right key "must stay
regular". The predicate on line 698 is `c.hidden ||
!c.expr.references.subsetOf(p.outputSet)`, so a candidate tagged hidden at a
lower node stays hidden at `p` even when `p.outputSet` contains its references.
For example, `dim["dk"]` resolved above `sel` passes through the USING wrapper
first, where `dk#dim` is only in `hiddenOutputTag`, so it stays hidden at `sel`
even though `sel` outputs it. The `Candidate.hidden` doc at lines 607-610 ("at
some plan node along the way") is accurate, so the two comments now disagree.
This tiering has needed several fixes already, so the next person changing it
should not have to work this out from the code. Something like:
```scala
// A candidate becomes hidden at `p` when its references are not all in
`p.outputSet`,
// and once hidden it stays hidden further up (`c.hidden`). An attribute
can be in both
// `p.output` and `p.metadataOutput`, e.g. a USING join's hidden right
key that a Project
// above re-selects (`Project.metadataOutput` inherits the child's).
That Project does not
// hide the key itself, but a candidate already hidden below it stays
hidden.
```
If the question below about widened nodes leads to a narrower rule, write
the comment for that rule instead.
**Verification:**
- **Inspection:** Read the reworded paragraph next to the predicate and the
Candidate.hidden description, and confirm it states the sticky behavior, makes
no claim that latched candidates contradict, and uses only ASCII characters
within the 100-character line limit.
--
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]
