Skip to content

feature/fix: parsing inserts/updates/delete within CTEs #2055

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
Merged
merged 5 commits into from
Aug 4, 2024
Merged

feature/fix: parsing inserts/updates/delete within CTEs #2055

merged 5 commits into from
Aug 4, 2024

Conversation

@nicky6s
Copy link
Contributor

@nicky6s nicky6s commented Aug 4, 2024

Problem
In Postgres it is valid syntax to have insert, update or delete statements appear within CTEs.

See here on https://www.postgresql.org/docs/current/sql-select.html

with_query_name [ ( column_name [, ...] ) ] AS [ [ NOT ] MATERIALIZED ] ( select | values | insert | update | delete )

e.g.

WITH inserted AS (
   INSERT INTO x (foo)
   SELECT bar FROM b
   RETURNING y
)
SELECT y
  FROM inserted;

The current version of the parser fails when faced with these statements as only ParenthesedSelects are allowed to appear within CTEs

Solution
Change the WithItem class so that...

  • WithItem no longer extends ParenthesedSelect
  • Instead WithItem has type T where T extends the new interface ParenthesedStatement
  • ParenthesedSelect implements ParenthesedStatement
  • New classes ParenthesedInsert + ParenthesedUpdate + ParenthesedDelete are also implementations ParenthesedStatement

Notes
I could definitely do with some feedback on the approach Ive taken here.

I've done my best to avoid impacting any existing functionality. The method getWithItemsList in various classes now returns a List<WithItem<?>> object rather than a List<WithItem> object. I think this should be the most "disruptive" thing I've included.

@manticore-projects
Copy link
Contributor

manticore-projects commented Aug 4, 2024

Impressive contribution, thank you already for your time and effort!
Please give me a time to digest and appreciate everything properly. We will definitely merge this eventually!

@manticore-projects
Copy link
Contributor

manticore-projects commented Aug 4, 2024

Clean, I really do like your work! Thank you again.
Please do me 2 favors though:

  1. fix the Codacy exceptions
  2. for the 2 failing Gradle performance tests, please reduce the number of loops from 10 to 6 (temporarily, there is a pending PR which fixes this properly)

@manticore-projects
Copy link
Contributor

manticore-projects commented Aug 4, 2024

Please fix the formatting, then I can merge promptly.
Run './gradlew :spotlessApply' to fix these violations.

@manticore-projects manticore-projects merged commit 82470e5 into JSQLParser:master Aug 4, 2024
@manticore-projects
Copy link
Contributor

manticore-projects commented Aug 4, 2024

Thank you again!

@nicky6s
Copy link
Contributor Author

nicky6s commented Aug 4, 2024

@manticore-projects I think Ive made the changes you've asked for...

fix the Codacy exceptions

85d114b

for the 2 failing Gradle performance tests, please reduce the number of loops from 10 to 6 (temporarily, there is a pending PR which fixes this properly)

1de2a39

Please fix the formatting

3f60913

However, Im still getting a Gradle test failure...

NestedBracketsPerformanceTest > testDeepFunctionParameters() FAILED
    net.sf.jsqlparser.JSQLParserException at NestedBracketsPerformanceTest.java:357
        Caused by: java.util.concurrent.TimeoutException at NestedBracketsPerformanceTest.java:357

3177 tests completed, 1 failed, 18 skipped

It's strange because when I run NestedBracketsPerformanceTest locally it all runs fine
image

@nicky6s nicky6s deleted the dmls-within-ctes branch August 4, 2024 16:46
@manticore-projects
Copy link
Contributor

manticore-projects commented Aug 4, 2024

However, Im still getting a Gradle test failure...

No worries, I check/fix this from here.

@nicky6s
Copy link
Contributor Author

nicky6s commented Aug 4, 2024

@manticore-projects Thanks a lot for your help 🙇

@nicky6s nicky6s mentioned this pull request Aug 17, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@nicky6s @manticore-projects