Skip to content
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.

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

Add support for case sensitive identifiers #2350

Closed
2 changes: 2 additions & 0 deletions presto-main/src/main/java/io/prestosql/metadata/Metadata.java
Original file line number Diff line number Diff line change
Expand Up @@ -485,4 +485,6 @@ default ResolvedFunction getCoercion(Type fromType, Type toType)
ColumnPropertyManager getColumnPropertyManager();

AnalyzePropertyManager getAnalyzePropertyManager();

NameCanonicalizer getNameCanonicalizer(Session session, String catalogName);
}
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,7 @@
import static com.google.common.collect.ImmutableSet.toImmutableSet;
import static io.prestosql.metadata.FunctionId.toFunctionId;
import static io.prestosql.metadata.FunctionKind.AGGREGATE;
import static io.prestosql.metadata.NameCanonicalizer.LEGACY_NAME_CANONICALIZER;
import static io.prestosql.metadata.QualifiedObjectName.convertFromSchemaTableName;
import static io.prestosql.metadata.Signature.mangleOperatorName;
import static io.prestosql.spi.StandardErrorCode.FUNCTION_IMPLEMENTATION_MISSING;
Expand Down Expand Up @@ -1510,6 +1511,16 @@ public AnalyzePropertyManager getAnalyzePropertyManager()
return analyzePropertyManager;
}

@Override
public NameCanonicalizer getNameCanonicalizer(Session session, String catalogName)
{
Optional<CatalogMetadata> metadata = getOptionalCatalogMetadata(session, catalogName);
if (metadata.isPresent()) {
return (identifier, delimited) -> metadata.get().getMetadata().canonicalize(session.toConnectorSession(metadata.get().getCatalogName()), identifier, delimited);
Copy link
Member

Choose a reason for hiding this comment

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

Why this is functional? I would simply convert getNameCanonicalizer to canonicalize

}
return LEGACY_NAME_CANONICALIZER;
}

//
// Helpers
//
Expand Down
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 io.prestosql.metadata;

import static java.util.Locale.ENGLISH;

public interface NameCanonicalizer
{
NameCanonicalizer LEGACY_NAME_CANONICALIZER = (identifier, delimited) -> identifier.toLowerCase(ENGLISH);

String canonicalize(String identifier, boolean delimited);

default String canonicalize(NamePart namePart)
{
return canonicalize(namePart.getValue(), namePart.isDelimited());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@

import static io.prestosql.metadata.FunctionId.toFunctionId;
import static io.prestosql.metadata.FunctionKind.SCALAR;
import static io.prestosql.metadata.NameCanonicalizer.LEGACY_NAME_CANONICALIZER;
import static io.prestosql.spi.StandardErrorCode.FUNCTION_NOT_FOUND;
import static io.prestosql.spi.type.DoubleType.DOUBLE;

Expand Down Expand Up @@ -692,4 +693,10 @@ public Optional<ProjectionApplicationResult<TableHandle>> applyProjection(Sessio
{
return Optional.empty();
}

@Override
public NameCanonicalizer getNameCanonicalizer(Session session, String catalogName)
{
return LEGACY_NAME_CANONICALIZER;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -661,4 +661,12 @@ public Optional<ConnectorTableHandle> applySample(ConnectorSession session, Conn
return delegate.applySample(session, table, sampleType, sampleRatio);
}
}

@Override
public String canonicalize(ConnectorSession session, String identifier, boolean delimited)
{
try (ThreadContextClassLoader ignored = new ThreadContextClassLoader(classLoader)) {
return delegate.canonicalize(session, identifier, delimited);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
import static java.lang.String.format;
import static java.util.Collections.emptyList;
import static java.util.Collections.emptyMap;
import static java.util.Locale.ENGLISH;
import static java.util.stream.Collectors.toList;

public interface ConnectorMetadata
Expand Down Expand Up @@ -806,4 +807,14 @@ default Optional<ConnectorTableHandle> applySample(ConnectorSession session, Con
{
return Optional.empty();
}

/**
* Canonicalizes the provided SQL identifier according to connector-specific rules
* for the purpose of providing the name in metadata APIs
*/
default String canonicalize(ConnectorSession session, String identifier, boolean delimited)
Copy link
Member

Choose a reason for hiding this comment

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

I think we can skip passing session here. The way how remote data source is typically static and it should not change at runtime.
Also this will make this change easier. If we ever hit a use case where session is needed we can always add it then, it should be easy change.

{
// TODO: Move to model which is in accordance with SQL specification
return identifier.toLowerCase(ENGLISH);
}
}