Skip to content

Conversation

@airborne12
Copy link
Member

@airborne12 airborne12 commented Oct 14, 2025

What problem does this PR solve?

Issue Number: close #xxx

Related PR: #50748 #56139

Problem Summary:

Release note

None

Check List (For Author)

  • Test

    • Regression test
    • Unit Test
    • Manual test (add detailed scripts or steps below)
    • No need to test or manual test. Explain why:
      • This is a refactor/code format and no logic has been changed.
      • Previous test can cover this change.
      • No code files have been changed.
      • Other reason
  • Behavior changed:

    • No.
    • Yes.
  • Does this need documentation?

    • No.
    • Yes.

Check List (For Reviewer who merge this PR)

  • Confirm the release note
  • Confirm test cases
  • Confirm document
  • Add branch pick label

@hello-stephen
Copy link
Contributor

Thank you for your contribution to Apache Doris.
Don't know what should be done next? See How to process your PR.

Please clearly describe your PR:

  1. What problem was fixed (it's best to include specific error reporting information). How it was fixed.
  2. Which behaviors were modified. What was the previous behavior, what is it now, why was it modified, and what possible impacts might there be.
  3. What features were added. Why was this function added?
  4. Which code was refactored and why was this part of the code refactored?
  5. Which functions were optimized and what is the difference before and after the optimization?

@airborne12
Copy link
Member Author

run buildall

@airborne12 airborne12 requested a review from Copilot October 14, 2025 10:50
Copy link
Contributor

@csun5285 csun5285 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR fixes the evaluation of IS NULL predicates when using inverted indexes. The issue was that IS NULL queries were returning incorrect results (0 rows) when relying on inverted index evaluation instead of the expected non-zero counts.

  • Corrected the logic in FunctionIsNull::evaluate_inverted_index to properly handle null bitmaps
  • Added comprehensive regression tests to verify IS NULL and NOT (IS NOT NULL) queries work correctly with inverted indexes
  • Ensured consistent results between inverted index enabled and disabled modes

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
be/src/vec/functions/is_null.h Fixed the inverted index evaluation logic for IS NULL predicates by correcting bitmap handling
regression-test/suites/inverted_index_p0/test_inverted_is_null.groovy Added comprehensive test coverage for IS NULL queries with inverted indexes

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

// null_bitmap is null bitmap
bitmap_result = segment_v2::InvertedIndexResultBitmap(null_bitmap, null_bitmap);
}
if (!index_iter->has_null()) {
Copy link

Copilot AI Oct 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When the index has no nulls, the function returns OK without setting bitmap_result, but the caller expects a valid bitmap. This could lead to undefined behavior when bitmap_result is accessed later.

Suggested change
if (!index_iter->has_null()) {
if (!index_iter->has_null()) {
auto empty_bitmap = std::make_shared<roaring::Roaring>();
bitmap_result = segment_v2::InvertedIndexResultBitmap(empty_bitmap, empty_bitmap);

Copilot uses AI. Check for mistakes.
segment_v2::InvertedIndexQueryCacheHandle null_bitmap_cache_handle;
RETURN_IF_ERROR(index_iter->read_null_bitmap(&null_bitmap_cache_handle));
std::shared_ptr<roaring::Roaring> null_bitmap = null_bitmap_cache_handle.get_bitmap();
if (!null_bitmap) {
Copy link

Copilot AI Oct 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar to the previous issue, returning OK without setting bitmap_result when null_bitmap is null could cause problems for the caller expecting a valid bitmap result.

Suggested change
if (!null_bitmap) {
if (!null_bitmap) {
// Set bitmap_result to an empty result if null_bitmap is null
auto empty_bitmap = std::make_shared<roaring::Roaring>();
bitmap_result = segment_v2::InvertedIndexResultBitmap(empty_bitmap, empty_bitmap);

Copilot uses AI. Check for mistakes.
@github-actions
Copy link
Contributor

PR approved by anyone and no changes requested.

@doris-robot
Copy link

ClickBench: Total hot run time: 30.08 s
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/clickbench-tools
ClickBench test result on commit 2d75eba332538ee13f1a7b2a3c330f534c98a6b0, data reload: false

query1	0.06	0.05	0.05
query2	0.09	0.06	0.05
query3	0.25	0.09	0.08
query4	1.62	0.12	0.12
query5	0.28	0.26	0.25
query6	1.23	0.66	0.65
query7	0.03	0.02	0.02
query8	0.06	0.04	0.05
query9	0.62	0.52	0.52
query10	0.58	0.61	0.57
query11	0.17	0.11	0.12
query12	0.15	0.12	0.13
query13	0.64	0.61	0.62
query14	1.02	1.03	1.03
query15	0.88	0.85	0.88
query16	0.40	0.40	0.40
query17	1.07	1.08	1.04
query18	0.24	0.20	0.20
query19	2.01	1.83	1.87
query20	0.02	0.02	0.01
query21	15.40	0.95	0.57
query22	0.76	1.19	0.68
query23	14.99	1.41	0.66
query24	7.41	1.35	0.38
query25	0.28	0.21	0.16
query26	0.63	0.16	0.13
query27	0.09	0.06	0.05
query28	9.10	1.39	0.93
query29	12.56	3.92	3.26
query30	0.27	0.14	0.11
query31	2.84	0.59	0.39
query32	3.23	0.57	0.49
query33	3.11	3.11	3.05
query34	16.26	5.46	4.87
query35	4.96	4.92	4.96
query36	0.69	0.51	0.51
query37	0.11	0.08	0.07
query38	0.07	0.05	0.04
query39	0.04	0.04	0.03
query40	0.18	0.15	0.14
query41	0.09	0.04	0.03
query42	0.04	0.03	0.03
query43	0.04	0.03	0.04
Total cold run time: 104.57 s
Total hot run time: 30.08 s

Copy link
Contributor

@zzzxl1993 zzzxl1993 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@hello-stephen
Copy link
Contributor

BE UT Coverage Report

Increment line coverage 75.00% (9/12) 🎉

Increment coverage report
Complete coverage report

Category Coverage
Function Coverage 52.54% (17754/33789)
Line Coverage 37.75% (161461/427752)
Region Coverage 32.19% (123172/382585)
Branch Coverage 33.62% (54102/160929)

eldenmoon
eldenmoon previously approved these changes Oct 14, 2025
Copy link
Member

@eldenmoon eldenmoon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@github-actions github-actions bot added the approved Indicates a PR has been approved by one committer. label Oct 14, 2025
@github-actions
Copy link
Contributor

PR approved by at least one committer and no changes requested.

@hello-stephen
Copy link
Contributor

BE Regression && UT Coverage Report

Increment line coverage 75.00% (9/12) 🎉

Increment coverage report
Complete coverage report

Category Coverage
Function Coverage 71.14% (23566/33124)
Line Coverage 57.56% (246055/427498)
Region Coverage 52.76% (204463/387535)
Branch Coverage 54.51% (88199/161818)

@github-actions github-actions bot removed the approved Indicates a PR has been approved by one committer. label Oct 15, 2025
@airborne12
Copy link
Member Author

run buildall

@doris-robot
Copy link

TPC-DS: Total hot run time: 189941 ms
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/tpcds-tools
TPC-DS sf100 test result on commit 78f36d6203445c534e53c1a66ad080632f49fa09, data reload: false

query1	1095	435	424	424
query2	6572	1644	1692	1644
query3	6754	227	226	226
query4	26198	23938	23076	23076
query5	5132	639	482	482
query6	333	268	226	226
query7	4659	503	315	315
query8	306	276	257	257
query9	8679	2554	2562	2554
query10	511	361	293	293
query11	15984	15381	14866	14866
query12	207	113	113	113
query13	1682	583	441	441
query14	11508	9288	9379	9288
query15	214	188	177	177
query16	7691	695	528	528
query17	1206	802	624	624
query18	2081	493	397	397
query19	246	225	195	195
query20	141	139	139	139
query21	258	142	140	140
query22	4705	4745	4541	4541
query23	34881	33708	33457	33457
query24	8788	2505	2496	2496
query25	601	517	466	466
query26	1283	279	175	175
query27	2875	533	385	385
query28	4425	2195	2169	2169
query29	1114	676	501	501
query30	309	232	202	202
query31	958	865	762	762
query32	87	80	79	79
query33	592	406	338	338
query34	819	958	589	589
query35	885	874	801	801
query36	1046	1061	950	950
query37	142	114	94	94
query38	3773	3737	3533	3533
query39	1462	1452	1473	1452
query40	213	121	116	116
query41	61	61	65	61
query42	119	108	109	108
query43	499	496	455	455
query44	1331	809	816	809
query45	183	181	176	176
query46	826	1002	631	631
query47	1752	1785	1701	1701
query48	395	427	310	310
query49	752	479	421	421
query50	638	689	409	409
query51	3845	3994	3956	3956
query52	112	108	101	101
query53	232	260	202	202
query54	591	600	539	539
query55	93	84	88	84
query56	331	336	307	307
query57	1180	1189	1099	1099
query58	299	275	282	275
query59	2530	2603	2510	2510
query60	360	335	336	335
query61	158	159	152	152
query62	810	744	662	662
query63	238	186	197	186
query64	4518	1321	949	949
query65	4121	3968	3980	3968
query66	1078	431	338	338
query67	15446	15511	15084	15084
query68	9181	957	597	597
query69	503	327	301	301
query70	1361	1279	1308	1279
query71	516	343	330	330
query72	5963	4943	4976	4943
query73	714	626	370	370
query74	8966	8970	8673	8673
query75	4513	3360	2893	2893
query76	3789	1172	751	751
query77	817	412	305	305
query78	9683	9748	8906	8906
query79	2213	822	602	602
query80	677	581	534	534
query81	500	266	233	233
query82	450	161	134	134
query83	297	276	256	256
query84	318	108	97	97
query85	961	463	429	429
query86	336	337	296	296
query87	3831	3783	3772	3772
query88	3524	2253	2220	2220
query89	386	334	300	300
query90	2079	220	222	220
query91	160	167	130	130
query92	90	69	66	66
query93	1290	994	647	647
query94	697	434	346	346
query95	417	315	317	315
query96	493	572	285	285
query97	2945	2981	2869	2869
query98	251	215	215	215
query99	1468	1394	1288	1288
Total cold run time: 282302 ms
Total hot run time: 189941 ms

@doris-robot
Copy link

ClickBench: Total hot run time: 30.68 s
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/clickbench-tools
ClickBench test result on commit 78f36d6203445c534e53c1a66ad080632f49fa09, data reload: false

query1	0.05	0.05	0.05
query2	0.09	0.05	0.06
query3	0.26	0.09	0.08
query4	1.62	0.12	0.12
query5	0.27	0.29	0.25
query6	1.18	0.66	0.65
query7	0.03	0.03	0.03
query8	0.05	0.04	0.04
query9	0.66	0.53	0.53
query10	0.58	0.58	0.57
query11	0.17	0.12	0.11
query12	0.15	0.12	0.12
query13	0.63	0.62	0.61
query14	1.03	1.05	1.04
query15	0.88	0.87	0.85
query16	0.39	0.40	0.40
query17	1.07	1.10	1.06
query18	0.22	0.20	0.20
query19	1.94	1.82	1.88
query20	0.01	0.01	0.02
query21	15.42	0.94	0.57
query22	0.77	1.28	0.70
query23	14.77	1.39	0.63
query24	6.92	1.57	0.98
query25	0.51	0.12	0.23
query26	0.74	0.16	0.13
query27	0.08	0.06	0.06
query28	10.50	1.35	0.94
query29	12.57	3.93	3.30
query30	0.28	0.16	0.12
query31	2.83	0.58	0.38
query32	3.27	0.58	0.47
query33	3.06	3.05	3.17
query34	15.96	5.44	4.82
query35	4.95	4.92	4.93
query36	0.69	0.51	0.52
query37	0.10	0.07	0.07
query38	0.06	0.06	0.04
query39	0.03	0.03	0.04
query40	0.17	0.15	0.15
query41	0.09	0.04	0.03
query42	0.04	0.03	0.04
query43	0.04	0.04	0.04
Total cold run time: 105.13 s
Total hot run time: 30.68 s

@hello-stephen
Copy link
Contributor

BE UT Coverage Report

Increment line coverage 75.00% (9/12) 🎉

Increment coverage report
Complete coverage report

Category Coverage
Function Coverage 52.57% (17788/33834)
Line Coverage 37.77% (161671/428029)
Region Coverage 32.22% (123296/382690)
Branch Coverage 33.64% (54163/160997)

@hello-stephen
Copy link
Contributor

BE Regression && UT Coverage Report

Increment line coverage 75.00% (9/12) 🎉

Increment coverage report
Complete coverage report

Category Coverage
Function Coverage 71.20% (23615/33166)
Line Coverage 57.60% (246403/427759)
Region Coverage 52.74% (204421/387626)
Branch Coverage 54.51% (88245/161880)

Copy link
Member

@eldenmoon eldenmoon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@github-actions github-actions bot added the approved Indicates a PR has been approved by one committer. label Oct 15, 2025
@github-actions
Copy link
Contributor

PR approved by at least one committer and no changes requested.

@airborne12 airborne12 merged commit 3038a44 into apache:master Oct 16, 2025
27 of 28 checks passed
@airborne12 airborne12 deleted the fix-search branch October 16, 2025 02:21
yiguolei pushed a commit that referenced this pull request Oct 16, 2025
…index evaluate #56964 (#57020)

Cherry-picked from #56964

Co-authored-by: Jack <jiangkai@selectdb.com>
@yiguolei yiguolei mentioned this pull request Nov 5, 2025
airborne12 added a commit to airborne12/apache-doris that referenced this pull request Jan 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by one committer. dev/4.0.1-merged reviewed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants