Skip to content

[SPARK-51720][SQL] Add Cross Join as legal in recursion of Recursive CTE #50308

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Conversation

Pajaraja
Copy link
Contributor

@Pajaraja Pajaraja commented Mar 18, 2025

What changes were proposed in this pull request?

Add Cross Joins as legal in recursion in Recursive CTEs.

Why are the changes needed?

Cross join is allowed in the recursion plan of Recursive CTEs in postgreSQL.

Does this PR introduce any user-facing change?

No.

How was this patch tested?

Add test to golden file cte-recursion.sql.

Was this patch authored or co-authored using generative AI tooling?

No.

@github-actions github-actions bot added the SQL label Mar 18, 2025
Copy link
Contributor

@peter-toth peter-toth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good and this scenario is allowed in some engines (like postgresql), but I would suggest a more intuitive test case like:

WITH RECURSIVE
    x(id) AS (SELECT 1 UNION SELECT 2),
    t(id, xid) AS (
        SELECT 0 AS id, 0 AS xid
        UNION ALL
        SELECT t.id + 1, xid * 10 + x.id FROM t CROSS JOIN x WHERE t.id < 3
    )                     
SELECT * FROM t
id | xid
-- | --
0  | 0
1  | 1
1  | 2
2  | 11
2  | 12
2  | 21
2  | 22
3  | 111
3  | 112
3  | 121
3  | 122
3  | 211
3  | 212
3  | 221
3  | 222


@Pajaraja
Copy link
Contributor Author

Pajaraja commented Apr 4, 2025

Looks good and this scenario is allowed in some engines (like postgresql), but I would suggest a more intuitive test case like:

WITH RECURSIVE
    x(id) AS (SELECT 1 UNION SELECT 2),
    t(id, xid) AS (
        SELECT 0 AS id, 0 AS xid
        UNION ALL
        SELECT t.id + 1, xid * 10 + x.id FROM t CROSS JOIN x WHERE t.id < 3
    )                     
SELECT * FROM t
id | xid
-- | --
0  | 0
1  | 1
1  | 2
2  | 11
2  | 12
2  | 21
2  | 22
3  | 111
3  | 112
3  | 121
3  | 122
3  | 211
3  | 212
3  | 221
3  | 222

Added this test too. Thanks!

@MaxGekk
Copy link
Member

MaxGekk commented Apr 4, 2025

@Pajaraja How about new JIRA? Specifically for the changes.

@Pajaraja Pajaraja changed the title [SPARK-50838][SQL] Add Cross Join as legal in recursion of Recursive CTE [SPARK-51720][SQL] Add Cross Join as legal in recursion of Recursive CTE Apr 4, 2025
@Pajaraja
Copy link
Contributor Author

Pajaraja commented Apr 4, 2025

@Pajaraja How about new JIRA? Specifically for the changes.

Added.

@peter-toth peter-toth closed this in 9d3f937 Apr 4, 2025
@peter-toth
Copy link
Contributor

Thanks @Pajaraja for the fix and @MaxGekk for the review!

Merged to master (4.1.0).

summaryzb pushed a commit to summaryzb/spark that referenced this pull request Apr 5, 2025
### What changes were proposed in this pull request?

Add Cross Joins as legal in recursion in Recursive CTEs.

### Why are the changes needed?

Cross join is allowed in the recursion plan of Recursive CTEs in postgreSQL.

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

No.

### How was this patch tested?
Add test to golden file cte-recursion.sql.

### Was this patch authored or co-authored using generative AI tooling?
No.

Closes apache#50308 from Pajaraja/pavle-martinovic_data/CrossJoinRecursiveCTE2.

Authored-by: pavle-martinovic_data <[email protected]>
Signed-off-by: Peter Toth <[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.

3 participants