Parse cursor queries with antlr before executing #3777

sharathbp · 2025-05-22T10:34:44Z

Description

Cursor queries are not parsed with antlr parser before sending it to Postgres parser, which leads to syntax error if there is any psql related keywords in the query. Antlr parser transforms the query by delimit the psql keywords in the query. Adding antlr parser logic for CURSOR queries in Pabelfish before sending it to Postgres for execution.

Issues Resolved

BABEL-5743

Test Scenarios Covered

Use case based -
Yes
Boundary conditions -
Yes
Arbitrary inputs -
Yes
Negative test cases -
Yes
Minor version upgrade tests -
NA
Major version upgrade tests -
NA
Performance tests -
NA
Tooling impact -
NA
Client tests -
NA

Check List

Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is under the terms of the Apache 2.0 and PostgreSQL licenses, and grant any person obtaining a copy of the contribution permission to relicense all or a portion of my contribution to the PostgreSQL License solely to contribute all or a portion of my contribution to the PostgreSQL open source project.

For more information on following Developer Certificate of Origin and signing off your commits, please check here.

coveralls · 2025-05-22T12:45:33Z

Pull Request Test Coverage Report for Build 15712163874

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

For more information on this, see Tracking coverage changes with pull request builds.
To avoid this issue with future PRs, see these Recommended CI Configurations.
For a quick fix, rebase this PR at GitHub. Your next report should be accurate.

Details

14 of 15 (93.33%) changed or added relevant lines in 3 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage increased (+0.02%) to 75.575%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
contrib/babelfishpg_tsql/src/cursor.c	10	11	90.91%

Totals
Change from base Build 15711252052:	0.02%
Covered Lines:	48899
Relevant Lines:	64703

💛 - Coveralls

tanscorpio7 · 2025-05-29T20:25:45Z

contrib/babelfishpg_tsql/src/cursor.c

+	 * Antlr parser will converts TSQL query into PSQL syntax and properly handles idenfier delimiters,
+	 * allowing PostgreSQL reserved words to be used as column aliases.
+	 */
+	if (stmt != NULL && sql_dialect == SQL_DIALECT_TSQL)


ANTLR can be a pretty heavy parser, so we maintain a session level cache for the ANTLR parse result for a procedure's body. Again invoking ANTLR here for sp_cursor procs would mean we always ANTLR parse during runtime.
Can we avoid that by doing this in the first parse of the statement itself ?

tanscorpio7 · 2025-06-11T07:02:21Z

contrib/babelfishpg_tsql/src/cursor.c

+	 * Antlr parser will converts TSQL query into PSQL syntax and properly handles idenfier delimiters,
+	 * allowing PostgreSQL reserved words to be used as column aliases.
+	 */
+	if (prepare && stmt != NULL && sql_dialect == SQL_DIALECT_TSQL)


Can we merge this block with the prepare block below ? Also why do we need dialect check here ?

Didn't merge it because we want to parse before creating SPI connection because if parse fails SPI connection is not required. Since SPI connection is used outside prepare if statement added it separately before SPI connection.

Dialect I added as safety as antlr parser should be used only for TSQL context right

I dont think we will ever reach here in non TSQL dialect, can you please confirm?

Verified its only getting called through ProcessTDSRequest->ProcessRPCRequest->HandleSPCursorOpenCommon

Then we dont need the TSQL Dialect check, right?

Yeah I can remove that.

contrib/babelfishpg_tsql/src/cursor.c

KushaalShroff · 2025-06-12T15:09:05Z

contrib/babelfishpg_tsql/src/cursor.c

+	 * Antlr parser will converts TSQL query into PSQL syntax and properly handles idenfier delimiters,
+	 * allowing PostgreSQL reserved words to be used as column aliases.
+	 */
+	if (prepare && stmt != NULL && sql_dialect == SQL_DIALECT_TSQL)


I dont think we will ever reach here in non TSQL dialect, can you please confirm?

KushaalShroff · 2025-06-12T15:09:22Z

contrib/babelfishpg_tsql/src/cursor.c

+	 */
+	if (prepare && stmt != NULL && sql_dialect == SQL_DIALECT_TSQL)
+	{
+		pltsql_curr_compile = NULL;


Why do we need to set this locally?

contrib/babelfishpg_tsql/src/cursor.c

test/JDBC/expected/TestCursorFetchNext.out

KushaalShroff · 2025-06-17T16:08:22Z

contrib/babelfishpg_tsql/src/cursor.c

+	 * Antlr parser will converts TSQL query into PSQL syntax and properly handles idenfier delimiters,
+	 * allowing PostgreSQL reserved words to be used as column aliases.
+	 */
+	if (prepare && stmt != NULL && sql_dialect == SQL_DIALECT_TSQL)


Then we dont need the TSQL Dialect check, right?

KushaalShroff · 2025-06-17T16:20:55Z

contrib/babelfishpg_tsql/src/pl_gram.y

+	if (pltsql_curr_compile)
+		collation = pltsql_curr_compile->fn_input_collation;


Is there a possibility that pltsql_curr_compile is NULL here? Thats serious isn't it?

If its NULL then we will not access it. It will be the assigned value above InvalidOid. Before it was causing seg fault because we were accessing it without null check below so added this.

tanscorpio7 · 2025-06-17T17:40:37Z

Please also add test cases for sp_cursor procedures. sp_cursoropen, sp_cursorexecute, etc. Thanks.

Sharath BP added 3 commits May 22, 2025 10:32

Parse cursor queries with antlr before executing

8e32f51

Merge branch 'BABEL_5_X_DEV' into babel-5743

3fbe30b

Add NULL check for stmt

e7c2a0c

sharathbp requested review from Deepesh125, sumitj824, tanyagupta17, rohit01010 and Yvinayak07 and removed request for tanyagupta17 and Yvinayak07 May 23, 2025 04:41

Sharath BP added 3 commits May 23, 2025 07:35

Added test cases

0e6ede0

Updated comments

885556a

Fix test failures

8cbc3f4

sharathbp force-pushed the babel-5743 branch from 37c8143 to 8cbc3f4 Compare May 26, 2025 10:21

Sharath BP added 2 commits May 29, 2025 05:54

Fix test failure

2d6c793

Fix test files

44c357d

sharathbp requested a review from tanscorpio7 May 29, 2025 07:49

tanscorpio7 suggested changes May 29, 2025

View reviewed changes

Parse cursor queries with antlr only for prepare statements

8e44cf5

sharathbp requested a review from KushaalShroff June 10, 2025 09:20

tanscorpio7 suggested changes Jun 11, 2025

View reviewed changes

KushaalShroff requested changes Jun 12, 2025

View reviewed changes

sharathbp force-pushed the babel-5743 branch 2 times, most recently from 4d4b7c1 to 8e44cf5 Compare June 17, 2025 15:38

Sharath BP added 3 commits June 17, 2025 15:39

Merge branch 'BABEL_5_X_DEV' into babel-5743

ae4c43e

Fix parser errors

16b55a0

fix cursor.c

c6d154e

KushaalShroff requested changes Jun 17, 2025

View reviewed changes

		if (pltsql_curr_compile)
		collation = pltsql_curr_compile->fn_input_collation;

Parse cursor queries with antlr before executing #3777

Are you sure you want to change the base?

Parse cursor queries with antlr before executing #3777

Uh oh!

Conversation

sharathbp commented May 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Issues Resolved

Test Scenarios Covered

Check List

Uh oh!

coveralls commented May 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull Request Test Coverage Report for Build 15712163874

Warning: This coverage report may be inaccurate.

Details

💛 - Coveralls

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sharathbp Jun 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tanscorpio7 commented Jun 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

sharathbp commented May 22, 2025 •

edited

Loading

coveralls commented May 22, 2025 •

edited

Loading

sharathbp Jun 17, 2025 •

edited

Loading

tanscorpio7 commented Jun 17, 2025 •

edited

Loading