Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/21243 )
Change subject: KUDU-3564: Fix IN list predicate pruning ...................................................................... Patch Set 1: (5 comments) Thank you for the fix! Overall, LGTM. Just a few nits. http://gerrit.cloudera.org:8080/#/c/21243/1/src/kudu/common/partition.cc File src/kudu/common/partition.cc: http://gerrit.cloudera.org:8080/#/c/21243/1/src/kudu/common/partition.cc@a810 PS1, Line 810: : Good catch! Indeed: EncodeColumns() shouldn't be used here since the row doesn't contain necessary values for all the primary columns (otherwise, the control wouldn't reach this point because of the 'return' statement at line 780). http://gerrit.cloudera.org:8080/#/c/21243/1/src/kudu/common/scan_spec-test.cc File src/kudu/common/scan_spec-test.cc: http://gerrit.cloudera.org:8080/#/c/21243/1/src/kudu/common/scan_spec-test.cc@102 PS1, Line 102: bound nit: could use structured binding here? https://en.cppreference.com/w/cpp/language/structured_binding http://gerrit.cloudera.org:8080/#/c/21243/1/src/kudu/common/scan_spec-test.cc@105 PS1, Line 105: bound ditto http://gerrit.cloudera.org:8080/#/c/21243/1/src/kudu/common/scan_spec-test.cc@110 PS1, Line 110: col_names_and_num_bucket nit: could use structural binding instead for this 'for()' cycle as well? http://gerrit.cloudera.org:8080/#/c/21243/1/src/kudu/common/scan_spec-test.cc@998 PS1, Line 998: : KuduPartialRow lower_bound(&schema_); : CHECK_OK(lower_bound.SetInt8("a", -128)); : CHECK_OK(lower_bound.SetInt8("b", -128)); : CHECK_OK(lower_bound.SetInt8("c", -128)); : SetLowerBound(&spec, lower_bound); What is this for? -- To view, visit http://gerrit.cloudera.org:8080/21243 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I964b1ccfb85602741843ab555cdee53343217033 Gerrit-Change-Number: 21243 Gerrit-PatchSet: 1 Gerrit-Owner: Yifan Zhang <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Comment-Date: Sat, 06 Apr 2024 05:32:35 +0000 Gerrit-HasComments: Yes
