sql: add plpgsql parser for telemetry purposes #98147

e-mbrown · 2023-03-07T16:30:13Z

Informs: #91569

This commit adds the in use portions of the plpgsql parser from #91606.

Release note: None

cockroach-teamcity · 2023-03-07T16:30:40Z

This change is

ZhouXing19 · 2023-03-07T16:32:43Z

Could you also rewrite the messages of the existing commits? I believe they are drafts. Thanks!

ZhouXing19 · 2023-03-13T17:47:45Z

The last commit contains lots of rewrites of build.bazel. I think it's because of rebasing. Can we squash it with the first commit (the scaffold one)?

rimadeodhar · 2023-03-24T22:16:18Z

@ZhouXing19 and @chengxiong-ruan gentle ping to review this PR when you get a chance. Thanks!

Adds the first test which parses an empty function or function with bare sql queries. Release note: None

This commit adds `lexer.ReadSqlExpressionStr()` which gives access to the return type of a plpgsql function. This commit also adds grammar rules and AST for `stmt_if` Release note: None

This commit adds AST and grammar rules for `decl_header` and `stmt_assert` Release note: None

This commit adds the AST and grammar rules for `stmt_call`, `stmt_close`, `stmt_dynaexec`, and `stmt_exit` Release note: None

This commit adds the AST and grammar rules for `stmt_assign` and `stmt_getdiag` Release note: None

This commit adds portions of the logic to parse `stmt_case`. Does not yet support ELSE, sql statements in THEN body, or END CASE (it uses an ENDCASE hack). Release note: None

This commit corrects the grammar rules for `stmt_case`. Release note: None

This commit corrects the grammar rules and AST of `stmt_if`, `elsif`, `else` statements. Release note: None

This commit adds logic for `stmt_open` Release note: None

e-mbrown

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @chengxiong-ruan, @DrewKimball, @knz, @rafiss, and @ZhouXing19)

pkg/sql/sem/plpgsqltree/constants.go line 16 at r70 (raw file):

Previously, knz (Raphael 'kena' Poss) wrote…

Here and for all the other comments:

each comment for an individual exported symbol needs to be prefixed by the name of the exported symbol.

comments contain sentence and thus should end with a period.

Done.

pkg/sql/sem/plpgsqltree/constants.go line 40 at r70 (raw file):

Previously, knz (Raphael 'kena' Poss) wrote…

String is special - usually we document // String implements the fmt.Stringer interface.

Done.

pkg/sql/plpgsql/parser/parser_test.go line 32 at r59 (raw file):

Previously, knz (Raphael 'kena' Poss) wrote…

Would it be possible to define the data driven test framework in 1 place, and us it from both packages?

We will do this at a later date.

chengxiong-ruan · 2023-04-12T20:17:39Z

pkg/sql/plpgsql/parser/parser_test.go line 32 at r59 (raw file):

Previously, e-mbrown (Ebony Brown) wrote…

We will do this at a later date.

Could you file an issue to track or at least a TODO? Thanks.

This commit makes Statement generic so it can be shared between the sql and plpgsql parsers. Release note: None

e-mbrown

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @chengxiong-ruan, @DrewKimball, @knz, @rafiss, and @ZhouXing19)

pkg/sql/plpgsql/parser/parser_test.go line 32 at r59 (raw file):

data driven test framework
I'll add a TODO for now.

chengxiong-ruan

LGTM! Thank you for your patience through this.

e-mbrown · 2023-04-13T17:20:45Z

Thanks for all the reviews!!

bors r+

craig · 2023-04-13T18:11:01Z

Build succeeded:

Bazel Essential CI (Cockroach)

99517: sql: add unimplemented errors and telemetry for plpgsql r=e-mbrown a=e-mbrown Part of: #98147 Closes: #91569 This PR expands grammar rules and adds unimplemented support for pl/pgsql statements. [sql: Add telemetry for parsable plpgsql statements](37aa95d) Is the first commit unique to this PR. Release Notes: None Co-authored-by: Jane Xing <zhouxing@uchicago.edu> Co-authored-by: e-mbrown <ebsonari@gmail.com>

e-mbrown · 2023-04-27T20:15:19Z

blathers backport 23.1 22.2

blathers-crl · 2023-04-27T20:15:39Z

Encountered an error creating backports. Some common things that can go wrong:

The backport branch might have already existed.
There was a merge conflict.
The backport branch contained merge commits.

You might need to create your backport manually using the backport tool.

error creating merge commit from 9688586 to blathers/backport-release-23.1-98147: POST https://api.github.com/repos/cockroachdb/cockroach/merges: 409 Merge conflict []

you may need to manually resolve merge conflicts with the backport tool.

Backport to branch 23.1 failed. See errors above.

error creating merge commit from 19b6cb9 to blathers/backport-release-22.2-98147: POST https://api.github.com/repos/cockroachdb/cockroach/merges: 409 Merge conflict []

you may need to manually resolve merge conflicts with the backport tool.

Backport to branch 22.2 failed. See errors above.

_{🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf.}

e-mbrown requested review from chengxiong-ruan and ZhouXing19 March 7, 2023 16:30

e-mbrown force-pushed the eb/bareplpg branch 2 times, most recently from 3c0cb70 to 019a4af Compare March 13, 2023 15:16

e-mbrown force-pushed the eb/bareplpg branch 3 times, most recently from a92f71d to 609e6a4 Compare March 14, 2023 18:41

e-mbrown requested a review from rafiss March 14, 2023 18:42

e-mbrown marked this pull request as ready for review March 14, 2023 18:42

e-mbrown requested a review from a team as a code owner March 14, 2023 18:42

e-mbrown force-pushed the eb/bareplpg branch 12 times, most recently from b4f2eb3 to 4e4d76f Compare March 16, 2023 19:10

e-mbrown mentioned this pull request Mar 24, 2023

sql: add unimplemented errors and telemetry for plpgsql #99517

Merged

e-mbrown force-pushed the eb/bareplpg branch from 9b3d9f6 to f9f3439 Compare March 29, 2023 16:58

e-mbrown requested a review from a team as a code owner March 29, 2023 16:58

e-mbrown force-pushed the eb/bareplpg branch from f9f3439 to 6d303fa Compare March 29, 2023 17:36

chengxiong-ruan and others added 9 commits April 12, 2023 15:16

sql: declare section and exec sql

df07315

Adds the first test which parses an empty function or function with bare sql queries. Release note: None

sql: add lexer.ReadSqlExpressionStr() and stmt_if

6414250

This commit adds `lexer.ReadSqlExpressionStr()` which gives access to the return type of a plpgsql function. This commit also adds grammar rules and AST for `stmt_if` Release note: None

sql: add plpgsql decl_header and stmt_assert

71a9821

This commit adds AST and grammar rules for `decl_header` and `stmt_assert` Release note: None

sql: stmt_call, stmt_close, stmt_dynaexec, stmt_exit

7505583

This commit adds the AST and grammar rules for `stmt_call`, `stmt_close`, `stmt_dynaexec`, and `stmt_exit` Release note: None

sql: stmt_assign and stmt_getdiag

2f24d05

This commit adds the AST and grammar rules for `stmt_assign` and `stmt_getdiag` Release note: None

sql: Adds support for stmt_case

e11a91a

This commit adds portions of the logic to parse `stmt_case`. Does not yet support ELSE, sql statements in THEN body, or END CASE (it uses an ENDCASE hack). Release note: None

sql: fix endif and endcase

9e9ebd1

This commit corrects the grammar rules for `stmt_case`. Release note: None

sql: Complete stmt_if, elsif, else logic

cbd3ca0

This commit corrects the grammar rules and AST of `stmt_if`, `elsif`, `else` statements. Release note: None

sql: add stmt_open

c6c8a9f

This commit adds logic for `stmt_open` Release note: None

e-mbrown force-pushed the eb/bareplpg branch from 716ef13 to 34cb286 Compare April 12, 2023 19:16

e-mbrown commented Apr 12, 2023

View reviewed changes

e-mbrown requested a review from chengxiong-ruan April 12, 2023 19:34

server,sql: Make the statement struct generic

9688586

This commit makes Statement generic so it can be shared between the sql and plpgsql parsers. Release note: None

e-mbrown force-pushed the eb/bareplpg branch from 34cb286 to 9688586 Compare April 12, 2023 21:00

e-mbrown commented Apr 12, 2023

View reviewed changes

chengxiong-ruan approved these changes Apr 13, 2023

View reviewed changes

craig bot merged commit 8fbb38c into cockroachdb:master Apr 13, 2023

e-mbrown deleted the eb/bareplpg branch April 27, 2023 20:12

e-mbrown added backport-22.2.x backport-23.1.x Flags PRs that need to be backported to 23.1 labels Apr 27, 2023

This was referenced Apr 28, 2023

release-22.2: sql: add plpgsql parser for telemetry purposes #102608

Merged

release-22.2: sql: add unimplemented errors and telemetry for plpgsql #102625

Merged

This was referenced May 10, 2023

release-23.1: sql: add plpgsql parser for telemetry purposes #103026

Merged

release-23.1: sql: add unimplemented errors and telemetry for plpgsql #103132

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

sql: add plpgsql parser for telemetry purposes #98147

sql: add plpgsql parser for telemetry purposes #98147

e-mbrown commented Mar 7, 2023 •

edited

Loading

cockroach-teamcity commented Mar 7, 2023

ZhouXing19 commented Mar 7, 2023

ZhouXing19 commented Mar 13, 2023

rimadeodhar commented Mar 24, 2023

e-mbrown left a comment

chengxiong-ruan commented Apr 12, 2023

e-mbrown left a comment

chengxiong-ruan left a comment

e-mbrown commented Apr 13, 2023

craig bot commented Apr 13, 2023

e-mbrown commented Apr 27, 2023

blathers-crl bot commented Apr 27, 2023

sql: add plpgsql parser for telemetry purposes #98147

sql: add plpgsql parser for telemetry purposes #98147

Conversation

e-mbrown commented Mar 7, 2023 • edited Loading

cockroach-teamcity commented Mar 7, 2023

ZhouXing19 commented Mar 7, 2023

ZhouXing19 commented Mar 13, 2023

rimadeodhar commented Mar 24, 2023

e-mbrown left a comment

Choose a reason for hiding this comment

chengxiong-ruan commented Apr 12, 2023

e-mbrown left a comment

Choose a reason for hiding this comment

chengxiong-ruan left a comment

Choose a reason for hiding this comment

e-mbrown commented Apr 13, 2023

craig bot commented Apr 13, 2023

e-mbrown commented Apr 27, 2023

blathers-crl bot commented Apr 27, 2023

e-mbrown commented Mar 7, 2023 •

edited

Loading