Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/18626 )
Change subject: IMPALA-9670: Fix unloaded views are shown as tables for GET_TABLES requests ...................................................................... Patch Set 4: (4 comments) Thanks for Yu-Wen's feedbacks! Addressed the comments. http://gerrit.cloudera.org:8080/#/c/18626/3/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java File fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java: http://gerrit.cloudera.org:8080/#/c/18626/3/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@46 PS3, Line 46: import org.apache.hadoop.hive.metastore.api.ColumnStatistics; > Maybe TableType can be removed as well? Done http://gerrit.cloudera.org:8080/#/c/18626/3/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/18626/3/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@1903 PS3, Line 1903: String tableName = tblMeta.getTableName().toLowerCase(); Yeah, we can save the other toLowerCase() calls. As mentioned in our doc: > Impala identifiers are always case-insensitive. That is, tables named t1 and > T1 always refer to the same table, regardless of quote characters. > Internally, Impala always folds all specified table and column names to > lowercase. https://impala.apache.org/docs/build/html/topics/impala_identifiers.html http://gerrit.cloudera.org:8080/#/c/18626/3/fe/src/main/java/org/apache/impala/catalog/Hive3MetastoreShimBase.java File fe/src/main/java/org/apache/impala/catalog/Hive3MetastoreShimBase.java: http://gerrit.cloudera.org:8080/#/c/18626/3/fe/src/main/java/org/apache/impala/catalog/Hive3MetastoreShimBase.java@290 PS3, Line 290: case MATERIALIZED_VIEW: > I figure I should ask the question here because sometimes we treat material Yeah, we treat materialized views as tables *internally*, i.e. we won't expand the view but use the underlying files directly. However, when exposing the table types to the user, materialized views are still views. I think this is consistent with Hive since I see HUE shows the view icons for materialized views in the Hive editor. BTW, TImpalaTableType is only used for the table types in users' perspective. I'll add some commends in the thrift file. http://gerrit.cloudera.org:8080/#/c/18626/3/tests/common/impala_test_suite.py File tests/common/impala_test_suite.py: http://gerrit.cloudera.org:8080/#/c/18626/3/tests/common/impala_test_suite.py@119 PS3, Line 119: IMPALAD_HOSTNAME_LIST[i] + ':' + > I might missed something. Could you explain why we need to calculate port h We calculate the hs2 ports and hs2-http ports based on the specified beeswax ports. The tests can be ran on some specifit impalads, configured by the --impalad option which is a comma-separated list of impalad Beeswax host:ports to target. More options can be found by running 'impala-py.test --help'. There are options for the hs2 port and hs2-http port (--impalad_hs2_port and --impalad_hs2_http_port). They are considered the ports of the first impalad. We calculate the hs2 ports of other impalads based on the delta of their beeswax ports and the first impalad's beeswax port. E.g. given --impalad=localhost:21000,localhost:21002 --impalad_hs2_port=21050 --impalad_hs2_http_port=28000 We know the tests are run on 2 impalads. The hs2 port of the second impalad is 21052. The hs2-http port of the second impalad is 28002. I'll add some comments for this. -- To view, visit http://gerrit.cloudera.org:8080/18626 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I528bb20272ebdd66a0118c30efc2b0566f2b0e2f Gerrit-Change-Number: 18626 Gerrit-PatchSet: 4 Gerrit-Owner: Quanlong Huang <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Quanlong Huang <[email protected]> Gerrit-Reviewer: Yu-Wen Lai <[email protected]> Gerrit-Comment-Date: Sun, 19 Jun 2022 12:28:27 +0000 Gerrit-HasComments: Yes
