Skip to content
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.

Sign up for GitHub

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

Jump to bottom

sql: UNION variant of WITH RECURSIVE not implemented #46642

Closed
RaduBerinde opened this issue Mar 26, 2020 · 4 comments · Fixed by #71685
Closed

sql: UNION variant of WITH RECURSIVE not implemented #46642

RaduBerinde opened this issue Mar 26, 2020 · 4 comments · Fixed by #71685
Assignees
@RaduBerinde
Labels
A-sql-pgcompat Semantic compatibility with PostgreSQL A-tools-postgrest T-sql-queries SQL Queries Team

Comments

@RaduBerinde
Copy link
Member

RaduBerinde commented Mar 26, 2020 •
edited by exalate-issue-sync bot
Loading

Postgres supports two forms of RECURSIVE WITH: using either UNION or UNION ALL. Other databases (like SQL Server) only implement UNION ALL.

CockroachDB currently only implements UNION ALL. If you are a user of CockroachDB who needs to use this variant, please post in this issue and provide more details on the usecase.

Epic CRDB-10357
@jlinder jlinder added the T-sql-queries SQL Queries Team label Jun 16, 2021
@nvanbenschoten
Copy link
Member

I don't understand the motivation, but we do see a use of the WITH RECURSIVE .. UNION .. variant in PostgREST here and here. My best guess is that the query (introduced here) is performing a breadth-first search through the Postgres data type inheritance hierarchy and needs UNION to deduplicate and avoid infinite loops in some cases. Or there is no motivation and the query was written this way because it didn't make a difference.

Either way, this may be a blocker for PostgREST support.

@awoods187 awoods187 mentioned this issue Aug 16, 2021
Closed
22 tasks
@nvanbenschoten
Copy link
Member

@RaduBerinde do you have a feel for the implementation complexity of this functionality?

@RaduBerinde
Copy link
Member Author

Probably 2-4 weeks of work I'd say

@nvanbenschoten nvanbenschoten added the A-sql-pgcompat Semantic compatibility with PostgreSQL label Sep 10, 2021
@RaduBerinde RaduBerinde self-assigned this Oct 14, 2021
@RaduBerinde
Copy link
Member Author

This looks easier than I thought, I think we can just use a de-duplicating row container.

RaduBerinde added a commit to RaduBerinde/cockroach that referenced this issue Oct 19, 2021
@RaduBerinde
sql: implement WITH RECURSIVE with UNION
9a1f644 
This change implements the UNION variant of WITH RECURSIVE, where rows
are deduplicated. We achieve this by storing all rows in a
deduplicating container and inserting in that container first,
detecting if the row is a duplicate.

Fixes cockroachdb#46642.

Release note (sql change): The WITH RECURSIVE variant that uses UNION
(as opposed to UNION ALL) is now supported.
@RaduBerinde RaduBerinde mentioned this issue Oct 19, 2021
Merged
RaduBerinde added a commit to RaduBerinde/cockroach that referenced this issue Oct 20, 2021
@RaduBerinde
sql: implement WITH RECURSIVE with UNION
42c009f 
This change implements the UNION variant of WITH RECURSIVE, where rows
are deduplicated. We achieve this by storing all rows in a
deduplicating container and inserting in that container first,
detecting if the row is a duplicate.

Fixes cockroachdb#46642.

Release note (sql change): The WITH RECURSIVE variant that uses UNION
(as opposed to UNION ALL) is now supported.
craig bot pushed a commit that referenced this issue Oct 21, 2021
@RaduBerinde @cameronnunez @bananabrick
Merge #71685 #71774 #71823
7265486 
71685: sql: implement WITH RECURSIVE with UNION r=RaduBerinde a=RaduBerinde

#### sql: implement WITH RECURSIVE with UNION

This change implements the UNION variant of WITH RECURSIVE, where rows
are deduplicated. We achieve this by storing all rows in a
deduplicating container and inserting in that container first,
detecting if the row is a duplicate.

Fixes #46642.

Release note (sql change): The WITH RECURSIVE variant that uses UNION
(as opposed to UNION ALL) is now supported.

#### sql: capitalize rowContainerHelper API

This change capitalizes the API of `rowContainerHelper` and
`rowContainerIterator` to make it more clear what functions are not
internal to the structure.

Release note: None

71774: roachtest: improve `rapid-restart` by using `c.Start` r=erikgrinaker a=cameronnunez

Fixes #70559.

This test manually started CRDB with `c.Run`. This patch uses `c.Start` instead of `c.Run`, avoiding
incorrect test failures from unexpected SSH exit codes resulting from using `c.Run`.

Release note: None

71823: vendor: bump Pebble to 7fec828fc1af r=bananabrick a=bananabrick

7fec828f db,sstable: block property collectors and filters
f2339cc7 compaction: read compaction fixes
e95e7374 pebble: add additional mechanism to schedule read compactions
cdbcfe9f db: Expose logic to calculate the table cache size
0ee4290c db: schedule sstable validation on ingestion
543bc6fa db: use locked suffix for function names; simplify boolean expression
5393ca16 internal/rate: skip two flaky tests on darwin
7cd88488 sstable: print additional fields in verbose block layout
4f6402b1 *: enable staticcheck

Release note:

Co-authored-by: Radu Berinde <radu@cockroachlabs.com>
Co-authored-by: Cameron Nunez <cameron@cockroachlabs.com>
Co-authored-by: Arjun Nair <nair@cockroachlabs.com>
@craig craig bot closed this as completed in e030c45 Oct 21, 2021
irfansharif pushed a commit to irfansharif/cockroach that referenced this issue Oct 26, 2021
@RaduBerinde @irfansharif
sql: implement WITH RECURSIVE with UNION
91c3a7c 
This change implements the UNION variant of WITH RECURSIVE, where rows
are deduplicated. We achieve this by storing all rows in a
deduplicating container and inserting in that container first,
detecting if the row is a duplicate.

Fixes cockroachdb#46642.

Release note (sql change): The WITH RECURSIVE variant that uses UNION
(as opposed to UNION ALL) is now supported.
@mgartner mgartner moved this to Done in SQL Queries Jul 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@RaduBerinde RaduBerinde

Labels
A-sql-pgcompat Semantic compatibility with PostgreSQL A-tools-postgrest T-sql-queries SQL Queries Team
Projects
SQL Queries
Archived in project
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

sql: implement WITH RECURSIVE with UNION RaduBerinde/cockroach
4 participants
@jlinder @rafiss @nvanbenschoten @RaduBerinde