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.

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

Jump to bottom

Add support for case sensitive identifiers #2350

Closed
wants to merge 11 commits into from
Closed

Add support for case sensitive identifiers #2350

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
b158659
Introduce NamePart for Metadata
Praveen2112 Dec 23, 2019
6edaa1b
!fixup Introduce NamePart for Metadata
Praveen2112 Jan 26, 2020
2e4b415
Introduce NameCanonicalizer for Metadata
Praveen2112 Dec 23, 2019
3299970
Make Session as a part of SecurityContext
Praveen2112 Dec 25, 2019
e601f5c
!fixup Make Session as a part of SecurityContext
Praveen2112 Jan 26, 2020
bdc19b2
Canonicalize names from QualifiedObjectName
Praveen2112 Dec 23, 2019
9a6f4f7
Remove lower case checks for SPI objects for table/view name
Praveen2112 Dec 23, 2019
7eb38cd
Add case senitive support for schema names
Praveen2112 Dec 23, 2019
534df54
Add case sensitive identifier support for Roles
Praveen2112 Dec 23, 2019
71e96d2
Add case sensitive support for Columns
Praveen2112 Dec 24, 2019
c194d5c
Add case sensitive support for Memory connector
Praveen2112 Dec 26, 2019
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
18 changes: 9 additions & 9 deletions presto-base-jdbc/src/main/java/io/prestosql/plugin/jdbc/BaseJdbcClient.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -196,12 +196,12 @@ protected Collection<String> listSchemas(Connection connection)
public List<SchemaTableName> getTableNames(JdbcIdentity identity, Optional<String> schema)
{
try (Connection connection = connectionFactory.openConnection(identity)) {
Optional<String> remoteSchema = schema.map(schemaName -> toRemoteSchemaName(identity, connection, schemaName));
Optional<String> remoteSchema = schema.map(schemaName -> toRemoteSchemaName(identity, connection, schemaName.toLowerCase(ENGLISH)));
try (ResultSet resultSet = getTables(connection, remoteSchema, Optional.empty())) {
ImmutableList.Builder<SchemaTableName> list = ImmutableList.builder();
while (resultSet.next()) {
String tableSchema = getTableSchemaName(resultSet);
String tableName = resultSet.getString("TABLE_NAME");
String tableSchema = getTableSchemaName(resultSet).toLowerCase(ENGLISH);
String tableName = resultSet.getString("TABLE_NAME").toLowerCase(ENGLISH);
list.add(new SchemaTableName(tableSchema, tableName));
}
return list.build();
Expand All @@ -216,8 +216,8 @@ public List<SchemaTableName> getTableNames(JdbcIdentity identity, Optional<Strin
public Optional<JdbcTableHandle> getTableHandle(JdbcIdentity identity, SchemaTableName schemaTableName)
{
try (Connection connection = connectionFactory.openConnection(identity)) {
String remoteSchema = toRemoteSchemaName(identity, connection, schemaTableName.getSchemaName());
String remoteTable = toRemoteTableName(identity, connection, remoteSchema, schemaTableName.getTableName());
String remoteSchema = toRemoteSchemaName(identity, connection, schemaTableName.getSchemaName().toLowerCase(ENGLISH));
String remoteTable = toRemoteTableName(identity, connection, remoteSchema, schemaTableName.getTableName().toLowerCase(ENGLISH));
try (ResultSet resultSet = getTables(connection, Optional.of(remoteSchema), Optional.of(remoteTable))) {
List<JdbcTableHandle> tableHandles = new ArrayList<>();
while (resultSet.next()) {
Expand Down Expand Up @@ -410,8 +410,8 @@ protected JdbcOutputTableHandle createTable(ConnectorSession session, ConnectorT

try (Connection connection = connectionFactory.openConnection(identity)) {
boolean uppercase = connection.getMetaData().storesUpperCaseIdentifiers();
String remoteSchema = toRemoteSchemaName(identity, connection, schemaTableName.getSchemaName());
String remoteTable = toRemoteTableName(identity, connection, remoteSchema, schemaTableName.getTableName());
String remoteSchema = toRemoteSchemaName(identity, connection, schemaTableName.getSchemaName().toLowerCase(ENGLISH));
String remoteTable = toRemoteTableName(identity, connection, remoteSchema, schemaTableName.getTableName().toLowerCase(ENGLISH));
if (uppercase) {
tableName = tableName.toUpperCase(ENGLISH);
}
Expand Down Expand Up @@ -467,8 +467,8 @@ public JdbcOutputTableHandle beginInsertTable(ConnectorSession session, JdbcTabl

try (Connection connection = connectionFactory.openConnection(identity)) {
boolean uppercase = connection.getMetaData().storesUpperCaseIdentifiers();
String remoteSchema = toRemoteSchemaName(identity, connection, schemaTableName.getSchemaName());
String remoteTable = toRemoteTableName(identity, connection, remoteSchema, schemaTableName.getTableName());
String remoteSchema = toRemoteSchemaName(identity, connection, schemaTableName.getSchemaName().toLowerCase(ENGLISH));
String remoteTable = toRemoteTableName(identity, connection, remoteSchema, schemaTableName.getTableName().toLowerCase(ENGLISH));
String tableName = generateTemporaryTableName();
if (uppercase) {
tableName = tableName.toUpperCase(ENGLISH);
Expand Down
2 changes: 1 addition & 1 deletion presto-blackhole/src/test/java/io/prestosql/plugin/blackhole/TestBlackHoleSmoke.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ public void blackHoleConnectorUsage()

List<QualifiedObjectName> tableNames = listBlackHoleTables();
assertTrue(tableNames.size() == 1, "Expected only one table.");
assertTrue(tableNames.get(0).getObjectName().equals("nation"), "Expected 'nation' table.");
assertTrue(tableNames.get(0).getLegacyObjectName().equals("nation"), "Expected 'nation' table.");

assertThatQueryReturnsValue("INSERT INTO nation SELECT * FROM tpch.tiny.nation", 25L);

Expand Down
2 changes: 1 addition & 1 deletion ...eospatial/src/test/java/io/prestosql/plugin/geospatial/BenchmarkGeometryAggregations.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ public void setUp()
{
Copy link
Member

Choose a reason for hiding this comment

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

Please add plenty of tests to BaseConnectorTest.

Things to cover:

  • whether output column aliases for remain their case
  • CREATE TABLE with case sensitive names
  • CREATE TABLE with columns that differs only with casing, do select, insert, update, delete from from that table
  • The similar like above with columns, but for schemas and catalogs.

queryRunner = LocalQueryRunner.create(testSessionBuilder()
.setCatalog("memory")
.setSchema("default")
.setSchema("DEFAULT")
.build());
queryRunner.installPlugin(new GeoPlugin());
queryRunner.createCatalog("memory", new MemoryConnectorFactory(), ImmutableMap.of());
Expand Down
2 changes: 1 addition & 1 deletion presto-geospatial/src/test/java/io/prestosql/plugin/geospatial/BenchmarkSpatialJoin.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ public void setUp()
{
queryRunner = LocalQueryRunner.create(testSessionBuilder()
.setCatalog("memory")
.setSchema("default")
.setSchema("DEFAULT")
.build());
queryRunner.installPlugin(new GeoPlugin());
queryRunner.createCatalog("memory", new MemoryConnectorFactory(), ImmutableMap.of());
Expand Down
2 changes: 1 addition & 1 deletion presto-geospatial/src/test/java/io/prestosql/plugin/geospatial/TestSpatialJoinPlanning.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ private static LocalQueryRunner createQueryRunner()
{
LocalQueryRunner queryRunner = LocalQueryRunner.create(testSessionBuilder()
.setCatalog("memory")
.setSchema("default")
.setSchema("DEFAULT")
.build());
queryRunner.installPlugin(new GeoPlugin());
queryRunner.createCatalog("tpch", new TpchConnectorFactory(1), ImmutableMap.of());
Expand Down
14 changes: 7 additions & 7 deletions presto-hive/src/test/java/io/prestosql/plugin/hive/HiveQueryRunner.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -200,30 +200,30 @@ public static void copyTpchTablesBucketed(
private static void copyTableBucketed(QueryRunner queryRunner, QualifiedObjectName table, Session session)
{
long start = System.nanoTime();
log.info("Running import for %s", table.getObjectName());
log.info("Running import for %s", table.getLegacyObjectName());
@Language("SQL") String sql;
switch (table.getObjectName()) {
switch (table.getLegacyObjectName()) {
case "part":
case "partsupp":
case "supplier":
case "nation":
case "region":
sql = format("CREATE TABLE %s AS SELECT * FROM %s", table.getObjectName(), table);
sql = format("CREATE TABLE %s AS SELECT * FROM %s", table.getLegacyObjectName(), table);
break;
case "lineitem":
sql = format("CREATE TABLE %s WITH (bucketed_by=array['orderkey'], bucket_count=11) AS SELECT * FROM %s", table.getObjectName(), table);
sql = format("CREATE TABLE %s WITH (bucketed_by=array['orderkey'], bucket_count=11) AS SELECT * FROM %s", table.getLegacyObjectName(), table);
break;
case "customer":
sql = format("CREATE TABLE %s WITH (bucketed_by=array['custkey'], bucket_count=11) AS SELECT * FROM %s", table.getObjectName(), table);
sql = format("CREATE TABLE %s WITH (bucketed_by=array['custkey'], bucket_count=11) AS SELECT * FROM %s", table.getLegacyObjectName(), table);
break;
case "orders":
sql = format("CREATE TABLE %s WITH (bucketed_by=array['custkey'], bucket_count=11) AS SELECT * FROM %s", table.getObjectName(), table);
sql = format("CREATE TABLE %s WITH (bucketed_by=array['custkey'], bucket_count=11) AS SELECT * FROM %s", table.getLegacyObjectName(), table);
break;
default:
throw new UnsupportedOperationException();
}
long rows = (Long) queryRunner.execute(session, sql).getMaterializedRows().get(0).getField(0);
log.info("Imported %s rows for %s in %s", rows, table.getObjectName(), nanosSince(start).convertToMostSuccinctTimeUnit());
log.info("Imported %s rows for %s in %s", rows, table.getLegacyObjectName(), nanosSince(start).convertToMostSuccinctTimeUnit());
}

public static void main(String[] args)
Expand Down
3 changes: 2 additions & 1 deletion ...to-kinesis/src/main/java/io/prestosql/plugin/kinesis/KinesisTableDescriptionSupplier.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@

import static com.google.common.base.MoreObjects.firstNonNull;
import static com.google.common.base.Throwables.throwIfUnchecked;
import static java.util.Locale.ENGLISH;
import static java.util.Objects.requireNonNull;

/**
Expand Down Expand Up @@ -81,7 +82,7 @@ public Map<SchemaTableName, KinesisStreamDescription> getTablesFromPath()
KinesisStreamDescription table = streamDescriptionCodec.fromJson(Files.readAllBytes(file));
String schemaName = firstNonNull(table.getSchemaName(), kinesisConfig.getDefaultSchema());
log.debug("Kinesis table %s %s %s", schemaName, table.getTableName(), table);
builder.put(new SchemaTableName(schemaName, table.getTableName()), table);
builder.put(new SchemaTableName(schemaName.toLowerCase(ENGLISH), table.getTableName().toLowerCase(ENGLISH)), table);
}
}

Expand Down
6 changes: 3 additions & 3 deletions presto-main/src/main/java/io/prestosql/Session.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -316,7 +316,7 @@ public Session beginTransactionId(TransactionId transactionId, TransactionManage

for (Entry<String, String> property : catalogProperties.entrySet()) {
// verify permissions
accessControl.checkCanSetCatalogSessionProperty(new SecurityContext(transactionId, identity), catalogName, property.getKey());
accessControl.checkCanSetCatalogSessionProperty(new SecurityContext(transactionId, identity, this), catalogName, property.getKey());

// validate session property value
sessionPropertyManager.validateCatalogSessionProperty(catalog, catalogName, property.getKey(), property.getValue());
Expand All @@ -332,7 +332,7 @@ public Session beginTransactionId(TransactionId transactionId, TransactionManage
.orElseThrow(() -> new PrestoException(NOT_FOUND, "Catalog does not exist: " + catalogName))
.getCatalogName();
if (role.getType() == SelectedRole.Type.ROLE) {
accessControl.checkCanSetRole(new SecurityContext(transactionId, identity), role.getRole().get(), catalogName);
accessControl.checkCanSetRole(new SecurityContext(transactionId, identity, this), role.getRole().get(), catalogName);
}
roles.put(catalog.getCatalogName(), role);

Expand Down Expand Up @@ -517,7 +517,7 @@ public static SessionBuilder builder(Session session)

public SecurityContext toSecurityContext()
{
return new SecurityContext(getRequiredTransactionId(), getIdentity());
return SecurityContext.of(this);
}

public static class SessionBuilder
Expand Down
22 changes: 8 additions & 14 deletions ...ain/src/main/java/io/prestosql/connector/informationschema/InformationSchemaMetadata.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,8 +64,9 @@
import static io.prestosql.connector.informationschema.InformationSchemaTable.TABLE_PRIVILEGES;
import static io.prestosql.connector.informationschema.InformationSchemaTable.VIEWS;
import static io.prestosql.metadata.MetadataUtil.findColumnMetadata;
import static io.prestosql.metadata.NamePart.createDefaultNamePart;
import static io.prestosql.metadata.NamePart.createDelimitedNamePart;
import static io.prestosql.spi.type.VarcharType.createUnboundedVarcharType;
import static java.util.Locale.ENGLISH;
import static java.util.Objects.requireNonNull;
import static java.util.function.Function.identity;
import static java.util.stream.Collectors.toMap;
Expand Down Expand Up @@ -245,7 +246,6 @@ private Set<QualifiedTablePrefix> calculatePrefixesWithSchemaName(
Optional<Set<String>> schemas = filterString(constraint, SCHEMA_COLUMN_HANDLE);
if (schemas.isPresent()) {
return schemas.get().stream()
.filter(this::isLowerCase)
.filter(schema -> !predicate.isPresent() || predicate.get().test(schemaAsFixedValues(schema)))
.map(schema -> new QualifiedTablePrefix(catalogName, schema))
.collect(toImmutableSet());
Expand Down Expand Up @@ -278,11 +278,10 @@ private Set<QualifiedTablePrefix> calculatePrefixesWithTableName(
.map(schemaName -> Stream.of(prefix))
.orElseGet(() -> listSchemaNames(session)))
.flatMap(prefix -> tables.get().stream()
.filter(this::isLowerCase)
.map(table -> new QualifiedObjectName(catalogName, prefix.getSchemaName().get(), table)))
.map(table -> new QualifiedObjectName(createDefaultNamePart(catalogName), createDelimitedNamePart(prefix.getSchemaName().get()), createDelimitedNamePart(table))))
.filter(objectName -> !isColumnsEnumeratingTable(informationSchemaTable) || metadata.getTableHandle(session, objectName).isPresent() || metadata.getView(session, objectName).isPresent())
.filter(objectName -> !predicate.isPresent() || predicate.get().test(asFixedValues(objectName)))
.map(QualifiedObjectName::asQualifiedTablePrefix)
.map(name -> name.asQualifiedTablePrefix(metadata.getNameCanonicalizer(session, name.getLegacyCatalogName())))
.collect(toImmutableSet());
}

Expand All @@ -295,7 +294,7 @@ private Set<QualifiedTablePrefix> calculatePrefixesWithTableName(
metadata.listTables(session, prefix).stream(),
metadata.listViews(session, prefix).stream()))
.filter(objectName -> predicate.get().test(asFixedValues(objectName)))
.map(QualifiedObjectName::asQualifiedTablePrefix)
.map(qualifiedObjectName -> qualifiedObjectName.asQualifiedTablePrefix(metadata.getNameCanonicalizer(session, qualifiedObjectName.getLegacyCatalogName())))
.collect(toImmutableSet());
}

Expand Down Expand Up @@ -355,13 +354,8 @@ private Map<ColumnHandle, NullableValue> schemaAsFixedValues(String schema)
private Map<ColumnHandle, NullableValue> asFixedValues(QualifiedObjectName objectName)
{
return ImmutableMap.of(
CATALOG_COLUMN_HANDLE, new NullableValue(createUnboundedVarcharType(), utf8Slice(objectName.getCatalogName())),
SCHEMA_COLUMN_HANDLE, new NullableValue(createUnboundedVarcharType(), utf8Slice(objectName.getSchemaName())),
TABLE_NAME_COLUMN_HANDLE, new NullableValue(createUnboundedVarcharType(), utf8Slice(objectName.getObjectName())));
}

private boolean isLowerCase(String value)
{
return value.toLowerCase(ENGLISH).equals(value);
CATALOG_COLUMN_HANDLE, new NullableValue(createUnboundedVarcharType(), utf8Slice(objectName.getLegacyCatalogName())),
SCHEMA_COLUMN_HANDLE, new NullableValue(createUnboundedVarcharType(), utf8Slice(objectName.getLegacySchemaName())),
TABLE_NAME_COLUMN_HANDLE, new NullableValue(createUnboundedVarcharType(), utf8Slice(objectName.getLegacyObjectName())));
}
}
6 changes: 3 additions & 3 deletions ...n/src/main/java/io/prestosql/connector/informationschema/InformationSchemaPageSource.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -290,9 +290,9 @@ private void addViewsRecords(QualifiedTablePrefix prefix)
{
for (Map.Entry<QualifiedObjectName, ConnectorViewDefinition> entry : metadata.getViews(session, prefix).entrySet()) {
addRecord(
entry.getKey().getCatalogName(),
entry.getKey().getSchemaName(),
entry.getKey().getObjectName(),
entry.getKey().getLegacyCatalogName(),
entry.getKey().getLegacySchemaName(),
entry.getKey().getLegacyObjectName(),
entry.getValue().getOriginalSql());
if (isLimitExhausted()) {
return;
Expand Down
6 changes: 2 additions & 4 deletions presto-main/src/main/java/io/prestosql/connector/system/SystemTableHandle.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,6 @@

import java.util.Objects;

import static io.prestosql.metadata.MetadataUtil.checkSchemaName;
import static io.prestosql.metadata.MetadataUtil.checkTableName;
import static java.util.Objects.requireNonNull;

public class SystemTableHandle
Expand All @@ -39,8 +37,8 @@ public SystemTableHandle(
@JsonProperty("tableName") String tableName,
@JsonProperty("constraint") TupleDomain<ColumnHandle> constraint)
{
this.schemaName = checkSchemaName(schemaName);
this.tableName = checkTableName(tableName);
this.schemaName = requireNonNull(schemaName, "schemaName is null");
this.tableName = requireNonNull(tableName, "tableName is null");
this.constraint = requireNonNull(constraint, "constraint is null");
}

Expand Down
9 changes: 5 additions & 4 deletions presto-main/src/main/java/io/prestosql/execution/AddColumnTask.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,14 +69,15 @@ public ListenableFuture<?> execute(AddColumn statement, TransactionManager trans
throw semanticException(TABLE_NOT_FOUND, statement, "Table '%s' does not exist", tableName);
}

CatalogName catalogName = metadata.getCatalogHandle(session, tableName.getCatalogName())
.orElseThrow(() -> new PrestoException(NOT_FOUND, "Catalog does not exist: " + tableName.getCatalogName()));
CatalogName catalogName = metadata.getCatalogHandle(session, tableName.getLegacyCatalogName())
.orElseThrow(() -> new PrestoException(NOT_FOUND, "Catalog does not exist: " + tableName.getLegacyCatalogName()));

accessControl.checkCanAddColumns(session.toSecurityContext(), tableName);

Map<String, ColumnHandle> columnHandles = metadata.getColumnHandles(session, tableHandle.get());

ColumnDefinition element = statement.getColumn();
String columnName = metadata.getNameCanonicalizer(session, catalogName.getCatalogName()).canonicalize(element.getName().getValue(), element.getName().isDelimited());
Type type;
try {
type = metadata.getType(toTypeSignature(element.getType()));
Expand All @@ -97,14 +98,14 @@ public ListenableFuture<?> execute(AddColumn statement, TransactionManager trans
Map<String, Expression> sqlProperties = mapFromProperties(element.getProperties());
Map<String, Object> columnProperties = metadata.getColumnPropertyManager().getProperties(
catalogName,
tableName.getCatalogName(),
tableName.getLegacyCatalogName(),
sqlProperties,
session,
metadata,
parameterExtractor(statement, parameters));

ColumnMetadata column = ColumnMetadata.builder()
.setName(element.getName().getValue())
.setName(columnName)
.setType(type)
.setNullable(element.isNullable())
.setComment(element.getComment())
Expand Down
6 changes: 3 additions & 3 deletions presto-main/src/main/java/io/prestosql/execution/CallTask.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,9 +76,9 @@ public ListenableFuture<?> execute(Call call, TransactionManager transactionMana

Session session = stateMachine.getSession();
QualifiedObjectName procedureName = createQualifiedObjectName(session, call, call.getName());
CatalogName catalogName = metadata.getCatalogHandle(stateMachine.getSession(), procedureName.getCatalogName())
.orElseThrow(() -> semanticException(CATALOG_NOT_FOUND, call, "Catalog %s does not exist", procedureName.getCatalogName()));
Procedure procedure = metadata.getProcedureRegistry().resolve(catalogName, procedureName.asSchemaTableName());
CatalogName catalogName = metadata.getCatalogHandle(stateMachine.getSession(), procedureName.getLegacyCatalogName())
.orElseThrow(() -> semanticException(CATALOG_NOT_FOUND, call, "Catalog %s does not exist", procedureName.getLegacyCatalogName()));
Procedure procedure = metadata.getProcedureRegistry().resolve(catalogName, procedureName.asSchemaTableName(metadata.getNameCanonicalizer(session, catalogName.getCatalogName())));

// map declared argument names to positions
Map<String, Integer> positions = new HashMap<>();
Expand Down
Loading