sarutak commented on PR #59425:
URL: https://github.com/apache/spark/pull/59425#issuecomment-6277612500
@rdtr Could you address @HyukjinKwon's two P3 comments when you get a
chance? Both look correct
to me:
1. `SparkConnectPlanner.scala` (the `case _` fallback): `transformRelation`
recurses into child
relations, so a valid SQL-with-references WITH_RELATIONS wrapped under
another node (e.g.
`Limit(WithRelations{root: CREATE TABLE ...})`) still runs inside the
guard, going through
`transformLimit`, then `transformWithRelations`, then
`executeSQLWithRefs`. The "side-effect
free" / "nested WITH_RELATIONS cannot re-trigger" wording promises more
than the two match arms
deliver; narrowing it to what the arms cover (top-level or direct
reference input) and noting the
wrapped case as a known limitation would be accurate.
2. The three new tests never reach the recursive reference arm:
`withRelationsRelation` always
builds reference inputs with `sqlRelation(...)`, so replacing the
recursion with a non-recursive
`transformRelation` check would keep them green. A reference whose input
is itself a
WITH_RELATIONS with a `CREATE TABLE` root would pin that arm.
I also left one related inline nit on the test helper (flow-function
coverage) that fits naturally
with HyukjinKwon's helper-variant suggestion.
--
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]
