GeezerVison

@ssteinhauser:

Thank you for your contribution, I will look into it over the weekend.
Though I have a question regarding your JMH: can you please share the specs of your machine?

a) your tests look super slow in general (I am recording only 80 ms on a cheap laptop)
b) your tests record a huge improvement over 5.3 (which I could not explain)
c) your tests have very high variances

Not a real concern or show stopper, I am just curious!

@ssteinhauser:

Thank you for your contribution, I will look into it over the weekend. Though I have a question regarding your JMH: can you please share the specs of your machine?

a) your tests look super slow in general (I am recording only 80 ms on a cheap laptop) b) your tests record a huge improvement over 5.3 (which I could not explain) c) your tests have very high variances

Not a real concern or show stopper, I am just curious!

Yes, the benchmark results are super high. Probably because I had quite a few other tools and tasks running consuming lots of CPU and memory. However, my specifications are 32GB of RAM and an Intel Core i7-1185G7 (3.00GHz) CPU.

I've just reexecuted the benchmark (inside WSL) while having lower load on the machine, but the results are still a bit high although the variances are lower:

Benchmark                               (version)  Mode  Cnt    Score    Error  Units
JSQLParserBenchmark.parseSQLStatements     latest  avgt   15  140.343 ± 26.198  ms/op
JSQLParserBenchmark.parseSQLStatements        5.3  avgt   15  103.779 ± 20.458  ms/op
JSQLParserBenchmark.parseSQLStatements        5.1  avgt   15  119.490 ± 27.174  ms/op

I have benchmarked your branch on my laptop and get more reasonable results:

Benchmark                               (version)  Mode  Cnt   Score   Error  Units
JSQLParserBenchmark.parseSQLStatements     latest  avgt   15  85.965 ± 2.753  ms/op
JSQLParserBenchmark.parseSQLStatements        5.3  avgt   15  81.911 ± 3.433  ms/op
JSQLParserBenchmark.parseSQLStatements        5.1  avgt   15  82.380 ± 2.856  ms/op

Although there is a deterioration in performance I think we can accept that.
Question: Could we introduce a Feature switch to avoid parsing any of your statements, unless explicitly requested for?

Hi @manticore-projects ,
thanks for your tests and feedback as well.
Sure we can add a feature flag. Which kind of flag are you thinking of? One flag for IMPORT/EXPORT or something else? Should its default be false or true?

Good Morning Stefan,

the decision is all yours because you understand your implementation best.
My take is:

• you added lots of Exasol specific Grammar (kudos!)
• this Grammar slows down the Parser somewhat (expected and justified)
• this Grammar does not serve most of the users (unless they want Exasol)

Best way would be to have RDBMS specific parsers and I will look for ways to achieve that soon.
In the meantime, I wonder if we can introduce Features like 'DIALECT_EXASOL' or 'RDBMS_EXASOL' and then wrap all/most of your extensions into semantic lookaheads like:

LOOKAHEAD( { "DIALECT_EXASOL".equals(Features.get("dialect")) } ) production = ExasolProduction()

(This is an abstract illustration only, of course.)

My hope and goal was to reduce the performance penalty for common RDMS and use-cases to a minimum.
And to have a template for similar exotic dialects like apache hive etc.

Example: line 4663/4664

 LOOKAHEAD(2) fromItem=SubImport() { fromItem = new ParenthesedFromItem(fromItem); }     

could be changed into:

 LOOKAHEAD(2,  { "DIALECT_EXASOL".equals(Features.get("dialect")) } ) fromItem=SubImport() { fromItem = new ParenthesedFromItem(fromItem); }     

You have introduced new tokens RTRIM and LTRIM, please double-check that those won't collide with parsing the normal functions.

Beside that we are good to merge!

Greetings!

I have added already a new Feature for dialect which you can set to Dialect.EXASOL.
This feature can be used now in your lookaheads.

Greetings!

I have added already a new Feature for dialect which you can set to Dialect.EXASOL. This feature can be used now in your lookaheads.

Thank you very much for your support and effort on this. I've implemented the feature flag for EXPORT and IMPORT statements including the SubImport().

Furthermore, I've added tests for RTRIM and LTRIM functions in order to ensure, there is no clash between the keywords and usage of functions (see 3b74289).

If you don't have any further suggestions or concerns, we are good to merge from my point of view.

Thank you very much, excellent work and I am going to merge this.

Interestingly, the performance deterioration is all about the addition tokens and the white listing of such tokens for Object Identifiers. Your productions have no impact. So in the future I will need to look for a way to reduce those tokens again (especially any token used less than 3 times in the grammar).

Also, the Dialect feature needs some syntax sugar -- but that will be on me.