From db4e1389f2af4e4b2ec9e56e3520cabadb939a16 Mon Sep 17 00:00:00 2001 From: Bo Date: Tue, 3 Dec 2024 16:20:33 -0800 Subject: [PATCH] ExecProcessReturning is unexpectedly called twice for psql request causing overflow error (#488) In community postgresql, ExecProcessReturning is called once after the main process of ExecInsert. But due to some merging errors related to Babelfish and T-SQL, this function is wrongly called twice for psql requests. At the first call, the xmax is not initialized causing unexpected error raising. The first intention of these pieces of codes is to call it in the first place only for T-SQL(CR-52326143). And then got wrongly cherry-picked in CR-57250041. Finally, the condition check gets removed in CR-65026879. We should keep the first intention of the codes, which also makes it consistent with the behaviour in ExecUpdate. Task: BABEL-5343 Signed-off-by: Bo Li --- src/backend/executor/nodeModifyTable.c | 4 ++-- .../regress/expected/babel_5343_returning.out | 21 +++++++++++++++++++ src/test/regress/parallel_schedule | 3 +++ src/test/regress/sql/babel_5343_returning.sql | 5 +++++ 4 files changed, 31 insertions(+), 2 deletions(-) create mode 100644 src/test/regress/expected/babel_5343_returning.out create mode 100644 src/test/regress/sql/babel_5343_returning.sql diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c index 1e473ded06f..cf3f23909f2 100644 --- a/src/backend/executor/nodeModifyTable.c +++ b/src/backend/executor/nodeModifyTable.c @@ -824,7 +824,7 @@ ExecInsert(ModifyTableContext *context, } /* Process RETURNING if present */ - if (resultRelInfo->ri_projectReturning) + if (resultRelInfo->ri_projectReturning && sql_dialect == SQL_DIALECT_TSQL) result = ExecProcessReturning(resultRelInfo, slot, planSlot); /* INSTEAD OF ROW INSERT Triggers */ @@ -1218,7 +1218,7 @@ ExecInsert(ModifyTableContext *context, ExecWithCheckOptions(WCO_VIEW_CHECK, resultRelInfo, slot, estate); /* Process RETURNING if present */ - if (resultRelInfo->ri_projectReturning) + if (resultRelInfo->ri_projectReturning && sql_dialect != SQL_DIALECT_TSQL) result = ExecProcessReturning(resultRelInfo, slot, planSlot); if (inserted_tuple) diff --git a/src/test/regress/expected/babel_5343_returning.out b/src/test/regress/expected/babel_5343_returning.out new file mode 100644 index 00000000000..8d9ac5d2c0d --- /dev/null +++ b/src/test/regress/expected/babel_5343_returning.out @@ -0,0 +1,21 @@ +CREATE TABLE toverflow(id SERIAL, col1 VARCHAR); +INSERT INTO toverflow VALUES (default, 'hope') RETURNING xmax::text::int; + xmax +------ + 0 +(1 row) + +INSERT INTO toverflow VALUES (default, 'jack') RETURNING xmax; + xmax +------ + 0 +(1 row) + +SELECT * FROM toverflow ORDER BY id; + id | col1 +----+------ + 1 | hope + 2 | jack +(2 rows) + +DROP TABLE toverflow; diff --git a/src/test/regress/parallel_schedule b/src/test/regress/parallel_schedule index 0a792f50dc4..7d74f2b2fb5 100644 --- a/src/test/regress/parallel_schedule +++ b/src/test/regress/parallel_schedule @@ -138,3 +138,6 @@ test: serializable # run tablespace test at the end because it drops the tablespace created during # setup that other tests may use. test: tablespace + +# Test Babel returning issue BABEL-5343 +test: babel_5343_returning diff --git a/src/test/regress/sql/babel_5343_returning.sql b/src/test/regress/sql/babel_5343_returning.sql new file mode 100644 index 00000000000..455788f803b --- /dev/null +++ b/src/test/regress/sql/babel_5343_returning.sql @@ -0,0 +1,5 @@ +CREATE TABLE toverflow(id SERIAL, col1 VARCHAR); +INSERT INTO toverflow VALUES (default, 'hope') RETURNING xmax::text::int; +INSERT INTO toverflow VALUES (default, 'jack') RETURNING xmax; +SELECT * FROM toverflow ORDER BY id; +DROP TABLE toverflow;