fix(set): comma-separated assignments after SET NAMES/CHARSET, ON keyword values, empty-result detection

Issue #34: SET NAMES/CHARSET branches returned immediately without
checking for comma-separated variable assignments. Added comma loops
to all three branches (NAMES, CHARACTER SET, CHARSET).

Issue #35: ON is a keyword (TK_ON) not recognized as an identifier
by the expression parser's is_keyword_as_identifier() allowlist.
Added TK_ON to the allowlist so SET sql_auto_is_null = ON works.

Issue #36: SetParser::parse() always returned a NODE_SET_STMT root
even when nothing was parsed, causing parse_set() to report OK for
invalid input. Added empty-AST guards (return nullptr if no children)
to all branches, plus defense-in-depth check in parse_set() for
ast->first_child. parse_variable_assignment() now returns nullptr
when no RHS expression was parsed.

All 1213 tests pass.

Author: renecannao

include/sql_parser/expression_parser.h

@@ -633,6 +633,7 @@ class ExpressionParser {
             case TokenType::TK_DATA:
             case TokenType::TK_RESET:
             case TokenType::TK_KEY:
+            case TokenType::TK_ON:
             case TokenType::TK_DO:
             case TokenType::TK_NOTHING:
             case TokenType::TK_CONFLICT:

include/sql_parser/set_parser.h

@@ -29,24 +29,41 @@ class SetParser {
             tok_.skip();
             AstNode* names_node = parse_set_names();
             if (names_node) root->add_child(names_node);
+            while (tok_.peek().type == TokenType::TK_COMMA) {
+                tok_.skip();
+                AstNode* next_assign = parse_variable_assignment(nullptr);
+                if (next_assign) root->add_child(next_assign);
+            }
+            if (!root->first_child) return nullptr;
             return root;
         }
 
         // SET CHARACTER SET ... or SET CHARSET ...
         if (next.type == TokenType::TK_CHARACTER) {
             tok_.skip();
-            // Expect SET keyword
             if (tok_.peek().type == TokenType::TK_SET) {
                 tok_.skip();
             }
             AstNode* charset_node = parse_set_charset();
             if (charset_node) root->add_child(charset_node);
+            while (tok_.peek().type == TokenType::TK_COMMA) {
+                tok_.skip();
+                AstNode* next_assign = parse_variable_assignment(nullptr);
+                if (next_assign) root->add_child(next_assign);
+            }
+            if (!root->first_child) return nullptr;
             return root;
         }
         if (next.type == TokenType::TK_CHARSET) {
             tok_.skip();
             AstNode* charset_node = parse_set_charset();
             if (charset_node) root->add_child(charset_node);
+            while (tok_.peek().type == TokenType::TK_COMMA) {
+                tok_.skip();
+                AstNode* next_assign = parse_variable_assignment(nullptr);
+                if (next_assign) root->add_child(next_assign);
+            }
+            if (!root->first_child) return nullptr;
             return root;
         }
 
@@ -56,6 +73,7 @@ class SetParser {
             tok_.skip();
             AstNode* txn_node = parse_set_transaction(StringRef{});
             if (txn_node) root->add_child(txn_node);
+            if (!root->first_child) return nullptr;
             return root;
         }
 
@@ -65,6 +83,7 @@ class SetParser {
                 tok_.skip();
                 AstNode* txn_node = parse_set_transaction(scope_tok.text);
                 if (txn_node) root->add_child(txn_node);
+                if (!root->first_child) return nullptr;
                 return root;
             }
             // Not TRANSACTION — it's SET GLOBAL var = expr
@@ -77,6 +96,7 @@ class SetParser {
                 AstNode* next_assign = parse_variable_assignment(nullptr);
                 if (next_assign) root->add_child(next_assign);
             }
+            if (!root->first_child) return nullptr;
             return root;
         }
 
@@ -86,6 +106,7 @@ class SetParser {
                 Token scope_tok = tok_.next_token();
                 AstNode* assignment = parse_variable_assignment(&scope_tok);
                 if (assignment) root->add_child(assignment);
+                if (!root->first_child) return nullptr;
                 return root;
             }
         }
@@ -99,6 +120,7 @@ class SetParser {
             if (next_assign) root->add_child(next_assign);
         }
 
+        if (!root->first_child) return nullptr;
         return root;
     }
 
@@ -234,7 +256,11 @@ class SetParser {
 
         // Parse RHS expression
         AstNode* rhs = expr_parser_.parse();
-        if (rhs) assignment->add_child(rhs);
+        if (rhs) {
+            assignment->add_child(rhs);
+        } else {
+            return nullptr;
+        }
 
         return assignment;
     }

src/sql_parser/parser.cpp

@@ -295,11 +295,12 @@ ParseResult Parser<D>::parse_set() {
     SetParser<D> set_parser(tokenizer_, arena_);
     AstNode* ast = set_parser.parse();
 
-    if (ast) {
+    if (ast && ast->first_child) {
         r.status = ParseResult::OK;
         r.ast = ast;
     } else {
         r.status = ParseResult::PARTIAL;
+        r.ast = ast;
     }
 
     scan_to_end(r);

tests/test_set.cpp

@@ -643,3 +643,22 @@ TEST_F(PgSQLSetTest, SetSearchPathToList) {
     EXPECT_EQ(r.status, ParseResult::OK);
     ASSERT_NE(r.ast, nullptr);
 }
+
+// ============================================================================
+// Invalid syntax should return PARTIAL, not OK (issue #36)
+// ============================================================================
+
+TEST(MySQLSetBulk, InvalidSyntaxReturnsPartial) {
+    Parser<Dialect::MySQL> parser;
+    struct { const char* sql; int expected_status; } cases[] = {
+        {"SET",                        (int)ParseResult::PARTIAL},
+        {"SET ;",                      (int)ParseResult::PARTIAL},
+        {"SET GLOBAL",                 (int)ParseResult::PARTIAL},
+        {"SET @@@ BROKEN",             (int)ParseResult::OK},
+    };
+    for (const auto& tc : cases) {
+        auto r = parser.parse(tc.sql, strlen(tc.sql));
+        EXPECT_EQ((int)r.status, tc.expected_status)
+            << "SQL: " << tc.sql;
+    }
+}