Skip to content

Commit

Permalink
Allow defining priorities on a bucket level
Browse files Browse the repository at this point in the history
  • Loading branch information
simolus3 committed Feb 5, 2025
1 parent c6770ff commit efb834e
Show file tree
Hide file tree
Showing 8 changed files with 90 additions and 6 deletions.
4 changes: 4 additions & 0 deletions packages/sync-rules/src/BucketDescription.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,10 @@ export type BucketPriority = 0 | 1 | 2 | 3;

export const defaultBucketPriority: BucketPriority = 3;

export const isValidPriority = (i: number): i is BucketPriority => {
return Number.isInteger(i) && i >= 0 && i <= 3;
};

export interface BucketDescription {
/**
* The id of the bucket, which is derived from the name of the bucket's definition
Expand Down
1 change: 1 addition & 0 deletions packages/sync-rules/src/SqlParameterQuery.ts
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,7 @@ export class SqlParameterQuery {
rows.descriptor_name = descriptor_name;
rows.bucket_parameters = bucket_parameters;
rows.input_parameters = filter.inputParameters!;
rows.priority = options.priority;
const expandedParams = rows.input_parameters!.filter((param) => param.expands);
if (expandedParams.length > 1) {
rows.errors.push(new SqlRuleError('Cannot have multiple array input parameters', sql));
Expand Down
17 changes: 15 additions & 2 deletions packages/sync-rules/src/SqlSyncRules.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import {
SqliteRow,
SyncRules
} from './types.js';
import { BucketDescription } from './BucketDescription.js';
import { BucketDescription, BucketPriority, isValidPriority } from './BucketDescription.js';

const ACCEPT_POTENTIALLY_DANGEROUS_QUERIES = Symbol('ACCEPT_POTENTIALLY_DANGEROUS_QUERIES');

Expand Down Expand Up @@ -118,9 +118,22 @@ export class SqlSyncRules implements SyncRules {

const accept_potentially_dangerous_queries =
value.get('accept_potentially_dangerous_queries', true)?.value == true;
let parseOptionPriority: BucketPriority | undefined = undefined;
if (value.has('priority')) {
const priorityValue = value.get('priority', true)!;
if (typeof priorityValue.value != 'number' || !isValidPriority(priorityValue.value)) {
rules.errors.push(
this.tokenError(priorityValue, 'Invalid priority, expected a number between 0 and 3 (inclusive).')
);
} else {
parseOptionPriority = priorityValue.value;
}
}

const queryOptions: QueryParseOptions = {
...options,
accept_potentially_dangerous_queries
accept_potentially_dangerous_queries,
priority: parseOptionPriority
};
const parameters = value.get('parameters', true) as unknown;
const dataQueries = value.get('data', true) as unknown;
Expand Down
6 changes: 6 additions & 0 deletions packages/sync-rules/src/StaticSqlParameterQuery.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ export class StaticSqlParameterQuery {
query.bucket_parameters = bucket_parameters;
query.columns = columns;
query.tools = tools;
query.priority = options?.priority;
if (!isClauseError(filter)) {
query.filter = filter;
}
Expand All @@ -47,6 +48,11 @@ export class StaticSqlParameterQuery {
}
const name = tools.getSpecificOutputName(column);
if (tools.isBucketPriorityParameter(name)) {
if (query.priority !== undefined) {
query.errors.push(new SqlRuleError('Cannot set priority multiple times.', sql));
continue;
}

query.priority = tools.extractBucketPriority(column.expr);
continue;
}
Expand Down
4 changes: 4 additions & 0 deletions packages/sync-rules/src/json_schema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,10 @@ export const syncRulesSchema: ajvModule.Schema = {
description: 'If true, disables warnings on potentially dangerous queries',
type: 'boolean'
},
priority: {
description: 'Priority for the bucket (lower values indicate higher priority).',
type: 'integer'
},
parameters: {
description: 'Parameter query(ies)',
anyOf: [
Expand Down
4 changes: 2 additions & 2 deletions packages/sync-rules/src/sql_filters.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ import {
TrueIfParametersMatch
} from './types.js';
import { isJsonValue } from './utils.js';
import { BucketPriority } from './BucketDescription.js';
import { BucketPriority, isValidPriority } from './BucketDescription.js';

export const MATCH_CONST_FALSE: TrueIfParametersMatch = [];
export const MATCH_CONST_TRUE: TrueIfParametersMatch = [{}];
Expand Down Expand Up @@ -751,7 +751,7 @@ export class SqlTools {
}

const value = expr.value;
if (!Number.isInteger(value) || value < 0 || value > 3) {
if (!isValidPriority(value)) {
this.error('Invalid value for priority, must be between 0 and 3 (inclusive).', expr);
return;
}
Expand Down
2 changes: 2 additions & 0 deletions packages/sync-rules/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { SourceTableInterface } from './SourceTableInterface.js';
import { SyncRulesOptions } from './SqlSyncRules.js';
import { TablePattern } from './TablePattern.js';
import { toSyncRulesParameters } from './utils.js';
import { BucketPriority } from './BucketDescription.js';

export interface SyncRules {
evaluateRow(options: EvaluateRowOptions): EvaluationResult[];
Expand All @@ -13,6 +14,7 @@ export interface SyncRules {

export interface QueryParseOptions extends SyncRulesOptions {
accept_potentially_dangerous_queries?: boolean;
priority?: BucketPriority;
}

export interface EvaluatedParameters {
Expand Down
58 changes: 56 additions & 2 deletions packages/sync-rules/test/src/sync_rules.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -817,7 +817,7 @@ bucket_definitions:
expect(rules.errors).toEqual([]);
});

test('priorities', () => {
test('priorities on queries', () => {
const rules = SqlSyncRules.fromYaml(
`
bucket_definitions:
Expand All @@ -836,7 +836,61 @@ bucket_definitions:

expect(rules.getStaticBucketDescriptions(normalizeTokenParameters({}))).toEqual([
{ bucket: 'highprio[]', priority: 0 },
{ bucket: 'defaultprio[]', priority: 3 },
{ bucket: 'defaultprio[]', priority: 3 }
]);
});

test('priorities on bucket', () => {
const rules = SqlSyncRules.fromYaml(
`
bucket_definitions:
highprio:
priority: 0
data:
- SELECT * FROM assets WHERE count <= 10;
defaultprio:
data:
- SELECT * FROM assets WHERE count > 10;
`,
{ schema: BASIC_SCHEMA, ...PARSE_OPTIONS }
);

expect(rules.errors).toEqual([]);

expect(rules.getStaticBucketDescriptions(normalizeTokenParameters({}))).toEqual([
{ bucket: 'highprio[]', priority: 0 },
{ bucket: 'defaultprio[]', priority: 3 }
]);
});

test(`invalid priority on bucket`, () => {
expect(() =>
SqlSyncRules.fromYaml(
`
bucket_definitions:
highprio:
priority: instant
data:
- SELECT * FROM assets WHERE count <= 10;
`,
{ schema: BASIC_SCHEMA, ...PARSE_OPTIONS }
)
).toThrowError(/Invalid priority/);
});

test(`can't duplicate priority`, () => {
expect(() =>
SqlSyncRules.fromYaml(
`
bucket_definitions:
highprio:
priority: 1
parameters: SELECT 0 as _priority;
data:
- SELECT * FROM assets WHERE count <= 10;
`,
{ schema: BASIC_SCHEMA, ...PARSE_OPTIONS }
)
).toThrowError(/Cannot set priority multiple times/);
});
});

0 comments on commit efb834e

Please sign in to comment.