Skip to content

Commit 835d6ff

Browse files
vkuttypclaude
andcommitted
v2.2 part 1 — parameterized TOP / OFFSET / FETCH NEXT
The v2.1 parser required integer literals in paging slots; v2.2 admits any scalar expression. Primary unlock: consumers can supply the page size as a parameter rather than string-interpolating it into the SQL. This was blocking 8 paging stored procs in CosmoMailServer's Phase C migration plus the UPDATE TOP(n) claim-pending rewrites. Surface: SELECT TOP @n … ORDER BY id SELECT TOP (@A + @b) … ORDER BY id -- arithmetic via parens SELECT … ORDER BY id OFFSET @offset ROWS FETCH NEXT @limit ROWS ONLY UPDATE TOP(@Batch) … DELETE TOP(@n) … AST: SelectStatement.Top/Offset/Fetch, UpdateStatement.Top, and DeleteStatement.Top change from `long?` to `Expression?`. Literal TOPs now wrap to LiteralExpression(SqlValue.From(n)). Parser: SELECT's bare-form `TOP n` keeps a tight `ParseUnary()`-only grammar to avoid the `SELECT TOP 10 * FROM t` wildcard-vs-multiplication ambiguity. The paren form `TOP (n)` opens up to full ParseExpression, matching T-SQL's spec — for non-trivial paging expressions you must parenthesise. OFFSET / FETCH and UPDATE/DELETE TOP(n) all accept full expressions (no wildcard ambiguity in those positions). Executor: a shared EvaluatePagingExpression helper resolves the Expression to a non-negative long using a dummy empty-row RowContext (same pattern as the no-FROM SELECT path). Called once per statement outside the scan loop so the cost is constant regardless of row count. Phase 9 tests (9 cases): parameter-driven TOP/OFFSET/FETCH, arithmetic in TOP-parens, UPDATE TOP(@Batch) for claim-pending, DELETE TOP(@n), negative-parameter rejection, string-parameter rejection, and the literal-TOP regression path. Full suite stable: 240/240 across 3 runs (231 v2.1 + 9 new). Unblocks: 8 of the 16 procs that throw NotSupportedException in CosmoMailServer's CosmoKvCompatibilitySqlDatabase. Phase C resumes once 2.2.0 ships. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 912ba0d commit 835d6ff

5 files changed

Lines changed: 255 additions & 29 deletions

File tree

src/CosmoSQLClient.CosmoKv/Ast/Statement.cs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -40,9 +40,9 @@ internal sealed record SelectStatement(
4040
IReadOnlyList<Expression>? GroupBy,
4141
Expression? Having,
4242
IReadOnlyList<OrderByItem>? OrderBy,
43-
long? Top,
44-
long? Offset,
45-
long? Fetch) : Statement
43+
Expression? Top,
44+
Expression? Offset,
45+
Expression? Fetch) : Statement
4646
{
4747
/// <summary>
4848
/// Convenience accessor: the base table's identifier (no alias). Used by
@@ -85,15 +85,15 @@ internal sealed record UpdateStatement(
8585
string TableName,
8686
IReadOnlyList<UpdateAssignment> Assignments,
8787
Expression? Where,
88-
long? Top,
88+
Expression? Top,
8989
OutputClause? Output) : Statement;
9090

9191
internal sealed record UpdateAssignment(string ColumnName, Expression Value);
9292

9393
internal sealed record DeleteStatement(
9494
string TableName,
9595
Expression? Where,
96-
long? Top,
96+
Expression? Top,
9797
OutputClause? Output) : Statement;
9898

9999
// ── OUTPUT clause (v2.1) ─────────────────────────────────────────────────────

src/CosmoSQLClient.CosmoKv/Execution/Executor.cs

Lines changed: 50 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -495,8 +495,10 @@ private async Task<long> NextIdentityAsync(string tableName, long seed, long inc
495495
}
496496

497497
// TOP / OFFSET-FETCH paging.
498-
int skip = (int)(s.Offset ?? 0);
499-
int? take = (int?)(s.Fetch ?? s.Top);
498+
int skip = (int)(EvaluatePagingExpression(s.Offset, bound, "OFFSET") ?? 0);
499+
long? takeLong = EvaluatePagingExpression(s.Fetch ?? s.Top, bound,
500+
s.Fetch is not null ? "FETCH NEXT" : "TOP");
501+
int? take = takeLong is null ? null : (int)takeLong.Value;
500502
if (skip < 0) skip = 0;
501503
if (skip > matched.Count) skip = matched.Count;
502504

@@ -513,6 +515,35 @@ private async Task<long> NextIdentityAsync(string tableName, long seed, long inc
513515
return (resultCols, rows);
514516
}
515517

518+
/// <summary>
519+
/// Evaluate a paging expression (TOP, OFFSET, FETCH NEXT) to a long.
520+
/// v2.2 admitted any scalar expression in these slots — typically a
521+
/// parameter (<c>TOP @limit</c>) but literals and arithmetic also work.
522+
/// Returns null when the expression is null. Throws if the resolved
523+
/// value is negative or not an integer.
524+
/// </summary>
525+
private static long? EvaluatePagingExpression(
526+
Expression? expr, IReadOnlyDictionary<string, SqlValue> bound, string clauseName)
527+
{
528+
if (expr is null) return null;
529+
var dummyTable = new TableSchema(
530+
Name: "(paging)",
531+
Columns: Array.Empty<ColumnSchema>(),
532+
PrimaryKeyColumnIndex: null,
533+
IdentityColumnIndex: null);
534+
var ctx = new ExpressionEvaluator.RowContext(
535+
dummyTable, ReadOnlySpan<SqlValue>.Empty, bound);
536+
var v = ExpressionEvaluator.Evaluate(expr, ctx);
537+
long? n = v.AsInt();
538+
if (n is null)
539+
throw new InvalidOperationException(
540+
$"{clauseName} value must be an integer; got {v.Kind}.");
541+
if (n < 0)
542+
throw new InvalidOperationException(
543+
$"{clauseName} value must be non-negative; got {n}.");
544+
return n;
545+
}
546+
516547
/// <summary>
517548
/// SELECT path for FROM clauses with JOINs. v2.1 ships INNER JOIN with
518549
/// 2-3 table chains; the executor materialises each table, then runs a
@@ -640,8 +671,10 @@ private async Task<long> NextIdentityAsync(string tableName, long seed, long inc
640671
}
641672

642673
// TOP / OFFSET / FETCH paging.
643-
int skip = (int)(s.Offset ?? 0);
644-
int? take = (int?)(s.Fetch ?? s.Top);
674+
int skip = (int)(EvaluatePagingExpression(s.Offset, bound, "OFFSET") ?? 0);
675+
long? takeLong = EvaluatePagingExpression(s.Fetch ?? s.Top, bound,
676+
s.Fetch is not null ? "FETCH NEXT" : "TOP");
677+
int? take = takeLong is null ? null : (int)takeLong.Value;
645678
if (skip < 0) skip = 0;
646679
if (skip > matched.Count) skip = matched.Count;
647680

@@ -869,8 +902,10 @@ private readonly record struct JoinProjectedColumn(
869902
}
870903

871904
// Paging.
872-
int skip = (int)(s.Offset ?? 0);
873-
int? take = (int?)(s.Fetch ?? s.Top);
905+
int skip = (int)(EvaluatePagingExpression(s.Offset, bound, "OFFSET") ?? 0);
906+
long? takeLong = EvaluatePagingExpression(s.Fetch ?? s.Top, bound,
907+
s.Fetch is not null ? "FETCH NEXT" : "TOP");
908+
int? take = takeLong is null ? null : (int)takeLong.Value;
874909
if (skip < 0) skip = 0;
875910
if (skip > keptGroups.Count) skip = keptGroups.Count;
876911
int end = take is null ? keptGroups.Count : Math.Min(keptGroups.Count, skip + take.Value);
@@ -1176,6 +1211,11 @@ private async Task<UpdateResult> ExecuteUpdateCore(
11761211
var plan = QueryPlanner.Plan(
11771212
table, _catalog.IndexesFor(table.Name), s.Where, orderBy: null, bound);
11781213

1214+
// Resolve TOP(n) once up-front — the expression may reference bound
1215+
// parameters but doesn't depend on the row being scanned, so a single
1216+
// evaluation outside the loop suffices.
1217+
long? topLimit = EvaluatePagingExpression(s.Top, bound, "TOP");
1218+
11791219
var pending = new List<(long RowId, SqlValue[] Old, SqlValue[] New)>();
11801220
await foreach (var (row, rowId) in EnumerateAsync(plan, ct))
11811221
{
@@ -1200,7 +1240,7 @@ private async Task<UpdateResult> ExecuteUpdateCore(
12001240
// TOP(n) — once we've accumulated n matches, stop the scan.
12011241
// Note: T-SQL UPDATE TOP(n) without ORDER BY is intentionally
12021242
// non-deterministic about WHICH n rows; we follow that.
1203-
if (s.Top is long topN && pending.Count >= topN) break;
1243+
if (topLimit is long topN && pending.Count >= topN) break;
12041244
}
12051245
foreach (var (rowId, oldRow, newRow) in pending)
12061246
{
@@ -1267,7 +1307,8 @@ private async Task<DeleteResult> ExecuteDeleteCore(
12671307
table, _catalog.IndexesFor(table.Name), s.Where, orderBy: null, bound);
12681308

12691309
// Collect (rowid, full row) for victims so we can clean up indexes
1270-
// too. TOP(n) caps the victim count.
1310+
// too. TOP(n) caps the victim count; evaluated once outside the loop.
1311+
long? topLimit = EvaluatePagingExpression(s.Top, bound, "TOP");
12711312
var victims = new List<(long RowId, SqlValue[] Row)>();
12721313
await foreach (var (row, rowId) in EnumerateAsync(plan, ct))
12731314
{
@@ -1277,7 +1318,7 @@ private async Task<DeleteResult> ExecuteDeleteCore(
12771318
if (!ExpressionEvaluator.EvaluatePredicate(s.Where, ctx)) continue;
12781319
}
12791320
victims.Add((rowId, row));
1280-
if (s.Top is long topN && victims.Count >= topN) break;
1321+
if (topLimit is long topN && victims.Count >= topN) break;
12811322
}
12821323
foreach (var (rowId, row) in victims)
12831324
{

src/CosmoSQLClient.CosmoKv/Parsing/TSqlParser.cs

Lines changed: 41 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -413,11 +413,13 @@ private SelectStatement ParseSelect()
413413
{
414414
Expect(TokenKind.Select);
415415

416-
long? top = null;
416+
Expression? top = null;
417417
if (Current().Kind == TokenKind.Top)
418418
{
419419
Advance();
420-
top = ParseIntegerLiteral();
420+
// v2.2: TOP accepts any expression (parameter, literal, arithmetic).
421+
// T-SQL also allows `TOP (expr)` with parens; both forms work here.
422+
top = ParseTopExpression();
421423
}
422424

423425
var items = new List<SelectItem>();
@@ -503,17 +505,20 @@ private SelectStatement ParseSelect()
503505
orderBy = list;
504506
}
505507

506-
long? offset = null, fetch = null;
508+
Expression? offset = null, fetch = null;
507509
if (Current().Kind == TokenKind.Offset)
508510
{
509511
Advance();
510-
offset = ParseIntegerLiteral();
512+
// v2.2: OFFSET / FETCH accept arbitrary expressions, not just
513+
// integer literals — primarily so callers can parameterise
514+
// page size (@offset / @limit).
515+
offset = ParseExpression();
511516
Expect(TokenKind.Rows);
512517
if (Current().Kind == TokenKind.Fetch)
513518
{
514519
Advance();
515520
Expect(TokenKind.Next);
516-
fetch = ParseIntegerLiteral();
521+
fetch = ParseExpression();
517522
Expect(TokenKind.Rows);
518523
Expect(TokenKind.Only);
519524
}
@@ -609,7 +614,7 @@ or TokenKind.Offset or TokenKind.EndOfInput or TokenKind.Semicolon
609614
private UpdateStatement ParseUpdate()
610615
{
611616
Expect(TokenKind.Update);
612-
long? top = ParseOptionalTopWithParens();
617+
Expression? top = ParseOptionalTopWithParens();
613618
string table = ParseDottedName();
614619
Expect(TokenKind.Set);
615620
var assigns = new List<UpdateAssignment>();
@@ -641,7 +646,7 @@ private UpdateStatement ParseUpdate()
641646
private DeleteStatement ParseDelete()
642647
{
643648
Expect(TokenKind.Delete);
644-
long? top = ParseOptionalTopWithParens();
649+
Expression? top = ParseOptionalTopWithParens();
645650
// FROM is optional in T-SQL DELETE.
646651
ConsumeOptional(TokenKind.From);
647652
string table = ParseDottedName();
@@ -662,18 +667,42 @@ private DeleteStatement ParseDelete()
662667
// ── OUTPUT clause + TOP(n) helpers ──────────────────────────────────────
663668

664669
/// <summary>
665-
/// Parse <c>TOP ( integer-literal )</c> if present. UPDATE/DELETE require
670+
/// Parse <c>TOP ( expression )</c> if present. UPDATE/DELETE require
666671
/// parens per T-SQL grammar; SELECT's bare-form TOP is parsed inline in
667-
/// <see cref="ParseSelect"/> and is unaffected.
672+
/// <see cref="ParseSelect"/>. v2.2: accepts any expression (parameter,
673+
/// literal, arithmetic) rather than just an integer literal so callers
674+
/// can supply <c>TOP(@batch_size)</c>.
668675
/// </summary>
669-
private long? ParseOptionalTopWithParens()
676+
private Expression? ParseOptionalTopWithParens()
670677
{
671678
if (Current().Kind != TokenKind.Top) return null;
672679
Advance();
673680
Expect(TokenKind.LParen);
674-
long n = ParseIntegerLiteral();
681+
var e = ParseExpression();
675682
Expect(TokenKind.RParen);
676-
return n;
683+
return e;
684+
}
685+
686+
/// <summary>
687+
/// Parse the SELECT-form <c>TOP n</c> or <c>TOP (expr)</c>.
688+
/// T-SQL distinguishes the two: bare-form TOP only accepts a single
689+
/// atom (literal, parameter, function call) because the SELECT-list's
690+
/// <c>*</c> wildcard would otherwise be ambiguous with multiplication
691+
/// (<c>SELECT TOP 10 * FROM t</c> — the <c>*</c> is the wildcard).
692+
/// Non-trivial expressions must be parenthesised: <c>TOP (10 * 2)</c>.
693+
/// </summary>
694+
private Expression ParseTopExpression()
695+
{
696+
if (Current().Kind == TokenKind.LParen)
697+
{
698+
Advance();
699+
var e = ParseExpression();
700+
Expect(TokenKind.RParen);
701+
return e;
702+
}
703+
// Bare form: parse only a single atom + unary minus to keep `*`
704+
// unambiguous with the SELECT-list wildcard.
705+
return ParseUnary();
677706
}
678707

679708
/// <summary>

tests/CosmoSQLClient.CosmoKv.Tests/Phase2ParserTests.cs

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -62,16 +62,20 @@ public void Select_OrderBy_MultiColumn_WithDesc()
6262
public void Select_Top()
6363
{
6464
var s = (SelectStatement)TSqlParser.Parse("SELECT TOP 10 * FROM t");
65-
Assert.Equal(10, s.Top);
65+
// v2.2: Top is now Expression; literal TOPs parse as LiteralExpression.
66+
var lit = Assert.IsType<LiteralExpression>(s.Top);
67+
Assert.Equal(10L, lit.Value.AsInt());
6668
}
6769

6870
[Fact]
6971
public void Select_OffsetFetch()
7072
{
7173
var s = (SelectStatement)TSqlParser.Parse(
7274
"SELECT * FROM t ORDER BY id OFFSET 20 ROWS FETCH NEXT 10 ROWS ONLY");
73-
Assert.Equal(20, s.Offset);
74-
Assert.Equal(10, s.Fetch);
75+
var offsetLit = Assert.IsType<LiteralExpression>(s.Offset);
76+
Assert.Equal(20L, offsetLit.Value.AsInt());
77+
var fetchLit = Assert.IsType<LiteralExpression>(s.Fetch);
78+
Assert.Equal(10L, fetchLit.Value.AsInt());
7579
}
7680

7781
[Fact]

0 commit comments

Comments
 (0)