Skip to content

Refactor Parenthesed SelectBody and FromItem #1754

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

Merged
merged 59 commits into from
Apr 27, 2023

Conversation

manticore-projects
Copy link
Contributor

@manticore-projects manticore-projects commented Mar 23, 2023

This is a heavy API breaking refactoring on the bracket-handling related to SelectBody, FromItem and OperationSet, SubSelect, WithItem:

It should warrant a major Version JSQLParser-5.0 and also people should properly test it before.

Goal:

  • get rid of all the "hasBrackets" properties for any entity
  • move JoinList from PlainSelect to FromItem (since there can't be a Join without a From)
  • SelectBody replaces SubSelect and implements FromItem directly
    This has the big advantaged, that any occurrence of ( table=Table() | subSelect=SubSelect() ) can be parsed as fromItem=FromItem() without LOOKAHEAD (for example Merge parses a simple FromItem now)
  • SelectBody implements Statement directly and so becomes the Select Statement (removing 1 hierarchy level from the AST):
-- before
statement = new Select().withSelectBody(new PlainSelect()
                        .addSelectItems(new AllTableColumns().withTable(t)).withFromItem(t));
statement = new Select().withSelectBody(new ValuesStatement().withExpressions(expressionList));

-- now better ( when SelectBody became Select, all its implementations became a Statement )
statement = new PlainSelect()
                        .addSelectItems(new AllTableColumns().withTable(t)).withFromItem(t);
statement = new Values().withExpressions(expressionList);
  • LateralSubSelect extends ParenthesedSelectBody (axing SubSelect and SpecialSubSelect )

  • values.ValuesStatement becomes select.Values implementing SelectBody (with Statement and FromItem). So we got rid of ValuesList

  • move ORDER BY, LIMIT, OFFSET, FETCH, WITH isolation into SelectBody since all its implementations support it

    -- valid SQL
    values(1,2,3) limit 1
    -- valid SQL
    (values(1,2,3)) limit 1
  • Nested WithItems

    with a 
        as ( with b 
               as ( with c 
                      as (select 1) 
                      select c.* from c ) 
               select b.* from b ) 
         select a.* from a
  • Merge implements the Visitor Pattern

  • all the Tests succeed

  • verify/validate performance. It is a little bit slower but much more correct.

Before:
42500 statements parsed in 13095 milliseconds
 (3245 statements per second,  0.0003082 seconds per statement )
38500 select scans for table name executed in 80 milliseconds
 (481250 select scans for table name per second,  0.0000021 seconds per select scans for table name)

New:
42500 statements parsed in 15062 milliseconds
 (2821 statements per second,  0.0003545 seconds per statement )
38500 select scans for table name executed in 101 milliseconds
 (381188 select scans for table name per second,  0.0000026 seconds per select scans for table name)
  • so far 900 lines less code at better functionality and simplicity (traversing the AST got much easier)

Fixes #1737
Fixes #1764
Succeeds on another Special Oracle Test Condition15
Succeeds on another Special Oracle Test Cast_MultiSet07

@wumpz Please start reviewing and provide feedback as early as possible.

zaza and others added 30 commits December 11, 2022 21:03
…RESH

Support parsing create view statements in Redshift with AUTO REFRESH
option.
Extract adding the force option into a dedicated method resulting in the
cyclomatic complexity reduction of the CreateView.toString method.
Add Keywords and document, which keywords are allowed for what purpose
Derive All Keywords from Grammar directly
Generate production for Object Names (semi-) automatically
Add parametrized Keyword Tests
Parallel Test execution
Gradle Caching
Explicitly request for latest JavaCC 7.0.10
Parallel Test execution
Gradle Caching
Explicitly request for latest JavaCC 7.0.10
Update the MANTICORE Sphinx Theme, but ignore it in GIT
Add the content to the Sphinx sites
Add a Gradle function to derive Stable and Snapshot version from GIT Tags
Add a Gradle GIT change task
Add a Gradle sphinx task
Add a special Test case for illustrating the use of JSQLParser
Implement Serializable for persisting via ObjectOutputStream
- apply neutral Sphinx theme
- insert the RR diagrams into the sphinx sources
- better documentation on Gradle dependencies
- link GitHub repository
- add support for Oracle Alternative Quoting e.g. `q'(...)'`
- fixes JSQLParser#1718
- add a Logo and FavIcon to the Website
- document recent changes on Quoting/Escaping
- add an example on building SQL from Java
- rework the README.md, promote the Website
- add Spotless Formatter, using Google Java Style (with Tab=4 Spaces)
- fix the issue template
- fix the -SNAPSHOT version number
- `GO`
- Slash `/`
- Two empty lines
- `FETCH` uses `EXPRESSION` instead of SimpleJDBCParameter only
- Visit/Accept `FETCH` `EXPRESSION` instead of `append` to String
- Visit/Accept `OFFSET` `EXPRESSION` instead of `append` to String
- Gradle: remove obsolete/incompatible `jvmArgs` from Test()
@manticore-projects manticore-projects marked this pull request as draft March 23, 2023 01:22
@manticore-projects manticore-projects self-assigned this Mar 23, 2023
- fix `NULLS FIRST` and `NULLS LAST`
- parse `SetOperation` only after a (first plain) SelectBody has found, this fixes the performance issue
- one more special Oracle Test succeeds
- 5 remaining test failures
- extract `OrderByElements` into `SelectBody`
- one more special Oracle Test succeeds
- all tests succeed
@manticore-projects manticore-projects marked this pull request as ready for review March 25, 2023 04:07
- `SelectBody` implements `FromItem`
- get rid of `SubSelect` and `SpecialSubSelect`
- `Merge` can use `FromItem` instead of `SubSelect` or `Table`
- `LateralSubSelect` extends `ParenthesedSelectBody` directly
- Simplify the `Select` statement, although it is still redundant since     `SelectBody` also could implement `Statement` directly
- `WithItem` can use `SelectBody` directly, which allows for nested `WithItems`

BREAKING-CHANGE: Lots of redundant methods and intermediate removed
- `SelectBody` implements `Statement` and so makes `Select` redundant
- get rid of `ValuesList`
- refactor `ValuesStatement` into `Values` which just implements `SelectBody` (and becomes a `Statement` and a `FromItem`), move to `select` package

BREAKING-CHANGE: Lots of redundant methods and intermediate removed
- remove 3 unused/obsolete productions
- appease PMD/Codacy
- former `SelectBody` implements `Statement` and so becomes `Select`
- this reduces the AST by 1 hierarchy level
@manticore-projects
Copy link
Contributor Author

Can we move this forward please since it is quite a big chunk?

@wumpz wumpz merged commit a312dcd into JSQLParser:master Apr 27, 2023
@manticore-projects manticore-projects deleted the ParenthesedSelectBody branch April 29, 2023 10:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants