Add SQL:2023 static method invocation#29385
Conversation
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
📝 WalkthroughWalkthroughThis pull request adds SQL:2023 static method call syntax to Trino, enabling expressions like Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ast-grep (0.42.1)core/trino-parser/src/main/java/io/trino/sql/parser/AstBuilder.javacore/trino-main/src/main/java/io/trino/sql/analyzer/ExpressionAnalyzer.javacore/trino-parser/src/test/java/io/trino/sql/parser/TestSqlParser.javaTip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@core/trino-main/src/main/java/io/trino/sql/analyzer/ExpressionAnalyzer.java`:
- Around line 1462-1511: visitStaticMethodCall currently skips the
argument-count guard used for regular calls; after computing argumentTypes via
getCallArgumentTypes in visitStaticMethodCall, add the same max-arguments check
used in the regular call path (the guard that throws
semanticException(TOO_MANY_ARGUMENTS, ... ) when argumentTypes.size() exceeds
the allowed maximum) before calling functionResolver.resolveStaticMethod so
static method calls produce the same error/reporting behavior as regular
function calls.
In `@core/trino-parser/src/main/java/io/trino/sql/tree/StaticMethodCall.java`:
- Around line 60-65: The getChildren() implementation in StaticMethodCall is
including the method Identifier (method) which is metadata, not an expression;
update StaticMethodCall.getChildren() to mirror FunctionCall by returning only
the argument expression nodes (arguments) and exclude the method Identifier so
generic expression walkers traverse just expressions; locate the getChildren()
method in class StaticMethodCall and remove the .add(method) step, ensuring it
returns an ImmutableList built from arguments only.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: bae76faf-6af8-446f-bb2e-fdb783fe0ab6
📒 Files selected for processing (19)
core/trino-grammar/src/main/antlr4/io/trino/grammar/sql/SqlBase.g4core/trino-main/src/main/java/io/trino/metadata/FunctionResolver.javacore/trino-main/src/main/java/io/trino/operator/scalar/ParametricScalar.javacore/trino-main/src/main/java/io/trino/operator/scalar/ScalarHeader.javacore/trino-main/src/main/java/io/trino/sql/analyzer/ExpressionAnalyzer.javacore/trino-main/src/main/java/io/trino/sql/planner/TranslationMap.javacore/trino-main/src/test/java/io/trino/operator/scalar/TestStaticMethodCall.javacore/trino-parser/src/main/java/io/trino/sql/ExpressionFormatter.javacore/trino-parser/src/main/java/io/trino/sql/parser/AstBuilder.javacore/trino-parser/src/main/java/io/trino/sql/tree/AstVisitor.javacore/trino-parser/src/main/java/io/trino/sql/tree/DefaultTraversalVisitor.javacore/trino-parser/src/main/java/io/trino/sql/tree/ExpressionRewriter.javacore/trino-parser/src/main/java/io/trino/sql/tree/ExpressionTreeRewriter.javacore/trino-parser/src/main/java/io/trino/sql/tree/StaticMethodCall.javacore/trino-parser/src/test/java/io/trino/sql/parser/TestSqlParser.javacore/trino-parser/src/test/java/io/trino/sql/parser/TestSqlParserErrorHandling.javacore/trino-spi/pom.xmlcore/trino-spi/src/main/java/io/trino/spi/function/FunctionMetadata.javacore/trino-spi/src/main/java/io/trino/spi/function/StaticMethod.java
178aafa to
08c3d5b
Compare
65fa2ca to
8f08322
Compare
SQL:2023 specifies <static method invocation>, T::method(args), for functions associated with a type. Trino has no equivalent today: every function lives in the global namespace, so the same SQL name cannot be reused across types. Functions tagged with the new @staticmethod SPI annotation are registered with a receiver type and become callable only as T::method(args). Plain method(args) calls cannot resolve to a static method, and T::method(args) cannot resolve to a regular function — the two namespaces are independent.
Description
SQL:2023 specifies
<static method invocation>—T::method(args)— for functionsassociated with a type. Trino has no equivalent today: every function lives in a single
global namespace, so the same SQL name cannot be reused across types (e.g. a
parsefunction on
bigintand a differentparseondate).This change introduces the syntax and a new
@StaticMethodSPI annotation. Functionstagged with
@StaticMethod("<type>")are registered against a receiver type and becomecallable only as
T::method(args). The two namespaces are independent:method(args)cannot resolve to a static method.T::method(args)cannot resolve to a regular function.Example:
Release notes
( ) This is not user-visible or is docs only, and no release notes are required.
( ) Release notes are required. Please propose a release note for me.
(x) Release notes are required, with the following suggested text: