Skip to content

[fix](parser) isNull should under primaryExpression#63619

Open
morrySnow wants to merge 1 commit into
apache:masterfrom
morrySnow:fix-is-null
Open

[fix](parser) isNull should under primaryExpression#63619
morrySnow wants to merge 1 commit into
apache:masterfrom
morrySnow:fix-is-null

Conversation

@morrySnow
Copy link
Copy Markdown
Contributor

What problem does this PR solve?

related PR #14967 #15163

Problem Summary:

The following SQL should have been executable, but it resulted in an unexpected error.

select * from t
where
isNull(c1) = 1; 

the error message is

Can not found function 'isNull'

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
Copy Markdown
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?

@morrySnow
Copy link
Copy Markdown
Contributor Author

run buildall

@morrySnow
Copy link
Copy Markdown
Contributor Author

/review

Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

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

Review result: no blocking issues found.

Critical checkpoint conclusions:

  • Goal/test: The PR moves ISNULL, IS_NULL_PRED, and IS_NOT_NULL_PRED from booleanExpression to primaryExpression, which addresses precedence so expressions like ISNULL(X) = 1 parse as a comparison. The added parser unit test covers the three affected forms.
  • Scope: The change is small and focused on grammar placement plus the corresponding visitor accessors and tests.
  • Concurrency/lifecycle: Not applicable; this is parser grammar/visitor code with no shared mutable runtime state, locks, or lifecycle changes.
  • Configuration/compatibility/persistence: Not applicable; no config, wire/storage format, EditLog, or transaction behavior is changed.
  • Parallel paths: The visitor methods were updated consistently with the generated context shape after changing the grammar rule argument from valueExpression to expression.
  • Tests: Coverage is targeted to the reported precedence case for all three null-predicate spellings. I attempted to run mvn -pl fe-core -Dtest=NereidsParserTest#testIsNullAndIsNotNullExpression -Dskip.doc=true test from fe/, but the runner could not resolve/install org.apache.doris:fe-foundation:jar:1.2-SNAPSHOT, so local execution was blocked by dependency setup.
  • Observability/performance/data correctness: Not applicable beyond parse-tree correctness; no query execution, storage visibility, memory tracking, or observability paths are touched.

User focus: no additional user-provided review focus was specified.

@hello-stephen
Copy link
Copy Markdown
Contributor

TPC-H: Total hot run time: 31986 ms
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/tpch-tools
Tpch sf100 test result on commit a578dbba57e131dcf89ba4fe3872d6e1bb0a66d8, data reload: false

------ Round 1 ----------------------------------
orders	Doris	NULL	NULL	0	0	0	NULL	0	NULL	NULL	2023-12-26 18:27:23	2023-12-26 18:42:55	NULL	utf-8	NULL	NULL	
============================================
q1	17694	4121	4049	4049
q2	q3	10777	1447	828	828
q4	4711	486	342	342
q5	7559	2307	2074	2074
q6	246	172	135	135
q7	949	780	671	671
q8	9358	1794	1571	1571
q9	5176	5054	4981	4981
q10	6432	2226	1859	1859
q11	423	278	247	247
q12	649	410	305	305
q13	18119	3403	2777	2777
q14	266	257	236	236
q15	q16	825	788	705	705
q17	964	972	938	938
q18	7211	5764	5664	5664
q19	1259	1353	1280	1280
q20	589	450	294	294
q21	6075	2869	2708	2708
q22	472	369	322	322
Total cold run time: 99754 ms
Total hot run time: 31986 ms

----- Round 2, with runtime_filter_mode=off -----
orders	Doris	NULL	NULL	150000000	42	6422171781	NULL	22778155	NULL	NULL	2023-12-26 18:27:23	2023-12-26 18:42:55	NULL	utf-8	NULL	NULL	
============================================
q1	4878	4809	4794	4794
q2	q3	5025	5234	4672	4672
q4	2332	2216	1445	1445
q5	4953	4754	4804	4754
q6	236	185	135	135
q7	1888	1719	1507	1507
q8	2405	2139	2110	2110
q9	7523	7405	7389	7389
q10	4800	4681	4240	4240
q11	541	386	359	359
q12	742	745	539	539
q13	2997	3406	2745	2745
q14	270	272	258	258
q15	q16	680	700	618	618
q17	1319	1280	1278	1278
q18	7416	6897	6721	6721
q19	1065	1072	1063	1063
q20	2244	2229	1940	1940
q21	5345	4672	4488	4488
q22	522	450	424	424
Total cold run time: 57181 ms
Total hot run time: 51479 ms

@hello-stephen
Copy link
Copy Markdown
Contributor

TPC-DS: Total hot run time: 172390 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 a578dbba57e131dcf89ba4fe3872d6e1bb0a66d8, data reload: false

query5	4313	655	529	529
query6	340	218	197	197
query7	4251	575	307	307
query8	327	238	221	221
query9	8836	4095	4075	4075
query10	444	340	298	298
query11	5820	2431	2250	2250
query12	185	127	129	127
query13	1300	613	448	448
query14	6207	5533	5210	5210
query14_1	4549	4546	4503	4503
query15	214	209	191	191
query16	1048	427	467	427
query17	975	722	582	582
query18	2466	485	347	347
query19	215	207	170	170
query20	137	134	131	131
query21	220	146	116	116
query22	13725	13628	13404	13404
query23	17397	16590	16222	16222
query23_1	16333	16442	16474	16442
query24	7569	1775	1313	1313
query24_1	1302	1288	1317	1288
query25	559	473	420	420
query26	1316	328	166	166
query27	2695	541	348	348
query28	4429	2012	1989	1989
query29	982	632	507	507
query30	316	238	203	203
query31	1124	1095	951	951
query32	88	77	77	77
query33	541	354	344	344
query34	1211	1139	652	652
query35	794	782	684	684
query36	1408	1409	1247	1247
query37	150	99	91	91
query38	3228	3185	3128	3128
query39	918	925	891	891
query39_1	870	868	877	868
query40	226	148	129	129
query41	66	65	61	61
query42	111	109	109	109
query43	334	340	291	291
query44	
query45	214	211	206	206
query46	1116	1201	721	721
query47	2377	2392	2271	2271
query48	404	422	290	290
query49	651	513	406	406
query50	996	336	260	260
query51	4306	4354	4292	4292
query52	107	103	93	93
query53	250	280	218	218
query54	311	276	257	257
query55	95	93	88	88
query56	307	297	293	293
query57	1451	1424	1359	1359
query58	300	272	270	270
query59	1608	1733	1455	1455
query60	331	329	316	316
query61	171	162	159	159
query62	709	660	591	591
query63	245	203	210	203
query64	2436	817	642	642
query65	
query66	1760	498	386	386
query67	30011	29989	29837	29837
query68	
query69	481	361	334	334
query70	1017	1012	994	994
query71	302	278	267	267
query72	3095	2680	2449	2449
query73	861	790	428	428
query74	5126	5010	4783	4783
query75	2695	2599	2279	2279
query76	2288	1182	793	793
query77	415	423	321	321
query78	12372	12445	11826	11826
query79	1256	1056	762	762
query80	614	540	471	471
query81	466	283	243	243
query82	231	159	120	120
query83	280	278	246	246
query84	288	136	114	114
query85	869	554	465	465
query86	401	345	318	318
query87	3432	3383	3260	3260
query88	3622	2716	2693	2693
query89	424	395	342	342
query90	2159	181	179	179
query91	177	168	143	143
query92	82	81	72	72
query93	1390	1444	824	824
query94	546	363	322	322
query95	674	380	372	372
query96	1030	853	355	355
query97	2739	2710	2631	2631
query98	239	226	225	225
query99	1184	1165	1020	1020
Total cold run time: 253462 ms
Total hot run time: 172390 ms

@hello-stephen
Copy link
Copy Markdown
Contributor

FE Regression Coverage Report

Increment line coverage 100.00% (2/2) 🎉
Increment coverage report
Complete coverage report

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants