Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/23932 )
Change subject: IMPALA-12374: Optimize trailing/leading % in LIKE ...................................................................... Patch Set 10: Code-Review+1 (2 comments) Thanks for adding the test. This is getting close. http://gerrit.cloudera.org:8080/#/c/23932/10/be/src/exprs/like-predicate-test.cc File be/src/exprs/like-predicate-test.cc: http://gerrit.cloudera.org:8080/#/c/23932/10/be/src/exprs/like-predicate-test.cc@26 PS10, Line 26: class LikePredicateTests { Nit: Googletest has some options for organizing tests into classes so they can share data / state. See how TEST_F works here: https://github.com/google/googletest/blob/main/docs/primer.md#test-fixtures-using-the-same-data-configuration-for-multiple-tests-same-data-multiple-tests We use this in a variety of tests. For something like this where there is no state, it isn't required to use that construct. A simple helper function defined here is fine. If I was writing this, I would skip defining a test class if it doesn't have any state. I would define a local helper function and put the whole test case in the TEST() body. http://gerrit.cloudera.org:8080/#/c/23932/10/be/src/exprs/like-predicate-test.cc@35 PS10, Line 35: void LikePredicateTests::TestMatchOptimization() { Thanks for adding this test. Let's go ahead and test the edge cases like escaped % and multiple %s. -- To view, visit http://gerrit.cloudera.org:8080/23932 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I37b472e056f791035d25633f17ad8a6e841cdd18 Gerrit-Change-Number: 23932 Gerrit-PatchSet: 10 Gerrit-Owner: Balazs Hevele <[email protected]> Gerrit-Reviewer: Balazs Hevele <[email protected]> Gerrit-Reviewer: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Joe McDonnell <[email protected]> Gerrit-Comment-Date: Wed, 18 Feb 2026 23:45:44 +0000 Gerrit-HasComments: Yes
