Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/20669 )
Change subject: IMPALA-3268: Add support for SHOW VIEWS statement ...................................................................... Patch Set 2: (4 comments) > Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/9908/ The failure is ERROR: Some python files contain these banned pylint warnings: tests/custom_cluster/test_show_views_statements.py:20:0: W1618: import missing `from __future__ import absolute_import` (no-absolute-import) http://gerrit.cloudera.org:8080/#/c/20669/2/be/src/service/client-request-state.cc File be/src/service/client-request-state.cc: http://gerrit.cloudera.org:8080/#/c/20669/2/be/src/service/client-request-state.cc@370 PS2, Line 370: case TCatalogOpType::SHOW_VIEWS: { It seems the only difference between SHOW_VIEWS and the above SHOW_TABLES clause is whether to use 'params->views_only'. Can we merge them into one? http://gerrit.cloudera.org:8080/#/c/20669/2/be/src/service/frontend.h File be/src/service/frontend.h: http://gerrit.cloudera.org:8080/#/c/20669/2/be/src/service/frontend.h@79 PS2, Line 79: const bool views_only); We can set a default value as 'views_only=false' then don't need to override the function. BTW, don't need 'const' for scalar parameter. http://gerrit.cloudera.org:8080/#/c/20669/2/fe/src/main/java/org/apache/impala/analysis/ShowTablesStmt.java File fe/src/main/java/org/apache/impala/analysis/ShowTablesStmt.java: http://gerrit.cloudera.org:8080/#/c/20669/2/fe/src/main/java/org/apache/impala/analysis/ShowTablesStmt.java@43 PS2, Line 43: getParsedDb getPattern? Can we add some tests in ToSqlTest? http://gerrit.cloudera.org:8080/#/c/20669/2/fe/src/main/java/org/apache/impala/service/Frontend.java File fe/src/main/java/org/apache/impala/service/Frontend.java: http://gerrit.cloudera.org:8080/#/c/20669/2/fe/src/main/java/org/apache/impala/service/Frontend.java@593 PS2, Line 593: else if (analysis.isShowDbsStmt()) { nit: move to the above line to be right after "} " -- To view, visit http://gerrit.cloudera.org:8080/20669 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I321fc5350392a815949a4e7d2a64d60466689788 Gerrit-Change-Number: 20669 Gerrit-PatchSet: 2 Gerrit-Owner: Fang-Yu Rao <fangyu....@cloudera.com> Gerrit-Reviewer: Aman Sinha <amsi...@cloudera.com> Gerrit-Reviewer: Fang-Yu Rao <fangyu....@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Michael Smith <michael.sm...@cloudera.com> Gerrit-Reviewer: Quanlong Huang <huangquanl...@gmail.com> Gerrit-Reviewer: Sai Hemanth Gantasala <saihema...@cloudera.com> Gerrit-Comment-Date: Thu, 09 Nov 2023 08:15:03 +0000 Gerrit-HasComments: Yes
