Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/17521 )
Change subject: IMPALA-10700: Add query options to skip deleting stats ...................................................................... Patch Set 1: Code-Review+2 (7 comments) LGTM. Just have some minor comments. Feel free to forward my +2. http://gerrit.cloudera.org:8080/#/c/17521/1/be/src/service/query-options.h File be/src/service/query-options.h: http://gerrit.cloudera.org:8080/#/c/17521/1/be/src/service/query-options.h@248 PS1, Line 248: QUERY_OPT_FN(skip_delete_table_stats, SKIP_DELETE_TABLE_STATS,\ : TQueryOptionLevel::ADVANCED)\ : QUERY_OPT_FN(skip_delete_column_stats, SKIP_DELETE_COLUMN_STATS,\ : TQueryOptionLevel::ADVANCED) nit: Can we mention TRUNCATE in the names since these only affects TRUNCATE? e.g. SKIP_DELETE_TABLE_STATS_IN_TRUNCATE SKIP_DELETE_COLUMN_STATS_IN_TRUNCATE http://gerrit.cloudera.org:8080/#/c/17521/1/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java File fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java: http://gerrit.cloudera.org:8080/#/c/17521/1/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@184 PS1, Line 184: import org.apache.impala.thrift.TQueryOptions; nit: unused import http://gerrit.cloudera.org:8080/#/c/17521/1/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@2193 PS1, Line 2193: to take nit: after taking http://gerrit.cloudera.org:8080/#/c/17521/1/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@2207 PS1, Line 2207: to truncate nit: after truncating http://gerrit.cloudera.org:8080/#/c/17521/1/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@2213 PS1, Line 2213: to create nit: after creating http://gerrit.cloudera.org:8080/#/c/17521/1/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@2239 PS1, Line 2239: to unset nit: after unsetting http://gerrit.cloudera.org:8080/#/c/17521/1/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@2312 PS1, Line 2312: if (!isTableBeingReplicated) { Ah, I think this fixes an existing bug. It'd be good to mention it in the commit message as well. -- To view, visit http://gerrit.cloudera.org:8080/17521 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9400c3586b4bdf46d9b4056ea1023aabae8cc519 Gerrit-Change-Number: 17521 Gerrit-PatchSet: 1 Gerrit-Owner: Vihang Karajgaonkar <[email protected]> Gerrit-Reviewer: Aman Sinha <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Quanlong Huang <[email protected]> Gerrit-Comment-Date: Thu, 27 May 2021 01:11:53 +0000 Gerrit-HasComments: Yes
