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 value in case of nvarchar(max), varchar(max), varbinary(max) #2036

Merged

Conversation

skumawat2025
Copy link
Contributor

@skumawat2025 skumawat2025 commented Nov 20, 2023

Description

Previously, there was an issue in handling varchar(max) , nvarchar(max) and varbinary(max) in the code. Specifically, when dealing with atttypmod with a value of -8000 (representing TSQLMaxTypmod) in the PrepareRowDescription function for nvarchar and varchar, an incorrect value of atttypmod was being passed to the SetColMetadataForCharType function. This resulted in setting an incorrect maxSize for the column metadata entry.
Because of this if we assign a long string value to a nvarchar(max) variable with a length such that UTF16 output size is large it will throw a FATAL log elog(FATAL, "UTF16 output of varchar/bpchar exceeds max length");

Additionally, This PR limits a maximum scale value for (var)char, (var)binary, and (n)(var)char types. To do that, we set a limit of 8000 for (var)char and (var)binary, and a limit of 4000 for (n)(var)char.

Signed-off-by: Sandeep Kumawat [email protected]

Issues Resolved

BABEL-4547

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.

Sandeep Kumawat added 6 commits November 20, 2023 04:17
Signed-off-by: Sandeep Kumawat <[email protected]>
Signed-off-by: Sandeep Kumawat <[email protected]>
Signed-off-by: Sandeep Kumawat <[email protected]>
Signed-off-by: Sandeep Kumawat <[email protected]>
Copy link
Contributor

@KushaalShroff KushaalShroff left a comment

Choose a reason for hiding this comment

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

We dont need tests to test these changes??

contrib/babelfishpg_tds/src/backend/tds/tdstypeio.c Outdated Show resolved Hide resolved
contrib/babelfishpg_tds/src/backend/tds/tdsresponse.c Outdated Show resolved Hide resolved
@KushaalShroff
Copy link
Contributor

Looks like this is a common issue for varbinarytypmodin, bpchartypmodin and varchartypmodin but you have fixed only n/varchar

1. Added test-cases
2. Changes value of atttypmod in parser for varchar, nvarchar
3. Changes value of atttypod in protocl for varbinary.

Signed-off-by: Sandeep Kumawat <[email protected]>
@skumawat2025
Copy link
Contributor Author

We dont need tests to test these changes??

Added test cases

@skumawat2025
Copy link
Contributor Author

skumawat2025 commented Nov 28, 2023

Looks like this is a common issue for varbinarytypmodin, bpchartypmodin and varchartypmodin but you have fixed only n/varchar

Yes, in tsql these three data-types support max as a scale. So these three have this issue.

1. varchar(max)
2. nvarchar(max)
3. varbinary(max)

In the latest commit I have made changes in parser for varchar(max), nvarchar(max).
For varbinary(max) we can not change typmod value to -1 before in parser because we are calling resolve_varbinary_typmod_from_exp (see code here) for some cases where typmod is -1 inside protocol. So I think we are left with option where we change atttypmod value in protocl for varbinary.

contrib/babelfishpg_tds/src/backend/tds/tdstypeio.c Outdated Show resolved Hide resolved
contrib/babelfishpg_tsql/src/pltsql_utils.c Outdated Show resolved Hide resolved
test/JDBC/expected/BABEL-4547-NVARCHAR.out Outdated Show resolved Hide resolved
@skumawat2025 skumawat2025 changed the title fix crash in TdsSendTypeNVarchar for nvarchar(max) fix crash occurring in support of data-type nvarchar(max), varchar(max), varbinary(max) Dec 19, 2023
@skumawat2025 skumawat2025 changed the title fix crash occurring in support of data-type nvarchar(max), varchar(max), varbinary(max) fix crash occurring in data-type nvarchar(max), varchar(max), varbinary(max) Dec 20, 2023
Sandeep Kumawat added 5 commits December 20, 2023 14:00
1. fixes crash in case of varchar(max), nvarchar(max), varbinary(max)
   when input value is very large.
2. Blocks the out of range value of scale in case char(), nchar(),
   varchar(), nvarchar(), binary(), varbinary()

Signed-off-by: Sandeep Kumawat <[email protected]>
parse_datatype now uses oid to validated data-type instead of string.
This will help in sys.varchar and varchar data-type to be treated as
same

Signed-off-by: Sandeep Kumawat <[email protected]>
Signed-off-by: Sandeep Kumawat <[email protected]>
@skumawat2025 skumawat2025 changed the title fix crash occurring in data-type nvarchar(max), varchar(max), varbinary(max) fix crash occurring in using data-type nvarchar(max), varchar(max), varbinary(max) Dec 21, 2023
contrib/babelfishpg_tds/src/backend/tds/tdstypeio.c Outdated Show resolved Hide resolved
contrib/babelfishpg_tsql/src/pl_gram.y Outdated Show resolved Hide resolved
contrib/babelfishpg_tsql/src/pl_gram.y Outdated Show resolved Hide resolved
contrib/babelfishpg_tds/src/backend/tds/tdsresponse.c Outdated Show resolved Hide resolved
test/JDBC/input/BABEL-4547.sql Show resolved Hide resolved
Sandeep Kumawat added 2 commits December 26, 2023 14:26
Signed-off-by: Sandeep Kumawat <[email protected]>
@skumawat2025 skumawat2025 changed the title fix crash occurring in using data-type nvarchar(max), varchar(max), varbinary(max) fix typmod value in case of nvarchar(max), varchar(max), varbinary(max) Dec 26, 2023
Signed-off-by: Sandeep Kumawat <[email protected]>
Deepesh125
Deepesh125 previously approved these changes Dec 28, 2023
contrib/babelfishpg_tds/src/backend/tds/tdstypeio.c Outdated Show resolved Hide resolved
contrib/babelfishpg_tsql/src/pl_gram.y Outdated Show resolved Hide resolved
contrib/babelfishpg_tsql/src/pl_gram.y Outdated Show resolved Hide resolved
test/JDBC/expected/BABEL-4547.out Outdated Show resolved Hide resolved
Signed-off-by: Sandeep Kumawat <[email protected]>
Signed-off-by: Sandeep Kumawat <[email protected]>
@jsudrik jsudrik merged commit 2d52dc0 into babelfish-for-postgresql:BABEL_3_X_DEV Jan 2, 2024
30 checks passed
@skumawat2025 skumawat2025 deleted the babel-dev-4547 branch January 2, 2024 06:31
rishabhtanwar29 pushed a commit to amazon-aurora/babelfish_extensions that referenced this pull request Jan 8, 2024
…x) (babelfish-for-postgresql#2036)

This PR fixes a crash in TdsSendTypeNVarchar for nvarchar(max) by resolving a bug in handling varchar(max) , nvarchar(max) and varbinary(max) and limits a maximum scale value for (var)char, (var)binary, and (n)(var)char types. To do that, we set a limit of 8000 for (var)char and (var)binary, and a limit of 4000 for (n)(var)char.

Signed-off-by: Sandeep Kumawat [email protected]
Issues Resolved

BABEL-4547
rishabhtanwar29 pushed a commit to amazon-aurora/babelfish_extensions that referenced this pull request Jan 8, 2024
…x) (babelfish-for-postgresql#2036)

This PR fixes a crash in TdsSendTypeNVarchar for nvarchar(max) by resolving a bug in handling varchar(max) , nvarchar(max) and varbinary(max) and limits a maximum scale value for (var)char, (var)binary, and (n)(var)char types. To do that, we set a limit of 8000 for (var)char and (var)binary, and a limit of 4000 for (n)(var)char.

Signed-off-by: Sandeep Kumawat [email protected]
Issues Resolved

BABEL-4547
rishabhtanwar29 pushed a commit to amazon-aurora/babelfish_extensions that referenced this pull request Jan 8, 2024
…x) (babelfish-for-postgresql#2036)

This PR fixes a crash in TdsSendTypeNVarchar for nvarchar(max) by resolving a bug in handling varchar(max) , nvarchar(max) and varbinary(max) and limits a maximum scale value for (var)char, (var)binary, and (n)(var)char types. To do that, we set a limit of 8000 for (var)char and (var)binary, and a limit of 4000 for (n)(var)char.

Signed-off-by: Sandeep Kumawat [email protected]
Issues Resolved

BABEL-4547
rishabhtanwar29 pushed a commit to amazon-aurora/babelfish_extensions that referenced this pull request Jan 8, 2024
…x) (babelfish-for-postgresql#2036)

This PR fixes a crash in TdsSendTypeNVarchar for nvarchar(max) by resolving a bug in handling varchar(max) , nvarchar(max) and varbinary(max) and limits a maximum scale value for (var)char, (var)binary, and (n)(var)char types. To do that, we set a limit of 8000 for (var)char and (var)binary, and a limit of 4000 for (n)(var)char.

Signed-off-by: Sandeep Kumawat [email protected]
Issues Resolved

BABEL-4547
rishabhtanwar29 pushed a commit to amazon-aurora/babelfish_extensions that referenced this pull request Jan 8, 2024
…x) (babelfish-for-postgresql#2036)

This PR fixes a crash in TdsSendTypeNVarchar for nvarchar(max) by resolving a bug in handling varchar(max) , nvarchar(max) and varbinary(max) and limits a maximum scale value for (var)char, (var)binary, and (n)(var)char types. To do that, we set a limit of 8000 for (var)char and (var)binary, and a limit of 4000 for (n)(var)char.

Signed-off-by: Sandeep Kumawat [email protected]
Issues Resolved

BABEL-4547
rishabhtanwar29 pushed a commit to amazon-aurora/babelfish_extensions that referenced this pull request Jan 8, 2024
…x) (babelfish-for-postgresql#2036)

This PR fixes a crash in TdsSendTypeNVarchar for nvarchar(max) by resolving a bug in handling varchar(max) , nvarchar(max) and varbinary(max) and limits a maximum scale value for (var)char, (var)binary, and (n)(var)char types. To do that, we set a limit of 8000 for (var)char and (var)binary, and a limit of 4000 for (n)(var)char.

Signed-off-by: Sandeep Kumawat [email protected]
Issues Resolved

BABEL-4547
rishabhtanwar29 pushed a commit to amazon-aurora/babelfish_extensions that referenced this pull request Jan 8, 2024
…x) (babelfish-for-postgresql#2036)

This PR fixes a crash in TdsSendTypeNVarchar for nvarchar(max) by resolving a bug in handling varchar(max) , nvarchar(max) and varbinary(max) and limits a maximum scale value for (var)char, (var)binary, and (n)(var)char types. To do that, we set a limit of 8000 for (var)char and (var)binary, and a limit of 4000 for (n)(var)char.

Signed-off-by: Sandeep Kumawat [email protected]
Issues Resolved

BABEL-4547
rishabhtanwar29 pushed a commit to amazon-aurora/babelfish_extensions that referenced this pull request Jan 8, 2024
…x) (babelfish-for-postgresql#2036)

This PR fixes a crash in TdsSendTypeNVarchar for nvarchar(max) by resolving a bug in handling varchar(max) , nvarchar(max) and varbinary(max) and limits a maximum scale value for (var)char, (var)binary, and (n)(var)char types. To do that, we set a limit of 8000 for (var)char and (var)binary, and a limit of 4000 for (n)(var)char.

Signed-off-by: Sandeep Kumawat [email protected]
Issues Resolved

BABEL-4547
rishabhtanwar29 pushed a commit to amazon-aurora/babelfish_extensions that referenced this pull request Jan 8, 2024
…x) (babelfish-for-postgresql#2036)

This PR fixes a crash in TdsSendTypeNVarchar for nvarchar(max) by resolving a bug in handling varchar(max) , nvarchar(max) and varbinary(max) and limits a maximum scale value for (var)char, (var)binary, and (n)(var)char types. To do that, we set a limit of 8000 for (var)char and (var)binary, and a limit of 4000 for (n)(var)char.

Signed-off-by: Sandeep Kumawat [email protected]
Issues Resolved

BABEL-4547
shardgupta pushed a commit that referenced this pull request Jan 9, 2024
…x) (#2036)

This PR fixes a crash in TdsSendTypeNVarchar for nvarchar(max) by resolving a bug in handling varchar(max) , nvarchar(max) and varbinary(max) and limits a maximum scale value for (var)char, (var)binary, and (n)(var)char types. To do that, we set a limit of 8000 for (var)char and (var)binary, and a limit of 4000 for (n)(var)char.

Signed-off-by: Sandeep Kumawat [email protected]
Issues Resolved

BABEL-4547
staticlibs pushed a commit to wiltondb/babelfish_extensions that referenced this pull request Oct 20, 2024
…x) (babelfish-for-postgresql#2036)

This PR fixes a crash in TdsSendTypeNVarchar for nvarchar(max) by resolving a bug in handling varchar(max) , nvarchar(max) and varbinary(max) and limits a maximum scale value for (var)char, (var)binary, and (n)(var)char types. To do that, we set a limit of 8000 for (var)char and (var)binary, and a limit of 4000 for (n)(var)char.

Signed-off-by: Sandeep Kumawat [email protected]
Issues Resolved

BABEL-4547
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.

5 participants