-
Notifications
You must be signed in to change notification settings - Fork 3k
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
Allow using legacy Hive view translation logic #6195
Allow using legacy Hive view translation logic #6195
Conversation
@@ -38,7 +38,7 @@ | |||
ImmutableNationTable.class, | |||
ImmutableOrdersTable.class, | |||
}) | |||
public class TestHiveViews | |||
public abstract class BaseTestHiveViews |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not super convinced that we need this base class.
Maybe just leave TestHiveViews
as is, and use it for testing default configuration (with Coral).
And add TestHiveViewsLegacy
based on how TestHiveViews
looked before Coral was introduced.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That would work too.
4a616eb
to
05822c4
Compare
presto-hive/src/main/java/io/prestosql/plugin/hive/ViewReaderUtil.java
Outdated
Show resolved
Hide resolved
} | ||
|
||
@Test(groups = HIVE_VIEWS) | ||
public void testShowCreateView() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did you mean to inherit these tests from the base class?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Those are slightly differnt actually.
On the second though I decided to just have separate test classes and drop common base.
05822c4
to
1883af4
Compare
No description provided.