Skip to content

fix: canonicalize keywords#66

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.

Sign up for GitHub

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

Open
Snehil-Shah wants to merge 1 commit into
base:main
Choose a base branch
from
Open

fix: canonicalize keywords #66

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Makefile
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -51,6 +51,7 @@ TEST_SRCS = $(TEST_DIR)/test_main.cpp \
$(TEST_DIR)/test_update.cpp \
$(TEST_DIR)/test_delete.cpp \
$(TEST_DIR)/test_compound.cpp \
$(TEST_DIR)/test_keyword_canonicalization.cpp \
$(TEST_DIR)/test_digest.cpp \
$(TEST_DIR)/test_misc_stmts.cpp \
$(TEST_DIR)/test_value.cpp \
Expand Down
4 changes: 4 additions & 0 deletions include/sql_parser/arena.h
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -30,6 +30,10 @@ class Arena {

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

StringRef allocate_upper(StringRef text);

StringRef allocate_lower(StringRef text);

void reset();

size_t bytes_used() const;
Expand Down
21 changes: 17 additions & 4 deletions include/sql_parser/compound_query_parser.h
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -73,7 +73,10 @@ class CompoundQueryParser {
AstNode* lock = make_node(arena_, NodeType::NODE_LOCKING_CLAUSE);
if (lock) {
Token strength = tok_.next_token();
lock->add_child(make_node(arena_, NodeType::NODE_IDENTIFIER, strength.text));
lock->add_child(make_node(arena_, NodeType::NODE_IDENTIFIER,
strength.type == TokenType::TK_UPDATE ? StringRef{"UPDATE", 6}
: strength.type == TokenType::TK_SHARE ? StringRef{"SHARE", 5}
: strength.text));
result->add_child(lock);
}
}
Expand DownExpand Up@@ -124,7 +127,11 @@ class CompoundQueryParser {

// Consume the set operator
tok_.skip();
StringRef op_text = t.text;
StringRef op_text =
t.type == TokenType::TK_UNION ? StringRef{"UNION", 5}
: t.type == TokenType::TK_INTERSECT ? StringRef{"INTERSECT", 9}
: t.type == TokenType::TK_EXCEPT ? StringRef{"EXCEPT", 6}
: t.text;

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

tok_.skip();
StringRef op_text = t.text;
StringRef op_text =
t.type == TokenType::TK_UNION ? StringRef{"UNION", 5}
: t.type == TokenType::TK_INTERSECT ? StringRef{"INTERSECT", 9}
: t.type == TokenType::TK_EXCEPT ? StringRef{"EXCEPT", 6}
: t.text;

uint16_t flags = 0;
if (tok_.peek().type == TokenType::TK_ALL) {
Expand DownExpand Up@@ -278,7 +289,9 @@ class CompoundQueryParser {
Token dir = tok_.peek();
if (dir.type == TokenType::TK_ASC || dir.type == TokenType::TK_DESC) {
tok_.skip();
item->add_child(make_node(arena_, NodeType::NODE_IDENTIFIER, dir.text));
item->add_child(make_node(arena_, NodeType::NODE_IDENTIFIER,
dir.type == TokenType::TK_ASC ? StringRef{"ASC", 3}
: StringRef{"DESC", 4}));
}

order_by->add_child(item);
Expand Down
4 changes: 3 additions & 1 deletion include/sql_parser/delete_parser.h
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -294,7 +294,9 @@ class DeleteParser {
Token dir = tok_.peek();
if (dir.type == TokenType::TK_ASC || dir.type == TokenType::TK_DESC) {
tok_.skip();
item->add_child(make_node(arena_, NodeType::NODE_IDENTIFIER, dir.text));
item->add_child(make_node(arena_, NodeType::NODE_IDENTIFIER,
dir.type == TokenType::TK_ASC ? StringRef{"ASC", 3}
: StringRef{"DESC", 4}));
}

order_by->add_child(item);
Expand Down
58 changes: 47 additions & 11 deletions include/sql_parser/expression_parser.h
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -41,6 +41,24 @@ using SubqueryParseCallback = AstNode*(*)(Tokenizer<D>&, Arena&);
template <Dialect D>
class ExpressionParser {
public:
// Keyword operators are stored under their canonical spelling.
static StringRef canonical_op(const Token& op) {
switch (op.type) {
case TokenType::TK_AND: return StringRef{"AND", 3};
case TokenType::TK_OR: return StringRef{"OR", 2};
case TokenType::TK_XOR: return StringRef{"XOR", 3};
case TokenType::TK_NOT: return StringRef{"NOT", 3};
case TokenType::TK_IS: return StringRef{"IS", 2};
case TokenType::TK_IN: return StringRef{"IN", 2};
case TokenType::TK_LIKE: return StringRef{"LIKE", 4};
case TokenType::TK_REGEXP: return StringRef{"REGEXP", 6};
case TokenType::TK_DIV: return StringRef{"DIV", 3};
case TokenType::TK_MOD: return StringRef{"MOD", 3};
case TokenType::TK_BETWEEN: return StringRef{"BETWEEN", 7};
default: return op.text;
}
}

ExpressionParser(Tokenizer<D>& tokenizer, Arena& arena)
: tok_(tokenizer), arena_(arena) {}

Expand DownExpand Up@@ -120,7 +138,14 @@ class ExpressionParser {
}
case TokenType::TK_NULL: {
tok_.skip();
return make_node_from_token(arena_, NodeType::NODE_LITERAL_NULL, t);
{
// Keep the source span lossless, but store the keyword under
// its canonical spelling.
AstNode* null_node =
make_node_from_token(arena_, NodeType::NODE_LITERAL_NULL, t);
if (null_node) null_node->set_value(StringRef{"NULL", 4});
return null_node;
}
}
case TokenType::TK_TRUE:
case TokenType::TK_FALSE: {
Expand DownExpand Up@@ -186,7 +211,7 @@ class ExpressionParser {
tok_.skip();
AstNode* operand = parse(Precedence::UNARY);
if (!operand) return nullptr;
AstNode* node = make_node(arena_, NodeType::NODE_UNARY_OP, t.text);
AstNode* node = make_node(arena_, NodeType::NODE_UNARY_OP, canonical_op(t));
set_span_through_node_(node, t.source, operand);
node->add_child(operand);
return node;
Expand All@@ -196,7 +221,7 @@ class ExpressionParser {
tok_.skip();
AstNode* operand = parse(Precedence::UNARY);
if (!operand) return nullptr;
AstNode* node = make_node(arena_, NodeType::NODE_UNARY_OP, t.text);
AstNode* node = make_node(arena_, NodeType::NODE_UNARY_OP, canonical_op(t));
set_span_through_node_(node, t.source, operand);
node->add_child(operand);
return node;
Expand All@@ -205,7 +230,7 @@ class ExpressionParser {
tok_.skip();
AstNode* operand = parse(Precedence::NOT);
if (!operand) return nullptr;
AstNode* node = make_node(arena_, NodeType::NODE_UNARY_OP, t.text);
AstNode* node = make_node(arena_, NodeType::NODE_UNARY_OP, canonical_op(t));
set_span_through_node_(node, t.source, operand);
node->add_child(operand);
return node;
Expand DownExpand Up@@ -326,7 +351,16 @@ class ExpressionParser {
// Check for function call: name(
if (tok_.peek().type == TokenType::TK_LPAREN) {
tok_.skip(); // consume (
AstNode* func = make_node(arena_, NodeType::NODE_FUNCTION_CALL, name_token.text);
// Function names are case-insensitive, so store them under a canonical spelling.
StringRef func_name = name_token.text;
if constexpr (D == Dialect::MySQL) {
// MySQL folds all function names up:
func_name = arena_.allocate_upper(func_name);
} else if (!token_was_delimited_(name_token)) {
// PostgreSQL folds undelimited function names down:
func_name = arena_.allocate_lower(func_name);
}
AstNode* func = make_node(arena_, NodeType::NODE_FUNCTION_CALL, func_name);
// CAST uses `CAST(expr AS type)` rather than a comma-separated
// argument list. Model it as a function call so consumers can
// reject or handle the expression without leaving valid input
Expand DownExpand Up@@ -449,25 +483,25 @@ class ExpressionParser {
tok_.skip();
AstNode* in_node = parse_in(left);
// Wrap in NOT
AstNode* not_node = make_node(arena_, NodeType::NODE_UNARY_OP, op.text);
AstNode* not_node = make_node(arena_, NodeType::NODE_UNARY_OP, canonical_op(op));
not_node->add_child(in_node);
return not_node;
}
if (actual_op.type == TokenType::TK_BETWEEN) {
tok_.skip();
AstNode* between_node = parse_between(left);
AstNode* not_node = make_node(arena_, NodeType::NODE_UNARY_OP, op.text);
AstNode* not_node = make_node(arena_, NodeType::NODE_UNARY_OP, canonical_op(op));
not_node->add_child(between_node);
return not_node;
}
if (actual_op.type == TokenType::TK_LIKE ||
actual_op.type == TokenType::TK_REGEXP) {
tok_.skip();
AstNode* right = parse(prec);
AstNode* like_node = make_node(arena_, NodeType::NODE_BINARY_OP, actual_op.text);
AstNode* like_node = make_node(arena_, NodeType::NODE_BINARY_OP, canonical_op(actual_op));
like_node->add_child(left);
if (right) like_node->add_child(right);
AstNode* not_node = make_node(arena_, NodeType::NODE_UNARY_OP, op.text);
AstNode* not_node = make_node(arena_, NodeType::NODE_UNARY_OP, canonical_op(op));
not_node->add_child(like_node);
return not_node;
}
Expand DownExpand Up@@ -511,7 +545,7 @@ class ExpressionParser {
// Standard binary operator
AstNode* right = parse(prec);
if (!right) return left;
AstNode* node = make_node(arena_, NodeType::NODE_BINARY_OP, op.text);
AstNode* node = make_node(arena_, NodeType::NODE_BINARY_OP, canonical_op(op));
node->add_child(left);
node->add_child(right);
return node;
Expand DownExpand Up@@ -758,7 +792,9 @@ class ExpressionParser {
Token dir = tok_.peek();
if (dir.type == TokenType::TK_ASC || dir.type == TokenType::TK_DESC) {
tok_.skip();
item->add_child(make_node(arena_, NodeType::NODE_IDENTIFIER, dir.text));
item->add_child(make_node(arena_, NodeType::NODE_IDENTIFIER,
dir.type == TokenType::TK_ASC ? StringRef{"ASC", 3}
: StringRef{"DESC", 4}));
}
ord->add_child(item);
if (tok_.peek().type == TokenType::TK_COMMA) tok_.skip();
Expand Down
16 changes: 12 additions & 4 deletions include/sql_parser/select_parser.h
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -123,11 +123,14 @@ class SelectParser {
if (t.type == TokenType::TK_DISTINCT || t.type == TokenType::TK_ALL) {
if (!opts) opts = make_node(arena_, NodeType::NODE_SELECT_OPTIONS);
tok_.skip();
opts->add_child(make_node(arena_, NodeType::NODE_IDENTIFIER, t.text));
opts->add_child(make_node(arena_, NodeType::NODE_IDENTIFIER,
t.type == TokenType::TK_DISTINCT ? StringRef{"DISTINCT", 8}
: StringRef{"ALL", 3}));
} else if (t.type == TokenType::TK_SQL_CALC_FOUND_ROWS) {
if (!opts) opts = make_node(arena_, NodeType::NODE_SELECT_OPTIONS);
tok_.skip();
opts->add_child(make_node(arena_, NodeType::NODE_IDENTIFIER, t.text));
opts->add_child(make_node(arena_, NodeType::NODE_IDENTIFIER,
StringRef{"SQL_CALC_FOUND_ROWS", 19}));
} else {
break;
}
Expand DownExpand Up@@ -298,7 +301,9 @@ class SelectParser {
Token dir = tok_.peek();
if (dir.type == TokenType::TK_ASC || dir.type == TokenType::TK_DESC) {
tok_.skip();
item->add_child(make_node(arena_, NodeType::NODE_IDENTIFIER, dir.text));
item->add_child(make_node(arena_, NodeType::NODE_IDENTIFIER,
dir.type == TokenType::TK_ASC ? StringRef{"ASC", 3}
: StringRef{"DESC", 4}));
}

order_by->add_child(item);
Expand DownExpand Up@@ -349,7 +354,10 @@ class SelectParser {

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

// Optional: OF table_list
if (tok_.peek().type == TokenType::TK_OF) {
Expand Down
4 changes: 2 additions & 2 deletions include/sql_parser/table_ref_parser.h
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -135,8 +135,8 @@ class TableRefParser {
}

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

Expand Down
4 changes: 3 additions & 1 deletion include/sql_parser/update_parser.h
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -238,7 +238,9 @@ class UpdateParser {
Token dir = tok_.peek();
if (dir.type == TokenType::TK_ASC || dir.type == TokenType::TK_DESC) {
tok_.skip();
item->add_child(make_node(arena_, NodeType::NODE_IDENTIFIER, dir.text));
item->add_child(make_node(arena_, NodeType::NODE_IDENTIFIER,
dir.type == TokenType::TK_ASC ? StringRef{"ASC", 3}
: StringRef{"DESC", 4}));
}

order_by->add_child(item);
Expand Down
22 changes: 22 additions & 0 deletions src/sql_parser/arena.cpp
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -62,6 +62,28 @@ StringRef Arena::allocate_string(const char* src, uint32_t len) {
return StringRef{static_cast<const char*>(mem), len};
}

StringRef Arena::allocate_upper(StringRef text) {
if (!text.ptr || text.len == 0) return text;
char* buf = static_cast<char*>(allocate(text.len));
if (!buf) return text;
for (uint32_t i = 0; i < text.len; ++i) {
const char c = text.ptr[i];
buf[i] = (c >= 'a' && c <= 'z') ? static_cast<char>(c - 32) : c;
}
return StringRef{buf, text.len};
}

StringRef Arena::allocate_lower(StringRef text) {
if (!text.ptr || text.len == 0) return text;
char* buf = static_cast<char*>(allocate(text.len));
if (!buf) return text;
for (uint32_t i = 0; i < text.len; ++i) {
const char c = text.ptr[i];
buf[i] = (c >= 'A' && c <= 'Z') ? static_cast<char>(c + 32) : c;
}
return StringRef{buf, text.len};
}

void Arena::reset() {
Block* b = primary_->next;
while (b) {
Expand Down
49 changes: 49 additions & 0 deletions tests/test_digest.cpp
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -119,6 +119,14 @@ TEST_F(MySQLDigestTest, KeywordsUppercased) {
"SELECT * FROM t WHERE id = ?");
}

TEST_F(MySQLDigestTest, KeywordsUppercasedFromAst) {
EXPECT_EQ(normalized("select a from t where a = 1 and b = 2"),
"SELECT a FROM t WHERE a = ? AND b = ?");
EXPECT_EQ(normalized("select 1 union select 2"), "SELECT ? UNION SELECT ?");
EXPECT_EQ(normalized("select a from t order by a desc"),
"SELECT a FROM t ORDER BY a DESC");
}

// ========== Token-level fallback for Tier 2 ==========

TEST_F(MySQLDigestTest, TokenLevelInsert) {
Expand DownExpand Up@@ -225,6 +233,16 @@ static const DigestTestCase digest_bulk_cases[] = {
{"SELECT a FROM t WHERE id = 1", "SELECT b FROM t WHERE id = 1", false, "different columns"},
{"SELECT * FROM t WHERE a = 1", "SELECT * FROM t WHERE b = 1", false, "different where cols"},
{"SELECT * FROM t ORDER BY a", "SELECT * FROM t ORDER BY b", false, "different order"},
// Keyword casing must not change the digest ...
{"SELECT 1 UNION SELECT 2", "SELECT 1 union SELECT 2", true, "union keyword casing"},
{"SELECT a FROM t WHERE a = 1 AND b = 2", "SELECT a FROM t WHERE a = 1 and b = 2", true, "AND keyword casing"},
{"SELECT a FROM t ORDER BY a DESC", "SELECT a FROM t ORDER BY a desc", true, "DESC keyword casing"},
{"SELECT DISTINCT a FROM t", "SELECT distinct a FROM t", true, "DISTINCT keyword casing"},
{"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"},
{"SELECT COUNT(*) FROM t", "SELECT count(*) FROM t", true, "function name casing"},
// ... but identifier casing must.
{"SELECT a FROM MyTable", "SELECT a FROM mytable", false, "table name case is significant"},
{"SELECT MyCol FROM t", "SELECT mycol FROM t", false, "column name case is significant"},
};

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

// AST-based digest (parses SQL, invalidates previous arena allocations)
StableDigest digest_ast(const char* sql) {
auto r = parser.parse(sql, strlen(sql));
Digest<Dialect::PostgreSQL> digest(parser.arena());
DigestResult dr;
if (r.ast) {
dr = digest.compute(r.ast);
} else {
dr = digest.compute(sql, strlen(sql));
}
return StableDigest{std::string(dr.normalized.ptr, dr.normalized.len), dr.hash};
}

StableDigest digest_token(const char* sql) {
parser.reset();
Digest<Dialect::PostgreSQL> digest(parser.arena());
Expand All@@ -290,6 +321,24 @@ class PgSQLDigestTest : public ::testing::Test {
}
};

// ========== Function name canonicalization ==========

TEST_F(PgSQLDigestTest, UndelimitedFunctionNamesFold) {
auto lower = digest_ast("SELECT myfunc(a) FROM t");
auto upper = digest_ast("SELECT MYFUNC(a) FROM t");
EXPECT_EQ(lower.normalized, "SELECT myfunc(a) FROM t");
EXPECT_EQ(upper.normalized, "SELECT myfunc(a) FROM t");
EXPECT_EQ(lower.hash, upper.hash);
}

TEST_F(PgSQLDigestTest, DelimitedFunctionNameKeepsItsOwnSpelling) {
// PostgreSQL folds undelimited names down, so "MYFUNC" is a different function.
auto undelimited = digest_ast("SELECT myfunc(a) FROM t");
auto delimited = digest_ast("SELECT \"MYFUNC\"(a) FROM t");
EXPECT_EQ(delimited.normalized, "SELECT MYFUNC(a) FROM t");
EXPECT_NE(undelimited.hash, delimited.hash);
}

TEST_F(PgSQLDigestTest, BasicDigest) {
EXPECT_EQ(normalized_token("SELECT * FROM users WHERE id = 42"),
"SELECT * FROM users WHERE id = ?");
Expand Down
Loading