Simplify merge translator + add test

indexer/src/main/java/bio/terra/tanagra/indexing/JobSequencer.java

@@ -74,15 +74,13 @@ public static SequencedJobSet getJobSetForEntity(
       jobSet.startNewStage();
 
       entity
-          .getOptimizeSearchByAttributes()
+          .getOptimizeSearchByAttributeNames()
           .forEach(
-              searchTableAttributes -> {
+              searchTableAttributeNames -> {
                 ITEntitySearchByAttributes searchTable =
                     underlay
                         .getIndexSchema()
-                        .getEntitySearchByAttributes(
-                            entity,
-                            searchTableAttributes.stream().map(Attribute::getName).toList());
+                        .getEntitySearchByAttributes(entity, searchTableAttributeNames);
                 jobSet.addJob(
                     new WriteEntitySearchByAttributes(
                         indexerConfig, entity, indexEntityMain, searchTable));

underlay/src/main/java/bio/terra/tanagra/api/filter/EntityFilter.java

@@ -47,8 +47,8 @@ public EntityFilter mergeWith(EntityFilter entityFilter) {
   }
 
   public static boolean areSameFilterType(List<EntityFilter> filters) {
-    String filterType = filters.get(0).getClass().getName();
-    return filters.stream().allMatch(filter -> filter.getClass().getName().equals(filterType));
+    Class<?> firstClazz = filters.get(0).getClass();
+    return filters.stream().skip(1).allMatch(filter -> filter.getClass().equals(firstClazz));
   }
 
   @Override

underlay/src/main/java/bio/terra/tanagra/query/bigquery/translator/filter/BQAttributeFilterTranslator.java

@@ -14,22 +14,21 @@
 import bio.terra.tanagra.underlay.entitymodel.Entity;
 import bio.terra.tanagra.underlay.indextable.ITEntityMain;
 import bio.terra.tanagra.underlay.indextable.ITEntitySearchByAttributes;
+import java.util.ArrayList;
 import java.util.List;
 import java.util.Map;
 
 public class BQAttributeFilterTranslator extends ApiFilterTranslator {
-  private final AttributeFilter attributeFilter;
-  private final List<AttributeFilter> attributeFilterList;
-  private final LogicalOperator logicalOperatorForList;
+  private final List<AttributeFilter> attributeFilters;
+  private final LogicalOperator logicalOperator; // not null if List.size > 1
 
   public BQAttributeFilterTranslator(
       ApiTranslator apiTranslator,
       AttributeFilter singleAttributeFilter,
       Map<Attribute, SqlField> attributeSwapFields) {
     super(apiTranslator, attributeSwapFields);
-    this.attributeFilter = singleAttributeFilter;
-    this.attributeFilterList = null;
-    this.logicalOperatorForList = null;
+    this.attributeFilters = List.of(singleAttributeFilter);
+    this.logicalOperator = null;
   }
 
   public BQAttributeFilterTranslator(
@@ -38,86 +37,92 @@ public BQAttributeFilterTranslator(
       LogicalOperator logicalOperator,
       Map<Attribute, SqlField> attributeSwapFields) {
     super(apiTranslator, attributeSwapFields);
-    this.attributeFilter = null;
-    this.attributeFilterList = attributeFilters;
-    this.logicalOperatorForList = logicalOperator;
+    this.attributeFilters = attributeFilters;
+    this.logicalOperator = logicalOperator;
   }
 
   @Override
   public String buildSql(SqlParams sqlParams, String tableAlias) {
-    if (attributeFilter != null) {
-      return buildSqlForSingleFilter(sqlParams, tableAlias);
-    } else {
-      return buildSqlForList(sqlParams, tableAlias);
-    }
+    return attributeFilters.size() == 1
+        ? buildSqlForSingleFilter(sqlParams, tableAlias)
+        : buildSqlForList(sqlParams, tableAlias);
   }
 
   private String buildSqlForSingleFilter(SqlParams sqlParams, String tableAlias) {
-    Entity entity = attributeFilter.getEntity();
+    AttributeFilter singleFilter = attributeFilters.get(0);
+    Entity entity = singleFilter.getEntity();
     ITEntityMain entityTable =
-        attributeFilter.getUnderlay().getIndexSchema().getEntityMain(entity.getName());
-    Attribute attribute = attributeFilter.getFilterAttributes().get(0);
+        singleFilter.getUnderlay().getIndexSchema().getEntityMain(entity.getName());
+
+    Attribute attribute = singleFilter.getFilterAttributes().get(0);
     SqlField valueField = fetchSelectField(entityTable, attribute);
+    String whereClause = buildWhereSql(singleFilter, sqlParams, tableAlias, valueField);
 
-    // search attribute-specific table if attribute is optimized for search
-    boolean isSearchOptimized =
-        entity.containsOptimizeSearchByAttributes(List.of(attribute.getName()));
-
-    if (!isSearchOptimized && attribute.isDataTypeRepeated()) {
-      boolean naryOperatorIn =
-          (attributeFilter.hasBinaryOperator()
-                  && BinaryOperator.EQUALS.equals(attributeFilter.getBinaryOperator()))
-              || (attributeFilter.hasNaryOperator()
-                  && NaryOperator.IN.equals(attributeFilter.getNaryOperator()));
-      boolean naryOperatorNotIn =
-          (attributeFilter.hasBinaryOperator()
-                  && BinaryOperator.NOT_EQUALS.equals(attributeFilter.getBinaryOperator()))
-              || (attributeFilter.hasNaryOperator()
-                  && NaryOperator.NOT_IN.equals(attributeFilter.getNaryOperator()));
-      if (!naryOperatorIn && !naryOperatorNotIn) {
-        throw new InvalidQueryException(
-            "Operator not supported for repeated data type attributes: "
-                + attributeFilter.getOperatorName()
-                + ", "
-                + attribute.getName());
-      }
-      return apiTranslator.naryFilterOnRepeatedFieldSql(
-          valueField,
-          naryOperatorIn ? NaryOperator.IN : NaryOperator.NOT_IN,
-          attributeFilter.getValues(),
-          tableAlias,
-          sqlParams);
+    // optimized for search on attribute (dataType is not repeated in searchOptimized tables)
+    if (entity.containsOptimizeSearchByAttributes(List.of(attribute.getName()))) {
+      return searchOptimizedSql(singleFilter, tableAlias, whereClause);
+    } else if (!attribute.isDataTypeRepeated()) {
+      return whereClause;
+    } else {
+      return buildSqlForRepeatedAttribute(sqlParams, tableAlias, singleFilter, valueField);
     }
+  }
 
-    // Build sql where clause for the attribute values
-    String whereClause = buildWhereSql(attributeFilter, sqlParams, tableAlias, valueField);
-    return isSearchOptimized
-        ? searchOptimizedSql(attributeFilter, tableAlias, whereClause)
-        : whereClause;
+  private String buildSqlForRepeatedAttribute(
+      SqlParams sqlParams, String tableAlias, AttributeFilter filter, SqlField valueField) {
+    boolean naryOperatorIn =
+        (filter.hasBinaryOperator() && BinaryOperator.EQUALS.equals(filter.getBinaryOperator()))
+            || (filter.hasNaryOperator() && NaryOperator.IN.equals(filter.getNaryOperator()));
+    boolean naryOperatorNotIn =
+        (filter.hasBinaryOperator() && BinaryOperator.NOT_EQUALS.equals(filter.getBinaryOperator()))
+            || (filter.hasNaryOperator() && NaryOperator.NOT_IN.equals(filter.getNaryOperator()));
+    if (!naryOperatorIn && !naryOperatorNotIn) {
+      throw new InvalidQueryException(
+          "Operator not supported for repeated data type attributes: "
+              + filter.getOperatorName()
+              + ", "
+              + filter.getFilterAttributes().get(0).getName());
+    }
+    return apiTranslator.naryFilterOnRepeatedFieldSql(
+        valueField,
+        naryOperatorIn ? NaryOperator.IN : NaryOperator.NOT_IN,
+        filter.getValues(),
+        tableAlias,
+        sqlParams);
   }
 
   private String buildSqlForList(SqlParams sqlParams, String tableAlias) {
-    AttributeFilter firstFilter = attributeFilterList.get(0);
+    AttributeFilter firstFilter = attributeFilters.get(0);
     Entity entity = firstFilter.getEntity();
     ITEntityMain entityTable =
         firstFilter.getUnderlay().getIndexSchema().getEntityMain(entity.getName());
 
+    List<String> attributeNames = new ArrayList<>();
+
     String[] subFilterClauses =
-        attributeFilterList.stream()
+        attributeFilters.stream()
             .map(
-                filter ->
-                    buildWhereSql(
-                        filter,
-                        sqlParams,
-                        tableAlias,
-                        fetchSelectField(entityTable, filter.getFilterAttributes().get(0))))
+                filter -> {
+                  Attribute attribute = filter.getFilterAttributes().get(0);
+                  attributeNames.add(attribute.getName());
+                  SqlField valueField = fetchSelectField(entityTable, attribute);
+                  return attribute.isDataTypeRepeated()
+                      ? buildSqlForRepeatedAttribute(sqlParams, tableAlias, filter, valueField)
+                      : buildWhereSql(filter, sqlParams, tableAlias, valueField);
+                })
             .toList()
             .toArray(new String[0]);
+    String whereClause = apiTranslator.booleanAndOrFilterSql(logicalOperator, subFilterClauses);
 
-    String whereClause =
-        apiTranslator.booleanAndOrFilterSql(logicalOperatorForList, subFilterClauses);
+    // case-1: all attrs are in 'optimizedForSearchByAttributes' list
+    // case-2: all attrs are not in 'optimizedForSearchByAttributes' list
 
-    return searchOptimizedSql(firstFilter, tableAlias, whereClause);
+    // optimized for search on attribute (dataType is not repeated in searchOptimized tables)
+    if (entity.containsOptimizeSearchByAttributes(attributeNames)) {
+      return searchOptimizedSql(firstFilter, tableAlias, whereClause);
+    } else {
+      return whereClause;
+    }
   }
 
   private String searchOptimizedSql(AttributeFilter filter, String tableAlias, String whereClause) {
@@ -159,35 +164,19 @@ private String buildWhereSql(
 
   @Override
   public boolean isFilterOnAttribute(Attribute attribute) {
-    return attributeFilter != null
-        && attribute.equals(attributeFilter.getFilterAttributes().get(0));
+    return attributeFilters.size() == 1
+        && attribute.equals(attributeFilters.get(0).getFilterAttributes().get(0));
   }
 
   public static boolean canMergeTranslation(List<AttributeFilter> attributeFilters) {
-    // Can merge (AND) the 'where' clauses if are all optimized on search together
-    AttributeFilter firstFilter = attributeFilters.get(0);
-    Entity firstEntity = firstFilter.getEntity();
-    List<String> attributeNames =
-        firstFilter.getFilterAttributes().stream().map(Attribute::getName).toList();
-
-    if (!firstEntity.containsOptimizeSearchByAttributes(List.of(attributeNames.get(0)))) {
-      // first attribute itself is not optimized for search
-      return false;
+    if (attributeFilters.size() == 1) {
+      return true;
     }
 
-    List<String> searchTableAttributes =
-        firstFilter
-            .getUnderlay()
-            .getIndexSchema()
-            .getEntitySearchByAttributes(firstEntity, attributeNames)
-            .getAttributeNames();
-
-    // check if all attributes in the filters are in the same search table for the same entity
+    // all filters must be on the same entity
+    String firstEntityName = attributeFilters.get(0).getEntity().getName();
     return attributeFilters.stream()
-        .allMatch(
-            filter ->
-                filter.getEntity().getName().equals(firstEntity.getName())
-                    && searchTableAttributes.contains(
-                        filter.getFilterAttributes().get(0).getName()));
+        .skip(1)
+        .allMatch(filter -> filter.getEntity().getName().equals(firstEntityName));
   }
 }

underlay/src/main/java/bio/terra/tanagra/query/bigquery/translator/filter/BQPrimaryWithCriteriaFilterTranslator.java

@@ -81,7 +81,7 @@ public String buildSql(SqlParams sqlParams, String tableAlias) {
 
     List<String> selectSqls = new ArrayList<>();
     CriteriaOccurrence criteriaOccurrence = primaryWithCriteriaFilter.getCriteriaOccurrence();
-    primaryWithCriteriaFilter.getCriteriaOccurrence().getOccurrenceEntities().stream()
+    criteriaOccurrence.getOccurrenceEntities().stream()
         .sorted(
             Comparator.comparing(
                 Entity::getName)) // Sort by name so the generated SQL is deterministic.

underlay/src/main/java/bio/terra/tanagra/query/sql/translator/ApiTranslator.java

@@ -420,18 +420,19 @@ default Optional<ApiFilterTranslator> optionalMergedTranslator(
       List<EntityFilter> entityFilters,
       LogicalOperator logicalOperator,
       Map<Attribute, SqlField> attributeSwapFields) {
-    // A list of sub-filters can be merged (optimized) if they are of the same type
+    // A list of different sub-filters cannot be merged (optimized)
     // Additional checks may be needed for individual sub-filter types
-    EntityFilter firstFilter = entityFilters.get(0);
-
-    // At this time only optimize attribute filters are supported
-    if (!(firstFilter instanceof AttributeFilter)
-        || !EntityFilter.areSameFilterType(entityFilters)) {
+    if (!EntityFilter.areSameFilterType(entityFilters)) {
       return Optional.empty();
     }
-    return mergedTranslator(
-        entityFilters.stream().map(filter -> (AttributeFilter) filter).toList(),
-        logicalOperator,
-        attributeSwapFields);
+
+    EntityFilter firstFilter = entityFilters.get(0);
+    if (firstFilter instanceof AttributeFilter) {
+      return mergedTranslator(
+          entityFilters.stream().map(filter -> (AttributeFilter) filter).toList(),
+          logicalOperator,
+          attributeSwapFields);
+    }
+    return Optional.empty();
   }
 }

underlay/src/main/java/bio/terra/tanagra/query/sql/translator/filter/BooleanAndOrFilterTranslator.java

@@ -14,38 +14,35 @@
 public class BooleanAndOrFilterTranslator extends ApiFilterTranslator {
   private final BooleanAndOrFilter booleanAndOrFilter;
   private final List<ApiFilterTranslator> subFilterTranslators;
-  private final Optional<ApiFilterTranslator> mergedSubFiltersTranslator;
 
   public BooleanAndOrFilterTranslator(
       ApiTranslator apiTranslator,
       BooleanAndOrFilter booleanAndOrFilter,
       Map<Attribute, SqlField> attributeSwapFields) {
     super(apiTranslator, attributeSwapFields);
     this.booleanAndOrFilter = booleanAndOrFilter;
-    this.mergedSubFiltersTranslator =
+
+    Optional<ApiFilterTranslator> mergedSubFiltersTranslator =
         apiTranslator.optionalMergedTranslator(
             booleanAndOrFilter.getSubFilters(),
             booleanAndOrFilter.getOperator(),
             attributeSwapFields);
     this.subFilterTranslators =
-        mergedSubFiltersTranslator.isEmpty()
-            ? booleanAndOrFilter.getSubFilters().stream()
-                .map(filter -> apiTranslator.translator(filter, attributeSwapFields))
-                .collect(Collectors.toList())
-            : List.of();
+        mergedSubFiltersTranslator
+            .map(List::of)
+            .orElseGet(
+                () ->
+                    booleanAndOrFilter.getSubFilters().stream()
+                        .map(filter -> apiTranslator.translator(filter, attributeSwapFields))
+                        .collect(Collectors.toList()));
   }
 
   @Override
   public String buildSql(SqlParams sqlParams, String tableAlias) {
-    List<String> subFilterSqls;
-    if (mergedSubFiltersTranslator.isPresent()) {
-      subFilterSqls = List.of(mergedSubFiltersTranslator.get().buildSql(sqlParams, tableAlias));
-    } else {
-      subFilterSqls =
-          subFilterTranslators.stream()
-              .map(subFilterTranslator -> subFilterTranslator.buildSql(sqlParams, tableAlias))
-              .toList();
-    }
+    List<String> subFilterSqls =
+        subFilterTranslators.stream()
+            .map(subFilterTranslator -> subFilterTranslator.buildSql(sqlParams, tableAlias))
+            .toList();
     return apiTranslator.booleanAndOrFilterSql(
         booleanAndOrFilter.getOperator(), subFilterSqls.toArray(new String[0]));
   }

underlay/src/main/java/bio/terra/tanagra/underlay/entitymodel/Entity.java

@@ -14,7 +14,6 @@ public final class Entity {
   private final ImmutableList<Attribute> attributes;
   private final ImmutableList<Hierarchy> hierarchies;
   private final ImmutableList<Attribute> optimizeGroupByAttributes;
-  private final ImmutableList<ImmutableList<Attribute>> optimizeSearchByAttributes;
   private final ImmutableList<ImmutableList<String>> optimizeSearchByAttributeNames;
   private final boolean hasTextSearch;
   private final ImmutableList<Attribute> optimizeTextSearchAttributes;
@@ -40,9 +39,6 @@ public Entity(
     this.attributes = ImmutableList.copyOf(attributes);
     this.hierarchies = ImmutableList.copyOf(hierarchies);
     this.optimizeGroupByAttributes = ImmutableList.copyOf(optimizeGroupByAttributes);
-    this.optimizeSearchByAttributes =
-        ImmutableList.copyOf(
-            optimizeSearchByAttributes.stream().map(ImmutableList::copyOf).toList());
     this.optimizeSearchByAttributeNames =
         ImmutableList.copyOf(
             optimizeSearchByAttributes.stream()
@@ -132,17 +128,17 @@ public ImmutableList<Attribute> getOptimizeGroupByAttributes() {
   }
 
   public boolean hasOptimizeSearchByAttributes() {
-    return !optimizeSearchByAttributes.isEmpty();
+    return !optimizeSearchByAttributeNames.isEmpty();
   }
 
-  public ImmutableList<ImmutableList<Attribute>> getOptimizeSearchByAttributes() {
-    return optimizeSearchByAttributes;
+  public ImmutableList<ImmutableList<String>> getOptimizeSearchByAttributeNames() {
+    return optimizeSearchByAttributeNames;
   }
 
   public boolean containsOptimizeSearchByAttributes(List<String> attributeNames) {
     return !attributeNames.isEmpty()
         && optimizeSearchByAttributeNames.stream()
-            .anyMatch(attributeList -> attributeList.containsAll(attributeNames));
+            .anyMatch(attributes -> attributes.containsAll(attributeNames));
   }
 
   public boolean hasTextSearch() {