Qifan Chen has posted comments on this change. ( http://gerrit.cloudera.org:8080/18433 )
Change subject: IMPALA-11141: Use exact data types in IN-list filter ...................................................................... Patch Set 4: Code-Review+2 (2 comments) Looks great! Thanks a lot for the rework. http://gerrit.cloudera.org:8080/#/c/18433/2/be/src/util/in-list-filter-test.cc File be/src/util/in-list-filter-test.cc: http://gerrit.cloudera.org:8080/#/c/18433/2/be/src/util/in-list-filter-test.cc@173 PS2, Line 173: char str_buffer[] = "aabbccddeeff"; : const char* ptr = str_buffer; : // Insert 3 values first : for (int i = 0; i < 3; ++i) { : filter->Insert(ptr); : ptr += 2; : } > The trouble is they use different Find() and Insert() methods. VARCHAR/STRI Done http://gerrit.cloudera.org:8080/#/c/18433/2/be/src/util/in-list-filter.cc File be/src/util/in-list-filter.cc: http://gerrit.cloudera.org:8080/#/c/18433/2/be/src/util/in-list-filter.cc@65 PS2, Line 65: default: > I think it's not a best practise to use float/double as join keys since flo Sure. But to the question whether to support float/double in InList filter, my thinking is as follows. 1. Min/max filters are inclusive in nature; 2. In 32 or 64 bit IEEE floating representation, integers (i.e., when fraction part is 0) can be precise for a very large range. See https://en.wikipedia.org/wiki/Single-precision_floating-point_format and https://en.wikipedia.org/wiki/Double-precision_floating-point_format. -- To view, visit http://gerrit.cloudera.org:8080/18433 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id434a542b2ced64efa3bfc974cb565b94a4193e9 Gerrit-Change-Number: 18433 Gerrit-PatchSet: 4 Gerrit-Owner: Quanlong Huang <[email protected]> Gerrit-Reviewer: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Qifan Chen <[email protected]> Gerrit-Reviewer: Quanlong Huang <[email protected]> Gerrit-Comment-Date: Mon, 25 Apr 2022 13:22:47 +0000 Gerrit-HasComments: Yes
