From 3a513e50b8ee03f783622ee34c5a98eba7f1fe8d Mon Sep 17 00:00:00 2001 From: Martin Traverso Date: Sun, 10 Mar 2019 16:59:27 -0700 Subject: [PATCH] Remove deprecated unnest behavior --- .../io/prestosql/SystemSessionProperties.java | 11 ------- .../io/prestosql/operator/UnnestOperator.java | 13 ++++---- .../sql/analyzer/FeaturesConfig.java | 14 +------- .../sql/analyzer/StatementAnalyzer.java | 3 +- .../sql/planner/RelationPlanner.java | 5 ++- .../sql/analyzer/TestFeaturesConfig.java | 5 +-- .../sql/query/TestLegacyUnnestArrayRows.java | 32 ------------------- 7 files changed, 11 insertions(+), 72 deletions(-) delete mode 100644 presto-main/src/test/java/io/prestosql/sql/query/TestLegacyUnnestArrayRows.java diff --git a/presto-main/src/main/java/io/prestosql/SystemSessionProperties.java b/presto-main/src/main/java/io/prestosql/SystemSessionProperties.java index 48f4047c48e21..9095f04997e73 100644 --- a/presto-main/src/main/java/io/prestosql/SystemSessionProperties.java +++ b/presto-main/src/main/java/io/prestosql/SystemSessionProperties.java @@ -113,7 +113,6 @@ public final class SystemSessionProperties public static final String PREFER_PARTIAL_AGGREGATION = "prefer_partial_aggregation"; public static final String OPTIMIZE_TOP_N_ROW_NUMBER = "optimize_top_n_row_number"; public static final String MAX_GROUPING_SETS = "max_grouping_sets"; - public static final String LEGACY_UNNEST = "legacy_unnest"; public static final String STATISTICS_CPU_TIMER_ENABLED = "statistics_cpu_timer_enabled"; public static final String ENABLE_STATS_CALCULATOR = "enable_stats_calculator"; public static final String IGNORE_STATS_CALCULATOR_FAILURES = "ignore_stats_calculator_failures"; @@ -528,11 +527,6 @@ public SystemSessionProperties( "Maximum number of grouping sets in a GROUP BY", featuresConfig.getMaxGroupingSets(), true), - booleanProperty( - LEGACY_UNNEST, - "Using legacy unnest semantic, where unnest(array(row)) will create one column of type row", - featuresConfig.isLegacyUnnestArrayRows(), - false), booleanProperty( STATISTICS_CPU_TIMER_ENABLED, "Experimental: Enable cpu time tracking for automatic column statistics collection on write", @@ -892,11 +886,6 @@ public static int getMaxGroupingSets(Session session) return session.getSystemProperty(MAX_GROUPING_SETS, Integer.class); } - public static boolean isLegacyUnnest(Session session) - { - return session.getSystemProperty(LEGACY_UNNEST, Boolean.class); - } - public static OptionalInt getMaxDriversPerTask(Session session) { Integer value = session.getSystemProperty(MAX_DRIVERS_PER_TASK, Integer.class); diff --git a/presto-main/src/main/java/io/prestosql/operator/UnnestOperator.java b/presto-main/src/main/java/io/prestosql/operator/UnnestOperator.java index ee1cee81295bf..521669082fdec 100644 --- a/presto-main/src/main/java/io/prestosql/operator/UnnestOperator.java +++ b/presto-main/src/main/java/io/prestosql/operator/UnnestOperator.java @@ -14,7 +14,6 @@ package io.prestosql.operator; import com.google.common.collect.ImmutableList; -import io.prestosql.SystemSessionProperties; import io.prestosql.spi.Page; import io.prestosql.spi.PageBuilder; import io.prestosql.spi.block.Block; @@ -65,7 +64,7 @@ public Operator createOperator(DriverContext driverContext) { checkState(!closed, "Factory is already closed"); OperatorContext operatorContext = driverContext.addOperatorContext(operatorId, planNodeId, UnnestOperator.class.getSimpleName()); - return new UnnestOperator(operatorContext, replicateChannels, replicateTypes, unnestChannels, unnestTypes, withOrdinality, SystemSessionProperties.isLegacyUnnest(driverContext.getSession())); + return new UnnestOperator(operatorContext, replicateChannels, replicateTypes, unnestChannels, unnestTypes, withOrdinality); } @Override @@ -94,7 +93,7 @@ public OperatorFactory duplicate() private int currentPosition; private int ordinalityCount; - public UnnestOperator(OperatorContext operatorContext, List replicateChannels, List replicateTypes, List unnestChannels, List unnestTypes, boolean withOrdinality, boolean isLegacyUnnest) + public UnnestOperator(OperatorContext operatorContext, List replicateChannels, List replicateTypes, List unnestChannels, List unnestTypes, boolean withOrdinality) { this.operatorContext = requireNonNull(operatorContext, "operatorContext is null"); this.replicateChannels = ImmutableList.copyOf(requireNonNull(replicateChannels, "replicateChannels is null")); @@ -106,7 +105,7 @@ public UnnestOperator(OperatorContext operatorContext, List replicateCh checkArgument(unnestChannels.size() == unnestTypes.size(), "unnest channels or types has wrong size"); ImmutableList.Builder outputTypesBuilder = ImmutableList.builder() .addAll(replicateTypes) - .addAll(getUnnestedTypes(unnestTypes, isLegacyUnnest)); + .addAll(getUnnestedTypes(unnestTypes)); if (withOrdinality) { outputTypesBuilder.add(BIGINT); } @@ -115,7 +114,7 @@ public UnnestOperator(OperatorContext operatorContext, List replicateCh for (Type type : unnestTypes) { if (type instanceof ArrayType) { Type elementType = ((ArrayType) type).getElementType(); - if (!isLegacyUnnest && elementType instanceof RowType) { + if (elementType instanceof RowType) { unnesters.add(new ArrayOfRowsUnnester(elementType)); } else { @@ -132,12 +131,12 @@ else if (type instanceof MapType) { } } - private static List getUnnestedTypes(List types, boolean isLegacyUnnest) + private static List getUnnestedTypes(List types) { ImmutableList.Builder builder = ImmutableList.builder(); for (Type type : types) { checkArgument(type instanceof ArrayType || type instanceof MapType, "Can only unnest map and array types"); - if (type instanceof ArrayType && !isLegacyUnnest && ((ArrayType) type).getElementType() instanceof RowType) { + if (type instanceof ArrayType && ((ArrayType) type).getElementType() instanceof RowType) { builder.addAll(((ArrayType) type).getElementType().getTypeParameters()); } else { diff --git a/presto-main/src/main/java/io/prestosql/sql/analyzer/FeaturesConfig.java b/presto-main/src/main/java/io/prestosql/sql/analyzer/FeaturesConfig.java index 20839ec7ea0c6..37ec6926ed628 100644 --- a/presto-main/src/main/java/io/prestosql/sql/analyzer/FeaturesConfig.java +++ b/presto-main/src/main/java/io/prestosql/sql/analyzer/FeaturesConfig.java @@ -45,6 +45,7 @@ import static java.util.concurrent.TimeUnit.MINUTES; @DefunctConfig({ + "deprecated.legacy-unnest-array-rows", "resource-group-manager", "experimental.resource-groups-enabled", "experimental-syntax-enabled", @@ -124,7 +125,6 @@ public class FeaturesConfig private DataSize filterAndProjectMinOutputPageSize = new DataSize(500, KILOBYTE); private int filterAndProjectMinOutputPageRowCount = 256; private int maxGroupingSets = 2048; - private boolean legacyUnnestArrayRows; public enum JoinReorderingStrategy { @@ -914,16 +914,4 @@ public FeaturesConfig setMaxGroupingSets(int maxGroupingSets) this.maxGroupingSets = maxGroupingSets; return this; } - - public boolean isLegacyUnnestArrayRows() - { - return legacyUnnestArrayRows; - } - - @Config("deprecated.legacy-unnest-array-rows") - public FeaturesConfig setLegacyUnnestArrayRows(boolean legacyUnnestArrayRows) - { - this.legacyUnnestArrayRows = legacyUnnestArrayRows; - return this; - } } diff --git a/presto-main/src/main/java/io/prestosql/sql/analyzer/StatementAnalyzer.java b/presto-main/src/main/java/io/prestosql/sql/analyzer/StatementAnalyzer.java index 3806c72da1230..68a00313c862a 100644 --- a/presto-main/src/main/java/io/prestosql/sql/analyzer/StatementAnalyzer.java +++ b/presto-main/src/main/java/io/prestosql/sql/analyzer/StatementAnalyzer.java @@ -22,7 +22,6 @@ import com.google.common.collect.Lists; import com.google.common.collect.Multimap; import io.prestosql.Session; -import io.prestosql.SystemSessionProperties; import io.prestosql.connector.ConnectorId; import io.prestosql.execution.warnings.WarningCollector; import io.prestosql.metadata.FunctionKind; @@ -751,7 +750,7 @@ protected Scope visitUnnest(Unnest node, Optional scope) Type expressionType = expressionAnalysis.getType(expression); if (expressionType instanceof ArrayType) { Type elementType = ((ArrayType) expressionType).getElementType(); - if (!SystemSessionProperties.isLegacyUnnest(session) && elementType instanceof RowType) { + if (elementType instanceof RowType) { ((RowType) elementType).getFields().stream() .map(field -> Field.newUnqualified(field.getName(), field.getType())) .forEach(outputFields::add); diff --git a/presto-main/src/main/java/io/prestosql/sql/planner/RelationPlanner.java b/presto-main/src/main/java/io/prestosql/sql/planner/RelationPlanner.java index 3b1b26304a953..4311dda48844e 100644 --- a/presto-main/src/main/java/io/prestosql/sql/planner/RelationPlanner.java +++ b/presto-main/src/main/java/io/prestosql/sql/planner/RelationPlanner.java @@ -20,7 +20,6 @@ import com.google.common.collect.ListMultimap; import com.google.common.collect.UnmodifiableIterator; import io.prestosql.Session; -import io.prestosql.SystemSessionProperties; import io.prestosql.metadata.Metadata; import io.prestosql.metadata.TableHandle; import io.prestosql.spi.connector.ColumnHandle; @@ -576,7 +575,7 @@ private RelationPlan planCrossJoinUnnest(RelationPlan leftPlan, Join joinNode, U Symbol inputSymbol = translations.get(expression); if (type instanceof ArrayType) { Type elementType = ((ArrayType) type).getElementType(); - if (!SystemSessionProperties.isLegacyUnnest(session) && elementType instanceof RowType) { + if (elementType instanceof RowType) { ImmutableList.Builder unnestSymbolBuilder = ImmutableList.builder(); for (int i = 0; i < ((RowType) elementType).getFields().size(); i++) { unnestSymbolBuilder.add(unnestedSymbolsIterator.next()); @@ -677,7 +676,7 @@ protected RelationPlan visitUnnest(Unnest node, Void context) argumentSymbols.add(inputSymbol); if (type instanceof ArrayType) { Type elementType = ((ArrayType) type).getElementType(); - if (!SystemSessionProperties.isLegacyUnnest(session) && elementType instanceof RowType) { + if (elementType instanceof RowType) { ImmutableList.Builder unnestSymbolBuilder = ImmutableList.builder(); for (int i = 0; i < ((RowType) elementType).getFields().size(); i++) { unnestSymbolBuilder.add(unnestedSymbolsIterator.next()); diff --git a/presto-main/src/test/java/io/prestosql/sql/analyzer/TestFeaturesConfig.java b/presto-main/src/test/java/io/prestosql/sql/analyzer/TestFeaturesConfig.java index c4b64538879b6..93b4939b72b62 100644 --- a/presto-main/src/test/java/io/prestosql/sql/analyzer/TestFeaturesConfig.java +++ b/presto-main/src/test/java/io/prestosql/sql/analyzer/TestFeaturesConfig.java @@ -107,8 +107,7 @@ public void testDefaults() .setArrayAggGroupImplementation(ArrayAggGroupImplementation.NEW) .setMultimapAggGroupImplementation(MultimapAggGroupImplementation.NEW) .setDistributedSortEnabled(true) - .setMaxGroupingSets(2048) - .setLegacyUnnestArrayRows(false)); + .setMaxGroupingSets(2048)); } @Test @@ -176,7 +175,6 @@ public void testExplicitPropertyMappings() .put("optimizer.optimize-top-n-row-number", "false") .put("distributed-sort", "false") .put("analyzer.max-grouping-sets", "2047") - .put("deprecated.legacy-unnest-array-rows", "true") .build(); FeaturesConfig expected = new FeaturesConfig() @@ -240,7 +238,6 @@ public void testExplicitPropertyMappings() .setMultimapAggGroupImplementation(MultimapAggGroupImplementation.LEGACY) .setDistributedSortEnabled(false) .setMaxGroupingSets(2047) - .setLegacyUnnestArrayRows(true) .setDefaultFilterFactorEnabled(true); assertFullMapping(properties, expected); } diff --git a/presto-main/src/test/java/io/prestosql/sql/query/TestLegacyUnnestArrayRows.java b/presto-main/src/test/java/io/prestosql/sql/query/TestLegacyUnnestArrayRows.java deleted file mode 100644 index 1e4c17dee7477..0000000000000 --- a/presto-main/src/test/java/io/prestosql/sql/query/TestLegacyUnnestArrayRows.java +++ /dev/null @@ -1,32 +0,0 @@ -/* - * 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.sql.query; - -import org.testng.annotations.Test; - -import static io.prestosql.SystemSessionProperties.LEGACY_UNNEST; -import static io.prestosql.testing.TestingSession.testSessionBuilder; - -public class TestLegacyUnnestArrayRows -{ - @Test - public void testLegacyUnnestArrayRows() - { - try (QueryAssertions assertions = new QueryAssertions(testSessionBuilder().setSystemProperty(LEGACY_UNNEST, "true").build())) { - assertions.assertQuery( - "SELECT * FROM UNNEST(ARRAY[ROW(1, 1.1), ROW(3, 3.3)], ARRAY[ROW('a', true), ROW('b', false)])", - "VALUES ((1, 1.1), ('a', true)), ((3, 3.3), ('b', false))"); - } - } -}