Quote ALTER SCHEMA SET values

[jacodegroothydrab]

update_schema builds its literal by hand:

new_value = f"'{new_value}'" if isinstance(new_value, str) else new_value

so an apostrophe ends the string early and the rest arrives as bare SQL:

001003 (42000): SQL compilation error: syntax error line 1 at position 39
unexpected 'comment'. on ALTER SCHEMA MY_DB.MY_SCHEMA SET comment = '...

Any schema comment containing an apostrophe fails to apply. CREATE is unaffected — it already renders through props.quote_value. update_schema was the one branch that hand-rolled its quoting; it now uses quote_value too.

quote_value had the mirror-image gap: $$ is a safe delimiter only while the value doesn't contain $$. It now falls back to a single-quoted literal with ' and backslash doubled in that case.

4 regression tests, all four failing against the current code.

[noel]

snowcap/props.py:36: The fallback for values containing $$ produces a single-quoted literal, but it only escapes apostrophes and backslashes. Actual control characters such as newlines remain unescaped, although Snowflake requires escape sequences inside single-quoted literals. This regresses multiline values that previously worked with dollar quoting.

Please escape control characters such as \n, \r, and \t after escaping backslashes, and add a regression test covering a value containing both $$ and a newline.

Also, please remove or substantially shorten the explanatory comment blocks in snowcap/lifecycle.py and snowcap/props.py; the code and regression tests already document the behavior clearly.

[jacodegroothydrab]

I have addressed your feedback

• Deleted the comment. Duplicate comment risks confusing LLMs if they diverge in the future.
• Escaped control characters
• Add regression tests

Please let me know if you require further changes.

[noel]

snowcap/props.py:35: The fallback now escapes newline, carriage return, and tab, but still emits backspace, form feed, NUL, and other C0 control characters directly into the SQL literal. Snowflake provides escape sequences for these characters, so the fallback is not yet safe for every string accepted by quote_value.

Rather than extending the replacement chain one character at a time, please use the existing json standard-library import to escape control characters, strip its surrounding quotes, and then double apostrophes—for example:

escaped = json.dumps(str(value), ensure_ascii=False)[1:-1].replace("'", "''")

Please add regression coverage containing $$ with \b, \f, and NUL. See Snowflake’s string-literal documentation.

[jacodegroothydrab]

Done — the fallback now uses json.dumps(str(value), ensure_ascii=False)[1:-1].replace("'", "''"), so the whole C0 range is escaped rather than the four characters the chain happened to list. Added regression tests for $$ with \b/\f and with NUL (json emits \u0000, which Snowflake accepts).

[noel]

The update_schema fix looks correct and is well covered by the new tests. A couple of things worth a look before merging:

• update_scanner_package (lifecycle.py:513) has the same unescaped f"'{new_value}'" pattern that was just fixed here, and quote_value is already imported two functions above it. A scanner-package value with an apostrophe (a cron schedule string, a comment) will still break the literal. Same root cause, same file, not covered by this PR. Worth fixing here too or explicitly scoping it out.
• The new $$ fallback in quote_value doubles single quotes but relies on Snowflake accepting JSON's \" for a literal double quote. That's correct per Snowflake's escape table, but there's no test with a " in the value, so it's an implicit assumption right now. A test would make it explicit.
• tests/test_props.py:128-171 has 11 separate test_quote_value_* methods that could collapse into one @pytest.mark.parametrize over (value, expected) pairs. Not blocking, just fewer lines to maintain.

[noel]

@jacodegroothydrab are you still interested in getting this merged?

[jacodegroothydrab]

@jacodegroothydrab are you still interested in getting this merged?

Yes, still interested. Apologies for the delay, I have been travelling and didn't spot your feedback

I will implement your improvements by EOD

[jacodegroothydrab]

Rebased on current main and addressed the 23 Sep review:

• update_scanner_package now renders both branches through quote_value, with a test for an apostrophe in the value. The existing enabled test asserts the $$TRUE$$ form it now emits.
• Added a " case to the $$ fallback tests, making the \" escape explicit.
• Collapsed the test_quote_value_* methods into one @pytest.mark.parametrize.

tests/test_props.py and tests/test_lifecycle.py pass locally; ruff check and format are clean.

[noel]

Nice fix for the embedded $$ case, but there's a boundary case it doesn't cover: quote_value only checks "$$" in str(value) before choosing the dollar-quote form. A value ending in a single trailing $ still breaks the dollar-quote, because that trailing $ combines with the literal's closing $$ to form an early $$.

Verified directly:

>>> quote_value("abc$")
'$$abc$$$'
>>> quote_value("$")
'$$$$$'

Both are unbalanced and reproduce the same syntax-error class this PR is fixing, just triggered by a trailing single $ instead of an embedded $$.

Suggested fix: also route the trailing-$ case to the single-quote fallback, e.g.

if "$$" in str(value) or str(value).endswith("$"):

Separately (lower confidence): the single-quote fallback escapes control characters using JSON-style sequences (backslash-u0000, backslash-b, backslash-f, etc.), assuming Snowflake decodes those identically to JSON. Might be worth confirming against a live account for the rarer ones (e.g. NUL), since only this PR's own test currently asserts that mapping is correct.