Skip to content

Commit

Permalink
Added fromStats() for spatial block values from doc-values
Browse files Browse the repository at this point in the history
  • Loading branch information
craigtaverner committed Dec 18, 2023
1 parent 1166900 commit 4b24c08
Show file tree
Hide file tree
Showing 10 changed files with 56 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,11 @@ public String indexName() {
return "benchmark";
}

@Override
public boolean forStats() {
return false;
}

@Override
public SearchLookup lookup() {
throw new UnsupportedOperationException();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -143,9 +143,9 @@ protected Object parseSourceValue(Object value) {
@Override
public BlockLoader blockLoader(BlockLoaderContext blContext) {
// TODO: If we have doc-values we have to use them, due to BlockSourceReader.columnAtATimeReader() returning null
// if (hasDocValues()) {
// return new BlockDocValuesReader.LongsBlockLoader(name());
// }
if (blContext.forStats() && hasDocValues()) {
return new BlockDocValuesReader.LongsBlockLoader(name());
}
// TODO: Enhance BlockLoaderContext with knowledge about preferring to load from source (see EsPhysicalOperationProviders)
return new BlockSourceReader.PointsBlockLoader(
valueFetcher(blContext.sourcePaths(name()), nullValue, GeometryFormatterFactory.WKT)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -649,6 +649,14 @@ public interface BlockLoaderContext {
*/
String indexName();

/**
* Whether the data will be used in stats.
* This is relevant to some types, where the choice between doc-values or reading from source is dependent on the usage.
* For example, spatial types use doc-values for stats, but read from source for search because doc-values are modified
* from the original.
*/
boolean forStats();

/**
* {@link SearchLookup} used for building scripts.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -422,6 +422,11 @@ public String indexName() {
throw new UnsupportedOperationException();
}

@Override
public boolean forStats() {
return false;
}

@Override
public SearchLookup lookup() {
return mockContext().lookup();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1282,6 +1282,11 @@ public String indexName() {
throw new UnsupportedOperationException();
}

@Override
public boolean forStats() {
return false;
}

@Override
public SearchLookup lookup() {
return searchLookup;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,12 @@ private BlockReaderFactories() {}
* @param asUnsupportedSource should the field be loaded as "unsupported"?
* These will always have {@code null} values
*/
public static List<BlockLoader> loaders(List<SearchContext> searchContexts, String fieldName, boolean asUnsupportedSource) {
public static List<BlockLoader> loaders(
List<SearchContext> searchContexts,
String fieldName,
boolean asUnsupportedSource,
final boolean forStats
) {
List<BlockLoader> loaders = new ArrayList<>(searchContexts.size());

for (SearchContext searchContext : searchContexts) {
Expand All @@ -53,6 +58,11 @@ public String indexName() {
return ctx.getFullyQualifiedIndex().getName();
}

@Override
public boolean forStats() {
return forStats;
}

@Override
public SearchLookup lookup() {
return ctx.lookup();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -363,6 +363,11 @@ public String indexName() {
return "test_index";
}

@Override
public boolean forStats() {
return false;
}

@Override
public SearchLookup lookup() {
throw new UnsupportedOperationException();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -276,7 +276,8 @@ private void doLookup(
var loaders = BlockReaderFactories.loaders(
List.of(searchContext),
extractField instanceof Alias a ? ((NamedExpression) a.child()).name() : extractField.name(),
EsqlDataTypes.isUnsupported(extractField.dataType())
EsqlDataTypes.isUnsupported(extractField.dataType()),
false
);
fields.add(new ValuesSourceReaderOperator.FieldInfo(extractField.name(), loaders));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,12 @@ public final PhysicalOperation fieldExtractPhysicalOperation(FieldExtractExec fi
layout.append(attr);
DataType dataType = attr.dataType();
String fieldName = attr.name();
List<BlockLoader> loaders = BlockReaderFactories.loaders(searchContexts, fieldName, EsqlDataTypes.isUnsupported(dataType));
List<BlockLoader> loaders = BlockReaderFactories.loaders(
searchContexts,
fieldName,
EsqlDataTypes.isUnsupported(dataType),
false
);
fields.add(new ValuesSourceReaderOperator.FieldInfo(fieldName, loaders));
}
return source.with(new ValuesSourceReaderOperator.Factory(fields, readers, docChannel), layout.build());
Expand Down Expand Up @@ -169,7 +174,7 @@ public final Operator.OperatorFactory ordinalGroupingOperatorFactory(
// The grouping-by values are ready, let's group on them directly.
// Costin: why are they ready and not already exposed in the layout?
return new OrdinalsGroupingOperator.OrdinalsGroupingOperatorFactory(
BlockReaderFactories.loaders(searchContexts, attrSource.name(), EsqlDataTypes.isUnsupported(attrSource.dataType())),
BlockReaderFactories.loaders(searchContexts, attrSource.name(), EsqlDataTypes.isUnsupported(attrSource.dataType()), true),
shardContexts,
groupElementType,
docChannel,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -239,6 +239,11 @@ public String indexName() {
throw new UnsupportedOperationException();
}

@Override
public boolean forStats() {
return false;
}

@Override
public SearchLookup lookup() {
throw new UnsupportedOperationException();
Expand Down

0 comments on commit 4b24c08

Please sign in to comment.