Vihang Karajgaonkar has posted comments on this change. ( http://gerrit.cloudera.org:8080/15290 )
Change subject: IMPALA-9363: Add support for skipping given table types ...................................................................... Patch Set 1: (7 comments) http://gerrit.cloudera.org:8080/#/c/15290/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/15290/1//COMMIT_MSG@14 PS1, Line 14: --catalogd_args=--blacklisted_table_types=<list_of_blacklisted_types>'. : Five table types are supported, namely, 'hdfs', 'hbase', 'view', : 'data_source', and 'kudu I think this is a very broad way to ignore the table types. Rather than ignoring the entire type of a table, can we ignore the table based on its Serde? For instance, if the user only wants to ignore only json tables it is not possible to do that with the patch except for ignoring all the HDFSTables. http://gerrit.cloudera.org:8080/#/c/15290/1/be/src/catalog/catalog.cc File be/src/catalog/catalog.cc: http://gerrit.cloudera.org:8080/#/c/15290/1/be/src/catalog/catalog.cc@54 PS1, Line 54: List of blacklisted table types Can you add more information here? Specifically, mention that this is a comma separate string where each value represents the table type (or fully qualified SerDe class names). Also, make sure to mention that the tables which are listed here will not be discovered by Catalog server. http://gerrit.cloudera.org:8080/#/c/15290/1/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java File fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java: http://gerrit.cloudera.org:8080/#/c/15290/1/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@1486 PS1, Line 1486: continue; may be add a info log which says that this table will be ignored because it is in the blacklisted table type. http://gerrit.cloudera.org:8080/#/c/15290/1/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@3085 PS1, Line 3085: private TTableType getTableType(org.apache.hadoop.hive.metastore.api.Table msTbl) { change to static method? http://gerrit.cloudera.org:8080/#/c/15290/1/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@3101 PS1, Line 3101: HBASE_TABLE Why do we return Hbase table type here? Can you please reconfirm? http://gerrit.cloudera.org:8080/#/c/15290/1/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@3105 PS1, Line 3105: return null; this line with throw a NPE at 1485. May be just ignore the table if you are unable to determine its type here. http://gerrit.cloudera.org:8080/#/c/15290/1/fe/src/main/java/org/apache/impala/util/CatalogBlacklistUtils.java File fe/src/main/java/org/apache/impala/util/CatalogBlacklistUtils.java: http://gerrit.cloudera.org:8080/#/c/15290/1/fe/src/main/java/org/apache/impala/util/CatalogBlacklistUtils.java@121 PS1, Line 121: else { nit, move to line 120 after } -- To view, visit http://gerrit.cloudera.org:8080/15290 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I49c4062b48f1bb87adfd851ee26cc144fb70b4b7 Gerrit-Change-Number: 15290 Gerrit-PatchSet: 1 Gerrit-Owner: Fang-Yu Rao <[email protected]> Gerrit-Reviewer: Fang-Yu Rao <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Quanlong Huang <[email protected]> Gerrit-Reviewer: Vihang Karajgaonkar <[email protected]> Gerrit-Comment-Date: Tue, 25 Feb 2020 20:11:52 +0000 Gerrit-HasComments: Yes
