-
Notifications
You must be signed in to change notification settings - Fork 4.8k
HIVE-29128 : Allow show tables format
="iceberg" command
#6021
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
base: master
Are you sure you want to change the base?
HIVE-29128 : Allow show tables format
="iceberg" command
#6021
Conversation
@deniskuzZ , @chinnaraolalam could you please take a look at this PR? |
@@ -53,17 +56,29 @@ public void analyzeInternal(ASTNode root) throws SemanticException { | |||
String tableNames = null; | |||
TableType tableTypeFilter = null; | |||
boolean isExtended = false; |
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.
nit: better to use isIceberg
, to have uniform variable naming as above line isExtended
if (type != null) { | ||
throw new HiveException("Iceberg Tables by default is External Table"); | ||
} else { | ||
if (pattern != null) { |
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.
nit: This looks cleaner.
if (ifIceberg) {
if (type != null) {
throw new HiveException("Iceberg tables are always of type EXTERNAL_TABLE");
}
String icebergPattern = (pattern != null) ? pattern : ".*";
List<TableName> icebergTables = getIcebergTables(getMSC(), null, dbName, icebergPattern).getTables();
return icebergTables.stream()
.map(TableName::getTable)
.collect(Collectors.toList());
}
Just thinking out loud: As the idea is to only do this for iceberg tables! Is it feasible to do something like this? |
Yes, it can be done. But to do this, Iceberg has to be exposed as a keyword in Hive which it is not atm. That would require changes in the alter and stored_by grammar as well. I purposely kept it this way to not make any changes on those layers. But yeah, i can make thee changes if there is a consensus here. |
ef65442
to
3196fa5
Compare
|
@@ -1974,6 +1996,15 @@ public List<String> getTablesByType(String dbName, String pattern, TableType typ | |||
|
|||
try { | |||
List<String> result; | |||
if (isIceberg) { | |||
if (type != null) { | |||
throw new HiveException("Iceberg Tables by default is External Table"); |
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.
should this be an exception in this case?
- can we show it like below?
# Table Name Table Type
test_iceberg EXTERNAL_TYPE
- Or you could handle it in analyse step itself -- In ShowTablesAnalyzer
What changes were proposed in this pull request?
Why are the changes needed?
Does this PR introduce any user-facing change?
How was this patch tested?