Skip to content

Commit

Permalink
Opt in number fields into fallback synthetic source when doc values a…
Browse files Browse the repository at this point in the history
…re disabled (#110160)

Contributes to #109546.
  • Loading branch information
lkts authored Jun 26, 2024
1 parent 8515c9a commit 63aae0d
Show file tree
Hide file tree
Showing 3 changed files with 71 additions and 34 deletions.
5 changes: 5 additions & 0 deletions docs/changelog/110160.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
pr: 110160
summary: Opt in number fields into fallback synthetic source when doc values a…
area: Mapping
type: enhancement
issues: []
Original file line number Diff line number Diff line change
Expand Up @@ -1988,29 +1988,28 @@ public void doValidate(MappingLookup lookup) {

@Override
protected SyntheticSourceMode syntheticSourceMode() {
return SyntheticSourceMode.NATIVE;
if (hasDocValues) {
return SyntheticSourceMode.NATIVE;
}

return SyntheticSourceMode.FALLBACK;
}

@Override
public SourceLoader.SyntheticFieldLoader syntheticFieldLoader() {
if (hasScript()) {
return SourceLoader.SyntheticFieldLoader.NOTHING;
}
if (hasDocValues == false) {
throw new IllegalArgumentException(
"field ["
+ fullPath()
+ "] of type ["
+ typeName()
+ "] doesn't support synthetic source because it doesn't have doc values"
);
}
if (copyTo.copyToFields().isEmpty() != true) {
throw new IllegalArgumentException(
"field [" + fullPath() + "] of type [" + typeName() + "] doesn't support synthetic source because it declares copy_to"
);
}
return type.syntheticFieldLoader(fullPath(), leafName(), ignoreMalformed.value());
if (hasDocValues) {
return type.syntheticFieldLoader(fullPath(), leafName(), ignoreMalformed.value());
}

return super.syntheticFieldLoader();
}

// For testing only:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@
import static org.hamcrest.Matchers.empty;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.matchesPattern;
import static org.hamcrest.Matchers.notANumber;

public abstract class NumberFieldMapperTests extends MapperTestCase {
Expand Down Expand Up @@ -376,6 +375,14 @@ public void testAllowMultipleValuesField() throws IOException {
assertThat(e.getCause().getMessage(), containsString("Only one field can be stored per key"));
}

@Override
protected BlockReaderSupport getSupportedReaders(MapperService mapper, String loaderFieldName) {
MappedFieldType ft = mapper.fieldType(loaderFieldName);
// Block loader can either use doc values or source.
// So with synthetic source it only works when doc values are enabled.
return new BlockReaderSupport(ft.hasDocValues(), ft.hasDocValues(), mapper, loaderFieldName);
}

@Override
protected Function<Object, Object> loadBlockExpected() {
return n -> ((Number) n); // Just assert it's a number
Expand All @@ -391,6 +398,7 @@ protected Matcher<?> blockItemMatcher(Object expected) {
protected final class NumberSyntheticSourceSupport implements SyntheticSourceSupport {
private final Long nullValue = usually() ? null : randomNumber().longValue();
private final boolean coerce = rarely();
private final boolean docValues = randomBoolean();

private final Function<Number, Number> round;
private final boolean ignoreMalformed;
Expand All @@ -400,10 +408,26 @@ protected NumberSyntheticSourceSupport(Function<Number, Number> round, boolean i
this.ignoreMalformed = ignoreMalformed;
}

@Override
public boolean preservesExactSource() {
// We opt in into fallback synthetic source if there is no doc values
// which preserves exact source.
return docValues == false;
}

@Override
public SyntheticSourceExample example(int maxVals) {
if (randomBoolean()) {
Tuple<Object, Object> v = generateValue();
if (preservesExactSource()) {
var rawInput = v.v1();

// This code actually runs with synthetic source disabled
// to test block loader loading from source.
// That's why we need to set expected block loader value here.
var blockLoaderResult = v.v2() instanceof Number n ? round.apply(n) : null;
return new SyntheticSourceExample(rawInput, rawInput, blockLoaderResult, this::mapping);
}
if (v.v2() instanceof Number n) {
Number result = round.apply(n);
return new SyntheticSourceExample(v.v1(), result, result, this::mapping);
Expand All @@ -413,19 +437,33 @@ public SyntheticSourceExample example(int maxVals) {
}
List<Tuple<Object, Object>> values = randomList(1, maxVals, this::generateValue);
List<Object> in = values.stream().map(Tuple::v1).toList();
List<Object> outList = values.stream()
.filter(v -> v.v2() instanceof Number)
.map(t -> round.apply((Number) t.v2()))
.sorted()
.collect(Collectors.toCollection(ArrayList::new));
values.stream().filter(v -> false == v.v2() instanceof Number).map(v -> v.v2()).forEach(outList::add);
Object out = outList.size() == 1 ? outList.get(0) : outList;

List<Object> outBlockList = values.stream()
.filter(v -> v.v2() instanceof Number)
.map(t -> round.apply((Number) t.v2()))
.sorted()
.collect(Collectors.toCollection(ArrayList::new));
Object out;
List<Object> outBlockList;
if (preservesExactSource()) {
// This code actually runs with synthetic source disabled
// to test block loader loading from source.
// That's why we need to set expected block loader value here.
out = in;
outBlockList = values.stream()
.filter(v -> v.v2() instanceof Number)
.map(t -> round.apply((Number) t.v2()))
.collect(Collectors.toCollection(ArrayList::new));
} else {
List<Object> outList = values.stream()
.filter(v -> v.v2() instanceof Number)
.map(t -> round.apply((Number) t.v2()))
.sorted()
.collect(Collectors.toCollection(ArrayList::new));
values.stream().filter(v -> false == v.v2() instanceof Number).map(Tuple::v2).forEach(outList::add);
out = outList.size() == 1 ? outList.get(0) : outList;

outBlockList = values.stream()
.filter(v -> v.v2() instanceof Number)
.map(t -> round.apply((Number) t.v2()))
.sorted()
.collect(Collectors.toCollection(ArrayList::new));
}

Object outBlock = outBlockList.size() == 1 ? outBlockList.get(0) : outBlockList;
return new SyntheticSourceExample(in, out, outBlock, this::mapping);
}
Expand Down Expand Up @@ -459,19 +497,14 @@ private void mapping(XContentBuilder b) throws IOException {
if (ignoreMalformed) {
b.field("ignore_malformed", true);
}
if (docValues == false) {
b.field("doc_values", "false");
}
}

@Override
public List<SyntheticSourceInvalidExample> invalidExample() throws IOException {
return List.of(
new SyntheticSourceInvalidExample(
matchesPattern("field \\[field] of type \\[.+] doesn't support synthetic source because it doesn't have doc values"),
b -> {
minimalMapping(b);
b.field("doc_values", false);
}
)
);
return List.of();
}
}
}

0 comments on commit 63aae0d

Please sign in to comment.