Skip to content

Commit 36b3aa3

Browse files
committed
fix(snowflake): emit task history time bounds as literals
TASK_HISTORY only accepts bind variables for RESULT_LIMIT and TASK_NAME per BCR-1410, and that change explicitly excludes a bind passed through another function first. A bind in SCHEDULED_TIME_RANGE_START/END is therefore dropped without an error, so the requested window became a no-op and the function fell back to returning the most recent runs. Emit validated literals instead, which also restores Snowflake's seven-day range error. Also reject a fractional skip-file percentage at the block boundary rather than in the builder, and correct the cancel description: a cancelled child marks the task graph run failed, so downstream tasks are skipped rather than continuing.
1 parent 04e9a87 commit 36b3aa3

8 files changed

Lines changed: 53 additions & 20 deletions

File tree

apps/docs/content/docs/en/integrations/snowflake.mdx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -785,7 +785,7 @@ Find one task history record by query ID within Snowflake’s seven-day window a
785785

786786
### Snowflake Cancel Task Query
787787

788-
Cancel one running task query by query ID with SYSTEM$CANCEL_QUERY. Other tasks in the same task graph keep running and must be cancelled separately.
788+
Cancel one running task query by query ID with SYSTEM$CANCEL_QUERY. Task runs already in flight are unaffected and must be cancelled individually; a cancelled child marks the task graph run failed, so downstream tasks are skipped.
789789

790790
#### Input
791791

apps/sim/blocks/blocks/snowflake.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -70,8 +70,8 @@ function resolveCopyOnError(value: unknown, threshold: unknown): string | undefi
7070
if (number === undefined || number <= 0) {
7171
throw new Error('Skip file threshold must be greater than zero')
7272
}
73-
if (value === 'SKIP_FILE_PERCENT' && number > 100) {
74-
throw new Error('Skip file percentage must be between 1 and 100')
73+
if (value === 'SKIP_FILE_PERCENT' && (number > 100 || !Number.isInteger(number))) {
74+
throw new Error('Skip file percentage must be a whole number between 1 and 100')
7575
}
7676
if (value === 'SKIP_FILE_NUMBER' && !Number.isInteger(number)) {
7777
throw new Error('Skip file error count must be a positive integer')

apps/sim/lib/integrations/integrations.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18322,7 +18322,7 @@
1832218322
},
1832318323
{
1832418324
"name": "Cancel Task Query",
18325-
"description": "Cancel one running task query by query ID with SYSTEM$CANCEL_QUERY. Other tasks in the same task graph keep running and must be cancelled separately."
18325+
"description": "Cancel one running task query by query ID with SYSTEM$CANCEL_QUERY. Task runs already in flight are unaffected and must be cancelled individually; a cancelled child marks the task graph run failed, so downstream tasks are skipped."
1832618326
},
1832718327
{
1832818328
"name": "Get Task Run Output",

apps/sim/tools/generated/tool-metadata.ts

Lines changed: 1 addition & 1 deletion
Large diffs are not rendered by default.

apps/sim/tools/snowflake/cancel_task_run.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ export const cancelTaskRunTool: ToolConfig<
1919
version: '1.0.0',
2020
name: 'Snowflake Cancel Task Query',
2121
description:
22-
'Cancel one running task query by query ID with SYSTEM$CANCEL_QUERY. Other tasks in the same task graph keep running and must be cancelled separately.',
22+
'Cancel one running task query by query ID with SYSTEM$CANCEL_QUERY. Task runs already in flight are unaffected and must be cancelled individually; a cancelled child marks the task graph run failed, so downstream tasks are skipped.',
2323
params: {
2424
host: {
2525
type: 'string',

apps/sim/tools/snowflake/sql.test.ts

Lines changed: 26 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -422,27 +422,46 @@ describe('Snowflake SQL builders', () => {
422422
limit: 50,
423423
})
424424
expect(history.statement).toContain('RESULT_LIMIT => 50, ERROR_ONLY => TRUE')
425-
expect(history.bindings).toEqual({
426-
'1': { type: 'TEXT', value: 'DAILY_LOAD' },
427-
'2': { type: 'TEXT', value: '2026-08-01T00:00:00Z' },
428-
})
425+
expect(history.bindings).toEqual({ '1': { type: 'TEXT', value: 'DAILY_LOAD' } })
429426
const run = buildGetTaskRun({
430427
...context,
431428
queryId,
432429
taskName: 'DAILY_LOAD',
433430
startTime: '2026-08-01T00:00:00Z',
434431
})
435432
expect(run.statement).toContain('TASK_NAME => ?')
436-
expect(run.statement).toContain('SCHEDULED_TIME_RANGE_START => TO_TIMESTAMP_LTZ(?)')
437433
expect(run.statement).toContain('WHERE QUERY_ID = ?')
438434
expect(run.bindings).toEqual({
439435
'1': { type: 'TEXT', value: 'DAILY_LOAD' },
440-
'2': { type: 'TEXT', value: '2026-08-01T00:00:00Z' },
441-
'3': { type: 'TEXT', value: queryId },
436+
'2': { type: 'TEXT', value: queryId },
442437
})
443438
expect(() =>
444439
buildListTaskRuns({ ...context, taskName: 'ANALYTICS.PUBLIC.DAILY_LOAD' })
445440
).toThrow('unqualified task name')
441+
442+
/**
443+
* TASK_HISTORY silently drops a bind in its time-range arguments, so the window has to
444+
* reach Snowflake as a literal or the filter becomes a no-op with no error.
445+
*/
446+
const window = buildListTaskRuns({
447+
...context,
448+
startTime: '2026-08-01T00:00:00Z',
449+
endTime: '2026-08-02T00:00:00Z',
450+
})
451+
expect(window.statement).toContain(
452+
"SCHEDULED_TIME_RANGE_START => TO_TIMESTAMP_LTZ('2026-08-01T00:00:00Z')"
453+
)
454+
expect(window.statement).toContain(
455+
"SCHEDULED_TIME_RANGE_END => TO_TIMESTAMP_LTZ('2026-08-02T00:00:00Z')"
456+
)
457+
expect(window.statement).not.toContain('TO_TIMESTAMP_LTZ(?)')
458+
expect(window.bindings).toEqual({})
459+
expect(() => buildListTaskRuns({ ...context, startTime: 'not-a-timestamp' })).toThrow(
460+
'startTime must be an ISO-8601 timestamp'
461+
)
462+
expect(() => buildGetTaskRun({ ...context, queryId, endTime: 'nope' })).toThrow(
463+
'endTime must be an ISO-8601 timestamp'
464+
)
446465
expect(() =>
447466
buildGetTaskRun({ ...context, queryId, taskName: 'ANALYTICS.PUBLIC.DAILY_LOAD' })
448467
).toThrow('unqualified task name')

apps/sim/tools/snowflake/sql.ts

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -533,6 +533,20 @@ export function buildRunTask(params: SnowflakeRunTaskParams): SnowflakeStatement
533533
* yields an empty result set rather than an error. The name is also resolved the way Snowflake
534534
* stored it, so an unquoted name is upper-cased and a quoted one keeps its exact spelling.
535535
*/
536+
/**
537+
* TASK_HISTORY time-range arguments are `constant_expr` and are not in BCR-1410's bind
538+
* allowlist for this function, which covers only RESULT_LIMIT and TASK_NAME. A bind here
539+
* is silently dropped rather than rejected, which would turn the requested window into a
540+
* no-op, so the timestamp is emitted as a literal instead.
541+
*/
542+
function taskHistoryTimestamp(value: string, field: string): string {
543+
const trimmed = value.trim()
544+
if (Number.isNaN(Date.parse(trimmed))) {
545+
throw new Error(`${field} must be an ISO-8601 timestamp within the last seven days`)
546+
}
547+
return `TO_TIMESTAMP_LTZ(${stringLiteral(trimmed)})`
548+
}
549+
536550
function unqualifiedTaskName(value: string): string {
537551
const trimmed = value.trim()
538552
if (splitQualifiedIdentifier(trimmed).length > 1) {
@@ -552,11 +566,11 @@ export function buildListTaskRuns(params: SnowflakeListTaskRunsParams): Snowflak
552566
}
553567
if (params.startTime?.trim()) {
554568
args.push(
555-
`SCHEDULED_TIME_RANGE_START => TO_TIMESTAMP_LTZ(${binds.add(params.startTime.trim())})`
569+
`SCHEDULED_TIME_RANGE_START => ${taskHistoryTimestamp(params.startTime, 'startTime')}`
556570
)
557571
}
558572
if (params.endTime?.trim()) {
559-
args.push(`SCHEDULED_TIME_RANGE_END => TO_TIMESTAMP_LTZ(${binds.add(params.endTime.trim())})`)
573+
args.push(`SCHEDULED_TIME_RANGE_END => ${taskHistoryTimestamp(params.endTime, 'endTime')}`)
560574
}
561575
return {
562576
statement: `SELECT * FROM TABLE(SNOWFLAKE.INFORMATION_SCHEMA.TASK_HISTORY(${args.join(', ')})) ORDER BY SCHEDULED_TIME DESC`,
@@ -572,11 +586,11 @@ export function buildGetTaskRun(params: SnowflakeGetTaskRunParams): SnowflakeSta
572586
}
573587
if (params.startTime?.trim()) {
574588
args.push(
575-
`SCHEDULED_TIME_RANGE_START => TO_TIMESTAMP_LTZ(${binds.add(params.startTime.trim())})`
589+
`SCHEDULED_TIME_RANGE_START => ${taskHistoryTimestamp(params.startTime, 'startTime')}`
576590
)
577591
}
578592
if (params.endTime?.trim()) {
579-
args.push(`SCHEDULED_TIME_RANGE_END => TO_TIMESTAMP_LTZ(${binds.add(params.endTime.trim())})`)
593+
args.push(`SCHEDULED_TIME_RANGE_END => ${taskHistoryTimestamp(params.endTime, 'endTime')}`)
580594
}
581595
const queryId = binds.add(requireQueryId(params.queryId))
582596
return {

apps/sim/tools/snowflake/utils.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -69,9 +69,9 @@ interface SnowflakeApiResponse {
6969
statementHandle?: string
7070
data?: Array<Array<string | null>>
7171
/**
72-
* The SQL API reference and OpenAPI spec both declare `stats` as a direct
73-
* property of the ResultSet object. The `resultSetMetaData` fallback below is
74-
* defensive only - no documented response uses it.
72+
* The SQL API reference declares `stats` as a direct property of the ResultSet
73+
* object, and also describes it under `resultSetMetaData`. Snowflake's own docs
74+
* are inconsistent here, so both shapes are read with the top-level one winning.
7575
*/
7676
stats?: SnowflakeApiStats
7777
resultSetMetaData?: {

0 commit comments

Comments
 (0)