chore(cubesql): Upgrade sqlparser to 0.62.0

[ovr]

No description provided.

[claude]

Claude finished @ovr's task in 2m 46s —— View job

---

Claude PR Review

Mechanical sqlparser-0.62 + datafusion-fork-bump migration; structurally clean. Verdict: approve with nits — 4 low-severity findings, all defensive hardening rather than correctness regressions.

Details

Checklist

• Read PR context
• Check existing review threads (none from claude)
• Analyze changed files
• Post review findings

Inline findings (4 low)

1. shim.rs:1419 — DECLARE silently drops extra names. Mirror the stmts.len() != 1 guard for symmetry.
2. router.rs:617 / :665 — temp-table name fallback uses ObjectNamePart::to_string() if as_ident() is None; in that case the create- and drop-key paths could diverge. Recommend CompilationError::internal instead of fallback rendering.
3. router.rs:336 — Set::ParenthesizedAssignments uses zip, silently truncating on length mismatch. Validate arity before zipping.
4. statement.rs:1049 — SqlParser062Normalizer::replace unwrap()s a fallible visit. Safe today (only transform_expr is overridden), brittle later — consider Infallible or surfacing the error.

General observations / things I checked and liked

• SqlParser062Normalizer is a nicely localized adapter for the parser-version drift; transform_expr hook is a clean addition.
• The Box::pin(execute_query(...)) workaround in test_date_add_sub_postgres (compile/mod.rs:4034) is well-documented (~2 MB frame from inline futures); the #[inline(never)] split-out in plan_normalize.rs similarly addresses stack pressure with a clear chore commit.
• The Expr::Value(UnicodeStringLiteral(...)) → SingleQuotedString downgrade and the EXTRACT('YEAR' …) quote-style stripping are well-commented.
• The two new parser tests in parser.rs (missing comma; dangling FROM) cover the playground regression motivation.

Not covered

• I did not build/test the workspace — heavy Rust compile, no obvious need given the diff is mostly mechanical AST shape changes and existing tests + snapshots are the regression gate. Recommend running cargo test + cargo insta review locally before merging.
• No documentation changes needed (this is internal).

[claude]

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

---

Claude PR Review (re-run on latest)

Re-reviewed at c05bb80. Latest commits since the previous pass are only chore: drop useless comment and the earlier stack/cast fixes; no material change to the four files my prior inline findings target. All 4 prior low-severity threads still apply and remain unresolved.

Verdict: approve with nits — no new findings.

Details

Checklist

• Pulled latest branch state (c05bb80)
• Diffed against origin/master — no changes to shim.rs, router.rs, statement.rs since previous review
• Re-affirmed 4 prior unresolved inline threads still apply
• No new issues introduced by 7030170 (stack reduction), 1181636 (unsigned cast normalize), c05bb80 (comment drop)

Re-affirmed prior threads (still unresolved, still applicable)

1. shim.rs:1428 — DECLARE silently drops extra cursor names; mirror the stmts.len() != 1 guard.
2. router.rs:617 — temp-table key fallback via ObjectNamePart::to_string() could diverge from drop-path key; prefer CompilationError::internal.
3. router.rs:357 — Set::ParenthesizedAssignments zip silently truncates on arity mismatch.
4. statement.rs:1049 — SqlParser062Normalizer::replace unwrap()s a fallible visit; safe today, brittle later.

Notes on the new commits

• 7030170 fix(cubesql): PlanNormalize - reduce stack usage — splitting plan_normalize.rs arms into #[inline(never)] helpers is a targeted fix for the ~2 MB frame issue and looks clean.
• 1181636 fix(cubesql): Normalize unsigned integer cast aliases — the SqlParser062Normalizer::transform_expr rewrite of UInt*/UnsignedInt* cast types to Custom("UInt*") aliases is well-scoped to the DataFusion fork's expected naming; the unit-test coverage in parser.rs covers the playground regression motivation.
• c05bb80 chore: drop useless comment — no-op.

Not covered

• I did not run cargo test / cargo insta review (heavy Rust compile). Recommend running locally before merge.

· branch [`chore/cubesql-upgrade-df`](https://github.com/cube-js/cube/tree/chore/cubesql-upgrade-df)

[claude]

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

---

Claude PR Review (re-run on 86a90c5)

Re-reviewed after 86a90c5 chore: handle review comment. The new commit addresses the DECLARE multi-name concern in shim.rs by adding an explicit FeatureNotSupported guard before pulling the single name. I've resolved that thread.

Verdict: approve with nits — 3 prior low-severity threads remain unresolved.

Details

Checklist

• Pulled latest branch state (86a90c5)
• Listed existing review threads
• Verified 86a90c5 addresses the shim.rs DECLARE multi-name finding
• Resolved the now-addressed thread (PRRT_kwDOCOH19M6GI43d)
• Re-checked remaining threads against current diff — still applicable
• No new findings on the new commit

Addressed in 86a90c5

• shim.rs:1419 DECLARE multi-name guard — fixed: added an explicit if names.len() > 1 returning FeatureNotSupported, mirroring the stmts.len() != 1 guard above. Thread resolved.

Re-affirmed prior threads (still unresolved, still applicable)

1. router.rs:617 — temp-table key fallback via ObjectNamePart::to_string() could diverge from the drop-path key; prefer CompilationError::internal.
2. router.rs:357 — Set::ParenthesizedAssignments zip silently truncates on arity mismatch.
3. statement.rs:1049 — SqlParser062Normalizer::replace unwrap()s a fallible visit; safe today, brittle later.

Not covered

• I did not run cargo test / cargo insta review (heavy Rust compile). Recommend running locally before merge.

· branch chore/cubesql-upgrade-df ·

[claude]

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

---

Claude PR Review (re-run on 379ac1d)

Re-reviewed after 379ac1d fix(cubesql): Validate arity in SET (...) = (...) assignments. The new commit addresses the ParenthesizedAssignments arity finding by returning a CompilationError::user when variables.len() != values.len(), instead of silently truncating via zip. I've resolved that thread.

Verdict: approve with nits — 1 prior low-severity thread remains unresolved.

Details

Checklist

• Pulled latest branch state (379ac1d)
• Listed existing review threads
• Verified 379ac1d addresses the router.rs ParenthesizedAssignments arity finding
• Resolved the now-addressed thread (PRRT_kwDOCOH19M6GI5Oc)
• Re-checked remaining thread against current diff — still applicable
• No new findings on the new commit

Addressed in 379ac1d

• router.rs:336 SET arity validation — fixed: explicit if variables.len() != values.len() guard returning CompilationError::user before the zip, matching the suggested fix. Thread resolved.

Status of prior threads

Resolved (now or previously):

• shim.rs:1441 DECLARE multi-name guard — addressed in 86a90c5
• statement.rs:1049 visitor unwrap() — withdrawn (consistency with sibling visitors)
• router.rs:336 SET arity — addressed in 379ac1d (this commit)

Still unresolved, still applicable:

1. router.rs:617 — temp-table key fallback via ObjectNamePart::to_string() could diverge from the drop-path key; prefer CompilationError::internal over the to_string() fallback.

Not covered

• I did not run cargo test / cargo insta review (heavy Rust compile). Recommend running locally before merge.

· branch chore/cubesql-upgrade-df ·

[claude]

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

---

Claude PR Review (re-run on e7e220a)

Re-reviewed after e7e220a chore: handle review comment. The new commit addresses the last remaining inline finding — temp-table name fallback in router.rs — by returning CompilationError::internal on the non-ident branch instead of to_string()ing the whole ObjectNamePart. Both create- and drop-paths now fail loudly and symmetrically. I've resolved that thread.

Verdict: approve — all prior inline findings are now resolved or withdrawn.

Details

Status of prior threads

All resolved or withdrawn:

• shim.rs:1441 DECLARE multi-name guard — addressed in 86a90c5
• router.rs:366 SET arity validation — addressed in 379ac1d
• router.rs:626 temp-table key fallback — addressed in e7e220a (this commit)
• statement.rs:1049 visitor unwrap() — withdrawn (matches sibling visitors ApproximateCountDistinctVisitor, SensitiveDataSanitizer, etc.)

Addressed in e7e220a

Both the create-path (create_temp_table_to_plan, ~line 625) and the drop-path (drop_temp_table_to_plan, ~line 677) now use:

table_name
    .as_ident()
    .ok_or_else(|| CompilationError::internal("table name is not a plain identifier".to_string()))?
    .value
    ...

— eliminating the risk of asymmetric keys between create and drop on an unusual ObjectNamePart.

Not covered

• I did not run cargo test / cargo insta review (heavy Rust compile). Recommend running locally before merge.

· branch chore/cubesql-upgrade-df ·

[codecov]

Codecov Report

❌ Patch coverage is 75.31100% with 258 lines in your changes missing coverage. Please review.
✅ Project coverage is 83.27%. Comparing base (48a3e41) to head (79c6e52).
⚠️ Report is 1 commits behind head on master.

Files with missing lines	Patch %	Lines
...src/compile/engine/df/optimizers/plan_normalize.rs	80.50%	101 Missing ⚠️
rust/cubesql/cubesql/src/sql/statement.rs	77.00%	83 Missing ⚠️
rust/cubesql/cubesql/src/compile/router.rs	48.48%	34 Missing ⚠️
rust/cubesql/cubesql/src/sql/postgres/shim.rs	43.33%	34 Missing ⚠️
rust/cubesql/cubesql/src/compile/parser.rs	86.11%	5 Missing ⚠️
rust/cubesql/cubesql/src/error.rs	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master   #10983      +/-   ##
==========================================
+ Coverage   78.81%   83.27%   +4.45%     
==========================================
  Files         470      254     -216     
  Lines       93438    76836   -16602     
  Branches     3466        0    -3466     
==========================================
- Hits        73643    63982    -9661     
+ Misses      19292    12854    -6438     
+ Partials      503        0     -503     

Flag	Coverage Δ
cube-backend	?
cubesql	83.27% <75.31%> (-0.10%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[claude]

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

---

Claude PR Review — PR already merged

This PR was merged as 5f3cb69 (squash to master). No further action required from my side.

Summary of prior review passes

All four inline findings from the initial review were either addressed or withdrawn before merge:

Finding	Resolution
shim.rs:1441 — DECLARE multi-name guard	Addressed in 86a90c5
router.rs:366 — SET (...) = (...) arity validation	Addressed in 379ac1d
router.rs:626 — temp-table key fallback via to_string()	Addressed in e7e220a
statement.rs:1049 — visitor unwrap()	Withdrawn (matches sibling visitors ApproximateCountDistinctVisitor, SensitiveDataSanitizer, etc.)

Verdict at merge: approve — no outstanding findings.

· branch [`claude/pr-10983-20260601-1515`](https://github.com/cube-js/cube/tree/claude/pr-10983-20260601-1515)