Skip to content

Commit c0ad6b6

Browse files
Snehil-ShahSnehil-Shah
committed
fix: canonicalize keywords
Signed-off-by: Snehil Shah <snehilshah.989@gmail.com>
1 parent b20c1cc commit c0ad6b6

11 files changed

Lines changed: 306 additions & 23 deletions

‎Makefile‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@ TEST_SRCS = $(TEST_DIR)/test_main.cpp \
5151
$(TEST_DIR)/test_update.cpp \
5252
$(TEST_DIR)/test_delete.cpp \
5353
$(TEST_DIR)/test_compound.cpp \
54+
$(TEST_DIR)/test_keyword_canonicalization.cpp \
5455
$(TEST_DIR)/test_digest.cpp \
5556
$(TEST_DIR)/test_misc_stmts.cpp \
5657
$(TEST_DIR)/test_value.cpp \

‎include/sql_parser/arena.h‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,10 @@ class Arena {
3030

3131
StringRef allocate_string(const char* src, uint32_t len);
3232

33+
StringRef allocate_upper(StringRef text);
34+
35+
StringRef allocate_lower(StringRef text);
36+
3337
void reset();
3438

3539
size_t bytes_used() const;

‎include/sql_parser/compound_query_parser.h‎

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,10 @@ class CompoundQueryParser {
7373
AstNode* lock = make_node(arena_, NodeType::NODE_LOCKING_CLAUSE);
7474
if (lock) {
7575
Token strength = tok_.next_token();
76-
lock->add_child(make_node(arena_, NodeType::NODE_IDENTIFIER, strength.text));
76+
lock->add_child(make_node(arena_, NodeType::NODE_IDENTIFIER,
77+
strength.type == TokenType::TK_UPDATE ? StringRef{"UPDATE", 6}
78+
: strength.type == TokenType::TK_SHARE ? StringRef{"SHARE", 5}
79+
: strength.text));
7780
result->add_child(lock);
7881
}
7982
}
@@ -124,7 +127,11 @@ class CompoundQueryParser {
124127

125128
// Consume the set operator
126129
tok_.skip();
127-
StringRef op_text = t.text;
130+
StringRef op_text =
131+
t.type == TokenType::TK_UNION ? StringRef{"UNION", 5}
132+
: t.type == TokenType::TK_INTERSECT ? StringRef{"INTERSECT", 9}
133+
: t.type == TokenType::TK_EXCEPT ? StringRef{"EXCEPT", 6}
134+
: t.text;
128135

129136
// Check for optional ALL
130137
uint16_t flags = 0;
@@ -226,7 +233,11 @@ class CompoundQueryParser {
226233
if (prec == 0 || prec <= min_prec) break;
227234

228235
tok_.skip();
229-
StringRef op_text = t.text;
236+
StringRef op_text =
237+
t.type == TokenType::TK_UNION ? StringRef{"UNION", 5}
238+
: t.type == TokenType::TK_INTERSECT ? StringRef{"INTERSECT", 9}
239+
: t.type == TokenType::TK_EXCEPT ? StringRef{"EXCEPT", 6}
240+
: t.text;
230241

231242
uint16_t flags = 0;
232243
if (tok_.peek().type == TokenType::TK_ALL) {
@@ -278,7 +289,9 @@ class CompoundQueryParser {
278289
Token dir = tok_.peek();
279290
if (dir.type == TokenType::TK_ASC || dir.type == TokenType::TK_DESC) {
280291
tok_.skip();
281-
item->add_child(make_node(arena_, NodeType::NODE_IDENTIFIER, dir.text));
292+
item->add_child(make_node(arena_, NodeType::NODE_IDENTIFIER,
293+
dir.type == TokenType::TK_ASC ? StringRef{"ASC", 3}
294+
: StringRef{"DESC", 4}));
282295
}
283296

284297
order_by->add_child(item);

‎include/sql_parser/delete_parser.h‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -294,7 +294,9 @@ class DeleteParser {
294294
Token dir = tok_.peek();
295295
if (dir.type == TokenType::TK_ASC || dir.type == TokenType::TK_DESC) {
296296
tok_.skip();
297-
item->add_child(make_node(arena_, NodeType::NODE_IDENTIFIER, dir.text));
297+
item->add_child(make_node(arena_, NodeType::NODE_IDENTIFIER,
298+
dir.type == TokenType::TK_ASC ? StringRef{"ASC", 3}
299+
: StringRef{"DESC", 4}));
298300
}
299301

300302
order_by->add_child(item);

‎include/sql_parser/expression_parser.h‎

Lines changed: 47 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,24 @@ using SubqueryParseCallback = AstNode*(*)(Tokenizer<D>&, Arena&);
4141
template <Dialect D>
4242
class ExpressionParser {
4343
public:
44+
// Keyword operators are stored under their canonical spelling.
45+
static StringRef canonical_op(const Token& op) {
46+
switch (op.type) {
47+
case TokenType::TK_AND: return StringRef{"AND", 3};
48+
case TokenType::TK_OR: return StringRef{"OR", 2};
49+
case TokenType::TK_XOR: return StringRef{"XOR", 3};
50+
case TokenType::TK_NOT: return StringRef{"NOT", 3};
51+
case TokenType::TK_IS: return StringRef{"IS", 2};
52+
case TokenType::TK_IN: return StringRef{"IN", 2};
53+
case TokenType::TK_LIKE: return StringRef{"LIKE", 4};
54+
case TokenType::TK_REGEXP: return StringRef{"REGEXP", 6};
55+
case TokenType::TK_DIV: return StringRef{"DIV", 3};
56+
case TokenType::TK_MOD: return StringRef{"MOD", 3};
57+
case TokenType::TK_BETWEEN: return StringRef{"BETWEEN", 7};
58+
default: return op.text;
59+
}
60+
}
61+
4462
ExpressionParser(Tokenizer<D>& tokenizer, Arena& arena)
4563
: tok_(tokenizer), arena_(arena) {}
4664

@@ -120,7 +138,14 @@ class ExpressionParser {
120138
}
121139
case TokenType::TK_NULL: {
122140
tok_.skip();
123-
return make_node_from_token(arena_, NodeType::NODE_LITERAL_NULL, t);
141+
{
142+
// Keep the source span lossless, but store the keyword under
143+
// its canonical spelling.
144+
AstNode* null_node =
145+
make_node_from_token(arena_, NodeType::NODE_LITERAL_NULL, t);
146+
if (null_node) null_node->set_value(StringRef{"NULL", 4});
147+
return null_node;
148+
}
124149
}
125150
case TokenType::TK_TRUE:
126151
case TokenType::TK_FALSE: {
@@ -186,7 +211,7 @@ class ExpressionParser {
186211
tok_.skip();
187212
AstNode* operand = parse(Precedence::UNARY);
188213
if (!operand) return nullptr;
189-
AstNode* node = make_node(arena_, NodeType::NODE_UNARY_OP, t.text);
214+
AstNode* node = make_node(arena_, NodeType::NODE_UNARY_OP, canonical_op(t));
190215
set_span_through_node_(node, t.source, operand);
191216
node->add_child(operand);
192217
return node;
@@ -196,7 +221,7 @@ class ExpressionParser {
196221
tok_.skip();
197222
AstNode* operand = parse(Precedence::UNARY);
198223
if (!operand) return nullptr;
199-
AstNode* node = make_node(arena_, NodeType::NODE_UNARY_OP, t.text);
224+
AstNode* node = make_node(arena_, NodeType::NODE_UNARY_OP, canonical_op(t));
200225
set_span_through_node_(node, t.source, operand);
201226
node->add_child(operand);
202227
return node;
@@ -205,7 +230,7 @@ class ExpressionParser {
205230
tok_.skip();
206231
AstNode* operand = parse(Precedence::NOT);
207232
if (!operand) return nullptr;
208-
AstNode* node = make_node(arena_, NodeType::NODE_UNARY_OP, t.text);
233+
AstNode* node = make_node(arena_, NodeType::NODE_UNARY_OP, canonical_op(t));
209234
set_span_through_node_(node, t.source, operand);
210235
node->add_child(operand);
211236
return node;
@@ -326,7 +351,16 @@ class ExpressionParser {
326351
// Check for function call: name(
327352
if (tok_.peek().type == TokenType::TK_LPAREN) {
328353
tok_.skip(); // consume (
329-
AstNode* func = make_node(arena_, NodeType::NODE_FUNCTION_CALL, name_token.text);
354+
// Function names are case-insensitive, so store them under a canonical spelling.
355+
StringRef func_name = name_token.text;
356+
if constexpr (D == Dialect::MySQL) {
357+
// MySQL folds all function names up:
358+
func_name = arena_.allocate_upper(func_name);
359+
} else if (!token_was_delimited_(name_token)) {
360+
// PostgreSQL folds undelimited function names down:
361+
func_name = arena_.allocate_lower(func_name);
362+
}
363+
AstNode* func = make_node(arena_, NodeType::NODE_FUNCTION_CALL, func_name);
330364
// CAST uses `CAST(expr AS type)` rather than a comma-separated
331365
// argument list. Model it as a function call so consumers can
332366
// reject or handle the expression without leaving valid input
@@ -449,25 +483,25 @@ class ExpressionParser {
449483
tok_.skip();
450484
AstNode* in_node = parse_in(left);
451485
// Wrap in NOT
452-
AstNode* not_node = make_node(arena_, NodeType::NODE_UNARY_OP, op.text);
486+
AstNode* not_node = make_node(arena_, NodeType::NODE_UNARY_OP, canonical_op(op));
453487
not_node->add_child(in_node);
454488
return not_node;
455489
}
456490
if (actual_op.type == TokenType::TK_BETWEEN) {
457491
tok_.skip();
458492
AstNode* between_node = parse_between(left);
459-
AstNode* not_node = make_node(arena_, NodeType::NODE_UNARY_OP, op.text);
493+
AstNode* not_node = make_node(arena_, NodeType::NODE_UNARY_OP, canonical_op(op));
460494
not_node->add_child(between_node);
461495
return not_node;
462496
}
463497
if (actual_op.type == TokenType::TK_LIKE ||
464498
actual_op.type == TokenType::TK_REGEXP) {
465499
tok_.skip();
466500
AstNode* right = parse(prec);
467-
AstNode* like_node = make_node(arena_, NodeType::NODE_BINARY_OP, actual_op.text);
501+
AstNode* like_node = make_node(arena_, NodeType::NODE_BINARY_OP, canonical_op(actual_op));
468502
like_node->add_child(left);
469503
if (right) like_node->add_child(right);
470-
AstNode* not_node = make_node(arena_, NodeType::NODE_UNARY_OP, op.text);
504+
AstNode* not_node = make_node(arena_, NodeType::NODE_UNARY_OP, canonical_op(op));
471505
not_node->add_child(like_node);
472506
return not_node;
473507
}
@@ -511,7 +545,7 @@ class ExpressionParser {
511545
// Standard binary operator
512546
AstNode* right = parse(prec);
513547
if (!right) return left;
514-
AstNode* node = make_node(arena_, NodeType::NODE_BINARY_OP, op.text);
548+
AstNode* node = make_node(arena_, NodeType::NODE_BINARY_OP, canonical_op(op));
515549
node->add_child(left);
516550
node->add_child(right);
517551
return node;
@@ -758,7 +792,9 @@ class ExpressionParser {
758792
Token dir = tok_.peek();
759793
if (dir.type == TokenType::TK_ASC || dir.type == TokenType::TK_DESC) {
760794
tok_.skip();
761-
item->add_child(make_node(arena_, NodeType::NODE_IDENTIFIER, dir.text));
795+
item->add_child(make_node(arena_, NodeType::NODE_IDENTIFIER,
796+
dir.type == TokenType::TK_ASC ? StringRef{"ASC", 3}
797+
: StringRef{"DESC", 4}));
762798
}
763799
ord->add_child(item);
764800
if (tok_.peek().type == TokenType::TK_COMMA) tok_.skip();

‎include/sql_parser/select_parser.h‎

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -123,11 +123,14 @@ class SelectParser {
123123
if (t.type == TokenType::TK_DISTINCT || t.type == TokenType::TK_ALL) {
124124
if (!opts) opts = make_node(arena_, NodeType::NODE_SELECT_OPTIONS);
125125
tok_.skip();
126-
opts->add_child(make_node(arena_, NodeType::NODE_IDENTIFIER, t.text));
126+
opts->add_child(make_node(arena_, NodeType::NODE_IDENTIFIER,
127+
t.type == TokenType::TK_DISTINCT ? StringRef{"DISTINCT", 8}
128+
: StringRef{"ALL", 3}));
127129
} else if (t.type == TokenType::TK_SQL_CALC_FOUND_ROWS) {
128130
if (!opts) opts = make_node(arena_, NodeType::NODE_SELECT_OPTIONS);
129131
tok_.skip();
130-
opts->add_child(make_node(arena_, NodeType::NODE_IDENTIFIER, t.text));
132+
opts->add_child(make_node(arena_, NodeType::NODE_IDENTIFIER,
133+
StringRef{"SQL_CALC_FOUND_ROWS", 19}));
131134
} else {
132135
break;
133136
}
@@ -298,7 +301,9 @@ class SelectParser {
298301
Token dir = tok_.peek();
299302
if (dir.type == TokenType::TK_ASC || dir.type == TokenType::TK_DESC) {
300303
tok_.skip();
301-
item->add_child(make_node(arena_, NodeType::NODE_IDENTIFIER, dir.text));
304+
item->add_child(make_node(arena_, NodeType::NODE_IDENTIFIER,
305+
dir.type == TokenType::TK_ASC ? StringRef{"ASC", 3}
306+
: StringRef{"DESC", 4}));
302307
}
303308

304309
order_by->add_child(item);
@@ -349,7 +354,10 @@ class SelectParser {
349354

350355
tok_.skip(); // consume FOR
351356
Token strength = tok_.next_token(); // UPDATE or SHARE
352-
lock->add_child(make_node(arena_, NodeType::NODE_IDENTIFIER, strength.text));
357+
lock->add_child(make_node(arena_, NodeType::NODE_IDENTIFIER,
358+
strength.type == TokenType::TK_UPDATE ? StringRef{"UPDATE", 6}
359+
: strength.type == TokenType::TK_SHARE ? StringRef{"SHARE", 5}
360+
: strength.text));
353361

354362
// Optional: OF table_list
355363
if (tok_.peek().type == TokenType::TK_OF) {

‎include/sql_parser/table_ref_parser.h‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -135,8 +135,8 @@ class TableRefParser {
135135
}
136136

137137
// Set join type as value (covers the span from first modifier to JOIN)
138-
StringRef join_type{join_type_start.ptr,
139-
static_cast<uint32_t>((join_type_end.ptr + join_type_end.len) - join_type_start.ptr)};
138+
StringRef join_type = arena_.allocate_upper(StringRef{join_type_start.ptr,
139+
static_cast<uint32_t>((join_type_end.ptr + join_type_end.len) - join_type_start.ptr)});
140140
join->value_ptr = join_type.ptr;
141141
join->value_len = join_type.len;
142142

‎include/sql_parser/update_parser.h‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -238,7 +238,9 @@ class UpdateParser {
238238
Token dir = tok_.peek();
239239
if (dir.type == TokenType::TK_ASC || dir.type == TokenType::TK_DESC) {
240240
tok_.skip();
241-
item->add_child(make_node(arena_, NodeType::NODE_IDENTIFIER, dir.text));
241+
item->add_child(make_node(arena_, NodeType::NODE_IDENTIFIER,
242+
dir.type == TokenType::TK_ASC ? StringRef{"ASC", 3}
243+
: StringRef{"DESC", 4}));
242244
}
243245

244246
order_by->add_child(item);

‎src/sql_parser/arena.cpp‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,28 @@ StringRef Arena::allocate_string(const char* src, uint32_t len) {
6262
return StringRef{static_cast<const char*>(mem), len};
6363
}
6464

65+
StringRef Arena::allocate_upper(StringRef text) {
66+
if (!text.ptr || text.len == 0) return text;
67+
char* buf = static_cast<char*>(allocate(text.len));
68+
if (!buf) return text;
69+
for (uint32_t i = 0; i < text.len; ++i) {
70+
const char c = text.ptr[i];
71+
buf[i] = (c >= 'a' && c <= 'z') ? static_cast<char>(c - 32) : c;
72+
}
73+
return StringRef{buf, text.len};
74+
}
75+
76+
StringRef Arena::allocate_lower(StringRef text) {
77+
if (!text.ptr || text.len == 0) return text;
78+
char* buf = static_cast<char*>(allocate(text.len));
79+
if (!buf) return text;
80+
for (uint32_t i = 0; i < text.len; ++i) {
81+
const char c = text.ptr[i];
82+
buf[i] = (c >= 'A' && c <= 'Z') ? static_cast<char>(c + 32) : c;
83+
}
84+
return StringRef{buf, text.len};
85+
}
86+
6587
void Arena::reset() {
6688
Block* b = primary_->next;
6789
while (b) {

‎tests/test_digest.cpp‎

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,14 @@ TEST_F(MySQLDigestTest, KeywordsUppercased) {
119119
"SELECT * FROM t WHERE id = ?");
120120
}
121121

122+
TEST_F(MySQLDigestTest, KeywordsUppercasedFromAst) {
123+
EXPECT_EQ(normalized("select a from t where a = 1 and b = 2"),
124+
"SELECT a FROM t WHERE a = ? AND b = ?");
125+
EXPECT_EQ(normalized("select 1 union select 2"), "SELECT ? UNION SELECT ?");
126+
EXPECT_EQ(normalized("select a from t order by a desc"),
127+
"SELECT a FROM t ORDER BY a DESC");
128+
}
129+
122130
// ========== Token-level fallback for Tier 2 ==========
123131

124132
TEST_F(MySQLDigestTest, TokenLevelInsert) {
@@ -225,6 +233,16 @@ static const DigestTestCase digest_bulk_cases[] = {
225233
{"SELECT a FROM t WHERE id = 1", "SELECT b FROM t WHERE id = 1", false, "different columns"},
226234
{"SELECT * FROM t WHERE a = 1", "SELECT * FROM t WHERE b = 1", false, "different where cols"},
227235
{"SELECT * FROM t ORDER BY a", "SELECT * FROM t ORDER BY b", false, "different order"},
236+
// Keyword casing must not change the digest ...
237+
{"SELECT 1 UNION SELECT 2", "SELECT 1 union SELECT 2", true, "union keyword casing"},
238+
{"SELECT a FROM t WHERE a = 1 AND b = 2", "SELECT a FROM t WHERE a = 1 and b = 2", true, "AND keyword casing"},
239+
{"SELECT a FROM t ORDER BY a DESC", "SELECT a FROM t ORDER BY a desc", true, "DESC keyword casing"},
240+
{"SELECT DISTINCT a FROM t", "SELECT distinct a FROM t", true, "DISTINCT keyword casing"},
241+
{"SELECT a FROM t1 INNER JOIN t2 ON t1.a = t2.b", "SELECT a FROM t1 inner join t2 ON t1.a = t2.b", true, "join type casing"},
242+
{"SELECT COUNT(*) FROM t", "SELECT count(*) FROM t", true, "function name casing"},
243+
// ... but identifier casing must.
244+
{"SELECT a FROM MyTable", "SELECT a FROM mytable", false, "table name case is significant"},
245+
{"SELECT MyCol FROM t", "SELECT mycol FROM t", false, "column name case is significant"},
228246
};
229247

230248
TEST(MySQLDigestBulk, HashConsistency) {
@@ -278,6 +296,19 @@ class PgSQLDigestTest : public ::testing::Test {
278296
protected:
279297
Parser<Dialect::PostgreSQL> parser;
280298

299+
// AST-based digest (parses SQL, invalidates previous arena allocations)
300+
StableDigest digest_ast(const char* sql) {
301+
auto r = parser.parse(sql, strlen(sql));
302+
Digest<Dialect::PostgreSQL> digest(parser.arena());
303+
DigestResult dr;
304+
if (r.ast) {
305+
dr = digest.compute(r.ast);
306+
} else {
307+
dr = digest.compute(sql, strlen(sql));
308+
}
309+
return StableDigest{std::string(dr.normalized.ptr, dr.normalized.len), dr.hash};
310+
}
311+
281312
StableDigest digest_token(const char* sql) {
282313
parser.reset();
283314
Digest<Dialect::PostgreSQL> digest(parser.arena());
@@ -290,6 +321,24 @@ class PgSQLDigestTest : public ::testing::Test {
290321
}
291322
};
292323

324+
// ========== Function name canonicalization ==========
325+
326+
TEST_F(PgSQLDigestTest, UndelimitedFunctionNamesFold) {
327+
auto lower = digest_ast("SELECT myfunc(a) FROM t");
328+
auto upper = digest_ast("SELECT MYFUNC(a) FROM t");
329+
EXPECT_EQ(lower.normalized, "SELECT myfunc(a) FROM t");
330+
EXPECT_EQ(upper.normalized, "SELECT myfunc(a) FROM t");
331+
EXPECT_EQ(lower.hash, upper.hash);
332+
}
333+
334+
TEST_F(PgSQLDigestTest, DelimitedFunctionNameKeepsItsOwnSpelling) {
335+
// PostgreSQL folds undelimited names down, so "MYFUNC" is a different function.
336+
auto undelimited = digest_ast("SELECT myfunc(a) FROM t");
337+
auto delimited = digest_ast("SELECT \"MYFUNC\"(a) FROM t");
338+
EXPECT_EQ(delimited.normalized, "SELECT MYFUNC(a) FROM t");
339+
EXPECT_NE(undelimited.hash, delimited.hash);
340+
}
341+
293342
TEST_F(PgSQLDigestTest, BasicDigest) {
294343
EXPECT_EQ(normalized_token("SELECT * FROM users WHERE id = 42"),
295344
"SELECT * FROM users WHERE id = ?");

0 commit comments

Comments
 (0)