Skip to content

Commit

Permalink
Merge pull request vespa-engine#23967 from vespa-engine/geirst/remove…
Browse files Browse the repository at this point in the history
…-summarymap-from-config-model

Remove producing summarymap config from the config model [run-systemtest]
  • Loading branch information
geirst committed Sep 7, 2022
2 parents 267649a + 3fe5b4c commit ceb337d
Show file tree
Hide file tree
Showing 41 changed files with 199 additions and 1,062 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ public class DerivedConfiguration implements AttributesConfig.Producer {

private final Schema schema;
private Summaries summaries;
private SummaryMap summaryMap;
private Juniperrc juniperrc;
private AttributeFields attributeFields;
private RankProfileList rankProfileList;
Expand Down Expand Up @@ -80,12 +79,11 @@ public DerivedConfiguration(Schema schema, DeployState deployState) {
if ( ! schema.isDocumentsOnly()) {
attributeFields = new AttributeFields(schema);
summaries = new Summaries(schema, deployState.getDeployLogger(), deployState.getProperties().featureFlags());
summaryMap = new SummaryMap(schema);
juniperrc = new Juniperrc(schema);
rankProfileList = new RankProfileList(schema, schema.rankExpressionFiles(), attributeFields, deployState);
indexingScript = new IndexingScript(schema);
indexInfo = new IndexInfo(schema);
schemaInfo = new SchemaInfo(schema, deployState.rankProfileRegistry(), summaries, summaryMap);
schemaInfo = new SchemaInfo(schema, deployState.rankProfileRegistry(), summaries);
indexSchema = new IndexSchema(schema);
importedFields = new ImportedFields(schema);
}
Expand All @@ -101,7 +99,6 @@ public DerivedConfiguration(Schema schema, DeployState deployState) {
public void export(String toDirectory) throws IOException {
if (!schema.isDocumentsOnly()) {
summaries.export(toDirectory);
summaryMap.export(toDirectory);
juniperrc.export(toDirectory);
attributeFields.export(toDirectory);
streamingFields.export(toDirectory);
Expand Down Expand Up @@ -199,10 +196,6 @@ public Juniperrc getJuniperrc() {
return juniperrc;
}

public SummaryMap getSummaryMap() {
return summaryMap;
}

public ImportedFields getImportedFields() {
return importedFields;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
import com.yahoo.schema.RankProfileRegistry;
import com.yahoo.schema.Schema;
import com.yahoo.searchlib.rankingexpression.Reference;
import com.yahoo.vespa.documentmodel.SummaryTransform;

import java.util.Collection;
import java.util.Collections;
Expand All @@ -27,14 +26,11 @@ public final class SchemaInfo extends Derived implements SchemaInfoConfig.Produc
private final Map<String, RankProfileInfo> rankProfiles;

private final Summaries summaries;
private final SummaryMap summaryMap;

public SchemaInfo(Schema schema, RankProfileRegistry rankProfileRegistry,
Summaries summaries, SummaryMap summaryMap) {
public SchemaInfo(Schema schema, RankProfileRegistry rankProfileRegistry, Summaries summaries) {
this.schema = schema;
this.rankProfiles = Collections.unmodifiableMap(toRankProfiles(rankProfileRegistry.rankProfilesOf(schema)));
this.summaries = summaries;
this.summaryMap = summaryMap;
}

public String name() { return schema.getName(); }
Expand Down Expand Up @@ -69,25 +65,13 @@ private void addSummaryConfig(SchemaInfoConfig.Schema.Builder schemaBuilder) {
var fieldsBuilder = new SchemaInfoConfig.Schema.Summaryclass.Fields.Builder();
fieldsBuilder.name(field.getName())
.type(field.getType().getName())
.dynamic(isDynamic(field.getName()));
.dynamic(SummaryClass.commandRequiringQuery(field.getCommand()));
summaryBuilder.fields(fieldsBuilder);
}
schemaBuilder.summaryclass(summaryBuilder);
}
}

/** Returns whether the given field is a dynamic summary field. */
private boolean isDynamic(String fieldName) {
if (summaryMap == null) return false; // not know for streaming, but also not used

var fieldTransform = summaryMap.resultTransforms().get(fieldName);
if (fieldTransform == null) return false;
// TODO: Move this into SummaryTransform and call it something else than "dynamic"
return fieldTransform.getTransform().isDynamic() ||
fieldTransform.getTransform() == SummaryTransform.MATCHED_ELEMENTS_FILTER ||
fieldTransform.getTransform() == SummaryTransform.MATCHED_ATTRIBUTE_ELEMENTS_FILTER;
}

private void addRankProfilesConfig(SchemaInfoConfig.Schema.Builder schemaBuilder) {
for (RankProfileInfo rankProfile : rankProfiles().values()) {
var rankProfileConfig = new SchemaInfoConfig.Schema.Rankprofile.Builder();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ private void deriveFields(Schema schema, DocumentSummary summary, Map<String, Su
accessingDiskSummary = true;
}
addField(summaryField.getName(), summaryField.getDataType(), summaryField.getTransform(),
SummaryMap.getSource(summaryField), fields);
getSource(summaryField), fields);
}
}

Expand Down Expand Up @@ -130,4 +130,53 @@ public String toString() {
return "summary class '" + getName() + "'";
}

/** Returns the command name of a transform */
static String getCommand(SummaryTransform transform) {
if (transform == SummaryTransform.NONE) {
return "";
} else if (transform == SummaryTransform.DISTANCE) {
return "absdist";
} else if (transform.isDynamic()) {
return "dynamicteaser";
} else {
return transform.getName();
}
}

static String getSource(SummaryField summaryField) {
if (summaryField.getTransform() == SummaryTransform.NONE) {
return "";
}

if (summaryField.getTransform() == SummaryTransform.ATTRIBUTE ||
(summaryField.getTransform() == SummaryTransform.ATTRIBUTECOMBINER && summaryField.hasExplicitSingleSource()) ||
summaryField.getTransform() == SummaryTransform.COPY ||
summaryField.getTransform() == SummaryTransform.DISTANCE ||
summaryField.getTransform() == SummaryTransform.GEOPOS ||
summaryField.getTransform() == SummaryTransform.POSITIONS ||
summaryField.getTransform() == SummaryTransform.MATCHED_ELEMENTS_FILTER ||
summaryField.getTransform() == SummaryTransform.MATCHED_ATTRIBUTE_ELEMENTS_FILTER)
{
return summaryField.getSingleSource();
} else {
// Note: Currently source mapping is handled in the indexing statement,
// by creating a summary field for each of the values
// This works, but is suboptimal. We could consolidate to a minimal set and
// use the right value from the minimal set as the third parameter here,
// and add "override" commands to multiple static values
boolean useFieldNameAsArgument = summaryField.getTransform().isDynamic();
return useFieldNameAsArgument ? summaryField.getName() : "";
}
}

/**
* A dynamic transform that needs the query to perform its computations.
* We need this because some model information is shared through configs instead of model - see usage
*/
static boolean commandRequiringQuery(String commandName) {
return (commandName.equals("dynamicteaser") ||
commandName.equals(SummaryTransform.MATCHED_ELEMENTS_FILTER.getName()) ||
commandName.equals(SummaryTransform.MATCHED_ATTRIBUTE_ELEMENTS_FILTER.getName()));
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ public String toString() {
public SummaryClassField(String name, DataType type, SummaryTransform transform, String source, boolean rawAsBase64) {
this.name = name;
this.type = convertDataType(type, transform, rawAsBase64);
this.command = SummaryMap.getCommand(transform);
this.command = SummaryClass.getCommand(transform);
this.source = source;
}

Expand Down
133 changes: 0 additions & 133 deletions config-model/src/main/java/com/yahoo/schema/derived/SummaryMap.java

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -245,7 +245,7 @@ private void addSchemas(DeployState deployState, List<ModelElement> searchDefs,
throw new IllegalArgumentException("Schema '" + schemaDefinitionXMLHandler.getName() + "' referenced in " +
this + " does not exist");

sc.add(new SchemaInfo(schema, deployState.rankProfileRegistry(), null, null));
sc.add(new SchemaInfo(schema, deployState.rankProfileRegistry(), null));
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
import com.yahoo.vespa.config.search.IndexschemaConfig;
import com.yahoo.vespa.config.search.RankProfilesConfig;
import com.yahoo.vespa.config.search.SummaryConfig;
import com.yahoo.vespa.config.search.SummarymapConfig;
import com.yahoo.vespa.config.search.core.OnnxModelsConfig;
import com.yahoo.vespa.config.search.core.RankingConstantsConfig;
import com.yahoo.vespa.config.search.core.RankingExpressionsConfig;
Expand All @@ -32,7 +31,6 @@ public class DocumentDatabase extends AbstractConfigProducer<DocumentDatabase> i
OnnxModelsConfig.Producer,
IndexschemaConfig.Producer,
JuniperrcConfig.Producer,
SummarymapConfig.Producer,
SummaryConfig.Producer,
ImportedFieldsConfig.Producer,
SchemaInfoConfig.Producer {
Expand Down Expand Up @@ -85,8 +83,6 @@ public DerivedConfiguration getDerivedConfiguration() {
@Override
public void getConfig(JuniperrcConfig.Builder builder) { derivedCfg.getJuniperrc().getConfig(builder); }

@Override
public void getConfig(SummarymapConfig.Builder builder) { derivedCfg.getSummaryMap().getConfig(builder); }
@Override
public void getConfig(SummaryConfig.Builder builder) { derivedCfg.getSummaries().getConfig(builder); }

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
import com.yahoo.vespa.config.search.AttributesConfig;
import com.yahoo.vespa.config.search.RankProfilesConfig;
import com.yahoo.vespa.config.search.SummaryConfig;
import com.yahoo.vespa.config.search.SummarymapConfig;
import com.yahoo.vespa.config.search.vsm.VsmfieldsConfig;
import com.yahoo.vespa.config.search.vsm.VsmsummaryConfig;
import com.yahoo.vespa.configdefinition.IlscriptsConfig;
Expand All @@ -28,7 +27,6 @@ public class StreamingSearchCluster extends SearchCluster implements
RankProfilesConfig.Producer,
VsmsummaryConfig.Producer,
VsmfieldsConfig.Producer,
SummarymapConfig.Producer,
SummaryConfig.Producer {

private final String storageRouteSpec;
Expand Down Expand Up @@ -119,12 +117,6 @@ public void getConfig(VsmfieldsConfig.Builder builder) {
derivedConfig.getVsmFields().getConfig(builder);
}

@Override
public void getConfig(SummarymapConfig.Builder builder) {
if (derivedConfig.getSummaryMap() != null)
derivedConfig.getSummaryMap().getConfig(builder);
}

@Override
public void getConfig(SummaryConfig.Builder builder) {
if (derivedConfig.getSummaries() != null)
Expand Down
13 changes: 0 additions & 13 deletions config-model/src/test/derived/advanced/summarymap.cfg

This file was deleted.

This file was deleted.

0 comments on commit ceb337d

Please sign in to comment.