Vihang Karajgaonkar 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 2: (10 comments) http://gerrit.cloudera.org:8080/#/c/17521/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/17521/1//COMMIT_MSG@16 PS1, Line 16: us > nit: remove 'to' Done http://gerrit.cloudera.org:8080/#/c/17521/1//COMMIT_MSG@24 PS1, Line 24: DELETE_STATS_IN_TRUNCATE. The default value of this option is 1 or true > nit: would be good to specify the default option value in the commit messag Done 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) > Agree with adding TRUNCATE in the name. Also why not just DELETE_TABLE_STA makes sense. Will rename as suggested and club them into 1 option. 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 Done 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 Done 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 Done 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 Done 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 Done 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 c done http://gerrit.cloudera.org:8080/#/c/17521/1/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@2323 PS1, Line 2323: if (!params.isSetSkip_delete_column_stats()) { > This is pre-existing behavior but I would have expected the stats deletion Yes, makes sense. I created IMPALA-10722 -- 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: 2 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-Reviewer: Vihang Karajgaonkar <[email protected]> Gerrit-Comment-Date: Thu, 27 May 2021 20:31:24 +0000 Gerrit-HasComments: Yes
