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

Fix typmod for sys.binary datatype #2037

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/upgrade-test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ jobs:
done

- name: Upload Logs
if: always() && steps.test-file-rename.outcome == 'success'
if: always() && steps.upgrade-and-test.outcome != 'success'
tanscorpio7 marked this conversation as resolved.
Show resolved Hide resolved
uses: actions/upload-artifact@v2
with:
name: upgrade-logs-${{ matrix.upgrade-path.title }}
Expand Down
5 changes: 5 additions & 0 deletions contrib/babelfishpg_common/sql/binary.sql
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,11 @@ CREATE TYPE sys.BBF_BINARY (
COLLATABLE = false
);

CREATE OR REPLACE FUNCTION sys.varbinarybinary (sys.BBF_VARBINARY, integer, boolean)
RETURNS sys.BBF_BINARY
AS 'babelfishpg_common', 'binary'
LANGUAGE C IMMUTABLE STRICT PARALLEL SAFE;

CREATE CAST (sys.BBF_BINARY AS sys.BBF_VARBINARY)
WITHOUT FUNCTION AS IMPLICIT;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,10 @@

SELECT set_config('search_path', 'sys, '||current_setting('search_path'), false);

CREATE OR REPLACE FUNCTION sys.varbinarybinary (sys.BBF_VARBINARY, integer, boolean)
RETURNS sys.BBF_BINARY
AS 'babelfishpg_common', 'binary'
LANGUAGE C IMMUTABLE STRICT PARALLEL SAFE;

-- Reset search_path to not affect any subsequent scripts
SELECT set_config('search_path', trim(leading 'sys, ' from current_setting('search_path')), false);
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
-- complain if script is sourced in psql, rather than via ALTER EXTENSION
\echo Use "ALTER EXTENSION ""babelfishpg_common"" UPDATE TO '3.0.0'" to load this file. \quit

SELECT set_config('search_path', 'sys, '||current_setting('search_path'), false);

/* This helper function would only be useful and strictly be used during 1.x->2.3 and 2.3->3.0 upgrade. */
CREATE OR REPLACE FUNCTION sys.babelfish_update_server_collation_name() RETURNS VOID
LANGUAGE C
AS 'babelfishpg_common', 'babelfish_update_server_collation_name';

SELECT sys.babelfish_update_server_collation_name();

DROP FUNCTION sys.babelfish_update_server_collation_name();

-- Reset search_path to not affect any subsequent scripts
SELECT set_config('search_path', trim(leading 'sys, ' from current_setting('search_path')), false);
25 changes: 22 additions & 3 deletions contrib/babelfishpg_tsql/src/pltsql_coerce.c
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,8 @@ extern coerce_string_literal_hook_type coerce_string_literal_hook;
extern select_common_type_hook_type select_common_type_hook;
extern select_common_typmod_hook_type select_common_typmod_hook;

extern bool babelfish_dump_restore;

PG_FUNCTION_INFO_V1(init_tsql_coerce_hash_tab);
PG_FUNCTION_INFO_V1(init_tsql_datatype_precedence_hash_tab);

Expand Down Expand Up @@ -111,7 +113,7 @@ tsql_cast_raw_info_t tsql_cast_raw_infos[] =
{PG_CAST_ENTRY, "sys", "bbf_varbinary", "pg_catalog", "int4", NULL, 'i', 'f'},
{PG_CAST_ENTRY, "sys", "bbf_varbinary", "pg_catalog", "int2", NULL, 'i', 'f'},
{TSQL_CAST_ENTRY, "sys", "bbf_varbinary", "sys", "rowversion", "varbinaryrowversion", 'i', 'f'},
{TSQL_CAST_WITHOUT_FUNC_ENTRY, "sys", "bbf_varbinary", "sys", "bbf_binary", NULL, 'i', 'b'},
{TSQL_CAST_ENTRY, "sys", "bbf_varbinary", "sys", "bbf_binary", "varbinarybinary", 'i', 'f'},
/* binary {only allow to cast to integral data type) */
{PG_CAST_ENTRY, "sys", "bbf_binary", "pg_catalog", "int8", NULL, 'i', 'f'},
{PG_CAST_ENTRY, "sys", "bbf_binary", "pg_catalog", "int4", NULL, 'i', 'f'},
Expand Down Expand Up @@ -638,9 +640,26 @@ init_tsql_coerce_hash_tab(PG_FUNCTION_ARGS)

if (!OidIsValid(entry->castfunc))
{
/*
* varbinary to binary implicit type cast without function should be allowed during MVU
* since the cast function might not exists when source version is before 14_11 and 15_6
*/
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;
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;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

continue;
}
/* function is not loaded. wait for next scan */
inited_ht_tsql_cast_info = false;
continue;
else
{
inited_ht_tsql_cast_info = false;
continue;
}
}
}
break;
Expand Down
8 changes: 8 additions & 0 deletions contrib/babelfishpg_tsql/src/pltsql_utils.c
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@ bool is_tsql_any_char_datatype(Oid oid); /* sys.char / sys.nchar /
* sys.varchar / sys.nvarchar */
bool is_tsql_text_ntext_or_image_datatype(Oid oid);

bool is_tsql_binary_or_varbinary_datatype(Oid oid);

bool
pltsql_createFunction(ParseState *pstate, PlannedStmt *pstmt, const char *queryString, ProcessUtilityContext context,
ParamListInfo params);
Expand Down Expand Up @@ -1082,6 +1084,12 @@ is_tsql_text_ntext_or_image_datatype(Oid oid)
(*common_utility_plugin_ptr->is_tsql_image_datatype) (oid);
}

bool is_tsql_binary_or_varbinary_datatype(Oid oid)
{
return (*common_utility_plugin_ptr->is_tsql_sys_binary_datatype) (oid) ||
(*common_utility_plugin_ptr->is_tsql_sys_varbinary_datatype) (oid);
}

/*
* Try to acquire a lock with no wait
*/
Expand Down
15 changes: 11 additions & 4 deletions contrib/babelfishpg_tsql/src/tsqlIface.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ extern "C"
extern PLtsql_type *parse_datatype(const char *string, int location);
extern bool is_tsql_any_char_datatype(Oid oid);
extern bool is_tsql_text_ntext_or_image_datatype(Oid oid);
extern bool is_tsql_binary_or_varbinary_datatype(Oid oid);

extern int CurrentLineNumber;

Expand Down Expand Up @@ -4558,9 +4559,12 @@ makeDeclareStmt(TSqlParser::Declare_statementContext *ctx, std::map<PLtsql_stmt
const char *name = downcase_truncate_identifier(nameStr.c_str(), nameStr.length(), true);
check_dup_declare(name);
PLtsql_type *type = parse_datatype(typeStr.c_str(), 0);
if (type->atttypmod == -1 && is_tsql_any_char_datatype(type->typoid))

/* (N)(VAR)CHAR and BINARY datatype length is treated as 1 when nothing is provided */
if (type->atttypmod == -1 && (is_tsql_any_char_datatype(type->typoid)
|| is_tsql_binary_or_varbinary_datatype(type->typoid)))
{
std::string newTypeStr = typeStr + "(1)"; /* in T-SQL, length-less (N)(VAR)CHAR's length is treated as 1 */
std::string newTypeStr = typeStr + "(1)";
type = parse_datatype(newTypeStr.c_str(), 0);
}

Expand Down Expand Up @@ -4589,9 +4593,12 @@ makeDeclareStmt(TSqlParser::Declare_statementContext *ctx, std::map<PLtsql_stmt
{
std::string typeStr = ::getFullText(local->data_type());
PLtsql_type *type = parse_datatype(typeStr.c_str(), 0); // FIXME: the second arg should be 'location'
if (type->atttypmod == -1 && is_tsql_any_char_datatype(type->typoid))

/* (N)(VAR)CHAR and BINARY datatype length is treated as 1 when nothing is provided */
if (type->atttypmod == -1 && (is_tsql_any_char_datatype(type->typoid)
|| is_tsql_binary_or_varbinary_datatype(type->typoid)))
{
std::string newTypeStr = typeStr + "(1)"; /* in T-SQL, length-less (N)(VAR)CHAR's length is treated as 1 */
std::string newTypeStr = typeStr + "(1)";
type = parse_datatype(newTypeStr.c_str(), 0);
}
else if (is_tsql_text_ntext_or_image_datatype(type->typoid))
Expand Down
2 changes: 1 addition & 1 deletion test/JDBC/expected/BABEL-1566-vu-verify.out
Original file line number Diff line number Diff line change
Expand Up @@ -281,7 +281,7 @@ select cast(cast(0xfe as binary) as text);
go
~~START~~
text
0xfe
0xfe0000000000000000000000000000000000000000000000000000000000
~~END~~


Expand Down
2 changes: 1 addition & 1 deletion test/JDBC/expected/BABEL-1566.out
Original file line number Diff line number Diff line change
Expand Up @@ -344,7 +344,7 @@ select cast(cast(0xfe as binary) as text);
go
~~START~~
text
0xfe
0xfe0000000000000000000000000000000000000000000000000000000000
~~END~~


Expand Down
43 changes: 43 additions & 0 deletions test/JDBC/expected/BABEL-3166-before-14_11-or-15_6-vu-prepare.out
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
-- function
CREATE FUNCTION babel_3166_func(@a numeric, @b varchar, @c varchar(max), @d varchar(8), @e binary(6))
RETURNS varbinary(8) AS BEGIN RETURN @e END;
go

-- Look at the probin for typmod information
SELECT proname, probin FROM pg_proc WHERE proname = 'babel_3166_func';
go
~~START~~
varchar#!#text
babel_3166_func#!#{"version_num": "1", "typmod_array": ["1179652", "-1", "-8000", "8", "6", "8"], "original_probin": ""}
~~END~~


SELECT babel_3166_func(1.2, 'abc', 'abcd', 'abcdefgh', 0x12bcfe);
go
~~START~~
varbinary
12BCFE
~~END~~


-- procedure
CREATE PROCEDURE babel_3166_proc @a numeric, @b varchar, @c varchar(max), @d varchar(8), @e binary(6)
AS SELECT @e;
go

-- Look at the probin for typmod information
SELECT proname, probin FROM pg_proc WHERE proname = 'babel_3166_proc';
go
~~START~~
varchar#!#text
babel_3166_proc#!#{"version_num": "1", "typmod_array": ["1179652", "-1", "-8000", "8", "6"], "original_probin": ""}
~~END~~


EXEC babel_3166_proc 1.2, 'abc', 'abcd', 'abcdefgh', 0x12bcfe;
go
~~START~~
binary
12BCFE000000
~~END~~

38 changes: 38 additions & 0 deletions test/JDBC/expected/BABEL-3166-before-14_11-or-15_6-vu-verify.out
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
-- Look at function's probin for typmod information
SELECT proname, probin FROM pg_proc WHERE proname = 'babel_3166_func';
tanscorpio7 marked this conversation as resolved.
Show resolved Hide resolved
go
~~START~~
varchar#!#text
babel_3166_func#!#{"version_num": "1", "typmod_array": ["1179652", "-1", "-8000", "8", "6", "8"], "original_probin": ""}
~~END~~


SELECT babel_3166_func(1.2, 'abc', 'abcd', 'abcdefgh', 0x12bcfe);
go
~~START~~
varbinary
12BCFE000000
~~END~~



DROP FUNCTION babel_3166_func;
tanscorpio7 marked this conversation as resolved.
Show resolved Hide resolved
-- Look at procedures's probin for typmod information
SELECT proname, probin FROM pg_proc WHERE proname = 'babel_3166_proc';
go
~~START~~
varchar#!#text
babel_3166_proc#!#{"version_num": "1", "typmod_array": ["1179652", "-1", "-8000", "8", "6"], "original_probin": ""}
~~END~~


EXEC babel_3166_proc 1.2, 'abc', 'abcd', 'abcdefgh', 0x12bcfe;
go
~~START~~
binary
12BCFE000000
~~END~~


DROP PROCEDURE babel_3166_proc;
go
2 changes: 1 addition & 1 deletion test/JDBC/expected/BABEL-3166-vu-prepare.out
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ SELECT babel_3166_func(1.2, 'abc', 'abcd', 'abcdefgh', 0x12bcfe);
go
~~START~~
varbinary
12BCFE
12BCFE000000
~~END~~


Expand Down
2 changes: 1 addition & 1 deletion test/JDBC/expected/BABEL-3166-vu-verify.out
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ SELECT babel_3166_func(1.2, 'abc', 'abcd', 'abcdefgh', 0x12bcfe);
go
~~START~~
varbinary
12BCFE
12BCFE000000
~~END~~


Expand Down
2 changes: 1 addition & 1 deletion test/JDBC/expected/BABEL_1940.out
Original file line number Diff line number Diff line change
Expand Up @@ -276,7 +276,7 @@ SELECT CAST(CAST(0x61 AS BINARY(3)) AS VARBINARY(2))
GO
~~START~~
varbinary
61
6100
~~END~~


Expand Down
Loading
Loading