Skip to content

Commit 0919e47

Browse files
hmusumhmusum
authored
Merge pull request #33223 from vespa-engine/revert-33179-bratseth/type-inference-cleanup
Revert "Bratseth/type inference cleanup"
2 parents b1e2487 + 44ca512 commit 0919e47

File tree

69 files changed

+371
-284
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

69 files changed

+371
-284
lines changed

‎config-model/src/main/java/com/yahoo/schema/processing/ExactMatch.java

+3-2
Original file line numberDiff line numberDiff line change
@@ -90,9 +90,10 @@ private void exactMatchSettingsForField(SDField field) {
9090

9191
private static class MyProvider extends TypedTransformProvider {
9292

93-
private final int maxTokenLength;
93+
private int maxTokenLength;
9494

95-
MyProvider(Schema schema, int maxTokenLength) {
95+
MyProvider(Schema schema, int maxTokenLength)
96+
{
9697
super(ExactExpression.class, schema);
9798
this.maxTokenLength = maxTokenLength;
9899
}

‎config-model/src/main/java/com/yahoo/schema/processing/IndexingValidation.java

+12-9
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ protected boolean shouldConvert(Expression expression) {
7575
outputs.add(fieldName);
7676
prevNames.add(fieldName);
7777
}
78-
if (expression.isMutating()) {
78+
if (expression.createdOutputType() != null) {
7979
prevNames.clear();
8080
}
8181
return false;
@@ -98,8 +98,9 @@ private static class MyAdapter implements FieldTypeAdapter {
9898
@Override
9999
public DataType getInputType(Expression exp, String fieldName) {
100100
SDField field = schema.getDocumentField(fieldName);
101-
if (field == null)
102-
throw new VerificationException(exp, "Input field '" + fieldName + "' not found");
101+
if (field == null) {
102+
throw new VerificationException(exp, "Input field '" + fieldName + "' not found.");
103+
}
103104
return field.getDataType();
104105
}
105106

@@ -109,14 +110,16 @@ public void tryOutputType(Expression expression, String fieldName, DataType valu
109110
DataType fieldType;
110111
if (expression instanceof AttributeExpression) {
111112
Attribute attribute = schema.getAttribute(fieldName);
112-
if (attribute == null)
113-
throw new VerificationException(expression, "Attribute '" + fieldName + "' not found");
113+
if (attribute == null) {
114+
throw new VerificationException(expression, "Attribute '" + fieldName + "' not found.");
115+
}
114116
fieldDesc = "attribute";
115117
fieldType = attribute.getDataType();
116118
} else if (expression instanceof IndexExpression) {
117119
SDField field = schema.getConcreteField(fieldName);
118-
if (field == null)
119-
throw new VerificationException(expression, "Index field '" + fieldName + "' not found");
120+
if (field == null) {
121+
throw new VerificationException(expression, "Index field '" + fieldName + "' not found.");
122+
}
120123
fieldDesc = "index field";
121124
fieldType = field.getDataType();
122125
} else if (expression instanceof SummaryExpression) {
@@ -128,7 +131,7 @@ public void tryOutputType(Expression expression, String fieldName, DataType valu
128131
fieldDesc = "document field";
129132
fieldType = sdField.getDataType();
130133
} else {
131-
throw new VerificationException(expression, "Summary field '" + fieldName + "' not found");
134+
throw new VerificationException(expression, "Summary field '" + fieldName + "' not found.");
132135
}
133136
} else {
134137
fieldDesc = "summary field";
@@ -139,7 +142,7 @@ public void tryOutputType(Expression expression, String fieldName, DataType valu
139142
}
140143
if ( ! fieldType.isAssignableFrom(valueType))
141144
throw new VerificationException(expression, "Can not assign " + valueType.getName() + " to " + fieldDesc +
142-
" '" + fieldName + "' which is " + fieldType.getName());
145+
" '" + fieldName + "' which is " + fieldType.getName() + ".");
143146
}
144147

145148
}

‎config-model/src/main/java/com/yahoo/schema/processing/IndexingValues.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ protected boolean shouldConvert(Expression exp) {
5757
}
5858
if (exp instanceof InputExpression && ((InputExpression)exp).getFieldName().equals(field.getName())) {
5959
mutatedBy = null;
60-
} else if (exp.isMutating()) {
60+
} else if (exp.createdOutputType() != null) {
6161
mutatedBy = exp;
6262
}
6363
return false;

‎container-search/src/test/java/com/yahoo/search/yql/YqlParserTestCase.java

-8
Original file line numberDiff line numberDiff line change
@@ -581,14 +581,6 @@ void testConnectivity() {
581581
"which does not exist in the query."));
582582
}
583583

584-
@Test
585-
void testWeight() {
586-
QueryTree parsed = parse("select * from sources * where " +
587-
"weakAnd(field1 contains ({weight: 120}'term1'), " +
588-
" field1 contains ({weight: 70}'term2'))");
589-
assertEquals("WEAKAND(100) field1:term1!120 field1:term2!70", parsed.toString());
590-
}
591-
592584
@Test
593585
void testAnnotatedPhrase() {
594586
QueryTree parsed =

‎indexinglanguage/src/main/java/com/yahoo/vespa/indexinglanguage/FieldValueConverter.java

+19-7
Original file line numberDiff line numberDiff line change
@@ -12,14 +12,26 @@
1212
*/
1313
public abstract class FieldValueConverter {
1414

15-
@SuppressWarnings({ "rawtypes", "unchecked" })
15+
@SuppressWarnings({ "unchecked" })
1616
public final FieldValue convert(FieldValue value) {
17-
if (value == null) return null;
18-
if (shouldConvert(value)) return doConvert(value);
19-
if (value instanceof Array arrayValue) return convertArray(arrayValue);
20-
if (value instanceof MapFieldValue mapValue) return convertMap(mapValue);
21-
if (value instanceof WeightedSet weightedSetValue) return convertWset(weightedSetValue);
22-
if (value instanceof StructuredFieldValue structuredFieldValue) return convertStructured(structuredFieldValue);
17+
if (value == null) {
18+
return null;
19+
}
20+
if (shouldConvert(value)) {
21+
return doConvert(value);
22+
}
23+
if (value instanceof Array) {
24+
return convertArray((Array)value);
25+
}
26+
if (value instanceof MapFieldValue) {
27+
return convertMap((MapFieldValue)value);
28+
}
29+
if (value instanceof WeightedSet) {
30+
return convertWset((WeightedSet)value);
31+
}
32+
if (value instanceof StructuredFieldValue) {
33+
return convertStructured((StructuredFieldValue)value);
34+
}
2335
return value;
2436
}
2537

‎indexinglanguage/src/main/java/com/yahoo/vespa/indexinglanguage/ValueTransformProvider.java

+12-4
Original file line numberDiff line numberDiff line change
@@ -25,17 +25,25 @@ public final ExpressionConverter branch() {
2525
}
2626

2727
@Override
28-
protected final boolean shouldConvert(Expression expression) {
29-
if (transformClass.isInstance(expression)) {
28+
protected final boolean shouldConvert(Expression exp) {
29+
if (transformClass.isInstance(exp)) {
3030
if (transformed) {
3131
duplicate = true;
3232
return true;
3333
}
3434
transformed = true;
3535
return false;
3636
}
37-
if ( ! requiresTransform(expression)) return false;
38-
if (transformed) return false;
37+
if (exp.createdOutputType() != null) {
38+
transformed = false;
39+
return false;
40+
}
41+
if ( ! requiresTransform(exp)) {
42+
return false;
43+
}
44+
if (transformed) {
45+
return false;
46+
}
3947
return true;
4048
}
4149

‎indexinglanguage/src/main/java/com/yahoo/vespa/indexinglanguage/expressions/BusyWaitExpression.java

-3
Original file line numberDiff line numberDiff line change
@@ -12,9 +12,6 @@
1212
*/
1313
public final class BusyWaitExpression extends Expression {
1414

15-
@Override
16-
public boolean isMutating() { return false; }
17-
1815
@Override
1916
protected void doExecute(ExecutionContext context) {
2017
FieldValue value = context.getCurrentValue();

‎indexinglanguage/src/main/java/com/yahoo/vespa/indexinglanguage/expressions/ConstantExpression.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ public DataType setOutputType(DataType outputType, VerificationContext context)
3737
throw new VerificationException(this, "Produces type " + value.getDataType().getName() + ", but type " +
3838
outputType.getName() + " is required");
3939
super.setOutputType(outputType, context);
40-
return AnyDataType.instance;
40+
return null;
4141
}
4242

4343
@Override

‎indexinglanguage/src/main/java/com/yahoo/vespa/indexinglanguage/expressions/EchoExpression.java

-3
Original file line numberDiff line numberDiff line change
@@ -20,9 +20,6 @@ public EchoExpression(PrintStream out) {
2020
this.out = out;
2121
}
2222

23-
@Override
24-
public boolean isMutating() { return false; }
25-
2623
public PrintStream getOutputStream() { return out; }
2724

2825
@Override

‎indexinglanguage/src/main/java/com/yahoo/vespa/indexinglanguage/expressions/ExactExpression.java

-3
Original file line numberDiff line numberDiff line change
@@ -37,9 +37,6 @@ public ExactExpression(int maxTokenLength) {
3737
this(OptionalInt.of(maxTokenLength));
3838
}
3939

40-
@Override
41-
public boolean isMutating() { return false; }
42-
4340
@Override
4441
public DataType setInputType(DataType inputType, VerificationContext context) {
4542
return super.setInputType(inputType, DataType.STRING, context);

‎indexinglanguage/src/main/java/com/yahoo/vespa/indexinglanguage/expressions/Expression.java

+46-21
Original file line numberDiff line numberDiff line change
@@ -44,12 +44,6 @@ public abstract class Expression extends Selectable {
4444
/** Returns whether this expression requires an input value. */
4545
public boolean requiresInput() { return true; }
4646

47-
/**
48-
* Returns whether this expression outputs a different value than what it gets as input.
49-
* Annotating a string value does not count as modifying it.
50-
*/
51-
public boolean isMutating() { return true; }
52-
5347
/**
5448
* Returns an expression where the children of this has been converted using the given converter.
5549
* This default implementation returns this as it has no children.
@@ -158,38 +152,50 @@ public final void verify(DocumentType type) {
158152
verify(new DocumentTypeAdapter(type));
159153
}
160154

161-
public final void verify(Document doc) {
162-
verify(new SimpleAdapterFactory(), doc);
155+
public final Document verify(Document doc) {
156+
return verify(new SimpleAdapterFactory(), doc);
163157
}
164158

165-
public final void verify(AdapterFactory factory, Document doc) {
166-
verify(factory.newDocumentAdapter(doc));
159+
public final Document verify(AdapterFactory factory, Document doc) {
160+
return verify(factory.newDocumentAdapter(doc));
167161
}
168162

169-
public final void verify(DocumentAdapter adapter) {
163+
public final Document verify(DocumentAdapter adapter) {
170164
verify((FieldTypeAdapter)adapter);
171-
adapter.getFullOutput();
165+
return adapter.getFullOutput();
172166
}
173167

174-
public final void verify(DocumentUpdate upd) {
175-
verify(new SimpleAdapterFactory(), upd);
168+
public final DocumentUpdate verify(DocumentUpdate upd) {
169+
return verify(new SimpleAdapterFactory(), upd);
176170
}
177171

178-
public final void verify(AdapterFactory factory, DocumentUpdate upd) {
179-
for (UpdateAdapter adapter : factory.newUpdateAdapterList(upd))
180-
verify(adapter);
172+
public final DocumentUpdate verify(AdapterFactory factory, DocumentUpdate upd) {
173+
DocumentUpdate ret = null;
174+
for (UpdateAdapter adapter : factory.newUpdateAdapterList(upd)) {
175+
DocumentUpdate output = verify(adapter);
176+
if (output == null) {
177+
// ignore
178+
} else if (ret != null) {
179+
ret.addAll(output);
180+
} else {
181+
ret = output;
182+
}
183+
}
184+
return ret;
181185
}
182186

183-
public final void verify(UpdateAdapter adapter) {
187+
public final DocumentUpdate verify(UpdateAdapter adapter) {
184188
verify((FieldTypeAdapter)adapter);
189+
return adapter.getOutput();
185190
}
186191

187-
public final void verify(FieldTypeAdapter adapter) {
188-
verify(new VerificationContext(adapter));
192+
public final DataType verify(FieldTypeAdapter adapter) {
193+
return verify(new VerificationContext(adapter));
189194
}
190195

191-
public final void verify(VerificationContext context) {
196+
public final DataType verify(VerificationContext context) {
192197
doVerify(context);
198+
return context.getCurrentType();
193199
}
194200

195201
protected void doVerify(VerificationContext context) {}
@@ -240,6 +246,14 @@ public final FieldValue execute(ExecutionContext context) {
240246
if (input == null) return null;
241247
}
242248
doExecute(context);
249+
DataType outputType = createdOutputType();
250+
if (outputType != null) {
251+
FieldValue output = context.getCurrentValue();
252+
if (output != null && !outputType.isValueCompatible(output)) {
253+
throw new IllegalStateException("Expression '" + this + "' expected " + outputType.getName() +
254+
" output, got " + output.getDataType().getName());
255+
}
256+
}
243257
return context.getCurrentValue();
244258
}
245259

@@ -262,6 +276,17 @@ public static Expression newInstance(ScriptParserContext context) throws ParseEx
262276
return ScriptParser.parseExpression(context);
263277
}
264278

279+
protected static boolean equals(Object lhs, Object rhs) {
280+
if (lhs == null) {
281+
return rhs == null;
282+
} else {
283+
if (rhs == null) {
284+
return false;
285+
}
286+
return lhs.equals(rhs);
287+
}
288+
}
289+
265290
// Convenience For testing
266291
public static Document execute(Expression expression, Document doc) {
267292
expression.verify(doc);

‎indexinglanguage/src/main/java/com/yahoo/vespa/indexinglanguage/expressions/ForEachExpression.java

+7-6
Original file line numberDiff line numberDiff line change
@@ -32,9 +32,6 @@ public ForEachExpression(Expression expression) {
3232
this.expression = Objects.requireNonNull(expression);
3333
}
3434

35-
@Override
36-
public boolean isMutating() { return expression.isMutating(); }
37-
3835
public Expression getInnerExpression() { return expression; }
3936

4037
@Override
@@ -170,8 +167,12 @@ protected void doExecute(ExecutionContext context) {
170167
FieldValue input = context.getCurrentValue();
171168
if (input instanceof Array || input instanceof WeightedSet) {
172169
FieldValue next = new ExecutionConverter(context, expression).convert(input);
173-
if (next == null)
174-
next = getOutputType().createFieldValue();
170+
if (next == null) {
171+
VerificationContext verificationContext = new VerificationContext(context.getFieldValue());
172+
context.fillVariableTypes(verificationContext);
173+
verificationContext.setCurrentType(input.getDataType()).verify(this);
174+
next = verificationContext.getCurrentType().createFieldValue();
175+
}
175176
context.setCurrentValue(next);
176177
} else if (input instanceof Struct || input instanceof Map) {
177178
context.setCurrentValue(new ExecutionConverter(context, expression).convert(input));
@@ -226,7 +227,7 @@ protected boolean shouldConvert(FieldValue value) {
226227
/** Converts a map into an array by passing each entry through the expression. */
227228
@Override
228229
protected FieldValue convertMap(MapFieldValue<FieldValue, FieldValue> map) {
229-
var values = new Array<>(new ArrayDataType(expression.getOutputType()), map.size());
230+
var values = new Array<>(new ArrayDataType(expression.createdOutputType()), map.size());
230231
for (var entry : map.entrySet())
231232
values.add(doConvert(new MapEntryFieldValue(entry.getKey(), entry.getValue())));
232233
return values;

‎indexinglanguage/src/main/java/com/yahoo/vespa/indexinglanguage/expressions/GuardExpression.java

-3
Original file line numberDiff line numberDiff line change
@@ -23,9 +23,6 @@ public GuardExpression(Expression innerExpression) {
2323
shouldExecute = shouldExecute(innerExpression);
2424
}
2525

26-
@Override
27-
public boolean isMutating() { return innerExpression.isMutating(); }
28-
2926
@Override
3027
public boolean requiresInput() { return innerExpression.requiresInput(); }
3128

‎indexinglanguage/src/main/java/com/yahoo/vespa/indexinglanguage/expressions/IfThenExpression.java

+1-7
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@
1111
import com.yahoo.vespa.objects.ObjectPredicate;
1212

1313
import java.math.BigDecimal;
14-
import java.util.Objects;
1514

1615
/**
1716
* @author Simon Thoresen Hult
@@ -57,11 +56,6 @@ public IfThenExpression(Expression lhs, Comparator cmp, Expression right, Expres
5756
this.ifFalse = ifFalse;
5857
}
5958

60-
@Override
61-
public boolean isMutating() {
62-
return ifTrue.isMutating() || (ifFalse != null && ifFalse.isMutating());
63-
}
64-
6559
@Override
6660
public boolean requiresInput() {
6761
return left.requiresInput() || right.requiresInput() || ifTrue.requiresInput() || (ifFalse != null && ifFalse.requiresInput());
@@ -184,7 +178,7 @@ public boolean equals(Object obj) {
184178
if ( ! comparator.equals(exp.comparator)) return false;
185179
if ( ! right.equals(exp.right)) return false;
186180
if ( ! ifTrue.equals(exp.ifTrue)) return false;
187-
if ( ! Objects.equals(ifFalse, exp.ifFalse)) return false;
181+
if ( ! equals(ifFalse, exp.ifFalse)) return false;
188182
return true;
189183
}
190184

0 commit comments

Comments
 (0)