Skip to content

Commit

Permalink
add new constructor to reduce changes in unrelated areas of the code …
Browse files Browse the repository at this point in the history
…base
  • Loading branch information
martijnvg committed Dec 3, 2024
1 parent ec88323 commit 2aecb15
Show file tree
Hide file tree
Showing 11 changed files with 17 additions and 13 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ public final ColumnAtATimeReader columnAtATimeReader(LeafReaderContext context)

@Override
public final StoredFieldsSpec rowStrideStoredFieldSpec() {
return new StoredFieldsSpec(false, false, Set.of(field), false);
return new StoredFieldsSpec(false, false, Set.of(field));
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ public RowStrideReader rowStrideReader(LeafReaderContext context) throws IOExcep

@Override
public StoredFieldsSpec rowStrideStoredFieldSpec() {
return new StoredFieldsSpec(true, false, Set.of(), false);
return new StoredFieldsSpec(true, false, Set.of());
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ public class StoredValueFetcher implements ValueFetcher {
public StoredValueFetcher(SearchLookup lookup, String fieldname) {
this.lookup = lookup;
this.fieldname = fieldname;
this.storedFieldsSpec = new StoredFieldsSpec(false, false, Set.of(fieldname), false);
this.storedFieldsSpec = new StoredFieldsSpec(false, false, Set.of(fieldname));
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ private SearchHits buildSearchHits(SearchContext context, int[] docIdsToLoad, Pr

List<FetchSubPhaseProcessor> processors = getProcessors(context.shardTarget(), fetchContext, profiler);
StoredFieldsSpec storedFieldsSpec = StoredFieldsSpec.build(processors, FetchSubPhaseProcessor::storedFieldsSpec);
storedFieldsSpec = storedFieldsSpec.merge(new StoredFieldsSpec(false, false, sourceLoader.requiredStoredFields(), false));
storedFieldsSpec = storedFieldsSpec.merge(new StoredFieldsSpec(false, false, sourceLoader.requiredStoredFields()));
// Ideally the required stored fields would be provided as constructor argument a few lines above, but that requires moving
// the getProcessors call to before the setLookupProviders call, which causes weird issues in InnerHitsPhase.
// setLookupProviders resets the SearchLookup used throughout the rest of the fetch phase, which StoredValueFetchers rely on
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,14 +22,18 @@
*/
public record StoredFieldsSpec(boolean requiresSource, boolean requiresMetadata, Set<String> requiredStoredFields, boolean hasScript) {

public StoredFieldsSpec(boolean requiresSource, boolean requiresMetadata, Set<String> requiredStoredFields) {
this(requiresSource, requiresMetadata, requiredStoredFields, false);
}

public boolean noRequirements() {
return requiresSource == false && requiresMetadata == false && requiredStoredFields.isEmpty();
}

/**
* Use when no stored fields are required
*/
public static final StoredFieldsSpec NO_REQUIREMENTS = new StoredFieldsSpec(false, false, Set.of(), false);
public static final StoredFieldsSpec NO_REQUIREMENTS = new StoredFieldsSpec(false, false, Set.of());

/**
* Use when scripts / runtime fields are used and no stored fields are required
Expand All @@ -39,7 +43,7 @@ public boolean noRequirements() {
/**
* Use when the source should be loaded but no other stored fields are required
*/
public static final StoredFieldsSpec NEEDS_SOURCE = new StoredFieldsSpec(true, false, Set.of(), false);
public static final StoredFieldsSpec NEEDS_SOURCE = new StoredFieldsSpec(true, false, Set.of());

/**
* Combine these stored field requirements with those from another StoredFieldsSpec
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ public FetchSubPhaseProcessor getProcessor(FetchContext searchContext) {
return null;
}
Map<String, InnerHitsContext.InnerHitSubContext> innerHits = searchContext.innerHits().getInnerHits();
StoredFieldsSpec storedFieldsSpec = new StoredFieldsSpec(requiresSource(innerHits.values()), false, Set.of(), false);
StoredFieldsSpec storedFieldsSpec = new StoredFieldsSpec(requiresSource(innerHits.values()), false, Set.of());
return new FetchSubPhaseProcessor() {
@Override
public void setNextReader(LeafReaderContext readerContext) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ public StoredFieldsSpec storedFieldsSpec() {
// which has its own lazy loading config that kicks in if not overridden
// by other sub phases that require source. However, if script fields
// are present then we enforce metadata loading
return new StoredFieldsSpec(false, true, Set.of(), false);
return new StoredFieldsSpec(false, true, Set.of());
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ public FetchSubPhaseProcessor getProcessor(FetchContext fetchContext) {
}
}
}
StoredFieldsSpec storedFieldsSpec = new StoredFieldsSpec(false, true, fieldsToLoad, false);
StoredFieldsSpec storedFieldsSpec = new StoredFieldsSpec(false, true, fieldsToLoad);

return new FetchSubPhaseProcessor() {
@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ private FieldContext contextBuilders(
)
);
}
storedFieldsSpec = storedFieldsSpec.merge(new StoredFieldsSpec(sourceRequired, false, storedFields, false));
storedFieldsSpec = storedFieldsSpec.merge(new StoredFieldsSpec(sourceRequired, false, storedFields));
}
return new FieldContext(storedFieldsSpec, builders);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1423,7 +1423,7 @@ protected final void testBlockLoader(
StoredFieldsSpec storedFieldsSpec = loader.rowStrideStoredFieldSpec();
if (storedFieldsSpec.requiresSource()) {
storedFieldsSpec = storedFieldsSpec.merge(
new StoredFieldsSpec(true, storedFieldsSpec.requiresMetadata(), sourceLoader.requiredStoredFields(), false)
new StoredFieldsSpec(true, storedFieldsSpec.requiresMetadata(), sourceLoader.requiredStoredFields())
);
}
BlockLoaderStoredFieldsFromLeafLoader storedFieldsLoader = new BlockLoaderStoredFieldsFromLeafLoader(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -244,7 +244,7 @@ private void loadFromSingleLeaf(Block[] blocks, int shard, int segment, BlockLoa
SourceLoader sourceLoader = null;
if (storedFieldsSpec.requiresSource()) {
sourceLoader = shardContexts.get(shard).newSourceLoader.get();
storedFieldsSpec = storedFieldsSpec.merge(new StoredFieldsSpec(true, false, sourceLoader.requiredStoredFields(), false));
storedFieldsSpec = storedFieldsSpec.merge(new StoredFieldsSpec(true, false, sourceLoader.requiredStoredFields()));
}

if (rowStrideReaders.isEmpty()) {
Expand Down Expand Up @@ -391,7 +391,7 @@ private void fieldsMoved(LeafReaderContext ctx, int shard) throws IOException {
SourceLoader sourceLoader = null;
if (storedFieldsSpec.requiresSource()) {
sourceLoader = shardContexts.get(shard).newSourceLoader.get();
storedFieldsSpec = storedFieldsSpec.merge(new StoredFieldsSpec(true, false, sourceLoader.requiredStoredFields(), false));
storedFieldsSpec = storedFieldsSpec.merge(new StoredFieldsSpec(true, false, sourceLoader.requiredStoredFields()));
}
storedFields = new BlockLoaderStoredFieldsFromLeafLoader(
StoredFieldLoader.fromSpec(storedFieldsSpec).getLoader(ctx, null),
Expand Down

0 comments on commit 2aecb15

Please sign in to comment.