Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Resolve TDS protocol violation occurring due to parallel query enforced #1937

Merged

Conversation

Deepesh125
Copy link
Contributor

@Deepesh125 Deepesh125 commented Oct 19, 2023

Description

This issue was mainly caused by incorrect inference of typmod information for numeric expression where its being set to default when
Parallel worker is planned. Reason for the same is explained below:

When parallel query is enforced, Postgres engine planes Gather node on the top of top plan. For example, consider following query -
SELECT amount + 100 FROM overflow_test where id = 1

-- If parallel query is not enforced- 
Query Text: SELECT amount + 100 FROM overflow_test where id = 1
Index Scan using overflow_test_pkey on master_dbo.overflow_test  (cost=0.15..8.17 rows=1 width=32)
  Output: ((amount)::numeric + '100'::numeric)
  Index Cond: (overflow_test.id = 1)

-- If parallel query is enforced then Postgres will wrap the above plan under Gather node - 
Query Text: SELECT amount + 100 FROM overflow_test where id = 1                                                                                                                                                                                 
Gather  (cost=0.15..8.17 rows=1 width=32)                                                                                                                                                                                                       
  Output: (((amount)::numeric + '100'::numeric))                                                                                                                                                                                                
  Workers Planned: 1                                                                                                                                                                                                                            
  Single Copy: true                                                                                                                                                                                                                             
  ->  Index Scan using overflow_test_pkey on master_dbo.overflow_test  (cost=0.15..8.17 rows=1 width=32)                                                                                                                                        
        Output: ((amount)::numeric + '100'::numeric)                                                                                                                                                                                            
        Index Cond: (overflow_test.id = 1)    

When Postgres does this, it also modifies targetlist of gather node to refer to the tuples returned by its lefttree subplan or outerplan (Indexscan in this case). Hence, targetlist of gather node will contain only one tle in above case which is Var with attno = OUTER_VAR (it indicates reference to its outer plan) and vartypmod being -1.

Now, While sending metadata from TDS layer we use this typmod to deduce max_scale and precision for numeric. This would be set to default since vartypmod = -1 whereas actual data being computed may not fit in it hence it will run into an error while sending numeric response. Hence, hang or crash of end client software.

This commit fixes this issue by taking special Var into account while We extract the information of outer plan from planned stmt and use the same to get the original/referenced tle from outer plan.

This commit further improves the implementation of resolve_numeric_typmod_from_exp to handle reference to outer var correctly. It can now handle Append and MergeAppend node correctly which was required for following test case -

-- setup
create table t1 (a numeric(6,4), b numeric(6,3));
insert into t1 values (4, 16);
insert into t1 values (10.1234, 10.123);
insert into t1 values (1.2, 6);
insert into t1 values (NULL, 101.123);

-- test
select a from t1 UNION All
select b from t1;

For this UNION ALL expression, typmod was always computed to be -1 which is not right and it should (7,4) ideally. This commit introduces helped function called resolve_numeric_typmod_from_append_or_mergeappend to compute typmod for expression correctly.

Task: BABEL-4424, BABEL-4359
Signed-off-by: Dipesh Dhameliya [email protected]

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.

Deepesh125 and others added 9 commits November 16, 2023 03:18
1/ Update Github action scripts
2/ Create 15_5 upgrade test folder.
3/ Bump Babelfish version string in babelfish_version.h
4/ Add upgrade script babelfishpg_tsql--3.4.0--3.5.0.sql

Signed-off-by: Sai Rohan Basa <[email protected]>
…sh-for-postgresql#1998)


This change reimplements the datediff, datediff_big, and dateadd functions in C to improve performance by 65% compared to the original implementation.

Task: BABEL-4496
Signed-off-by: Jake Owen <[email protected]>
Test BABEL-4281 gives the memory usage with STATISTICS PROFILE. During
testing, this value can change, causing test failures. Switching to
SHOWPLAN_ALL will give a plan without outputing memory usage.

Signed-off-by: Walt Boettge <[email protected]>
…t exists". (babelfish-for-postgresql#2029)

Some tests listed in issue BABEL-4540 failing with error " 'tds_fdw' extension does not exists". This is because of
tds_fdw extension is not installed locally. However while running jdbc test check in GitHub we install 'tds_fdw'
extension in GitHub check. These tests are running now as expected in parallel query mode. Removed these tests from
ignore file. One of the test four-part-names-vu-verify still failing due to time-out. Included this test to appropriate group.

Task: BABEL-4540
Signed-off-by: Sandeep Kumawat <[email protected]>
@Deepesh125 Deepesh125 merged commit a695d88 into babelfish-for-postgresql:BABEL_3_X_DEV Nov 21, 2023
29 checks passed
@Deepesh125 Deepesh125 deleted the jira-babel-4424 branch November 21, 2023 12:54
Deepesh125 added a commit to amazon-aurora/babelfish_extensions that referenced this pull request Nov 21, 2023
…ed (babelfish-for-postgresql#1937)

This issue was mainly caused by incorrect inference of typmod information for numeric expression where its being set
to default when Parallel worker is planned. Reason for the same is explained below:

When parallel query is enforced, Postgres engine planes Gather node on the top of top plan. For example, consider
following query -

SELECT amount + 100 FROM overflow_test where id = 1

-- If parallel query is not enforced-
Query Text: SELECT amount + 100 FROM overflow_test where id = 1
Index Scan using overflow_test_pkey on master_dbo.overflow_test  (cost=0.15..8.17 rows=1 width=32)
  Output: ((amount)::numeric + '100'::numeric)
  Index Cond: (overflow_test.id = 1)

-- If parallel query is enforced then Postgres will wrap the above plan under Gather node -
Query Text: SELECT amount + 100 FROM overflow_test where id = 1
Gather  (cost=0.15..8.17 rows=1 width=32)
  Output: (((amount)::numeric + '100'::numeric))
  Workers Planned: 1
  Single Copy: true
  ->  Index Scan using overflow_test_pkey on master_dbo.overflow_test  (cost=0.15..8.17 rows=1 width=32)
        Output: ((amount)::numeric + '100'::numeric)
        Index Cond: (overflow_test.id = 1)

When Postgres does this, it also modifies targetlist of gather node to refer to the tuples returned by its lefttree subplan
or outerplan (Indexscan in this case). Hence, targetlist of gather node will contain only one tle in above case which is
Var with attno = OUTER_VAR (it indicates reference to its outer plan) and vartypmod being -1.

Now, While sending metadata from TDS layer we use this typmod to deduce max_scale and precision for numeric. This
would be set to default since vartypmod = -1 whereas actual data being computed may not fit in it hence it will run into
an error while sending numeric response. Hence, hang or crash of end client software.

This commit fixes this issue by taking special Var into account while We extract the information of outer plan from
planned stmt and use the same to get the original/referenced tle from outer plan.

This commit further improves the implementation of resolve_numeric_typmod_from_exp to handle reference to outer
var correctly. It can now handle Append and MergeAppend node correctly which was required for following test case -

-- setup
create table t1 (a numeric(6,4), b numeric(6,3));
insert into t1 values (4, 16);
insert into t1 values (10.1234, 10.123);
insert into t1 values (1.2, 6);
insert into t1 values (NULL, 101.123);

-- test
select a from t1 UNION All
select b from t1;
For this UNION ALL expression, typmod was always computed to be -1 which is not right and it should (7,4) ideally. This commit introduces helped function called resolve_numeric_typmod_from_append_or_mergeappend to compute typmod for expression correctly.

Task: BABEL-4424, BABEL-4359
Signed-off-by: Dipesh Dhameliya <[email protected]>
Deepesh125 added a commit that referenced this pull request Nov 21, 2023
…ed (#1937) (#2046)

This issue was mainly caused by incorrect inference of typmod information for numeric expression where its being set
to default when Parallel worker is planned. Reason for the same is explained below:

When parallel query is enforced, Postgres engine planes Gather node on the top of top plan. For example, consider
following query -

SELECT amount + 100 FROM overflow_test where id = 1

-- If parallel query is not enforced-
Query Text: SELECT amount + 100 FROM overflow_test where id = 1
Index Scan using overflow_test_pkey on master_dbo.overflow_test  (cost=0.15..8.17 rows=1 width=32)
  Output: ((amount)::numeric + '100'::numeric)
  Index Cond: (overflow_test.id = 1)

-- If parallel query is enforced then Postgres will wrap the above plan under Gather node -
Query Text: SELECT amount + 100 FROM overflow_test where id = 1
Gather  (cost=0.15..8.17 rows=1 width=32)
  Output: (((amount)::numeric + '100'::numeric))
  Workers Planned: 1
  Single Copy: true
  ->  Index Scan using overflow_test_pkey on master_dbo.overflow_test  (cost=0.15..8.17 rows=1 width=32)
        Output: ((amount)::numeric + '100'::numeric)
        Index Cond: (overflow_test.id = 1)

When Postgres does this, it also modifies targetlist of gather node to refer to the tuples returned by its lefttree subplan
or outerplan (Indexscan in this case). Hence, targetlist of gather node will contain only one tle in above case which is
Var with attno = OUTER_VAR (it indicates reference to its outer plan) and vartypmod being -1.

Now, While sending metadata from TDS layer we use this typmod to deduce max_scale and precision for numeric. This
would be set to default since vartypmod = -1 whereas actual data being computed may not fit in it hence it will run into
an error while sending numeric response. Hence, hang or crash of end client software.

This commit fixes this issue by taking special Var into account while We extract the information of outer plan from
planned stmt and use the same to get the original/referenced tle from outer plan.

This commit further improves the implementation of resolve_numeric_typmod_from_exp to handle reference to outer
var correctly. It can now handle Append and MergeAppend node correctly which was required for following test case -

-- setup
create table t1 (a numeric(6,4), b numeric(6,3));
insert into t1 values (4, 16);
insert into t1 values (10.1234, 10.123);
insert into t1 values (1.2, 6);
insert into t1 values (NULL, 101.123);

-- test
select a from t1 UNION All
select b from t1;
For this UNION ALL expression, typmod was always computed to be -1 which is not right and it should (7,4) ideally. This commit introduces helped function called resolve_numeric_typmod_from_append_or_mergeappend to compute typmod for expression correctly.

Task: BABEL-4424, BABEL-4359

Signed-off-by: Dipesh Dhameliya <[email protected]>
Sairakan pushed a commit to amazon-aurora/babelfish_extensions that referenced this pull request Dec 21, 2023
…ed (babelfish-for-postgresql#1937)

This issue was mainly caused by incorrect inference of typmod information for numeric expression where its being set
to default when Parallel worker is planned. Reason for the same is explained below:

When parallel query is enforced, Postgres engine planes Gather node on the top of top plan. For example, consider
following query -

SELECT amount + 100 FROM overflow_test where id = 1

-- If parallel query is not enforced- 
Query Text: SELECT amount + 100 FROM overflow_test where id = 1
Index Scan using overflow_test_pkey on master_dbo.overflow_test  (cost=0.15..8.17 rows=1 width=32)
  Output: ((amount)::numeric + '100'::numeric)
  Index Cond: (overflow_test.id = 1)

-- If parallel query is enforced then Postgres will wrap the above plan under Gather node - 
Query Text: SELECT amount + 100 FROM overflow_test where id = 1                                                                                                                                                                                 
Gather  (cost=0.15..8.17 rows=1 width=32)                                                                                                                                                                                                       
  Output: (((amount)::numeric + '100'::numeric))                                                                                                                                                                                                
  Workers Planned: 1                                                                                                                                                                                                                            
  Single Copy: true                                                                                                                                                                                                                             
  ->  Index Scan using overflow_test_pkey on master_dbo.overflow_test  (cost=0.15..8.17 rows=1 width=32)                                                                                                                                        
        Output: ((amount)::numeric + '100'::numeric)                                                                                                                                                                                            
        Index Cond: (overflow_test.id = 1)    

When Postgres does this, it also modifies targetlist of gather node to refer to the tuples returned by its lefttree subplan
or outerplan (Indexscan in this case). Hence, targetlist of gather node will contain only one tle in above case which is
Var with attno = OUTER_VAR (it indicates reference to its outer plan) and vartypmod being -1.

Now, While sending metadata from TDS layer we use this typmod to deduce max_scale and precision for numeric. This
would be set to default since vartypmod = -1 whereas actual data being computed may not fit in it hence it will run into
an error while sending numeric response. Hence, hang or crash of end client software.

This commit fixes this issue by taking special Var into account while We extract the information of outer plan from
planned stmt and use the same to get the original/referenced tle from outer plan.

This commit further improves the implementation of resolve_numeric_typmod_from_exp to handle reference to outer
var correctly. It can now handle Append and MergeAppend node correctly which was required for following test case -

-- setup
create table t1 (a numeric(6,4), b numeric(6,3));
insert into t1 values (4, 16);
insert into t1 values (10.1234, 10.123);
insert into t1 values (1.2, 6);
insert into t1 values (NULL, 101.123);

-- test
select a from t1 UNION All
select b from t1;
For this UNION ALL expression, typmod was always computed to be -1 which is not right and it should (7,4) ideally. This commit introduces helped function called resolve_numeric_typmod_from_append_or_mergeappend to compute typmod for expression correctly.

Task: BABEL-4424, BABEL-4359
Signed-off-by: Dipesh Dhameliya <[email protected]>
Sairakan pushed a commit to amazon-aurora/babelfish_extensions that referenced this pull request Dec 24, 2023
…ed (babelfish-for-postgresql#1937)

This issue was mainly caused by incorrect inference of typmod information for numeric expression where its being set
to default when Parallel worker is planned. Reason for the same is explained below:

When parallel query is enforced, Postgres engine planes Gather node on the top of top plan. For example, consider
following query -

SELECT amount + 100 FROM overflow_test where id = 1

-- If parallel query is not enforced- 
Query Text: SELECT amount + 100 FROM overflow_test where id = 1
Index Scan using overflow_test_pkey on master_dbo.overflow_test  (cost=0.15..8.17 rows=1 width=32)
  Output: ((amount)::numeric + '100'::numeric)
  Index Cond: (overflow_test.id = 1)

-- If parallel query is enforced then Postgres will wrap the above plan under Gather node - 
Query Text: SELECT amount + 100 FROM overflow_test where id = 1                                                                                                                                                                                 
Gather  (cost=0.15..8.17 rows=1 width=32)                                                                                                                                                                                                       
  Output: (((amount)::numeric + '100'::numeric))                                                                                                                                                                                                
  Workers Planned: 1                                                                                                                                                                                                                            
  Single Copy: true                                                                                                                                                                                                                             
  ->  Index Scan using overflow_test_pkey on master_dbo.overflow_test  (cost=0.15..8.17 rows=1 width=32)                                                                                                                                        
        Output: ((amount)::numeric + '100'::numeric)                                                                                                                                                                                            
        Index Cond: (overflow_test.id = 1)    

When Postgres does this, it also modifies targetlist of gather node to refer to the tuples returned by its lefttree subplan
or outerplan (Indexscan in this case). Hence, targetlist of gather node will contain only one tle in above case which is
Var with attno = OUTER_VAR (it indicates reference to its outer plan) and vartypmod being -1.

Now, While sending metadata from TDS layer we use this typmod to deduce max_scale and precision for numeric. This
would be set to default since vartypmod = -1 whereas actual data being computed may not fit in it hence it will run into
an error while sending numeric response. Hence, hang or crash of end client software.

This commit fixes this issue by taking special Var into account while We extract the information of outer plan from
planned stmt and use the same to get the original/referenced tle from outer plan.

This commit further improves the implementation of resolve_numeric_typmod_from_exp to handle reference to outer
var correctly. It can now handle Append and MergeAppend node correctly which was required for following test case -

-- setup
create table t1 (a numeric(6,4), b numeric(6,3));
insert into t1 values (4, 16);
insert into t1 values (10.1234, 10.123);
insert into t1 values (1.2, 6);
insert into t1 values (NULL, 101.123);

-- test
select a from t1 UNION All
select b from t1;
For this UNION ALL expression, typmod was always computed to be -1 which is not right and it should (7,4) ideally. This commit introduces helped function called resolve_numeric_typmod_from_append_or_mergeappend to compute typmod for expression correctly.

Task: BABEL-4424, BABEL-4359
Signed-off-by: Dipesh Dhameliya <[email protected]>
Sairakan pushed a commit to amazon-aurora/babelfish_extensions that referenced this pull request Dec 24, 2023
…ed (babelfish-for-postgresql#1937)

This issue was mainly caused by incorrect inference of typmod information for numeric expression where its being set
to default when Parallel worker is planned. Reason for the same is explained below:

When parallel query is enforced, Postgres engine planes Gather node on the top of top plan. For example, consider
following query -

SELECT amount + 100 FROM overflow_test where id = 1

-- If parallel query is not enforced- 
Query Text: SELECT amount + 100 FROM overflow_test where id = 1
Index Scan using overflow_test_pkey on master_dbo.overflow_test  (cost=0.15..8.17 rows=1 width=32)
  Output: ((amount)::numeric + '100'::numeric)
  Index Cond: (overflow_test.id = 1)

-- If parallel query is enforced then Postgres will wrap the above plan under Gather node - 
Query Text: SELECT amount + 100 FROM overflow_test where id = 1                                                                                                                                                                                 
Gather  (cost=0.15..8.17 rows=1 width=32)                                                                                                                                                                                                       
  Output: (((amount)::numeric + '100'::numeric))                                                                                                                                                                                                
  Workers Planned: 1                                                                                                                                                                                                                            
  Single Copy: true                                                                                                                                                                                                                             
  ->  Index Scan using overflow_test_pkey on master_dbo.overflow_test  (cost=0.15..8.17 rows=1 width=32)                                                                                                                                        
        Output: ((amount)::numeric + '100'::numeric)                                                                                                                                                                                            
        Index Cond: (overflow_test.id = 1)    

When Postgres does this, it also modifies targetlist of gather node to refer to the tuples returned by its lefttree subplan
or outerplan (Indexscan in this case). Hence, targetlist of gather node will contain only one tle in above case which is
Var with attno = OUTER_VAR (it indicates reference to its outer plan) and vartypmod being -1.

Now, While sending metadata from TDS layer we use this typmod to deduce max_scale and precision for numeric. This
would be set to default since vartypmod = -1 whereas actual data being computed may not fit in it hence it will run into
an error while sending numeric response. Hence, hang or crash of end client software.

This commit fixes this issue by taking special Var into account while We extract the information of outer plan from
planned stmt and use the same to get the original/referenced tle from outer plan.

This commit further improves the implementation of resolve_numeric_typmod_from_exp to handle reference to outer
var correctly. It can now handle Append and MergeAppend node correctly which was required for following test case -

-- setup
create table t1 (a numeric(6,4), b numeric(6,3));
insert into t1 values (4, 16);
insert into t1 values (10.1234, 10.123);
insert into t1 values (1.2, 6);
insert into t1 values (NULL, 101.123);

-- test
select a from t1 UNION All
select b from t1;
For this UNION ALL expression, typmod was always computed to be -1 which is not right and it should (7,4) ideally. This commit introduces helped function called resolve_numeric_typmod_from_append_or_mergeappend to compute typmod for expression correctly.

Task: BABEL-4424, BABEL-4359
Signed-off-by: Dipesh Dhameliya <[email protected]>
Sairakan pushed a commit to amazon-aurora/babelfish_extensions that referenced this pull request Dec 24, 2023
…ed (babelfish-for-postgresql#1937)

This issue was mainly caused by incorrect inference of typmod information for numeric expression where its being set
to default when Parallel worker is planned. Reason for the same is explained below:

When parallel query is enforced, Postgres engine planes Gather node on the top of top plan. For example, consider
following query -

SELECT amount + 100 FROM overflow_test where id = 1

-- If parallel query is not enforced- 
Query Text: SELECT amount + 100 FROM overflow_test where id = 1
Index Scan using overflow_test_pkey on master_dbo.overflow_test  (cost=0.15..8.17 rows=1 width=32)
  Output: ((amount)::numeric + '100'::numeric)
  Index Cond: (overflow_test.id = 1)

-- If parallel query is enforced then Postgres will wrap the above plan under Gather node - 
Query Text: SELECT amount + 100 FROM overflow_test where id = 1                                                                                                                                                                                 
Gather  (cost=0.15..8.17 rows=1 width=32)                                                                                                                                                                                                       
  Output: (((amount)::numeric + '100'::numeric))                                                                                                                                                                                                
  Workers Planned: 1                                                                                                                                                                                                                            
  Single Copy: true                                                                                                                                                                                                                             
  ->  Index Scan using overflow_test_pkey on master_dbo.overflow_test  (cost=0.15..8.17 rows=1 width=32)                                                                                                                                        
        Output: ((amount)::numeric + '100'::numeric)                                                                                                                                                                                            
        Index Cond: (overflow_test.id = 1)    

When Postgres does this, it also modifies targetlist of gather node to refer to the tuples returned by its lefttree subplan
or outerplan (Indexscan in this case). Hence, targetlist of gather node will contain only one tle in above case which is
Var with attno = OUTER_VAR (it indicates reference to its outer plan) and vartypmod being -1.

Now, While sending metadata from TDS layer we use this typmod to deduce max_scale and precision for numeric. This
would be set to default since vartypmod = -1 whereas actual data being computed may not fit in it hence it will run into
an error while sending numeric response. Hence, hang or crash of end client software.

This commit fixes this issue by taking special Var into account while We extract the information of outer plan from
planned stmt and use the same to get the original/referenced tle from outer plan.

This commit further improves the implementation of resolve_numeric_typmod_from_exp to handle reference to outer
var correctly. It can now handle Append and MergeAppend node correctly which was required for following test case -

-- setup
create table t1 (a numeric(6,4), b numeric(6,3));
insert into t1 values (4, 16);
insert into t1 values (10.1234, 10.123);
insert into t1 values (1.2, 6);
insert into t1 values (NULL, 101.123);

-- test
select a from t1 UNION All
select b from t1;
For this UNION ALL expression, typmod was always computed to be -1 which is not right and it should (7,4) ideally. This commit introduces helped function called resolve_numeric_typmod_from_append_or_mergeappend to compute typmod for expression correctly.

Task: BABEL-4424, BABEL-4359
Signed-off-by: Dipesh Dhameliya <[email protected]>
Sairakan pushed a commit to amazon-aurora/babelfish_extensions that referenced this pull request Dec 28, 2023
…ed (babelfish-for-postgresql#1937)

This issue was mainly caused by incorrect inference of typmod information for numeric expression where its being set
to default when Parallel worker is planned. Reason for the same is explained below:

When parallel query is enforced, Postgres engine planes Gather node on the top of top plan. For example, consider
following query -

SELECT amount + 100 FROM overflow_test where id = 1

-- If parallel query is not enforced- 
Query Text: SELECT amount + 100 FROM overflow_test where id = 1
Index Scan using overflow_test_pkey on master_dbo.overflow_test  (cost=0.15..8.17 rows=1 width=32)
  Output: ((amount)::numeric + '100'::numeric)
  Index Cond: (overflow_test.id = 1)

-- If parallel query is enforced then Postgres will wrap the above plan under Gather node - 
Query Text: SELECT amount + 100 FROM overflow_test where id = 1                                                                                                                                                                                 
Gather  (cost=0.15..8.17 rows=1 width=32)                                                                                                                                                                                                       
  Output: (((amount)::numeric + '100'::numeric))                                                                                                                                                                                                
  Workers Planned: 1                                                                                                                                                                                                                            
  Single Copy: true                                                                                                                                                                                                                             
  ->  Index Scan using overflow_test_pkey on master_dbo.overflow_test  (cost=0.15..8.17 rows=1 width=32)                                                                                                                                        
        Output: ((amount)::numeric + '100'::numeric)                                                                                                                                                                                            
        Index Cond: (overflow_test.id = 1)    

When Postgres does this, it also modifies targetlist of gather node to refer to the tuples returned by its lefttree subplan
or outerplan (Indexscan in this case). Hence, targetlist of gather node will contain only one tle in above case which is
Var with attno = OUTER_VAR (it indicates reference to its outer plan) and vartypmod being -1.

Now, While sending metadata from TDS layer we use this typmod to deduce max_scale and precision for numeric. This
would be set to default since vartypmod = -1 whereas actual data being computed may not fit in it hence it will run into
an error while sending numeric response. Hence, hang or crash of end client software.

This commit fixes this issue by taking special Var into account while We extract the information of outer plan from
planned stmt and use the same to get the original/referenced tle from outer plan.

This commit further improves the implementation of resolve_numeric_typmod_from_exp to handle reference to outer
var correctly. It can now handle Append and MergeAppend node correctly which was required for following test case -

-- setup
create table t1 (a numeric(6,4), b numeric(6,3));
insert into t1 values (4, 16);
insert into t1 values (10.1234, 10.123);
insert into t1 values (1.2, 6);
insert into t1 values (NULL, 101.123);

-- test
select a from t1 UNION All
select b from t1;
For this UNION ALL expression, typmod was always computed to be -1 which is not right and it should (7,4) ideally. This commit introduces helped function called resolve_numeric_typmod_from_append_or_mergeappend to compute typmod for expression correctly.

Task: BABEL-4424, BABEL-4359
Signed-off-by: Dipesh Dhameliya <[email protected]>
Sairakan pushed a commit to amazon-aurora/babelfish_extensions that referenced this pull request Dec 28, 2023
…ed (babelfish-for-postgresql#1937)

This issue was mainly caused by incorrect inference of typmod information for numeric expression where its being set
to default when Parallel worker is planned. Reason for the same is explained below:

When parallel query is enforced, Postgres engine planes Gather node on the top of top plan. For example, consider
following query -

SELECT amount + 100 FROM overflow_test where id = 1

-- If parallel query is not enforced- 
Query Text: SELECT amount + 100 FROM overflow_test where id = 1
Index Scan using overflow_test_pkey on master_dbo.overflow_test  (cost=0.15..8.17 rows=1 width=32)
  Output: ((amount)::numeric + '100'::numeric)
  Index Cond: (overflow_test.id = 1)

-- If parallel query is enforced then Postgres will wrap the above plan under Gather node - 
Query Text: SELECT amount + 100 FROM overflow_test where id = 1                                                                                                                                                                                 
Gather  (cost=0.15..8.17 rows=1 width=32)                                                                                                                                                                                                       
  Output: (((amount)::numeric + '100'::numeric))                                                                                                                                                                                                
  Workers Planned: 1                                                                                                                                                                                                                            
  Single Copy: true                                                                                                                                                                                                                             
  ->  Index Scan using overflow_test_pkey on master_dbo.overflow_test  (cost=0.15..8.17 rows=1 width=32)                                                                                                                                        
        Output: ((amount)::numeric + '100'::numeric)                                                                                                                                                                                            
        Index Cond: (overflow_test.id = 1)    

When Postgres does this, it also modifies targetlist of gather node to refer to the tuples returned by its lefttree subplan
or outerplan (Indexscan in this case). Hence, targetlist of gather node will contain only one tle in above case which is
Var with attno = OUTER_VAR (it indicates reference to its outer plan) and vartypmod being -1.

Now, While sending metadata from TDS layer we use this typmod to deduce max_scale and precision for numeric. This
would be set to default since vartypmod = -1 whereas actual data being computed may not fit in it hence it will run into
an error while sending numeric response. Hence, hang or crash of end client software.

This commit fixes this issue by taking special Var into account while We extract the information of outer plan from
planned stmt and use the same to get the original/referenced tle from outer plan.

This commit further improves the implementation of resolve_numeric_typmod_from_exp to handle reference to outer
var correctly. It can now handle Append and MergeAppend node correctly which was required for following test case -

-- setup
create table t1 (a numeric(6,4), b numeric(6,3));
insert into t1 values (4, 16);
insert into t1 values (10.1234, 10.123);
insert into t1 values (1.2, 6);
insert into t1 values (NULL, 101.123);

-- test
select a from t1 UNION All
select b from t1;
For this UNION ALL expression, typmod was always computed to be -1 which is not right and it should (7,4) ideally. This commit introduces helped function called resolve_numeric_typmod_from_append_or_mergeappend to compute typmod for expression correctly.

Task: BABEL-4424, BABEL-4359
Signed-off-by: Dipesh Dhameliya <[email protected]>
priyansx pushed a commit that referenced this pull request Dec 29, 2023
…ed (#1937)

This issue was mainly caused by incorrect inference of typmod information for numeric expression where its being set
to default when Parallel worker is planned. Reason for the same is explained below:

When parallel query is enforced, Postgres engine planes Gather node on the top of top plan. For example, consider
following query -

SELECT amount + 100 FROM overflow_test where id = 1

-- If parallel query is not enforced- 
Query Text: SELECT amount + 100 FROM overflow_test where id = 1
Index Scan using overflow_test_pkey on master_dbo.overflow_test  (cost=0.15..8.17 rows=1 width=32)
  Output: ((amount)::numeric + '100'::numeric)
  Index Cond: (overflow_test.id = 1)

-- If parallel query is enforced then Postgres will wrap the above plan under Gather node - 
Query Text: SELECT amount + 100 FROM overflow_test where id = 1                                                                                                                                                                                 
Gather  (cost=0.15..8.17 rows=1 width=32)                                                                                                                                                                                                       
  Output: (((amount)::numeric + '100'::numeric))                                                                                                                                                                                                
  Workers Planned: 1                                                                                                                                                                                                                            
  Single Copy: true                                                                                                                                                                                                                             
  ->  Index Scan using overflow_test_pkey on master_dbo.overflow_test  (cost=0.15..8.17 rows=1 width=32)                                                                                                                                        
        Output: ((amount)::numeric + '100'::numeric)                                                                                                                                                                                            
        Index Cond: (overflow_test.id = 1)    

When Postgres does this, it also modifies targetlist of gather node to refer to the tuples returned by its lefttree subplan
or outerplan (Indexscan in this case). Hence, targetlist of gather node will contain only one tle in above case which is
Var with attno = OUTER_VAR (it indicates reference to its outer plan) and vartypmod being -1.

Now, While sending metadata from TDS layer we use this typmod to deduce max_scale and precision for numeric. This
would be set to default since vartypmod = -1 whereas actual data being computed may not fit in it hence it will run into
an error while sending numeric response. Hence, hang or crash of end client software.

This commit fixes this issue by taking special Var into account while We extract the information of outer plan from
planned stmt and use the same to get the original/referenced tle from outer plan.

This commit further improves the implementation of resolve_numeric_typmod_from_exp to handle reference to outer
var correctly. It can now handle Append and MergeAppend node correctly which was required for following test case -

-- setup
create table t1 (a numeric(6,4), b numeric(6,3));
insert into t1 values (4, 16);
insert into t1 values (10.1234, 10.123);
insert into t1 values (1.2, 6);
insert into t1 values (NULL, 101.123);

-- test
select a from t1 UNION All
select b from t1;
For this UNION ALL expression, typmod was always computed to be -1 which is not right and it should (7,4) ideally. This commit introduces helped function called resolve_numeric_typmod_from_append_or_mergeappend to compute typmod for expression correctly.

Task: BABEL-4424, BABEL-4359
Signed-off-by: Dipesh Dhameliya <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants