From 161571200f7b81caa6ca3964764fd0e2a0e3e692 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 1 Dec 2025 02:53:54 +0000 Subject: [PATCH 1/3] fix: output OVER clause in rewritten SQL for window functions The FuncExpr cases in astvisit files were manually constructing function output but not including the OVER clause, causing argument-less window functions like ROW_NUMBER() and RANK() to fail with "misuse of window function" errors. --- internal/stackql/astvisit/fragment_rewriting.go | 3 +++ internal/stackql/astvisit/from_rewrite.go | 3 +++ internal/stackql/astvisit/provider_string_extract.go | 3 +++ 3 files changed, 9 insertions(+) diff --git a/internal/stackql/astvisit/fragment_rewriting.go b/internal/stackql/astvisit/fragment_rewriting.go index a3bebbe6..201514be 100644 --- a/internal/stackql/astvisit/fragment_rewriting.go +++ b/internal/stackql/astvisit/fragment_rewriting.go @@ -905,6 +905,9 @@ func (v *standardFragmentRewriteAstVisitor) Visit(node sqlparser.SQLNode) error buf.WriteString(funcName) } buf.AstPrintf(node, "(%s%v)", distinct, node.Exprs) + if node.Over != nil { + buf.AstPrintf(node, " %v", node.Over) + } v.rewrittenQuery = buf.String() case *sqlparser.GroupConcatExpr: diff --git a/internal/stackql/astvisit/from_rewrite.go b/internal/stackql/astvisit/from_rewrite.go index 4db8ba71..b77af0cd 100644 --- a/internal/stackql/astvisit/from_rewrite.go +++ b/internal/stackql/astvisit/from_rewrite.go @@ -975,6 +975,9 @@ func (v *standardFromRewriteAstVisitor) Visit(node sqlparser.SQLNode) error { buf.WriteString(funcName) } buf.AstPrintf(node, "(%s%v)", distinct, node.Exprs) + if node.Over != nil { + buf.AstPrintf(node, " %v", node.Over) + } v.rewrittenQuery = buf.String() case *sqlparser.GroupConcatExpr: diff --git a/internal/stackql/astvisit/provider_string_extract.go b/internal/stackql/astvisit/provider_string_extract.go index 62cda611..ce7eab26 100644 --- a/internal/stackql/astvisit/provider_string_extract.go +++ b/internal/stackql/astvisit/provider_string_extract.go @@ -904,6 +904,9 @@ func (v *standardProviderStringAstVisitor) Visit(node sqlparser.SQLNode) error { buf.WriteString(funcName) } buf.AstPrintf(node, "(%s%v)", distinct, node.Exprs) + if node.Over != nil { + buf.AstPrintf(node, " %v", node.Over) + } // v.rewrittenQuery = buf.String() case *sqlparser.GroupConcatExpr: From 5b9e1aaa704f9e4e5cc0feafc5bcbd0a4ad64265 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 1 Dec 2025 03:46:31 +0000 Subject: [PATCH 2/3] fix: use astformat.String for multi-arg functions to preserve OVER clause The inferColNameFromExpr function was manually constructing the decorated column for functions with 0 or 2+ arguments using fmt.Sprintf, which did not include the OVER clause for window functions like ROW_NUMBER(), RANK(), etc. Changed to use astformat.String(expr, formatter) which properly calls FuncExpr.Format() and includes the OVER clause. --- internal/stackql/parserutil/parser_util.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/internal/stackql/parserutil/parser_util.go b/internal/stackql/parserutil/parser_util.go index f94f03d4..85c8493a 100644 --- a/internal/stackql/parserutil/parser_util.go +++ b/internal/stackql/parserutil/parser_util.go @@ -676,7 +676,8 @@ func inferColNameFromExpr( exprsDecorated = append(exprsDecorated, rv.DecoratedColumn) } } - decoratedColumn := fmt.Sprintf("%s(%s)", funcNameLowered, strings.Join(exprsDecorated, ", ")) + // Use astformat.String to include OVER clause for window functions + decoratedColumn := astformat.String(expr, formatter) if retVal.Name != constants.SQLFuncJSONExtractPostgres { retVal.DecoratedColumn = getDecoratedColRendition(decoratedColumn, alias) } From e755b7a13dc8abf2c002b265aadbfb5bd05e4198 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 1 Dec 2025 05:12:54 +0000 Subject: [PATCH 3/3] fix: only use astformat.String for window functions with OVER clause The previous change broke json_extract and other multi-argument functions because astformat.String doesn't apply the same recursive processing to arguments that the manual construction does. Now only use astformat.String when expr.Over != nil (actual window functions), keeping the original fmt.Sprintf behavior for regular functions to maintain proper argument decoration. --- internal/stackql/parserutil/parser_util.go | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/internal/stackql/parserutil/parser_util.go b/internal/stackql/parserutil/parser_util.go index 85c8493a..0ddf2f86 100644 --- a/internal/stackql/parserutil/parser_util.go +++ b/internal/stackql/parserutil/parser_util.go @@ -676,8 +676,14 @@ func inferColNameFromExpr( exprsDecorated = append(exprsDecorated, rv.DecoratedColumn) } } - // Use astformat.String to include OVER clause for window functions - decoratedColumn := astformat.String(expr, formatter) + // Use astformat.String for window functions to include OVER clause, + // otherwise use the manually constructed decorated column for proper arg handling + var decoratedColumn string + if expr.Over != nil { + decoratedColumn = astformat.String(expr, formatter) + } else { + decoratedColumn = fmt.Sprintf("%s(%s)", funcNameLowered, strings.Join(exprsDecorated, ", ")) + } if retVal.Name != constants.SQLFuncJSONExtractPostgres { retVal.DecoratedColumn = getDecoratedColRendition(decoratedColumn, alias) }