Fredy Wijaya has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12635 )

Change subject: IMPALA-8254: Fix error when running compute stats with 
compression_codec set
......................................................................


Patch Set 9:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/12635/7/be/src/service/query-options.cc
File be/src/service/query-options.cc:

http://gerrit.cloudera.org:8080/#/c/12635/7/be/src/service/query-options.cc@178
PS7, Line 178:
> Can you add a few tests to verify case insensitivity?
Done


http://gerrit.cloudera.org:8080/#/c/12635/8/be/src/service/query-options.cc
File be/src/service/query-options.cc:

http://gerrit.cloudera.org:8080/#/c/12635/8/be/src/service/query-options.cc@176
PS8, Line 176: s GetThriftEnum
> Other functions in the class use camel case.
Done


http://gerrit.cloudera.org:8080/#/c/12635/8/be/src/service/query-options.cc@188
PS8, Line 188: lue = static_cast<ENUM
> same as line 176
Done


http://gerrit.cloudera.org:8080/#/c/12635/8/be/src/service/query-options.cc@264
PS8, Line 264:         query_options->__set_hbase_caching(atoi(value.c_str()));
             :         break;
             :       case TImpalaQueryOptions::HBASE_CACHE_BLOCKS:
             :         query_options->__set_hbase_cache_blocks(
             :             iequals(value, "true") || iequals(value, "1"));
> optional: get_thrift_enum could have a signature similar to ParseMemValue()
Good idea. I can make the whole thing in a single pass.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2cb546fbd3d2a02e0ed30d85a33a04852bed9dd2
Gerrit-Change-Number: 12635
Gerrit-PatchSet: 9
Gerrit-Owner: Fredy Wijaya <[email protected]>
Gerrit-Reviewer: Csaba Ringhofer <[email protected]>
Gerrit-Reviewer: Fredy Wijaya <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Tim Armstrong <[email protected]>
Gerrit-Comment-Date: Fri, 01 Mar 2019 17:42:25 +0000
Gerrit-HasComments: Yes

Reply via email to