build: Add better SQL Test Infrastructure #31

manticore-projects · 2022-12-26T11:32:54Z

Switch to JUnit5 for parametrized tests and parallel execution
Maven: Add Airlist Check SKIP switches for fast prototyping
~~Maven: remove Checklist Modules, since those are provided by Airbase~~ turns out, those are needed r/ Java 8
Eclipse formatter configuration for Airlift style
SQL Parser and Unparser Tests, read from SQL Files
Updated some Tests to benefit from JUnit5
Sphinx Website for deployment Github Pages
Readme updated
Added Gradle Build support
Updated Airlift/Base to io.airlift ~~219~~ 100 (Last Java 8 supporting version)
Provide explicit Airlift Checkstyle config
Move the JavaCC Grammar to a more appropriate place
Generate RailRoad Diagrams for the Grammar
Downgrade Maven JUnit5 as requested by Airlift
Add an ASCII Tree for visualizing the AST

- Switch to JUnit5 for parametrized tests and parallel execution - Maven: Add Airlist Check SKIP switches for fast prototyping - Maven: remove Checklist Modules, since those are provided by Airbase - Eclipse formatter configuration for Airlift style - SQL Parser and Unparser Tests, read from SQL Files - Updated some Tests to benefit from JUnit5

linux-foundation-easycla · 2022-12-26T11:32:58Z

The committers listed above are authorized under a signed CLA.

✅ login: manticore-projects (4a5d898)

Maven Spotless Plugin to enforce Airlift compliant formatting based on Eclipse formatter

Add basic Sphinx Template with minimal content Add Gradle task for Changelog Add Gradle task for Rail Road Diagrams (although it does not work yet because this Grammar is strong) Add Gradle task for building the Sphinx documentation, including the Prologue Add Github Action for deploying the HTML to Github Pages Add Github Action for Maven build Add Github Issue template Tweak the .gitignore file

manticore-projects · 2022-12-26T16:51:00Z

Greetings.

I have worked a bit around the infrastructure. Added some Readme, Website/Github Pages and Test SQLs (so to read the statements from a formatted SQL file instead from Java Code).

I also wanted to add Rail-Road Diagrams, but this did not work yet since the Grammar uses some features I have never seen before. Bug reported GuntherRademacher/rr#20

Added some support for formatting.

Please do let me know what you like or don't like.
At the moment some of the links may be wrong. I can fix that only when I know what you will accept and what the final location is.

Previews:
https://manticore-projects.github.io/sql/
https://github.com/manticore-projects/sql/tree/test_infrastructure

Cheers

parser/src/test/java/com/facebook/coresql/parser/TestSqlParser.java

rewriter/src/test/java/com/facebook/coresql/rewriter/TestOrderByRewriter.java

kaikalur · 2022-12-26T17:07:43Z

sphinx/src/keywords.rst

+Restricted Keywords
+***********************
+
+The following Keywords are **restricted** in Version |JSQLPARSER_VERSION| and must not be used for **Naming Objects**: 


Why are we having references to JSQL here? I thought JSQL would simply use this package

Thanks for feedback.

This particular reference is only a variable name used in a template. It will be replaced/filled with the actual version.

Before changing it, we will want to agree on a proper Brand Name for this software and the decision is all yours. Will it be Presto SQL Parser, since it is linked to Presto?

Or should it aim broader, like `JSQL:2016', 'JavaSQL16', 'ParseSQL16', 'JSQLSuite', 'JSQLTools'?

Since we are on it, the Git Repo checks out as sql (which immediately collided with my curated collection of the worst SQL statements I have seen in my life.)

Please make a choice and I would adjust everything accordingly.

Hmm this is a tricky one. Definitely let's not call it jsql especially since we have C++ version (which your changes will hopefully not impact). Presto SQL parser might be most appropriate

So be it! I would adjust this tomorrow, also remove any manticore references and submit.
Thanks for the feedback!

I did not touch any C++ Version.

Although: I would like to have a comprehensive Test/CI Suite, which can warn us. If it succeeds, we should be good. When not, then reject.

Update the Line Wrapping Options for Function and Array Parameters. Tweak the Eclipse Formatter Configuration to match the Airlift Checkstyle rules precisely. Now, calling `mvn spotless:apply` will guarantee compliance with Airlift style.

Add plenty of SQL samples Separate succeeding samples from failing samples so that failures will not block the build Let the tests show the Statement and the Parser Error Further tune the Eclipse Formatter since Airlift is really strict Simplify some Regular Expressions

manticore-projects · 2022-12-27T13:05:36Z

I would like to apologize upfront for the massive change-set.
However, I feel like we achieve something:

We have now a kind of SQL-File Test Cases, which DO PASS and should always pass -- whatever we do. mvn package would fail when those break.
We have an additional set of FAIL SAFE SQL-Files, which illustrate missing features of the parser. Those will show only with mvn integration-test.

Now we can pick from here one by one, analyze what goes wrong and decide if we want to support it or not.

Any new success should be moved to the mandatory test suite.
I like the fact, that the Tests can be provided as properly formatted SQL and so are easy to read.

All tests are parseable with JSQLParser and have been formatted with JSQLFormatter.
I would like to suggest that as a first realistic goal (minus a few things like old Oracle joins or multiple column drop statements).

manticore-projects · 2022-12-27T13:07:04Z

Illustration of the Test Suite:

Added Gradle Build support Updated Airlift/Base to io.airlift 219 Provide explicit Airlift Checkstyle config Move the JavaCC Grammar to a more appropriate place Generate RailRoad Diagrams for the Grammar Downgrade Maven JUnit5 as requested by Airlift Add an ASCII Tree for visualizing the AST

Signed-off-by: Andreas Reichel <[email protected]>

kaikalur · 2023-01-03T15:42:04Z

It maybe ok for now to have this large PR as it's just adding a bunch of new things and doesn't seem to change the grammar. But in general we prefer separate small PRs .

Also we generally squash all commits into one before merging but looks like you have many different things so it maybe ok. Also looks like this repo is also under Linux Foundation. We need to check if it's ok to have manticore as a package under sql. I suggest creating a sql-manticore toplevel package so the grammar module is clean with just the grammar. So we should eventually have parser, linter, rewriter and formatter as the toplevel packages.

manticore-projects · 2023-01-03T15:45:48Z

Happy new year 2023.

I know, that this PR is way too large -- however, it is really hard to work on Build/Test infrastructure and Documentation and keep it small, when the PR is kept pending for a long time.

I can remove any "manticore" related reference in the packages of course, it was just copy'n paste.

Btw, Rail Road Diagrams: https://manticore-projects.github.io/sql/syntax.html

kaikalur · 2023-01-03T15:50:18Z

It's fine to have manticore references but keep that in a separate package. I just want to keep the parser package super clean with no specific reference to anything else. Just like presto has a lot of connectors that all sit in presto-xxx packages.

kaikalur · 2023-01-03T15:50:41Z

Btw, Rail Road Diagrams: https://manticore-projects.github.io/sql/syntax.html

The railroad diagram is amazing!

kaikalur · 2023-01-03T16:07:33Z

So just two changes needed to merge this PR: 1. fix the naming to 'Presto SQL Parser' and 2. move the formatter out to a separate package.

manticore-projects · 2023-01-03T16:08:59Z

So just two changes needed to merge this PR: 1. fix the naming to 'Presto SQL Parser' and 2. move the formatter out to a separate package.

Agreed. You will have it tomorrow. Thanks again for the guidance.

Signed-off-by: Andreas Reichel <[email protected]>

manticore-projects · 2023-01-04T11:05:33Z

Greetings.

I have removed all references to "manticore-projects" and/or "JSQLParser" and have applied "Presto SQL:2016 Parser" where possible.
I have also restored full Java 8 compatibility (which airlift silently dropped with version 100).
Gradle Build for the Parser module is fully function and I will stick with that one in the future.

Github Actions run CI and publish site.
Cheers!

Signed-off-by: Andreas Reichel <[email protected]>

sphinx/src/contribution.rst

Signed-off-by: Andreas Reichel <[email protected]>

kaikalur

LGTM

kaikalur · 2023-01-04T16:16:25Z

Thank you very much for doing this much needed setup cleanup!

kaikalur · 2023-01-04T16:19:26Z

@mbasmanova do you think you can stamp this PR - it's simply adding a bunch of test/ci stuff with no changes to the grammar/ast side of things? I need to find some others who can do this now so that @manticore-projects and I can work on it and merge without delays.

Signed-off-by: Andreas Reichel <[email protected]>

fixes #1706

manticore-projects marked this pull request as draft December 26, 2022 11:33

manticore-projects added 5 commits December 26, 2022 19:12

build: Add Spotless Plugin

075fa94

Maven Spotless Plugin to enforce Airlift compliant formatting based on Eclipse formatter

fix: Git Branch is MAIN

bf9ec8d

fix: Try using native GIT

da733b5

doc: Update the readme

1d7a198

manticore-projects marked this pull request as ready for review December 26, 2022 16:44

kaikalur suggested changes Dec 26, 2022

View reviewed changes

manticore-projects added 2 commits December 27, 2022 16:37

style: Tune the Eclipse Formatter Configuration

24ddd02

Update the Line Wrapping Options for Function and Array Parameters. Tweak the Eclipse Formatter Configuration to match the Airlift Checkstyle rules precisely. Now, calling `mvn spotless:apply` will guarantee compliance with Airlift style.

manticore-projects mentioned this pull request Dec 27, 2022

Is there any introduction about this repository? #21

Open

manticore-projects added 2 commits January 3, 2023 21:07

doc: changelog.rst

fa5d506

Signed-off-by: Andreas Reichel <[email protected]>

refactor: fix links and names

551671a

Signed-off-by: Andreas Reichel <[email protected]>

doc: fix 2 more name references

4ff8532

Signed-off-by: Andreas Reichel <[email protected]>

kaikalur suggested changes Jan 4, 2023

View reviewed changes

sphinx/src/contribution.rst Outdated Show resolved Hide resolved

doc: US compliant wording

1cee025

Signed-off-by: Andreas Reichel <[email protected]>

kaikalur approved these changes Jan 4, 2023

View reviewed changes

kaikalur requested a review from mbasmanova January 4, 2023 16:17

manticore-projects added 2 commits January 7, 2023 17:38

doc: better RR Diagram integration

09f73d0

Signed-off-by: Andreas Reichel <[email protected]>

doc: show Dependencies and Artefacts for both Maven and Gradle

7a920bb

kaikalur requested review from rongrong and removed request for mbasmanova and rongrong January 9, 2023 23:29

refactor: Merge REPLACE into UPSERT

ab8e63d

fixes #1706

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

build: Add better SQL Test Infrastructure #31

build: Add better SQL Test Infrastructure #31

manticore-projects commented Dec 26, 2022 •

edited

Loading

linux-foundation-easycla bot commented Dec 26, 2022 •

edited

Loading

manticore-projects commented Dec 26, 2022

kaikalur Dec 26, 2022

manticore-projects Dec 27, 2022

kaikalur Jan 3, 2023

manticore-projects Jan 3, 2023 •

edited

Loading

manticore-projects commented Dec 27, 2022

manticore-projects commented Dec 27, 2022

kaikalur commented Jan 3, 2023

manticore-projects commented Jan 3, 2023

kaikalur commented Jan 3, 2023 •

edited

Loading

kaikalur commented Jan 3, 2023

kaikalur commented Jan 3, 2023

manticore-projects commented Jan 3, 2023

manticore-projects commented Jan 4, 2023

kaikalur left a comment

kaikalur commented Jan 4, 2023

kaikalur commented Jan 4, 2023

build: Add better SQL Test Infrastructure #31

Are you sure you want to change the base?

build: Add better SQL Test Infrastructure #31

Conversation

manticore-projects commented Dec 26, 2022 • edited Loading

linux-foundation-easycla bot commented Dec 26, 2022 • edited Loading

manticore-projects commented Dec 26, 2022

kaikalur Dec 26, 2022

Choose a reason for hiding this comment

manticore-projects Dec 27, 2022

Choose a reason for hiding this comment

kaikalur Jan 3, 2023

Choose a reason for hiding this comment

manticore-projects Jan 3, 2023 • edited Loading

Choose a reason for hiding this comment

manticore-projects commented Dec 27, 2022

manticore-projects commented Dec 27, 2022

kaikalur commented Jan 3, 2023

manticore-projects commented Jan 3, 2023

kaikalur commented Jan 3, 2023 • edited Loading

kaikalur commented Jan 3, 2023

kaikalur commented Jan 3, 2023

manticore-projects commented Jan 3, 2023

manticore-projects commented Jan 4, 2023

kaikalur left a comment

Choose a reason for hiding this comment

kaikalur commented Jan 4, 2023

kaikalur commented Jan 4, 2023

manticore-projects commented Dec 26, 2022 •

edited

Loading

linux-foundation-easycla bot commented Dec 26, 2022 •

edited

Loading

manticore-projects Jan 3, 2023 •

edited

Loading

kaikalur commented Jan 3, 2023 •

edited

Loading