ExecProcessReturning is unexpectedly called twice for psql request ca…

…using 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 <[email protected]>

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)

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;

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

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;