Skip to content

Commit 6307f93

Browse files
markjhoymarkjhoy
committed
serialization refactor; pass index version around
1 parent 3f4801b commit 6307f93

File tree

4 files changed

+107
-103
lines changed

4 files changed

+107
-103
lines changed

‎server/src/main/java/org/elasticsearch/index/mapper/vectors/SparseVectorFieldMapper.java‎

Lines changed: 76 additions & 96 deletions
Original file line numberDiff line numberDiff line change
@@ -88,45 +88,29 @@ public class SparseVectorFieldMapper extends FieldMapper {
8888

8989
public static final NodeFeature SPARSE_VECTOR_INDEX_OPTIONS_FEATURE = new NodeFeature("sparse_vector.index_options_supported");
9090

91+
private final IndexVersion indexVersion;
92+
9193
private static SparseVectorFieldMapper toType(FieldMapper in) {
9294
return (SparseVectorFieldMapper) in;
9395
}
9496

9597
public static class Builder extends FieldMapper.Builder {
98+
private final IndexVersion indexVersion;
99+
96100
private final Parameter<Boolean> stored = Parameter.storeParam(m -> toType(m).fieldType().isStored(), false);
97101
private final Parameter<Map<String, String>> meta = Parameter.metaParam();
98-
private final Parameter<IndexOptions> indexOptions = new Parameter<>(
99-
SPARSE_VECTOR_INDEX_OPTIONS,
102+
private final Parameter<IndexOptions> indexOptions = new Parameter<>(SPARSE_VECTOR_INDEX_OPTIONS,
100103
true,
101104
() -> null,
102105
(n, c, o) -> parseIndexOptions(c, o),
103106
m -> toType(m).fieldType().indexOptions,
104-
this::serializeIndexOptions,
105-
// XContentBuilder::field,
107+
XContentBuilder::field,
106108
Objects::toString
107-
).acceptsNull();
108-
109-
private void serializeIndexOptions(XContentBuilder builder, String name, IndexOptions value) throws IOException {
110-
if (value instanceof IndexOptions serializeIndexOptions) {
111-
if (IndexOptions.isDefaultOptions(serializeIndexOptions)) {
112-
// do not emit anything if it's the default options
113-
return;
114-
}
115-
}
116-
117-
builder.field(name, value);
118-
}
119-
120-
private final MappingParserContext mappingParserContext;
109+
).acceptsNull().setSerializerCheck(this::indexOptionsSerializerCheck);
121110

122-
public Builder(String name) {
111+
public Builder(String name, @Nullable IndexVersion indexVersion) {
123112
super(name);
124-
mappingParserContext = null;
125-
}
126-
127-
public Builder(String name, @Nullable MappingParserContext mappingParserContext) {
128-
super(name);
129-
this.mappingParserContext = mappingParserContext;
113+
this.indexVersion = indexVersion;
130114
}
131115

132116
public Builder setStored(boolean value) {
@@ -142,18 +126,27 @@ protected Parameter<?>[] getParameters() {
142126
@Override
143127
public SparseVectorFieldMapper build(MapperBuilderContext context) {
144128
IndexOptions builderIndexOptions = indexOptions.getValue();
145-
if (builderIndexOptions == null
146-
&& mappingParserContext != null
147-
&& shouldHaveDefaultPruningConfig(mappingParserContext.indexVersionCreated())) {
129+
if (builderIndexOptions == null && indexVersion != null && indexVersionSupportsDefaultPruningConfig(indexVersion)) {
148130
builderIndexOptions = new IndexOptions(true, new TokenPruningConfig());
149131
}
150132

151133
return new SparseVectorFieldMapper(
152134
leafName(),
153-
new SparseVectorFieldType(context.buildFullName(leafName()), stored.getValue(), meta.getValue(), builderIndexOptions),
154-
builderParams(this, context)
135+
new SparseVectorFieldType(
136+
indexVersion,
137+
context.buildFullName(leafName()),
138+
stored.getValue(),
139+
meta.getValue(),
140+
builderIndexOptions
141+
),
142+
builderParams(this, context),
143+
indexVersion
155144
);
156145
}
146+
147+
private boolean indexOptionsSerializerCheck(boolean includeDefaults, boolean isConfigured, IndexOptions value) {
148+
return includeDefaults || (value == null || IndexOptions.isDefaultOptions(value, indexVersion)) == false;
149+
}
157150
}
158151

159152
public IndexOptions getIndexOptions() {
@@ -177,12 +170,8 @@ private static SparseVectorFieldMapper.IndexOptions parseIndexOptions(MappingPar
177170

178171
Map<String, Object> indexOptionsMap = XContentMapValues.nodeMapValue(propNode, SPARSE_VECTOR_INDEX_OPTIONS);
179172

180-
XContentParser parser = new MapXContentParser(
181-
NamedXContentRegistry.EMPTY,
182-
DeprecationHandler.IGNORE_DEPRECATIONS,
183-
indexOptionsMap,
184-
XContentType.JSON
185-
);
173+
XContentParser parser =
174+
new MapXContentParser(NamedXContentRegistry.EMPTY, DeprecationHandler.IGNORE_DEPRECATIONS, indexOptionsMap, XContentType.JSON);
186175

187176
try {
188177
return INDEX_OPTIONS_PARSER.parse(parser, null);
@@ -198,24 +187,29 @@ private static SparseVectorFieldMapper.IndexOptions parseIndexOptions(MappingPar
198187
throw new IllegalArgumentException(ERROR_MESSAGE_8X);
199188
}
200189

201-
return new Builder(n, c);
190+
return new Builder(n, c.indexVersionCreated());
202191
}, notInMultiFields(CONTENT_TYPE));
203192

204193
public static final class SparseVectorFieldType extends MappedFieldType {
205194
private final IndexOptions indexOptions;
206195

207-
public SparseVectorFieldType(String name, boolean isStored, Map<String, String> meta) {
208-
this(name, isStored, meta, null);
196+
public SparseVectorFieldType(IndexVersion indexVersion, String name, boolean isStored, Map<String, String> meta) {
197+
this(indexVersion, name, isStored, meta, null);
209198
}
210199

211200
public SparseVectorFieldType(
201+
IndexVersion indexVersion,
212202
String name,
213203
boolean isStored,
214204
Map<String, String> meta,
215205
@Nullable SparseVectorFieldMapper.IndexOptions indexOptions
216206
) {
217207
super(name, true, isStored, false, TextSearchInfo.SIMPLE_MATCH_ONLY, meta);
218-
this.indexOptions = indexOptions;
208+
this.indexOptions = indexOptions != null
209+
? indexOptions
210+
: SparseVectorFieldMapper.indexVersionSupportsDefaultPruningConfig(indexVersion)
211+
? new IndexOptions(true, new TokenPruningConfig())
212+
: null;
219213
}
220214

221215
public IndexOptions getIndexOptions() {
@@ -269,35 +263,27 @@ public Query finalizeSparseVectorQuery(
269263
Boolean shouldPruneTokensFromQuery,
270264
TokenPruningConfig tokenPruningConfigFromQuery
271265
) throws IOException {
272-
TokenPruningConfig pruningConfig = null;
273-
274-
if (shouldPruneTokensFromQuery != null) {
275-
// if this is not null, the query is overriding the index config
276-
pruningConfig = shouldPruneTokensFromQuery ? tokenPruningConfigFromQuery : null;
277-
} else {
278-
// check and see if we explicitly do not prune in the index_options
279-
boolean explicitlyDoNotPrune = this.indexOptions != null
280-
&& this.indexOptions.prune != null
281-
&& this.indexOptions.prune == false;
282-
283-
if (explicitlyDoNotPrune == false) {
284-
// get the explicit pruning config from the index_options if available
285-
pruningConfig = this.indexOptions != null ? this.indexOptions.pruningConfig : null;
286-
287-
// if we're still null, set the default based on the index version
288-
// newer index versions default to true, while older is false
289-
pruningConfig = pruningConfig == null ? getDefaultPruningConfig(context) : pruningConfig;
266+
Boolean shouldPruneTokens = shouldPruneTokensFromQuery;
267+
TokenPruningConfig tokenPruningConfig = tokenPruningConfigFromQuery;
268+
269+
if (indexOptions != null) {
270+
if (shouldPruneTokens == null && indexOptions.prune != null) {
271+
shouldPruneTokens = indexOptions.prune;
290272
}
291-
}
292273

293-
return (pruningConfig != null)
294-
? WeightedTokensUtils.queryBuilderWithPrunedTokens(fieldName, pruningConfig, queryVectors, this, context)
295-
: WeightedTokensUtils.queryBuilderWithAllTokens(fieldName, queryVectors, this, context);
296-
}
274+
if (tokenPruningConfig == null && indexOptions.pruningConfig != null) {
275+
tokenPruningConfig = indexOptions.pruningConfig;
276+
}
277+
}
297278

298-
private TokenPruningConfig getDefaultPruningConfig(SearchExecutionContext context) {
299-
IndexVersion indexVersion = context.indexVersionCreated();
300-
return (shouldHaveDefaultPruningConfig(indexVersion)) ? new TokenPruningConfig() : null;
279+
return (shouldPruneTokens != null && shouldPruneTokens)
280+
? WeightedTokensUtils.queryBuilderWithPrunedTokens(
281+
fieldName,
282+
tokenPruningConfig == null ? new TokenPruningConfig() : tokenPruningConfig,
283+
queryVectors,
284+
this,
285+
context
286+
) : WeightedTokensUtils.queryBuilderWithAllTokens(fieldName, queryVectors, this, context);
301287
}
302288

303289
private static String indexedValueForSearch(Object value) {
@@ -308,8 +294,14 @@ private static String indexedValueForSearch(Object value) {
308294
}
309295
}
310296

311-
private SparseVectorFieldMapper(String simpleName, MappedFieldType mappedFieldType, BuilderParams builderParams) {
297+
private SparseVectorFieldMapper(
298+
String simpleName,
299+
MappedFieldType mappedFieldType,
300+
BuilderParams builderParams,
301+
IndexVersion indexVersion
302+
) {
312303
super(simpleName, mappedFieldType, builderParams);
304+
this.indexVersion = indexVersion;
313305
}
314306

315307
@Override
@@ -327,7 +319,7 @@ public Map<String, NamedAnalyzer> indexAnalyzers() {
327319

328320
@Override
329321
public FieldMapper.Builder getMergeBuilder() {
330-
return new Builder(leafName(), null).init(this);
322+
return new Builder(leafName(), indexVersion).init(this);
331323
}
332324

333325
@Override
@@ -352,8 +344,7 @@ public void parse(DocumentParserContext context) throws IOException {
352344

353345
if (context.parser().currentToken() != Token.START_OBJECT) {
354346
throw new IllegalArgumentException(
355-
"[sparse_vector] fields must be json objects, expected a START_OBJECT but got: " + context.parser().currentToken()
356-
);
347+
"[sparse_vector] fields must be json objects, expected a START_OBJECT but got: " + context.parser().currentToken());
357348
}
358349

359350
final boolean isWithinLeaf = context.path().isWithinLeafObject();
@@ -381,10 +372,8 @@ public void parse(DocumentParserContext context) throws IOException {
381372
}
382373
} else {
383374
throw new IllegalArgumentException(
384-
"[sparse_vector] fields take hashes that map a feature to a strictly positive "
385-
+ "float, but got unexpected token "
386-
+ token
387-
);
375+
"[sparse_vector] fields take hashes that map a feature to a strictly positive " + "float, but got unexpected token "
376+
+ token);
388377
}
389378
}
390379
if (context.indexSettings().getIndexVersionCreated().onOrAfter(SPARSE_VECTOR_IN_FIELD_NAMES_INDEX_VERSION)) {
@@ -405,18 +394,12 @@ protected String contentType() {
405394
return CONTENT_TYPE;
406395
}
407396

408-
private static boolean shouldHaveDefaultPruningConfig(IndexVersion indexVersion) {
409-
if (indexVersion.onOrAfter(SPARSE_VECTOR_PRUNING_INDEX_OPTIONS_VERSION)) {
410-
// default pruning for 9.1.0+ is true for this index
411-
return true;
412-
}
413-
414-
// default pruning for 8.19.0+ is true for this index
415-
if (indexVersion.between(SPARSE_VECTOR_PRUNING_INDEX_OPTIONS_VERSION_8_X, IndexVersions.UPGRADE_TO_LUCENE_10_0_0)) {
416-
return true;
417-
}
418-
419-
return false;
397+
private static boolean indexVersionSupportsDefaultPruningConfig(IndexVersion indexVersion) {
398+
// default pruning for 9.1.0+ or 8.19.0+ is true for this index
399+
return (
400+
indexVersion.onOrAfter(SPARSE_VECTOR_PRUNING_INDEX_OPTIONS_VERSION) ||
401+
indexVersion.between(SPARSE_VECTOR_PRUNING_INDEX_OPTIONS_VERSION_8_X, IndexVersions.UPGRADE_TO_LUCENE_10_0_0)
402+
);
420403
}
421404

422405
private static class SparseVectorValueFetcher implements ValueFetcher {
@@ -539,21 +522,19 @@ public static class IndexOptions implements ToXContent {
539522
IndexOptions(@Nullable Boolean prune, @Nullable TokenPruningConfig pruningConfig) {
540523
if (pruningConfig != null && (prune == null || prune == false)) {
541524
throw new IllegalArgumentException(
542-
"["
543-
+ SPARSE_VECTOR_INDEX_OPTIONS
544-
+ "] field ["
545-
+ PRUNING_CONFIG_FIELD_NAME.getPreferredName()
546-
+ "] should only be set if ["
547-
+ PRUNE_FIELD_NAME.getPreferredName()
548-
+ "] is set to true"
549-
);
525+
"[" + SPARSE_VECTOR_INDEX_OPTIONS + "] field [" + PRUNING_CONFIG_FIELD_NAME.getPreferredName()
526+
+ "] should only be set if [" + PRUNE_FIELD_NAME.getPreferredName() + "] is set to true");
550527
}
551528

552529
this.prune = prune;
553530
this.pruningConfig = pruningConfig;
554531
}
555532

556-
public static boolean isDefaultOptions(IndexOptions indexOptions) {
533+
public static boolean isDefaultOptions(IndexOptions indexOptions, IndexVersion indexVersion) {
534+
if (indexVersionSupportsDefaultPruningConfig(indexVersion) == false) {
535+
return indexOptions == null;
536+
}
537+
557538
if (indexOptions == null || indexOptions.prune == null || indexOptions.prune == false || indexOptions.pruningConfig == null) {
558539
return false;
559540
}
@@ -604,5 +585,4 @@ public final int hashCode() {
604585
return Objects.hash(prune, pruningConfig);
605586
}
606587
}
607-
608588
}

‎server/src/test/java/org/elasticsearch/index/mapper/vectors/SparseVectorFieldMapperTests.java‎

Lines changed: 23 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -629,7 +629,7 @@ public void testTypeQueryFinalizationDefaultsCurrentVersion() throws Exception {
629629
MapperService mapperService = createMapperService(version, fieldMapping(this::minimalMapping));
630630

631631
// query should be pruned by default on newer index versions
632-
performTypeQueryFinalizationTest(mapperService, null, null, null, true);
632+
performTypeQueryFinalizationTest(version, mapperService, null, null, null, true);
633633
}
634634

635635
public void testTypeQueryFinalizationDefaultsPreviousVersion() throws Exception {
@@ -641,7 +641,7 @@ public void testTypeQueryFinalizationDefaultsPreviousVersion() throws Exception
641641
MapperService mapperService = createMapperService(version, fieldMapping(this::minimalMapping));
642642

643643
// query should _not_ be pruned by default on older index versions
644-
performTypeQueryFinalizationTest(mapperService, null, null, null, false);
644+
performTypeQueryFinalizationTest(version, mapperService, null, null, null, false);
645645
}
646646

647647
public void testTypeQueryFinalizationWithIndexExplicit() throws Exception {
@@ -650,6 +650,7 @@ public void testTypeQueryFinalizationWithIndexExplicit() throws Exception {
650650

651651
// query should be pruned via explicit index options
652652
performTypeQueryFinalizationTest(
653+
version,
653654
mapperService,
654655
new SparseVectorFieldMapper.IndexOptions(true, new TokenPruningConfig()),
655656
null,
@@ -663,7 +664,14 @@ public void testTypeQueryFinalizationWithIndexExplicitDoNotPrune() throws Except
663664
MapperService mapperService = createMapperService(version, fieldMapping(this::mappingWithIndexOptionsPruneFalse));
664665

665666
// query should be pruned via explicit index options
666-
performTypeQueryFinalizationTest(mapperService, new SparseVectorFieldMapper.IndexOptions(false, null), null, null, false);
667+
performTypeQueryFinalizationTest(
668+
version,
669+
mapperService,
670+
new SparseVectorFieldMapper.IndexOptions(false, null),
671+
null,
672+
null,
673+
false
674+
);
667675
}
668676

669677
public void testTypeQueryFinalizationQueryOverridesPruning() throws Exception {
@@ -672,6 +680,7 @@ public void testTypeQueryFinalizationQueryOverridesPruning() throws Exception {
672680

673681
// query should still be pruned due to query builder setting it
674682
performTypeQueryFinalizationTest(
683+
version,
675684
mapperService,
676685
new SparseVectorFieldMapper.IndexOptions(false, null),
677686
true,
@@ -686,6 +695,7 @@ public void testTypeQueryFinalizationQueryOverridesPruningOff() throws Exception
686695

687696
// query should not pruned due to query builder setting it
688697
performTypeQueryFinalizationTest(
698+
version,
689699
mapperService,
690700
new SparseVectorFieldMapper.IndexOptions(true, new TokenPruningConfig()),
691701
false,
@@ -695,6 +705,7 @@ public void testTypeQueryFinalizationQueryOverridesPruningOff() throws Exception
695705
}
696706

697707
private void performTypeQueryFinalizationTest(
708+
IndexVersion indexVersion,
698709
MapperService mapperService,
699710
SparseVectorFieldMapper.IndexOptions indexOptions,
700711
@Nullable Boolean queryPrune,
@@ -703,6 +714,7 @@ private void performTypeQueryFinalizationTest(
703714
) throws IOException {
704715
withSearchExecutionContext(mapperService, (context) -> {
705716
SparseVectorFieldMapper.SparseVectorFieldType ft = new SparseVectorFieldMapper.SparseVectorFieldType(
717+
indexVersion,
706718
"field",
707719
false,
708720
Collections.emptyMap(),
@@ -776,7 +788,14 @@ private void runTestTypeQueryFinalization(
776788
}
777789

778790
try {
779-
performTypeQueryFinalizationTest(mapperService, indexOptions, shouldQueryPrune, queryPruningConfig, resultShouldBePruned);
791+
performTypeQueryFinalizationTest(
792+
version,
793+
mapperService,
794+
indexOptions,
795+
shouldQueryPrune,
796+
queryPruningConfig,
797+
resultShouldBePruned
798+
);
780799
} catch (AssertionError e) {
781800
String message = "performTypeQueryFinalizationTest failed using parameters: "
782801
+ "usePreviousIndex: "

0 commit comments

Comments
 (0)