Skip to content

Commit

Permalink
Merge pull request #32822 from vespa-engine/bratseth/indexing-type-in…
Browse files Browse the repository at this point in the history
…ference-2

Bratseth/indexing type inference 2
  • Loading branch information
bratseth authored Nov 9, 2024
2 parents d52a051 + fddc998 commit 50159e2
Show file tree
Hide file tree
Showing 9 changed files with 103 additions and 29 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
// 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.document.NumericDataType;
import com.yahoo.document.datatypes.DoubleFieldValue;
import com.yahoo.document.datatypes.FieldValue;

/**
* A DataType representing any numeric type. This is (so far) only needed during type resolution of indexing pipelines
* so it is placed here.
*
* @author bratseth
*/
class AnyNumericDataType extends NumericDataType {

static final AnyNumericDataType instance = new AnyNumericDataType();

private AnyNumericDataType() {
super("numeric", DataType.lastPredefinedDataTypeId() + 2, DoubleFieldValue.class, new UnsupportedFactory());
}

@Override
public boolean isAssignableFrom(DataType other) {
return other instanceof NumericDataType;
}

@Override
public boolean isAssignableTo(DataType other) {
return other instanceof AnyNumericDataType || other instanceof AnyDataType;
}

@Override
public boolean isValueCompatible(FieldValue value) {
return isAssignableFrom(value.getDataType());
}

@Override
public FieldValue createFieldValue() { throw new UnsupportedOperationException(); }

private static class UnsupportedFactory extends Factory {

public FieldValue create() {
throw new UnsupportedOperationException();
}

public FieldValue create(String value) {
throw new UnsupportedOperationException();
}

}
}
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,6 @@ public ArithmeticExpression(Expression left, Operator op, Expression right) {

@Override
public ArithmeticExpression convertChildren(ExpressionConverter converter) {
// TODO: branch()?
return new ArithmeticExpression(converter.convert(left), op, converter.convert(right));
}

Expand All @@ -77,9 +76,11 @@ public DataType setInputType(DataType inputType, VerificationContext context) {
@Override
public DataType setOutputType(DataType outputType, VerificationContext context) {
super.setOutputType(outputType, context);
DataType leftInput = left.setOutputType(outputType, context);
DataType rightInput = right.setOutputType(outputType, context);
if (leftInput == rightInput) // TODO: Generalize
DataType leftInput = left.setOutputType(AnyNumericDataType.instance, context);
DataType rightInput = right.setOutputType(AnyNumericDataType.instance, context);
if (leftInput.isAssignableTo(rightInput))
return rightInput;
else if (rightInput.isAssignableTo(leftInput))
return leftInput;
else
return getInputType(context);
Expand All @@ -95,9 +96,9 @@ protected void doVerify(VerificationContext context) {
private DataType resultingType(DataType left, DataType right) {
if (left == null || right == null)
return null;
if (!(left instanceof NumericDataType))
if ( ! (left instanceof NumericDataType))
throw new VerificationException(this, "The first argument must be a number, but has type " + left.getName());
if (!(right instanceof NumericDataType))
if ( ! (right instanceof NumericDataType))
throw new VerificationException(this, "The second argument must be a number, but has type " + right.getName());

if (left == DataType.FLOAT || left == DataType.DOUBLE || right == DataType.FLOAT || right == DataType.DOUBLE) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,9 @@ public DataType setInputType(DataType inputType, VerificationContext context) {

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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,10 +67,10 @@ else if ( ! embedders.containsKey(embedderId)) {
@Override
public DataType setInputType(DataType type, VerificationContext context) {
super.setInputType(type, context);
// TODO: Activate type checking
// 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 " + type);
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 " +
type.getName());
return getOutputType(context); // embed cannot determine the output type from the input
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,9 +64,8 @@ public void setStatementOutput(DocumentType documentType, Field field) {}
* @throws IllegalArgumentException if inputType isn't assignable to requiredType
*/
protected final DataType setInputType(DataType inputType, DataType requiredType, VerificationContext context) {
// TODO: Activate type checking
// if ( ! (inputType.isAssignableTo(requiredType))
// throw new VerificationException(this, "This requires type " + requiredType.getName() + ", but gets " + inputType.getName());
if ( ! (inputType.isAssignableTo(requiredType)))
throw new VerificationException(this, "This requires type " + requiredType.getName() + ", but gets " + inputType.getName());
return assignInputType(inputType);
}

Expand Down Expand Up @@ -119,11 +118,10 @@ public DataType requireOutputType() {
*/
protected final DataType setOutputType(DataType actualOutput, DataType requiredOutput, DataType requiredType,
VerificationContext context) {
// TODO: Activate type checking
// if (actualOutput != null && requiredOutput != null && ! actualOutput.isAssignableTo(requiredOutput))
// throw new VerificationException(this, "This produces type " + actualOutput.getName() + " but " + requiredOutput.getName() + " is required");
// if (requiredType != null && requiredOutput != null && ! requiredOutputOutput.isAssignableTo(requiredType))
// throw new VerificationException(this, "This is required to produce type " + requiredOutput.getName() + " but is produces " + requiredType.getName());;
if (actualOutput != null && requiredOutput != null && ! actualOutput.isAssignableTo(requiredOutput))
throw new VerificationException(this, "This produces type " + actualOutput.getName() + " but " + requiredOutput.getName() + " is required");
if (requiredType != null && requiredOutput != null && ! requiredOutput.isAssignableTo(requiredType))
throw new VerificationException(this, "This is required to produce type " + requiredOutput.getName() + " but is produces " + requiredType.getName());;
return assignOutputType(actualOutput != null ? actualOutput : requiredOutput); // Use the more precise type when known
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,11 +36,17 @@ protected void doVerify(VerificationContext context) {

private void setVariableType(DataType newType, VerificationContext context) {
DataType existingType = context.getVariable(varName);
if (existingType != null && ! newType.equals(existingType)) {
throw new VerificationException(this, "Cannot set variable '" + varName + "' to type " + newType.getName() +
": It is already set to type " + existingType.getName());
DataType mostGeneralType = newType;
if (existingType != null) {
if (existingType.isAssignableTo(newType))
mostGeneralType = newType;
else if (newType.isAssignableTo(existingType))
mostGeneralType = existingType;
else
throw new VerificationException(this, "Cannot set variable '" + varName + "' to type " + newType.getName() +
": It is already set to type " + existingType.getName());
}
context.setVariable(varName, newType);
context.setVariable(varName, mostGeneralType);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,8 @@ public DataType setInputType(DataType input, VerificationContext context) {
@Override
public DataType setOutputType(DataType output, VerificationContext context) {
super.setOutputType(output, context);
// TODO: Activate type checking
// if ( ! (output instanceof ArrayDataType) && output.getNestedType() == DataType.STRING)
// throw new VerificationException(this, "This produces a string array, but needs type " + output.getName());
if ( ! (output instanceof ArrayDataType) && output.getNestedType() == DataType.STRING)
throw new VerificationException(this, "This produces a string array, but " + output.getName() + " is required");
return DataType.STRING;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import com.yahoo.document.Field;
import com.yahoo.document.datatypes.Array;
import com.yahoo.document.datatypes.BoolFieldValue;
import com.yahoo.document.datatypes.FloatFieldValue;
import com.yahoo.document.datatypes.IntegerFieldValue;
import com.yahoo.document.datatypes.LongFieldValue;
import com.yahoo.document.datatypes.MapFieldValue;
Expand Down Expand Up @@ -218,4 +219,21 @@ public void testForEachFollowedByGetVar() {
assertEquals("value2", ((StringFieldValue)adapter.values.get("id")).getString());
}

@Test
public void testFloatAndIntArithmetic() {
var tester = new ScriptTester();
var expression = tester.expressionFrom("input myFloat * 10 | attribute myFloat");

SimpleTestAdapter adapter = new SimpleTestAdapter();
var myFloat = new Field("myFloat", DataType.FLOAT);
adapter.createField(myFloat);
adapter.setValue("myFloat", new FloatFieldValue(1.3f));

expression.verify(adapter);

ExecutionContext context = new ExecutionContext(adapter);
expression.execute(context);
assertEquals(13.0f, ((FloatFieldValue)adapter.values.get("myFloat")).getFloat(), 0.000001);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import com.yahoo.document.DataType;
import com.yahoo.document.datatypes.FieldValue;
import com.yahoo.document.datatypes.IntegerFieldValue;
import com.yahoo.vespa.indexinglanguage.ScriptTester;
import com.yahoo.vespa.indexinglanguage.SimpleTestAdapter;
import org.junit.Test;

Expand Down

0 comments on commit 50159e2

Please sign in to comment.