Skip to content

Commit 02b8d44

Browse files
def-def-
committed
address reviewer comments (#2)
1 parent dd99b69 commit 02b8d44

9 files changed

Lines changed: 99 additions & 66 deletions

File tree

‎src/expr/src/explain/text.rs‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1197,6 +1197,14 @@ impl HumanizerMode for HumanizedExplain {
11971197
fn humanize_ident(col: usize, ident: Ident, f: &mut fmt::Formatter<'_>) -> fmt::Result {
11981198
if ident.as_str() == UNKNOWN_COLUMN_NAME {
11991199
write!(f, "#{col}")
1200+
} else if ident.has_only_bare_chars() {
1201+
// The leading `#c` already disambiguates the column, and EXPLAIN
1202+
// output is never reparsed, so a keyword-named column (`any`,
1203+
// `all`, …) needs no quoting here — unlike in SQL display. Print
1204+
// the name bare for legibility; only fall back to the quoting
1205+
// `Ident` display for names with awkward characters (whitespace,
1206+
// braces, quotes) that would otherwise muddle the `{…}` annotation.
1207+
write!(f, "#{col}{{{}}}", ident.as_str())
12001208
} else {
12011209
write!(f, "#{col}{{{ident}}}")
12021210
}

‎src/sql-parser/src/ast/defs/name.rs‎

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -284,16 +284,29 @@ impl Ident {
284284
self.0.push_str(&suffix);
285285
}
286286

287-
/// An identifier can be printed in bare mode if
288-
/// * it matches the regex `[a-z_][a-z0-9_]*` and
289-
/// * it is not a "reserved keyword."
290-
pub fn can_be_printed_bare(&self) -> bool {
287+
/// Reports whether the identifier matches the regex `[a-z_][a-z0-9_]*`,
288+
/// i.e. it is composed only of characters that never require quoting.
289+
///
290+
/// This is the character-level half of [`Ident::can_be_printed_bare`]. It
291+
/// deliberately does *not* consider keywords: whether a keyword-named
292+
/// identifier needs quoting depends on the surrounding grammar (a
293+
/// reparsing concern), not on its characters. Contexts that only need
294+
/// legible, unambiguous output — rather than a SQL round-trip — should use
295+
/// this instead (see `HumanizedExplain::humanize_ident`).
296+
pub fn has_only_bare_chars(&self) -> bool {
291297
let mut chars = self.0.chars();
292298
chars
293299
.next()
294300
.map(|ch| matches!(ch, 'a'..='z' | '_'))
295301
.unwrap_or(false)
296302
&& chars.all(|ch| matches!(ch, 'a'..='z' | '0'..='9' | '_'))
303+
}
304+
305+
/// An identifier can be printed in bare mode if
306+
/// * it matches the regex `[a-z_][a-z0-9_]*` and
307+
/// * it is not a "reserved keyword."
308+
pub fn can_be_printed_bare(&self) -> bool {
309+
self.has_only_bare_chars()
297310
&& !self
298311
.as_keyword()
299312
.map(|kw| {

‎src/sql-parser/src/ast/metadata.rs‎

Lines changed: 33 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -275,27 +275,7 @@ impl AstDisplay for RawDataType {
275275
.0
276276
.first()
277277
.and_then(|id| id.as_keyword())
278-
.map(|kw| {
279-
use mz_sql_lexer::keywords::*;
280-
matches!(
281-
kw,
282-
MAP | STRING
283-
| BIGINT
284-
| SMALLINT
285-
| DEC
286-
| DECIMAL
287-
| DOUBLE
288-
| FLOAT
289-
| INT
290-
| INTEGER
291-
| REAL
292-
| BOOLEAN
293-
| BYTES
294-
| JSON
295-
| CHAR
296-
| CHARACTER
297-
)
298-
})
278+
.map(data_type_keyword_needs_quoting)
299279
.unwrap_or(false);
300280
if first_ident_clashes {
301281
f.write_str(&name.to_ast_string_stable());
@@ -313,6 +293,38 @@ impl AstDisplay for RawDataType {
313293
}
314294
impl_display!(RawDataType);
315295

296+
/// Reports whether `kw`, as the first component of a data type name, would be
297+
/// reparsed differently by [`crate::parser::Parser::parse_data_type`] — either
298+
/// dispatched into a special grammar (`map[...]`) or canonicalized to a
299+
/// different spelling (`string` → `text`, `bigint` → `int8`, …). Such names
300+
/// must be emitted always-quoted to round-trip.
301+
///
302+
/// Keywords that `parse_data_type` parses back to themselves verbatim
303+
/// (`bpchar`, `varchar`, `time`, `timestamp`, `timestamptz`) round-trip
304+
/// unquoted and are intentionally excluded. Keep this in sync with the keyword
305+
/// arms of `parse_data_type`.
306+
fn data_type_keyword_needs_quoting(kw: mz_sql_lexer::keywords::Keyword) -> bool {
307+
use mz_sql_lexer::keywords::*;
308+
matches!(
309+
kw,
310+
MAP | STRING
311+
| BIGINT
312+
| SMALLINT
313+
| DEC
314+
| DECIMAL
315+
| DOUBLE
316+
| FLOAT
317+
| INT
318+
| INTEGER
319+
| REAL
320+
| BOOLEAN
321+
| BYTES
322+
| JSON
323+
| CHAR
324+
| CHARACTER
325+
)
326+
}
327+
316328
impl<T> FoldNode<Raw, T> for RawDataType
317329
where
318330
T: AstInfo,

‎test/sqllogictest/advent-of-code/2023/aoc_1207.slt‎

Lines changed: 34 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -324,7 +324,7 @@ Explained Query:
324324
Get l6 // { arity: 2 }
325325
Get l5 // { arity: 1 }
326326
cte l8 =
327-
Project (#0, #1, #3{"any"}) // { arity: 3 }
327+
Project (#0, #1, #3{any}) // { arity: 3 }
328328
Join on=(#0 = #2) type=differential // { arity: 4 }
329329
implementation
330330
%1[#0]UK » %0:l0[#0]K
@@ -341,7 +341,7 @@ Explained Query:
341341
Get l5 // { arity: 1 }
342342
cte l9 =
343343
Project (#0, #1) // { arity: 2 }
344-
Filter ((#2{"any"}) IS NULL OR (#2{"any"} = false)) // { arity: 3 }
344+
Filter ((#2{any}) IS NULL OR (#2{any} = false)) // { arity: 3 }
345345
Get l8 // { arity: 3 }
346346
cte l10 =
347347
Distinct project=[#0] // { arity: 1 }
@@ -368,7 +368,7 @@ Explained Query:
368368
Get l11 // { arity: 2 }
369369
Get l10 // { arity: 1 }
370370
cte l13 =
371-
Project (#0, #1, #3{"any"}) // { arity: 3 }
371+
Project (#0, #1, #3{any}) // { arity: 3 }
372372
Join on=(#0 = #2) type=differential // { arity: 4 }
373373
implementation
374374
%1[#0]UK » %0:l9[#0]Kenf
@@ -385,7 +385,7 @@ Explained Query:
385385
Get l10 // { arity: 1 }
386386
cte l14 =
387387
Project (#0, #1) // { arity: 2 }
388-
Filter ((#2{"any"}) IS NULL OR (#2{"any"} = false)) // { arity: 3 }
388+
Filter ((#2{any}) IS NULL OR (#2{any} = false)) // { arity: 3 }
389389
Get l13 // { arity: 3 }
390390
cte l15 =
391391
Distinct project=[#0] // { arity: 1 }
@@ -414,7 +414,7 @@ Explained Query:
414414
Get l17 // { arity: 2 }
415415
Get l15 // { arity: 1 }
416416
cte l19 =
417-
Project (#0, #1, #3{"any"}) // { arity: 3 }
417+
Project (#0, #1, #3{any}) // { arity: 3 }
418418
Join on=(#0 = #2) type=differential // { arity: 4 }
419419
implementation
420420
%1[#0]UK » %0:l14[#0]Kenf
@@ -456,7 +456,7 @@ Explained Query:
456456
Get l22 // { arity: 2 }
457457
Get l20 // { arity: 1 }
458458
cte l24 =
459-
Project (#0..=#2{"any"}, #4{"any"}) // { arity: 4 }
459+
Project (#0..=#2{any}, #4{any}) // { arity: 4 }
460460
Join on=(#0 = #3) type=differential // { arity: 5 }
461461
implementation
462462
%1[#0]UK » %0:l19[#0]K
@@ -474,7 +474,7 @@ Explained Query:
474474
cte l25 =
475475
Project (#0, #1) // { arity: 2 }
476476
Filter ((#4) IS NULL OR (#4 = false)) // { arity: 5 }
477-
Map ((#2{"any"} AND #3{"any"})) // { arity: 5 }
477+
Map ((#2{any} AND #3{any})) // { arity: 5 }
478478
Get l24 // { arity: 4 }
479479
cte l26 =
480480
Distinct project=[#0] // { arity: 1 }
@@ -498,7 +498,7 @@ Explained Query:
498498
Get l27 // { arity: 2 }
499499
Get l26 // { arity: 1 }
500500
cte l29 =
501-
Project (#0, #1, #3{"any"}) // { arity: 3 }
501+
Project (#0, #1, #3{any}) // { arity: 3 }
502502
Join on=(#0 = #2) type=differential // { arity: 4 }
503503
implementation
504504
%1[#0]UK » %0:l25[#0]Kenf
@@ -515,7 +515,7 @@ Explained Query:
515515
Get l26 // { arity: 1 }
516516
cte l30 =
517517
Project (#0, #1) // { arity: 2 }
518-
Filter ((#2{"any"}) IS NULL OR (#2{"any"} = false)) // { arity: 3 }
518+
Filter ((#2{any}) IS NULL OR (#2{any} = false)) // { arity: 3 }
519519
Get l29 // { arity: 3 }
520520
cte l31 =
521521
Distinct project=[#0] // { arity: 1 }
@@ -542,7 +542,7 @@ Explained Query:
542542
Get l32 // { arity: 2 }
543543
Get l31 // { arity: 1 }
544544
cte l34 =
545-
Project (#0, #1, #3{"any"}) // { arity: 3 }
545+
Project (#0, #1, #3{any}) // { arity: 3 }
546546
Join on=(#0 = #2) type=differential // { arity: 4 }
547547
implementation
548548
%1[#0]UK » %0:l30[#0]Kenf
@@ -559,7 +559,7 @@ Explained Query:
559559
Get l31 // { arity: 1 }
560560
cte l35 =
561561
Project (#0, #1) // { arity: 2 }
562-
Filter ((#2{"any"}) IS NULL OR (#2{"any"} = false)) // { arity: 3 }
562+
Filter ((#2{any}) IS NULL OR (#2{any} = false)) // { arity: 3 }
563563
Get l34 // { arity: 3 }
564564
cte l36 =
565565
Distinct project=[#0] // { arity: 1 }
@@ -587,27 +587,27 @@ Explained Query:
587587
Map (translate(#0{hand}, "AKQJT98765432", "ABCDEFGHIJKLM")) // { arity: 4 }
588588
Union // { arity: 3 }
589589
Project (#0, #1, #3) // { arity: 3 }
590-
Filter #2{"any"} // { arity: 4 }
590+
Filter #2{any} // { arity: 4 }
591591
Map (1) // { arity: 4 }
592592
Get l8 // { arity: 3 }
593593
Project (#0, #1, #3) // { arity: 3 }
594-
Filter #2{"any"} // { arity: 4 }
594+
Filter #2{any} // { arity: 4 }
595595
Map (2) // { arity: 4 }
596596
Get l13 // { arity: 3 }
597597
Project (#0, #1, #4) // { arity: 3 }
598-
Filter #2{"any"} AND #3{"any"} // { arity: 5 }
598+
Filter #2{any} AND #3{any} // { arity: 5 }
599599
Map (3) // { arity: 5 }
600600
Get l24 // { arity: 4 }
601601
Project (#0, #1, #3) // { arity: 3 }
602-
Filter #2{"any"} // { arity: 4 }
602+
Filter #2{any} // { arity: 4 }
603603
Map (4) // { arity: 4 }
604604
Get l29 // { arity: 3 }
605605
Project (#0, #1, #3) // { arity: 3 }
606-
Filter #2{"any"} // { arity: 4 }
606+
Filter #2{any} // { arity: 4 }
607607
Map (5) // { arity: 4 }
608608
Get l34 // { arity: 3 }
609609
Project (#0, #1, #4) // { arity: 3 }
610-
Map (case when #3{"any"} then 6 else 7 end) // { arity: 5 }
610+
Map (case when #3{any} then 6 else 7 end) // { arity: 5 }
611611
Join on=(#0 = #2) type=differential // { arity: 4 }
612612
implementation
613613
%1[#0]UK » %0:l35[#0]Kenf
@@ -722,7 +722,7 @@ Explained Query:
722722
Get l47 // { arity: 2 }
723723
Get l46 // { arity: 1 }
724724
cte l49 =
725-
Project (#0, #1, #3{"any"}) // { arity: 3 }
725+
Project (#0, #1, #3{any}) // { arity: 3 }
726726
Join on=(#1 = #2) type=differential // { arity: 4 }
727727
implementation
728728
%1[#0]UK » %0:l43[#1]K
@@ -739,7 +739,7 @@ Explained Query:
739739
Get l46 // { arity: 1 }
740740
cte l50 =
741741
Project (#0, #1) // { arity: 2 }
742-
Filter ((#2{"any"}) IS NULL OR (#2{"any"} = false)) // { arity: 3 }
742+
Filter ((#2{any}) IS NULL OR (#2{any} = false)) // { arity: 3 }
743743
Get l49 // { arity: 3 }
744744
cte l51 =
745745
Distinct project=[#0] // { arity: 1 }
@@ -766,7 +766,7 @@ Explained Query:
766766
Get l52 // { arity: 2 }
767767
Get l51 // { arity: 1 }
768768
cte l54 =
769-
Project (#0, #1, #3{"any"}) // { arity: 3 }
769+
Project (#0, #1, #3{any}) // { arity: 3 }
770770
Join on=(#1 = #2) type=differential // { arity: 4 }
771771
implementation
772772
%1[#0]UK » %0:l50[#1]Kenf
@@ -783,7 +783,7 @@ Explained Query:
783783
Get l51 // { arity: 1 }
784784
cte l55 =
785785
Project (#0, #1) // { arity: 2 }
786-
Filter ((#2{"any"}) IS NULL OR (#2{"any"} = false)) // { arity: 3 }
786+
Filter ((#2{any}) IS NULL OR (#2{any} = false)) // { arity: 3 }
787787
Get l54 // { arity: 3 }
788788
cte l56 =
789789
Distinct project=[#0] // { arity: 1 }
@@ -812,7 +812,7 @@ Explained Query:
812812
Get l58 // { arity: 2 }
813813
Get l56 // { arity: 1 }
814814
cte l60 =
815-
Project (#0, #1, #3{"any"}) // { arity: 3 }
815+
Project (#0, #1, #3{any}) // { arity: 3 }
816816
Join on=(#1 = #2) type=differential // { arity: 4 }
817817
implementation
818818
%1[#0]UK » %0:l55[#1]Kenf
@@ -854,7 +854,7 @@ Explained Query:
854854
Get l63 // { arity: 2 }
855855
Get l61 // { arity: 1 }
856856
cte l65 =
857-
Project (#0..=#2{"any"}, #4{"any"}) // { arity: 4 }
857+
Project (#0..=#2{any}, #4{any}) // { arity: 4 }
858858
Join on=(#1 = #3) type=differential // { arity: 5 }
859859
implementation
860860
%1[#0]UK » %0:l60[#1]K
@@ -872,7 +872,7 @@ Explained Query:
872872
cte l66 =
873873
Project (#0, #1) // { arity: 2 }
874874
Filter ((#4) IS NULL OR (#4 = false)) // { arity: 5 }
875-
Map ((#2{"any"} AND #3{"any"})) // { arity: 5 }
875+
Map ((#2{any} AND #3{any})) // { arity: 5 }
876876
Get l65 // { arity: 4 }
877877
cte l67 =
878878
Distinct project=[#0] // { arity: 1 }
@@ -896,7 +896,7 @@ Explained Query:
896896
Get l68 // { arity: 2 }
897897
Get l67 // { arity: 1 }
898898
cte l70 =
899-
Project (#0, #1, #3{"any"}) // { arity: 3 }
899+
Project (#0, #1, #3{any}) // { arity: 3 }
900900
Join on=(#1 = #2) type=differential // { arity: 4 }
901901
implementation
902902
%1[#0]UK » %0:l66[#1]Kenf
@@ -913,7 +913,7 @@ Explained Query:
913913
Get l67 // { arity: 1 }
914914
cte l71 =
915915
Project (#0, #1) // { arity: 2 }
916-
Filter ((#2{"any"}) IS NULL OR (#2{"any"} = false)) // { arity: 3 }
916+
Filter ((#2{any}) IS NULL OR (#2{any} = false)) // { arity: 3 }
917917
Get l70 // { arity: 3 }
918918
cte l72 =
919919
Distinct project=[#0] // { arity: 1 }
@@ -940,7 +940,7 @@ Explained Query:
940940
Get l73 // { arity: 2 }
941941
Get l72 // { arity: 1 }
942942
cte l75 =
943-
Project (#0, #1, #3{"any"}) // { arity: 3 }
943+
Project (#0, #1, #3{any}) // { arity: 3 }
944944
Join on=(#1 = #2) type=differential // { arity: 4 }
945945
implementation
946946
%1[#0]UK » %0:l71[#1]Kenf
@@ -957,7 +957,7 @@ Explained Query:
957957
Get l72 // { arity: 1 }
958958
cte l76 =
959959
Project (#0, #1) // { arity: 2 }
960-
Filter ((#2{"any"}) IS NULL OR (#2{"any"} = false)) // { arity: 3 }
960+
Filter ((#2{any}) IS NULL OR (#2{any} = false)) // { arity: 3 }
961961
Get l75 // { arity: 3 }
962962
cte l77 =
963963
Distinct project=[#0] // { arity: 1 }
@@ -993,27 +993,27 @@ Explained Query:
993993
Map (translate(#0{hand}, "AKQT98765432J", "ABCDEFGHIJKLM")) // { arity: 3 }
994994
Union // { arity: 2 }
995995
Project (#0, #3) // { arity: 2 }
996-
Filter #2{"any"} // { arity: 4 }
996+
Filter #2{any} // { arity: 4 }
997997
Map (1) // { arity: 4 }
998998
Get l49 // { arity: 3 }
999999
Project (#0, #3) // { arity: 2 }
1000-
Filter #2{"any"} // { arity: 4 }
1000+
Filter #2{any} // { arity: 4 }
10011001
Map (2) // { arity: 4 }
10021002
Get l54 // { arity: 3 }
10031003
Project (#0, #4) // { arity: 2 }
1004-
Filter #2{"any"} AND #3{"any"} // { arity: 5 }
1004+
Filter #2{any} AND #3{any} // { arity: 5 }
10051005
Map (3) // { arity: 5 }
10061006
Get l65 // { arity: 4 }
10071007
Project (#0, #3) // { arity: 2 }
1008-
Filter #2{"any"} // { arity: 4 }
1008+
Filter #2{any} // { arity: 4 }
10091009
Map (4) // { arity: 4 }
10101010
Get l70 // { arity: 3 }
10111011
Project (#0, #3) // { arity: 2 }
1012-
Filter #2{"any"} // { arity: 4 }
1012+
Filter #2{any} // { arity: 4 }
10131013
Map (5) // { arity: 4 }
10141014
Get l75 // { arity: 3 }
10151015
Project (#0, #4) // { arity: 2 }
1016-
Map (case when #3{"any"} then 6 else 7 end) // { arity: 5 }
1016+
Map (case when #3{any} then 6 else 7 end) // { arity: 5 }
10171017
Join on=(#1 = #2) type=differential // { arity: 4 }
10181018
implementation
10191019
%1[#0]UK » %0:l76[#1]Kenf

‎test/sqllogictest/advent-of-code/2023/aoc_1208.slt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ Explained Query:
7575
Map (0) // { arity: 4 }
7676
ReadStorage materialize.public.paths // { arity: 3 }
7777
Project (#3, #8, #9) // { arity: 3 }
78-
Map (case when (#7{step} = "L") then #1{"left"} else case when (#7{step} = "R") then #2{"right"} else "???" end end, (#5{steps} + 1)) // { arity: 10 }
78+
Map (case when (#7{step} = "L") then #1{left} else case when (#7{step} = "R") then #2{right} else "???" end end, (#5{steps} + 1)) // { arity: 10 }
7979
Join on=(#0{state} = #4{state} AND #6{steps} = (1 + (#5{steps} % 263))) type=delta // { arity: 8 }
8080
implementation
8181
%0:paths » %1:l0[#1{state}]Kf » %2[#0{steps}]K

0 commit comments

Comments
 (0)