Yida Wu has posted comments on this change. ( http://gerrit.cloudera.org:8080/21698 )
Change subject: IMPALA-13186: Tag query option scope for tuple cache ...................................................................... Patch Set 5: (6 comments) LGTM, some minor stuff http://gerrit.cloudera.org:8080/#/c/21698/5/be/src/service/query-options.cc File be/src/service/query-options.cc: http://gerrit.cloudera.org:8080/#/c/21698/5/be/src/service/query-options.cc@120 PS5, Line 120: \ nit. unnecessary change http://gerrit.cloudera.org:8080/#/c/21698/5/be/src/service/query-options.cc@138 PS5, Line 138: \ nit. unnecessary change http://gerrit.cloudera.org:8080/#/c/21698/5/be/src/service/query-options.cc@165 PS5, Line 165: \ nit. unnecessary change http://gerrit.cloudera.org:8080/#/c/21698/5/be/src/service/query-options.cc@1398 PS5, Line 1398: template<typename T, typename std::enable_if_t<std::is_enum<T>::value>* = nullptr> : static void HashQueryOptionValue(const T& option, HashState& hash) { : MurmurHash3_x64_128(&option, sizeof(option), hash); : } : : template<typename T, typename std::enable_if_t<std::is_arithmetic<T>::value>* = nullptr> : static void HashQueryOptionValue(const T& option, HashState& hash) { : MurmurHash3_x64_128(&option, sizeof(option), hash); : } Seems merging the two into one is possible. template<typename T, typename std::enable_if_t<std::is_enum<T>::value || std::is_arithmetic<T>::value>* = nullptr> static void HashQueryOptionValue(const T& option, HashState& hash) { MurmurHash3_x64_128(&option, sizeof(option), hash); } http://gerrit.cloudera.org:8080/#/c/21698/5/be/src/service/query-options.cc@1450 PS5, Line 1450: \ nit. unnecessary change http://gerrit.cloudera.org:8080/#/c/21698/5/be/src/service/query-options.cc@1454 PS5, Line 1454: auto Is the original 'int' not working? -- To view, visit http://gerrit.cloudera.org:8080/21698 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1f4802ad9548749cd43df8848b6f46dca3739ae7 Gerrit-Change-Number: 21698 Gerrit-PatchSet: 5 Gerrit-Owner: Michael Smith <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Joe McDonnell <[email protected]> Gerrit-Reviewer: Kurt Deschler <[email protected]> Gerrit-Reviewer: Michael Smith <[email protected]> Gerrit-Reviewer: Yida Wu <[email protected]> Gerrit-Comment-Date: Thu, 12 Sep 2024 18:19:43 +0000 Gerrit-HasComments: Yes
