feat(trino): parse CASE routine statements [CLAUDE] - #8068
Conversation
Reuses exp.IfBlock per WHEN branch (only `this`/`true` populated) inside
a new, minimal exp.CaseStatement wrapper mirroring exp.Case's shape
(this=operand, ifs, default), rather than inventing a full new branch
type. The operand form ("CASE a WHEN 0 THEN ...") is normalized into an
EQ condition at parse time and unwrapped at generation time, so both
forms share one code path through _parse_routine_statements.
Confirmed against a real Trino instance that the docs' "Searched case"
synopsis (bare END) contradicts the docs' own worked example (END CASE);
only END CASE is actually accepted for either form.
…tatement Closes two gaps in the phase-5 CASE test suite found while auditing test sufficiency: the no-operand + no-ELSE combination wasn't exercised (only its operand-form counterpart was), and no test had CASE immediately followed by the enclosing END with nothing between them - the same adjacent-END token-matching shape the BEGIN/IF phases already covered for their own constructs. Both confirmed against a real Trino instance; the second hits the same function-body completeness-check limitation already documented on the IF phase (asserts grammar only).
| # The operand form ("CASE a WHEN 0 THEN ...") is normalized into | ||
| # WHEN a = 0 THEN ... at parse time, so each branch can reuse | ||
| # exp.IfBlock (only `this`/`true` populated) like _parse_routine_if(). | ||
| this = None if self._match(TokenType.WHEN, advance=False) else self._parse_disjunction() |
There was a problem hiding this comment.
I don't get it - why do we need to rewrite this? Let's not make this a special case; can we instead parse it like the usual CASE expression in declarative SQL?
So, everything would be like CASE...END, only the resulting node would be CaseStatement, so that we can disambiguate the ;-delimited one from the existing one.
There was a problem hiding this comment.
Fair, no reason to special-case it. Dropped the EQ rewrite entirely, this now just mirrors _parse_case() - operand and match values are carried through as-is, only the branch bodies differ since they're ;-delimited statement lists instead of single expressions.
|
|
||
| return f"{' '.join(branches)} END IF" | ||
|
|
||
| def casestatement_sql(self, expression: exp.CaseStatement) -> str: |
There was a problem hiding this comment.
This looks more complicated than necessary. Again, can we mirror case_sql, using ; delimiters?
There was a problem hiding this comment.
Done, mirrors case_sql() pretty closely now, just swapping in ;-terminated bodies and END CASE. Also ran annotate_types over a parsed tree, just to make sure reusing exp.If here doesn't confuse anything expecting scalar values. Seems fine.
Co-authored-by: Jo <46752250+georgesittas@users.noreply.github.com>
…ng CASE branches Per review feedback on tobymao#8068: drop the EQ-rewrite that normalized the operand form ("CASE a WHEN 0 THEN ...") into WHEN a = 0 THEN ... at parse time. Each branch now carries its raw match/condition value untouched, exactly like the base parser's _parse_case()/case_sql() already do for the CASE *expression* - only the branch bodies differ, since they're ;-delimited statement lists instead of single value expressions. Reuses exp.If per branch (matching exp.Case's own ifs shape) instead of exp.IfBlock, inside the same exp.CaseStatement wrapper.
2 participants
Hi again :)
Continuing with PR 5 of ~7, in the series following #7934/#7981/#8004/#8044, building out Trino inline SQL UDF support (see the original roadmap in #7926, and apache/superset#26162).
This adds parsing/generation for Trino's
CASE ... WHEN ... THEN ... [ELSE ...] END CASEroutine statement, in both its operand form (CASE a WHEN 0 THEN ...) and no-operand form (CASE WHEN a = 0 THEN ...) (https://trino.io/docs/current/udf/sql/case.html).Design
Continuing the reuse-over-invent approach from #8044: each
WHENbranch reusesexp.IfBlock(onlythis/truepopulated, no per-branchfalse), the same statement-block condition/then container already introduced forIF. These are collected into a new, minimalexp.CaseStatement(this, ifs, default), mirroring the shape of the existingexp.Caseexpression rather than conflating with it -exp.Caseis taggedFuncand expected to hold scalar-valued branches, so giving itstrue/defaultslots statementBlocks instead (asIfBlockneeded its own class for the same reason) risked confusing anything that walksexp.Caseexpecting a value.The operand form is normalized into
WHEN <operand> = <match> THEN ...at parse time (matching how Trino itself evaluates it) and unwrapped back out at generation time, so both forms share the same branch-parsing code path through_parse_routine_statements.A note on the docs
Trino's own CASE docs page contradicts itself: the synopsis for the operand form (which it confusingly labels "Searched case" - the reverse of standard SQL terminology) shows it terminating in a bare
END, but the page's own worked example uses the operand form and ends inEND CASE. I confirmed against a real Trino instance (Docker) that onlyEND CASEis actually accepted for either form, and implemented/tested against that.Also confirmed via Docker: no comma-separated match values per
WHEN(unlike the CASE expression), CASE nests fine, and falling through with no matchingWHENand noELSEjust continues to the next statement rather than erroring.Remaining phases
exp.WhileBlockwith a label arg)Related: apache/superset#26162
make unitandmake stylepass locally.Disclosure: Claude Code wrote this one with me steering (including docker-driven Trino verification), I reviewed and understood every line to the best of my ability. Still made with hand-typed human oversight, I promise.