-
Notifications
You must be signed in to change notification settings - Fork 25.6k
Add block loader from stored field and source for ip field #126644
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.
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
Changes from 2 commits
5ad383f
6770b79
73525cd
0f11bf7
2b77823
074ae5d
0d97213
bf1ae8e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -43,6 +43,7 @@ | |
| import org.elasticsearch.search.aggregations.support.CoreValuesSourceType; | ||
| import org.elasticsearch.search.lookup.FieldValues; | ||
| import org.elasticsearch.search.lookup.SearchLookup; | ||
| import org.elasticsearch.xcontent.XContentParser; | ||
|
|
||
| import java.io.IOException; | ||
| import java.net.InetAddress; | ||
|
|
@@ -51,8 +52,10 @@ | |
| import java.util.Arrays; | ||
| import java.util.Collection; | ||
| import java.util.Collections; | ||
| import java.util.List; | ||
| import java.util.Map; | ||
| import java.util.Objects; | ||
| import java.util.Set; | ||
| import java.util.function.BiFunction; | ||
|
|
||
| import static org.elasticsearch.index.mapper.FieldArrayContext.getOffsetsFieldName; | ||
|
|
@@ -213,7 +216,8 @@ public IpFieldMapper build(MapperBuilderContext context) { | |
| parseNullValue(), | ||
| scriptValues(), | ||
| meta.getValue(), | ||
| dimension.getValue() | ||
| dimension.getValue(), | ||
| context.isSourceSynthetic() | ||
| ), | ||
| builderParams(this, context), | ||
| context.isSourceSynthetic(), | ||
|
|
@@ -234,6 +238,7 @@ public static final class IpFieldType extends SimpleMappedFieldType { | |
| private final InetAddress nullValue; | ||
| private final FieldValues<InetAddress> scriptValues; | ||
| private final boolean isDimension; | ||
| private final boolean isSyntheticSource; | ||
|
|
||
| public IpFieldType( | ||
| String name, | ||
|
|
@@ -243,12 +248,14 @@ public IpFieldType( | |
| InetAddress nullValue, | ||
| FieldValues<InetAddress> scriptValues, | ||
| Map<String, String> meta, | ||
| boolean isDimension | ||
| boolean isDimension, | ||
| boolean isSyntheticSource | ||
| ) { | ||
| super(name, indexed, stored, hasDocValues, TextSearchInfo.SIMPLE_MATCH_WITHOUT_TERMS, meta); | ||
| this.nullValue = nullValue; | ||
| this.scriptValues = scriptValues; | ||
| this.isDimension = isDimension; | ||
| this.isSyntheticSource = isSyntheticSource; | ||
| } | ||
|
|
||
| public IpFieldType(String name) { | ||
|
|
@@ -260,7 +267,7 @@ public IpFieldType(String name, boolean isIndexed) { | |
| } | ||
|
|
||
| public IpFieldType(String name, boolean isIndexed, boolean hasDocValues) { | ||
| this(name, isIndexed, false, hasDocValues, null, null, Collections.emptyMap(), false); | ||
| this(name, isIndexed, false, hasDocValues, null, null, Collections.emptyMap(), false, false); | ||
| } | ||
|
|
||
| @Override | ||
|
|
@@ -455,7 +462,75 @@ public BlockLoader blockLoader(BlockLoaderContext blContext) { | |
| if (hasDocValues()) { | ||
| return new BlockDocValuesReader.BytesRefsFromOrdsBlockLoader(name()); | ||
| } | ||
| return null; | ||
|
|
||
| if (isStored()) { | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I added this because it was very easy to do. I am not sure if there are any cases when this is not beneficial? Some existing implementations don't bother and just go straight to loading from source.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍 - seems easy to just do in this pr. |
||
| return new BlockStoredFieldsReader.BytesFromBytesRefsBlockLoader(name()); | ||
| } | ||
|
|
||
| if (isSyntheticSource) { | ||
| return blockLoaderFromFallbackSyntheticSource(blContext); | ||
| } | ||
|
|
||
| BlockSourceReader.LeafIteratorLookup lookup = isIndexed() | ||
| ? BlockSourceReader.lookupFromFieldNames(blContext.fieldNames(), name()) | ||
| : BlockSourceReader.lookupMatchingAll(); | ||
| return new BlockSourceReader.IpsBlockLoader(sourceValueFetcher(blContext.sourcePaths(name())), lookup); | ||
| } | ||
|
|
||
| private BlockLoader blockLoaderFromFallbackSyntheticSource(BlockLoaderContext blContext) { | ||
| var reader = new FallbackSyntheticSourceBlockLoader.SingleValueReader<InetAddress>(nullValue) { | ||
| @Override | ||
| public void convertValue(Object value, List<InetAddress> accumulator) { | ||
| if (value instanceof InetAddress ia) { | ||
| accumulator.add(ia); | ||
| } | ||
|
|
||
| try { | ||
| var address = InetAddresses.forString(value.toString()); | ||
| accumulator.add(address); | ||
| } catch (Exception e) { | ||
| // Malformed value, skip it | ||
| } | ||
| } | ||
|
|
||
| @Override | ||
| protected void parseNonNullValue(XContentParser parser, List<InetAddress> accumulator) throws IOException { | ||
| // aligned with #parseCreateField() | ||
| String value = parser.text(); | ||
|
|
||
| try { | ||
| var address = InetAddresses.forString(value); | ||
| accumulator.add(address); | ||
| } catch (Exception e) { | ||
| // Malformed value, skip it | ||
| } | ||
| } | ||
|
|
||
| @Override | ||
| public void writeToBlock(List<InetAddress> values, BlockLoader.Builder blockBuilder) { | ||
| var bytesRefBuilder = (BlockLoader.BytesRefBuilder) blockBuilder; | ||
|
|
||
| for (var value : values) { | ||
| bytesRefBuilder.appendBytesRef(new BytesRef(InetAddressPoint.encode(value))); | ||
| } | ||
| } | ||
| }; | ||
|
|
||
| return new FallbackSyntheticSourceBlockLoader(reader, name()) { | ||
| @Override | ||
| public Builder builder(BlockFactory factory, int expectedCount) { | ||
| return factory.bytesRefs(expectedCount); | ||
| } | ||
| }; | ||
| } | ||
|
|
||
| private SourceValueFetcher sourceValueFetcher(Set<String> sourcePaths) { | ||
| return new SourceValueFetcher(sourcePaths, nullValue) { | ||
| @Override | ||
| public InetAddress parseSourceValue(Object value) { | ||
| return parse(value); | ||
| } | ||
| }; | ||
| } | ||
|
|
||
| @Override | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,72 @@ | ||
| /* | ||
| * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one | ||
| * or more contributor license agreements. Licensed under the "Elastic License | ||
| * 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side | ||
| * Public License v 1"; you may not use this file except in compliance with, at | ||
| * your election, the "Elastic License 2.0", the "GNU Affero General Public | ||
| * License v3.0 only", or the "Server Side Public License, v 1". | ||
| */ | ||
|
|
||
| package org.elasticsearch.index.mapper.blockloader; | ||
|
|
||
| import org.apache.lucene.document.InetAddressPoint; | ||
| import org.apache.lucene.util.BytesRef; | ||
| import org.elasticsearch.common.network.InetAddresses; | ||
| import org.elasticsearch.index.mapper.BlockLoaderTestCase; | ||
| import org.elasticsearch.logsdb.datageneration.FieldType; | ||
|
|
||
| import java.util.List; | ||
| import java.util.Map; | ||
| import java.util.Objects; | ||
|
|
||
| public class IpFieldBlockLoaderTests extends BlockLoaderTestCase { | ||
| public IpFieldBlockLoaderTests(Params params) { | ||
| super(FieldType.IP.toString(), params); | ||
| } | ||
|
|
||
| @Override | ||
| @SuppressWarnings("unchecked") | ||
| protected Object expected(Map<String, Object> fieldMapping, Object value, TestContext testContext) { | ||
| var rawNullValue = (String) fieldMapping.get("null_value"); | ||
| BytesRef nullValue = convert(rawNullValue, null); | ||
|
|
||
| if (value == null) { | ||
| return convert(null, nullValue); | ||
| } | ||
| if (value instanceof String s) { | ||
| return convert(s, nullValue); | ||
| } | ||
|
|
||
| if (hasDocValues(fieldMapping, true)) { | ||
| var resultList = ((List<String>) value).stream() | ||
| .map(v -> convert(v, nullValue)) | ||
| .filter(Objects::nonNull) | ||
| .distinct() | ||
| .sorted() | ||
| .toList(); | ||
| return maybeFoldList(resultList); | ||
| } | ||
|
|
||
| // field is stored or using source | ||
| var resultList = ((List<String>) value).stream().map(v -> convert(v, nullValue)).filter(Objects::nonNull).toList(); | ||
| return maybeFoldList(resultList); | ||
| } | ||
|
|
||
| private static BytesRef convert(Object value, BytesRef nullValue) { | ||
| if (value == null) { | ||
| return nullValue; | ||
| } | ||
|
|
||
| if (value instanceof String s) { | ||
| try { | ||
| var address = InetAddresses.forString(s); | ||
| return new BytesRef(InetAddressPoint.encode(address)); | ||
| } catch (Exception ex) { | ||
| // malformed | ||
| return null; | ||
| } | ||
| } | ||
|
|
||
| return null; | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should i check for
FieldExtractPreference.DOC_VALUEShere?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, I don't think we have to. This only seems to be used in geo based field types.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are right, sorry, i actually meant
FieldExtractPreference.STORED.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it would be nice to check, yes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For context - that's the user asking you to load from _source. If you ignore it you are ignoring their kind request. You are sure allowed, but you probably shouldn't ignore it.