[SPARK-18989][SQL] DESC TABLE should not fail with format class not found #16388
Conversation
|
Test build #70548 has finished for PR 16388 at commit
|
| inputFormat = Option(h.getInputFormatClass).map(_.getName), | ||
| outputFormat = Option(h.getOutputFormatClass).map(_.getName), | ||
| inputFormat = Option(h.getTTable.getSd.getInputFormat), | ||
| outputFormat = Option(h.getTTable.getSd.getOutputFormat), |
There was a problem hiding this comment.
I checked the impl of getInputFormatClass and getOutputFormatClass . It has extra logics if either getTTable.getSd.getInputFormat / getTTable.getSd.getOutputFormat is null.
I checked the change history. These extra logics were added in https://issues.apache.org/jira/browse/HIVE-1122, https://issues.apache.org/jira/browse/HIVE-705, and https://issues.apache.org/jira/browse/HIVE-5260.
There was a problem hiding this comment.
When we actually read the hive table, we still use getInputFormatClass. So this will only affect the DESC TABLE, and should be OK?
There was a problem hiding this comment.
After more readings, getTTable.getSd.getInputFormat and getTTable.getSd.getOutputFormat will be null for non-native Hive tables, e.g., JDBC tables, HBase tables and Cassandra tables. See the link for more details: https://cwiki.apache.org/confluence/display/Hive/StorageHandlers
So far, this is OK. I am just afraid we might expand the usage of getTableOption in the future. Maybe at least document the restrictions?
|
Test build #70581 has started for PR 16388 at commit |
|
retest this please |
|
Test build #70595 has finished for PR 16388 at commit
|
|
LGTM |
|
Thanks! Merging to master. |
| // the class name directly. However, for non-native tables, there is no interface to get | ||
| // the format class name, so we may still throw ClassNotFound in this case. | ||
| inputFormat = Option(h.getTTable.getSd.getInputFormat).orElse { | ||
| Option(h.getStorageHandler).map(_.getInputFormatClass.getName) |
There was a problem hiding this comment.
Is it actually also fixed a bug (we did not look for storage handler)? Is it possible to have a test?
There was a problem hiding this comment.
no it's not a bug, getInputFormatClass will look for storage handler.
…ound ## What changes were proposed in this pull request? When we describe a table, we only wanna see the information of this table, not read it, so it's ok even if the format class is not present at the classpath. ## How was this patch tested? new regression test Author: Wenchen Fan <[email protected]> Closes apache#16388 from cloud-fan/hive.
…ound ## What changes were proposed in this pull request? When we describe a table, we only wanna see the information of this table, not read it, so it's ok even if the format class is not present at the classpath. ## How was this patch tested? new regression test Author: Wenchen Fan <[email protected]> Closes apache#16388 from cloud-fan/hive.
What changes were proposed in this pull request?
When we describe a table, we only wanna see the information of this table, not read it, so it's ok even if the format class is not present at the classpath.
How was this patch tested?
new regression test