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

[#2035] fix(trino-connector): Fix type conversion about type varhcar and char for PG catalog #2056

Closed
wants to merge 1 commit into from

Conversation

yuqi1129
Copy link
Contributor

@yuqi1129 yuqi1129 commented Feb 5, 2024

What changes were proposed in this pull request?

Remove logic that convert varchar and char to type string.

Why are the changes needed?

Type conversion should be aligned with their definition.

Fix: #2035

Does this PR introduce any user-facing change?

N/A.

How was this patch tested?

new IT cases in trino-ci-testset

@yuqi1129 yuqi1129 requested a review from diqiu50 February 5, 2024 02:04
@yuqi1129 yuqi1129 self-assigned this Feb 5, 2024
@yuqi1129 yuqi1129 added this to the Gravitino 0.4.0 milestone Feb 5, 2024
@yuqi1129 yuqi1129 added need backport Issues that need to backport to another branch branch-0.4 labels Feb 5, 2024
@jerryshao
Copy link
Contributor

@diqiu50 please help to review this.

@@ -5,6 +5,35 @@ CREATE TABLE "test.gt_postgresql".gt_db1.tb01 (
salary int
);

Copy link
Contributor

Choose a reason for hiding this comment

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

Can we move the varchar test to a new file?

);

show create table "test.gt_postgresql".gt_db1.tb04;

Copy link
Contributor

Choose a reason for hiding this comment

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

We discussed this issue before In Trino, and the conclusion should be..

varchar ==> string
varchar(n) ==> varchar(n) if n > limit  string

char == char (char(1))
char(n) = char(n) if n > limit  throw exception

@yuqi1129
Copy link
Contributor Author

yuqi1129 commented Feb 5, 2024

@jerryshao
I will move this out of release 0.4.0. There is something unsettled about Trino varchar type.

@yuqi1129 yuqi1129 removed need backport Issues that need to backport to another branch branch-0.4 labels Feb 5, 2024
@jerryshao
Copy link
Contributor

@jerryshao I will move this out of release 0.4.0. There is something unsettled about Trino varchar type.

OK.

@yuqi1129 yuqi1129 marked this pull request as draft February 5, 2024 04:49
@yuqi1129
Copy link
Contributor Author

No need any more.

@yuqi1129 yuqi1129 closed this Feb 28, 2024
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.

[Bug report] Varchar type of PG catalog is not handle correctly by Gravitino
3 participants