fix(athena-driver): Correct SQL parameter escaping

[ovr]

No description provided.

[claude]

Claude finished @ovr's task in 54s —— View job

---

Code review

Verdict: LGTM — clean parallel to #11345 (trino/prestodb), well-tested.

Detailed review (0 blocking, 0 medium, 2 nits)

Summary

Replaces sqlstring (MySQL-dialect, backslash-escaping) with formatAnsi from @cubejs-backend/shared (ANSI/standard SQL, no backslash escaping, '' for quotes). Athena runs on Trino/Presto and expects standard SQL, so this is the correct dialect. The previous code piled hacks on top of MySQL escaping (.replace(/\\\\([_%])/g, '\\$1').replace(/\\'/g, '\'\'')) which the tests explicitly cover as previously-broken cases (trailing backslash, array params).

Code quality

• applyParams is now a two-line pass-through around formatAnsi. Kept as a named export so the unit tests can hit it directly — reasonable.
• Removal of sqlstring from dependencies is correctly followed through; no remaining imports.
• The .replace(...) hack chain and the toSqlString wrapper trick around array param handling are gone. Array params now escape correctly (test at test/unit/params-escaping.test.ts:107 covers this — previously broken).

Security

• Standard-SQL escaping via formatAnsi doubles ' and does NOT interpret \, so foo\' OR 1=1 -- and payload\ (trailing backslash) now stay inside the literal. Both are covered by tests (lines 48, 60).
• sqlstring removed from deps — one fewer supply-chain surface.

Testing

Coverage is strong. Ten cases exercise: LIKE escape sequence preservation, no double-escape of backslashes, quote injection, backslash-then-quote, trailing backslash (previously broke the literal), literal %, unescaped % in LIKE, quotes+percent, arrays, and multi-placeholder ordering.

Nits (non-blocking)

1. packages/cubejs-athena-driver/package.json:19 — "unit": "jest --verbose dist/test/unit" runs the compiled JS. This works given tsconfig.json (rootDir: ".", include: ["src","test"]), but requires yarn tsc before yarn unit. Matches the existing integration:athena pattern, so consistent — just worth noting for CI wiring.
2. AthenaDriver.ts:70 — applyParams is now a trivial wrapper. Since formatAnsi is imported at the top, you could call it directly at the one call site (line 591) and drop the wrapper + its export. Keeping it is fine too — it's a stable seam for tests.

• [`fix/athena-param-escaper`](https://github.com/cube-js/cube/tree/fix/athena-param-escaper)

[claude]

Claude finished @ovr's task in 36s —— View job

---

Code review

Verdict: LGTM — 0 blocking, 0 medium, 2 nits. No changes since prior review (same single commit 8b78b68).

Detailed review

• Fetch and inspect the diff
• Verify no new commits since prior review
• Re-check code quality, security, testing

Summary

Replaces sqlstring (MySQL-dialect, backslash-escaping) with formatAnsi from @cubejs-backend/shared (ANSI/standard SQL — doubles ', does not interpret \). Athena runs on Trino/Presto and expects standard SQL, so this is the correct dialect. The previous code piled hacks on top of MySQL escaping (.replace(/\\\\([_%])/g, '\\$1').replace(/\\'/g, '\'\'')) which the new tests explicitly cover as previously-broken cases (trailing backslash, array params).

Code quality

• applyParams (AthenaDriver.ts:70) is now a two-line pass-through around formatAnsi. Kept as a named export so unit tests hit it directly — reasonable.
• Removal of sqlstring from dependencies is followed through; no remaining imports.
• The .replace(...) hack chain and the toSqlString wrapper trick for array params are gone. Arrays now escape correctly (test at test/unit/params-escaping.test.ts:107).

Security

• Standard-SQL escaping via formatAnsi doubles ' and does NOT interpret \, so foo\' OR 1=1 -- and payload\ (trailing backslash) stay inside the literal. Both covered by tests (lines 48, 60).
• sqlstring removed from deps — one fewer supply-chain surface.

Testing

Ten cases exercise: LIKE escape sequence preservation, no double-escape of backslashes, quote injection, backslash-then-quote, trailing backslash (previously broke the literal), literal %, unescaped % in LIKE, quotes+percent, arrays, and multi-placeholder ordering.

Nits (non-blocking)

1. packages/cubejs-athena-driver/package.json:19 — "unit": "jest --verbose dist/test/unit" runs the compiled JS, so requires yarn tsc before yarn unit. Matches the existing integration:athena pattern, so consistent — just worth noting for CI wiring.
2. AthenaDriver.ts:70 — applyParams is now a trivial wrapper; since formatAnsi is already imported, the one call site could invoke it directly and drop the wrapper. Keeping it is fine — stable seam for tests.

• fix/athena-param-escaper •

[ovr]

Similar issue to what I've fixed in trino/presto