Jason Fehr has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20770 )

Change subject: IMPALA-12426: Query History Table
......................................................................


Patch Set 15:

(2 comments)

> Patch Set 15:
>
> (6 comments)

http://gerrit.cloudera.org:8080/#/c/20770/12/be/src/benchmarks/expr-benchmark.cc
File be/src/benchmarks/expr-benchmark.cc:

http://gerrit.cloudera.org:8080/#/c/20770/12/be/src/benchmarks/expr-benchmark.cc@591
PS12, Line 591: //                prettyprint_duration               5.67     
5.67     5.67     0.048X    0.0475X    0.0471X
> Thanks for updating the table.
I did not modify any of the code for those functions.  Thus, it is either a 
regression from another change or else my dev machine is slower.


http://gerrit.cloudera.org:8080/#/c/20770/15/be/src/service/workload-management.cc
File be/src/service/workload-management.cc:

http://gerrit.cloudera.org:8080/#/c/20770/15/be/src/service/workload-management.cc@622
PS15, Line 622: string op_type = to_string(query_handle->catalog_op_type());
              :
              :     if (boost::algorithm::equals(op_type.substr(0,3), "USE")
              :         || boost::algorithm::equals(op_type.substr(0,4), 
"SHOW")) {
              :       return;
              :     }
> I think using switch and fall through is more reliable rather than parsing
I considered using that method because it's cleaner and faster than string 
comparison.  My concern is if new DDL types are added, how will we remember to 
update the code here?  Looking at the blame for the TCatalogOpType enum 
definition in Frontend.thrift, there has not been an addition to this enum for 
6 years, and the change before that was 9 years ago.

Given that the DDL types enum is very stable, I am going to switch to the 
switch method.



--
To view, visit http://gerrit.cloudera.org:8080/20770
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2d2da9d450fba4e789400cfa62927fc25d34f844
Gerrit-Change-Number: 20770
Gerrit-PatchSet: 15
Gerrit-Owner: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <asher...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com>
Gerrit-Reviewer: Jason Fehr <jf...@cloudera.com>
Gerrit-Reviewer: Michael Smith <michael.sm...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <riza.sumi...@cloudera.com>
Gerrit-Comment-Date: Tue, 30 Jan 2024 18:37:09 +0000
Gerrit-HasComments: Yes

Reply via email to