cloud-fan commented on code in PR #59267:
URL: https://github.com/apache/spark/pull/59267#discussion_r4257662392
##########
sql/core/src/test/scala/org/apache/spark/sql/AsOfJoinSQLSuite.scala:
##########
@@ -157,6 +157,69 @@ class AsOfJoinSQLSuite extends QueryTest with
SharedSparkSession {
stop = 137)))
}
+ // Tests run both analyzers and compare their plans (dualRunWithLegacy), so
checkAnswer also
+ // covers the single-pass analyzer. LEFT JOIN leaves d.k NULL on some rows,
so a match on the
+ // visible k column gives a different answer.
+ test("MATCH_CONDITION accepts a hidden USING column of the right input") {
Review Comment:
**Nit (P3):** All the new positive tests, here and in
`join-asof-semantics.sql`, use `SELECT l.lts, r.rts` directly over the ASOF
JOIN. So the only path they exercise is ProjectResolver wrapping the join.
Three other paths reach the new AsOfJoin case in `retainOriginalJoinOutput`:
- FilterResolver, which passes an empty output list
- AggregateResolver, which passes the aggregate list
- the no-wrap path, taken when the hidden key is itself selected
None of them would fail a test under dual-run if it broke. Example queries:
`SELECT count(*) ... MATCH_CONDITION (l.lts >= d.k)`, the same join with `WHERE
l.lts > 0`, and `SELECT l.lts, r.rts, d.k ...`. By my trace they match
fixed-point today, so these tests would guard against future regressions rather
than expose a current bug.
**Verification:**
- **Behavior:** The Aggregate-parent, Filter-parent and selected-hidden-key
ASOF JOIN queries return the expected rows, and single-pass matches fixed-point
plans under dual-run.
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/resolver/RetainsOriginalJoinOutput.scala:
##########
@@ -127,10 +127,30 @@ trait RetainsOriginalJoinOutput {
referencedAttributes = childReferencedAttributes
) =>
Project(scopes.current.output, join)
+ case asOfJoin: AsOfJoin
Review Comment:
**Non-blocking (P2):** This case only fires when the AsOfJoin is the direct
child of the parent Project, Filter or Aggregate. When the ASOF JOIN has its
own USING clause, `AsOfJoinResolver.buildUsingProject` has already wrapped it
in the USING Project, so the parent sees a `Project` and takes `case other`.
Fixed-point handles the same query differently. `AddMetadataColumns` skips
the USING Project, which has no metadata reference, and wraps the AsOfJoin,
whose match condition references `d.k`, in `Project(original output)`. For
example:
```sql
SELECT l.lts, r.rts
FROM (VALUES (1, 10), (2, 20)) AS l(k, lts)
ASOF JOIN ((VALUES (1, 5), (8, 15)) AS r(k, rts) LEFT JOIN (VALUES (1)) AS
d(k) USING (k))
MATCH_CONDITION (l.lts >= d.k) USING (k)
```
Fixed-point builds `Project <- Project(using) <- Project(AsOfJoin output) <-
AsOfJoin`. Single-pass builds `Project <- Project(using) <- AsOfJoin`, and
`NormalizePlan` does not collapse adjacent Projects.
Before this PR, this query failed in both analyzers with
ASOF_JOIN_MATCH_CONDITION_TABLE_REFERENCE. Now it resolves, and in dual-run
(the default in tests) it throws LOGICAL_PLAN_COMPARISON_MISMATCH. The default
fixed-point path returns correct rows. No new test covers `MATCH_CONDITION ...
USING` with a hidden operand.
**Recommended change:** In AsOfJoinResolver.buildUsingProject, when the
resolved AsOfJoin's own expressions reference a hidden metadata column that is
resolvable from hidden output and absent from both children's visible output,
insert Project(AsOfJoin.computeOutput(joinType, left output, right output))
over it, and build the USING Project on top of that. Use the same guard helper
as RetainsOriginalJoinOutput. Keep the USING Project's appended
qualified-access-only columns limited to attributes the restored child still
produces. Add ASOF JOIN analyzer tests for an outer USING clause combined with
a hidden-key MATCH_CONDITION operand.
**Why this works:** This reproduces fixed-point's plan, Project(using) <-
Project(original AsOfJoin output) <- AsOfJoin, at the one place single-pass
builds that shape, so dual-run normalized plans match. Reusing the shared guard
keeps this site aligned with any later generalization of the restoring-Project
semantics (SPARK-60035).
**Scope:** Single-pass ASOF JOIN USING branch: insert the inner restoring
Project and keep the USING Project's appended columns consistent. Add parity
tests for the outer-USING form.
**Compatibility:** Preserve behavior outside the stated repair boundary.
**Risks:** If the inner Project drops the hidden key while the USING Project
still appends it as a qualified-access-only column, the plan references a
missing attribute. Changing the USING scope's hidden output could alter
single-pass name visibility for references above the ASOF USING. Keep the
existing visibility unless a test proves fixed-point parity.
**Constraints:** The ASOF USING plan must stay unchanged when no AsOfJoin
expression references a nested hidden key. The existing MATCH_CONDITION ...
USING goldens must not change. Fixed-point behaviour and user-visible results
must not change.
**Success:** For `l ASOF JOIN (r LEFT JOIN d USING (k)) MATCH_CONDITION
(l.lts >= d.k) USING (k)`, single-pass produces Project(using) <-
Project(original AsOfJoin output) <- AsOfJoin, matching fixed-point, and
dual-run raises no plan mismatch. ASOF USING queries without a hidden-key
reference keep their current analyzed plans.
**Verification:**
- **Regression:** An ASOF JOIN with an outer USING clause and a
MATCH_CONDITION operand that is a hidden key of a nested USING join resolves
under dual-run with identical single-pass and fixed-point plans, and returns
the expected rows.
- **Compatibility:** Existing ASOF JOIN MATCH_CONDITION ... USING golden
plans and results are unchanged.
--
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]
