Skip to content

Commit e6e094c

Browse files
authored
Fixed error while using PosgreSQL reserved keyword in cursor operations
During the cursor prepare, execution flow for statement is not passed through antlr parser. This causes issue when query contains PosgreSQL reserved keywords. This PR addresses this issue, along with any potential issue with query which is not compatible with PostgreSQL Task: BABEL-5743 Signed-off-by: Shard Gupta shardga@amazon.com
1 parent 7840f56 commit e6e094c

File tree

7 files changed

+180
-2
lines changed

7 files changed

+180
-2
lines changed

contrib/babelfishpg_tsql/src/cursor.c

Lines changed: 49 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1449,6 +1449,10 @@ execute_sp_cursoropen_common(int *stmt_handle, int *cursor_handle, const char *s
14491449
Portal portal;
14501450
MemoryContext oldcontext;
14511451
MemoryContext savedPortalCxt;
1452+
PLtsql_stmt_execsql *parse_result;
1453+
PLtsql_function *func;
1454+
char *stmt_copy;
1455+
char *tsql_stmt = NULL; /* Use this for potentially modified statement */
14521456

14531457
/*
14541458
* Connect to SPI manager. should be handled in the same way with
@@ -1468,10 +1472,53 @@ execute_sp_cursoropen_common(int *stmt_handle, int *cursor_handle, const char *s
14681472

14691473
if (prepare)
14701474
{
1475+
/*
1476+
* This entire block is to parse the statement by antlr and use the statement for
1477+
* cursor execution. This is necessary in some use case, for example when we have
1478+
* PostgreSQL reversed keywords in query which is valid in TSQL.
1479+
* Antlr parser will add the quotes at necessary places in query so that Postgres engine
1480+
* can resolve this query correctly.
1481+
*/
1482+
if (stmt)
1483+
{
1484+
/* Copy the original statement, because we don't want to use the resultant query in every case. */
1485+
stmt_copy = pstrdup(stmt);
1486+
/* Send to antlr parser */
1487+
func = pltsql_compile_inline(stmt_copy, NULL);
1488+
1489+
/*
1490+
* Check the node list of type PLtsql_stmt_type. Cursor only support single statement.
1491+
* If there are more than 1 statement we will through the error. func->action-body
1492+
* returned by pltsql_compile_inline generally contains two default nodes, PLTSQL_STMT_INIT being first
1493+
* and PLTSQL_STMT_RETURN being last. In case of empty statement only PLTSQL_STMT_RETURN node will be present.
1494+
* So total number of nodes should be 3 for valid cursor execution. Actual query statement will be at second
1495+
* position of type PLTSQL_STMT_EXECSQL.
1496+
*
1497+
* This is defensive code, where we only assign the tsql_stmt variable to parsed query,
1498+
* if the cmd_type is PLTSQL_STMT_EXECSQL. There might be other types of cmd_type like
1499+
* PLTSQL_STMT_EXECSQL (for procedures), for them we will keep the old behavior.
1500+
* list length should be checked first before trying to deference second node from the list.
1501+
* FIXME:We are handling only PLtsql_stmt_execsql type statement, but TSQL support procedure with
1502+
* single statement. We also need to explore all other statement types for completion.
1503+
*/
1504+
if(list_length(func->action->body) == 3 &&
1505+
((PLtsql_stmt *) lsecond(func->action->body))->cmd_type == PLTSQL_STMT_EXECSQL)
1506+
{
1507+
parse_result = (PLtsql_stmt_execsql *) lsecond(func->action->body);
1508+
tsql_stmt = pstrdup(parse_result->sqlstmt->query);
1509+
}
1510+
1511+
/*Free up function memory as this is not needed anymore */
1512+
pltsql_free_function_memory(func);
1513+
}
1514+
14711515
/* prepare plan and insert a cursor entry */
1472-
plan = SPI_prepare_cursor(stmt, nBindParams, boundParamsOidList, cursor_options);
1516+
plan = SPI_prepare_cursor(tsql_stmt?tsql_stmt:stmt, nBindParams, boundParamsOidList, cursor_options);
14731517
if (plan == NULL)
14741518
return 1; /* procedure failed */
1519+
/* Free memory if tsql_stmt is not null */
1520+
if (tsql_stmt)
1521+
pfree((void *)tsql_stmt);
14751522

14761523
if (save_plan)
14771524
{
@@ -1887,4 +1934,4 @@ reset_cached_cursor(void)
18871934

18881935
/* Reset sp_cursor_params. */
18891936
reset_sp_cursor_params();
1890-
}
1937+
}

test/JDBC/expected/BABEL-SPCURSOR.out

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -468,7 +468,28 @@ int#!#int#!#int#!#int#!#int
468468
~~END~~
469469

470470

471+
# cursor with procedure execution test
472+
CREATE PROCEDURE p1 AS BEGIN select * from babel_cursor_t1 end
473+
go
474+
DECLARE @cursor_handle int;
475+
EXEC sp_cursoropen @cursor_handle OUTPUT, N'exec p1', 2, 8193;
476+
go
477+
~~ERROR (Code: 33557097)~~
478+
479+
~~ERROR (Message: cannot open CALL query as cursor)~~
480+
481+
482+
# cursor with empty statement will result in error
483+
DECLARE @cursor_handle int;
484+
EXEC sp_cursoropen @cursor_handle OUTPUT, N'', 2, 8193;
485+
go
486+
~~ERROR (Code: 33557097)~~
487+
488+
~~ERROR (Message: cannot open multi-query plan as cursor)~~
489+
490+
471491
DROP TABLE babel_cursor_t1
472492
DROP TABLE t1812
493+
DROP PROCEDURE p1
473494
GO
474495

test/JDBC/expected/TestCursorFetchNext.out

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,44 @@ int#!#smallint#!#bigint#!#tinyint#!#bit
6464
#cursor#!#fetch#!#next
6565
#cursor#!#fetch#!#afterlast
6666
#cursor#!#fetch#!#next
67+
cursor#!#close
68+
~~SUCCESS~~
69+
cursor#!#open#!#SELECT a year, b month, c quarter FROM test_cursors_fetch_next#!#TYPE_SCROLL_INSENSITIVE#!#CONCUR_READ_ONLY#!#CLOSE_CURSORS_AT_COMMIT
70+
~~SUCCESS~~
71+
cursor#!#fetch#!#next
72+
~~START~~
73+
int#!#smallint#!#bigint
74+
0#!#0#!#0
75+
~~END~~
76+
77+
cursor#!#fetch#!#first
78+
~~START~~
79+
int#!#smallint#!#bigint
80+
0#!#0#!#0
81+
~~END~~
82+
83+
cursor#!#fetch#!#next
84+
~~START~~
85+
int#!#smallint#!#bigint
86+
<NULL>#!#<NULL>#!#<NULL>
87+
~~END~~
88+
89+
cursor#!#fetch#!#last
90+
~~START~~
91+
int#!#smallint#!#bigint
92+
211234#!#9780#!#891372401
93+
~~END~~
94+
95+
cursor#!#close
96+
~~SUCCESS~~
97+
cursor#!#open#!#DECLARE @var INT; select @var = 5; select @var + 1;#!#CONCUR_READ_ONLY#!#CLOSE_CURSORS_AT_COMMIT
98+
~~SUCCESS~~
99+
cursor#!#fetch#!#next
100+
~~START~~
101+
int
102+
6
103+
~~END~~
104+
67105
cursor#!#close
68106
~~SUCCESS~~
69107
DROP TABLE test_cursors_fetch_next

test/JDBC/expected/TestCursorPrepExecFetchNext.out

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,38 @@ int#!#smallint#!#bigint#!#tinyint#!#bit
6969
#cursor#!#fetch#!#next
7070
#cursor#!#fetch#!#afterlast
7171
#cursor#!#fetch#!#next
72+
cursor#!#close
73+
~~SUCCESS~~
74+
cursor#!#open#!#prepst#!#SELECT a year, b month, c quarter from test_cursor_prep_exec_fetch_next#!#TYPE_SCROLL_INSENSITIVE#!#CONCUR_READ_ONLY#!#CLOSE_CURSORS_AT_COMMIT
75+
~~SUCCESS~~
76+
cursor#!#fetch#!#next
77+
~~START~~
78+
int#!#smallint#!#bigint
79+
0#!#0#!#0
80+
~~END~~
81+
82+
cursor#!#fetch#!#first
83+
~~START~~
84+
int#!#smallint#!#bigint
85+
0#!#0#!#0
86+
~~END~~
87+
88+
cursor#!#fetch#!#next
89+
~~START~~
90+
int#!#smallint#!#bigint
91+
<NULL>#!#<NULL>#!#<NULL>
92+
~~END~~
93+
94+
cursor#!#close
95+
~~SUCCESS~~
96+
cursor#!#open#!#DECLARE @var INT; select @var = 5; select @var + 1;#!#CONCUR_READ_ONLY#!#CLOSE_CURSORS_AT_COMMIT
97+
~~SUCCESS~~
98+
cursor#!#fetch#!#next
99+
~~START~~
100+
int
101+
6
102+
~~END~~
103+
72104
cursor#!#close
73105
~~SUCCESS~~
74106
DROP TABLE test_cursor_prep_exec_fetch_next
@@ -285,4 +317,11 @@ float#!#real#!#money#!#smallmoney
285317
#cursor#!#fetch#!#next
286318
cursor#!#close
287319
~~SUCCESS~~
320+
# Cursor with multiple selects should return error
321+
cursor#!#open#!#prepst#!#SELECT * FROM test_cursor_prep_exec_fetch_next;SELECT a FROM test_cursor_prep_exec_fetch_next#!#TYPE_SCROLL_INSENSITIVE#!#CONCUR_READ_ONLY#!#CLOSE_CURSORS_AT_COMMIT
322+
~~ERROR (Code: 33557097)~~
323+
324+
~~ERROR (Message: cannot open multi-query plan as cursor)~~
325+
326+
288327
DROP TABLE test_cursor_prep_exec_fetch_next

test/JDBC/input/BABEL-SPCURSOR.sql

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -225,7 +225,20 @@ exec sp_cursorclose @p2;
225225
exec sp_cursorunprepare @p1;
226226
go
227227

228+
# cursor with procedure execution test
229+
CREATE PROCEDURE p1 AS BEGIN select * from babel_cursor_t1 end
230+
go
231+
DECLARE @cursor_handle int;
232+
EXEC sp_cursoropen @cursor_handle OUTPUT, N'exec p1', 2, 8193;
233+
go
234+
235+
# cursor with empty statement will result in error
236+
DECLARE @cursor_handle int;
237+
EXEC sp_cursoropen @cursor_handle OUTPUT, N'', 2, 8193;
238+
go
239+
228240
DROP TABLE babel_cursor_t1
229241
DROP TABLE t1812
242+
DROP PROCEDURE p1
230243
GO
231244

test/JDBC/input/cursors/TestCursorFetchNext.txt

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,15 @@ cursor#!#fetch#!#next
2020
#cursor#!#fetch#!#afterlast
2121
#cursor#!#fetch#!#next
2222
cursor#!#close
23+
cursor#!#open#!#SELECT a year, b month, c quarter FROM test_cursors_fetch_next#!#TYPE_SCROLL_INSENSITIVE#!#CONCUR_READ_ONLY#!#CLOSE_CURSORS_AT_COMMIT
24+
cursor#!#fetch#!#next
25+
cursor#!#fetch#!#first
26+
cursor#!#fetch#!#next
27+
cursor#!#fetch#!#last
28+
cursor#!#close
29+
cursor#!#open#!#DECLARE @var INT; select @var = 5; select @var + 1;#!#CONCUR_READ_ONLY#!#CLOSE_CURSORS_AT_COMMIT
30+
cursor#!#fetch#!#next
31+
cursor#!#close
2332
DROP TABLE test_cursors_fetch_next
2433

2534
CREATE TABLE test_cursors_fetch_next(a CHAR(30), b VARCHAR(30), c NCHAR(30), d NVARCHAR(30));

test/JDBC/input/cursors/TestCursorPrepExecFetchNext.txt

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,14 @@ cursor#!#fetch#!#next
2323
#cursor#!#fetch#!#afterlast
2424
#cursor#!#fetch#!#next
2525
cursor#!#close
26+
cursor#!#open#!#prepst#!#SELECT a year, b month, c quarter from test_cursor_prep_exec_fetch_next#!#TYPE_SCROLL_INSENSITIVE#!#CONCUR_READ_ONLY#!#CLOSE_CURSORS_AT_COMMIT
27+
cursor#!#fetch#!#next
28+
cursor#!#fetch#!#first
29+
cursor#!#fetch#!#next
30+
cursor#!#close
31+
cursor#!#open#!#DECLARE @var INT; select @var = 5; select @var + 1;#!#CONCUR_READ_ONLY#!#CLOSE_CURSORS_AT_COMMIT
32+
cursor#!#fetch#!#next
33+
cursor#!#close
2634
DROP TABLE test_cursor_prep_exec_fetch_next
2735

2836
CREATE TABLE test_cursor_prep_exec_fetch_next(a CHAR(30), b VARCHAR(30), c NCHAR(30), d NVARCHAR(30));
@@ -94,4 +102,7 @@ cursor#!#fetch#!#next
94102
#cursor#!#fetch#!#afterlast
95103
#cursor#!#fetch#!#next
96104
cursor#!#close
105+
# Cursor with multiple selects should return error
106+
cursor#!#open#!#prepst#!#SELECT * FROM test_cursor_prep_exec_fetch_next;SELECT a FROM test_cursor_prep_exec_fetch_next#!#TYPE_SCROLL_INSENSITIVE#!#CONCUR_READ_ONLY#!#CLOSE_CURSORS_AT_COMMIT
107+
97108
DROP TABLE test_cursor_prep_exec_fetch_next

0 commit comments

Comments
 (0)