Skip to content

Make flattened synthetic source concatenate object keys on scalar/object mismatch #129600

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.

Sign up for GitHub

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

Merged
merged 14 commits into from
Jun 20, 2025
Merged

Make flattened synthetic source concatenate object keys on scalar/object mismatch #129600

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
438dc85
Initial working version
parkertimmins Jun 17, 2025
e9cbdc0
some cleanup
parkertimmins Jun 18, 2025
dd88ff7
move some stuff around
parkertimmins Jun 18, 2025
8ef6d51
revert some changes
parkertimmins Jun 18, 2025
529a510
cleanup tests
parkertimmins Jun 18, 2025
d0a3365
[CI] Auto commit changes from spotless
Jun 18, 2025
bd571fd
updates from feedback
parkertimmins Jun 18, 2025
e893e73
Merge branch 'main' into parker/flattened-scalar-object-mismatch
parkertimmins Jun 18, 2025
55480d4
add a few mapper tests
parkertimmins Jun 18, 2025
2ac1c55
Update docs/changelog/129600.yaml
parkertimmins Jun 18, 2025
c699bfd
updates from review
parkertimmins Jun 18, 2025
408393c
update docs
parkertimmins Jun 19, 2025
8ff8b82
Refactor main loop, improve comments, test, docs
parkertimmins Jun 19, 2025
e1457a3
Merge branch 'main' into parker/flattened-scalar-object-mismatch
parkertimmins Jun 20, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
210 changes: 81 additions & 129 deletions ...in/java/org/elasticsearch/index/mapper/flattened/FlattenedFieldSyntheticWriterHelper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@

import java.io.IOException;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
import java.util.Objects;

Expand All @@ -35,6 +34,7 @@
* "root.object.items": "102202929"
* "root.object.items": "290092911"
* "root.to": "Atlanta"
* "root.to.state": "Georgia"
*
* it turns them into the corresponding object using {@link XContentBuilder}.
* If, for instance JSON is used, it generates the following after deserialization:
Expand All @@ -50,7 +50,8 @@
* "object": {
* "items": ["102202929", "290092911"],
* },
* "to": "Atlanta"
* "to": "Atlanta",
* "to.state": "Georgia"
* }
* }`
*
Expand All @@ -60,63 +61,42 @@ public class FlattenedFieldSyntheticWriterHelper {
private record Prefix(List<String> prefix) {

Prefix() {
this(Collections.emptyList());
}

Prefix(final String key) {
this(key.split("\\."));
this(new ArrayList<>());
}

Prefix(String[] keyAsArray) {
this(List.of(keyAsArray).subList(0, keyAsArray.length - 1));
}

private Prefix shared(final Prefix other) {
return shared(this.prefix, other.prefix);
}

private static Prefix shared(final List<String> curr, final List<String> next) {
final List<String> shared = new ArrayList<>();
for (int i = 0; i < Math.min(curr.size(), next.size()); i++) {
if (curr.get(i).equals(next.get(i))) {
shared.add(curr.get(i));
private static int numObjectsToEnd(final List<String> curr, final List<String> next) {
int i = 0;
for (; i < Math.min(curr.size(), next.size()); i++) {
if (curr.get(i).equals(next.get(i)) == false) {
break;
}
}

return new Prefix(shared);
return Math.max(0, curr.size() - i);
}

private Prefix diff(final Prefix other) {
return diff(this.prefix, other.prefix);
}

private static Prefix diff(final List<String> a, final List<String> b) {
if (a.size() > b.size()) {
return diff(b, a);
}
final List<String> diff = new ArrayList<>();
if (a.isEmpty()) {
diff.addAll(b);
return new Prefix(diff);
}
assert a.size() >= b.size();
int i = 0;
for (; i < a.size(); i++) {
for (; i < b.size(); i++) {
if (a.get(i).equals(b.get(i)) == false) {
break;
}
}
for (; i < b.size(); i++) {
diff.add(b.get(i));
final List<String> diff = new ArrayList<>();
for (; i < a.size(); i++) {
diff.add(a.get(i));
}
return new Prefix(diff);
}

private Prefix reverse() {
final Prefix p = new Prefix(this.prefix);
Collections.reverse(p.prefix);
return p;
}

@Override
public int hashCode() {
return Objects.hash(this.prefix);
Expand All @@ -135,69 +115,32 @@ public boolean equals(Object obj) {
}
}

private record Suffix(List<String> suffix) {

Suffix() {
this(Collections.emptyList());
}

Suffix(final String key) {
this(key.split("\\."));
}

Suffix(final String[] keyAsArray) {
this(List.of(keyAsArray).subList(keyAsArray.length - 1, keyAsArray.length));
}

@Override
public int hashCode() {
return Objects.hash(this.suffix);
}

@Override
public boolean equals(Object obj) {
if (obj == null) {
return false;
}
if (getClass() != obj.getClass()) {
return false;
}
Suffix other = (Suffix) obj;
return Objects.equals(this.suffix, other.suffix);
}
}

private static class KeyValue {

public static final KeyValue EMPTY = new KeyValue(null, new Prefix(), new Suffix());
public static final KeyValue EMPTY = new KeyValue(null, new Prefix(), null);
private final String value;
private final Prefix prefix;
private final Suffix suffix;
private final String leaf;

private KeyValue(final String value, final Prefix prefix, final Suffix suffix) {
private KeyValue(final String value, final Prefix prefix, final String leaf) {
this.value = value;
this.prefix = prefix;
this.suffix = suffix;
this.leaf = leaf;
}

KeyValue(final BytesRef keyValue) {
this(FlattenedFieldParser.extractKey(keyValue).utf8ToString(), FlattenedFieldParser.extractValue(keyValue).utf8ToString());
}

private KeyValue(final String key, final String value) {
this(value, new Prefix(key), new Suffix(key));
}

public Suffix suffix() {
return this.suffix;
this(
FlattenedFieldParser.extractKey(keyValue).utf8ToString().split("\\."),
FlattenedFieldParser.extractValue(keyValue).utf8ToString()
);
}

public Prefix start(final KeyValue other) {
return this.prefix.diff(this.prefix.shared(other.prefix));
private KeyValue(final String[] key, final String value) {
this(value, new Prefix(key), key[key.length - 1]);
}

public Prefix end(final KeyValue other) {
return start(other).reverse();
public String leaf() {
return this.leaf;
}

public String value() {
Expand All @@ -207,7 +150,11 @@ public String value() {

@Override
public int hashCode() {
return Objects.hash(this.value, this.prefix, this.suffix);
return Objects.hash(this.value, this.prefix, this.leaf);
}

public boolean pathEquals(final KeyValue other) {
return prefix.equals(other.prefix) && leaf.equals(other.leaf);
}

@Override
Expand All @@ -221,7 +168,7 @@ public boolean equals(Object obj) {
KeyValue other = (KeyValue) obj;
return Objects.equals(this.value, other.value)
&& Objects.equals(this.prefix, other.prefix)
&& Objects.equals(this.suffix, other.suffix);
&& Objects.equals(this.leaf, other.leaf);
}
}

Expand All @@ -235,73 +182,78 @@ public FlattenedFieldSyntheticWriterHelper(final SortedKeyedValues sortedKeyedVa
this.sortedKeyedValues = sortedKeyedValues;
}

private String concatPath(Prefix prefix, String leaf) {
StringBuilder builder = new StringBuilder();
for (String part : prefix.prefix) {
builder.append(part).append(".");
}
builder.append(leaf);
return builder.toString();
}

public void write(final XContentBuilder b) throws IOException {
KeyValue curr = new KeyValue(sortedKeyedValues.next());
KeyValue prev = KeyValue.EMPTY;
final List<String> values = new ArrayList<>();
values.add(curr.value());
for (BytesRef nextValue = sortedKeyedValues.next(); nextValue != null; nextValue = sortedKeyedValues.next()) {
KeyValue next = new KeyValue(nextValue);
writeObject(b, curr, next, curr.start(prev), curr.end(next), values);
values.add(next.value());
prev = curr;
curr = next;
}
if (values.isEmpty() == false) {
writeObject(b, curr, KeyValue.EMPTY, curr.start(prev), curr.end(KeyValue.EMPTY), values);
}
}
var openObjects = new Prefix();
String lastScalarSingleLeaf = null;
KeyValue next;

do {
values.add(curr.value());
BytesRef nextValue = sortedKeyedValues.next();
next = nextValue == null ? KeyValue.EMPTY : new KeyValue(nextValue);

var startPrefix = curr.prefix.diff(openObjects);
if (startPrefix.prefix.isEmpty() == false && startPrefix.prefix.getFirst().equals(lastScalarSingleLeaf)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if the conflict doesn't happen on the first part of the prefix, e.g.

field {
  path {
    to: 10
    to {
      foo: bar
    }
  }
}

Would this be caught here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, so this will become:

field {
  path {
    to: 10
    to.foo: bar
 }
}

When it get to the first key/value field.path.to|10 it will take the else block and traverse down into the object, adding field and path to the openObject context. When it reaches the key value field.path.to.foo|bar that object will still be open, and seeing that lastScalarSingleLeaf has a value of to, and that to is the first token in the startPrexix (to.foo), it will make a concatenated path.

(Updated a test to this situation to verify it)

// In the open object, there is a leaf with a scalar value, which is also the first
// part of the current path. Instead of traversing down into the path and building objects,
// combine the path into a single leaf and add it as a field.
if (curr.pathEquals(next) == false) {
Copy link
Contributor

@lkts lkts Jun 18, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you explain what this if does to me?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This if is where we add fields to the output. Specifically, fields that need to be specified with the concatenated path due to a scalar/object mismatch. The similar if in the else block below serves the same purpose. The if is necessary to handle multiple values for the same field. For example if we have the following key/value pairs (where I'm using | instead of \0 to separate key and value):

foo|5
foo.bar|10
foo.bar|20
baz|99

Assume we've already written out foo|5. When we go through the loop for foo.bar|10, we'll add 10 to the values array (which will be empty) at the beginning of the loop. But when we get to this if statement, we'll see that the next pair (foo.bar|20) has the same path foo.bar. Because of this we won't enter the if and won't write out the field yet. In the next loop iteration, curr is now foo.bar|20. We'll add 20 to values, making it [10, 20]. When we get to this if statement, we see that next path is baz. Since this not equal to foo.bar we know that we have seen all values for foo.bar. So we enter the if statement and write out all values for the field.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a good example, consider adding it to the comment above, or splitting some of that before the first branch.

Copy link
Contributor

@lkts lkts Jun 19, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @parkertimmins, i think it's worth a comment too. Alternatively it sounds like we could restructure this to first read all the values for a field (move the if into a separate loop above this one) and then write them but this is not critical of course.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think gathering up all values for the same key in its own loop is a nice refactoring. I went ahead and added that. I'd like y'all to take another look at the main loop with that refactoring.

String combinedPath = concatPath(startPrefix, curr.leaf());
writeField(b, values, combinedPath);
}
} else {
// Traverse down into path, writing object keys to output, and adding to the openObject context.
startObject(b, startPrefix.prefix, openObjects.prefix);
if (curr.pathEquals(next) == false) {
lastScalarSingleLeaf = curr.leaf();
writeField(b, values, curr.leaf());
}
}

private static void writeObject(
final XContentBuilder b,
final KeyValue currKeyValue,
final KeyValue nextKeyValue,
final Prefix startPrefix,
final Prefix endPrefix,
final List<String> values
) throws IOException {
startObject(b, startPrefix.prefix);
if (currKeyValue.suffix.equals(nextKeyValue.suffix) == false) {
writeNestedObject(b, values, currKeyValue.suffix().suffix);
}
endObject(b, endPrefix.prefix);
}
int numObjectsToEnd = Prefix.numObjectsToEnd(openObjects.prefix, next.prefix.prefix);
endObject(b, numObjectsToEnd, openObjects.prefix);

private static void writeNestedObject(final XContentBuilder b, final List<String> values, final List<String> currSuffix)
throws IOException {
for (int i = 0; i < currSuffix.size() - 1; i++) {
b.startObject(currSuffix.get(i));
}
writeField(b, values, currSuffix);
for (int i = 0; i < currSuffix.size() - 1; i++) {
b.endObject();
}
values.clear();
curr = next;
} while (next.equals(KeyValue.EMPTY) == false);
}

private static void endObject(final XContentBuilder b, final List<String> objects) throws IOException {
for (final String ignored : objects) {
private static void endObject(final XContentBuilder b, int numObjectsToClose, List<String> openObjects) throws IOException {
for (int i = 0; i < numObjectsToClose; i++) {
b.endObject();
openObjects.removeLast();
}
}

private static void startObject(final XContentBuilder b, final List<String> objects) throws IOException {
private static void startObject(final XContentBuilder b, final List<String> objects, List<String> openObjects) throws IOException {
for (final String object : objects) {
b.startObject(object);
openObjects.add(object);
}
}

private static void writeField(XContentBuilder b, List<String> values, List<String> currSuffix) throws IOException {
private static void writeField(XContentBuilder b, List<String> values, String leaf) throws IOException {
if (values.size() > 1) {
b.field(currSuffix.get(currSuffix.size() - 1), values);
b.field(leaf, values);
} else {
// NOTE: here we make the assumption that fields with just one value are not arrays.
// Flattened fields have no mappings, and even if we had mappings, there is no way
// in Elasticsearch to distinguish single valued fields from multi-valued fields.
// As a result, there is no way to know, after reading a single value, if that value
// is the value for a single-valued field or a multi-valued field (array) with just
// one value (array of size 1).
b.field(currSuffix.get(currSuffix.size() - 1), values.get(0));
b.field(leaf, values.getFirst());
}
values.clear();
}
}
56 changes: 55 additions & 1 deletion ...va/org/elasticsearch/index/mapper/flattened/FlattenedFieldSyntheticWriterHelperTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ public void testSingleObject() throws IOException {

// THEN
assertEquals(
"{\"a\":\"value_a\",\"a\":{\"b\":\"value_b\",\"b\":{\"c\":\"value_c\"},\"d\":\"value_d\"}}",
"{\"a\":\"value_a\",\"a.b\":\"value_b\",\"a.b.c\":\"value_c\",\"a.d\":\"value_d\"}",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interestingly, there was a test which had a scalar/object mismatch. But it produced duplicate keys. When the xcontent was converted to jsont these duplicate keys originally threw an error, but now just drop the duplicates. (Something must have changed in xcontent stuff since the issue was opened to cause this change from an error to deduplication)

baos.toString(StandardCharsets.UTF_8)
);
}
Expand Down Expand Up @@ -192,6 +192,60 @@ public void testMultipleArrays() throws IOException {
assertEquals("{\"a\":{\"x\":[\"10\",\"20\"]},\"b\":{\"y\":[\"30\",\"40\",\"50\"]}}", baos.toString(StandardCharsets.UTF_8));
}

public void testSameLeafDifferentPrefix() throws IOException {
// GIVEN
final SortedSetDocValues dv = mock(SortedSetDocValues.class);
final FlattenedFieldSyntheticWriterHelper writer = new FlattenedFieldSyntheticWriterHelper(new SortedSetSortedKeyedValues(dv));
final ByteArrayOutputStream baos = new ByteArrayOutputStream();
final XContentBuilder builder = new XContentBuilder(XContentType.JSON.xContent(), baos);
final List<byte[]> bytes = List.of("a.x" + '\0' + "10", "b.x" + '\0' + "20")
.stream()
.map(x -> x.getBytes(StandardCharsets.UTF_8))
.collect(Collectors.toList());
when(dv.getValueCount()).thenReturn(Long.valueOf(bytes.size()));
when(dv.docValueCount()).thenReturn(bytes.size());
when(dv.nextOrd()).thenReturn(0L, 1L);
for (int i = 0; i < bytes.size(); i++) {
when(dv.lookupOrd(ArgumentMatchers.eq((long) i))).thenReturn(new BytesRef(bytes.get(i), 0, bytes.get(i).length));
}

// WHEN
builder.startObject();
writer.write(builder);
builder.endObject();
builder.flush();

// THEN
assertEquals("{\"a\":{\"x\":\"10\"},\"b\":{\"x\":\"20\"}}", baos.toString(StandardCharsets.UTF_8));
}

public void testScalarObjectMismatchInNestedObject() throws IOException {
// GIVEN
final SortedSetDocValues dv = mock(SortedSetDocValues.class);
final FlattenedFieldSyntheticWriterHelper writer = new FlattenedFieldSyntheticWriterHelper(new SortedSetSortedKeyedValues(dv));
final ByteArrayOutputStream baos = new ByteArrayOutputStream();
final XContentBuilder builder = new XContentBuilder(XContentType.JSON.xContent(), baos);
final List<byte[]> bytes = List.of("a.b" + '\0' + "10", "a.b.c" + '\0' + "20")
.stream()
.map(x -> x.getBytes(StandardCharsets.UTF_8))
.collect(Collectors.toList());
when(dv.getValueCount()).thenReturn(Long.valueOf(bytes.size()));
when(dv.docValueCount()).thenReturn(bytes.size());
when(dv.nextOrd()).thenReturn(0L, 1L);
for (int i = 0; i < bytes.size(); i++) {
when(dv.lookupOrd(ArgumentMatchers.eq((long) i))).thenReturn(new BytesRef(bytes.get(i), 0, bytes.get(i).length));
}

// WHEN
builder.startObject();
writer.write(builder);
builder.endObject();
builder.flush();

// THEN
assertEquals("{\"a\":{\"b\":\"10\",\"b.c\":\"20\"}}", baos.toString(StandardCharsets.UTF_8));
}

private class SortedSetSortedKeyedValues implements FlattenedFieldSyntheticWriterHelper.SortedKeyedValues {
private final SortedSetDocValues dv;
private int seen = 0;
Expand Down