Skip to content

Commit 2aa6af6

Browse files
committed
fix(snowflake): unload a table, not an inline query
The COPY INTO grammar places the source immediately before its copy options, so an inlined query sits one parenthesis from being able to rewrite them. Guarding that means matching Snowflake's tokenizer exactly, and three successive versions of the guard were each defeated: // line comments, $$ dollar quoting, and a bare carriage return, which the scanner did not treat as a line terminator but Snowflake does. Each fix was a guess at a lexer the public docs do not specify. Removes the inline-query source instead of guessing a fourth time. A table name goes through qualifiedIdentifier, which is provably safe. Exporting a query result now means materializing it first — a view, or CREATE TABLE AS SELECT via Execute SQL — which the tool description, the block skill and the docs all say. Also from the final audit: - optionalBoolean accepts the string forms a direct tool call delivers, matching the other boolean readers on this block, and its TSDoc no longer states the serializer rule backwards - the five JSON editors declare language: 'json', so invalid JSON is caught inline instead of at execution - bound the RESULT_SCAN read in SQL, not only by rows_per_resultset - pin every migration target to a live subblock id, for all blocks
1 parent d5ede2e commit 2aa6af6

11 files changed

Lines changed: 133 additions & 203 deletions

File tree

apps/docs/content/docs/en/integrations/snowflake-service-account.mdx

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,14 @@ With a credential selected, these fields become pickers backed by metadata-only
9696

9797
Each picker runs as the token's user under its **default** role — not the execution role set on the block — so an empty list is usually a privilege gap rather than an empty account. Switch any field to advanced mode to type a name directly or reference an upstream block's output instead.
9898

99+
<Callout type="info">
100+
**Unload Data exports a table, not a query.** The COPY INTO grammar places the
101+
source immediately before its options, so an inline query would sit one
102+
parenthesis away from being able to rewrite them. To export a query result,
103+
materialize it first — a view, or `CREATE TABLE AS SELECT` via Execute SQL —
104+
then unload that object.
105+
</Callout>
106+
99107
## Rotating and Revoking
100108

101109
A token's expiry is fixed at creation. To rotate, generate a new token on the same user and update the credential in Sim — the old one stays valid until you remove it. `ALTER USER ... REMOVE PROGRAMMATIC ACCESS TOKEN <name>` revokes immediately and cannot be undone.

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

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -373,7 +373,7 @@ Load files from an existing Snowflake stage with COPY INTO.
373373

374374
### Snowflake Unload Data
375375

376-
Export a table or query result to files in a Snowflake stage with COPY INTO.
376+
Export a Snowflake table to files in a stage with COPY INTO.
377377

378378
#### Input
379379

@@ -386,8 +386,7 @@ Export a table or query result to files in a Snowflake stage with COPY INTO.
386386
| `database` | string | Yes | Database name |
387387
| `schema` | string | Yes | Schema name |
388388
| `stagePath` | string | Yes | Destination stage reference, for example @EXPORTS/daily |
389-
| `table` | string | No | Source table to unload; provide either this or a statement, not both |
390-
| `statement` | string | No | Source SELECT statement to unload; provide either this or a table, not both |
389+
| `table` | string | Yes | Source table to unload. To export a query result, materialize it first as a view or with CREATE TABLE AS SELECT, then unload that |
391390
| `fileFormat` | string | No | Named file format applied to the unloaded files |
392391
| `header` | boolean | No | Whether to write column headings into the unloaded files; supported for CSV and Parquet only |
393392
| `overwrite` | boolean | No | Whether to replace existing files with matching names in the stage |

apps/sim/app/workspace/[workspaceId]/w/[workflowId]/hooks/use-wand.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -55,11 +55,15 @@ function buildWandContextInfo({
5555

5656
case 'json-schema':
5757
case 'json-object':
58+
case 'json-array':
5859
case 'table-schema':
5960
try {
6061
const parsed = JSON.parse(currentValue)
61-
const keys = Object.keys(parsed)
62-
contextInfo += `\n\nJSON analysis: Valid JSON with ${keys.length} top-level keys: ${keys.join(', ')}`
62+
// Reporting "top-level keys" for an array would list numeric indices,
63+
// which tells the model nothing about the shape it should produce.
64+
contextInfo += Array.isArray(parsed)
65+
? `\n\nJSON analysis: Valid JSON array with ${parsed.length} items`
66+
: `\n\nJSON analysis: Valid JSON with ${Object.keys(parsed).length} top-level keys: ${Object.keys(parsed).join(', ')}`
6367
} catch {
6468
contextInfo += `\n\nJSON analysis: Invalid JSON - needs fixing`
6569
}

apps/sim/blocks/blocks/snowflake.ts

Lines changed: 23 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -157,7 +157,7 @@ const tableOperations = [
157157
'list_copy_history',
158158
'introspect_schema',
159159
] as const
160-
const tableRequiredOperations = [...dataOperations, 'list_copy_history'] as const
160+
const tableRequiredOperations = [...dataOperations, 'unload_data', 'list_copy_history'] as const
161161

162162
/** Operations that bound their result with a `limit` parameter. */
163163
const limitOperations = [
@@ -192,13 +192,22 @@ const maxRowsOperationSet: ReadonlySet<string> = new Set(maxRowsOperations)
192192
const limitOperationSet: ReadonlySet<string> = new Set(limitOperations)
193193

194194
/**
195-
* A switch the user never touched serializes as `null`, and in advanced mode the
196-
* serializer emits every advanced sub-block regardless. Builders test optional
197-
* booleans with `!== undefined`, so an untouched switch would otherwise emit a
198-
* clause the user never asked for — `AUTO_RESUME = FALSE` being the damaging one.
195+
* Normalizes an optional switch to a real boolean or `undefined`.
196+
*
197+
* With advanced mode on, the serializer evaluates each advanced sub-block's
198+
* condition and emits an untouched switch as `null`. Builders test these with
199+
* `!== undefined`, so that `null` would otherwise emit a clause the user never
200+
* asked for — `AUTO_RESUME = FALSE`, which permanently disables auto-resume on
201+
* the warehouse, being the damaging one.
202+
*
203+
* The string forms are accepted because a direct tool call delivers booleans
204+
* that way, matching the other boolean readers on this block.
199205
*/
200206
function optionalBoolean(value: unknown): boolean | undefined {
201-
return typeof value === 'boolean' ? value : undefined
207+
if (typeof value === 'boolean') return value
208+
if (value === 'true') return true
209+
if (value === 'false') return false
210+
return undefined
202211
}
203212

204213
function resolveCopyOnError(value: unknown, threshold: unknown): string | undefined {
@@ -294,12 +303,12 @@ export const SnowflakeBlock: BlockConfig<SnowflakeStatementResponse> = {
294303
title: 'SQL Statement',
295304
type: 'code',
296305
placeholder: 'SELECT * FROM ANALYTICS.PUBLIC.EVENTS LIMIT 100',
297-
condition: { field: 'operation', value: ['execute_sql', 'unload_data'] },
306+
condition: { field: 'operation', value: 'execute_sql' },
298307
required: { field: 'operation', value: 'execute_sql' },
299308
wandConfig: {
300309
enabled: true,
301310
prompt:
302-
'Generate one Snowflake SQL statement for the described request. For Execute SQL, use positional ? placeholders for any values that will be bound; for Unload Data, write a complete SELECT with literal values and no placeholders. Return ONLY the SQL statement - no explanations, no extra text.',
311+
'Generate one Snowflake SQL statement for the described request. Use positional ? placeholders for any values that will be bound. Return ONLY the SQL statement - no explanations, no extra text.',
303312
placeholder: 'Describe the query to run...',
304313
generationType: 'sql-query',
305314
},
@@ -308,6 +317,7 @@ export const SnowflakeBlock: BlockConfig<SnowflakeStatementResponse> = {
308317
id: 'bindings',
309318
title: 'Bindings',
310319
type: 'code',
320+
language: 'json',
311321
placeholder: '{"1":{"type":"TEXT","value":"active"}}',
312322
condition: { field: 'operation', value: 'execute_sql' },
313323
mode: 'advanced',
@@ -423,6 +433,7 @@ export const SnowflakeBlock: BlockConfig<SnowflakeStatementResponse> = {
423433
id: 'rows',
424434
title: 'Rows',
425435
type: 'code',
436+
language: 'json',
426437
placeholder: '[{"id":1,"status":"active"}]',
427438
condition: { field: 'operation', value: ['insert_rows', 'update_rows', 'upsert_rows'] },
428439
required: { field: 'operation', value: ['insert_rows', 'update_rows', 'upsert_rows'] },
@@ -438,6 +449,7 @@ export const SnowflakeBlock: BlockConfig<SnowflakeStatementResponse> = {
438449
id: 'matchColumns',
439450
title: 'Match Columns',
440451
type: 'code',
452+
language: 'json',
441453
placeholder: '["id"]',
442454
condition: { field: 'operation', value: ['update_rows', 'upsert_rows'] },
443455
required: { field: 'operation', value: ['update_rows', 'upsert_rows'] },
@@ -453,6 +465,7 @@ export const SnowflakeBlock: BlockConfig<SnowflakeStatementResponse> = {
453465
id: 'filters',
454466
title: 'Match Filters',
455467
type: 'code',
468+
language: 'json',
456469
placeholder: '{"status":"expired","tenant_id":42,"archived_at":null}',
457470
condition: { field: 'operation', value: 'delete_rows' },
458471
required: { field: 'operation', value: 'delete_rows' },
@@ -773,6 +786,7 @@ export const SnowflakeBlock: BlockConfig<SnowflakeStatementResponse> = {
773786
id: 'procedureArguments',
774787
title: 'Procedure Arguments',
775788
type: 'code',
789+
language: 'json',
776790
placeholder: '[{"type":"TEXT","value":"daily"}]',
777791
condition: { field: 'operation', value: 'call_procedure' },
778792
mode: 'advanced',
@@ -1210,7 +1224,7 @@ export const SnowflakeBlockMeta = {
12101224
name: 'export-snowflake-results',
12111225
description: 'Unload a table or query result to files in a Snowflake stage.',
12121226
content:
1213-
'# Export Snowflake Results\n\n## Steps\n1. Confirm the destination stage path and whether existing files may be overwritten.\n2. Choose exactly one source: a table, or a SELECT statement.\n3. Pick a named file format, and decide on column headings and whether one file or several.\n4. Run the unload and review the reported file count and row totals.\n\n## Output\nReturn the stage path, files written, and rows unloaded.',
1227+
'# Export Snowflake Results\n\n## Steps\n1. Confirm the destination table and the stage path, and whether existing files may be overwritten.\n2. To export a query result rather than a whole table, materialize it first — create a view or use CREATE TABLE AS SELECT via Execute SQL — then unload that object.\n3. Pick a named file format, and decide on column headings and whether one file or several.\n4. Run the unload and review the reported file count and row totals.\n\n## Output\nReturn the stage path, files written, and rows unloaded.',
12141228
},
12151229
{
12161230
name: 'browse-snowflake-objects',

apps/sim/lib/integrations/integrations.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18286,7 +18286,7 @@
1828618286
},
1828718287
{
1828818288
"name": "Unload Data",
18289-
"description": "Export a table or query result to files in a Snowflake stage with COPY INTO."
18289+
"description": "Export a Snowflake table to files in a stage with COPY INTO."
1829018290
},
1829118291
{
1829218292
"name": "List Databases",

apps/sim/lib/workflows/migrations/subblock-migrations.test.ts

Lines changed: 62 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,18 @@
22
* @vitest-environment node
33
*/
44
import { afterAll, describe, expect, it, vi } from 'vitest'
5+
import { getAllBlocks } from '@/blocks/registry'
56
import type { BlockState } from '@/stores/workflows/workflow/types'
67

78
vi.unmock('@/blocks/registry')
89

910
import * as blocksBarrel from '@/blocks'
1011
import { getBlock as getRealBlock } from '@/blocks/registry'
11-
import { backfillCanonicalModes, migrateSubblockIds } from './subblock-migrations'
12+
import {
13+
backfillCanonicalModes,
14+
migrateSubblockIds,
15+
SUBBLOCK_ID_MIGRATIONS,
16+
} from './subblock-migrations'
1217

1318
/**
1419
* Under `isolate: false` the module under test may already be cached from an
@@ -35,6 +40,47 @@ function makeBlock(overrides: Partial<BlockState> & { type: string }): BlockStat
3540
} as BlockState
3641
}
3742

43+
/**
44+
* `dropParkedSubblocks` deletes any subblock whose id starts with `_removed_`,
45+
* on the assumption that no live block declares one. Nothing enforces that
46+
* naming rule at the block level, so pin it here — a block that adopted the
47+
* prefix for a real field would have its value silently deleted on every load.
48+
*/
49+
describe('_removed_ prefix invariant', () => {
50+
it('is never used as a live subblock id', () => {
51+
const offenders = getAllBlocks().flatMap((block) =>
52+
(block.subBlocks ?? [])
53+
.filter((subBlock) => subBlock.id.startsWith('_removed_'))
54+
.map((subBlock) => `${block.type}.${subBlock.id}`)
55+
)
56+
expect(offenders).toEqual([])
57+
})
58+
})
59+
60+
/**
61+
* A migration target that names no live subblock silently drops the value: the
62+
* rename writes a key nothing reads, and the sweep or the serializer discards
63+
* it. Nothing else checks the right-hand side of the map.
64+
*/
65+
describe('migration targets', () => {
66+
it('every rename points at a subblock that still exists', () => {
67+
const offenders: string[] = []
68+
for (const [blockType, renames] of Object.entries(SUBBLOCK_ID_MIGRATIONS)) {
69+
const config = getAllBlocks().find((block) => block.type === blockType)
70+
if (!config) {
71+
offenders.push(`${blockType} (block not registered)`)
72+
continue
73+
}
74+
const liveIds = new Set((config.subBlocks ?? []).map((subBlock) => subBlock.id))
75+
for (const [legacyId, currentId] of Object.entries(renames)) {
76+
if (currentId.startsWith('_removed_')) continue
77+
if (!liveIds.has(currentId)) offenders.push(`${blockType}.${legacyId} -> ${currentId}`)
78+
}
79+
}
80+
expect(offenders).toEqual([])
81+
})
82+
})
83+
3884
describe('migrateSubblockIds', () => {
3985
it('should preserve Instagram insight metrics after the subblock rename', () => {
4086
const input: Record<string, BlockState> = {
@@ -68,9 +114,14 @@ describe('migrateSubblockIds', () => {
68114
type: 'snowflake',
69115
subBlocks: {
70116
operation: { id: 'operation', type: 'dropdown', value: 'insert_rows' },
71-
database: { id: 'database', type: 'short-input', value: 'ANALYTICS' },
72-
schema: { id: 'schema', type: 'short-input', value: 'PUBLIC' },
73-
table: { id: 'table', type: 'short-input', value: 'EVENTS' },
117+
// Every legacy id in the map, so a rename added later without a
118+
// matching assertion still fails here.
119+
...Object.fromEntries(
120+
Object.keys(SUBBLOCK_ID_MIGRATIONS.snowflake).map((legacyId) => [
121+
legacyId,
122+
{ id: legacyId, type: 'short-input', value: `value-${legacyId}` },
123+
])
124+
),
74125
},
75126
}),
76127
}
@@ -80,10 +131,13 @@ describe('migrateSubblockIds', () => {
80131
expect(migrated).toBe(true)
81132
// The advanced text members, not the pickers: a migrated block has no
82133
// credential yet, so a picker could not hydrate the stored name.
83-
expect(blocks.b1.subBlocks.databaseName?.value).toBe('ANALYTICS')
84-
expect(blocks.b1.subBlocks.schemaName?.value).toBe('PUBLIC')
85-
expect(blocks.b1.subBlocks.tableName?.value).toBe('EVENTS')
86-
expect(blocks.b1.subBlocks.database).toBeUndefined()
134+
for (const [legacyId, currentId] of Object.entries(SUBBLOCK_ID_MIGRATIONS.snowflake)) {
135+
if (currentId.startsWith('_removed_')) continue
136+
expect(blocks.b1.subBlocks[currentId]?.value, `${legacyId} -> ${currentId}`).toBe(
137+
`value-${legacyId}`
138+
)
139+
expect(blocks.b1.subBlocks[legacyId], legacyId).toBeUndefined()
140+
}
87141
})
88142

89143
/**

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

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

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

Lines changed: 18 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -482,7 +482,14 @@ describe('Snowflake SQL builders', () => {
482482
)
483483
})
484484

485-
it('unloads from exactly one source and orders the COPY INTO clauses', () => {
485+
/**
486+
* The source is a table name, never an inline query. An inlined query sits
487+
* directly before the copy-option slot, so anything escaping its parentheses
488+
* becomes a copy clause — a guard for that has to match Snowflake's tokenizer
489+
* exactly, and three versions of one were each defeated. `qualifiedIdentifier`
490+
* removes the class instead of re-guarding it.
491+
*/
492+
it('unloads a table and orders the COPY INTO clauses', () => {
486493
const base = {
487494
...context,
488495
database: 'ANALYTICS',
@@ -500,68 +507,19 @@ describe('Snowflake SQL builders', () => {
500507
maxFileSizeBytes: 16_777_216,
501508
}).statement
502509
).toBe(
510+
// OVERWRITE/SINGLE/MAX_FILE_SIZE are all copyOptions members, so their
511+
// order among themselves is free; HEADER must stay last.
503512
"COPY INTO @EXPORTS/daily FROM ANALYTICS.PUBLIC.EVENTS FILE_FORMAT = (FORMAT_NAME = 'ANALYTICS.PUBLIC.CSV_FORMAT') OVERWRITE = FALSE SINGLE = TRUE MAX_FILE_SIZE = 16777216 HEADER = TRUE"
504513
)
505-
// A trailing semicolon on the source query would close the COPY statement.
506-
// The newline before the closing paren keeps a trailing line comment in the
507-
// source query from swallowing it.
508-
expect(buildUnloadData({ ...base, statement: 'SELECT 1 -- daily' }).statement).toBe(
509-
'COPY INTO @EXPORTS/daily FROM (SELECT 1 -- daily\n) OVERWRITE = FALSE'
510-
)
511-
expect(buildUnloadData({ ...base, statement: 'SELECT 1;' }).statement).toBe(
512-
'COPY INTO @EXPORTS/daily FROM (SELECT 1\n) OVERWRITE = FALSE'
513-
)
514-
expect(() => buildUnloadData({ ...base, table: 'EVENTS', statement: 'SELECT 1' })).toThrow(
515-
/exactly one of table or statement/
516-
)
517-
// Each of these ends the derived table early and supplies its own copy
518-
// options, including OVERWRITE = TRUE, by hiding a paren in a construct the
519-
// paren counter must skip. All are valid Snowflake syntax.
520-
const breakouts: Array<[string, string, RegExp]> = [
521-
[
522-
'bare paren',
523-
'SELECT 1) OVERWRITE = TRUE FILE_FORMAT = (TYPE = CSV',
524-
/unbalanced parentheses/,
525-
],
526-
[
527-
'double-slash comment',
528-
'SELECT 1 // (\n) OVERWRITE = TRUE FILE_FORMAT = (TYPE = CSV // )',
529-
/unbalanced parentheses/,
530-
],
531-
[
532-
'dollar quoting',
533-
'SELECT $$($$ ) OVERWRITE = TRUE FILE_FORMAT = (TYPE = CSV, RECORD_DELIMITER = $$)$$',
534-
/unbalanced parentheses/,
535-
],
536-
[
537-
'nested block comment',
538-
'SELECT 1 /* /* */ ( */ ) OVERWRITE = TRUE /* /* */ ) */',
539-
/nested block comment/,
540-
],
541-
]
542-
for (const [name, statement, expected] of breakouts) {
543-
expect(() => buildUnloadData({ ...base, statement }), name).toThrow(expected)
544-
}
545-
546-
// A paren legitimately inside a dollar-quoted string is not a breakout.
547-
expect(buildUnloadData({ ...base, statement: 'SELECT $$a)b$$ AS x' }).statement).toContain(
548-
'FROM (SELECT $$a)b$$ AS x\n)'
549-
)
550-
expect(() => buildUnloadData({ ...base, statement: 'SELECT $$unterminated' })).toThrow(
551-
/unterminated dollar-quoted string/
552-
)
553-
554-
// OVERWRITE is always emitted, so an injected duplicate collides instead of
555-
// silently replacing staged files.
556-
expect(buildUnloadData({ ...base, table: 'EVENTS' }).statement).toContain('OVERWRITE = FALSE')
557-
expect(() => buildUnloadData({ ...base, statement: 'DROP TABLE EVENTS' })).toThrow(
558-
/must be a SELECT or WITH query/
514+
// OVERWRITE is always emitted so the option can never be left to a default.
515+
expect(buildUnloadData({ ...base, table: 'EVENTS' }).statement).toBe(
516+
'COPY INTO @EXPORTS/daily FROM ANALYTICS.PUBLIC.EVENTS OVERWRITE = FALSE'
559517
)
560-
// Parens and quotes inside string literals and comments are not miscounted.
561-
expect(
562-
buildUnloadData({ ...base, statement: "SELECT ')' AS a -- )\nFROM T" }).statement
563-
).toContain("FROM (SELECT ')' AS a -- )\nFROM T\n)")
564-
expect(() => buildUnloadData(base)).toThrow(/exactly one of table or statement/)
518+
expect(() => buildUnloadData({ ...base, table: '' })).toThrow(/table is required/)
519+
// No SQL text can reach the statement, so no breakout is expressible.
520+
expect(() =>
521+
buildUnloadData({ ...base, table: 'EVENTS) OVERWRITE = TRUE FILE_FORMAT = (TYPE = CSV' })
522+
).toThrow(/Invalid Snowflake identifier/)
565523
expect(() =>
566524
buildUnloadData({ ...base, table: 'EVENTS', maxFileSizeBytes: 5_368_709_121 })
567525
).toThrow(/maxFileSizeBytes/)

0 commit comments

Comments
 (0)