Skip to content

Conversation

@MaxGekk
Copy link
Member

@MaxGekk MaxGekk commented Jul 10, 2023

What changes were proposed in this pull request?

In the PR, I propose to resolve WITH on top of INSERT INTO via the regular code path similar to non-commands. In this way, InsertIntoStatement is excluded from commands in the rule CTESubstitution, and UnresolvedWith is moved into InsertIntoStatement on top of its query.

Why are the changes needed?

To improve code maintenance. Right now the CTE resolution code path is diverged: query with commands go into CTE inline code path where non-command queries go into CTEDef code path.

Does this PR introduce any user-facing change?

No.

How was this patch tested?

By running new test:

$ build/sbt "test:testOnly *InsertSuite"

@github-actions github-actions bot added the SQL label Jul 10, 2023
@MaxGekk MaxGekk changed the title [WIP][SQL] Support WITH ... INSERT INTO [WIP][SPARK-44356][SQL] Support WITH ... INSERT INTO Jul 10, 2023
@MaxGekk MaxGekk changed the title [WIP][SPARK-44356][SQL] Support WITH ... INSERT INTO [WIP][SPARK-44356][SQL] Resolve WITH on top of INSERT INTO via CTEDef Jul 11, 2023
@MaxGekk MaxGekk changed the title [WIP][SPARK-44356][SQL] Resolve WITH on top of INSERT INTO via CTEDef [SPARK-44356][SQL] Resolve WITH on top of INSERT INTO via CTEDef Jul 11, 2023
@MaxGekk MaxGekk marked this pull request as ready for review July 11, 2023 07:12
@cloud-fan cloud-fan closed this in da84f81 Jul 25, 2023
turboFei pushed a commit to turboFei/spark that referenced this pull request Nov 6, 2025
This PR is based on apache#42022 to fix
tests, as the PR author is on vacation.

### What changes were proposed in this pull request?
In the PR, I propose to add new trait `CTEInChildren` and mix it to some
commands that should have `WithCTE` on top of their children (queries)
instead of main query. Also I modified the `CTESubstitution` rule and
removed special handling of `Command`s and similar nodes. After the
changes, `Command`, `ParsedStatement` and `InsertIntoDir` are handled in
the same way as other queries by referring to CTE Defs. Only the
difference is in `WithCTE` is not not placed on the top of main query
but on top of command queries.

Closes apache#41922

### Why are the changes needed?
To improve code maintenance. Right now the CTE resolution code path is
diverged: query with commands go into CTE inline code path where
non-command queries go into CTEDef code path.

### Does this PR introduce _any_ user-facing change?
No.

### How was this patch tested?
By running new test:
```
$ build/sbt "test:testOnly *InsertSuite"
```

Closes apache#42036 from cloud-fan/help.

Lead-authored-by: Max Gekk <[email protected]>
Co-authored-by: Wenchen Fan <[email protected]>
Co-authored-by: Wenchen Fan <[email protected]>
Signed-off-by: Wenchen Fan <[email protected]>

(cherry picked from commit da84f81)

Co-authored-by: Max Gekk <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant