feat(trino): parse routine characteristics for inline UDFs [CLAUDE] - #7981
Open
rusackas wants to merge 2 commits into
Open
feat(trino): parse routine characteristics for inline UDFs [CLAUDE]#7981rusackas wants to merge 2 commits into
rusackas wants to merge 2 commits into
Conversation
LANGUAGE, DETERMINISTIC/NOT DETERMINISTIC, CALLED/RETURNS NULL ON NULL INPUT, SECURITY DEFINER/INVOKER, and COMMENT now parse and generate correctly for WITH FUNCTION inline UDFs (tobymao#7934 was PR 1 of ~7).
Trino has no SQL SECURITY clause of its own; the merged base keyword otherwise swallows the SQL in LANGUAGE SQL SECURITY DEFINER.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
I'm back! :)
PR 2 of 7-ish, continuing to build on #7934.
Adds parsing/generation for the routine characteristics that can follow a Trino inline
WITH FUNCTIONUDF'sRETURNSclause:LANGUAGE,DETERMINISTIC/NOT DETERMINISTIC,CALLED ON NULL INPUT/RETURNS NULL ON NULL INPUT,SECURITY DEFINER/INVOKER, andCOMMENT(https://trino.io/docs/current/udf/sql.html):Most of this already worked for free:
_parse_function_specification(from #7934) calls the base_parse_properties()betweenRETURNSandRETURN, soLANGUAGE,CALLED/RETURNS NULL ON NULL INPUT,SECURITY DEFINER/INVOKER, andCOMMENTall parsed and generated correctly with zero new code, via existingexp.LanguageProperty/exp.CalledOnNullInputProperty/exp.SqlSecurityProperty/exp.SchemaCommentProperty+ their existing generic generators.Two things actually needed fixing:
DETERMINISTIC/NOT DETERMINISTIC— reusedexp.StabilityProperty(base already parses bareDETERMINISTICinto it), and mirrored BigQuery's existingStabilityPropertyhandling exactly: aNOT DETERMINISTICentry inTrinoParser.PROPERTY_PARSERS, the sameTrinoGenerator.TRANSFORMSlambda BigQuery uses, and the same tokenizer-level"NOT DETERMINISTIC"keyword-merge BigQuery has (_match_textsonly inspects one token, so the two words need to merge into one before the parser can dispatch on them). Before this, bareDETERMINISTICround-tripped as the invalid-for-TrinoIMMUTABLE, andNOT DETERMINISTICdidn't parse at all.LANGUAGE SQLimmediately followed by aSECURITYclause — the base tokenizer already mergesSQL SECURITYinto oneTokenType.SQL_SECURITYtoken (for MySQL/StarRocks'... SQL SECURITY DEFINER VIEW), which greedily ate theSQLthat was supposed to beLANGUAGE's value, e.g.LANGUAGE SQL SECURITY DEFINERtokenized asLANGUAGE/SQL SECURITY/DEFINERinstead ofLANGUAGE/SQL/SECURITY/DEFINER. Trino has noSQL SECURITYphrase in its own grammar (only bareSECURITY DEFINER/INVOKER), and the base parser'sPROPERTY_PARSERSalready maps plain"SECURITY"to the identical_parse_sql_securitythe merged token maps to, soTrino.Tokenizerjust pops the merged keyword (same idiom as its existingKEYWORDS.pop("/*+")on the Presto side, or BigQuery'sKEYWORDS.pop("DIV")). Confirmed this doesn't affect MySQL/StarRocks, whose tokenizers are untouched.Next steps (remaining follow-up PRs, each gated on the previous)
Core(feat(trino): parse WITH FUNCTION ... RETURNS ... RETURN inline UDFs [CLAUDE] #7934)WITH FUNCTION ... RETURNS ... RETURN ...BEGIN...ENDblock bodies +DECLARE/SET, including the semicolon/chunk-continuation handling so routine bodies don't get split as separate statementsIF/ELSEIF/ELSE— reusing/extendingexp.IfBlockCASE...WHEN...END CASEWHILE...DO...END WHILE— reusing/extendingexp.WhileBlockwith alabelargLOOP/REPEAT/ITERATE/LEAVERelated: apache/superset#26162 (Superset issue asking for Trino inline SQL UDF support end-to-end).
Disclosure: implemented with Claude Code, reviewed, tested (
make unit,make style) and understood by me.