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

Merge 4_X changes back to 5_X #3149

Merged

Conversation

sumitj824
Copy link
Contributor

@sumitj824 sumitj824 commented Nov 22, 2024

Description

This is a cherry-pick PR which merges newer 4_X engine commits into 5_X.

Engine PR: babelfish-for-postgresql/postgresql_modified_for_babelfish#479

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.

@forestkeeper
Copy link
Contributor

Will you make sure we merge it using the original commit id and commit msg ? Not like squash and merge into just a single commit.

@forestkeeper
Copy link
Contributor

I think you might need to sync up with @kuntalghosh since last time from 3X to 4X we also had a discussion similar.

anju15bharti and others added 8 commits November 25, 2024 05:41
…lfish-for-postgresql#3073)

Valid names of user-defined T-SQL variables and/or parameters are @@Myvar, @Myvar#, @#myvar, etc. and indeed we see customer T-SQL applications using such names.
Until now, such names have raised errors in the backend because we can only handle variable/parameter names with one leading @ and without # characters.
This fix adds support by enclosing such variable names in square bracket delimiters; when using double quotes as delimiters (as is currently done in most places for regular-named variables), a query like select @@v will produce the string '@@v' instead of the value of variable @@v.
The fix is partly done through ANTLR rewriting and partly at execution time elsewhere in the babelfish_tsql codeline. Things are complicated by the fact that the ANTLR rewriting logic is very fragmented, as well as that for some variables reference contexts inside the body of a procedure or trigger, rewriting must be postponed until the body is executed. This means there are multiple code paths where these variable names must be intercepted.
This fix also addresses variable names in some contexts (like a cursor variable) that previously could not be longer than 63 characters.
This fix also solves the previous issue that a variable could be declared with a name matching an internal sys function, but was resolved as the intern function instead of the variable, i.e. DECLARE @@rand INT=123 SELECT @@rand would return a random number instead of 123.

Issues Resolved: 
BABEL-2481 Parameter declarations containing # characters not handled correctly
BABEL-476 Support local variables/parameters with multiple '@' characters
BABEL-5384 User-defined @@variable should not be mapped to sys function

Signed-off-by: Rob Verschoor [email protected]
Update the antlr version to 4.13.2. This dramatically improves ANTLR parsing performance for certain queries, reducing their execution times by up to 40%.

Note that this will require developers to have Java 11+ installed in order to develop locally.

Also update the github actions to make it so that previous versions of Babelfish will be compiled using the original antlr-4.9.3 version. This is purely a fix for testing.

Task: BABEL-5371
Signed-off-by: Jason Teng <[email protected]>
…postgresql#3121)

This fixes the long-standing bug that a reference to a double-quoted empty string, when located in the body of a procedure, function or trigger, raises an error even though QUOTED_IDENTIFIER is OFF. This is due to the PG backend interpreting any double-quoted string as an identifier, whose length cannot be zero. It is fixed by replacing "" by '' at the ANTLR stage.

Signed-off-by: Rob Verschoor <[email protected]>
…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 <[email protected]>
…tatements (babelfish-for-postgresql#3131)

This commit resolves the syntax error issue that occurs when using '[primary]' as a column name in DML and DDL statements.

Task: BABEL-5422
Issue: babelfish-for-postgresql#3127

Signed-off-by: Sumit Jaiswal [email protected]
…tgresql#3126)

T-SQL does not allow calling a stored procedure in a user-defined SQL function and will raise an execution-time error when such an execution is attempted. Babelfish currently enforces this for regular stored procedures, but not for system stored procedures (those whose names start with sp_). Some Babelfish releases ago, executing a system stored procedure in a function would crash the PG server, though in the latest codeline this may just raise an error like InstrStartNode called twice in a row.
The reason is simply that the check in exec_stmt_exec() does not cover the case of a call to a system stored procedure, therefore this fix adds this check to exec_stmt_exec_sp() as well.

NB. This is an execution-time check, presumably because SQL Server allows so-called extended stored procedures (named xp_something) to be called from functions, and these can also be called through a variable -- though neither variable procedure names nor extended stored procedures are currently supported in Babelfish.

Task: BABEL-1730

Signed-off-by: Rob Verschoor [email protected]
…for-postgresql#3096)

Some GUCs had an issue with being reset at time of TDSResetConnection:

1. Explain GUCs, they were not getting setting reset after TDSResetConnection. To fix this we need to reset it at time TDSResetConnection AND also at time of sys.sp_reset_connection execution(which would otherwise have given explain output).
2. ANSI_DEFAULTS and IMPLICIT_TRANSACTION related GUCs. These were getting set when driver sends some TDS flags. But after ResetConnection they were being rolled-back. To avoid this we implement TdsResetLoginFlags a wrapper to Re-ProcessLoginFlags.
3. We restrict the setting and re-setting of important GUCs like "babelfishpg_tsql.migration_mode" and "role", since once set, cannot be reset.

Issues Resolved:
BABEL-5280, BABEL-5276, BABEL-5278
rishabhtanwar29 and others added 5 commits November 25, 2024 07:32
babelfish-for-postgresql#3088)

Previously, a guest users always remain member of a login even if login has a mapped user in a particular database. Due to this any database user is able to access the objects which are accessible to guest user which is undesirable.

Fixed this issue by only keeping one user member of a login at a time, which means, guest will be a member of login only if there is no mapped user to that login otherwise only that mapped user will be a member of login.

Task: BABEL-5389

Co-authored-by: Rishabh Tanwar <[email protected]>
Signed-off-by: Shalini Lohia <[email protected]>
… variables (babelfish-for-postgresql#3028)

currently, Any variable being set as part of UPDATE...SET... command is being re-written using RETURNING clause. But there was an issue where any dynamic update on variable was not visible to subsequent statement execution. That issue is fixed with 6be12f1 and this commit aims to fix the same issue when dynamic variables are being updated as part of UPDATE...SET... command. In order to make variables updated dynamically, assignment statement is re-written using sys.pltsql_assign_var(dno, expr). For example,

SET @A+=expr, col=expr2

will be re-written to

SET col=expr2 ... RETURNING sys.pltsql_assign_var(dno, "@var" + cast((expr) as type))

Task: BABEL-5188
Signed-off-by: Dipesh Dhameliya <[email protected]>
…or-postgresql#3142)

Adds SET QUOTED_IDENTIFIER OFF to the prepare script of test case unary_plus_op_string-vu-prepare.sql. This will make the test run correctly even when it is executed in a context where QUOTED_IDENTIFIER happens to be ON.

Signed-off-by: Rob Verschoor [email protected]
…or-postgresql#3151)

* Update versions from 14.15, 15.10 and 16.5 to 14.16, 15.11 and 16.6 respectively in upgrade-test-configuration.
* Rename 14_14/schedule file to 14_16/schedule, 15_9/schedule file to 15_11/schedule and 16_5/schedule file to 16_6/schedule.
* Build ANTLR from 16.7 onwards.

Task: OSS-ONLY
Signed-off-by: Rishabh Tanwar <[email protected]>
@rishabhtanwar29 rishabhtanwar29 force-pushed the merge_4x branch 2 times, most recently from 58e2a4e to 5a82d06 Compare November 25, 2024 07:37
@shardgupta shardgupta merged commit 98c2dfc into babelfish-for-postgresql:BABEL_5_X_DEV Nov 25, 2024
42 checks passed
@shardgupta shardgupta deleted the merge_4x branch November 25, 2024 11:32
roshan0708 added a commit to amazon-aurora/babelfish_extensions that referenced this pull request Nov 25, 2024
@rishabhtanwar29 rishabhtanwar29 restored the merge_4x branch November 25, 2024 12:28
@shardgupta shardgupta deleted the merge_4x branch November 25, 2024 17:02
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.

9 participants