Optimise prefix/identifier parsing#2227
Conversation
alexander-beedie
force-pushed
the
perf-parse-prefix
branch
from
ce51f0c to
5d9eb33
Compare
alexander-beedie
force-pushed
the
perf-parse-prefix
branch
from
5d9eb33 to
777387e
Compare
|
Rebased / conflicts resolved 👍 |
| // Fast path: for non-keyword words not followed by | ||
| // special tokens, produce an identifier directly. | ||
| let peek = &self.peek_token_ref().token; | ||
| let is_special = matches!( | ||
| peek, | ||
| Token::LParen | ||
| | Token::Arrow | ||
| | Token::SingleQuotedString(_) | ||
| | Token::DoubleQuotedString(_) | ||
| | Token::HexStringLiteral(_) | ||
| ); | ||
| // Typed lambda: `a INT -> a * 2` | ||
| let is_typed_lambda = matches!(peek, Token::Word(_)) | ||
| && self.dialect.supports_lambda_functions() | ||
| && self.peek_nth_token_ref(1).token == Token::Arrow; | ||
| if !is_special && !is_typed_lambda { | ||
| Ok(Expr::Identifier(w.to_ident(span))) | ||
| } else { | ||
| // Non-keyword followed by special token (e.g. function call) | ||
| let w = w.clone(); | ||
| Ok(self.parse_expr_prefix_by_unreserved_word(w, span)?) | ||
| } |
There was a problem hiding this comment.
I think this block isnt ideal. its not clear what the special flag entails and why we're specifically looking ahead for a lambda function and I'm afraid that we'll likely accumulate similar special cases going forward
There was a problem hiding this comment.
Sure, no problem. I can drop this part of the patch for now and see if there's a simpler way to get the benefits of the fast path 🤔
There was a problem hiding this comment.
Actually, it looks like the NoKeyword fast-path was responsible for the vast majority of the performance gains here; the reduced patch would just save us a single String clone, which isn't measurable in benchmarks.
I'll close this one out for simplicity as I have several other more meaningful optimisations brewing 👍
2 participants
Description
parse_expr_prefix_by_unreserved_wordto useinto_ident()(move) instead ofto_ident()(clone).NoKeywordfast path inparse_prefix(most column references are likely plain identifiers).Results
Other
sqlparser_benchtests just show noise (not really impacted by this change):