Skip to content

Add a native type manager #24179

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
pdabre12 merged 2 commits into from
Mar 26, 2025
Merged

Add a native type manager #24179

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
403d4f3
Add UnknownTypeException and an error code UNKNOWN_TYPE
pdabre12 Mar 19, 2025
b816611
Add a native type manager
pdabre12 Mar 19, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions presto-common/src/main/java/com/facebook/presto/common/type/StandardTypes.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ public final class StandardTypes
public static final String VARCHAR_ENUM = "VarcharEnum";
public static final String DISTINCT_TYPE = "DistinctType";
public static final String UUID = "uuid";
public static final String UNKNOWN = "unknown";

private StandardTypes() {}

Expand Down
16 changes: 16 additions & 0 deletions presto-common/src/main/java/com/facebook/presto/common/type/TypeManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
*/
package com.facebook.presto.common.type;

import java.util.Collection;
import java.util.List;

public interface TypeManager
Expand All @@ -28,4 +29,19 @@ public interface TypeManager
Type getParameterizedType(String baseTypeName, List<TypeSignatureParameter> typeParameters);

boolean canCoerce(Type actualType, Type expectedType);

default Type instantiateParametricType(TypeSignature typeSignature)
Copy link
Contributor

@rschlussel rschlussel Feb 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we extract a separate interface to split out the TypeManager from a TypeNamespaceManager. Currently the responsibilities of FunctionAndTypeManager and BuiltinTypeAndFunctionNamespaceManager are distinct, but the methods for both are all in one interface, and we throw unsupportedoperationexceptions for the things that we don't expect to be under the responsibility for the particular class, which is not very safe.

Copy link
Contributor Author

@pdabre12 pdabre12 Feb 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was a huge refactor, I have a WIP PR for it: #24640.
Is it okay if we let this one pass and handle in the PR described above?

{
throw new UnsupportedOperationException();
}

List<Type> getTypes();

/**
* Gets all registered parametric types.
*/
default Collection<ParametricType> getParametricTypes()
{
throw new UnsupportedOperationException();
}
}
3 changes: 2 additions & 1 deletion presto-common/src/test/java/com/facebook/presto/common/type/TestingTypeManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,8 @@ public boolean canCoerce(Type actualType, Type expectedType)
throw new UnsupportedOperationException();
}

private List<Type> getTypes()
@Override
public List<Type> getTypes()
{
return ImmutableList.of(BOOLEAN, INTEGER, BIGINT, DOUBLE, VARCHAR, VARBINARY, TIMESTAMP, DATE, ID, HYPER_LOG_LOG);
}
Expand Down
3 changes: 2 additions & 1 deletion presto-hudi/src/test/java/com/facebook/presto/hudi/TestingTypeManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,8 @@ public boolean canCoerce(Type actualType, Type expectedType)
throw new UnsupportedOperationException();
}

private List<Type> getTypes()
@Override
public List<Type> getTypes()
{
return ImmutableList.of(BOOLEAN, INTEGER, BIGINT, DOUBLE, VARCHAR, VARBINARY, TIMESTAMP, DATE, HYPER_LOG_LOG);
}
Expand Down
3 changes: 2 additions & 1 deletion presto-iceberg/src/test/java/com/facebook/presto/iceberg/TestIcebergFileWriter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,8 @@ public boolean canCoerce(Type actualType, Type expectedType)
throw new UnsupportedOperationException();
}

private List<Type> getTypes()
@Override
public List<Type> getTypes()
{
return ImmutableList.of(BooleanType.BOOLEAN, INTEGER, BIGINT, DoubleType.DOUBLE, VARCHAR, VARBINARY, TIMESTAMP, DATE, HYPER_LOG_LOG);
}
Expand Down
28 changes: 28 additions & 0 deletions presto-main/src/main/java/com/facebook/presto/UnknownTypeException.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
/*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package com.facebook.presto;

import com.facebook.presto.common.type.TypeSignature;
import com.facebook.presto.spi.PrestoException;

import static com.facebook.presto.spi.StandardErrorCode.UNKNOWN_TYPE;

public class UnknownTypeException
extends PrestoException
{
public UnknownTypeException(TypeSignature signature)
{
super(UNKNOWN_TYPE, "Unknown type " + signature);
}
}
3 changes: 2 additions & 1 deletion presto-main/src/main/java/com/facebook/presto/execution/AddColumnTask.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
package com.facebook.presto.execution;

import com.facebook.presto.Session;
import com.facebook.presto.UnknownTypeException;
import com.facebook.presto.common.QualifiedObjectName;
import com.facebook.presto.common.type.Type;
import com.facebook.presto.metadata.Metadata;
Expand Down Expand Up @@ -88,7 +89,7 @@ public ListenableFuture<?> execute(AddColumn statement, TransactionManager trans
try {
type = metadata.getType(parseTypeSignature(element.getType()));
}
catch (IllegalArgumentException e) {
catch (UnknownTypeException e) {
throw new SemanticException(TYPE_MISMATCH, element, "Unknown type '%s' for column '%s'", element.getType(), element.getName());
}
if (type.equals(UNKNOWN)) {
Expand Down
3 changes: 2 additions & 1 deletion presto-main/src/main/java/com/facebook/presto/execution/CreateTableTask.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
package com.facebook.presto.execution;

import com.facebook.presto.Session;
import com.facebook.presto.UnknownTypeException;
import com.facebook.presto.common.QualifiedObjectName;
import com.facebook.presto.common.type.Type;
import com.facebook.presto.metadata.Metadata;
Expand Down Expand Up @@ -125,7 +126,7 @@ public ListenableFuture<?> internalExecute(CreateTable statement, Metadata metad
try {
type = metadata.getType(parseTypeSignature(column.getType()));
}
catch (IllegalArgumentException e) {
catch (UnknownTypeException e) {
throw new SemanticException(TYPE_MISMATCH, element, "Unknown type '%s' for column '%s'", column.getType(), column.getName());
}
if (type.equals(UNKNOWN)) {
Expand Down
36 changes: 29 additions & 7 deletions ...in/src/main/java/com/facebook/presto/metadata/BuiltInTypeAndFunctionNamespaceManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
*/
package com.facebook.presto.metadata;

import com.facebook.presto.UnknownTypeException;
import com.facebook.presto.common.CatalogSchemaName;
import com.facebook.presto.common.Page;
import com.facebook.presto.common.QualifiedObjectName;
Expand Down Expand Up @@ -531,7 +532,7 @@

@ThreadSafe
public class BuiltInTypeAndFunctionNamespaceManager
implements FunctionNamespaceManager<SqlFunction>
implements FunctionNamespaceManager<SqlFunction>, TypeManager
{
public static final CatalogSchemaName JAVA_BUILTIN_NAMESPACE = new CatalogSchemaName("presto", "default");
public static final String ID = "builtin";
Expand Down Expand Up @@ -1261,21 +1262,34 @@ public ScalarFunctionImplementation getScalarFunctionImplementation(Signature si
}
}

public Optional<Type> getType(TypeSignature typeSignature)
@Override
public Type getType(TypeSignature typeSignature)
{
Type type = types.get(typeSignature);
if (type != null) {
return Optional.of(type);
return type;
}
try {
return Optional.ofNullable(parametricTypeCache.getUnchecked(new ExactTypeSignature(typeSignature)));
return parametricTypeCache.getUnchecked(new ExactTypeSignature(typeSignature));
}
catch (UncheckedExecutionException e) {
throwIfUnchecked(e.getCause());
throw new RuntimeException(e.getCause());
}
}

@Override
public Type getParameterizedType(String baseTypeName, List<TypeSignatureParameter> typeParameters)
{
throw new UnsupportedOperationException();
}

@Override
public boolean canCoerce(Type actualType, Type expectedType)
{
throw new UnsupportedOperationException();
}

public List<Type> getTypes()
{
return ImmutableList.copyOf(types.values());
Expand All @@ -1295,14 +1309,22 @@ public void addParametricType(ParametricType parametricType)
parametricTypes.putIfAbsent(name, parametricType);
}

@Override
public Collection<ParametricType> getParametricTypes()
{
return parametricTypes.values();
}

private Type instantiateParametricType(ExactTypeSignature exactSignature)
private Type instantiateParametricType(ExactTypeSignature exactTypeSignature)
{
return instantiateParametricType(exactTypeSignature.getTypeSignature(), functionAndTypeManager, parametricTypes);
}

public Type instantiateParametricType(
TypeSignature signature,
FunctionAndTypeManager functionAndTypeManager,
Map<String, ParametricType> parametricTypes)
{
TypeSignature signature = exactSignature.getTypeSignature();
List<TypeParameter> parameters = new ArrayList<>();

for (TypeSignatureParameter parameter : signature.getParameters()) {
Expand All @@ -1312,7 +1334,7 @@ private Type instantiateParametricType(ExactTypeSignature exactSignature)

ParametricType parametricType = parametricTypes.get(signature.getBase().toLowerCase(Locale.ENGLISH));
if (parametricType == null) {
throw new IllegalArgumentException("Unknown type " + signature);
throw new UnknownTypeException(signature);
}

if (parametricType instanceof MapParametricType) {
Expand Down
Loading
Loading