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

Retain result type after UNION as base type in T-SQL #3175

Conversation

hopebo
Copy link
Contributor

@hopebo hopebo commented Nov 27, 2024

Description

When a base type such as money is UNIONed with another integer type, the result type will be fixeddecimal instead of money, which is inconsistent with SQL Server's behaviour. And could also cause some characters not displaying well due to inappropriate result type.

Issues Resolved

The root cause of the issue is Postgres will consider the base type of domain type as the result type. In Babelfish, money is implemented as a domain type of fixeddecimal. Actually, money is a base type in T-SQL, thus Babelfish should retain money type after UNION.

After a final type has determined by the optimizer, check if it's a base type in T-SQL. If it is, try to use the domain type as result type if the domain type has higher precision than other types.

BABEL-5242

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.

@coveralls
Copy link
Collaborator

coveralls commented Nov 29, 2024

Pull Request Test Coverage Report for Build 12362083072

Details

  • 7 of 7 (100.0%) changed or added relevant lines in 1 file are covered.
  • 28 unchanged lines in 1 file lost coverage.
  • Overall coverage increased (+0.002%) to 74.824%

Files with Coverage Reduction New Missed Lines %
contrib/babelfishpg_tsql/src/pltsql_coerce.c 28 81.5%
Totals Coverage Status
Change from base Build 12352168391: 0.002%
Covered Lines: 46440
Relevant Lines: 62066

💛 - Coveralls

@hopebo hopebo force-pushed the jira-babel-5242 branch 2 times, most recently from 8dbe378 to 74a23e8 Compare December 4, 2024 22:10
@hopebo hopebo requested a review from forestkeeper December 6, 2024 23:41
@hopebo hopebo requested a review from 2jungkook December 7, 2024 01:26
@hopebo hopebo force-pushed the jira-babel-5242 branch 2 times, most recently from ad794df to 93a0aee Compare December 13, 2024 00:47
Description
-----------
When a base type such as `money` is UNIONed with another integer type,
the result type will be `fixeddecimal` instead of `money`, which is
inconsistent with SQL Server's behaviour. And could also cause some
characters not displaying well.

Analysis
--------
The root cause of the issue is Postgres will consider the base type of
domain type as the result type. In Babelfish, `money` is implemented
as a domain type of `fixeddecimal`. Actually, `money` is a base type
in T-SQL, thus Babelfish should retain `money` type after UNION.

Fix
---
After a final type has determined by the optimizer, check if it's a
base type in T-SQL. If it is, try to use the domain type as result
type if the domain type has higher precision than other types.

Task: BABEL-5242
Signed-off-by: Bo Li <[email protected]>
GO
~~ERROR (Code: 33557097)~~

~~ERROR (Message: invalid input syntax for type numeric: "unknown")~~
Copy link
Contributor

Choose a reason for hiding this comment

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

shouldn't this be resolved as money??

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For this SQL, the expected behaviour should be raising an error. This is the output from SQL Server:

mssql@f209a6999266:/$ /opt/mssql-tools18/bin/sqlcmd -S localhost -U SA -P "<YourStrong@Passw0rd>" -C
1> SELECT 'unknown'
2> UNION
3> SELECT $1.0
4> UNION
5> SELECT N'xyz';
6> GO

Msg 235, Level 16, State 0, Server f209a6999266, Line 1
Cannot convert a char value to money. The char value has incorrect syntax.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the output from Babel:

dev-dsk-zboli-2c-08732257 % sqlcmd -S localhost -U jdbc_user -P '12345678'
1> CREATE VIEW TestView AS SELECT traamo = $1.0;
2> GO
1> SELECT c.[name] AS [sys_types_user_type_id],
2>        TYPE_NAME(a.user_type_id) AS type_name_function_user_type_id
3> FROM sys.columns a
4>     INNER JOIN sys.objects b
5>         ON a.[object_id] = b.[object_id]
6>     LEFT JOIN sys.types c
7>         ON a.user_type_id = c.user_type_id
8> WHERE b.[name] = 'TestView'
9>     AND a.[name] = 'traamo';
10> GO
sys_types_user_type_id                                                                                                           type_name_function_user_type_id
-------------------------------------------------------------------------------------------------------------------------------- ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
numeric                                                                                                                          numeric

Copy link
Contributor Author

Choose a reason for hiding this comment

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

SQL Server:

1> CREATE VIEW TestView AS SELECT traamo = $1.0 FROM t1;
2> GO
1> SELECT c.[name] AS [sys_types_user_type_id],
2>        TYPE_NAME(a.user_type_id) AS type_name_function_user_type_id
3> FROM sys.columns a
4>     INNER JOIN sys.objects b
5>         ON a.[object_id] = b.[object_id]
6>     LEFT JOIN sys.types c
7>         ON a.user_type_id = c.user_type_id
8> WHERE b.[name] = 'TestView'
9>     AND a.[name] = 'traamo';
10> GO
sys_types_user_type_id                                                                                                           type_name_function_user_type_id
-------------------------------------------------------------------------------------------------------------------------------- --------------------------------------------------------------------------------------------------------------------------------
money                                                                                                                            money

@Deepesh125 Deepesh125 merged commit e089337 into babelfish-for-postgresql:BABEL_5_X_DEV Dec 17, 2024
44 checks passed
hopebo added a commit to amazon-aurora/babelfish_extensions that referenced this pull request Dec 17, 2024
…ostgresql#3175)

Description
-----------
When a base type such as `money` is UNIONed with another integer type,
the result type will be `fixeddecimal` instead of `money`, which is
inconsistent with SQL Server's behaviour. And could also cause some
characters not displaying well.

Analysis
--------
The root cause of the issue is Postgres will consider the base type of
domain type as the result type. In Babelfish, `money` is implemented
as a domain type of `fixeddecimal`. Actually, `money` is a base type
in T-SQL, thus Babelfish should retain `money` type after UNION.

Fix
---
After a final type has determined by the optimizer, check if it's a
base type in T-SQL. If it is, try to use the domain type as result
type if the domain type has higher precision than other types.

Task: BABEL-5242
Signed-off-by: Bo Li <[email protected]>
hopebo added a commit to amazon-aurora/babelfish_extensions that referenced this pull request Dec 17, 2024
…ostgresql#3175)

Description
-----------
When a base type such as `money` is UNIONed with another integer type,
the result type will be `fixeddecimal` instead of `money`, which is
inconsistent with SQL Server's behaviour. And could also cause some
characters not displaying well.

Analysis
--------
The root cause of the issue is Postgres will consider the base type of
domain type as the result type. In Babelfish, `money` is implemented
as a domain type of `fixeddecimal`. Actually, `money` is a base type
in T-SQL, thus Babelfish should retain `money` type after UNION.

Fix
---
After a final type has determined by the optimizer, check if it's a
base type in T-SQL. If it is, try to use the domain type as result
type if the domain type has higher precision than other types.

Task: BABEL-5242
Signed-off-by: Bo Li <[email protected]>
hopebo added a commit to amazon-aurora/babelfish_extensions that referenced this pull request Dec 17, 2024
…ostgresql#3175)

Description
-----------
When a base type such as `money` is UNIONed with another integer type,
the result type will be `fixeddecimal` instead of `money`, which is
inconsistent with SQL Server's behaviour. And could also cause some
characters not displaying well.

Analysis
--------
The root cause of the issue is Postgres will consider the base type of
domain type as the result type. In Babelfish, `money` is implemented
as a domain type of `fixeddecimal`. Actually, `money` is a base type
in T-SQL, thus Babelfish should retain `money` type after UNION.

Fix
---
After a final type has determined by the optimizer, check if it's a
base type in T-SQL. If it is, try to use the domain type as result
type if the domain type has higher precision than other types.

Task: BABEL-5242
Signed-off-by: Bo Li <[email protected]>
hopebo added a commit to amazon-aurora/babelfish_extensions that referenced this pull request Dec 18, 2024
…ostgresql#3175)

Description
-----------
When a base type such as `money` is UNIONed with another integer type,
the result type will be `fixeddecimal` instead of `money`, which is
inconsistent with SQL Server's behaviour. And could also cause some
characters not displaying well.

Analysis
--------
The root cause of the issue is Postgres will consider the base type of
domain type as the result type. In Babelfish, `money` is implemented
as a domain type of `fixeddecimal`. Actually, `money` is a base type
in T-SQL, thus Babelfish should retain `money` type after UNION.

Fix
---
After a final type has determined by the optimizer, check if it's a
base type in T-SQL. If it is, try to use the domain type as result
type if the domain type has higher precision than other types.

Task: BABEL-5242
Signed-off-by: Bo Li <[email protected]>
shardgupta pushed a commit that referenced this pull request Dec 19, 2024
Description
-----------
When a base type such as `money` is UNIONed with another integer type,
the result type will be `fixeddecimal` instead of `money`, which is
inconsistent with SQL Server's behaviour. And could also cause some
characters not displaying well.

Analysis
--------
The root cause of the issue is Postgres will consider the base type of
domain type as the result type. In Babelfish, `money` is implemented
as a domain type of `fixeddecimal`. Actually, `money` is a base type
in T-SQL, thus Babelfish should retain `money` type after UNION.

Fix
---
After a final type has determined by the optimizer, check if it's a
base type in T-SQL. If it is, try to use the domain type as result
type if the domain type has higher precision than other types.

Task: BABEL-5242

Signed-off-by: Bo Li <[email protected]>
shardgupta pushed a commit that referenced this pull request Dec 19, 2024
Description
-----------
When a base type such as `money` is UNIONed with another integer type,
the result type will be `fixeddecimal` instead of `money`, which is
inconsistent with SQL Server's behaviour. And could also cause some
characters not displaying well.

Analysis
--------
The root cause of the issue is Postgres will consider the base type of
domain type as the result type. In Babelfish, `money` is implemented
as a domain type of `fixeddecimal`. Actually, `money` is a base type
in T-SQL, thus Babelfish should retain `money` type after UNION.

Fix
---
After a final type has determined by the optimizer, check if it's a
base type in T-SQL. If it is, try to use the domain type as result
type if the domain type has higher precision than other types.

Task: BABEL-5242

Signed-off-by: Bo Li <[email protected]>
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.

3 participants