Skip to content

Commit 3ad57ba

Browse files
committed
Reject static subqueries
1 parent 65b5536 commit 3ad57ba

File tree

5 files changed

+71
-21
lines changed

5 files changed

+71
-21
lines changed

packages/sync-rules/src/streams/from_sql.ts

Lines changed: 11 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,21 +1,5 @@
11
import { SqlRuleError } from '../errors.js';
2-
import {
3-
ClauseError,
4-
CompiledClause,
5-
FilterParameters,
6-
InputParameter,
7-
ParameterMatchClause,
8-
ParameterValueClause,
9-
ParameterValueSet,
10-
QueryParameters,
11-
QuerySchema,
12-
RowValueClause,
13-
SqliteJsonValue,
14-
SqliteValue,
15-
StaticValueClause,
16-
StreamParseOptions,
17-
TrueIfParametersMatch
18-
} from '../types.js';
2+
import { CompiledClause, QuerySchema, StaticValueClause, StreamParseOptions } from '../types.js';
193
import { isSelectStatement } from '../utils.js';
204
import {
215
andFilters,
@@ -72,7 +56,6 @@ class SyncStreamCompiler {
7256

7357
compile(): SyncStream {
7458
const [stmt, ...illegalRest] = parse(this.sql, { locationTracking: true });
75-
const errors: SqlRuleError[] = [];
7659

7760
// TODO: Share more of this code with SqlDataQuery
7861
if (illegalRest.length > 0) {
@@ -100,7 +83,7 @@ class SyncStreamCompiler {
10083
stream.subscribedToByDefault = this.options.default ?? false;
10184
stream.variants = filter.compileVariants(this.descriptorName);
10285

103-
errors.push(...tools.errors);
86+
this.errors.push(...tools.errors);
10487
return stream;
10588
}
10689

@@ -282,7 +265,15 @@ class SyncStreamCompiler {
282265
}
283266
const [subquery, subqueryTools] = subqueryResult;
284267

285-
if (isStaticValueClause(left) || isParameterValueClause(left)) {
268+
if (isStaticValueClause(left)) {
269+
tools.error(
270+
'For IN subqueries, the left operand must either depend on the row to sync or stream parameters.',
271+
clause.left
272+
);
273+
return recoverErrorClause(tools);
274+
}
275+
276+
if (isParameterValueClause(left)) {
286277
// Case 2: We can't implement this as an actual IN operator because we need to use exact parameter lookups (so
287278
// we can't, for instance, resolve `SELECT * FROM users WHERE is_admin` via parameter data sets). Since the
288279
// left clause doesn't depend on row data however, we can push it down into the subquery where it would be
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
import 'vitest';
2+
3+
interface CustomMatchers<R = unknown> {
4+
toBeSqlRuleError: (message: string, location: string) => R;
5+
}
6+
7+
declare module 'vitest' {
8+
interface Assertion<T = any> extends CustomMatchers<T> {}
9+
interface AsymmetricMatchersContaining extends CustomMatchers {}
10+
}
Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
import { beforeAll, expect } from 'vitest';
2+
import { SqlRuleError } from '../src/index.js';
3+
4+
beforeAll(() => {
5+
expect.extend({
6+
toBeSqlRuleError(received, expectedMessage, expectedLocation) {
7+
const { isNot } = this;
8+
9+
const message = () => {
10+
return `expected ${received} ${isNot ? ' not' : ''} to be SQL error with ${expectedMessage} at ${expectedLocation}`;
11+
};
12+
13+
if (received instanceof SqlRuleError) {
14+
const actualLocation =
15+
received.location && received.sql.substring(received.location.start, received.location.end);
16+
17+
return {
18+
pass: received.message == expectedMessage && actualLocation == expectedLocation,
19+
message
20+
};
21+
} else {
22+
return {
23+
pass: false,
24+
message
25+
};
26+
}
27+
}
28+
});
29+
});

packages/sync-rules/test/src/streams.test.ts

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
/// <reference path="../matchers.d.ts" />
12
import { describe, expect, test } from 'vitest';
23
import {
34
BucketParameterQuerier,
@@ -126,6 +127,23 @@ describe('streams', () => {
126127
});
127128
});
128129

130+
describe('errors', () => {
131+
test('IN operator with static left clause', () => {
132+
const [_, errors] = syncStreamFromSql(
133+
's',
134+
"SELECT * FROM issues WHERE 'static' IN (SELECT id FROM users WHERE is_admin)",
135+
options
136+
);
137+
138+
expect(errors).toMatchObject([
139+
expect.toBeSqlRuleError(
140+
'For IN subqueries, the left operand must either depend on the row to sync or stream parameters.',
141+
"'static'"
142+
)
143+
]);
144+
});
145+
});
146+
129147
/*
130148
test('static filter', () => {
131149
const desc = parseSingleBucketDescription(`

packages/sync-rules/vitest.config.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,5 +2,7 @@ import { defineConfig } from 'vitest/config';
22

33
export default defineConfig({
44
plugins: [],
5-
test: {}
5+
test: {
6+
setupFiles: ['test/matchers.ts']
7+
}
68
});

0 commit comments

Comments
 (0)