Skip to content
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

Share data between DocValuesField and ScriptDocValues #79587

Merged
merged 9 commits into from
Oct 25, 2021
Merged

Share data between DocValuesField and ScriptDocValues #79587

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
1070208
Share data between DocValuesField and ScriptDocValues to prevent double
jdconrad Oct 20, 2021
ed4d852
fix tests
jdconrad Oct 20, 2021
a486e44
Merge branch 'master' into sdvshare
jdconrad Oct 20, 2021
2709d7f
fix import
jdconrad Oct 20, 2021
cbb541b
Merge branch 'master' into sdvshare
jdconrad Oct 21, 2021
902948b
Merge branch 'master' into sdvshare
jdconrad Oct 21, 2021
8ebf0bc
add delegate unsupported methods to allow list and test
jdconrad Oct 21, 2021
224a36c
Merge branch 'master' into sdvshare
jdconrad Oct 25, 2021
384d2af
response to pr feedback
jdconrad Oct 25, 2021
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
7 changes: 6 additions & 1 deletion ...ainless/src/main/resources/org/elasticsearch/painless/org.elasticsearch.script.fields.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,5 +16,10 @@ class org.elasticsearch.script.field.Field @dynamic_type {
}

class org.elasticsearch.script.DocBasedScript {
org.elasticsearch.script.field.Field field(String)
org.elasticsearch.script.field.Field field(String)
}

class org.elasticsearch.script.field.DelegateDocValuesField @dynamic_type {
def getValue(def)
Copy link
Contributor

@stu-elastic stu-elastic Oct 25, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Why did we need to add this now? I would have figured we needed this when adding unsigned long?

Copy link
Contributor Author

@jdconrad jdconrad Oct 25, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Delegate field isn't used for unsigned long; it's only used for types that we still provide through only doc. The base Field class doesn't have a getValue or getValues. I added these methods here so a user attempting to access a field type we don't have a new mapped type for, will know they have to use doc.

Edit:
We removed getValue and getValues from the base Field class so that sub classes can return primitives directly with getValue. Dynamic languages can provide this automatically, and non-dynamic languages would require a cast to get the appropriate value type either way, so it didn't make sense to keep them.

List getValues()
}
29 changes: 29 additions & 0 deletions ...s/lang-painless/src/yamlRestTest/resources/rest-api-spec/test/painless/20_scriptfield.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -154,3 +154,32 @@ setup:
- match: { error.failed_shards.0.reason.caused_by.reason: "no paths escape from while loop" }
- match: { error.failed_shards.0.reason.type: "script_exception" }
- match: { error.failed_shards.0.reason.reason: "compile error" }

---
"Scripted Field with error accessing an unsupported field via the script fields api":
- do:
catch: bad_request
search:
rest_total_hits_as_int: true
body:
script_fields:
bar:
script:
source: "field('foo').getValue('')"

- match: { error.failed_shards.0.reason.caused_by.type: "unsupported_operation_exception" }
- match: { error.failed_shards.0.reason.caused_by.reason: "field [foo] is not supported through the fields api, use [doc] instead"}

- do:
catch: bad_request
search:
rest_total_hits_as_int: true
body:
script_fields:
bar:
script:
source: "field('foo').getValues()"

- match: { error.failed_shards.0.reason.caused_by.type: "unsupported_operation_exception" }
- match: { error.failed_shards.0.reason.caused_by.reason: "field [foo] is not supported through the fields api, use [doc] instead" }

6 changes: 4 additions & 2 deletions ...er-extras/src/main/java/org/elasticsearch/index/mapper/extras/ScaledFloatFieldMapper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@
import org.elasticsearch.common.Explicit;
import org.elasticsearch.common.settings.Setting;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.script.field.DelegateDocValuesField;
import org.elasticsearch.script.field.DocValuesField;
import org.elasticsearch.xcontent.XContentParser;
import org.elasticsearch.xcontent.XContentParser.Token;
import org.elasticsearch.common.xcontent.support.XContentMapValues;
Expand Down Expand Up @@ -505,8 +507,8 @@ private static class ScaledFloatLeafFieldData implements LeafNumericFieldData {
}

@Override
public ScriptDocValues.Doubles getScriptValues() {
return new ScriptDocValues.Doubles(getDoubleValues());
public DocValuesField getScriptField(String name) {
return new DelegateDocValuesField(new ScriptDocValues.Doubles(getDoubleValues()), name);
}

@Override
Expand Down
6 changes: 4 additions & 2 deletions server/src/main/java/org/elasticsearch/index/fielddata/IpScriptFieldData.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@
import org.elasticsearch.index.mapper.IpFieldMapper;
import org.elasticsearch.indices.breaker.CircuitBreakerService;
import org.elasticsearch.script.IpFieldScript;
import org.elasticsearch.script.field.DelegateDocValuesField;
import org.elasticsearch.script.field.DocValuesField;
import org.elasticsearch.search.aggregations.support.CoreValuesSourceType;
import org.elasticsearch.search.aggregations.support.ValuesSourceType;

Expand Down Expand Up @@ -50,8 +52,8 @@ public BinaryScriptLeafFieldData loadDirect(LeafReaderContext context) throws Ex
IpFieldScript script = leafFactory.newInstance(context);
return new BinaryScriptLeafFieldData() {
@Override
public ScriptDocValues<String> getScriptValues() {
return new IpScriptDocValues(getBytesValues());
public DocValuesField getScriptField(String name) {
return new DelegateDocValuesField(new IpScriptDocValues(getBytesValues()), name);
}

@Override
Expand Down
9 changes: 1 addition & 8 deletions server/src/main/java/org/elasticsearch/index/fielddata/LeafFieldData.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,17 +20,10 @@
*/
public interface LeafFieldData extends Accountable, Releasable {

/**
* Returns field values for use in scripting.
*/
ScriptDocValues<?> getScriptValues();

/**
* Returns an {@code Field} for use in accessing field values in scripting.
*/
default DocValuesField getScriptField(String name) {
throw new UnsupportedOperationException();
}
DocValuesField getScriptField(String name);

/**
* Return a String representation of the values.
Expand Down
6 changes: 4 additions & 2 deletions server/src/main/java/org/elasticsearch/index/fielddata/StringScriptFieldData.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@
import org.apache.lucene.index.LeafReaderContext;
import org.elasticsearch.indices.breaker.CircuitBreakerService;
import org.elasticsearch.script.StringFieldScript;
import org.elasticsearch.script.field.DelegateDocValuesField;
import org.elasticsearch.script.field.DocValuesField;
import org.elasticsearch.search.aggregations.support.CoreValuesSourceType;
import org.elasticsearch.search.aggregations.support.ValuesSourceType;

Expand Down Expand Up @@ -42,8 +44,8 @@ public BinaryScriptLeafFieldData loadDirect(LeafReaderContext context) throws Ex
StringFieldScript script = leafFactory.newInstance(context);
return new BinaryScriptLeafFieldData() {
@Override
public ScriptDocValues<?> getScriptValues() {
return new ScriptDocValues.Strings(getBytesValues());
public DocValuesField getScriptField(String name) {
return new DelegateDocValuesField(new ScriptDocValues.Strings(getBytesValues()), name);
}

@Override
Expand Down
6 changes: 4 additions & 2 deletions .../src/main/java/org/elasticsearch/index/fielddata/plain/AbstractLeafGeoPointFieldData.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@
import org.elasticsearch.index.fielddata.MultiGeoPointValues;
import org.elasticsearch.index.fielddata.ScriptDocValues;
import org.elasticsearch.index.fielddata.SortedBinaryDocValues;
import org.elasticsearch.script.field.DelegateDocValuesField;
import org.elasticsearch.script.field.DocValuesField;

import java.util.Collection;
import java.util.Collections;
Expand All @@ -25,8 +27,8 @@ public final SortedBinaryDocValues getBytesValues() {
}

@Override
public final ScriptDocValues.GeoPoints getScriptValues() {
return new ScriptDocValues.GeoPoints(getGeoPointValues());
public final DocValuesField getScriptField(String name) {
return new DelegateDocValuesField(new ScriptDocValues.GeoPoints(getGeoPointValues()), name);
}

public static LeafGeoPointFieldData empty(final int maxDoc) {
Expand Down
6 changes: 4 additions & 2 deletions .../src/main/java/org/elasticsearch/index/fielddata/plain/AbstractLeafOrdinalsFieldData.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@
import org.elasticsearch.index.fielddata.FieldData;
import org.elasticsearch.index.fielddata.ScriptDocValues;
import org.elasticsearch.index.fielddata.SortedBinaryDocValues;
import org.elasticsearch.script.field.DelegateDocValuesField;
import org.elasticsearch.script.field.DocValuesField;

import java.util.Collection;
import java.util.Collections;
Expand All @@ -34,8 +36,8 @@ protected AbstractLeafOrdinalsFieldData(Function<SortedSetDocValues, ScriptDocVa
}

@Override
public final ScriptDocValues<?> getScriptValues() {
return scriptFunction.apply(getOrdinalsValues());
public final DocValuesField getScriptField(String name) {
return new DelegateDocValuesField(scriptFunction.apply(getOrdinalsValues()), name);
}

@Override
Expand Down
9 changes: 5 additions & 4 deletions server/src/main/java/org/elasticsearch/index/fielddata/plain/BinaryDVLeafFieldData.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,12 @@
import org.apache.lucene.index.DocValues;
import org.apache.lucene.index.LeafReader;
import org.apache.lucene.util.Accountable;
import org.elasticsearch.index.fielddata.LeafFieldData;
import org.elasticsearch.index.fielddata.FieldData;
import org.elasticsearch.index.fielddata.LeafFieldData;
import org.elasticsearch.index.fielddata.ScriptDocValues;
import org.elasticsearch.index.fielddata.ScriptDocValues.Strings;
import org.elasticsearch.index.fielddata.SortedBinaryDocValues;
import org.elasticsearch.script.field.DelegateDocValuesField;
import org.elasticsearch.script.field.DocValuesField;

import java.io.IOException;
import java.util.Collection;
Expand Down Expand Up @@ -44,8 +45,8 @@ public SortedBinaryDocValues getBytesValues() {
}

@Override
public Strings getScriptValues() {
return new ScriptDocValues.Strings(getBytesValues());
public DocValuesField getScriptField(String name) {
return new DelegateDocValuesField(new ScriptDocValues.Strings(getBytesValues()), name);
}

@Override
Expand Down
7 changes: 4 additions & 3 deletions server/src/main/java/org/elasticsearch/index/fielddata/plain/BytesBinaryDVLeafFieldData.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,9 @@
package org.elasticsearch.index.fielddata.plain;

import org.apache.lucene.index.BinaryDocValues;
import org.apache.lucene.util.BytesRef;
import org.elasticsearch.index.fielddata.ScriptDocValues;
import org.elasticsearch.script.field.DelegateDocValuesField;
import org.elasticsearch.script.field.DocValuesField;


final class BytesBinaryDVLeafFieldData extends AbstractBinaryDVLeafFieldData {
Expand All @@ -19,8 +20,8 @@ final class BytesBinaryDVLeafFieldData extends AbstractBinaryDVLeafFieldData {
}

@Override
public ScriptDocValues<BytesRef> getScriptValues() {
return new ScriptDocValues.BytesRefs(getBytesValues());
public DocValuesField getScriptField(String name) {
return new DelegateDocValuesField(new ScriptDocValues.BytesRefs(getBytesValues()), name);
}
}

6 changes: 4 additions & 2 deletions server/src/main/java/org/elasticsearch/index/fielddata/plain/LeafDoubleFieldData.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@
import org.elasticsearch.index.fielddata.ScriptDocValues;
import org.elasticsearch.index.fielddata.SortedBinaryDocValues;
import org.elasticsearch.index.fielddata.SortedNumericDoubleValues;
import org.elasticsearch.script.field.DelegateDocValuesField;
import org.elasticsearch.script.field.DocValuesField;
import org.elasticsearch.search.DocValueFormat;

import java.io.IOException;
Expand All @@ -40,8 +42,8 @@ public long ramBytesUsed() {
}

@Override
public final ScriptDocValues<Double> getScriptValues() {
return new ScriptDocValues.Doubles(getDoubleValues());
public final DocValuesField getScriptField(String name) {
return new DelegateDocValuesField(new ScriptDocValues.Doubles(getDoubleValues()), name);
}

@Override
Expand Down
15 changes: 9 additions & 6 deletions server/src/main/java/org/elasticsearch/index/fielddata/plain/LeafLongFieldData.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@
import org.elasticsearch.index.fielddata.ScriptDocValues;
import org.elasticsearch.index.fielddata.SortedBinaryDocValues;
import org.elasticsearch.index.fielddata.SortedNumericDoubleValues;
import org.elasticsearch.script.field.DelegateDocValuesField;
import org.elasticsearch.script.field.DocValuesField;
import org.elasticsearch.search.DocValueFormat;

import java.io.IOException;
Expand All @@ -27,7 +29,7 @@ public abstract class LeafLongFieldData implements LeafNumericFieldData {

private final long ramBytesUsed;
/**
* Type of this field. Used to expose appropriate types in {@link #getScriptValues()}.
* Type of this field. Used to expose appropriate types in {@link #getScriptField(String)}}.
*/
private final NumericType numericType;

Expand All @@ -42,18 +44,19 @@ public long ramBytesUsed() {
}

@Override
public final ScriptDocValues<?> getScriptValues() {
public final DocValuesField getScriptField(String name) {
switch (numericType) {
// for now, dates and nanoseconds are treated the same, which also means, that the precision is only on millisecond level
case DATE:
return new ScriptDocValues.Dates(getLongValues(), false);
return new DelegateDocValuesField(new ScriptDocValues.Dates(getLongValues(), false), name);
case DATE_NANOSECONDS:
assert this instanceof SortedNumericIndexFieldData.NanoSecondFieldData;
return new ScriptDocValues.Dates(((SortedNumericIndexFieldData.NanoSecondFieldData) this).getLongValuesAsNanos(), true);
return new DelegateDocValuesField(
new ScriptDocValues.Dates(((SortedNumericIndexFieldData.NanoSecondFieldData) this).getLongValuesAsNanos(), true), name);
case BOOLEAN:
return new ScriptDocValues.Booleans(getLongValues());
return new DelegateDocValuesField(new ScriptDocValues.Booleans(getLongValues()), name);
default:
return new ScriptDocValues.Longs(getLongValues());
return new DelegateDocValuesField(new ScriptDocValues.Longs(getLongValues()), name);
}
}

Expand Down
6 changes: 4 additions & 2 deletions ...er/src/main/java/org/elasticsearch/index/fielddata/plain/StringBinaryDVLeafFieldData.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,16 @@

import org.apache.lucene.index.BinaryDocValues;
import org.elasticsearch.index.fielddata.ScriptDocValues;
import org.elasticsearch.script.field.DelegateDocValuesField;
import org.elasticsearch.script.field.DocValuesField;

final class StringBinaryDVLeafFieldData extends AbstractBinaryDVLeafFieldData {
StringBinaryDVLeafFieldData(BinaryDocValues values) {
super(values);
}

@Override
public ScriptDocValues<String> getScriptValues() {
return new ScriptDocValues.Strings(getBytesValues());
public DocValuesField getScriptField(String name) {
return new DelegateDocValuesField(new ScriptDocValues.Strings(getBytesValues()), name);
}
}
6 changes: 4 additions & 2 deletions server/src/main/java/org/elasticsearch/index/mapper/IdFieldMapper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@
import org.elasticsearch.index.query.SearchExecutionContext;
import org.elasticsearch.indices.IndicesService;
import org.elasticsearch.indices.breaker.CircuitBreakerService;
import org.elasticsearch.script.field.DelegateDocValuesField;
import org.elasticsearch.script.field.DocValuesField;
import org.elasticsearch.search.DocValueFormat;
import org.elasticsearch.search.MultiValueMode;
import org.elasticsearch.search.aggregations.support.CoreValuesSourceType;
Expand Down Expand Up @@ -211,8 +213,8 @@ public long ramBytesUsed() {
}

@Override
public ScriptDocValues<?> getScriptValues() {
return new ScriptDocValues.Strings(getBytesValues());
public DocValuesField getScriptField(String name) {
return new DelegateDocValuesField(new ScriptDocValues.Strings(getBytesValues()), name);
}

@Override
Expand Down
11 changes: 6 additions & 5 deletions ...r/src/main/java/org/elasticsearch/index/mapper/flattened/KeyedFlattenedLeafFieldData.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,12 @@
import org.apache.lucene.util.Accountable;
import org.apache.lucene.util.BytesRef;
import org.elasticsearch.index.fielddata.AbstractSortedSetDocValues;
import org.elasticsearch.index.fielddata.LeafOrdinalsFieldData;
import org.elasticsearch.index.fielddata.FieldData;
import org.elasticsearch.index.fielddata.ScriptDocValues;
import org.elasticsearch.index.fielddata.LeafOrdinalsFieldData;
import org.elasticsearch.index.fielddata.SortedBinaryDocValues;
import org.elasticsearch.index.fielddata.plain.AbstractLeafOrdinalsFieldData;
import org.elasticsearch.script.field.DelegateDocValuesField;
import org.elasticsearch.script.field.DocValuesField;

import java.io.IOException;
import java.io.UncheckedIOException;
Expand Down Expand Up @@ -79,9 +80,9 @@ public void close() {
}

@Override
public ScriptDocValues<?> getScriptValues() {
return AbstractLeafOrdinalsFieldData.DEFAULT_SCRIPT_FUNCTION
.apply(getOrdinalsValues());
public DocValuesField getScriptField(String name) {
return new DelegateDocValuesField(
AbstractLeafOrdinalsFieldData.DEFAULT_SCRIPT_FUNCTION.apply(getOrdinalsValues()), name);
}

@Override
Expand Down
Loading