diff --git a/src/parser/parser_select.cpp b/src/parser/parser_select.cpp index aa59201..1f0988c 100644 --- a/src/parser/parser_select.cpp +++ b/src/parser/parser_select.cpp @@ -425,45 +425,49 @@ ast::ASTNode* Parser::parse_select_stmt() { } } - // Parse ORDER BY clause - if (current_token_ && current_token_->type == tokenizer::TokenType::Keyword && - (current_token_->keyword_id == db25::Keyword::ORDER)) { - advance(); // consume ORDER - if (current_token_ && current_token_->keyword_id == db25::Keyword::BY) { - advance(); // consume BY - - auto* order_by = parse_order_by_clause(); - if (order_by) { - order_by->parent = select_node; - // Add to the end of child list - auto* last_child = select_node->first_child; - while (last_child->next_sibling) { - last_child = last_child->next_sibling; + // Parse a trailing ORDER BY and/or LIMIT and append them to `target`. Shared + // by the plain-SELECT path (here) and the set-operation path below: a + // trailing ORDER BY / LIMIT after `A UNION B` applies to the WHOLE result, so + // it must attach to the set-op node, not to B. + auto attach_order_by_limit = [&](ast::ASTNode* target) { + const auto append = [&](ast::ASTNode* clause) { + clause->parent = target; + auto* last_child = target->first_child; + while (last_child->next_sibling) { + last_child = last_child->next_sibling; + } + last_child->next_sibling = clause; + target->child_count++; + }; + if (current_token_ && current_token_->type == tokenizer::TokenType::Keyword && + (current_token_->keyword_id == db25::Keyword::ORDER)) { + advance(); // consume ORDER + if (current_token_ && current_token_->keyword_id == db25::Keyword::BY) { + advance(); // consume BY + if (auto* order_by = parse_order_by_clause()) { + append(order_by); } - last_child->next_sibling = order_by; - select_node->child_count++; } } - } - - // Parse LIMIT clause - if (current_token_ && current_token_->type == tokenizer::TokenType::Keyword && - (current_token_->keyword_id == db25::Keyword::LIMIT)) { - advance(); // consume LIMIT - - auto* limit_clause = parse_limit_clause(); - if (limit_clause) { - limit_clause->parent = select_node; - // Add to the end of child list - auto* last_child = select_node->first_child; - while (last_child->next_sibling) { - last_child = last_child->next_sibling; + if (current_token_ && current_token_->type == tokenizer::TokenType::Keyword && + (current_token_->keyword_id == db25::Keyword::LIMIT)) { + advance(); // consume LIMIT + if (auto* limit_clause = parse_limit_clause()) { + append(limit_clause); } - last_child->next_sibling = limit_clause; - select_node->child_count++; } + }; + + // A bare set-operation operand (the right branch, and the left before folding) + // must NOT swallow a trailing ORDER BY / LIMIT: those bind to the whole set + // operation and are parsed once, after folding, below. `SELECT 1 UNION SELECT + // 2 ORDER BY 1` previously let the right branch eat `ORDER BY 1`, then failed + // validation. A parenthesized operand keeps its own ORDER BY (it re-enters + // this function outside setop-rhs mode), matching SQL scoping. + if (!is_setop_rhs) { + attach_order_by_limit(select_node); } - + // Set operations (UNION / INTERSECT / EXCEPT / MINUS) fold LEFT- // associatively: `A op B op C` parses as `(A op B) op C`. If this invocation // is itself the right-hand branch of an enclosing set operation, return the @@ -487,8 +491,32 @@ ast::ASTNode* Parser::parse_select_stmt() { // in_setop_rhs_ guard so the nested call returns its branch without folding // its own set-op tail (this loop owns the folding). - // Parse one bare operand: a SELECT with no set-op tail of its own. + // Parse one operand: a bare SELECT with no set-op tail of its own, OR a + // parenthesized query block `( )` (which may itself be a set + // operation, and keeps its own ORDER BY / LIMIT). `SELECT 1 UNION (SELECT 2)` + // previously dropped the parenthesized branch entirely. auto parse_setop_operand = [&]() -> ast::ASTNode* { + if (current_token_ && current_token_->type == tokenizer::TokenType::Delimiter && + current_token_->value == "(") { + parenthesis_depth_++; + advance(); // consume '(' + const bool saved_rhs = in_setop_rhs_; + in_setop_rhs_ = false; // inside the parens it is a complete query block + ast::ASTNode* inner = + (current_token_ && current_token_->type == tokenizer::TokenType::Keyword && + current_token_->keyword_id == db25::Keyword::WITH) + ? parse_with_statement() + : parse_select_stmt(); + in_setop_rhs_ = saved_rhs; + if (current_token_ && current_token_->value == ")") { + if (parenthesis_depth_ > 0) parenthesis_depth_--; + advance(); // consume ')' + } else { + error("expected ')' after parenthesized set-operation operand"); + return nullptr; + } + return inner; + } if (current_token_ && current_token_->type == tokenizer::TokenType::Keyword && (current_token_->keyword_id == db25::Keyword::SELECT)) { in_setop_rhs_ = true; @@ -577,7 +605,13 @@ ast::ASTNode* Parser::parse_select_stmt() { left = make_setop_node(keyword, set_op_type, left, right, all); } select_node = left; - + + // A trailing ORDER BY / LIMIT after the whole set operation binds to the + // combined result, so it attaches to the (folded) set-op node here. For a + // plain SELECT (no operator folded) the clauses were already consumed above, + // so this is a no-op. + attach_order_by_limit(select_node); + // Consume any trailing semicolon if (current_token_ && current_token_->type == tokenizer::TokenType::Delimiter && current_token_->value == ";") { diff --git a/tests/test_precedence_regression.cpp b/tests/test_precedence_regression.cpp index 3a444d7..1dc2521 100644 --- a/tests/test_precedence_regression.cpp +++ b/tests/test_precedence_regression.cpp @@ -211,3 +211,54 @@ TEST_F(PrecedenceRegressionTest, GroupByMissingItemNoCrash) { // No GroupByClause child should have been attached (the item failed to parse). EXPECT_EQ(find(ast, NodeType::GroupByClause), nullptr); } + +// ---- Set-operation operand / trailing-clause fixes ------------------------ + +// A parenthesized right operand of a set operation must be kept, not dropped: +// `SELECT 1 UNION (SELECT 2)` is a UNION over two SELECT branches. +TEST_F(PrecedenceRegressionTest, SetOpParenthesizedRightOperand) { + auto* root = parse("SELECT 1 UNION (SELECT 2)"); + ASSERT_NE(root, nullptr); + EXPECT_EQ(root->node_type, NodeType::UnionStmt); + int selects = 0; + for (auto* c = root->first_child; c; c = c->next_sibling) { + if (c->node_type == NodeType::SelectStmt) ++selects; + } + EXPECT_EQ(selects, 2) << "both UNION branches must survive"; +} + +// A trailing ORDER BY after a set operation binds to the whole result: it is a +// direct child of the set-op node, not swallowed by the right branch (which +// previously made `... UNION SELECT 2 ORDER BY 1` fail to parse). +TEST_F(PrecedenceRegressionTest, SetOpTrailingOrderByBindsToWhole) { + auto* root = parse("SELECT a FROM t UNION SELECT a FROM u ORDER BY 1"); + ASSERT_NE(root, nullptr); + EXPECT_EQ(root->node_type, NodeType::UnionStmt); + ASTNode* order_by = nullptr; + for (auto* c = root->first_child; c; c = c->next_sibling) { + if (c->node_type == NodeType::OrderByClause) order_by = c; + } + ASSERT_NE(order_by, nullptr) << "ORDER BY must attach to the UNION node"; +} + +TEST_F(PrecedenceRegressionTest, SetOpTrailingOrderByAndLimitBindToWhole) { + auto* root = parse("SELECT a FROM t UNION SELECT a FROM u ORDER BY 1 LIMIT 5"); + ASSERT_NE(root, nullptr); + EXPECT_EQ(root->node_type, NodeType::UnionStmt); + bool has_order = false, has_limit = false; + for (auto* c = root->first_child; c; c = c->next_sibling) { + if (c->node_type == NodeType::OrderByClause) has_order = true; + if (c->node_type == NodeType::LimitClause) has_limit = true; + } + EXPECT_TRUE(has_order); + EXPECT_TRUE(has_limit); +} + +// Regression guard: a plain SELECT still attaches its own ORDER BY / LIMIT. +TEST_F(PrecedenceRegressionTest, PlainSelectStillAttachesOrderByLimit) { + auto* root = parse("SELECT a FROM t ORDER BY a LIMIT 3"); + ASSERT_NE(root, nullptr); + EXPECT_EQ(root->node_type, NodeType::SelectStmt); + EXPECT_NE(find(root, NodeType::OrderByClause), nullptr); + EXPECT_NE(find(root, NodeType::LimitClause), nullptr); +}