Make local variable to evaluate dynamically (only if it appears on TargetList) #3011

Deepesh125 · 2024-10-08T09:09:25Z

Description

Currently, declared local variables are assumed to be static by analyser and hence it is replaced with const node by optimiser. Currently, the parameter value will be supplied through pltsql_param_fetch(..) which is setup during pltsql_estate_setup as part of ParamListInfo setup. This hook is purely used during planning phase and is not used during query execution.

If parameter value is required during execution then values will be read from estate->ndatums through pltsql_param_eval_var callback for EEOP_PARAM_CALLBACK step. This estate->ndatums setup during pltsql_estate_setup and values are copied through copy_pltsql_datums(…). And currently, any assignments to declare variables are implemented using "select target" which will be executed at the end of executor. Hence, any use of variables will read original (value copied during pltsql_estate_setup(...)) and it would not reflect any dynamic change in the value of any variable.

In order to make this behaviour dynamic, this commit introduces internal function sys.pltsql_assign_var(dno, expr). Argument dno is item number of declared variable and expr could be anything whose value will be assigned to declared variable pointed by dno during execution. We will use this function to rewrite any variables assignment operation during ANTLR parsing as stated in following example,

select @a = expr

Will be written to

select @a = sys.pltsql_assign_var(dno, expr)

Internally, sys.pltsql_assign_var invokes exec_assign_value(…) to assign appropriate value directly to variable by updating estate->ndatums array and hence any subsequent read will read latest value.

Engine PR: babelfish-for-postgresql/postgresql_modified_for_babelfish#469
Task: BABEL-5325
Signed-off-by: Dipesh Dhameliya dddhamel@amazon.com

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.

… which are part of assignement expression

…-5188

coveralls · 2024-10-15T06:25:24Z

Pull Request Test Coverage Report for Build 11839208255

Details

116 of 119 (97.48%) changed or added relevant lines in 4 files are covered.
14 unchanged lines in 2 files lost coverage.
Overall coverage increased (+0.02%) to 74.742%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
contrib/babelfishpg_tsql/src/tsqlIface.cpp	86	89	96.63%

Files with Coverage Reduction	New Missed Lines	%
contrib/babelfishpg_tsql/src/tsqlIface.cpp	4	90.29%
contrib/babelfishpg_tsql/src/hooks.c	10	83.51%

Totals
Change from base Build 11831695974:	0.02%
Covered Lines:	45715
Relevant Lines:	61164

💛 - Coveralls

…-5188

test/JDBC/expected/BABEL-2843.out

test/JDBC/expected/BABEL-1181.out

contrib/babelfishpg_tsql/src/pl_exec.c

contrib/babelfishpg_tsql/src/tsqlIface.cpp

test/JDBC/expected/Test-spatial-functions-vu-verify.out

test/JDBC/expected/test_dynamic_local_vars.out

…-5188

contrib/babelfishpg_tsql/src/pl_exec.c

test/JDBC/input/test_dynamic_local_vars.sql

…ext of expression being processed (#469) Extend the ability of planner_node_transformer_hook to store the context of expression being process. This saved context will be used during planning to transform declared variable either as a Param (if it appears in TargetList) or as a const (all the other places). Extension: babelfish-for-postgresql/babelfish_extensions#3011 Task: BABEL-5188 Signed-off-by: Dipesh Dhameliya <dddhamel@amazon.com>

…ext of expression being processed (babelfish-for-postgresql#469) Extend the ability of planner_node_transformer_hook to store the context of expression being process. This saved context will be used during planning to transform declared variable either as a Param (if it appears in TargetList) or as a const (all the other places). Extension: babelfish-for-postgresql/babelfish_extensions#3011 Task: BABEL-5188 Signed-off-by: Dipesh Dhameliya <dddhamel@amazon.com>

@A

…rgetList) (babelfish-for-postgresql#3011) Currently, declared local variables are assumed to be static by analyser and hence it is replaced with const node by optimiser. Currently, the parameter value will be supplied through pltsql_param_fetch(..) which is setup during pltsql_estate_setup as part of ParamListInfo setup. This hook is purely used during planning phase and is not used during query execution. If parameter value is required during execution then values will be read from estate->ndatums through pltsql_param_eval_var callback for EEOP_PARAM_CALLBACK step. This estate->ndatums setup during pltsql_estate_setup and values are copied through copy_pltsql_datums(…). And currently, any assignments to declare variables are implemented using "select target" which will be executed at the end of executor. Hence, any use of variables will read original (value copied during pltsql_estate_setup(...)) and it would not reflect any dynamic change in the value of any variable. In order to make this behaviour dynamic, this commit introduces internal function sys.pltsql_assign_var(dno, expr). Argument dno is item number of declared variable and expr could be anything whose value will be assigned to declared variable pointed by dno during execution. We will use this function to rewrite any variables assignment operation during ANTLR parsing as stated in following example, select @A = expr Will be written to select @A = sys.pltsql_assign_var(dno, expr) Internally, sys.pltsql_assign_var invokes exec_assign_value(…) to assign appropriate value directly to variable by updating estate->ndatums array and hence any subsequent read will read latest value. Engine PR: babelfish-for-postgresql/postgresql_modified_for_babelfish#469 Task: BABEL-5325 Signed-off-by: Dipesh Dhameliya <dddhamel@amazon.com>

@A

…rgetList) (babelfish-for-postgresql#3011) Currently, declared local variables are assumed to be static by analyser and hence it is replaced with const node by optimiser. Currently, the parameter value will be supplied through pltsql_param_fetch(..) which is setup during pltsql_estate_setup as part of ParamListInfo setup. This hook is purely used during planning phase and is not used during query execution. If parameter value is required during execution then values will be read from estate->ndatums through pltsql_param_eval_var callback for EEOP_PARAM_CALLBACK step. This estate->ndatums setup during pltsql_estate_setup and values are copied through copy_pltsql_datums(…). And currently, any assignments to declare variables are implemented using "select target" which will be executed at the end of executor. Hence, any use of variables will read original (value copied during pltsql_estate_setup(...)) and it would not reflect any dynamic change in the value of any variable. In order to make this behaviour dynamic, this commit introduces internal function sys.pltsql_assign_var(dno, expr). Argument dno is item number of declared variable and expr could be anything whose value will be assigned to declared variable pointed by dno during execution. We will use this function to rewrite any variables assignment operation during ANTLR parsing as stated in following example, select @A = expr Will be written to select @A = sys.pltsql_assign_var(dno, expr) Internally, sys.pltsql_assign_var invokes exec_assign_value(…) to assign appropriate value directly to variable by updating estate->ndatums array and hence any subsequent read will read latest value. Engine PR: babelfish-for-postgresql/postgresql_modified_for_babelfish#469 Task: BABEL-5325 Signed-off-by: Dipesh Dhameliya <dddhamel@amazon.com>

@A

…rgetList) (babelfish-for-postgresql#3011) Currently, declared local variables are assumed to be static by analyser and hence it is replaced with const node by optimiser. Currently, the parameter value will be supplied through pltsql_param_fetch(..) which is setup during pltsql_estate_setup as part of ParamListInfo setup. This hook is purely used during planning phase and is not used during query execution. If parameter value is required during execution then values will be read from estate->ndatums through pltsql_param_eval_var callback for EEOP_PARAM_CALLBACK step. This estate->ndatums setup during pltsql_estate_setup and values are copied through copy_pltsql_datums(…). And currently, any assignments to declare variables are implemented using "select target" which will be executed at the end of executor. Hence, any use of variables will read original (value copied during pltsql_estate_setup(...)) and it would not reflect any dynamic change in the value of any variable. In order to make this behaviour dynamic, this commit introduces internal function sys.pltsql_assign_var(dno, expr). Argument dno is item number of declared variable and expr could be anything whose value will be assigned to declared variable pointed by dno during execution. We will use this function to rewrite any variables assignment operation during ANTLR parsing as stated in following example, select @A = expr Will be written to select @A = sys.pltsql_assign_var(dno, expr) Internally, sys.pltsql_assign_var invokes exec_assign_value(…) to assign appropriate value directly to variable by updating estate->ndatums array and hence any subsequent read will read latest value. Engine PR: babelfish-for-postgresql/postgresql_modified_for_babelfish#469 Task: BABEL-5325 Signed-off-by: Dipesh Dhameliya <dddhamel@amazon.com>

…ext of expression being processed (babelfish-for-postgresql#469) Extend the ability of planner_node_transformer_hook to store the context of expression being process. This saved context will be used during planning to transform declared variable either as a Param (if it appears in TargetList) or as a const (all the other places). Extension: babelfish-for-postgresql/babelfish_extensions#3011 Task: BABEL-5188 Signed-off-by: Dipesh Dhameliya <dddhamel@amazon.com>

@A

…rgetList) (babelfish-for-postgresql#3011) Currently, declared local variables are assumed to be static by analyser and hence it is replaced with const node by optimiser. Currently, the parameter value will be supplied through pltsql_param_fetch(..) which is setup during pltsql_estate_setup as part of ParamListInfo setup. This hook is purely used during planning phase and is not used during query execution. If parameter value is required during execution then values will be read from estate->ndatums through pltsql_param_eval_var callback for EEOP_PARAM_CALLBACK step. This estate->ndatums setup during pltsql_estate_setup and values are copied through copy_pltsql_datums(…). And currently, any assignments to declare variables are implemented using "select target" which will be executed at the end of executor. Hence, any use of variables will read original (value copied during pltsql_estate_setup(...)) and it would not reflect any dynamic change in the value of any variable. In order to make this behaviour dynamic, this commit introduces internal function sys.pltsql_assign_var(dno, expr). Argument dno is item number of declared variable and expr could be anything whose value will be assigned to declared variable pointed by dno during execution. We will use this function to rewrite any variables assignment operation during ANTLR parsing as stated in following example, select @A = expr Will be written to select @A = sys.pltsql_assign_var(dno, expr) Internally, sys.pltsql_assign_var invokes exec_assign_value(…) to assign appropriate value directly to variable by updating estate->ndatums array and hence any subsequent read will read latest value. Engine PR: babelfish-for-postgresql/postgresql_modified_for_babelfish#469 Task: BABEL-5325 Signed-off-by: Dipesh Dhameliya <dddhamel@amazon.com>

…ext of expression being processed (#469) Extend the ability of planner_node_transformer_hook to store the context of expression being process. This saved context will be used during planning to transform declared variable either as a Param (if it appears in TargetList) or as a const (all the other places). Extension: babelfish-for-postgresql/babelfish_extensions#3011 Task: BABEL-5188 Signed-off-by: Dipesh Dhameliya <dddhamel@amazon.com>

@A

…rgetList) (#3011) Currently, declared local variables are assumed to be static by analyser and hence it is replaced with const node by optimiser. Currently, the parameter value will be supplied through pltsql_param_fetch(..) which is setup during pltsql_estate_setup as part of ParamListInfo setup. This hook is purely used during planning phase and is not used during query execution. If parameter value is required during execution then values will be read from estate->ndatums through pltsql_param_eval_var callback for EEOP_PARAM_CALLBACK step. This estate->ndatums setup during pltsql_estate_setup and values are copied through copy_pltsql_datums(…). And currently, any assignments to declare variables are implemented using "select target" which will be executed at the end of executor. Hence, any use of variables will read original (value copied during pltsql_estate_setup(...)) and it would not reflect any dynamic change in the value of any variable. In order to make this behaviour dynamic, this commit introduces internal function sys.pltsql_assign_var(dno, expr). Argument dno is item number of declared variable and expr could be anything whose value will be assigned to declared variable pointed by dno during execution. We will use this function to rewrite any variables assignment operation during ANTLR parsing as stated in following example, select @A = expr Will be written to select @A = sys.pltsql_assign_var(dno, expr) Internally, sys.pltsql_assign_var invokes exec_assign_value(…) to assign appropriate value directly to variable by updating estate->ndatums array and hence any subsequent read will read latest value. Engine PR: babelfish-for-postgresql/postgresql_modified_for_babelfish#469 Task: BABEL-5325 Signed-off-by: Dipesh Dhameliya <dddhamel@amazon.com>

Dipesh Dhameliya and others added 14 commits October 8, 2024 09:08

Make local variable to evaluate dynamically

bc055df

Empty-Commit

6c357cc

Do not re-write local_id assignment for batch level statements

842a74c

Handle param for calculating numeric typmod and do not quote local_id…

1897646

… which are part of assignement expression

Handle map.erase() more cleanly

f30d58e

Properly cast value of expression, handle NULL value clearly

cb28d37

fix seg fault and fix few expected output

ef1e9f3

Minor fix to handle re-written segments before handling select list

3b818fa

Better way of accommodating already re-written expressions

d6662d8

Common handling of assignment expression

c99513f

Handling max typmod correctly

6763d6d

Merge branch 'babelfish-for-postgresql:BABEL_4_X_DEV' into jira-babel…

eb290f2

…-5188

Update few expected file

3a189b2

Final expected files changes

a70ff8f

Dipesh Dhameliya added 9 commits October 15, 2024 07:52

Parallel query expected files changes

cd499da

Upgrade sql changes

0d49d39

Some more expected file changes

fe9e739

Some more expected file changes

6836672

Some more expected file changes

989c98f

Some more expected file changes

3ddcbde

New test cases

1f76dd5

Update test cases

3c19ac3

Patch cleanup

7dacb86

Deepesh125 requested a review from KushaalShroff October 16, 2024 13:36

Dipesh Dhameliya and others added 3 commits October 18, 2024 06:47

Just one additional tests

0a7bd12

Merge branch 'babelfish-for-postgresql:BABEL_4_X_DEV' into jira-babel…

97829bc

…-5188

Empty-Commit

806ba58

KushaalShroff requested changes Oct 18, 2024

View reviewed changes

Fixed few expected output

c46fef1

Dipesh Dhameliya and others added 8 commits November 11, 2024 10:23

Update expected files

3949316

Update expected files

66a27b3

Merge branch 'babelfish-for-postgresql:BABEL_4_X_DEV' into jira-babel…

bfc913d

…-5188

Update upgrade scripts

fdb3e24

Empty-Commit

926c9a9

Empty-Commit

a7e66ed

Remove unnecessary changes

8be61f7

Remove unnecessary changes

58faa2b

Deepesh125 changed the title ~~Make local variable to evaluate dynamically~~ Make local variable to evaluate dynamically (only if it appears on TargetList) Nov 12, 2024

Deepesh125 mentioned this pull request Nov 12, 2024

Extend the ability of planner_node_transformer_hook to store the context of expression being process babelfish-for-postgresql/postgresql_modified_for_babelfish#469

Merged

1 task

KushaalShroff requested changes Nov 13, 2024

View reviewed changes

contrib/babelfishpg_tsql/src/pl_exec.c Show resolved Hide resolved

contrib/babelfishpg_tsql/src/pl_exec.c Outdated Show resolved Hide resolved

contrib/babelfishpg_tsql/src/pl_exec.c Show resolved Hide resolved

test/JDBC/input/test_dynamic_local_vars.sql Show resolved Hide resolved

Dipesh Dhameliya and others added 7 commits November 13, 2024 09:00

Minor comment address

8e64705

Diff expected file for parallel query testing

0cb4a12

Merge branch 'BABEL_4_X_DEV' into jira-babel-5188

21ea498

Merge branch 'BABEL_4_X_DEV' into jira-babel-5188

efc1963

update expected file

81fbfe9

Fixed for @@

604a98c

variable case

052eb51

KushaalShroff approved these changes Nov 19, 2024

View reviewed changes

Deepesh125 merged commit 6be12f1 into babelfish-for-postgresql:BABEL_4_X_DEV Nov 19, 2024
46 checks passed

Deepesh125 deleted the jira-babel-5188 branch November 19, 2024 08:08

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make local variable to evaluate dynamically (only if it appears on TargetList) #3011

Make local variable to evaluate dynamically (only if it appears on TargetList) #3011

Deepesh125 commented Oct 8, 2024 •

edited

Loading

coveralls commented Oct 15, 2024 •

edited

Loading

Make local variable to evaluate dynamically (only if it appears on TargetList) #3011

Make local variable to evaluate dynamically (only if it appears on TargetList) #3011

Conversation

Deepesh125 commented Oct 8, 2024 • edited Loading

Description

Check List

coveralls commented Oct 15, 2024 • edited Loading

Pull Request Test Coverage Report for Build 11839208255

Details

💛 - Coveralls

Deepesh125 commented Oct 8, 2024 •

edited

Loading

coveralls commented Oct 15, 2024 •

edited

Loading