Skip to content

Commit

Permalink
Merge pull request #33023 from vespa-engine/revert-32968-bratseth/rem…
Browse files Browse the repository at this point in the history
…ove-static-type-inference

Revert "Bratseth/remove static type inference"
  • Loading branch information
bratseth authored Dec 10, 2024
2 parents 10eb469 + eefc671 commit a334b42
Show file tree
Hide file tree
Showing 122 changed files with 779 additions and 653 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ protected boolean shouldConvert(Expression exp) {

@Override
protected Expression doConvert(Expression exp) {
if (exp.requiresInput()) {
if (exp.requiredInputType() != null) {
return new StatementExpression(new InputExpression(field.getName()), exp);
} else {
return exp;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ void requireThatExtraFieldInputImplicitThrows() throws ParseException {
}
catch (IllegalArgumentException e) {
assertEquals("For schema 'indexing_extra_field_input_implicit', field 'foo': " +
"Invalid expression 'tokenize normalize stem:\"BEST\"': Expected string input, but no input is provided",
"Invalid expression '{ tokenize normalize stem:\"BEST\" | index foo; }': Expected string input, but no input is specified",
Exceptions.toMessageString(e));
}
}
Expand Down Expand Up @@ -156,8 +156,8 @@ void testNoInputInDerivedField() throws ParseException {
fail("Expected exception");
}
catch (IllegalArgumentException e) {
assertEquals("For schema 'test', field 'derived1': Invalid expression 'attribute derived1': " +
"Expected int input, but no input is provided",
assertEquals("For schema 'test', field 'derived1': Invalid expression '{ attribute derived1; }': " +
"Expected any input, but no input is specified",
Exceptions.toMessageString(e));
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,7 @@

import com.yahoo.document.DataType;
import com.yahoo.document.NumericDataType;
import com.yahoo.document.datatypes.ByteFieldValue;
import com.yahoo.document.datatypes.DoubleFieldValue;
import com.yahoo.document.datatypes.FieldValue;
import com.yahoo.document.datatypes.FloatFieldValue;
import com.yahoo.document.datatypes.IntegerFieldValue;
import com.yahoo.document.datatypes.LongFieldValue;
import com.yahoo.document.datatypes.NumericFieldValue;
import com.yahoo.document.datatypes.*;
import com.yahoo.vespa.indexinglanguage.ExpressionConverter;
import com.yahoo.vespa.objects.ObjectOperation;
import com.yahoo.vespa.objects.ObjectPredicate;
Expand Down Expand Up @@ -54,14 +48,12 @@ public String toString() {
private final Expression right;

public ArithmeticExpression(Expression left, Operator op, Expression right) {
super(requiredInputType(left, right));
this.left = Objects.requireNonNull(left);
this.op = Objects.requireNonNull(op);
this.right = Objects.requireNonNull(right);
}

@Override
public boolean requiresInput() { return false; }

public Expression getLeftHandSide() { return left; }

public Operator getOperator() { return op; }
Expand All @@ -83,21 +75,15 @@ public DataType setInputType(DataType inputType, VerificationContext context) {

@Override
public DataType setOutputType(DataType outputType, VerificationContext context) {
if (outputType == null) return null;
super.setOutputType(outputType, context);
DataType leftInput = left.setOutputType(AnyNumericDataType.instance, context);
DataType rightInput = right.setOutputType(AnyNumericDataType.instance, context);

if (leftInput == null) return getInputType(context);
if (rightInput == null) return getInputType(context);
if (leftInput.isAssignableTo(rightInput))
return rightInput;
else if (rightInput.isAssignableTo(leftInput))
return leftInput;
else
throw new VerificationException(this, "The left argument requires " + leftInput.getName() +
", while the right argument requires " + rightInput.getName() +
": These are incompatible");
return getInputType(context);
}

@Override
Expand Down Expand Up @@ -132,6 +118,17 @@ protected void doExecute(ExecutionContext context) {
context.setCurrentValue(input).execute(right).getCurrentValue()));
}

private static DataType requiredInputType(Expression left, Expression right) {
DataType leftType = left.requiredInputType();
DataType rightType = right.requiredInputType();
if (leftType == null) return rightType;
if (rightType == null) return leftType;
if (!leftType.equals(rightType))
throw new VerificationException(ArithmeticExpression.class, "Operands require conflicting input types, " +
leftType.getName() + " vs " + rightType.getName());
return leftType;
}

@Override
public DataType createdOutputType() {
return UnresolvedDataType.INSTANCE;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
package com.yahoo.vespa.indexinglanguage.expressions;

import com.yahoo.document.DataType;
import com.yahoo.document.TensorDataType;
import com.yahoo.document.datatypes.LongFieldValue;

import java.util.Base64;
Expand All @@ -11,6 +12,10 @@
*/
public final class Base64DecodeExpression extends Expression {

public Base64DecodeExpression() {
super(DataType.STRING);
}

@Override
public DataType setInputType(DataType inputType, VerificationContext context) {
super.setInputType(inputType, DataType.STRING, context);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,10 @@
*/
public final class Base64EncodeExpression extends Expression {

public Base64EncodeExpression() {
super(DataType.LONG);
}

@Override
public DataType setInputType(DataType inputType, VerificationContext context) {
super.setInputType(inputType, DataType.LONG, context);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ public class BinarizeExpression extends Expression {
* @param threshold the value which the tensor cell value must be larger than to be set to 1 and not 0.
*/
public BinarizeExpression(double threshold) {
super(TensorDataType.any());
this.threshold = threshold;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,10 @@
*/
public final class BusyWaitExpression extends Expression {

public BusyWaitExpression() {
super(UnresolvedDataType.INSTANCE);
}

@Override
protected void doExecute(ExecutionContext context) {
FieldValue value = context.getCurrentValue();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,36 +4,31 @@
import com.yahoo.document.ArrayDataType;
import com.yahoo.document.CollectionDataType;
import com.yahoo.document.DataType;
import com.yahoo.document.TensorDataType;
import com.yahoo.document.WeightedSetDataType;
import com.yahoo.document.datatypes.Array;
import com.yahoo.document.datatypes.FieldValue;
import com.yahoo.document.datatypes.StringFieldValue;
import com.yahoo.document.datatypes.WeightedSet;
import com.yahoo.vespa.indexinglanguage.ExpressionConverter;

import java.util.ArrayList;
import java.util.Collection;
import java.util.Iterator;
import java.util.LinkedList;
import java.util.*;
import java.util.List;

/**
* @author Simon Thoresen Hult
*/
// TODO: Support Map in addition to Array and Weghted Set (doc just says "collection type")
// TODO: Support Map in addition to Array and Wighted Set (doc just says "collection type")
public final class CatExpression extends ExpressionList<Expression> {

public CatExpression(Expression... expressions) {
this(List.of(expressions));
}

public CatExpression(Collection<? extends Expression> expressions) {
super(expressions);
super(expressions, resolveInputType(expressions));
}

@Override
public boolean requiresInput() { return false; }

@Override
public CatExpression convertChildren(ExpressionConverter converter) {
return new CatExpression(convertChildList(converter));
Expand All @@ -47,14 +42,11 @@ public DataType setInputType(DataType inputType, VerificationContext context) {
for (var expression : expressions())
outputTypes.add(expression.setInputType(inputType, context));
DataType outputType = resolveOutputType(outputTypes);
if (outputType == null) outputType = getOutputType(context); // TODO: Remove this line
super.setOutputType(outputType, context);
return outputType;
return outputType != null ? outputType : getOutputType(context);
}

@Override
public DataType setOutputType(DataType outputType, VerificationContext context) {
if (outputType == null) return null;
if (outputType != DataType.STRING && ! (outputType instanceof CollectionDataType))
throw new VerificationException(this, "Required to produce " + outputType.getName() +
", but this produces a string or collection");
Expand Down Expand Up @@ -84,15 +76,43 @@ protected void doVerify(VerificationContext context) {
@Override
protected void doExecute(ExecutionContext context) {
FieldValue input = context.getCurrentValue();
DataType inputType = input != null ? input.getDataType() : null;
VerificationContext verificationContext = new VerificationContext(context.getFieldValue());
context.fillVariableTypes(verificationContext);
List<FieldValue> values = new LinkedList<>();
for (Expression expression : this)
values.add(context.setCurrentValue(input).execute(expression).getCurrentValue());
DataType type = getOutputType();
if (type == null)
throw new RuntimeException("Output type is not resolved in " + this);
List<DataType> types = new LinkedList<>();
for (Expression expression : this) {
FieldValue val = context.setCurrentValue(input).execute(expression).getCurrentValue();
values.add(val);

DataType type;
if (val != null) {
type = val.getDataType();
} else {
type = verificationContext.setCurrentType(inputType).verify(this).getCurrentType();
}
types.add(type);
}
DataType type = resolveOutputType(types);
context.setCurrentValue(type == DataType.STRING ? asString(values) : asCollection(type, values));
}

private static DataType resolveInputType(Collection<? extends Expression> list) {
DataType prev = null;
for (Expression exp : list) {
DataType next = exp.requiredInputType();
if (next == null) {
// ignore
} else if (prev == null) {
prev = next;
} else if (!prev.isAssignableFrom(next)) {
throw new VerificationException(CatExpression.class, "Operands require conflicting input types, " +
prev.getName() + " vs " + next.getName());
}
}
return prev;
}

@Override
public DataType createdOutputType() {
return UnresolvedDataType.INSTANCE;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
// Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
package com.yahoo.vespa.indexinglanguage.expressions;

import com.yahoo.document.CollectionDataType;
import com.yahoo.document.DataType;
import com.yahoo.document.datatypes.FieldValue;
import com.yahoo.vespa.indexinglanguage.ExpressionConverter;

import java.util.ArrayList;
import java.util.Collection;
import java.util.List;
import java.util.stream.Collectors;
Expand All @@ -28,7 +30,7 @@ public ChoiceExpression(Expression... choices) {
}

public ChoiceExpression(Collection<? extends Expression> choices) {
super(choices);
super(choices, resolveInputType(choices));
}

@Override
Expand Down Expand Up @@ -82,6 +84,27 @@ protected void doExecute(ExecutionContext context) {
}
}

private static DataType resolveInputType(Collection<? extends Expression> list) {
DataType previousInput = null;
DataType previousOutput = null;
for (Expression choice : list) {
DataType thisInput = choice.requiredInputType();
if (previousInput == null)
previousInput = thisInput;
else if (thisInput != null && !previousInput.isAssignableFrom(thisInput))
throw new VerificationException(ScriptExpression.class, "Choice expression require conflicting input types, " +
previousInput.getName() + " vs " + thisInput.getName());

DataType thisOutput = choice.createdOutputType();
if (previousOutput == null)
previousOutput = thisOutput;
else if (thisOutput != null && !previousOutput.isAssignableFrom(thisOutput))
throw new VerificationException(ScriptExpression.class, "Choice expression produce conflicting output types, " +
previousOutput.getName() + " vs " + thisOutput.getName());
}
return previousInput;
}

@Override
public DataType createdOutputType() {
return UnresolvedDataType.INSTANCE;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,9 @@
*/
public final class ClearStateExpression extends Expression {

@Override
public boolean requiresInput() { return false; }
public ClearStateExpression() {
super(null);
}

@Override
protected void doVerify(VerificationContext context) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
package com.yahoo.vespa.indexinglanguage.expressions;

import com.yahoo.document.DataType;
import com.yahoo.vespa.indexinglanguage.ExpressionConverter;

/**
Expand All @@ -11,6 +12,10 @@ public abstract class CompositeExpression extends Expression {
@Override
public abstract CompositeExpression convertChildren(ExpressionConverter converter);

protected CompositeExpression(DataType inputType) {
super(inputType);
}

protected static String toScriptBlock(Expression exp) {
if (exp instanceof ScriptExpression)
return exp.toString();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,10 @@ public final class ConstantExpression extends Expression {
private final FieldValue value;

public ConstantExpression(FieldValue value) {
super(null);
this.value = Objects.requireNonNull(value);
}

@Override
public boolean requiresInput() { return false; }

public FieldValue getValue() { return value; }

@Override
Expand All @@ -33,11 +31,10 @@ public DataType setInputType(DataType inputType, VerificationContext context) {

@Override
public DataType setOutputType(DataType outputType, VerificationContext context) {
if (outputType != null && ! value.getDataType().isAssignableTo(outputType))
if ( ! value.getDataType().isAssignableTo(outputType))
throw new VerificationException(this, "Produces type " + value.getDataType().getName() + ", but type " +
outputType.getName() + " is required");
super.setOutputType(outputType, context);
return null;
return super.setOutputType(outputType, context);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ public EchoExpression() {
}

public EchoExpression(PrintStream out) {
super(UnresolvedDataType.INSTANCE);
this.out = out;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ public class EmbedExpression extends Expression {
private TensorType targetType;

public EmbedExpression(Linguistics linguistics, Map<String, Embedder> embedders, String embedderId, List<String> embedderArguments) {
super(null);
this.linguistics = linguistics;
this.embedderId = embedderId;
this.embedderArguments = List.copyOf(embedderArguments);
Expand All @@ -64,13 +65,12 @@ else if ( ! embedders.containsKey(embedderId)) {
}

@Override
public DataType setInputType(DataType inputType, VerificationContext context) {
super.setInputType(inputType, context);
if ( inputType != null &&
! (inputType == DataType.STRING) &&
! (inputType instanceof ArrayDataType array && array.getNestedType() == DataType.STRING))
public DataType setInputType(DataType type, VerificationContext context) {
super.setInputType(type, context);
if ( ! (type == DataType.STRING) &&
! (type instanceof ArrayDataType array && array.getNestedType() == DataType.STRING))
throw new VerificationException(this, "This requires either a string or array<string> input type, but got " +
inputType.getName());
type.getName());
return getOutputType(context); // embed cannot determine the output type from the input
}

Expand Down
Loading

0 comments on commit a334b42

Please sign in to comment.