kasakrisz commented on code in PR #5798:
URL: https://github.com/apache/hive/pull/5798#discussion_r2074834833
##########
ql/src/java/org/apache/hadoop/hive/ql/metadata/Table.java:
##########
@@ -1364,4 +1365,16 @@ public SourceTable createSourceTable() {
sourceTable.setDeletedCount(0L);
return sourceTable;
}
+
+ public boolean isDruidTable() {
+ return isNonNative() &&
getStorageHandler().toString().equals(Constants.DRUID_HIVE_STORAGE_HANDLER_ID);
+ }
+
+ public boolean isJdbcTable() {
+ return isNonNative() &&
getStorageHandler().toString().equals(Constants.JDBC_HIVE_STORAGE_HANDLER_ID);
+ }
Review Comment:
There are two issues with this solution:
1. The equality check
```
getStorageHandler().toString().equals(Constants.DRUID_HIVE_STORAGE_HANDLER_ID)
```
is not null safe. We have to know the implementation of `isNonNative` to
consider it null safe.
It was before the refactor:
```
private TableType obtainTableType(Table tabMetaData) {
if (tabMetaData.getStorageHandler() != null) {
final String storageHandlerStr =
tabMetaData.getStorageHandler().toString();
```
2. Every time a new table format is added a new method has to be created.
Similarly a new enum constant had to be added before this change.
IIUC this is needed for creating the `RelOptHiveTable` object so the `if`
blocks should be moved to some factory and/or converter(QB to RelNode) class to
get rid of the enum. However it seems to be out of the scope of this patch or
I'm missing something.
Could you please share how this refactor is related to collecting the
virtual columns?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
For additional commands, e-mail: gitbox-h...@hive.apache.org
