Skip to content

Commit

Permalink
Add support for creating view with properties in Hive
Browse files Browse the repository at this point in the history
  • Loading branch information
@Praveen2112
Praveen2112 committed Apr 13, 2024
1 parent 757d3be commit 0990a43
Show file tree
Hide file tree
Showing 6 changed files with 199 additions and 5 deletions.
9 changes: 9 additions & 0 deletions plugin/trino-hive/src/main/java/io/trino/plugin/hive/HiveConnector.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ public class HiveConnector
private final List<PropertyMetadata<?>> sessionProperties;
private final List<PropertyMetadata<?>> schemaProperties;
private final List<PropertyMetadata<?>> tableProperties;
private final List<PropertyMetadata<?>> viewProperties;
private final List<PropertyMetadata<?>> columnProperties;
private final List<PropertyMetadata<?>> analyzeProperties;
private final List<PropertyMetadata<?>> materializedViewProperties;
Expand All @@ -84,6 +85,7 @@ public HiveConnector(
Set<SessionPropertiesProvider> sessionPropertiesProviders,
List<PropertyMetadata<?>> schemaProperties,
List<PropertyMetadata<?>> tableProperties,
List<PropertyMetadata<?>> viewProperties,
List<PropertyMetadata<?>> columnProperties,
List<PropertyMetadata<?>> analyzeProperties,
List<PropertyMetadata<?>> materializedViewProperties,
Expand All @@ -107,6 +109,7 @@ public HiveConnector(
.collect(toImmutableList());
this.schemaProperties = ImmutableList.copyOf(requireNonNull(schemaProperties, "schemaProperties is null"));
this.tableProperties = ImmutableList.copyOf(requireNonNull(tableProperties, "tableProperties is null"));
this.viewProperties = ImmutableList.copyOf(requireNonNull(viewProperties, "viewProperties is null"));
this.columnProperties = ImmutableList.copyOf(requireNonNull(columnProperties, "columnProperties is null"));
this.analyzeProperties = ImmutableList.copyOf(requireNonNull(analyzeProperties, "analyzeProperties is null"));
this.materializedViewProperties = requireNonNull(materializedViewProperties, "materializedViewProperties is null");
Expand Down Expand Up @@ -179,6 +182,12 @@ public List<PropertyMetadata<?>> getTableProperties()
return tableProperties;
}

@Override
public List<PropertyMetadata<?>> getViewProperties()
{
return viewProperties;
}

@Override
public List<PropertyMetadata<?>> getColumnProperties()
{
Expand Down
2 changes: 2 additions & 0 deletions plugin/trino-hive/src/main/java/io/trino/plugin/hive/HiveConnectorFactory.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,7 @@ public static Connector createConnector(
ConnectorNodePartitioningProvider connectorDistributionProvider = injector.getInstance(ConnectorNodePartitioningProvider.class);
Set<SessionPropertiesProvider> sessionPropertiesProviders = injector.getInstance(new Key<>() {});
HiveTableProperties hiveTableProperties = injector.getInstance(HiveTableProperties.class);
HiveViewProperties hiveViewProperties = injector.getInstance(HiveViewProperties.class);
HiveColumnProperties hiveColumnProperties = injector.getInstance(HiveColumnProperties.class);
HiveAnalyzeProperties hiveAnalyzeProperties = injector.getInstance(HiveAnalyzeProperties.class);
HiveMaterializedViewPropertiesProvider hiveMaterializedViewPropertiesProvider = injector.getInstance(HiveMaterializedViewPropertiesProvider.class);
Expand All @@ -159,6 +160,7 @@ public static Connector createConnector(
sessionPropertiesProviders,
HiveSchemaProperties.SCHEMA_PROPERTIES,
hiveTableProperties.getTableProperties(),
hiveViewProperties.getViewProperties(),
hiveColumnProperties.getColumnProperties(),
hiveAnalyzeProperties.getAnalyzeProperties(),
hiveMaterializedViewPropertiesProvider.getMaterializedViewProperties(),
Expand Down
41 changes: 36 additions & 5 deletions plugin/trino-hive/src/main/java/io/trino/plugin/hive/HiveMetadata.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -309,6 +309,7 @@
import static io.trino.spi.StandardErrorCode.INVALID_COLUMN_PROPERTY;
import static io.trino.spi.StandardErrorCode.INVALID_SCHEMA_PROPERTY;
import static io.trino.spi.StandardErrorCode.INVALID_TABLE_PROPERTY;
import static io.trino.spi.StandardErrorCode.INVALID_VIEW_PROPERTY;
import static io.trino.spi.StandardErrorCode.NOT_SUPPORTED;
import static io.trino.spi.StandardErrorCode.TABLE_NOT_FOUND;
import static io.trino.spi.StandardErrorCode.UNSUPPORTED_TABLE_TYPE;
Expand Down Expand Up @@ -2672,19 +2673,41 @@ private static boolean exists(TrinoFileSystem fs, Location location)
}

@Override
public void createView(ConnectorSession session, SchemaTableName viewName, ConnectorViewDefinition definition, boolean replace)
public void createView(
ConnectorSession session,
SchemaTableName viewName,
ConnectorViewDefinition definition,
Map<String, Object> viewProperties,
boolean replace)
{
if (accessControlMetadata.isUsingSystemSecurity()) {
definition = definition.withoutOwner();
}

Map<String, String> properties = ImmutableMap.<String, String>builder()
ImmutableMap.Builder<String, String> propertiesBuilder = ImmutableMap.builder();
propertiesBuilder
.put(TABLE_COMMENT, PRESTO_VIEW_COMMENT)
.put(PRESTO_VIEW_FLAG, "true")
.put(TRINO_CREATED_BY, "Trino Hive connector")
.put(TRINO_VERSION_NAME, trinoVersion)
.put(TRINO_QUERY_ID_NAME, session.getQueryId())
.buildOrThrow();
.put(TRINO_QUERY_ID_NAME, session.getQueryId());

Map<String, String> baseProperties = propertiesBuilder.buildOrThrow();

// Extra properties
Map<String, String> extraProperties = HiveViewProperties.getExtraProperties(viewProperties)
.orElseGet(ImmutableMap::of);
Set<String> illegalExtraProperties = Sets.intersection(
ImmutableSet.<String>builder()
.addAll(baseProperties.keySet())
.build(),
extraProperties.keySet());
if (!illegalExtraProperties.isEmpty()) {
throw new TrinoException(
INVALID_VIEW_PROPERTY,
"Illegal keys in extra_properties: " + illegalExtraProperties);
}
propertiesBuilder.putAll(extraProperties);

Column dummyColumn = new Column("dummy", HIVE_STRING, Optional.empty(), ImmutableMap.of());

Expand All @@ -2695,7 +2718,7 @@ public void createView(ConnectorSession session, SchemaTableName viewName, Conne
.setTableType(VIRTUAL_VIEW.name())
.setDataColumns(ImmutableList.of(dummyColumn))
.setPartitionColumns(ImmutableList.of())
.setParameters(properties)
.setParameters(propertiesBuilder.buildOrThrow())
.setViewOriginalText(Optional.of(encodeViewData(definition)))
.setViewExpandedText(Optional.of(PRESTO_VIEW_EXPANDED_TEXT_MARKER));

Expand Down Expand Up @@ -2763,6 +2786,14 @@ public List<SchemaTableName> listViews(ConnectorSession session, Optional<String
.collect(toImmutableList());
}

@Override
public Map<String, Object> getViewProperties(ConnectorSession session, SchemaTableName viewName)
{
// The only currently existing view property (extra_properties) is hidden,
// no need to retrieve its value for SHOW CREATE VIEW.
return ImmutableMap.of();
}

@Override
public Map<String, Object> getSchemaProperties(ConnectorSession session, String schemaName)
{
Expand Down
1 change: 1 addition & 0 deletions plugin/trino-hive/src/main/java/io/trino/plugin/hive/HiveModule.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ public void configure(Binder binder)

binder.bind(HiveSessionProperties.class).in(Scopes.SINGLETON);
binder.bind(HiveTableProperties.class).in(Scopes.SINGLETON);
binder.bind(HiveViewProperties.class).in(Scopes.SINGLETON);
binder.bind(HiveColumnProperties.class).in(Scopes.SINGLETON);
binder.bind(HiveAnalyzeProperties.class).in(Scopes.SINGLETON);
newOptionalBinder(binder, HiveMaterializedViewPropertiesProvider.class)
Expand Down
70 changes: 70 additions & 0 deletions plugin/trino-hive/src/main/java/io/trino/plugin/hive/HiveViewProperties.java
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
/*
* 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.trino.plugin.hive;

import com.google.common.collect.ImmutableList;
import com.google.inject.Inject;
import io.trino.spi.TrinoException;
import io.trino.spi.session.PropertyMetadata;
import io.trino.spi.type.MapType;
import io.trino.spi.type.TypeManager;

import java.util.List;
import java.util.Map;
import java.util.Optional;

import static io.trino.spi.StandardErrorCode.INVALID_VIEW_PROPERTY;
import static io.trino.spi.type.VarcharType.VARCHAR;
import static java.lang.String.format;

public class HiveViewProperties
{
public static final String EXTRA_PROPERTIES = "extra_properties";

private final List<PropertyMetadata<?>> viewProperties;

@Inject
public HiveViewProperties(TypeManager typeManager)
{
viewProperties = ImmutableList.of(
new PropertyMetadata<>(
EXTRA_PROPERTIES,
"Extra view properties",
new MapType(VARCHAR, VARCHAR, typeManager.getTypeOperators()),
Map.class,
null,
true, // currently not shown in SHOW CREATE VIEW
value -> {
Map<String, String> extraProperties = (Map<String, String>) value;
if (extraProperties.containsValue(null)) {
throw new TrinoException(INVALID_VIEW_PROPERTY, format("Extra view property value cannot be null '%s'", extraProperties));
}
if (extraProperties.containsKey(null)) {
throw new TrinoException(INVALID_VIEW_PROPERTY, format("Extra view property key cannot be null '%s'", extraProperties));
}
return extraProperties;
},
value -> value));
}

public List<PropertyMetadata<?>> getViewProperties()
{
return viewProperties;
}

public static Optional<Map<String, String>> getExtraProperties(Map<String, Object> tableProperties)
{
return Optional.ofNullable((Map<String, String>) tableProperties.get(EXTRA_PROPERTIES));
}
}
81 changes: 81 additions & 0 deletions plugin/trino-hive/src/test/java/io/trino/plugin/hive/BaseHiveConnectorTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,10 @@
import static io.trino.plugin.hive.HiveColumnHandle.PARTITION_COLUMN_NAME;
import static io.trino.plugin.hive.HiveColumnHandle.PATH_COLUMN_NAME;
import static io.trino.plugin.hive.HiveMetadata.MODIFYING_NON_TRANSACTIONAL_TABLE_MESSAGE;
import static io.trino.plugin.hive.HiveMetadata.TABLE_COMMENT;
import static io.trino.plugin.hive.HiveMetadata.TRINO_CREATED_BY;
import static io.trino.plugin.hive.HiveMetadata.TRINO_QUERY_ID_NAME;
import static io.trino.plugin.hive.HiveMetadata.TRINO_VERSION_NAME;
import static io.trino.plugin.hive.HiveQueryRunner.HIVE_CATALOG;
import static io.trino.plugin.hive.HiveQueryRunner.TPCH_SCHEMA;
import static io.trino.plugin.hive.HiveQueryRunner.createBucketedSession;
Expand All @@ -149,6 +153,7 @@
import static io.trino.plugin.hive.HiveTableProperties.STORAGE_FORMAT_PROPERTY;
import static io.trino.plugin.hive.HiveType.toHiveType;
import static io.trino.plugin.hive.TestingHiveUtils.getConnectorService;
import static io.trino.plugin.hive.ViewReaderUtil.PRESTO_VIEW_FLAG;
import static io.trino.plugin.hive.util.HiveUtil.columnExtraInfo;
import static io.trino.spi.security.Identity.ofUser;
import static io.trino.spi.security.SelectedRole.Type.ROLE;
Expand Down Expand Up @@ -9101,6 +9106,82 @@ public void testCollidingMixedCaseProperty()
assertUpdate("DROP TABLE %s".formatted(tableName));
}

@Test
public void testExtraPropertiesOnView()
{
String tableName = "create_view_with_multiple_extra_properties_" + randomNameSuffix();
assertUpdate("CREATE VIEW %s WITH (extra_properties = MAP(ARRAY['extra.property.one', 'extra.property.two'], ARRAY['one', 'two'])) AS SELECT 1 as colA".formatted(tableName));

assertQuery(
"SELECT \"extra.property.one\", \"extra.property.two\" FROM \"%s$properties\"".formatted(tableName),
"SELECT 'one', 'two'");
assertThat(computeActual("SHOW CREATE VIEW %s".formatted(tableName)).getOnlyValue())
.isEqualTo("""
CREATE VIEW hive.tpch.%s SECURITY DEFINER AS
SELECT 1 colA""".formatted(tableName));
assertUpdate("DROP VIEW %s".formatted(tableName));
}

@Test
public void testDuplicateExtraPropertiesOnView()
{
assertQueryFails(
"CREATE VIEW create_view_with_duplicate_extra_properties WITH (extra_properties = MAP(ARRAY['extra.property', 'extra.property'], ARRAY['true', 'false'])) AS SELECT 1 as colA",
"Invalid value for catalog 'hive' view property 'extra_properties': Cannot convert.*");
}

@Test
public void testNullExtraPropertyOnView()
{
assertQueryFails(
"CREATE VIEW create_view_with_duplicate_extra_properties WITH (extra_properties = MAP(ARRAY['null.property'], ARRAY[null])) AS SELECT 1 as c1",
".*Extra view property value cannot be null '\\{null.property=null}'.*");
}

@Test
public void testCollidingMixedCasePropertyOnView()
{
String tableName = "create_view_with_mixed_case_extra_properties" + randomNameSuffix();

assertUpdate("CREATE VIEW %s WITH (extra_properties = MAP(ARRAY['one', 'ONE'], ARRAY['one', 'ONE'])) AS SELECT 1 as colA".formatted(tableName));
// TODO: (https://github.com/trinodb/trino/issues/17) This should run successfully
assertThat(query("SELECT * FROM \"%s$properties\"".formatted(tableName)))
.nonTrinoExceptionFailure().hasMessageContaining("Multiple entries with same key: one=one and one=one");

assertUpdate("DROP VIEW %s".formatted(tableName));
}

@Test
public void testCreateViewWithTableProperties()
{
assertQueryFails(
"CREATE VIEW create_view_with_table_properties WITH (format = 'ORC', extra_properties = MAP(ARRAY['extra.property'], ARRAY['true'])) AS SELECT 1 as colA",
"Catalog 'hive' view property 'format' does not exist");
}

@Test
public void testCreateViewWithPreDefinedPropertiesAsExtraProperties()
{
assertQueryFails(
"CREATE VIEW create_view_with_predefined_view_properties WITH (extra_properties = MAP(ARRAY['%s'], ARRAY['true'])) AS SELECT 1 as colA".formatted(TABLE_COMMENT),
"Illegal keys in extra_properties: \\[comment]");

assertQueryFails(
"CREATE VIEW create_view_with_predefined_view_properties WITH (extra_properties = MAP(ARRAY['%s'], ARRAY['true'])) AS SELECT 1 as colA".formatted(PRESTO_VIEW_FLAG),
"Illegal keys in extra_properties: \\[presto_view]");

assertQueryFails("CREATE VIEW create_view_with_predefined_view_properties WITH (extra_properties = MAP(ARRAY['%s'], ARRAY['true'])) AS SELECT 1 as colA".formatted(TRINO_CREATED_BY),
"Illegal keys in extra_properties: \\[trino_created_by]");

assertQueryFails("CREATE VIEW create_view_with_predefined_view_properties WITH (extra_properties = MAP(ARRAY['%s'], ARRAY['true'])) AS SELECT 1 as colA".formatted(TRINO_VERSION_NAME),
"Illegal keys in extra_properties: \\[trino_version]");

assertQueryFails(
"CREATE VIEW create_view_with_predefined_view_properties WITH (extra_properties = MAP(ARRAY['%s'], ARRAY['true'])) AS SELECT 1 as colA".formatted(TRINO_QUERY_ID_NAME),
"Illegal keys in extra_properties: \\[trino_query_id]");

}

@Test
public void testCommentWithPartitionedTable()
{
Expand Down

0 comments on commit 0990a43

Please sign in to comment.