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

SqlServer TINYINT incorrect results #11209

Closed
tangjiangling opened this issue Feb 27, 2022 · 4 comments · Fixed by #11274
Closed

SqlServer TINYINT incorrect results #11209

tangjiangling opened this issue Feb 27, 2022 · 4 comments · Fixed by #11274
Assignees
Labels
bug Something isn't working correctness

Comments

@tangjiangling
Copy link
Member

tangjiangling commented Feb 27, 2022

We mapped Trino TINYINT to SqlServer TINYINT, but I found that the mapping relationship is wrong, why?

  • In Trino, the range of TINYINT is [-128, 127]
  • In SqlServer, the range of TINYINT is [0, 255]

So we can't read or write TINYINT values of [128, 255] through Trino.

Assuming I have a table test_table (row_id integer, data tinyint) in SqlServer, and the following data in the table:

1 127
2 128

Then, if I read it through Trino, I will get the following result:

1 127
2 -128 <- error value

Since Trino's TINYINT supports negative numbers, what happens if I write a negative number to SqlServer via Trino? (We can't write negative numbers via SqlServer JDBC Statement)

INSERT INTO test_table VALUES (3, -1)

query via SqlServer JDBC:

1 127
2 128
3 255 <- error value

query via Trino:

1 127
2 -128
3 -1 <- error value
@tangjiangling
Copy link
Member Author

Background

I found this bug while fixing this issue, so I filed an issue to track the problem and I will send a PR to fix it.

@tangjiangling tangjiangling self-assigned this Feb 27, 2022
@findepi findepi added the bug Something isn't working label Feb 27, 2022
@findepi
Copy link
Member

findepi commented Feb 27, 2022

In SqlServer, the range of TINYINT is [0, 255]

so it's effectively "unsigned tinyint" and should be mapped to smallint.
Note that we dom't have unsigned numeric types.

Since Trino's TINYINT supports negative numbers, what happens if I write a negative number to SqlServer via Trino?

Failure. We don't need to do validation, since SQL Server will do it (right?)

This is how "unsigned tinyint" is handled in MySQL connector

if (typeName.equalsIgnoreCase("tinyint unsigned")) {
return Optional.of(smallintColumnMapping());

@tangjiangling
Copy link
Member Author

tangjiangling commented Feb 28, 2022

Failure. We don't need to do validation, since SQL Server will do it (right?)

No, I have confirmed that if we use Statement, SqlServer will check if the value range is correct, but with PreparedStatement, SqlServer does not check the value range, and in Trino we are using PreparedStatement, so we should have to check the value range in Trino.

This is how "unsigned tinyint" is handled in MySQL connector

Got it, thank you for your guidance.

@tangjiangling
Copy link
Member Author

No, I have confirmed that if we use Statement, SqlServer will check if the value range is correct, but with PreparedStatement, SqlServer does not check the value range, and in Trino we are using PreparedStatement, so we should have to check the value range in Trino.

This conclusion is wrong, SQL Server will check the range of values.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working correctness
Development

Successfully merging a pull request may close this issue.

2 participants