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

Support non-hive types in hive views #15065

Merged
merged 2 commits into from
Sep 1, 2020
Merged

Support non-hive types in hive views #15065

merged 2 commits into from
Sep 1, 2020

Conversation

rschlussel
Copy link
Contributor

Test plan - unit test

== RELEASE NOTES ==


Hive Changes
* Add support for non-hive types to hive views.  This support had been removed in 0.233.  If a view uses an unsupported type for any columns ,we will store only a single dummy column for that view in the metastore.

mayankgarg1990
mayankgarg1990 suggested changes Aug 24, 2020
View reviewed changes
Copy link
Contributor

@mayankgarg1990 mayankgarg1990 left a comment

Choose a reason for hiding this comment

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

Proper messaging is necessary in my opinion to clarify why we have the dummy instead of a proper field.

presto-hive/src/main/java/com/facebook/presto/hive/HiveMetadata.java Outdated
Comment on lines 1986 to 1988
if (e.getErrorCode().equals(NOT_SUPPORTED.toErrorCode())) {
columns = ImmutableList.of(new Column("dummy", HIVE_STRING, Optional.empty()));
}
Copy link
Contributor

Choose a reason for hiding this comment

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

we should add a comment explaining why we have the dummy instead of proper column metadata. In my opinion, the message should be very clear - Using dummy since column "abc" has an unsupported Hive type "xyz" to avoid getting questions around why is my column metadata missing.

@rschlussel
Suport non-hive types for Hive views
8d43701 
When a non-hive type is used for any column in a hive view, include only
a single dummy column in the metastore.
@rschlussel
Fix formatting in TestHiveIntegrationSmokeTest
fcacca6
@rschlussel
Copy link
Contributor Author

thanks @mayankgarg1990 for the review. updated

@rschlussel rschlussel requested a review from arhimondr September 1, 2020 16:16
@rschlussel rschlussel merged commit 4375cd2 into prestodb:master Sep 1, 2020
@caithagoras caithagoras mentioned this pull request Sep 9, 2020
Merged
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mayankgarg1990 mayankgarg1990 mayankgarg1990 approved these changes

@arhimondr arhimondr arhimondr approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@rschlussel @arhimondr @mayankgarg1990