feat: Add set operation (select ... union select ...) for insert stat… #200

antoineB · 2023-10-08T15:24:04Z

…ements.

feat: Add comment on FUNCTION and EXTENSION. feat: Add SET and RESET statement.

… postgres syntax. The empty list or arguments also appear in the CST.

…UENCE.

dmfay

something interesting's happening here: state count is much higher in this branch (7776 vs 4536 on main), which I think is largely down to the addition of function argument modes -- invocation punches way above its weight class (big increases also in binary_expression, between_expression, implicit_cast, and a bunch of types like double and tinyint -- maybe due to the time additions?). Large state count has dropped by a lot, though, 775 in main to 333 here, and build time from an empty src/ is actually around ten seconds faster on my machine. Do you know what's happening here?

also if you have more changes in the mean time please create separate pull requests! this one's getting unwieldy

grammar.js

dmfay · 2023-10-12T01:14:58Z

grammar.js

@@ -551,8 +558,21 @@ module.exports = grammar({
    nchar: $ => parametric_type($, $.keyword_nchar),
    nvarchar: $ => parametric_type($, $.keyword_nvarchar),

+    _time_zone: $ => seq(


Suggested change

_time_zone: $ => seq(

_include_time_zone: $ => seq(

this is clearer imo, but I definitely like the more detailed deconstruction of timestamps overall!

Fine by me.

antoineB · 2023-10-12T09:30:37Z

How do you get the number of parser states ?

matthias-Q · 2023-10-12T10:07:13Z

The state count is shown in the generated parser.c

dmfay · 2023-10-12T12:39:53Z

yeah, head -18 src/parser.c shows you all the potentially useful counts.

antoineB · 2023-10-12T17:38:15Z

Here is all the numbers

f75b8ea
1,4M target/parser.so
#define LANGUAGE_VERSION 14
#define STATE_COUNT 7637
#define LARGE_STATE_COUNT 328
#define SYMBOL_COUNT 702
#define ALIAS_COUNT 0
#define TOKEN_COUNT 407
#define EXTERNAL_TOKEN_COUNT 0
#define FIELD_COUNT 42
#define MAX_ALIAS_SEQUENCE_LENGTH 16
#define PRODUCTION_ID_COUNT 197

531d243
1,4M target/parser.so
#define LANGUAGE_VERSION 14
#define STATE_COUNT 7528
#define LARGE_STATE_COUNT 323
#define SYMBOL_COUNT 702
#define ALIAS_COUNT 0
#define TOKEN_COUNT 407
#define EXTERNAL_TOKEN_COUNT 0
#define FIELD_COUNT 42
#define MAX_ALIAS_SEQUENCE_LENGTH 16
#define PRODUCTION_ID_COUNT 190

bec206c
1,4M target/parser.so
#define LANGUAGE_VERSION 14
#define STATE_COUNT 7362
#define LARGE_STATE_COUNT 317
#define SYMBOL_COUNT 698
#define ALIAS_COUNT 0
#define TOKEN_COUNT 406
#define EXTERNAL_TOKEN_COUNT 0
#define FIELD_COUNT 42
#define MAX_ALIAS_SEQUENCE_LENGTH 16
#define PRODUCTION_ID_COUNT 190

c223823
1,4M target/parser.so
#define LANGUAGE_VERSION 14
#define STATE_COUNT 7362
#define LARGE_STATE_COUNT 317
#define SYMBOL_COUNT 698
#define ALIAS_COUNT 0
#define TOKEN_COUNT 406
#define EXTERNAL_TOKEN_COUNT 0
#define FIELD_COUNT 42
#define MAX_ALIAS_SEQUENCE_LENGTH 16
#define PRODUCTION_ID_COUNT 190

1e3f464
1,4M target/parser.so
#define LANGUAGE_VERSION 14
#define STATE_COUNT 7362
#define LARGE_STATE_COUNT 317
#define SYMBOL_COUNT 698
#define ALIAS_COUNT 0
#define TOKEN_COUNT 406
#define EXTERNAL_TOKEN_COUNT 0
#define FIELD_COUNT 42
#define MAX_ALIAS_SEQUENCE_LENGTH 16
#define PRODUCTION_ID_COUNT 190

bde914f
1,4M target/parser.so
#define LANGUAGE_VERSION 14
#define STATE_COUNT 7362
#define LARGE_STATE_COUNT 317
#define SYMBOL_COUNT 698
#define ALIAS_COUNT 0
#define TOKEN_COUNT 406
#define EXTERNAL_TOKEN_COUNT 0
#define FIELD_COUNT 42
#define MAX_ALIAS_SEQUENCE_LENGTH 16
#define PRODUCTION_ID_COUNT 190

1a2b0da
1,4M target/parser.so
#define LANGUAGE_VERSION 14
#define STATE_COUNT 7373
#define LARGE_STATE_COUNT 322
#define SYMBOL_COUNT 696
#define ALIAS_COUNT 0
#define TOKEN_COUNT 406
#define EXTERNAL_TOKEN_COUNT 0
#define FIELD_COUNT 42
#define MAX_ALIAS_SEQUENCE_LENGTH 16
#define PRODUCTION_ID_COUNT 190

970b548
1,6M target/parser.so
#define LANGUAGE_VERSION 14
#define STATE_COUNT 4590
#define LARGE_STATE_COUNT 787
#define SYMBOL_COUNT 670
#define ALIAS_COUNT 0
#define TOKEN_COUNT 386
#define EXTERNAL_TOKEN_COUNT 0
#define FIELD_COUNT 42
#define MAX_ALIAS_SEQUENCE_LENGTH 16
#define PRODUCTION_ID_COUNT 190

385aff4 (main)
1,5M target/parser.so
#define LANGUAGE_VERSION 14
#define STATE_COUNT 4536
#define LARGE_STATE_COUNT 775
#define SYMBOL_COUNT 670
#define ALIAS_COUNT 0
#define TOKEN_COUNT 386
#define EXTERNAL_TOKEN_COUNT 0
#define FIELD_COUNT 42
#define MAX_ALIAS_SEQUENCE_LENGTH 16
#define PRODUCTION_ID_COUNT 190

matthias-Q

Hi, thanks for you PR. I have made some remarks. Would be nice If you could address them.

For future PRs, I would be nice if you could limit the scope of the PR to one topic and don't let the PR get too large. I makes it hard to review. If I am not mistaken, most of the suggested changes have nothing to do with the title of the PR either. A (very short) description of what you want to address/change/implement is also welcome.

matthias-Q · 2023-10-15T19:05:23Z

grammar.js

+    keyword_session: _ => make_keyword("session"),
+    keyword_isolation: _ => make_keyword("isolation"),
+    keyword_level: _ => make_keyword("level"),
+    keyword_serializable: _ => make_keyword("seria"),


is this called seria? I have search pg and mariadb docs and was not able to find this. Can you give me an example?

It is a typo.

matthias-Q · 2023-10-15T19:06:39Z

grammar.js

+    keyword_repeatable: _ => make_keyword("repeatable: _ ="),
+    keyword_read: _ => make_keyword("read: _ =>"),


Again, I was not able to fin this structure in PG and MariaDB. Also separating tokens in the keyword statement is not the right way. In SQL there can be multiple spaces/tabs between tokens.

Same as before a keyboard macro typo.

grammar.js

matthias-Q · 2023-10-15T19:11:36Z

grammar.js

@@ -735,6 +757,31 @@ module.exports = grammar({
      ),
    ),

+    _argmode: $ => choice(


can you add one or two test cases for this?

matthias-Q · 2023-10-15T19:12:53Z

grammar.js

@@ -904,6 +961,54 @@ module.exports = grammar({
      ),
    ),

+    reset_statement: $ => seq(


Again, can you add test cases here?

grammar.js

test/corpus/insert.txt

Add test to set/reset statement. Correct keyword typos. Add test on function arguments with default value and IN/OUT mode.

dmfay

@matthias-Q if you're alright with the current state feel free to land it, or let me know and I can later 🚀

antoineB added 9 commits October 8, 2023 17:21

feat: Add set operation (select ... union select ...) for insert stat…

970b548

…ements.

feat: Add ONLY for ALTER TABLE statement.

1a2b0da

feat: Add comment on FUNCTION and EXTENSION. feat: Add SET and RESET statement.

change: The arguments functions is changed to reflect the mariadb and…

bde914f

… postgres syntax. The empty list or arguments also appear in the CST.

fix: Missing identifier as value of a SET statement.

1e3f464

fix: Correct CREATE TYPE to accept also a schema as identifier.

c223823

fix: Add missing option 'no maxvalue' and 'no minvalue' to CREATE SEQ…

bec206c

…UENCE.

feat: Add storage parameters to table definition.

531d243

fix: Modify custom type to accept a schema prefix.

f75b8ea

fix: Make timestamp type a parametric type.

2f5d0cd

dmfay reviewed Oct 12, 2023

View reviewed changes

Change rule name _time_zone to _include_time_zone.

8421bb7

matthias-Q requested changes Oct 15, 2023

View reviewed changes

Change set statement to accept on/off as keyword and not identifier.

88ff038

Add test to set/reset statement. Correct keyword typos. Add test on function arguments with default value and IN/OUT mode.

dmfay approved these changes Oct 15, 2023

View reviewed changes

matthias-Q approved these changes Oct 16, 2023

View reviewed changes

matthias-Q merged commit 2b782fd into DerekStride:main Oct 16, 2023
4 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Add set operation (select ... union select ...) for insert stat… #200

feat: Add set operation (select ... union select ...) for insert stat… #200

antoineB commented Oct 8, 2023

dmfay left a comment

dmfay Oct 12, 2023

antoineB Oct 12, 2023

antoineB commented Oct 12, 2023

matthias-Q commented Oct 12, 2023

dmfay commented Oct 12, 2023

antoineB commented Oct 12, 2023

matthias-Q left a comment

matthias-Q Oct 15, 2023

antoineB Oct 15, 2023

matthias-Q Oct 15, 2023

antoineB Oct 15, 2023

matthias-Q Oct 15, 2023

antoineB Oct 15, 2023

matthias-Q Oct 15, 2023

antoineB Oct 15, 2023

dmfay left a comment

		keyword_repeatable: _ => make_keyword("repeatable: _ ="),
		keyword_read: _ => make_keyword("read: _ =>"),

feat: Add set operation (select ... union select ...) for insert stat… #200

feat: Add set operation (select ... union select ...) for insert stat… #200

Conversation

antoineB commented Oct 8, 2023

dmfay left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

antoineB commented Oct 12, 2023

matthias-Q commented Oct 12, 2023

dmfay commented Oct 12, 2023

antoineB commented Oct 12, 2023

matthias-Q left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dmfay left a comment

Choose a reason for hiding this comment