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

Remove legacy array_agg semantics #77

Merged
merged 1 commit into from Jan 26, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -602,7 +602,7 @@ public FunctionRegistry(TypeManager typeManager, BlockEncodingSerde blockEncodin
.function(ARRAY_FLATTEN_FUNCTION)
.function(ARRAY_CONCAT_FUNCTION)
.functions(ARRAY_CONSTRUCTOR, ARRAY_SUBSCRIPT, ARRAY_TO_JSON, JSON_TO_ARRAY, JSON_STRING_TO_ARRAY)
.function(new ArrayAggregationFunction(featuresConfig.isLegacyArrayAgg(), featuresConfig.getArrayAggGroupImplementation()))
.function(new ArrayAggregationFunction(featuresConfig.getArrayAggGroupImplementation()))
.functions(new MapSubscriptOperator(featuresConfig.isLegacyMapSubscript()))
.functions(MAP_CONSTRUCTOR, MAP_TO_JSON, JSON_TO_MAP, JSON_STRING_TO_MAP)
.functions(MAP_AGG, MAP_UNION)
Expand Down
Expand Up @@ -39,7 +39,6 @@
import static com.google.common.collect.ImmutableList.toImmutableList;
import static io.prestosql.metadata.Signature.typeVariable;
import static io.prestosql.operator.aggregation.AggregationMetadata.ParameterMetadata.ParameterType.BLOCK_INDEX;
import static io.prestosql.operator.aggregation.AggregationMetadata.ParameterMetadata.ParameterType.BLOCK_INPUT_CHANNEL;
import static io.prestosql.operator.aggregation.AggregationMetadata.ParameterMetadata.ParameterType.NULLABLE_BLOCK_INPUT_CHANNEL;
import static io.prestosql.operator.aggregation.AggregationMetadata.ParameterMetadata.ParameterType.STATE;
import static io.prestosql.operator.aggregation.AggregationUtils.generateAggregationName;
Expand All @@ -55,17 +54,15 @@ public class ArrayAggregationFunction
private static final MethodHandle COMBINE_FUNCTION = methodHandle(ArrayAggregationFunction.class, "combine", Type.class, ArrayAggregationState.class, ArrayAggregationState.class);
private static final MethodHandle OUTPUT_FUNCTION = methodHandle(ArrayAggregationFunction.class, "output", Type.class, ArrayAggregationState.class, BlockBuilder.class);

private final boolean legacyArrayAgg;
private final ArrayAggGroupImplementation groupMode;

public ArrayAggregationFunction(boolean legacyArrayAgg, ArrayAggGroupImplementation groupMode)
public ArrayAggregationFunction(ArrayAggGroupImplementation groupMode)
{
super(NAME,
ImmutableList.of(typeVariable("T")),
ImmutableList.of(),
parseTypeSignature("array(T)"),
ImmutableList.of(parseTypeSignature("T")));
this.legacyArrayAgg = legacyArrayAgg;
this.groupMode = requireNonNull(groupMode, "groupMode is null");
}

Expand All @@ -79,10 +76,10 @@ public String getDescription()
public InternalAggregationFunction specialize(BoundVariables boundVariables, int arity, TypeManager typeManager, FunctionRegistry functionRegistry)
{
Type type = boundVariables.getTypeVariable("T");
return generateAggregation(type, legacyArrayAgg, groupMode);
return generateAggregation(type, groupMode);
}

private static InternalAggregationFunction generateAggregation(Type type, boolean legacyArrayAgg, ArrayAggGroupImplementation groupMode)
private static InternalAggregationFunction generateAggregation(Type type, ArrayAggGroupImplementation groupMode)
{
DynamicClassLoader classLoader = new DynamicClassLoader(ArrayAggregationFunction.class.getClassLoader());

Expand All @@ -92,7 +89,7 @@ private static InternalAggregationFunction generateAggregation(Type type, boolea
List<Type> inputTypes = ImmutableList.of(type);
Type outputType = new ArrayType(type);
Type intermediateType = stateSerializer.getSerializedType();
List<ParameterMetadata> inputParameterMetadata = createInputParameterMetadata(type, legacyArrayAgg);
List<ParameterMetadata> inputParameterMetadata = createInputParameterMetadata(type);

MethodHandle inputFunction = INPUT_FUNCTION.bindTo(type);
MethodHandle combineFunction = COMBINE_FUNCTION.bindTo(type);
Expand All @@ -115,9 +112,9 @@ private static InternalAggregationFunction generateAggregation(Type type, boolea
return new InternalAggregationFunction(NAME, inputTypes, ImmutableList.of(intermediateType), outputType, true, true, factory);
}

private static List<ParameterMetadata> createInputParameterMetadata(Type value, boolean legacyArrayAgg)
private static List<ParameterMetadata> createInputParameterMetadata(Type value)
{
return ImmutableList.of(new ParameterMetadata(STATE), new ParameterMetadata(legacyArrayAgg ? BLOCK_INPUT_CHANNEL : NULLABLE_BLOCK_INPUT_CHANNEL, value), new ParameterMetadata(BLOCK_INDEX));
return ImmutableList.of(new ParameterMetadata(STATE), new ParameterMetadata(NULLABLE_BLOCK_INPUT_CHANNEL, value), new ParameterMetadata(BLOCK_INDEX));
}

public static void input(Type type, ArrayAggregationState state, Block value, int position)
Expand Down
Expand Up @@ -81,7 +81,6 @@ public class FeaturesConfig
private boolean enableIntermediateAggregations;
private boolean pushTableWriteThroughUnion = true;
private boolean exchangeCompressionEnabled;
private boolean legacyArrayAgg;
private boolean groupByUsesEqualTo;
private boolean legacyTimestamp = true;
private boolean legacyMapSubscript;
Expand Down Expand Up @@ -220,18 +219,6 @@ public boolean isLegacyCharToVarcharCoercion()
return legacyCharToVarcharCoercion;
}

@Config("deprecated.legacy-array-agg")
public FeaturesConfig setLegacyArrayAgg(boolean legacyArrayAgg)
{
this.legacyArrayAgg = legacyArrayAgg;
return this;
}

public boolean isLegacyArrayAgg()
{
return legacyArrayAgg;
}

@Config("deprecated.group-by-uses-equal")
public FeaturesConfig setGroupByUsesEqualTo(boolean value)
{
Expand Down
Expand Up @@ -68,7 +68,6 @@ public void testDefaults()
.setOptimizeHashGeneration(true)
.setPushTableWriteThroughUnion(true)
.setDictionaryAggregation(false)
.setLegacyArrayAgg(false)
.setGroupByUsesEqualTo(false)
.setLegacyMapSubscript(false)
.setRegexLibrary(JONI)
Expand Down Expand Up @@ -121,7 +120,6 @@ public void testExplicitPropertyMappings()
.put("experimental.enable-stats-calculator", "false")
.put("optimizer.ignore-stats-calculator-failures", "false")
.put("optimizer.default-filter-factor-enabled", "true")
.put("deprecated.legacy-array-agg", "true")
.put("deprecated.group-by-uses-equal", "true")
.put("deprecated.legacy-map-subscript", "true")
.put("deprecated.legacy-row-field-ordinal-access", "true")
Expand Down Expand Up @@ -203,7 +201,6 @@ public void testExplicitPropertyMappings()
.setPushTableWriteThroughUnion(false)
.setDictionaryAggregation(true)
.setPushAggregationThroughJoin(false)
.setLegacyArrayAgg(true)
.setGroupByUsesEqualTo(true)
.setLegacyMapSubscript(true)
.setRegexLibrary(RE2J)
Expand Down