Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[fix](parser) Syntax error for add partition with null null #45865

Merged
merged 1 commit into from
Dec 25, 2024

Conversation

wyxxxcat
Copy link
Contributor

What problem does this PR solve?

0e31f5f3cc47576fc8bbcb613b4404eb.jpeg

Issue Number: close #xxx

Related PR: #xxx

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?

@wyxxxcat
Copy link
Contributor Author

run buildall

fe/fe-core/src/main/cup/sql_parser.cup Outdated Show resolved Hide resolved
fe/fe-core/src/main/cup/sql_parser.cup Outdated Show resolved Hide resolved
fe/fe-core/src/main/cup/sql_parser.cup Outdated Show resolved Hide resolved
fe/fe-core/src/main/cup/sql_parser.cup Outdated Show resolved Hide resolved
@wyxxxcat
Copy link
Contributor Author

run buildall

@wyxxxcat wyxxxcat requested a review from w41ter December 24, 2024 09:58
@doris-robot
Copy link

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

------ Round 1 ----------------------------------
q1	17582	7486	7291	7291
q2	2047	176	165	165
q3	10592	1101	1226	1101
q4	10588	803	839	803
q5	7619	2687	2699	2687
q6	235	151	151	151
q7	1001	659	601	601
q8	9226	1855	1943	1855
q9	6599	6373	6433	6373
q10	7000	2336	2318	2318
q11	462	261	257	257
q12	412	228	218	218
q13	17813	2917	2913	2913
q14	241	209	207	207
q15	562	506	497	497
q16	657	586	601	586
q17	968	532	557	532
q18	7189	6709	6758	6709
q19	1352	966	928	928
q20	468	186	187	186
q21	4020	3256	3319	3256
q22	363	325	311	311
Total cold run time: 106996 ms
Total hot run time: 39945 ms

----- Round 2, with runtime_filter_mode=off -----
q1	7298	7272	7211	7211
q2	331	233	231	231
q3	2886	2746	2946	2746
q4	2044	1832	1783	1783
q5	5652	5693	5628	5628
q6	225	142	143	142
q7	2238	1793	1842	1793
q8	3394	3582	3586	3582
q9	8926	9013	9000	9000
q10	3632	3575	3608	3575
q11	609	509	538	509
q12	812	583	586	583
q13	13130	3146	3067	3067
q14	324	292	290	290
q15	552	504	501	501
q16	671	651	668	651
q17	1861	1650	1620	1620
q18	8334	7769	7803	7769
q19	1763	1593	1453	1453
q20	2094	1819	1816	1816
q21	5541	5414	5437	5414
q22	669	591	608	591
Total cold run time: 72986 ms
Total hot run time: 59955 ms

@wyxxxcat
Copy link
Contributor Author

run p0

@doris-robot
Copy link

TPC-DS: Total hot run time: 197684 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 8267636b46bcaa2f1c5b2bfb3d05d23dd8563385, data reload: false

query1	1298	960	916	916
query2	6250	2340	2323	2323
query3	11108	4840	4872	4840
query4	33296	23512	23439	23439
query5	3923	469	457	457
query6	286	182	185	182
query7	4004	315	321	315
query8	295	241	248	241
query9	9725	2752	2747	2747
query10	448	244	247	244
query11	17862	15182	15311	15182
query12	154	104	98	98
query13	1563	408	412	408
query14	9069	7877	7487	7487
query15	249	216	189	189
query16	7464	451	449	449
query17	1642	614	607	607
query18	1403	334	345	334
query19	382	174	165	165
query20	135	122	121	121
query21	216	108	115	108
query22	4697	4566	4343	4343
query23	34410	33525	33639	33525
query24	10479	2525	2598	2525
query25	630	399	388	388
query26	1201	154	154	154
query27	2353	341	350	341
query28	7702	2491	2513	2491
query29	732	420	422	420
query30	230	153	150	150
query31	1018	841	854	841
query32	90	58	56	56
query33	767	291	302	291
query34	1186	559	520	520
query35	894	749	799	749
query36	1123	981	969	969
query37	154	77	79	77
query38	4282	4119	4234	4119
query39	1537	1474	1457	1457
query40	213	102	100	100
query41	47	42	44	42
query42	123	111	111	111
query43	538	536	498	498
query44	1326	836	846	836
query45	194	174	167	167
query46	1212	746	701	701
query47	2047	1981	1986	1981
query48	413	343	325	325
query49	1000	384	392	384
query50	819	407	383	383
query51	7471	7233	7150	7150
query52	107	97	95	95
query53	260	190	186	186
query54	1213	431	419	419
query55	79	83	86	83
query56	279	260	239	239
query57	1336	1160	1175	1160
query58	246	227	225	225
query59	3471	3319	3277	3277
query60	322	259	249	249
query61	115	105	110	105
query62	933	740	765	740
query63	226	186	193	186
query64	4020	744	677	677
query65	3314	3294	3263	3263
query66	881	306	316	306
query67	16591	15559	15509	15509
query68	5639	569	573	569
query69	494	258	260	258
query70	1268	1151	1129	1129
query71	492	265	254	254
query72	6976	4079	4158	4079
query73	793	374	372	372
query74	9974	8947	8756	8756
query75	3514	2621	2615	2615
query76	3821	1096	1078	1078
query77	569	276	267	267
query78	10371	9404	9435	9404
query79	1882	603	609	603
query80	1208	423	434	423
query81	511	230	233	230
query82	494	121	119	119
query83	207	151	141	141
query84	282	71	76	71
query85	1101	325	304	304
query86	354	302	302	302
query87	4573	4600	4572	4572
query88	4190	2222	2217	2217
query89	433	304	289	289
query90	2095	184	185	184
query91	141	101	115	101
query92	70	52	51	51
query93	4007	551	541	541
query94	913	300	282	282
query95	358	255	296	255
query96	628	279	283	279
query97	2827	2670	2661	2661
query98	220	195	195	195
query99	1710	1423	1418	1418
Total cold run time: 305168 ms
Total hot run time: 197684 ms

@doris-robot
Copy link

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

query1	0.03	0.03	0.03
query2	0.08	0.04	0.04
query3	0.24	0.07	0.07
query4	1.60	0.11	0.10
query5	0.42	0.43	0.41
query6	1.15	0.65	0.65
query7	0.03	0.02	0.02
query8	0.04	0.03	0.03
query9	0.58	0.51	0.50
query10	0.56	0.58	0.55
query11	0.16	0.11	0.11
query12	0.14	0.11	0.11
query13	0.61	0.60	0.61
query14	2.85	2.86	2.75
query15	0.90	0.83	0.82
query16	0.39	0.38	0.39
query17	1.00	1.06	1.08
query18	0.23	0.21	0.23
query19	1.91	1.85	1.95
query20	0.02	0.01	0.01
query21	15.37	0.58	0.58
query22	2.58	2.28	2.47
query23	17.37	0.93	0.91
query24	2.73	1.03	1.18
query25	0.22	0.14	0.18
query26	0.33	0.14	0.14
query27	0.03	0.04	0.05
query28	10.58	1.10	1.08
query29	12.54	3.28	3.27
query30	0.24	0.06	0.06
query31	2.86	0.39	0.38
query32	3.23	0.47	0.45
query33	3.13	3.07	3.04
query34	16.97	4.53	4.53
query35	4.56	4.57	4.50
query36	0.68	0.50	0.48
query37	0.10	0.06	0.06
query38	0.04	0.03	0.03
query39	0.03	0.02	0.02
query40	0.17	0.12	0.12
query41	0.07	0.03	0.03
query42	0.03	0.02	0.02
query43	0.03	0.03	0.03
Total cold run time: 106.83 s
Total hot run time: 33.52 s

Copy link
Contributor

@zclllyybb zclllyybb left a comment

Choose a reason for hiding this comment

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

see fe/fe-core/src/main/antlr4/org/apache/doris/nereids/DorisParser.g4#L681. the Nereids seems already supported alter table add partition .... So we should only make sure the execution of Nereids is ok. ignore legacy planner.

@wyxxxcat
Copy link
Contributor Author

see fe/fe-core/src/main/antlr4/org/apache/doris/nereids/DorisParser.g4#L681. the Nereids seems already supported alter table add partition .... So we should only make sure the execution of Nereids is ok. ignore legacy planner.

6108f10f123ee928404e171a18dbc0a4.jpeg

@w41ter w41ter requested a review from zclllyybb December 25, 2024 02:45
Copy link
Contributor

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

@github-actions github-actions bot added the approved Indicates a PR has been approved by one committer. label Dec 25, 2024
Copy link
Contributor

PR approved by anyone and no changes requested.

@w41ter w41ter merged commit 24e7322 into apache:master Dec 25, 2024
28 of 31 checks passed
github-actions bot pushed a commit that referenced this pull request Dec 25, 2024
```
mysql> alter table t add partition p1 values in (("1", NULL));
ERROR 1105 (HY000): errCode = 2, detailMessage = Syntax error in line 1:
alter table t add partition p1 values in (("1", NULL))
                                                ^
Encountered: NULL
Expected
```
github-actions bot pushed a commit that referenced this pull request Dec 25, 2024
```
mysql> alter table t add partition p1 values in (("1", NULL));
ERROR 1105 (HY000): errCode = 2, detailMessage = Syntax error in line 1:
alter table t add partition p1 values in (("1", NULL))
                                                ^
Encountered: NULL
Expected
```
dataroaring pushed a commit that referenced this pull request Dec 25, 2024
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/2.1.8-merged dev/3.0.4-merged reviewed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants