Fix typmod for sys.binary datatype #2037

tanscorpio7 · 2023-11-20T08:43:42Z

Description

sys.binary datatype is created as a domain over sys.bbf_binary. In babelfish we assume hex code as varbinary which must then be casted to binary. For casting we fetch rule from hashmap after coercing both source and target to base type.
So binary --> bbf_binary && varbinary-->bbf_varbinary
This cast is currently defined as binary coercible, that is NO modification needed.

babelfish_extensions/contrib/babelfishpg_tsql/src/pltsql_coerce.c

Line 114 in c57e8bc

    
           {TSQL_CAST_WITHOUT_FUNC_ENTRY, "sys", "bbf_varbinary", "sys", "bbf_binary", NULL, 'i', 'b'},

We fix it by adding proper function for varbinary --> binary cast

-> The default length for local variables defined for binary and varbinary should be 1
DECLARE @A binary should be equivalent to DECLARE @A binary(1)
DECLARE @A varbinary should be equivalent to DECLARE @A varbinary(1)

->Problems during dump and restore done during upgrade.
The new function created for the cast may not exists in the source version and the ugrade scripts are run only after pg dump and restore carried out during version upgrade.
So ultimately during dump and restore no cast exists for varbinary to binary, which is a necessary requirement or upgrade will fail with error :
"pg_restore: error: could not execute query: ERROR: argument of DEFAULT must be type sys."binary", not type sys.varbinary".
if we are dumping a procedure with sys.binary arg and a default value

-> binary to varbinary
we do not need to modify binary to varbinary cast since we have a type modifer defined for base type of sys.varbinary
i.e. CAST(sys.bbf_varbiniary as sys.bbf_varbiniary).

Issues Resolved

[BABEL-4544]

Sign Off

Signed-off-by: Tanzeel Khan [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.

test/JDBC/expected/TestBinary-before-14_11-or-15_6-vu-prepare.out

test/JDBC/expected/BABEL-3166-before-14_11-or-15_6-vu-verify.out

Deepesh125 · 2023-12-01T13:18:40Z

contrib/babelfishpg_tsql/src/pltsql_coerce.c

+						   /*
+							* varbinary to binary implicit type cast without function should be allowed
+							* during upgrade since the cast function might not exists in older version
+							*/ 
+							if (babelfish_dump_restore && ((*common_utility_plugin_ptr->is_tsql_varbinary_datatype) (castsource) 
+								&& (*common_utility_plugin_ptr->is_tsql_binary_datatype) (casttarget)))
+								entry->castfunc = 0;
+


can you describe use case and explain why this changes are required? And what error or differences you will see if we do not make this change?

Until now varbinary to binary cast was implicitly done without any function call, i.e. binary coercible.
Now we are moving to cast with function. i.e. sys.varbinarybinary

If we do not make this change,
during pg_upgrade init_tsql_coerce_hash_tab will not be able to find the function sys.varbinarybinary for varbinary to binary cast because it may not exists in the source version and will not be created until we run our upgrade scripts (happens after dump restore in version upgrade).
Ultimately we end up with no cast for varbinary --> binary during dump and restore.

A failure case for this situation is default value for a sys.binary in a procedure
proc( id sys.binary default value 0x42)
pre dump --> 0x42 is sys.binary
psot dump --> 0x42 is sys.varbinary (since hex code is always assumed as varbinary first and pg could not find cast to binary type)

"pg_restore: error: could not execute query: ERROR: argument of DEFAULT must be type sys."binary", not type sys.varbinary"

test/JDBC/expected/BABEL_4544.out

.github/workflows/upgrade-test.yml

contrib/babelfishpg_tsql/sql/upgrades/babelfishpg_tsql--3.4.0--3.5.0.sql

contrib/babelfishpg_tsql/src/pltsql_coerce.c

test/JDBC/expected/BABEL-3166-before-14_11-or-15_6-vu-verify.out

contrib/babelfishpg_tsql/src/pltsql_coerce.c

contrib/babelfishpg_tsql/src/tsqlIface.cpp

test/JDBC/expected/BABEL_4544.out

Deepesh125 · 2023-12-04T14:12:48Z

test/JDBC/expected/TestBinary.out

@@ -49,7 +49,7 @@ INSERT INTO BINARY_dt(a, b) values (1234, 12345);
 prepst#!# INSERT INTO BINARY_dt(a, b) values(?, ?) #!#binary|-|a|-|1234#!#varbinary|-|b|-|12345
 ~~ROW COUNT: 1~~

-prepst#!#exec#!#binary|-|a|-|123456789#!#varbinary|-|b|-|12345
+prepst#!#exec#!#binary|-|a|-|12345678#!#varbinary|-|b|-|12345


why this change?

Otherwise we will get error instead, since the procedure argument is defined as binary(8)
binary does not allow implicit truncation

Deepesh125 · 2023-12-04T14:13:19Z

test/JDBC/expected/babel_datatype.out

 insert into testing6 values (cast('ab' as varbinary));
 GO
-~~ROW COUNT: 1~~
+~~ERROR (Code: 33557097)~~
+
+~~ERROR (Message: String or binary data would be truncated.
+The statement has been terminated.)~~


what was the default typmod previously? 30?

No, it was 1, which is correct.
Typmod for declare and column should be 1.
For cast(something as binary) should be 30.
But we never called the typmod function while inserting, so we never actually got to truncation part.
Error is the right behaviour.

surendravishnoi · 2023-12-05T08:23:28Z

contrib/babelfishpg_tsql/src/pltsql_coerce.c

+								entry->castcontext = COERCION_CODE_IMPLICIT;
+								entry->castmethod = COERCION_METHOD_BINARY;
+								value = hash_search(ht_tsql_cast_info, key, HASH_ENTER, NULL);
+								*(tsql_cast_info_entry_t *) value = *entry;


how this entry will be replaced by actual function once minor upgrade happens?

after the upgrade script has been run, we reload this hash map and then we will have the new function available.

Signed-off-by: Tanzeel Khan <[email protected]>

@A

Original PR: #2037 1. Cherry Picked From Original PR 2. Bump Version for babelfish common since we are adding sql changes babelfishpg_common--2.5.0--2.6.0.sql 3. Add helper functions missing in 2 X that is s_tsql_sys_varbinary_datatype & is_tsql_sys_binary_datatype sys.binary datatype is created as a domain over sys.bbf_binary. In babelfish we assume hex code as varbinary which must then be casted to binary. For casting we fetch rule from hashmap after coercing both source and target to base type. So binary --> bbf_binary && varbinary-->bbf_varbinary, this cast is currently defined as binary coercible, that is NO modification needed. https://github.com/babelfish-for-postgresql/babelfish_extensions/blob/c57e8bc7820e1ef697dd9898b3aa433ba504a7ab/contrib/babelfishpg_tsql/src/pltsql_coerce.c#L114 We fix it by adding proper function for varbinary --> binary cast -> The default length for local variables defined for binary and varbinary should be 1 DECLARE @A binary should be equivalent to DECLARE @A binary(1) DECLARE @A varbinary should be equivalent to DECLARE @A varbinary(1) ->Problems during dump and restore done during upgrade. The new function created for the cast may not exists in the source version and the ugrade scripts are run only after pg dump and restore carried out during version upgrade. So ultimately during dump and restore no cast exists for varbinary to binary, which is a necessary requirement or upgrade will fail with error : _"pg_restore: error: could not execute query: ERROR: argument of DEFAULT must be type sys."binary", not type sys.varbinary"_. if we are dumping a procedure with sys.binary arg and a default value -> binary to varbinary we do not need to modify binary to varbinary cast since we have a type modifer defined for base type of sys.varbinary i.e. CAST(sys.bbf_varbiniary as sys.bbf_varbiniary). Task: BABEL-4544 Signed-off-by: Tanzeel Khan <[email protected]>

Signed-off-by: Tanzeel Khan <[email protected]>

@A

sys.binary datatype is created as a domain over sys.bbf_binary. In babelfish we assume hex code as varbinary which must then be casted to binary. For casting we fetch rule from hashmap after coercing both source and target to base type. So binary --> bbf_binary && varbinary-->bbf_varbinary This cast is currently defined as binary coercible, that is NO modification needed. https://github.com/babelfish-for-postgresql/babelfish_extensions/blob/c57e8bc7820e1ef697dd9898b3aa433ba504a7ab/contrib/babelfishpg_tsql/src/pltsql_coerce.c#L114 We fix it by adding proper function for varbinary --> binary cast -> The default length for local variables defined for binary and varbinary should be 1 DECLARE @A binary should be equivalent to DECLARE @A binary(1) DECLARE @A varbinary should be equivalent to DECLARE @A varbinary(1) ->Problems during dump and restore done during upgrade. The new function created for the cast may not exists in the source version and the ugrade scripts are run only after pg dump and restore carried out during version upgrade. So ultimately during dump and restore no cast exists for varbinary to binary, which is a necessary requirement or upgrade will fail with error : _"pg_restore: error: could not execute query: ERROR: argument of DEFAULT must be type sys."binary", not type sys.varbinary"_. if we are dumping a procedure with sys.binary arg and a default value -> binary to varbinary we do not need to modify binary to varbinary cast since we have a type modifer defined for base type of sys.varbinary i.e. CAST(sys.bbf_varbiniary as sys.bbf_varbiniary). Task: BABEL-4544 Signed-off-by: Tanzeel Khan <[email protected]>

@A

sys.binary datatype is created as a domain over sys.bbf_binary. In babelfish we assume hex code as varbinary which must then be casted to binary. For casting we fetch rule from hashmap after coercing both source and target to base type. So binary --> bbf_binary && varbinary-->bbf_varbinary This cast is currently defined as binary coercible, that is NO modification needed. https://github.com/babelfish-for-postgresql/babelfish_extensions/blob/c57e8bc7820e1ef697dd9898b3aa433ba504a7ab/contrib/babelfishpg_tsql/src/pltsql_coerce.c#L114 We fix it by adding proper function for varbinary --> binary cast -> The default length for local variables defined for binary and varbinary should be 1 DECLARE @A binary should be equivalent to DECLARE @A binary(1) DECLARE @A varbinary should be equivalent to DECLARE @A varbinary(1) ->Problems during dump and restore done during upgrade. The new function created for the cast may not exists in the source version and the ugrade scripts are run only after pg dump and restore carried out during version upgrade. So ultimately during dump and restore no cast exists for varbinary to binary, which is a necessary requirement or upgrade will fail with error : _"pg_restore: error: could not execute query: ERROR: argument of DEFAULT must be type sys."binary", not type sys.varbinary"_. if we are dumping a procedure with sys.binary arg and a default value -> binary to varbinary we do not need to modify binary to varbinary cast since we have a type modifer defined for base type of sys.varbinary i.e. CAST(sys.bbf_varbiniary as sys.bbf_varbiniary). Task: BABEL-4544 Signed-off-by: Tanzeel Khan <[email protected]>

@key

* TSQL connection ps status always shows idle. (babelfish-for-postgresql#2107) This causes confusion because it would appear as if there is a memory leak issue because ps status shows idle while memory usage increases. This commit fixes that issue and will set the ps status to active when it is really active. PG engine code resets it back to idle. Task: BABEL-4604 Signed-off-by: Kristian Lejao <[email protected]> * Run MVU tests in release mode (babelfish-for-postgresql#2113) Task: BABEL-4449 Signed-off-by: Nirmit Shah <[email protected]> * Create and use composite actions for code coverage workflow to improve maintainability (babelfish-for-postgresql#2100) Github actions for running jdbc, odbc, python and dotnet were copied from the respective workflow files into code-coverage workflow file. Now any changes in the original action demands a similar change in the code coverage file as well increasing redundant effort. To improve maintainability we move these actions to new composite actions and reuse them in workflows. Also added a new optional variable in build-modified-postgres/action.yml to optionally build postgres with the enable code coverage flag. Issues Resolved [BABEL-4605] Signed-off-by: Tanzeel Khan [email protected] * Fix the issue that inserted table refered in update join will cause crash (babelfish-for-postgresql#2140) Previously when we implement update join, we missed a corner case that one of the join relations can be a named tuple store instead of RTE relation. And it'll lead analyzer to wrongly set resultRelation for the Query. This fix resolved the corner case by add a if condition to exclude RTE_NAMEDTUPLESTORE for being resultRelation. Task: BABEL-4606 Signed-off-by: Zhibai Song <[email protected]> * Fix system procedure sp_pkeys which may fail to return any row if parallel mode is enforced (babelfish-for-postgresql#2147) It is Postgres peculiarity with Hash cond (with operator text = name and text = name) that hash functions being used to hash text and name data type values are totally different, for example, hashtext would consider explicit collation whereas hashname does not care about explicit collation. That is the reason why hash condition would fail every time. (hashtext and hashname may produce different hash value for same input under collation other than "C"). This means that whenever there is such hash condition for Hash join then it would always fail and will not return any row. And optimiser can choose such hash join based on stats. Such issue is observed with helper view sys.sp_pkeys_view which is being used by system procedure sp_pkeys causing procedure to return zero rows when optimiser chooses certain hash join. This commit fixes such issue by casting name data type to appropriate T-SQL data type. Task: BABEL-4603 Signed-off-by: Dipesh Dhameliya [email protected] * Add Support for Uploading Coredumps in Github Actions (babelfish-for-postgresql#2148) In case of crash during the Tests run , if there is a server crash this action will create the text backtrace as well as upload the corefile as an artifact of Github action run. Example of workflow where coredump generated -https://github.com/babelfish-for-postgresql/babelfish_extensions/actions/runs/7194251970 Signed-off-by: Nirmit Shah [email protected] * Unblock test cases blocked due to BABEL-4539 (babelfish-for-postgresql#2047) With engine changes babelfish-for-postgresql/postgresql_modified_for_babelfish#260, we introduced new field in Port data structure to track whether given backend is Babelfish backend or not. This commit changes extension part of code to initialise it properly. Additionally, it also unblocks test cases blocked due to BABEL-4539. Engine changes: babelfish-for-postgresql/postgresql_modified_for_babelfish#260 Task: BABEL-4539 Signed-off-by: Dipesh Dhameliya <[email protected]> * Blocked CS_AS collation for server_collation_name as it is not supported in babelfish (babelfish-for-postgresql#2141) Babelfish currently supports only CI_AS type of collation for server collation and other collations such as CI_AI, CS_AI and CS_AS should be blocked. Currently CI_AI and CS_AI collation for server collation name is blocked but CS_AS is not blocked. It should also be blocked. This changes will block CS_AS collation for server collation name Task: BABEL-4632 Signed-off-by: Rohit Bhagat <[email protected]> * Unblock few more test cases for parallel worker testing (babelfish-for-postgresql#2156) After fixing few jiras related to parallel worker support for Babelfish, some new test cases are now started passing. This commit unblocks all such cases. Issues Resolved Task: BABEL-4392 Signed-off-by: Dipesh Dhameliya [email protected] * Add PR workflow for Static Code Analyzer and fix existing errors and warnings (babelfish-for-postgresql#2143) This commit integrates Static Code Analyzer with PR workflow to improve code quality and catch errors quickly, along with that this commit also fixes the existing errors and warnings. Signed-off-by: Sumit Jaiswal [email protected] * Fixed definition of parsename, session_context and sp_set_session_context (babelfish-for-postgresql#2152) Following is the current definition of PARSENAME, session_context and sp_set_session_context -- parsename CREATE OR REPLACE FUNCTION sys.parsename(object_name sys.VARCHAR, object_piece int) RETURNS sys.SYSNAME AS 'babelfishpg_tsql', 'parsename' LANGUAGE C IMMUTABLE STRICT; -- session_context CREATE OR REPLACE FUNCTION sys.session_context ("@key" sys.sysname) RETURNS sys.SQL_VARIANT AS 'babelfishpg_tsql', 'session_context' LANGUAGE C; GRANT EXECUTE ON FUNCTION sys.session_context TO PUBLIC; -- sp_set_session_context CREATE OR REPLACE PROCEDURE sys.sp_set_session_context ("@key" sys.sysname, "@value" sys.SQL_VARIANT, "@read_only" sys.bit = 0) AS 'babelfishpg_tsql', 'sp_set_session_context' LANGUAGE C; GRANT EXECUTE ON PROCEDURE sys.sp_set_session_context TO PUBLIC; But the correct definitions should be as follows, in these definitions sys.NVARCHAR should be used instead of sys.VARCHAR and sys.NVARCHAR(128) instead of sys.SYSNAME. This commit fixes such issues. --parsename CREATE OR REPLACE FUNCTION sys.parsename(object_name sys.NVARCHAR(128), object_piece int) RETURNS sys.NVARCHAR(128) AS 'babelfishpg_tsql', 'parsename' LANGUAGE C IMMUTABLE STRICT; -- session_context CREATE OR REPLACE FUNCTION sys.session_context ("@key" sys.NVARCHAR(128)) RETURNS sys.SQL_VARIANT AS 'babelfishpg_tsql', 'session_context' LANGUAGE C; GRANT EXECUTE ON FUNCTION sys.session_context TO PUBLIC; -- sp_set_session_context CREATE OR REPLACE PROCEDURE sys.sp_set_session_context ("@key" sys.NVARCHAR(128), "@value" sys.SQL_VARIANT, "@read_only" sys.bit = 0) AS 'babelfishpg_tsql', 'sp_set_session_context' LANGUAGE C; GRANT EXECUTE ON PROCEDURE sys.sp_set_session_context TO PUBLIC; Task: BABEL-4583 Signed-off-by: Rohit Bhagat <[email protected]> * Fix typmod for sys.binary datatype (babelfish-for-postgresql#2037) sys.binary datatype is created as a domain over sys.bbf_binary. In babelfish we assume hex code as varbinary which must then be casted to binary. For casting we fetch rule from hashmap after coercing both source and target to base type. So binary --> bbf_binary && varbinary-->bbf_varbinary This cast is currently defined as binary coercible, that is NO modification needed. https://github.com/babelfish-for-postgresql/babelfish_extensions/blob/c57e8bc7820e1ef697dd9898b3aa433ba504a7ab/contrib/babelfishpg_tsql/src/pltsql_coerce.c#L114 We fix it by adding proper function for varbinary --> binary cast -> The default length for local variables defined for binary and varbinary should be 1 DECLARE @A binary should be equivalent to DECLARE @A binary(1) DECLARE @A varbinary should be equivalent to DECLARE @A varbinary(1) ->Problems during dump and restore done during upgrade. The new function created for the cast may not exists in the source version and the ugrade scripts are run only after pg dump and restore carried out during version upgrade. So ultimately during dump and restore no cast exists for varbinary to binary, which is a necessary requirement or upgrade will fail with error : _"pg_restore: error: could not execute query: ERROR: argument of DEFAULT must be type sys."binary", not type sys.varbinary"_. if we are dumping a procedure with sys.binary arg and a default value -> binary to varbinary we do not need to modify binary to varbinary cast since we have a type modifer defined for base type of sys.varbinary i.e. CAST(sys.bbf_varbiniary as sys.bbf_varbiniary). Task: BABEL-4544 Signed-off-by: Tanzeel Khan <[email protected]> --------- Signed-off-by: Kristian Lejao <[email protected]> Signed-off-by: Nirmit Shah <[email protected]> Signed-off-by: Tanzeel Khan [email protected] Signed-off-by: Zhibai Song <[email protected]> Signed-off-by: Dipesh Dhameliya [email protected] Signed-off-by: Nirmit Shah [email protected] Signed-off-by: Rohit Bhagat <[email protected]> Signed-off-by: Sumit Jaiswal [email protected] Signed-off-by: Tanzeel Khan <[email protected]> Co-authored-by: Kristian Lejao <[email protected]> Co-authored-by: Nirmit Shah <[email protected]> Co-authored-by: Tanzeel Khan <[email protected]> Co-authored-by: Zhibai Song <[email protected]> Co-authored-by: Dipesh Dhameliya <[email protected]> Co-authored-by: Rohit bhagat <[email protected]> Co-authored-by: Sumit Jaiswal <[email protected]>

@A

sys.binary datatype is created as a domain over sys.bbf_binary. In babelfish we assume hex code as varbinary which must then be casted to binary. For casting we fetch rule from hashmap after coercing both source and target to base type. So binary --> bbf_binary && varbinary-->bbf_varbinary This cast is currently defined as binary coercible, that is NO modification needed. https://github.com/babelfish-for-postgresql/babelfish_extensions/blob/c57e8bc7820e1ef697dd9898b3aa433ba504a7ab/contrib/babelfishpg_tsql/src/pltsql_coerce.c#L114 We fix it by adding proper function for varbinary --> binary cast -> The default length for local variables defined for binary and varbinary should be 1 DECLARE @A binary should be equivalent to DECLARE @A binary(1) DECLARE @A varbinary should be equivalent to DECLARE @A varbinary(1) ->Problems during dump and restore done during upgrade. The new function created for the cast may not exists in the source version and the ugrade scripts are run only after pg dump and restore carried out during version upgrade. So ultimately during dump and restore no cast exists for varbinary to binary, which is a necessary requirement or upgrade will fail with error : _"pg_restore: error: could not execute query: ERROR: argument of DEFAULT must be type sys."binary", not type sys.varbinary"_. if we are dumping a procedure with sys.binary arg and a default value -> binary to varbinary we do not need to modify binary to varbinary cast since we have a type modifer defined for base type of sys.varbinary i.e. CAST(sys.bbf_varbiniary as sys.bbf_varbiniary). Task: BABEL-4544 Signed-off-by: Tanzeel Khan <[email protected]>

@A

sys.binary datatype is created as a domain over sys.bbf_binary. In babelfish we assume hex code as varbinary which must then be casted to binary. For casting we fetch rule from hashmap after coercing both source and target to base type. So binary --> bbf_binary && varbinary-->bbf_varbinary This cast is currently defined as binary coercible, that is NO modification needed. https://github.com/babelfish-for-postgresql/babelfish_extensions/blob/c57e8bc7820e1ef697dd9898b3aa433ba504a7ab/contrib/babelfishpg_tsql/src/pltsql_coerce.c#L114 We fix it by adding proper function for varbinary --> binary cast -> The default length for local variables defined for binary and varbinary should be 1 DECLARE @A binary should be equivalent to DECLARE @A binary(1) DECLARE @A varbinary should be equivalent to DECLARE @A varbinary(1) ->Problems during dump and restore done during upgrade. The new function created for the cast may not exists in the source version and the ugrade scripts are run only after pg dump and restore carried out during version upgrade. So ultimately during dump and restore no cast exists for varbinary to binary, which is a necessary requirement or upgrade will fail with error : _"pg_restore: error: could not execute query: ERROR: argument of DEFAULT must be type sys."binary", not type sys.varbinary"_. if we are dumping a procedure with sys.binary arg and a default value -> binary to varbinary we do not need to modify binary to varbinary cast since we have a type modifer defined for base type of sys.varbinary i.e. CAST(sys.bbf_varbiniary as sys.bbf_varbiniary). Task: BABEL-4544 Signed-off-by: Tanzeel Khan <[email protected]>

@A

sys.binary datatype is created as a domain over sys.bbf_binary. In babelfish we assume hex code as varbinary which must then be casted to binary. For casting we fetch rule from hashmap after coercing both source and target to base type. So binary --> bbf_binary && varbinary-->bbf_varbinary This cast is currently defined as binary coercible, that is NO modification needed. https://github.com/babelfish-for-postgresql/babelfish_extensions/blob/c57e8bc7820e1ef697dd9898b3aa433ba504a7ab/contrib/babelfishpg_tsql/src/pltsql_coerce.c#L114 We fix it by adding proper function for varbinary --> binary cast -> The default length for local variables defined for binary and varbinary should be 1 DECLARE @A binary should be equivalent to DECLARE @A binary(1) DECLARE @A varbinary should be equivalent to DECLARE @A varbinary(1) ->Problems during dump and restore done during upgrade. The new function created for the cast may not exists in the source version and the ugrade scripts are run only after pg dump and restore carried out during version upgrade. So ultimately during dump and restore no cast exists for varbinary to binary, which is a necessary requirement or upgrade will fail with error : _"pg_restore: error: could not execute query: ERROR: argument of DEFAULT must be type sys."binary", not type sys.varbinary"_. if we are dumping a procedure with sys.binary arg and a default value -> binary to varbinary we do not need to modify binary to varbinary cast since we have a type modifer defined for base type of sys.varbinary i.e. CAST(sys.bbf_varbiniary as sys.bbf_varbiniary). Task: BABEL-4544 Signed-off-by: Tanzeel Khan <[email protected]>

@A

sys.binary datatype is created as a domain over sys.bbf_binary. In babelfish we assume hex code as varbinary which must then be casted to binary. For casting we fetch rule from hashmap after coercing both source and target to base type. So binary --> bbf_binary && varbinary-->bbf_varbinary This cast is currently defined as binary coercible, that is NO modification needed. https://github.com/babelfish-for-postgresql/babelfish_extensions/blob/c57e8bc7820e1ef697dd9898b3aa433ba504a7ab/contrib/babelfishpg_tsql/src/pltsql_coerce.c#L114 We fix it by adding proper function for varbinary --> binary cast -> The default length for local variables defined for binary and varbinary should be 1 DECLARE @A binary should be equivalent to DECLARE @A binary(1) DECLARE @A varbinary should be equivalent to DECLARE @A varbinary(1) ->Problems during dump and restore done during upgrade. The new function created for the cast may not exists in the source version and the ugrade scripts are run only after pg dump and restore carried out during version upgrade. So ultimately during dump and restore no cast exists for varbinary to binary, which is a necessary requirement or upgrade will fail with error : _"pg_restore: error: could not execute query: ERROR: argument of DEFAULT must be type sys."binary", not type sys.varbinary"_. if we are dumping a procedure with sys.binary arg and a default value -> binary to varbinary we do not need to modify binary to varbinary cast since we have a type modifer defined for base type of sys.varbinary i.e. CAST(sys.bbf_varbiniary as sys.bbf_varbiniary). Task: BABEL-4544 Signed-off-by: Tanzeel Khan <[email protected]>

@A

sys.binary datatype is created as a domain over sys.bbf_binary. In babelfish we assume hex code as varbinary which must then be casted to binary. For casting we fetch rule from hashmap after coercing both source and target to base type. So binary --> bbf_binary && varbinary-->bbf_varbinary This cast is currently defined as binary coercible, that is NO modification needed. https://github.com/babelfish-for-postgresql/babelfish_extensions/blob/c57e8bc7820e1ef697dd9898b3aa433ba504a7ab/contrib/babelfishpg_tsql/src/pltsql_coerce.c#L114 We fix it by adding proper function for varbinary --> binary cast -> The default length for local variables defined for binary and varbinary should be 1 DECLARE @A binary should be equivalent to DECLARE @A binary(1) DECLARE @A varbinary should be equivalent to DECLARE @A varbinary(1) ->Problems during dump and restore done during upgrade. The new function created for the cast may not exists in the source version and the ugrade scripts are run only after pg dump and restore carried out during version upgrade. So ultimately during dump and restore no cast exists for varbinary to binary, which is a necessary requirement or upgrade will fail with error : _"pg_restore: error: could not execute query: ERROR: argument of DEFAULT must be type sys."binary", not type sys.varbinary"_. if we are dumping a procedure with sys.binary arg and a default value -> binary to varbinary we do not need to modify binary to varbinary cast since we have a type modifer defined for base type of sys.varbinary i.e. CAST(sys.bbf_varbiniary as sys.bbf_varbiniary). Task: BABEL-4544 Signed-off-by: Tanzeel Khan <[email protected]>

@A

sys.binary datatype is created as a domain over sys.bbf_binary. In babelfish we assume hex code as varbinary which must then be casted to binary. For casting we fetch rule from hashmap after coercing both source and target to base type. So binary --> bbf_binary && varbinary-->bbf_varbinary This cast is currently defined as binary coercible, that is NO modification needed. https://github.com/babelfish-for-postgresql/babelfish_extensions/blob/c57e8bc7820e1ef697dd9898b3aa433ba504a7ab/contrib/babelfishpg_tsql/src/pltsql_coerce.c#L114 We fix it by adding proper function for varbinary --> binary cast -> The default length for local variables defined for binary and varbinary should be 1 DECLARE @A binary should be equivalent to DECLARE @A binary(1) DECLARE @A varbinary should be equivalent to DECLARE @A varbinary(1) ->Problems during dump and restore done during upgrade. The new function created for the cast may not exists in the source version and the ugrade scripts are run only after pg dump and restore carried out during version upgrade. So ultimately during dump and restore no cast exists for varbinary to binary, which is a necessary requirement or upgrade will fail with error : _"pg_restore: error: could not execute query: ERROR: argument of DEFAULT must be type sys."binary", not type sys.varbinary"_. if we are dumping a procedure with sys.binary arg and a default value -> binary to varbinary we do not need to modify binary to varbinary cast since we have a type modifer defined for base type of sys.varbinary i.e. CAST(sys.bbf_varbiniary as sys.bbf_varbiniary). Task: BABEL-4544 Signed-off-by: Tanzeel Khan <[email protected]>

@A

sys.binary datatype is created as a domain over sys.bbf_binary. In babelfish we assume hex code as varbinary which must then be casted to binary. For casting we fetch rule from hashmap after coercing both source and target to base type. So binary --> bbf_binary && varbinary-->bbf_varbinary This cast is currently defined as binary coercible, that is NO modification needed. https://github.com/babelfish-for-postgresql/babelfish_extensions/blob/c57e8bc7820e1ef697dd9898b3aa433ba504a7ab/contrib/babelfishpg_tsql/src/pltsql_coerce.c#L114 We fix it by adding proper function for varbinary --> binary cast -> The default length for local variables defined for binary and varbinary should be 1 DECLARE @A binary should be equivalent to DECLARE @A binary(1) DECLARE @A varbinary should be equivalent to DECLARE @A varbinary(1) ->Problems during dump and restore done during upgrade. The new function created for the cast may not exists in the source version and the ugrade scripts are run only after pg dump and restore carried out during version upgrade. So ultimately during dump and restore no cast exists for varbinary to binary, which is a necessary requirement or upgrade will fail with error : _"pg_restore: error: could not execute query: ERROR: argument of DEFAULT must be type sys."binary", not type sys.varbinary"_. if we are dumping a procedure with sys.binary arg and a default value -> binary to varbinary we do not need to modify binary to varbinary cast since we have a type modifer defined for base type of sys.varbinary i.e. CAST(sys.bbf_varbiniary as sys.bbf_varbiniary). Task: BABEL-4544 Signed-off-by: Tanzeel Khan <[email protected]>

tanscorpio7 force-pushed the BABEL_4544 branch 2 times, most recently from 5f7927a to fbddcde Compare November 28, 2023 10:23

tanscorpio7 requested review from kuntalghosh and Deepesh125 November 28, 2023 11:08

Deepesh125 requested changes Nov 29, 2023

View reviewed changes

test/JDBC/expected/TestBinary-before-14_11-or-15_6-vu-prepare.out Outdated Show resolved Hide resolved

test/JDBC/expected/BABEL-3166-before-14_11-or-15_6-vu-verify.out Show resolved Hide resolved

tanscorpio7 force-pushed the BABEL_4544 branch from ecd616d to 2ec8609 Compare November 30, 2023 14:04

Deepesh125 reviewed Dec 1, 2023

View reviewed changes

test/JDBC/expected/BABEL_4544.out Show resolved Hide resolved

tanscorpio7 requested review from rishabhtanwar29 and Deepesh125 and removed request for kuntalghosh December 1, 2023 14:23

rishabhtanwar29 reviewed Dec 1, 2023

View reviewed changes

tanscorpio7 requested a review from KushaalShroff December 4, 2023 13:57

Deepesh125 requested changes Dec 4, 2023

View reviewed changes

surendravishnoi reviewed Dec 5, 2023

View reviewed changes

tanscorpio7 requested a review from Deepesh125 December 5, 2023 13:14

tanscorpio7 changed the title ~~Fix typmod for binary datatype~~ Fix typmod for sys.binary datatype Dec 5, 2023

tanscorpio7 requested a review from surendravishnoi December 11, 2023 10:59

fix typmod for sys binary

fb573f2

Signed-off-by: Tanzeel Khan <[email protected]>

tanscorpio7 force-pushed the BABEL_4544 branch from 054d35a to fb573f2 Compare December 14, 2023 06:30

tanscorpio7 mentioned this pull request Dec 14, 2023

Fix typmod for sys.binary datatype #2149

Merged

1 task

update tests

a4b1460

Signed-off-by: Tanzeel Khan <[email protected]>

tanscorpio7 added 3 commits December 20, 2023 16:38

update schedule fie for 14.11

f828384

Signed-off-by: Tanzeel Khan <[email protected]>

add upgrade path babelfish common from 2.6.0 to 3.0.0

af41012

Signed-off-by: Tanzeel Khan <[email protected]>

update expected drop file

9dfae97

Signed-off-by: Tanzeel Khan <[email protected]>

Deepesh125 approved these changes Dec 21, 2023

View reviewed changes

Deepesh125 merged commit 8c3b386 into babelfish-for-postgresql:BABEL_3_X_DEV Dec 21, 2023
30 checks passed

tanscorpio7 deleted the BABEL_4544 branch February 7, 2024 09:21

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix typmod for sys.binary datatype #2037

Fix typmod for sys.binary datatype #2037

tanscorpio7 commented Nov 20, 2023 •

edited

Loading

Deepesh125 Dec 1, 2023

tanscorpio7 Dec 1, 2023

Deepesh125 Dec 4, 2023

tanscorpio7 Dec 5, 2023

Deepesh125 Dec 4, 2023

tanscorpio7 Dec 5, 2023

surendravishnoi Dec 5, 2023

tanscorpio7 Dec 5, 2023

Fix typmod for sys.binary datatype #2037

Fix typmod for sys.binary datatype #2037

Conversation

tanscorpio7 commented Nov 20, 2023 • edited Loading

Description

Issues Resolved

Sign Off

Check List

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tanscorpio7 commented Nov 20, 2023 •

edited

Loading