Skip to content

[fix](fe) Quote variant predefined field comments in generated SQL - #68194

Open
eldenmoon wants to merge 1 commit into
apache:masterfrom
eldenmoon:fix-variant-field-comment-quote
Open

eldenmoon wants to merge 1 commit into
apache:masterfrom
eldenmoon:fix-variant-field-comment-quote

Conversation

@eldenmoon

Copy link
Copy Markdown
Member

What problem does this PR solve?

Issue Number: None

Related PR: None

Problem Summary:

A comment on a VARIANT predefined field is printed without SQL-literal escaping, so the output of SHOW CREATE TABLE cannot be replayed when the comment contains a quote:

CREATE TABLE t (id INT, v VARIANT<'price':INT COMMENT "O'Reilly">) ...;
SHOW CREATE TABLE t;
-- v variant<'price':int COMMENT 'O'Reilly', ...>
-- replaying it fails: no viable alternative at input 'variant<'price':int COMMENT 'O'Reilly'

Root cause: both catalog.VariantField.toSql (used by SHOW CREATE TABLE through Column.toSql) and nereids.types.VariantField.toSql wrap the raw comment in single quotes. The parser side was also inconsistent with the rest of the grammar: visitVariantSubColType only strips the outer quotes and unescapes backslashes, so a doubled quote such as 'O''Reilly' is stored as O''Reilly.

Fix, following how STRUCT field comments are already handled:

  • print the comment with SqlUtils.quoteStringLiteral / SqlLiteralUtils.quoteStringLiteral;
  • parse it with SqlLiteralUtils.parseStringLiteral, like visitComplexColType does.

After the fix the comment above is printed as COMMENT "O'Reilly", and quotes, doubled quotes and backslashes survive a SHOW CREATE TABLE → CREATE TABLE round trip.

Release note

Fix SHOW CREATE TABLE output that could not be replayed when a VARIANT predefined field comment contains quotes or backslashes.

Check List (For Author)

  • Test: Regression test (variant_p0/predefine/test_variant_predefine_comment_escape) / Unit Test (TypeTest#testVariantFieldCommentToSqlRoundTrip)
  • Behavior changed: Yes. SHOW CREATE TABLE prints predefined field comments as double-quoted, escaped literals; a doubled quote inside a comment is now stored as a single quote, like column and STRUCT field comments.
  • Does this need documentation: No

🤖 Generated with Claude Code

### What problem does this PR solve?

Issue Number: None

Related PR: None

Problem Summary:

A comment on a VARIANT predefined field is printed without SQL-literal escaping, so the output of SHOW CREATE TABLE cannot be replayed when the comment contains a quote:

```sql
CREATE TABLE t (id INT, v VARIANT<'price':INT COMMENT "O'Reilly">) ...;
SHOW CREATE TABLE t;
-- v variant<'price':int COMMENT 'O'Reilly', ...>
-- replaying it fails: no viable alternative at input 'variant<'price':int COMMENT 'O'Reilly'
```

Root cause: both `catalog.VariantField.toSql` (used by SHOW CREATE TABLE through `Column.toSql`) and `nereids.types.VariantField.toSql` wrap the raw comment in single quotes. The parser side was also inconsistent with the rest of the grammar: `visitVariantSubColType` only strips the outer quotes and unescapes backslashes, so a doubled quote such as `'O''Reilly'` is stored as `O''Reilly`.

Fix, following how STRUCT field comments are already handled:
- print the comment with `SqlUtils.quoteStringLiteral` / `SqlLiteralUtils.quoteStringLiteral`;
- parse it with `SqlLiteralUtils.parseStringLiteral`, like `visitComplexColType` does.

After the fix the comment above is printed as `COMMENT "O'Reilly"`, and quotes, doubled quotes and backslashes survive a SHOW CREATE TABLE → CREATE TABLE round trip.

### Release note

Fix SHOW CREATE TABLE output that could not be replayed when a VARIANT predefined field comment contains quotes or backslashes.

### Check List (For Author)

- Test: Regression test (variant_p0/predefine/test_variant_predefine_comment_escape) / Unit Test (TypeTest#testVariantFieldCommentToSqlRoundTrip)
- Behavior changed: Yes. SHOW CREATE TABLE prints predefined field comments as double-quoted, escaped literals; a doubled quote inside a comment is now stored as a single quote, like column and STRUCT field comments.
- Does this need documentation: No

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@eldenmoon

Copy link
Copy Markdown
Member Author

run buildall

@hello-stephen

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

------ Round 1 ----------------------------------
============================================
q1	17643	3921	3881	3881
q2	2298	349	308	308
q3	10059	1372	796	796
q4	4707	477	343	343
q5	7600	819	556	556
q6	205	173	134	134
q7	759	799	582	582
q8	10208	1454	1562	1454
q9	6062	4198	4157	4157
q10	6797	1680	1363	1363
q11	444	267	246	246
q12	697	407	287	287
q13	18173	2619	2008	2008
q14	256	261	235	235
q15	q16	732	716	672	672
q17	1774	1174	981	981
q18	6520	5591	5549	5549
q19	1146	1228	1060	1060
q20	488	383	265	265
q21	5598	2555	2372	2372
q22	414	346	289	289
Total cold run time: 102580 ms
Total hot run time: 27538 ms

----- Round 2, with runtime_filter_mode=off -----
============================================
q1	4259	4142	4183	4142
q2	797	569	580	569
q3	4506	4937	4393	4393
q4	2218	2313	1431	1431
q5	4206	4151	4105	4105
q6	222	171	124	124
q7	1707	1620	1389	1389
q8	2607	2116	2068	2068
q9	7412	7293	7216	7216
q10	4280	4300	3857	3857
q11	563	413	373	373
q12	721	744	525	525
q13	2463	2833	2143	2143
q14	289	293	272	272
q15	q16	728	929	657	657
q17	7736	7196	7093	7093
q18	11948	11073	11802	11073
q19	1183	1080	1081	1080
q20	2258	2208	1973	1973
q21	5876	4593	4581	4581
q22	520	459	414	414
Total cold run time: 66499 ms
Total hot run time: 59478 ms

@hello-stephen

Copy link
Copy Markdown
Contributor
TPC-DS: Total hot run time: 153415 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 b32566b685af8bf87fe2e5fdcf94fc732e6006df, data reload: false

query5	4315	606	461	461
query6	444	186	159	159
query7	4843	535	283	283
query8	337	178	162	162
query9	8781	3928	3907	3907
query10	443	325	250	250
query11	5850	2160	2024	2024
query12	156	100	100	100
query13	1292	580	405	405
query14	6508	4528	4219	4219
query14_1	4007	3967	3969	3967
query15	199	197	184	184
query16	972	448	454	448
query17	912	708	529	529
query18	2443	458	354	354
query19	197	178	138	138
query20	108	105	102	102
query21	220	133	116	116
query22	13013	12944	12800	12800
query23	15488	14621	14069	14069
query23_1	14047	14049	14171	14049
query24	7488	1710	1207	1207
query24_1	1235	1232	1212	1212
query25	538	432	360	360
query26	1252	319	164	164
query27	2692	552	344	344
query28	4580	1972	1978	1972
query29	1054	589	479	479
query30	310	230	201	201
query31	890	770	623	623
query32	147	96	102	96
query33	516	314	245	245
query34	1184	1131	619	619
query35	728	783	651	651
query36	784	820	720	720
query37	156	103	93	93
query38	1808	1758	1713	1713
query39	698	702	652	652
query39_1	643	664	650	650
query40	227	123	102	102
query41	69	67	71	67
query42	91	91	88	88
query43	336	342	294	294
query44	1325	683	702	683
query45	192	177	173	173
query46	1033	1134	701	701
query47	1477	1503	1375	1375
query48	397	400	348	348
query49	582	395	277	277
query50	1010	342	242	242
query51	10816	10393	10760	10393
query52	83	83	71	71
query53	248	253	180	180
query54	259	197	200	197
query55	75	71	68	68
query56	229	214	203	203
query57	1459	1451	1362	1362
query58	238	211	207	207
query59	1961	2044	1787	1787
query60	282	240	214	214
query61	146	146	144	144
query62	384	318	265	265
query63	216	179	175	175
query64	2840	1042	805	805
query65	4013	4019	3967	3967
query66	1848	419	304	304
query67	23961	20111	19898	19898
query68	3229	1473	925	925
query69	400	308	267	267
query70	941	889	879	879
query71	304	230	233	230
query72	2846	2458	2100	2100
query73	843	766	430	430
query74	4586	4463	4261	4261
query75	2284	2271	1925	1925
query76	2318	1097	710	710
query77	353	393	299	299
query78	9053	9022	8486	8486
query79	1342	1199	730	730
query80	569	447	353	353
query81	460	279	235	235
query82	608	162	128	128
query83	348	269	248	248
query84	309	140	110	110
query85	823	482	408	408
query86	320	240	236	236
query87	1990	1971	1838	1838
query88	3603	2728	2671	2671
query89	347	286	244	244
query90	1925	175	171	171
query91	171	157	125	125
query92	107	90	88	88
query93	1629	1460	867	867
query94	544	364	306	306
query95	658	360	417	360
query96	1101	829	335	335
query97	2457	2434	2287	2287
query98	196	191	182	182
query99	732	723	614	614
Total cold run time: 244719 ms
Total hot run time: 153415 ms

@hello-stephen

Copy link
Copy Markdown
Contributor
ClickBench: Total hot run time: 24.06 s
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/clickbench-tools
ClickBench test result on commit b32566b685af8bf87fe2e5fdcf94fc732e6006df, data reload: false

query1	0.01	0.01	0.01
query2	0.08	0.05	0.05
query3	0.26	0.14	0.14
query4	1.62	0.14	0.14
query5	0.24	0.23	0.23
query6	1.16	0.97	0.93
query7	0.04	0.01	0.01
query8	0.05	0.04	0.04
query9	0.38	0.34	0.34
query10	0.57	0.60	0.58
query11	0.20	0.14	0.14
query12	0.19	0.15	0.15
query13	0.47	0.48	0.46
query14	0.95	0.94	0.96
query15	0.59	0.58	0.58
query16	0.31	0.35	0.32
query17	1.09	1.06	1.09
query18	0.23	0.20	0.20
query19	2.00	1.99	1.98
query20	0.02	0.01	0.01
query21	15.46	0.21	0.13
query22	4.89	0.06	0.05
query23	16.14	0.31	0.11
query24	2.96	0.42	0.31
query25	0.12	0.05	0.04
query26	0.72	0.21	0.15
query27	0.05	0.04	0.04
query28	3.51	0.76	0.36
query29	12.46	4.05	3.22
query30	0.28	0.15	0.15
query31	2.78	0.58	0.30
query32	3.22	0.60	0.49
query33	3.23	3.15	3.14
query34	15.61	4.04	3.28
query35	3.22	3.19	3.22
query36	0.55	0.43	0.44
query37	0.08	0.06	0.06
query38	0.05	0.04	0.04
query39	0.04	0.03	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.05	0.03	0.03
Total cold run time: 96.19 s
Total hot run time: 24.06 s

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants