[BugFix] handle null value in dict colum in parquet file #25800

dirtysalt · 2023-06-22T12:50:16Z

Fixes #issue

This PR is to fix dict filtering when null value in dict column.

Right now we intercept is_null function(on top) on dict column, however, that's not enough. Because for some predicates like (c = 'a' or c is null) or coalesce(c, 'a'), there is no is_null function on top, there is is_null function inside.

So the right way to handle null value, is to evaluate null value in conjuncts.

TODO: add UT to cover predicates like above and is_null function on top.

What type of PR is this:

Checklist:

I have added test cases for my bug fix or my new feature
This pr will affect users' behaviors
This pr needs user documentation (for new or modified features or behaviors)
- I have added documentation for my new feature or new function

Bugfix cherry-pick branch check:

be/src/formats/parquet/encoding_dict.h

be/src/formats/parquet/group_reader.cpp

build.sh

be/src/formats/parquet/group_reader.cpp

be/src/formats/parquet/encoding_dict.h

dirtysalt · 2023-06-27T07:50:23Z

The test case is too flaky in the CI test. I'll submit another PR to add test cases.

It's because oss is not well supported in sql-tester right now. I'll submit another PR to fix that.

be/src/formats/parquet/group_reader.cpp

be/src/formats/parquet/encoding_dict.h

Signed-off-by: yanz <dirtysalt1987@gmail.com>

dirtysalt · 2023-06-29T07:49:17Z

another implementation

-------------------------------------------------------------
Benchmark                   Time             CPU   Iterations
-------------------------------------------------------------
BM_DictDecoder/0        0.013 ms        0.013 ms        52330
BM_DictDecoder/10       0.017 ms        0.017 ms        40683
BM_DictDecoder/20       0.018 ms        0.018 ms        38447
BM_DictDecoder/30       0.018 ms        0.018 ms        39328
BM_DictDecoder/40       0.017 ms        0.017 ms        38572
BM_DictDecoder/50       0.018 ms        0.018 ms        37454
BM_DictDecoder/60       0.018 ms        0.018 ms        38174
BM_DictDecoder/70       0.017 ms        0.017 ms        40298
BM_DictDecoder/80       0.017 ms        0.017 ms        40637
BM_DictDecoder/90       0.017 ms        0.017 ms        42525
BM_DictDecoder/100      0.012 ms        0.012 ms        58747

Signed-off-by: yanz <dirtysalt1987@gmail.com>

dirtysalt · 2023-06-29T08:45:02Z

after saving a branch, a little better

-------------------------------------------------------------
Benchmark                   Time             CPU   Iterations
-------------------------------------------------------------
BM_DictDecoder/0        0.014 ms        0.014 ms        51400
BM_DictDecoder/10       0.016 ms        0.016 ms        45682
BM_DictDecoder/20       0.015 ms        0.015 ms        46104
BM_DictDecoder/30       0.015 ms        0.015 ms        46383
BM_DictDecoder/40       0.015 ms        0.015 ms        46376
BM_DictDecoder/50       0.015 ms        0.015 ms        45959
BM_DictDecoder/60       0.015 ms        0.015 ms        45928
BM_DictDecoder/70       0.015 ms        0.015 ms        46383
BM_DictDecoder/80       0.015 ms        0.015 ms        46571
BM_DictDecoder/90       0.015 ms        0.015 ms        46623
BM_DictDecoder/100      0.013 ms        0.014 ms        51097

Signed-off-by: yanz <dirtysalt1987@gmail.com>

be/src/bench/CMakeLists.txt

test/lib/skip.py

Signed-off-by: yanz <dirtysalt1987@gmail.com>

wanpengfei-git · 2023-06-30T05:06:37Z

[FE PR Coverage Check]

😍 pass : 0 / 0 (0%)

dirtysalt · 2023-06-30T14:38:15Z

@Mergifyio backport branch-3.1

mergify · 2023-06-30T14:38:18Z

backport branch-3.1

✅ Backports have been created

#26351 [BugFix] handle null value in dict colum in parquet file (backport #25800) has been created for branch branch-3.1 but encountered conflicts

This PR is to fix dict filtering when null value in dict column. Right now we intercept `is_null` function(on top) on dict column, however, that's not enough. Because for some predicates like `(c = 'a' or c is null)` or `coalesce(c, 'a')`, there is no `is_null` function on top, there is `is_null` function inside. So the right way to handle null value, is to evaluate null value in conjuncts. (cherry picked from commit a0a54ac) # Conflicts: # be/src/bench/CMakeLists.txt

…5800) This PR is to fix dict filtering when null value in dict column. Right now we intercept `is_null` function(on top) on dict column, however, that's not enough. Because for some predicates like `(c = 'a' or c is null)` or `coalesce(c, 'a')`, there is no `is_null` function on top, there is `is_null` function inside. So the right way to handle null value, is to evaluate null value in conjuncts.

This PR is to fix dict filtering when null value in dict column. Right now we intercept `is_null` function(on top) on dict column, however, that's not enough. Because for some predicates like `(c = 'a' or c is null)` or `coalesce(c, 'a')`, there is no `is_null` function on top, there is `is_null` function inside. So the right way to handle null value, is to evaluate null value in conjuncts. (cherry picked from commit a0a54ac) # Conflicts: # be/src/bench/CMakeLists.txt (cherry picked from commit 47771bf) # Conflicts: # be/src/bench/CMakeLists.txt # be/src/bench/parquet_writer_array_bench.cpp # be/src/bench/parquet_writer_bench.cpp

dirtysalt · 2023-07-02T05:15:12Z

@mergify backport branch-2.5

mergify · 2023-07-02T05:15:15Z

backport branch-2.5

✅ Backports have been created

#26367 [BugFix] handle null value in dict colum in parquet file (backport #25800) has been created for branch branch-2.5 but encountered conflicts

This PR is to fix dict filtering when null value in dict column. Right now we intercept `is_null` function(on top) on dict column, however, that's not enough. Because for some predicates like `(c = 'a' or c is null)` or `coalesce(c, 'a')`, there is no `is_null` function on top, there is `is_null` function inside. So the right way to handle null value, is to evaluate null value in conjuncts. (cherry picked from commit a0a54ac) # Conflicts: # be/src/bench/CMakeLists.txt # be/src/bench/parquet_writer_array_bench.cpp # be/src/bench/parquet_writer_bench.cpp # be/src/formats/parquet/column_chunk_reader.h # be/src/formats/parquet/column_reader.cpp # be/src/formats/parquet/column_reader.h # be/src/formats/parquet/encoding.h # be/src/formats/parquet/encoding_dict.h # be/src/formats/parquet/group_reader.cpp # be/src/formats/parquet/stored_column_reader.h # test/lib/choose_cases.py # test/lib/skip.py

mergify bot assigned dirtysalt Jun 22, 2023

dirtysalt enabled auto-merge (squash) June 25, 2023 04:52

DorianZheng reviewed Jun 25, 2023

View reviewed changes

be/src/formats/parquet/encoding_dict.h Outdated Show resolved Hide resolved

DorianZheng requested changes Jun 25, 2023

View reviewed changes

be/src/formats/parquet/encoding_dict.h Outdated Show resolved Hide resolved

DorianZheng reviewed Jun 25, 2023

View reviewed changes

be/src/formats/parquet/group_reader.cpp Outdated Show resolved Hide resolved

Smith-Cruise reviewed Jun 25, 2023

View reviewed changes

build.sh Outdated Show resolved Hide resolved

Smith-Cruise reviewed Jun 25, 2023

View reviewed changes

be/src/formats/parquet/group_reader.cpp Outdated Show resolved Hide resolved

DorianZheng reviewed Jun 27, 2023

View reviewed changes

be/src/formats/parquet/encoding_dict.h Show resolved Hide resolved

dirtysalt force-pushed the fix-or-predicate branch from c044597 to ed217c1 Compare June 27, 2023 08:34

zombee0 reviewed Jun 27, 2023

View reviewed changes

be/src/formats/parquet/group_reader.cpp Show resolved Hide resolved

Youngwb previously approved these changes Jun 27, 2023

View reviewed changes

zombee0 reviewed Jun 27, 2023

View reviewed changes

be/src/formats/parquet/encoding_dict.h Show resolved Hide resolved

zombee0 reviewed Jun 27, 2023

View reviewed changes

be/src/formats/parquet/encoding_dict.h Outdated Show resolved Hide resolved

dirtysalt dismissed Youngwb’s stale review via 521152b June 27, 2023 15:05

zombee0 reviewed Jun 28, 2023

View reviewed changes

be/src/formats/parquet/encoding_dict.h Outdated Show resolved Hide resolved

zombee0 approved these changes Jun 28, 2023

View reviewed changes

dirtysalt added 12 commits June 29, 2023 07:38

[BugFix] handle null value in dict colum in parquet file

f58a093

Signed-off-by: yanz <dirtysalt1987@gmail.com>

add unit test cases

32e5e74

Signed-off-by: yanz <dirtysalt1987@gmail.com>

add more test cases

6a03678

Signed-off-by: yanz <dirtysalt1987@gmail.com>

revert sr.conf

6aa21c2

Signed-off-by: yanz <dirtysalt1987@gmail.com>

fix for comment

beb050f

Signed-off-by: yanz <dirtysalt1987@gmail.com>

fix for comment

db6264b

Signed-off-by: yanz <dirtysalt1987@gmail.com>

fix for compiling ut

c2defea

Signed-off-by: yanz <dirtysalt1987@gmail.com>

update ossutil command

33f9cbf

Signed-off-by: yanz <dirtysalt1987@gmail.com>

fix for comment and sql-tester supports skip test cases

b9bbd5f

Signed-off-by: yanz <dirtysalt1987@gmail.com>

fix for comment

1cf56cd

Signed-off-by: yanz <dirtysalt1987@gmail.com>

fix for comment

8005cdb

Signed-off-by: yanz <dirtysalt1987@gmail.com>

fix for commnet

020dcfd

Signed-off-by: yanz <dirtysalt1987@gmail.com>

dirtysalt force-pushed the fix-or-predicate branch from 3bf2962 to 020dcfd Compare June 28, 2023 23:38

another better implementation

27d9b94

Signed-off-by: yanz <dirtysalt1987@gmail.com>

a trcikier way to save branch

06af241

Signed-off-by: yanz <dirtysalt1987@gmail.com>

revert build.sh

c1ca299

Signed-off-by: yanz <dirtysalt1987@gmail.com>

Youngwb reviewed Jun 30, 2023

View reviewed changes

be/src/bench/CMakeLists.txt Outdated Show resolved Hide resolved

Youngwb reviewed Jun 30, 2023

View reviewed changes

test/lib/skip.py Show resolved Hide resolved

fix for comment

62f31dc

Signed-off-by: yanz <dirtysalt1987@gmail.com>

Youngwb previously approved these changes Jun 30, 2023

View reviewed changes

fix for comment

de80552

Signed-off-by: yanz <dirtysalt1987@gmail.com>

dirtysalt dismissed Youngwb’s stale review via de80552 June 30, 2023 04:34

Youngwb approved these changes Jun 30, 2023

View reviewed changes

stephen-shelby approved these changes Jun 30, 2023

View reviewed changes

dirtysalt merged commit a0a54ac into StarRocks:main Jun 30, 2023
25 checks passed

mergify bot mentioned this pull request Jun 30, 2023

[BugFix] handle null value in dict colum in parquet file (backport #25800) #26351

Merged

mergify bot mentioned this pull request Jul 2, 2023

[BugFix] handle null value in dict colum in parquet file (backport #25800) #26367

Closed

dirtysalt mentioned this pull request Jul 3, 2023

[BugFix][branch-2.5] handle null value in dict colum in parquet file #26370

Merged

16 tasks

dirtysalt deleted the fix-or-predicate branch July 3, 2023 02:32

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[BugFix] handle null value in dict colum in parquet file #25800

[BugFix] handle null value in dict colum in parquet file #25800

dirtysalt commented Jun 22, 2023

dirtysalt commented Jun 27, 2023

dirtysalt commented Jun 29, 2023

dirtysalt commented Jun 29, 2023

wanpengfei-git commented Jun 30, 2023

dirtysalt commented Jun 30, 2023

mergify bot commented Jun 30, 2023 •

edited

dirtysalt commented Jul 2, 2023

mergify bot commented Jul 2, 2023 •

edited

[BugFix] handle null value in dict colum in parquet file #25800

[BugFix] handle null value in dict colum in parquet file #25800

Conversation

dirtysalt commented Jun 22, 2023

What type of PR is this:

Checklist:

Bugfix cherry-pick branch check:

dirtysalt commented Jun 27, 2023

dirtysalt commented Jun 29, 2023

dirtysalt commented Jun 29, 2023

wanpengfei-git commented Jun 30, 2023

[FE PR Coverage Check]

dirtysalt commented Jun 30, 2023

mergify bot commented Jun 30, 2023 • edited

✅ Backports have been created

dirtysalt commented Jul 2, 2023

mergify bot commented Jul 2, 2023 • edited

✅ Backports have been created

mergify bot commented Jun 30, 2023 •

edited

mergify bot commented Jul 2, 2023 •

edited