diff --git a/README.md b/README.md index 14c701a..eaef6db 100644 --- a/README.md +++ b/README.md @@ -156,7 +156,9 @@ On DuckDB 1.5+, queries containing `AGGREGATE()` are automatically intercepted b On DuckDB builds with grammar-extension support, Yardstick recognizes `AS MEASURE`, `AT (...)`, and `CURRENT dimension` / `CURRENT(dimension)` through native PEG grammar rules. `LOAD yardstick` enables this adapter without setting `active_grammar_extensions`. Native references retain their expression spans and local relation scope, including quoted identifiers and CTE shadowing. `CURRENT` belongs to an AT SET or WHERE expression; ordinary SQL aliases and nested queries keep their own scope. Measure registration and context semantics remain shared with DuckDB 1.5.5. -Native query traversal lowers CTE bodies, subqueries, and set-operation operands independently. Aggregate calls are discovered from expression nodes, and visible filters retain outer query correlations. Subquery projections group their outer column dependencies, while implicit measure projections preserve their column names. DuckDB 1.5.5 retains the compatibility parser; native forms whose source spans cannot be represented also use that path. +Native query traversal lowers CTE bodies, subqueries, and set-operation operands independently. Aggregate calls are discovered from expression nodes, including parenthesized `AT` operands and queries inside INSERT, UPDATE, DELETE, CREATE VIEW, CREATE TABLE AS, EXPLAIN, and COPY statements. Visible filters retain outer query correlations. Subquery projections group their outer column dependencies, while implicit measure projections preserve their column names. DuckDB 1.5.5 retains the compatibility parser; native forms whose source spans cannot be represented also use that path. + +The native frontend rejects `DISTINCT`, `FILTER`, `ORDER BY`, `OVER`, and `EXPORT_STATE` directly on one-argument `AGGREGATE()` calls. Their semantics are not implemented, and compatibility lowering could produce incorrect results. Define aggregation behavior in the `AS MEASURE` expression or use supported `AT` modifiers instead. DuckDB's multiargument `aggregate(list, function_name)` remains ordinary DuckDB syntax. The native frontend also supports full and partial `CREATE VIEW` column lists for measure views. Header names apply to both dimensions and measures, while derived measures retain their declaration dependencies. Star projections are expanded against the originating session before header positions are mapped. Temporary definitions stay session-local; a single statement cannot combine temporary and permanent measure views with the same name. diff --git a/src/frontend_peg.cpp b/src/frontend_peg.cpp index 45ef2db..e94668c 100644 --- a/src/frontend_peg.cpp +++ b/src/frontend_peg.cpp @@ -1,6 +1,7 @@ #include "frontend_peg.hpp" #if YARDSTICK_GRAMMAR_EXTENSION +#include "native_statement_traversal.hpp" #include "duckdb/main/client_context.hpp" #include "duckdb/main/database.hpp" #include "duckdb/parser/expression/function_expression.hpp" @@ -57,6 +58,7 @@ struct SyntaxCapture { bool has_measure = false; const string *source = nullptr; vector clauses; + vector> modifier_expressions; struct Measure { string expression; string name; @@ -208,7 +210,12 @@ ParseResult &UnwrapModifier(ParseResult &result) { string RenderExpression(PEGTransformer &transformer, ParseResult &result) { // Resolve syntax supplied by every active grammar before shared lowering. - return transformer.Transform>(result)->ToString(); + auto expression = transformer.Transform>(result); + auto sql = expression->ToString(); + // AT markers retain only their operand in the main AST. Keep modifier + // expressions alive for validation before rendering loses their structure. + active_capture->modifier_expressions.push_back(std::move(expression)); + return sql; } string DimensionSource(ParseResult &result) { @@ -964,6 +971,7 @@ YardstickAggregateCallList *FindNativeYardstickAggregates(const char *sql_p) { if (!sql_p || !active_parse_scope || !active_parse_scope->available) { return nullptr; } + string semantic_error; try { string sql(sql_p); if (sql.size() > std::numeric_limits::max() || Parser::NormalizeSQLString(sql) != sql) { @@ -979,6 +987,41 @@ YardstickAggregateCallList *FindNativeYardstickAggregates(const char *sql_p) { return nullptr; } + // Validate every expression before source-span adaptation can choose + // compatibility lowering for a different operand in the same query. + std::function &)> validate_expression; + validate_expression = [&](unique_ptr &expression) { + bool unsupported = false; + if (expression->GetExpressionClass() == ExpressionClass::WINDOW) { + auto &window = expression->Cast(); + unsupported = StringUtil::CIEquals(window.FunctionName().GetIdentifierName(), "aggregate") && + window.GetArguments().size() == 1; + } else if (expression->GetExpressionClass() == ExpressionClass::FUNCTION) { + auto &function = expression->Cast(); + unsupported = StringUtil::CIEquals(function.FunctionName().GetIdentifierName(), "aggregate") && + function.GetArguments().size() == 1 && + (function.Distinct() || function.Filter() || function.ExportState() || + (function.OrderBy() && !function.OrderBy()->orders.empty())); + } + if (unsupported) { + semantic_error = "Yardstick AGGREGATE does not support DISTINCT, FILTER, ORDER BY, OVER, or EXPORT_STATE; " + "define aggregation behavior in AS MEASURE or use AT modifiers"; + throw ParserException(semantic_error); + } + if (expression->GetExpressionClass() == ExpressionClass::SUBQUERY) { + auto &subquery = expression->Cast(); + ParsedExpressionIterator::EnumerateQueryNodeChildren(*subquery.SubqueryMutable()->node, + validate_expression); + } + ParsedExpressionIterator::EnumerateChildren(*expression, validate_expression); + }; + for (auto &statement : parser.statements) { + EnumerateNativeStatementExpressions(*statement, validate_expression); + } + for (auto &expression : capture.modifier_expressions) { + validate_expression(expression); + } + struct Aggregate { string measure; idx_t start; @@ -995,18 +1038,28 @@ YardstickAggregateCallList *FindNativeYardstickAggregates(const char *sql_p) { } return location; }; - auto only_trivia = [&](idx_t start, idx_t end) { - // Parentheses are erased from the operand AST, while the marker's - // own location covers only AT (...). Validate that extending the - // function span does not consume an unmatched closing parenthesis. - // Tokens validate this source gap only; AST nodes discover calls. - auto gap = sql.substr(start, end - start); - vector tokens; - TokenizerBehavior behavior(gap, tokens); - active_parse_scope->ParserConfig().compiled_grammar->GetTokenizer().TokenizeInput(behavior); - return std::all_of(tokens.begin(), tokens.end(), [](const MatcherToken &token) { - return token.type == TokenType::COMMENT || token.type == TokenType::END_OF_INPUT; - }); + vector source_tokens; + TokenizerBehavior token_behavior(sql, source_tokens); + active_parse_scope->ParserConfig().compiled_grammar->GetTokenizer().TokenizeInput(token_behavior); + source_tokens.erase(std::remove_if(source_tokens.begin(), source_tokens.end(), [](const MatcherToken &token) { + return token.type == TokenType::COMMENT || token.type == TokenType::END_OF_INPUT; + }), source_tokens.end()); + auto extend_operand = [&](idx_t &start, idx_t end, idx_t suffix_start) { + // The AST erases grouping parentheses. Consume a closing parenthesis + // before AT only with its immediately enclosing opening parenthesis. + // Calls and suffix parentage still come exclusively from the AST. + auto first = std::lower_bound(source_tokens.begin(), source_tokens.end(), start, + [](const MatcherToken &token, idx_t offset) { return token.offset < offset; }); + auto next = std::lower_bound(source_tokens.begin(), source_tokens.end(), end, + [](const MatcherToken &token, idx_t offset) { return token.offset < offset; }); + while (next != source_tokens.end() && next->offset < suffix_start) { + if (next->text != ")" || first == source_tokens.begin() || (first - 1)->text != "(") { + return false; + } + start = (--first)->offset; + ++next; + } + return next != source_tokens.end() && next->offset == suffix_start; }; std::function &)> visit_expression; visit_expression = [&](unique_ptr &expression) { @@ -1032,21 +1085,11 @@ YardstickAggregateCallList *FindNativeYardstickAggregates(const char *sql_p) { } bool is_measure_call = false; - if (base->GetExpressionClass() == ExpressionClass::WINDOW && - StringUtil::CIEquals(base->Cast().FunctionName().GetIdentifierName(), "aggregate")) { - throw ParserException("Windowed Yardstick aggregate requires compatibility lowering"); - } if (base->GetExpressionClass() == ExpressionClass::FUNCTION) { auto &function = base->Cast(); is_measure_call = StringUtil::CIEquals(function.FunctionName().GetIdentifierName(), "aggregate") && function.GetArguments().size() == 1; if (is_measure_call) { - if (function.Distinct() || function.Filter() || function.ExportState() || - (function.OrderBy() && !function.OrderBy()->orders.empty())) { - // These decorations are not represented in this ABI. - // Extending the call span must not silently discard them. - throw ParserException("Decorated Yardstick aggregate requires compatibility lowering"); - } auto call_location = source_range(function); auto &argument = function.GetArguments()[0]; auto argument_location = source_range(argument.GetExpression()); @@ -1061,7 +1104,7 @@ YardstickAggregateCallList *FindNativeYardstickAggregates(const char *sql_p) { for (auto suffix = suffixes.rbegin(); suffix != suffixes.rend(); ++suffix) { auto &clause = capture.clauses[*suffix]; if (clause.start < aggregate.end || clause.end > sql.size() || - !only_trivia(aggregate.end, clause.start)) { + !extend_operand(aggregate.start, aggregate.end, clause.start)) { throw ParserException("Invalid Yardstick AT source range"); } aggregate.modifiers.insert(aggregate.modifiers.end(), clause.modifiers.begin(), @@ -1085,13 +1128,9 @@ YardstickAggregateCallList *FindNativeYardstickAggregates(const char *sql_p) { ParsedExpressionIterator::EnumerateChildren(*base, visit_expression); }; for (auto &statement : parser.statements) { - if (statement->type != StatementType::SELECT_STATEMENT) { - // Do not report a successful partial traversal of other statement - // kinds. Their existing compatibility path remains available. + if (!EnumerateNativeStatementExpressions(*statement, visit_expression)) { return nullptr; } - ParsedExpressionIterator::EnumerateQueryNodeChildren(*statement->Cast().node, - visit_expression); } std::sort(aggregates.begin(), aggregates.end(), [](const Aggregate &left, const Aggregate &right) { return left.start < right.start; @@ -1147,6 +1186,12 @@ YardstickAggregateCallList *FindNativeYardstickAggregates(const char *sql_p) { } return result.release(); } catch (const std::exception &) { + if (!semantic_error.empty()) { + auto *result = new YardstickAggregateCallList {}; + result->native_parsed = true; + result->error = strdup(semantic_error.c_str()); + return result; + } return nullptr; } } diff --git a/src/include/frontend_peg.hpp b/src/include/frontend_peg.hpp index cd61fa8..c6ab433 100644 --- a/src/include/frontend_peg.hpp +++ b/src/include/frontend_peg.hpp @@ -50,8 +50,9 @@ ClientContext *CurrentNativeYardstickClientContext(); // Parse through the active grammar while retaining Yardstick syntax capture. bool ParseNativeYardstickQuery(const string &sql, Parser &parser); -// Returns a complete native result, freed with yardstick_free_aggregate_list, -// or nullptr when no native scope is available or the syntax is unsupported. +// Returns a complete native result or recognized semantic error, freed with +// yardstick_free_aggregate_list. nullptr retains compatibility for unavailable +// native grammar or source forms outside the native adapter. YardstickAggregateCallList *FindNativeYardstickAggregates(const char *sql); // Source-preserving measure declarations. nullptr retains the legacy parser. diff --git a/src/include/native_statement_traversal.hpp b/src/include/native_statement_traversal.hpp new file mode 100644 index 0000000..d7bf42c --- /dev/null +++ b/src/include/native_statement_traversal.hpp @@ -0,0 +1,85 @@ +#pragma once + +#include "duckdb/parser/parsed_expression_iterator.hpp" +#include "duckdb/parser/parsed_data/create_table_info.hpp" +#include "duckdb/parser/parsed_data/create_view_info.hpp" +#include "duckdb/parser/query_node/update_query_node.hpp" +#include "duckdb/parser/statement/copy_statement.hpp" +#include "duckdb/parser/statement/create_statement.hpp" +#include "duckdb/parser/statement/delete_statement.hpp" +#include "duckdb/parser/statement/explain_statement.hpp" +#include "duckdb/parser/statement/insert_statement.hpp" +#include "duckdb/parser/statement/select_statement.hpp" +#include "duckdb/parser/statement/update_statement.hpp" + +namespace duckdb { + +// The callback owns expression recursion, including scalar subqueries. Return +// false for unsupported wrappers so discovery never accepts a partial result. +inline bool EnumerateNativeStatementExpressions( + SQLStatement &statement, const std::function &)> &callback) { + switch (statement.type) { + case StatementType::SELECT_STATEMENT: + ParsedExpressionIterator::EnumerateQueryNodeChildren(*statement.Cast().node, callback); + return true; + case StatementType::INSERT_STATEMENT: + ParsedExpressionIterator::EnumerateQueryNodeChildren(*statement.Cast().node, callback); + return true; + case StatementType::UPDATE_STATEMENT: + ParsedExpressionIterator::EnumerateQueryNodeChildren(*statement.Cast().node, callback); + return true; + case StatementType::DELETE_STATEMENT: + ParsedExpressionIterator::EnumerateQueryNodeChildren(*statement.Cast().node, callback); + return true; + case StatementType::EXPLAIN_STATEMENT: + return EnumerateNativeStatementExpressions(*statement.Cast().stmt, callback); + case StatementType::CREATE_STATEMENT: { + auto &info = *statement.Cast().info; + if (info.type == CatalogType::VIEW_ENTRY) { + auto &view = info.Cast(); + return view.query && EnumerateNativeStatementExpressions(*view.query, callback); + } + if (info.type != CatalogType::TABLE_ENTRY) { + return false; + } + auto &table = info.Cast(); + // Defaults, generated columns and CHECK constraints have separate AST + // owners. Keep those forms on compatibility lowering until traversed. + if (!table.query || !table.columns.empty() || !table.constraints.empty()) { + return false; + } + EnumerateNativeStatementExpressions(*table.query, callback); + for (auto &expression : table.partition_keys) { + callback(expression); + } + for (auto &expression : table.sort_keys) { + callback(expression); + } + for (auto &option : table.options) { + if (option.second) { + callback(option.second); + } + } + return true; + } + case StatementType::COPY_STATEMENT: { + auto © = *statement.Cast().info; + if (copy.select_statement) { + ParsedExpressionIterator::EnumerateQueryNodeChildren(*copy.select_statement, callback); + } + if (copy.file_path_expression) { + callback(copy.file_path_expression); + } + for (auto &option : copy.parsed_options) { + if (option.second) { + callback(option.second); + } + } + return true; + } + default: + return false; + } +} + +} // namespace duckdb diff --git a/test/sql/native_aggregate_detection.test b/test/sql/native_aggregate_detection.test index eaa4971..f81b05f 100644 --- a/test/sql/native_aggregate_detection.test +++ b/test/sql/native_aggregate_detection.test @@ -41,7 +41,25 @@ FROM aggregate_detection_v; ---- 30.0 -# CTEs, FROM subqueries and expression subqueries all contain query nodes. +# Grouping parentheses belong to the AT operand, including chained modifiers. +query TR +SELECT '東京', ((/* before */ AGGREGATE(revenue) /* after */)) AT (WHERE year = 2023) +FROM aggregate_detection_v; +---- +東京 30.0 + +query RR +SELECT (AGGREGATE(revenue) AT (WHERE year = 2023)) AT (ALL), + 2 * ((AGGREGATE(revenue))) AT (WHERE year = 2024) +FROM aggregate_detection_v; +---- +70.0 80.0 + +query R +SELECT aggregate([1, ((AGGREGATE(revenue))) AT (ALL)], 'sum') FROM aggregate_detection_v; +---- +71.0 + query R WITH totals AS (SELECT AGGREGATE(revenue) AS total FROM aggregate_detection_v) SELECT total FROM totals; diff --git a/test/sql/native_aggregate_errors.test b/test/sql/native_aggregate_errors.test new file mode 100644 index 0000000..fa54741 --- /dev/null +++ b/test/sql/native_aggregate_errors.test @@ -0,0 +1,171 @@ +# name: test/sql/native_aggregate_errors.test +# description: Unsupported measure-call decorations fail instead of losing semantics in compatibility lowering. +# group: [yardstick] + +require-env YARDSTICK_NATIVE_PEG 1 + +require yardstick + +statement ok +CREATE TABLE aggregate_error_sales(region VARCHAR, amount INTEGER); + +statement ok +INSERT INTO aggregate_error_sales VALUES ('a', 10), ('a', 10), ('b', 30); + +statement ok +CREATE VIEW aggregate_error_v AS +SELECT region, amount, SUM(amount) AS MEASURE revenue, + SUM(amount) FILTER (WHERE amount > 10) AS MEASURE large_revenue +FROM aggregate_error_sales; + +statement ok +CREATE TABLE aggregate_error_results(total INTEGER); + +foreach parser_mode true false + +statement ok +SET heap_based_parser=${parser_mode}; + +statement error +SELECT AGGREGATE(DISTINCT revenue) FROM aggregate_error_v; +---- +Yardstick AGGREGATE does not support + +statement error +SELECT AGGREGATE(revenue ORDER BY amount) FROM aggregate_error_v; +---- +Yardstick AGGREGATE does not support + +statement error +SELECT AGGREGATE(revenue) FILTER (WHERE amount > 10) FROM aggregate_error_v; +---- +Yardstick AGGREGATE does not support + +statement error +SELECT AGGREGATE(revenue) OVER () FROM aggregate_error_v; +---- +Yardstick AGGREGATE does not support + +statement error +SELECT AGGREGATE(revenue) OVER (PARTITION BY region) FROM aggregate_error_v; +---- +Yardstick AGGREGATE does not support + +statement error +SELECT AGGREGATE(revenue) EXPORT_STATE FROM aggregate_error_v; +---- +Yardstick AGGREGATE does not support + +# Trivia, nested scopes, built-in calls, and AT must not hide the error. +statement error +SELECT "AGGREGATE"/* comment */(revenue) FILTER (WHERE FALSE) FROM aggregate_error_v; +---- +Yardstick AGGREGATE does not support + +statement error +SELECT aggregate([1, 2], 'sum'), AGGREGATE(revenue), + AGGREGATE(revenue) FILTER (WHERE FALSE) FROM aggregate_error_v; +---- +Yardstick AGGREGATE does not support + +statement error +SELECT revenue AT (ALL), AGGREGATE(revenue) FILTER (WHERE FALSE) FROM aggregate_error_v; +---- +Yardstick AGGREGATE does not support + +statement error +SELECT AGGREGATE(revenue) FILTER (WHERE FALSE), revenue AT (ALL) FROM aggregate_error_v; +---- +Yardstick AGGREGATE does not support + +statement error +SELECT (SELECT AGGREGATE(revenue) FILTER (WHERE FALSE) FROM aggregate_error_v); +---- +Yardstick AGGREGATE does not support + +statement error +WITH totals AS (SELECT AGGREGATE(revenue) OVER (PARTITION BY region) AS total FROM aggregate_error_v) +SELECT * FROM totals; +---- +Yardstick AGGREGATE does not support + +statement error +SELECT AGGREGATE(revenue) FILTER (WHERE FALSE) FROM aggregate_error_v +UNION ALL SELECT AGGREGATE(revenue) FROM aggregate_error_v; +---- +Yardstick AGGREGATE does not support + +statement error +SELECT AGGREGATE(DISTINCT revenue) AT (ALL) FROM aggregate_error_v; +---- +Yardstick AGGREGATE does not support + +statement error +SELECT AGGREGATE(revenue ORDER BY amount) AT (ALL) FROM aggregate_error_v; +---- +Yardstick AGGREGATE does not support + +statement error +SELECT AGGREGATE(revenue) FILTER (WHERE FALSE) AT (ALL) FROM aggregate_error_v; +---- +Yardstick AGGREGATE does not support + +statement error +SELECT AGGREGATE(revenue) OVER () AT (ALL) FROM aggregate_error_v; +---- +Yardstick AGGREGATE does not support + +statement error +INSERT INTO aggregate_error_results +SELECT AGGREGATE(revenue) FILTER (WHERE FALSE) FROM aggregate_error_v; +---- +Yardstick AGGREGATE does not support + +# Modifier subqueries retain their own expression validation. +statement error +SELECT AGGREGATE(revenue) AT (WHERE amount = ( + SELECT AGGREGATE(revenue) FILTER (WHERE FALSE) FROM aggregate_error_v +)) FROM aggregate_error_v; +---- +Yardstick AGGREGATE does not support + +statement error +SELECT AGGREGATE(revenue) AT (SET amount = ( + SELECT AGGREGATE(revenue) OVER () FROM aggregate_error_v LIMIT 1 +)) FROM aggregate_error_v; +---- +Yardstick AGGREGATE does not support + +# Ordinary aggregate decorations in a measure definition remain valid. +query RR +SELECT AGGREGATE(revenue), AGGREGATE(large_revenue) FROM aggregate_error_v; +---- +50.0 30.0 + +query IR +SELECT aggregate([1, 2], 'sum'), AGGREGATE(revenue) FROM aggregate_error_v; +---- +3 50.0 + +query R +SELECT AGGREGATE(revenue) AS export_state FROM aggregate_error_v; +---- +50.0 + +# DuckDB retains ownership of invalid decorations on its multiargument scalar. +statement error +SELECT aggregate(DISTINCT [1, 2], 'sum'); +---- +Scalar Function + +statement error +SELECT aggregate([1, 2], 'sum') OVER (); +---- +aggregate is not an aggregate function + +endloop + +query I +SELECT COUNT(*) FROM aggregate_error_results; +---- +0 diff --git a/test/sql/native_aggregate_statements.test b/test/sql/native_aggregate_statements.test new file mode 100644 index 0000000..0671886 --- /dev/null +++ b/test/sql/native_aggregate_statements.test @@ -0,0 +1,104 @@ +# name: test/sql/native_aggregate_statements.test +# description: Native aggregate discovery traverses statement query wrappers. +# group: [yardstick] + +require-env YARDSTICK_NATIVE_PEG 1 + +require yardstick + +statement ok +CREATE TABLE native_statement_sales(year INTEGER, amount INTEGER); + +statement ok +INSERT INTO native_statement_sales VALUES (2023, 10), (2023, 20), (2024, 50); + +statement ok +CREATE VIEW native_statement_v AS +SELECT year, SUM(amount) AS MEASURE revenue FROM native_statement_sales; + +foreach parser_mode true false + +statement ok +SET heap_based_parser=${parser_mode}; + +statement ok +CREATE OR REPLACE TABLE native_statement_output AS +SELECT year, AGGREGATE(revenue) AT (ALL) AS total FROM native_statement_v GROUP BY year; + +query IR rowsort +SELECT * FROM native_statement_output; +---- +2023 80.0 +2024 80.0 + +statement ok +CREATE OR REPLACE VIEW native_statement_totals AS +SELECT year, AGGREGATE(revenue) AS total FROM native_statement_v GROUP BY year; + +query IR rowsort +SELECT * FROM native_statement_totals; +---- +2023 30.0 +2024 50.0 + +statement ok +DELETE FROM native_statement_output; + +statement ok +WITH totals AS ( + SELECT year, AGGREGATE(revenue) AS total FROM native_statement_v GROUP BY year +) +INSERT INTO native_statement_output SELECT * FROM totals; + +query IR rowsort +SELECT * FROM native_statement_output; +---- +2023 30.0 +2024 50.0 + +# UPDATE traverses both its SET expression and condition subquery. +statement ok +UPDATE native_statement_output +SET total = (SELECT AGGREGATE(revenue) AT (ALL) FROM native_statement_v) +WHERE total < (SELECT AGGREGATE(revenue) FROM native_statement_v WHERE year = 2024); + +query IR rowsort +SELECT * FROM native_statement_output; +---- +2023 80.0 +2024 50.0 + +# DELETE visits scalar subqueries inside the predicate. +statement ok +DELETE FROM native_statement_output +WHERE total = (SELECT AGGREGATE(revenue) AT (ALL) FROM native_statement_v); + +query IR +SELECT * FROM native_statement_output; +---- +2024 50.0 + +statement ok +EXPLAIN SELECT AGGREGATE(revenue) AT (ALL) FROM native_statement_v; + +# EXPLAIN ANALYZE executes its wrapped INSERT, making its rewrite observable. +statement ok +EXPLAIN ANALYZE INSERT INTO native_statement_output +SELECT 2025, AGGREGATE(revenue) AT (ALL) FROM native_statement_v; + +query IR rowsort +SELECT * FROM native_statement_output; +---- +2024 50.0 +2025 80.0 + +statement ok +COPY (SELECT AGGREGATE(revenue) AT (ALL) AS total FROM native_statement_v) +TO '__TEST_DIR__/native_statement_totals.csv' (FORMAT CSV, HEADER); + +query R +SELECT total FROM read_csv('__TEST_DIR__/native_statement_totals.csv'); +---- +80.0 + +endloop diff --git a/yardstick-rs/src/parser_ffi.rs b/yardstick-rs/src/parser_ffi.rs index 7c5ea84..b7f4002 100644 --- a/yardstick-rs/src/parser_ffi.rs +++ b/yardstick-rs/src/parser_ffi.rs @@ -795,28 +795,51 @@ unsafe fn c_str_to_string(ptr: *const c_char) -> Option { /// assert_eq!(calls[0].measure_name, "revenue"); /// ``` pub fn find_aggregates(sql: &str) -> Result, String> { - find_aggregates_with_source(sql).map(|(calls, _)| calls) + find_aggregates_with_source(sql) + .map(|(calls, _)| calls) + .map_err(|error| error.message) } -pub(crate) fn find_aggregates_with_source(sql: &str) -> Result<(Vec, bool), String> { +#[derive(Debug)] +pub(crate) struct AggregateParseError { + pub message: String, + pub native_parsed: bool, +} + +impl AggregateParseError { + fn compatibility(message: impl Into) -> Self { + Self { + message: message.into(), + native_parsed: false, + } + } +} + +pub(crate) fn find_aggregates_with_source( + sql: &str, +) -> Result<(Vec, bool), AggregateParseError> { if FN_FIND_AGGREGATES.load(Ordering::SeqCst).is_null() { - return Err("Parser FFI not initialized".to_string()); + return Err(AggregateParseError::compatibility("Parser FFI not initialized")); } - let c_sql = CString::new(sql).map_err(|e| format!("Invalid SQL string: {e}"))?; + let c_sql = CString::new(sql) + .map_err(|e| AggregateParseError::compatibility(format!("Invalid SQL string: {e}")))?; unsafe { let list_ptr = yardstick_find_aggregates(c_sql.as_ptr()); if list_ptr.is_null() { - return Err("Failed to parse SQL".to_string()); + return Err(AggregateParseError::compatibility("Failed to parse SQL")); } let list = &*list_ptr; // Check for error if !list.error.is_null() { - let error_msg = c_str_to_string(list.error).unwrap_or_else(|| "Unknown error".to_string()); + let error = AggregateParseError { + message: c_str_to_string(list.error).unwrap_or_else(|| "Unknown error".to_string()), + native_parsed: list.native_parsed, + }; yardstick_free_aggregate_list(list_ptr); - return Err(error_msg); + return Err(error); } // Convert calls to Rust types diff --git a/yardstick-rs/src/sql/measures.rs b/yardstick-rs/src/sql/measures.rs index 4efa410..7bf43b9 100644 --- a/yardstick-rs/src/sql/measures.rs +++ b/yardstick-rs/src/sql/measures.rs @@ -284,8 +284,12 @@ pub fn has_as_measure(sql: &str) -> bool { /// Check if SQL contains AGGREGATE( function pub fn has_aggregate_function(sql: &str) -> bool { - if let Ok((calls, true)) = parser_ffi::find_aggregates_with_source(sql) { - return !calls.is_empty(); + match parser_ffi::find_aggregates_with_source(sql) { + Ok((calls, true)) => return !calls.is_empty(), + // Recognized native syntax errors must reach expansion, which reports + // them before compatibility scanning can discard call decorations. + Err(error) if error.native_parsed => return true, + _ => {} } let chars: Vec = sql.chars().collect(); let len = chars.len(); @@ -7462,6 +7466,18 @@ fn warning_for_at_all_ungrouped_where_with_qualifiers( /// Expand AGGREGATE() with AT modifiers in SQL pub fn expand_aggregate_with_at(sql: &str) -> AggregateExpandResult { + // Validate the complete statement before rewriting individual query scopes. + // This also covers calls in statement wrappers and nested expressions. + if let Err(error) = parser_ffi::find_aggregates_with_source(sql) { + if error.native_parsed { + return AggregateExpandResult { + had_aggregate: true, + expanded_sql: sql.to_string(), + error: Some(error.message), + warnings: Vec::new(), + }; + } + } if let Some(scopes) = parser_ffi::find_query_scopes(sql) { return expand_native_query_scopes(sql, &scopes); }