Change SQL path to match spec

core/trino-main/src/main/java/io/trino/Session.java

@@ -14,6 +14,7 @@
 package io.trino;
 
 import com.google.common.annotations.VisibleForTesting;
+import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Maps;
@@ -23,12 +24,14 @@
 import io.airlift.units.Duration;
 import io.opentelemetry.api.trace.Span;
 import io.trino.client.ProtocolHeaders;
+import io.trino.connector.system.GlobalSystemConnector;
 import io.trino.metadata.SessionPropertyManager;
 import io.trino.security.AccessControl;
 import io.trino.security.SecurityContext;
 import io.trino.spi.QueryId;
 import io.trino.spi.TrinoException;
 import io.trino.spi.connector.CatalogHandle;
+import io.trino.spi.connector.CatalogSchemaName;
 import io.trino.spi.connector.ConnectorSession;
 import io.trino.spi.security.Identity;
 import io.trino.spi.security.SelectedRole;
@@ -42,6 +45,7 @@
 import java.security.Principal;
 import java.time.Instant;
 import java.util.HashMap;
+import java.util.List;
 import java.util.Locale;
 import java.util.Map;
 import java.util.Map.Entry;
@@ -54,7 +58,9 @@
 import static com.google.common.base.Preconditions.checkArgument;
 import static com.google.common.base.Preconditions.checkState;
 import static io.trino.client.ProtocolHeaders.TRINO_HEADERS;
+import static io.trino.metadata.GlobalFunctionCatalog.BUILTIN_SCHEMA;
 import static io.trino.spi.StandardErrorCode.NOT_FOUND;
+import static io.trino.sql.SqlPath.EMPTY_PATH;
 import static io.trino.util.Failures.checkCondition;
 import static java.util.Objects.requireNonNull;
 
@@ -578,6 +584,34 @@ private void validateSystemProperties(AccessControl accessControl, Map<String, S
         }
     }
 
+    public Session createViewSession(Optional<String> catalog, Optional<String> schema, Identity identity, List<CatalogSchemaName> path)
+    {
+        // For a view, we prepend the global function schema to the path, which should not be in the path
+        // We do not change the raw path, as that is use for the current_path function
+        SqlPath sqlPath = new SqlPath(
+                ImmutableList.<CatalogSchemaName>builder()
+                        .add(new CatalogSchemaName(GlobalSystemConnector.NAME, BUILTIN_SCHEMA))
+                        .addAll(path)
+                        .build(),
+                getPath().getRawPath());
+        return builder(sessionPropertyManager)
+                .setQueryId(getQueryId())
+                .setTransactionId(getTransactionId().orElse(null))
+                .setIdentity(identity)
+                .setOriginalIdentity(getOriginalIdentity())
+                .setSource(getSource().orElse(null))
+                .setCatalog(catalog)
+                .setSchema(schema)
+                .setPath(sqlPath)
+                .setTimeZoneKey(getTimeZoneKey())
+                .setLocale(getLocale())
+                .setRemoteUserAddress(getRemoteUserAddress().orElse(null))
+                .setUserAgent(getUserAgent().orElse(null))
+                .setClientInfo(getClientInfo().orElse(null))
+                .setStart(getStart())
+                .build();
+    }
+
     public static SessionBuilder builder(SessionPropertyManager sessionPropertyManager)
     {
         return new SessionBuilder(sessionPropertyManager);
@@ -605,7 +639,7 @@ public static class SessionBuilder
         private String source;
         private String catalog;
         private String schema;
-        private SqlPath path;
+        private SqlPath path = EMPTY_PATH;
         private Optional<String> traceToken = Optional.empty();
         private TimeZoneKey timeZoneKey;
         private Locale locale;
@@ -742,13 +776,6 @@ public SessionBuilder setPath(SqlPath path)
             return this;
         }
 
-        @CanIgnoreReturnValue
-        public SessionBuilder setPath(Optional<SqlPath> path)
-        {
-            this.path = path.orElse(null);
-            return this;
-        }
-
         @CanIgnoreReturnValue
         public SessionBuilder setSource(String source)
         {
@@ -915,7 +942,7 @@ public Session build()
                     Optional.ofNullable(source),
                     Optional.ofNullable(catalog),
                     Optional.ofNullable(schema),
-                    path != null ? path : new SqlPath(Optional.empty()),
+                    path,
                     traceToken,
                     timeZoneKey != null ? timeZoneKey : TimeZoneKey.getTimeZoneKey(TimeZone.getDefault().getID()),
                     locale != null ? locale : Locale.getDefault(),

core/trino-main/src/main/java/io/trino/connector/ConnectorServices.java

@@ -32,10 +32,13 @@
 import io.trino.spi.connector.ConnectorPageSinkProvider;
 import io.trino.spi.connector.ConnectorPageSourceProvider;
 import io.trino.spi.connector.ConnectorRecordSetProvider;
+import io.trino.spi.connector.ConnectorSecurityContext;
 import io.trino.spi.connector.ConnectorSplitManager;
+import io.trino.spi.connector.SchemaRoutineName;
 import io.trino.spi.connector.SystemTable;
 import io.trino.spi.connector.TableProcedureMetadata;
 import io.trino.spi.eventlistener.EventListener;
+import io.trino.spi.function.FunctionKind;
 import io.trino.spi.function.FunctionProvider;
 import io.trino.spi.function.table.ArgumentSpecification;
 import io.trino.spi.function.table.ConnectorTableFunction;
@@ -45,12 +48,14 @@
 import io.trino.spi.session.PropertyMetadata;
 import io.trino.split.RecordPageSourceProvider;
 
+import java.util.Arrays;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Optional;
 import java.util.Set;
 import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.stream.Collectors;
 
 import static com.google.common.base.Preconditions.checkArgument;
 import static com.google.common.base.Verify.verify;
@@ -175,6 +180,7 @@ public ConnectorServices(Tracer tracer, CatalogHandle catalogHandle, Connector c
         }
         catch (UnsupportedOperationException ignored) {
         }
+        verifyAccessControl(accessControl);
         this.accessControl = Optional.ofNullable(accessControl);
 
         Iterable<EventListener> eventListeners = connector.getEventListeners();
@@ -374,4 +380,23 @@ private static void validateTableFunction(ConnectorTableFunction tableFunction)
             checkArgument(describedTable.getDescriptor().isTyped(), "field types missing in returned type specification");
         }
     }
+
+    private static void verifyAccessControl(ConnectorAccessControl accessControl)
+    {
+        if (accessControl != null) {
+            mustNotDeclareMethod(accessControl.getClass(), "checkCanExecuteFunction", ConnectorSecurityContext.class, FunctionKind.class, SchemaRoutineName.class);
+        }
+    }
+
+    private static void mustNotDeclareMethod(Class<?> clazz, String name, Class<?>... parameterTypes)
+    {
+        try {
+            clazz.getMethod(name, parameterTypes);
+            throw new IllegalArgumentException(format("Access control %s must not implement removed method %s(%s)",
+                    clazz.getName(),
+                    name, Arrays.stream(parameterTypes).map(Class::getName).collect(Collectors.joining(", "))));
+        }
+        catch (ReflectiveOperationException ignored) {
+        }
+    }
 }

core/trino-main/src/main/java/io/trino/execution/CreateMaterializedViewTask.java

@@ -16,6 +16,7 @@
 import com.google.common.util.concurrent.ListenableFuture;
 import com.google.inject.Inject;
 import io.trino.Session;
+import io.trino.connector.system.GlobalSystemConnector;
 import io.trino.execution.warnings.WarningCollector;
 import io.trino.metadata.MaterializedViewDefinition;
 import io.trino.metadata.MaterializedViewPropertyManager;
@@ -155,6 +156,10 @@ public ListenableFuture<Void> execute(
                 gracePeriod,
                 statement.getComment(),
                 session.getIdentity(),
+                session.getPath().getPath().stream()
+                        // system path elements are not stored
+                        .filter(element -> !element.getCatalogName().equals(GlobalSystemConnector.NAME))
+                        .collect(toImmutableList()),
                 Optional.empty(),
                 properties);
 

core/trino-main/src/main/java/io/trino/execution/CreateViewTask.java

@@ -16,6 +16,7 @@
 import com.google.common.util.concurrent.ListenableFuture;
 import com.google.inject.Inject;
 import io.trino.Session;
+import io.trino.connector.system.GlobalSystemConnector;
 import io.trino.execution.warnings.WarningCollector;
 import io.trino.metadata.Metadata;
 import io.trino.metadata.QualifiedObjectName;
@@ -111,7 +112,11 @@ else if (metadata.getTableHandle(session, name).isPresent()) {
                 session.getSchema(),
                 columns,
                 statement.getComment(),
-                owner);
+                owner,
+                session.getPath().getPath().stream()
+                        // system path elements currently are not stored
+                        .filter(element -> !element.getCatalogName().equals(GlobalSystemConnector.NAME))
+                        .collect(toImmutableList()));
 
         metadata.createView(session, name, definition, statement.isReplace());
 

core/trino-main/src/main/java/io/trino/execution/SetPathTask.java

@@ -26,7 +26,6 @@
 import io.trino.sql.tree.SetPath;
 
 import java.util.List;
-import java.util.Optional;
 
 import static com.google.common.util.concurrent.Futures.immediateVoidFuture;
 import static io.trino.metadata.MetadataUtil.getRequiredCatalogHandle;
@@ -67,9 +66,8 @@ public ListenableFuture<Void> execute(
         }
 
         // convert to IR before setting HTTP headers - ensures that the representations of all path objects outside the parser remain consistent
-        SqlPath sqlPath = new SqlPath(Optional.of(statement.getPathSpecification().toString()));
-
-        for (SqlPathElement element : sqlPath.getParsedPath()) {
+        String rawPath = statement.getPathSpecification().toString();
+        for (SqlPathElement element : SqlPath.parsePath(rawPath)) {
             if (element.getCatalog().isEmpty() && session.getCatalog().isEmpty()) {
                 throw semanticException(MISSING_CATALOG_NAME, statement, "Catalog must be specified for each path element when session catalog is not set");
             }
@@ -79,7 +77,7 @@ public ListenableFuture<Void> execute(
                 getRequiredCatalogHandle(metadata, session, statement, catalogName);
             });
         }
-        stateMachine.setSetPath(sqlPath.toString());
+        stateMachine.setSetPath(rawPath);
         return immediateVoidFuture();
     }
 }

core/trino-main/src/main/java/io/trino/metadata/BuiltinFunctionResolver.java

@@ -20,7 +20,6 @@
 import io.trino.connector.system.GlobalSystemConnector;
 import io.trino.metadata.FunctionBinder.CatalogFunctionBinding;
 import io.trino.spi.TrinoException;
-import io.trino.spi.function.CatalogSchemaFunctionName;
 import io.trino.spi.function.FunctionDependencyDeclaration;
 import io.trino.spi.function.OperatorType;
 import io.trino.spi.function.Signature;
@@ -37,6 +36,7 @@
 import static io.trino.cache.SafeCaches.buildNonEvictableCache;
 import static io.trino.metadata.FunctionResolver.resolveFunctionBinding;
 import static io.trino.metadata.GlobalFunctionCatalog.BUILTIN_SCHEMA;
+import static io.trino.metadata.GlobalFunctionCatalog.isBuiltinFunctionName;
 import static io.trino.metadata.OperatorNameUtil.mangleOperatorName;
 import static io.trino.spi.StandardErrorCode.FUNCTION_IMPLEMENTATION_ERROR;
 import static io.trino.spi.StandardErrorCode.FUNCTION_IMPLEMENTATION_MISSING;
@@ -144,7 +144,7 @@ private ResolvedFunction resolveBuiltin(CatalogFunctionBinding functionBinding)
                 dependencies,
                 catalogSchemaFunctionName -> {
                     // builtin functions can only depend on other builtin functions
-                    if (!isBuiltinFunction(catalogSchemaFunctionName)) {
+                    if (!isBuiltinFunctionName(catalogSchemaFunctionName)) {
                         throw new TrinoException(
                                 FUNCTION_IMPLEMENTATION_ERROR,
                                 format("Builtin function %s cannot depend on a non-builtin function: %s", functionBinding.functionBinding().getBoundSignature().getName(), catalogSchemaFunctionName));
@@ -161,11 +161,6 @@ private Collection<CatalogFunctionMetadata> getBuiltinFunctions(String functionN
                 .collect(toImmutableList());
     }
 
-    private static boolean isBuiltinFunction(CatalogSchemaFunctionName name)
-    {
-        return name.getCatalogName().equals(GlobalSystemConnector.NAME) && name.getSchemaFunctionName().getSchemaName().equals(BUILTIN_SCHEMA);
-    }
-
     private record OperatorCacheKey(OperatorType operatorType, List<? extends Type> argumentTypes)
     {
         private OperatorCacheKey

core/trino-main/src/main/java/io/trino/metadata/FunctionBinder.java

@@ -32,6 +32,7 @@
 import java.util.List;
 import java.util.Optional;
 import java.util.Set;
+import java.util.TreeSet;
 import java.util.stream.Collectors;
 
 import static com.google.common.base.MoreObjects.toStringHelper;
@@ -403,7 +404,7 @@ static TrinoException functionNotFound(String name, List<TypeSignatureProvider>
             return new TrinoException(FUNCTION_NOT_FOUND, format("Function '%s' not registered", name));
         }
 
-        List<String> expectedParameters = new ArrayList<>();
+        Set<String> expectedParameters = new TreeSet<>();
         for (CatalogFunctionMetadata function : candidates) {
             String arguments = Joiner.on(", ").join(function.functionMetadata().getSignature().getArgumentTypes());
             String constraints = Joiner.on(", ").join(function.functionMetadata().getSignature().getTypeVariableConstraints());