[CALCITE-5615] Program to run SQL Logic Tests for Calcite

[mihaibudiu]

This PR essentially adds more than 7 million SQL tests to Calcite, from the SQL Logic Test Suite built by sqlite.

This addresses https://issues.apache.org/jira/browse/CALCITE-5615

This is a larger PR but it is for a completely self-contained program with a main entry point.
The README.md contained in this PR documents the program structure and its intended usage.
Using this program will require users to install separately the tests from the SQL Logic Test Suite: https://www.sqlite.org/sqllogictest/doc/trunk/about.wiki

The testing program finds multiple bugs in the default configuration of the Calcite compiler, both crashes and incorrect outputs. The plan is to file separate issues for each of these bugs as they are found.
However, this program is also intended to be extended to test other Calcite-based compilers (or it could be used for other SQL query engines; there is nothing special about Calcite in the implementation.)

[sonarqubecloud]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
36 Code Smells

0.0% Coverage
0.0% Duplication

[zabetak]

Quick pass leaving some comments along the way while trying to grasp what the contribution is about. I plan to leave also some questions comments under the JIRA ticket.

[zabetak]

It's confusing to have javadoc and non-javadoc comment coming one after the other. Please combine them together and use proper javadoc annotations to ensure it displays properly.

[mihaibudiu]

I will only address @zabetak 's comments once we understand how this should be packaged; I have sent an email to the dev mailing list asking for suggestions. One thing I would encourage people to do in the meantime is to actually run the code (first on one or two test files, these are specified on the command-line, e.g., select1.test) and look at the bugs found.

[zabetak]

@mbudiu-vmw Sure it makes sense to wait a bit. I will actually run the code during the weekend; I am already convinced that it is a very useful contribution :) I am just not sure yet where/how it should land. Thanks again for taking the time to contribute this to the community.

[julianhyde]

I think this code could go into the existing plus module, not create a new slt module. Calcite has too many modules already.

[mihaibudiu]

@zabetak thank you for the comments, I will address them once sql-logic-test 2.0 is available in my next PR.
But I would appreciate help to decide how to package this tool.

[zabetak]

But I would appreciate help to decide how to package this tool.

@mihaibudiu From my perspective (and Julian's I think) this PR should contain something equivalent to what is now TestCalcite inside the plus module and allow all SQL logic tests that pass to run. It should probably contain CalciteExecutor but nothing more. We just want to run to be able to run the tests as part of the Calcite suite but nothing more for now. If you want I can push the relevant commits following this idea in this PR.

[mihaibudiu]

With the latest commit this is testing the CalciteExecutor against the candidate 0.2 release of sql-logic-test.

[sonarqubecloud]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
No Duplication information

[julianhyde]

Rather than checking that the number of failures doesn't change, it's better to check that there are no failures other than the list of 'expected failures'

You don't want the situation where you fix one test and introduce a new failure. It's much better to have a list of known failures rather than just a count.

[mihaibudiu]

Currently the "execute" method from Main only returns an integer and writes all diagnostic to the supplied streams.
We have to make it return the actual TestStatistics object.
So this will require another release.

[mihaibudiu]

It took about 6 minutes to run 10K tests using CalciteExecutor, so by extrapolation it would take about 2 days to run all tests.
Memory consumption increases slightly with time. The failures are saved in a list which keeps growing. I am not sure whether there could be leaks somewhere else.

[zabetak]

It took about 6 minutes to run 10K tests using CalciteExecutor, so by extrapolation it would take about 2 days to run all tests.

Here are some thoughts regarding duration and frequency:

• Tests runtime ~5 minutes -> could run on every PR;
• Tests runtime between 1h and 3h -> could possibly run once daily;
• Tests runtime under 6h -> could run once per week;
• Tests runtime spanning days -> per request/ not automated (maybe before release)
  It's up to us I guess to divide them into meaningful buckets (e..g, those that are likely to break more often should run on every PR) and setup the relevant CI infra.

Obviously we don't have to setup everything as part of this PR but it would be nice if we could at least put the 5minute bucket in place.

Memory consumption increases slightly with time.

I guess we can find ways to address memory pressure; some quick ideas:

• Run each test file separately; this in principle should reset the memory after each run.
• Provide options/APIs to save failures in files (instead of keeping ever growing in memory structures)

[mihaibudiu]

I think having many small tests is probably better. There are 622 test files in the project.
We could write a small script to generate a test for each one (or a group of files).
This could also be beneficial if the tests can run in parallel.

However, JUnit5 has a capability called DynamicTests using the @TestFactory annotation, but Calcite does not currently use it: https://www.baeldung.com/junit5-dynamic-tests.
I will try to prototype this solution to see what happens. With DynamicTests we could even choose how many tests to run by sampling the test file list.

[sonarqubecloud]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
No Duplication information

[zabetak]

LGTM, I am doing some final touches locally and will merge this to master in the next few days.

[mihaibudiu]

We could also regenerate the golden file to account for the fixes made in the meantime. But I am on vacation for the next week so I can't do it right away.

[zabetak]

@mihaibudiu I am now testing everything along with my changes here: https://github.com/zabetak/calcite/tree/slt

If run comes back green I will merge it and we can work on follow-ups after that.