GeezerVison

The World Wide Web, restored to 800 pixels of glory.

GeezerVison is showing github.com — feature/fix: parsing inserts/updates/delete within CTEs by nicky6s · Pull Request #2055 · JSQLParser/JSqlParser

Conversation

nicky6s commented Aug 4, 2024

Copy link

Copy Markdown

Contributor

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

manticore-projects reacted with heart emoji

All reactions

❤️ 1 reaction

nicky6s added 2 commits

August 4, 2024 15:54


          feature parsing inserts/updates/delete within CTEs

a8611c8


          removing System lines

a5a73fd

manticore-projects commented Aug 4, 2024

Copy link

Copy Markdown

Contributor

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!

nicky6s reacted with hooray emoji

All reactions

🎉 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

manticore-projects commented Aug 4, 2024

Copy link

Copy Markdown

Contributor

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

fix the Codacy exceptions
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)

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

nicky6s added 2 commits

August 4, 2024 17:01


          fixing codacy issues

85d114b


          reducing the looping in NestedBracketsPerformanceTest to just 6

1de2a39

manticore-projects commented Aug 4, 2024

Copy link

Copy Markdown

Contributor

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

nicky6s reacted with thumbs up emoji

All reactions

👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.


          formatting fixes via spotlessApply

3f60913

manticore-projects merged commit 82470e5 into JSQLParser:master

Aug 4, 2024

manticore-projects commented Aug 4, 2024

Copy link

Copy Markdown

Contributor

Thank you again!

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

nicky6s commented Aug 4, 2024

Copy link

Copy Markdown

Contributor Author

@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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

nicky6s deleted the dmls-within-ctes branch

August 4, 2024 16:46

manticore-projects commented Aug 4, 2024

Copy link

Copy Markdown

Contributor

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

No worries, I check/fix this from here.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

nicky6s commented Aug 4, 2024

Copy link

Copy Markdown

Contributor Author

@manticore-projects Thanks a lot for your help 🙇

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

nicky6s mentioned this pull request

Aug 17, 2024

chore removing system.out.println lines + minor clean up of unit test scripts #2060

Merged

Reviewers

No reviews

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

2 participants

Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

You are visitor 01402520 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.