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

[native] Convert input field names to lower case in plan conversion #21265

Merged

Conversation

kevinwilfong
Copy link
Contributor

Presto is case insensitive, and so always converts column names and field names to lower case in its plan.

When Presto constructs the Velox query plan it passes in the types for Hive tables as they were read from the metastore, with capital letters if they were there in the metastore. Velox uses these types to construct the output type of the TableScan.

This leads to errors as the output of the TableScan will have capital letters in field names, while field accesses in the rest of the plan will use the field names Presto used in its original plan, which are all lower case.

Converting the field names from the types in the input Hive tables to lower case fixes this inconsistency.

@kevinwilfong kevinwilfong requested a review from a team as a code owner October 28, 2023 00:16
@kevinwilfong kevinwilfong force-pushed the native_fields_case_sensitive branch 3 times, most recently from 1c45dab to 5116aa8 Compare October 30, 2023 15:38
Copy link
Contributor

@xiaoxmeng xiaoxmeng left a comment

Choose a reason for hiding this comment

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

@kevinwilfong LGTM. Thanks!

@kevinwilfong kevinwilfong force-pushed the native_fields_case_sensitive branch 2 times, most recently from ee1a432 to 0b05eb5 Compare November 1, 2023 18:34
Presto is case insensitive, and so always converts column names and field names
to lower case in its plan.

When Presto constructs the Velox query plan it passes in the types for Hive
tables as they were read from the metastore, with capital letters if they were
there in the metastore. Velox uses these types to construct the output type of
the TableScan.

This leads to errors as the output of the TableScan will have capital letters in
field names, while field accesses in the rest of the plan will use the field
names Presto used in its original plan, which are all lower case.

Converting the field names from the types in the input Hive tables to lower case
fixes this inconsistency.
@kevinwilfong kevinwilfong force-pushed the native_fields_case_sensitive branch from 0b05eb5 to b69c7c6 Compare November 13, 2023 22:54
Copy link
Contributor

@amitkdutta amitkdutta left a comment

Choose a reason for hiding this comment

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

Thanks @kevinwilfong

@amitkdutta amitkdutta merged commit 81850dc into prestodb:master Nov 14, 2023
@wanglinsong wanglinsong mentioned this pull request Dec 8, 2023
26 tasks
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.

None yet

4 participants