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.

Sign up for GitHub

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

Jump to bottom

Do not truncate while encoding values as varchar literal #10172

Merged
merged 1 commit into from
Dec 7, 2021
Merged

Do not truncate while encoding values as varchar literal #10172

merged 1 commit into from
Dec 7, 2021

Conversation

kasiafi
Copy link
Member

@kasiafi kasiafi commented Dec 3, 2021 •
edited
Loading

Related to #552

@cla-bot cla-bot bot added the cla-signed label Dec 3, 2021
findepi
findepi reviewed Dec 3, 2021
View reviewed changes
Copy link
Member

@findepi findepi left a comment

Choose a reason for hiding this comment

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

last commit (Do not truncate while encoding values as varchar literal) LGTM.
i think it can go independently from the others

core/trino-main/src/main/java/io/trino/sql/planner/LiteralEncoder.java Outdated
if (boundedLength > valueLength) {
return new Cast(stringLiteral, toSqlType(type), false, true);
}
throw new IllegalArgumentException(format("value %s does not fit in type %s", value.toStringUtf8(), varcharType));
Copy link
Member

Choose a reason for hiding this comment

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

value %s -> Value [%s]

@kasiafi
Do not truncate while encoding values as varchar literal
6c84aa5 
Before this change, the LiteralEncoder truncated values which did
not fit in the expected varchar type.
This behavior led to silent correctness issue: certain cast
operators returned incorrect values (inconsistent with the declared
return type). The values were then adjusted by the LiteralEncoder
so that the query didn't fail.
Those cast operators are now fixed. Removing the truncating behavior
from the LiteralEncoder should expose any other existing issues
of this kind as well as prevent adding incorrect functions in the
future.
@kasiafi
Copy link
Member Author

kasiafi commented Dec 7, 2021

@findepi AC

@kasiafi kasiafi merged commit d3456a8 into trinodb:master Dec 7, 2021
@github-actions github-actions bot added this to the 366 milestone Dec 7, 2021
@takezoe takezoe mentioned this pull request Feb 5, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@findepi findepi findepi approved these changes

Assignees
No one assigned
Labels
cla-signed
Milestone
366
Development

Successfully merging this pull request may close these issues.
2 participants
@kasiafi @findepi