Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 58 additions & 2 deletions packages/2-sql/4-lanes/relational-core/src/ast/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,12 @@ export type BinaryOp =
| 'like'
| 'ilike'
| 'in'
| 'notIn';
| 'notIn'
| 'add'
| 'sub'
| 'mul'
| 'div'
| 'mod';

export type AggregateCountFn = 'count';
export type AggregateOpFn = 'sum' | 'avg' | 'min' | 'max';
Expand Down Expand Up @@ -52,6 +57,7 @@ export interface ExprVisitor<R> {
exists(expr: ExistsExpr): R;
nullCheck(expr: NullCheckExpr): R;
not(expr: NotExpr): R;
cast(expr: CastExpr): R;
literal(expr: LiteralExpr): R;
param(expr: ParamRef): R;
list(expr: ListExpression): R;
Expand Down Expand Up @@ -839,6 +845,26 @@ export class BinaryExpr extends Expression {
return new BinaryExpr('notIn', left, right);
}

static add(left: AnyExpression, right: AnyExpression): BinaryExpr {
return new BinaryExpr('add', left, right);
}

static sub(left: AnyExpression, right: AnyExpression): BinaryExpr {
return new BinaryExpr('sub', left, right);
}

static mul(left: AnyExpression, right: AnyExpression): BinaryExpr {
return new BinaryExpr('mul', left, right);
}

static div(left: AnyExpression, right: AnyExpression): BinaryExpr {
return new BinaryExpr('div', left, right);
}

static mod(left: AnyExpression, right: AnyExpression): BinaryExpr {
return new BinaryExpr('mod', left, right);
}

override accept<R>(visitor: ExprVisitor<R>): R {
return visitor.binary(this);
}
Expand Down Expand Up @@ -1020,6 +1046,35 @@ export class NotExpr extends Expression {
}
}

export class CastExpr extends Expression {
readonly kind = 'cast' as const;
readonly expr: AnyExpression;
readonly targetCodecId: string;

constructor(expr: AnyExpression, targetCodecId: string) {
super();
this.expr = expr;
this.targetCodecId = targetCodecId;
this.freeze();
}

static of(expr: AnyExpression, targetCodecId: string): CastExpr {
return new CastExpr(expr, targetCodecId);
}

override accept<R>(visitor: ExprVisitor<R>): R {
return visitor.cast(this);
}

override rewrite(rewriter: ExpressionRewriter): AnyExpression {
return new CastExpr(this.expr.rewrite(rewriter), this.targetCodecId);
}

override fold<T>(folder: ExpressionFolder<T>): T {
return this.expr.fold(folder);
}
}

export class EqColJoinOn extends AstNode {
readonly kind = 'eq-col-join-on' as const;
readonly left: ColumnRef;
Expand Down Expand Up @@ -1714,7 +1769,8 @@ export type AnyExpression =
| OrExpr
| ExistsExpr
| NullCheckExpr
| NotExpr;
| NotExpr
| CastExpr;
export type AnyInsertOnConflictAction = DoNothingConflictAction | DoUpdateSetConflictAction;
export type AnyInsertValue = ColumnRef | ParamRef | DefaultValueExpr;
export type AnyOperationArg = AnyExpression | ParamRef | LiteralExpr;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import {
AggregateExpr,
AndExpr,
BinaryExpr,
CastExpr,
DefaultValueExpr,
DeleteAst,
DerivedTableSource,
Expand Down Expand Up @@ -58,6 +59,7 @@ const allKindEntries: Array<[string, { kind: string }]> = [
['InsertOnConflict', InsertOnConflict.on([col('t', 'id')])],
['DoNothingConflictAction', new DoNothingConflictAction()],
['DoUpdateSetConflictAction', new DoUpdateSetConflictAction({ id: col('t', 'id') })],
['CastExpr', CastExpr.of(col('t', 'id'), 'pg/text@1')],
];

describe('AST kind discriminants', () => {
Expand Down Expand Up @@ -90,6 +92,7 @@ describe('AST kind discriminants', () => {
['InsertOnConflict', 'insert-on-conflict'],
['DoNothingConflictAction', 'do-nothing'],
['DoUpdateSetConflictAction', 'do-update-set'],
['CastExpr', 'cast'],
])('%s has kind "%s"', (className, expectedKind) => {
const entry = allKindEntries.find(([name]) => name === className);
expect(entry).toBeDefined();
Expand Down
26 changes: 26 additions & 0 deletions packages/2-sql/4-lanes/sql-builder/src/expression.ts
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,32 @@ export type BuiltinFunctions<CT extends Record<string, { readonly input: unknown
and: (...ands: ExpressionOrValue<BooleanCodecType, CT>[]) => Expression<BooleanCodecType>;
or: (...ors: ExpressionOrValue<BooleanCodecType, CT>[]) => Expression<BooleanCodecType>;

add: <T extends ScopeField>(
a: ExpressionOrValue<T, CT>,
b: ExpressionOrValue<T, CT>,
) => Expression<T>;
sub: <T extends ScopeField>(
a: ExpressionOrValue<T, CT>,
b: ExpressionOrValue<T, CT>,
) => Expression<T>;
mul: <T extends ScopeField>(
a: ExpressionOrValue<T, CT>,
b: ExpressionOrValue<T, CT>,
) => Expression<T>;
div: <T extends ScopeField>(
a: ExpressionOrValue<T, CT>,
b: ExpressionOrValue<T, CT>,
) => Expression<T>;
mod: <T extends ScopeField>(
a: ExpressionOrValue<T, CT>,
b: ExpressionOrValue<T, CT>,
) => Expression<T>;

cast: <TargetCodecId extends string, Nullable extends boolean>(
expr: Expression<ScopeField>,
target: { codecId: TargetCodecId; nullable: Nullable },
) => Expression<{ codecId: TargetCodecId; nullable: Nullable }>;
Comment on lines +132 to +135
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

cast() should preserve source nullability.

This signature lets fns.cast(nullableExpr, { codecId: 'pg/text@1', nullable: false }) type-check as non-null, and the runtime implementation copies that flag straight into ExpressionImpl.field. SQL casts do not turn NULL into non-NULL, so the output nullability should be derived from the source expression instead of caller input.

🐛 Proposed fix
-  cast: <TargetCodecId extends string, Nullable extends boolean>(
-    expr: Expression<ScopeField>,
-    target: { codecId: TargetCodecId; nullable: Nullable },
-  ) => Expression<{ codecId: TargetCodecId; nullable: Nullable }>;
+  cast: <Source extends ScopeField, TargetCodecId extends string>(
+    expr: Expression<Source>,
+    target: { codecId: TargetCodecId },
+  ) => Expression<{ codecId: TargetCodecId; nullable: Source['nullable'] }>;
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/2-sql/4-lanes/sql-builder/src/expression.ts` around lines 132 - 135,
The cast function currently takes a target nullable flag and uses it for the
resulting Expression, allowing callers to incorrectly cast a nullable source to
a non-null type; update the cast signature and implementation so the resulting
Expression preserves the source expression's nullability instead of trusting
target.nullable: keep TargetCodecId generic for codecId but derive Nullable from
the input expr (e.g. read expr.field.codecId.nullable or the expr type
parameter) and construct the returned Expression<{ codecId: TargetCodecId;
nullable: SourceNullable }>, and ensure ExpressionImpl.field is assigned using
that source-nullable value rather than target.nullable.


exists: (subquery: Subquery<Record<string, ScopeField>>) => Expression<BooleanCodecType>;
notExists: (subquery: Subquery<Record<string, ScopeField>>) => Expression<BooleanCodecType>;

Expand Down
45 changes: 45 additions & 0 deletions packages/2-sql/4-lanes/sql-builder/src/runtime/functions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import {
type AnyExpression as AstExpression,
BinaryExpr,
type BinaryOp,
CastExpr,
ExistsExpr,
ListExpression,
LiteralExpr,
Expand Down Expand Up @@ -76,6 +77,24 @@ function inOrNotIn(
return boolExpr(binaryFn(left, SubqueryExpr.of(valuesOrSubquery.buildAst())));
}

function arithmetic<T extends ScopeField>(
a: ExpressionOrValue<T, CodecTypes>,
b: ExpressionOrValue<T, CodecTypes>,
op: BinaryOp,
): ExpressionImpl<T> {
const field = (
a instanceof ExpressionImpl
? a.field
: b instanceof ExpressionImpl
? b.field
: { codecId: 'unknown', nullable: false }
) as T;
return new ExpressionImpl(
new BinaryExpr(op, resolve(a as ExprOrVal), resolve(b as ExprOrVal)),
field,
);
}

function numericAgg(
fn: 'sum' | 'avg' | 'min' | 'max',
expr: Expression<ScopeField>,
Expand All @@ -96,6 +115,32 @@ function createBuiltinFunctions() {
lte: (a: ExprOrVal, b: ExprOrVal) => comparison(a, b, 'lte'),
and: (...exprs: ExprOrVal<BooleanCodecType>[]) => boolExpr(AndExpr.of(exprs.map(resolveToAst))),
or: (...exprs: ExprOrVal<BooleanCodecType>[]) => boolExpr(OrExpr.of(exprs.map(resolveToAst))),
add: <T extends ScopeField>(
a: ExpressionOrValue<T, CodecTypes>,
b: ExpressionOrValue<T, CodecTypes>,
) => arithmetic(a, b, 'add'),
sub: <T extends ScopeField>(
a: ExpressionOrValue<T, CodecTypes>,
b: ExpressionOrValue<T, CodecTypes>,
) => arithmetic(a, b, 'sub'),
mul: <T extends ScopeField>(
a: ExpressionOrValue<T, CodecTypes>,
b: ExpressionOrValue<T, CodecTypes>,
) => arithmetic(a, b, 'mul'),
div: <T extends ScopeField>(
a: ExpressionOrValue<T, CodecTypes>,
b: ExpressionOrValue<T, CodecTypes>,
) => arithmetic(a, b, 'div'),
mod: <T extends ScopeField>(
a: ExpressionOrValue<T, CodecTypes>,
b: ExpressionOrValue<T, CodecTypes>,
) => arithmetic(a, b, 'mod'),
cast: <TargetCodecId extends string, Nullable extends boolean>(
expr: Expression<ScopeField>,
target: { codecId: TargetCodecId; nullable: Nullable },
) => {
return new ExpressionImpl(CastExpr.of(expr.buildAst(), target.codecId), target);
},
exists: (subquery: Subquery<Record<string, ScopeField>>) =>
boolExpr(ExistsExpr.exists(subquery.buildAst())),
notExists: (subquery: Subquery<Record<string, ScopeField>>) =>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
import { describe, expect, it } from 'vitest';
import { setupIntegrationTest } from './setup';

describe('integration: arithmetic operations', () => {
const { db, runtime } = setupIntegrationTest();

it('add computes column + literal', async () => {
const rows = await runtime().execute(
db()
.posts.select('id')
.select('viewsPlus', (f, fns) => fns.add(f.views, 10))
.where((f, fns) => fns.eq(f.id, 1))
.build(),
);
expect(rows).toHaveLength(1);
expect(rows[0]!.viewsPlus).toBe(110);
});

it('sub computes column - literal', async () => {
const rows = await runtime().execute(
db()
.posts.select('id')
.select('viewsMinus', (f, fns) => fns.sub(f.views, 10))
.where((f, fns) => fns.eq(f.id, 1))
.build(),
);
expect(rows[0]!.viewsMinus).toBe(90);
});

it('mul computes column * literal', async () => {
const rows = await runtime().execute(
db()
.posts.select('id')
.select('viewsDouble', (f, fns) => fns.mul(f.views, 2))
.where((f, fns) => fns.eq(f.id, 1))
.build(),
);
expect(rows[0]!.viewsDouble).toBe(200);
});

it('div computes column / literal', async () => {
const rows = await runtime().execute(
db()
.posts.select('id')
.select('viewsHalf', (f, fns) => fns.div(f.views, 2))
.where((f, fns) => fns.eq(f.id, 1))
.build(),
);
expect(rows[0]!.viewsHalf).toBe(50);
});

it('mod computes column % literal', async () => {
const rows = await runtime().execute(
db()
.posts.select('id')
.select('viewsMod', (f, fns) => fns.mod(f.views, 3))
.where((f, fns) => fns.eq(f.id, 1))
.build(),
);
expect(rows[0]!.viewsMod).toBe(1);
});

it('arithmetic in WHERE clause filters correctly', async () => {
const rows = await runtime().execute(
db()
.posts.select('id', 'views')
.where((f, fns) => fns.gt(fns.add(f.views, 50), 150))
.build(),
);
expect(rows.every((r) => r.views + 50 > 150)).toBe(true);
Comment on lines +63 to +70
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

This WHERE assertion is vacuously true on an empty result set.

rows.every(...) returns true for [], so this test still passes if the predicate is rendered incorrectly and filters everything out. Assert against a concrete expected row set, or at least verify the query returned rows before checking the predicate.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@packages/2-sql/4-lanes/sql-builder/test/integration/arithmetic-and-cast.test.ts`
around lines 63 - 70, The test currently uses rows.every(...) which is vacuously
true for an empty result set; update the test in arithmetic-and-cast.test.ts to
assert the query returns results before checking the predicate (e.g., assert
rows.length > 0) or replace the loose predicate with a concrete expected result
comparison; locate the test using
runtime().execute(db().posts.select('id','views').where((f,fns) =>
fns.gt(fns.add(f.views,50),150)).build()) and add a precondition on rows (or an
explicit expectedRows assertion) before calling rows.every(...) to ensure the
filter actually matched rows.

});

it('nested arithmetic works', async () => {
const rows = await runtime().execute(
db()
.posts.select('id')
.select('computed', (f, fns) => fns.add(fns.mul(f.views, 2), 1))
.where((f, fns) => fns.eq(f.id, 1))
.build(),
);
expect(rows[0]!.computed).toBe(201);
});
});
28 changes: 28 additions & 0 deletions packages/2-sql/4-lanes/sql-builder/test/integration/cast.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
import { describe, expect, it } from 'vitest';
import { setupIntegrationTest } from './setup';

describe('integration: type cast', () => {
const { db, runtime } = setupIntegrationTest();

it('cast int to text', async () => {
const rows = await runtime().execute(
db()
.posts.select('id')
.select('idText', (f, fns) => fns.cast(f.id, { codecId: 'pg/text@1', nullable: false }))
.where((f, fns) => fns.eq(f.id, 1))
.build(),
);
expect(rows[0]!.idText).toBe('1');
});

it('cast in WHERE clause', async () => {
const rows = await runtime().execute(
db()
.posts.select('id')
.where((f, fns) => fns.eq(fns.cast(f.id, { codecId: 'pg/text@1', nullable: false }), '1'))
.build(),
);
expect(rows).toHaveLength(1);
expect(rows[0]!.id).toBe(1);
});
});
Loading
Loading