fixup: address review comments

Author: martint

core/trino-grammar/src/main/antlr4/io/trino/grammar/sql/SqlBase.g4

@@ -607,7 +607,7 @@ primaryExpression
     | TRY_CAST '(' expression AS type ')'                                                   #cast
     // the target is a primaryExpression to support PostgreSQL-style casts
     // of the form <complex expression>::<type>, which are syntactically ambiguous with
-    // static method calls
+    // static method calls defined by the SQL spec (and we reserve it for future use)
     | primaryExpression DOUBLE_COLON identifier ('(' (expression (',' expression)*)? ')')?  #staticMethodCall
     | ARRAY '[' (expression (',' expression)*)? ']'                                         #arrayConstructor
     | '[' (expression (',' expression)*)? ']'                                               #arrayConstructor

core/trino-main/src/main/java/io/trino/sql/analyzer/ExpressionAnalyzer.java

@@ -1708,31 +1708,30 @@ private void analyzeFrameRangeOffset(Expression offsetValue, FrameBound.Type bou
         @Override
         protected Type visitStaticMethodCall(StaticMethodCall node, Context context)
         {
-            // PostgreSQL-style are syntactically ambiguous with static method calls. So, static method call semantics take precendence.
+            // PostgreSQL-style casts are syntactically ambiguous with static method calls. So, static method call semantics take precendence.
             // A static method call is characterized by the target being an expression whose type is "type". This not yet supported
             // as a first-class concept, so we fake it by analyzing the expression normally. If the analysis succeeds, we treat it as
-            // the target of a cast. If the analysis fails, check whether the target is an identifier matching a known type name.
-            try {
-                process(node.getTarget(), context);
-            }
-            catch (TrinoException e) {
-                // assume it might be a type name, so check if it's an identifier matching a known type
-                if (node.getTarget() instanceof Identifier target) {
-                    try {
-                        plannerContext.getTypeManager().fromSqlType(target.getValue());
-                    }
-                    catch (TypeNotFoundException typeException) {
-                        // since the type is not found, treat the expression as normal expression that failed analysis
-                        throw e;
-                    }
+            // the target of a cast.
+
+            // Trino allows resolving column names that match type names, so we need to check explicitly
+            // if this is a type reference in the context of a static method call
+            if (node.getTarget() instanceof Identifier target) {
+                try {
+                    plannerContext.getTypeManager().fromSqlType(target.getValue());
+                    throw semanticException(NOT_SUPPORTED, node, "Static method calls are not supported");
+                }
+                catch (TypeNotFoundException typeException) {
+                    // since the type is not found, this must be a normal value-producing expression. Treat it as a candidate for
+                    // resolving the PostgreSQL-style cast, as explained above.
                 }
-                throw semanticException(NOT_SUPPORTED, node, "Static method calls are not supported");
             }
 
             if (!node.getArguments().isEmpty()) {
                 throw semanticException(NOT_SUPPORTED, node, "Static method calls are not supported");
             }
 
+            process(node.getTarget(), context);
+
             // assume it's a PostgreSQL-style cast unless result type is not a known type
             try {
                 Type type = plannerContext.getTypeManager().fromSqlType(node.getMethod().getValue());

core/trino-main/src/test/java/io/trino/operator/scalar/TestStaticMethodCall.java

@@ -14,6 +14,7 @@
 package io.trino.operator.scalar;
 
 import io.trino.spi.type.DoubleType;
+import io.trino.spi.type.VarcharType;
 import io.trino.sql.query.QueryAssertions;
 import org.junit.jupiter.api.AfterAll;
 import org.junit.jupiter.api.BeforeAll;
@@ -52,11 +53,18 @@ void testPostgreSqlStyleCast()
                 .hasType(DoubleType.DOUBLE)
                 .isEqualTo(1.0);
 
+        assertThat(assertions.expression("1::varchar"))
+                .hasType(VarcharType.VARCHAR)
+                .isEqualTo("1");
+
         assertThat(assertions.expression("(a + b)::double")
                 .binding("a", "1")
                 .binding("b", "2"))
                 .hasType(DoubleType.DOUBLE)
                 .isEqualTo(3.0);
+
+        assertThatThrownBy(() -> assertions.expression("1::decimal(3, 2)").evaluate())
+                .hasMessage("line 1:13: Static method calls are not supported");
     }
 
     @Test
@@ -73,5 +81,9 @@ void testCall()
 
         assertThatThrownBy(() -> assertions.expression("integer::foo(1, 2)").evaluate())
                 .hasMessage("line 1:19: Static method calls are not supported");
+
+        assertThat(assertions.query("SELECT bigint::real FROM (VALUES 1) AS t(bigint)"))
+                .failure()
+                .hasMessage("line 1:14: Static method calls are not supported");
     }
 }

core/trino-parser/src/test/java/io/trino/sql/parser/TestSqlParser.java

@@ -850,6 +850,16 @@ void testStaticMethodCall()
                                 new Identifier(location(1, 6), "b", false)),
                         new Identifier(location(1, 10), "double", false),
                         emptyList()));
+
+
+        assertThatThrownBy(() -> SQL_PARSER.createExpression("1::double precision"))
+                .hasMessageMatching(".*mismatched input.*");
+
+        assertThatThrownBy(() -> SQL_PARSER.createExpression("'abc'::timestamp with time zone"))
+                .hasMessageMatching(".*mismatched input.*");
+
+        assertThatThrownBy(() -> SQL_PARSER.createExpression("'abc'::interval year to month"))
+                .hasMessageMatching(".*mismatched input.*");
     }
 
     @Test