Fix the issue that PIVOT execution not return row affected line #2071

forestkeeper · 2023-11-28T01:21:17Z

Previous our implement of PIVOT has a issue that it didn't return the row affected line as result, this fix has fixed the bug.

Task: BABEL-284

Description

This fix mainly fixed this problem :

select *
from 
(
select store, week, xCount
from yt 
) src
pivot
(
sum(xcount)
for week in ([1], [2], [3])
) piv;
 
store       1           2           3          
----------- ----------- ----------- -----------
        101         138         282         220
        102          96         212         123
        105          37          78          60
        109          59          97          87

(4 rows affected)

the

(4 rows affected)

will be missing on previous fix, it's due to too early release of the cached plan

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.

Previous our implement of PIVOT has a issue that it didn't return the row affected line as result, this fix has fixed the bug. Task: BABEL-284 Signed-off-by: Zhibai Song <[email protected]>

lejaokri · 2023-11-28T01:38:47Z

contrib/babelfishpg_tsql/src/pl_exec.c

@@ -5059,8 +5065,7 @@ exec_stmt_execsql(PLtsql_execstate *estate,
 		/* If current plan constains a pivot operator, we remove the plan */


This comment does not match the code anymore. Please update comment.

lejaokri · 2023-11-28T01:39:46Z

contrib/babelfishpg_tsql/src/pl_exec.c

+		if (expr->plan && expr->plan->oneshot)
+		{
+			SPI_freeplan(expr->plan);
+			expr->plan = NULL;
+		}


This code isn't pivot specific anymore so any generic expr->plan->oneshot will go here. Has that been tested?

Yes, it has passed all known cases

As for the oneshot variable, is it specific in this case ? My understand is yes, since in PG execution whether oneshot is defined at spi_prepare stage, and it'll release in plan execution ends. But our usage is define at end of execution, and free plan at the begining of query before preparing. So they don't have overlap in this case.

lejaokri · 2023-11-28T01:41:21Z

test/JDBC/expected/pivot-vu-verify.out

@@ -738,6 +738,8 @@ PIVOT (
 ) AS pvt;
 SELECT TOP 10 * FROM pivot_insert_into ORDER by 1, 2;
 GO
+~~ROW COUNT: 197~~


wouldn't TOP 10 return only 10 rows?

This row count is corresponding to the insert select, and it insert 197 lines

This one :

INSERT INTO pivot_insert_into SELECT ManufactureID, EmployeeID, [2] AS STORE2, [3] AS STORE3, [4] AS STORE4, [5] AS STORE5, [6] AS STORE6 FROM ( SELECT ManufactureID, EmployeeID, ItemID, StoreID FROM StoreReceipt )AS srctable PIVOT ( COUNT (ItemID) FOR StoreID IN ([2], [3], [4], [5], [6]) ) AS pvt;

The TOP 10 is another statement

wouldn't TOP 10 return only 10 rows?

There's a long-standing issue with SELECT-INTO that it does not report the row count (though @@rowcount is correct). See BABEL-540

…lfish-for-postgresql#2071) Previous our implement of PIVOT has a issue that it didn't return the row affected line as result, this fix has fixed the bug. Task: BABEL-284 Signed-off-by: Zhibai Song <[email protected]>

* PIVOT Bugs Fix (#2018) Previously, Babelfish PIVOT operator has many bugs and limitation. In this PR, we fixed the bugs and blocked some unsupported edge cases. Issues Resolved 1. fixed pivot used in function/procedure will cause server restart 2. fixed table/schema name used in 'FOR (colname)' will cause server restart 3. fixed view usage in pivot stmt produce empty result 4. NULL should be the aggregate result when no row is selected(except COUNT) 5. Error message for creating view on pivot, 6. Error message when use pivot in WITH CTE 7. Error message when use join with PIVOT stmt Task: BABEL-4567, BABEL-4568, BABEL-4569, BABEL-4576, BABEL-4558, BABEL-4559 Signed-off-by: Yanjie Xu <[email protected]> * Fix the issue that PIVOT execution not return row affected line (#2071) Previous our implement of PIVOT has a issue that it didn't return the row affected line as result, this fix has fixed the bug. Task: BABEL-284 Signed-off-by: Zhibai Song <[email protected]> --------- Signed-off-by: Yanjie Xu <[email protected]> Signed-off-by: Zhibai Song <[email protected]> Co-authored-by: Zhibai Song <[email protected]>

…lfish-for-postgresql#2071) Previous our implement of PIVOT has a issue that it didn't return the row affected line as result, this fix has fixed the bug. Task: BABEL-284 Signed-off-by: Zhibai Song <[email protected]>

Previous our implement of PIVOT has a issue that it didn't return the row affected line as result, this fix has fixed the bug. Task: BABEL-284 Signed-off-by: Zhibai Song <[email protected]>

Fix the issue that PIVOT execution not return row affected line

bf94b82

Previous our implement of PIVOT has a issue that it didn't return the row affected line as result, this fix has fixed the bug. Task: BABEL-284 Signed-off-by: Zhibai Song <[email protected]>

lejaokri reviewed Nov 28, 2023

View reviewed changes

change comment

c58216c

lejaokri approved these changes Nov 28, 2023

View reviewed changes

2jungkook approved these changes Nov 28, 2023

View reviewed changes

forestkeeper merged commit 369ea1b into babelfish-for-postgresql:BABEL_3_X_DEV Nov 28, 2023
29 checks passed

RIC06X mentioned this pull request Dec 4, 2023

PIVOT Bug Fix Cherry-Pick BABEL_3_4_STABLE #2102

Merged

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix the issue that PIVOT execution not return row affected line #2071

Fix the issue that PIVOT execution not return row affected line #2071

forestkeeper commented Nov 28, 2023

lejaokri Nov 28, 2023

forestkeeper Nov 28, 2023

lejaokri Nov 28, 2023

forestkeeper Nov 28, 2023 •

edited

Loading

lejaokri Nov 28, 2023

forestkeeper Nov 28, 2023

forestkeeper Nov 28, 2023

forestkeeper Nov 28, 2023

robverschoor Nov 29, 2023

		@@ -5059,8 +5065,7 @@ exec_stmt_execsql(PLtsql_execstate *estate,
		/* If current plan constains a pivot operator, we remove the plan */

Fix the issue that PIVOT execution not return row affected line #2071

Fix the issue that PIVOT execution not return row affected line #2071

Conversation

forestkeeper commented Nov 28, 2023

Description

Check List

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

forestkeeper Nov 28, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

forestkeeper Nov 28, 2023 •

edited

Loading