add placeholders in all appropriate select expressions (#3451)

Author: normen662

fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/AggregateIndexExpansionVisitor.java

@@ -173,8 +173,6 @@ private NonnullPair<Quantifier, List<Placeholder>> constructSelectWhereAndPlaceh
 
         // add the SELECT-WHERE part, where we expose grouping and grouped columns, allowing query fragments that governs
         // only these columns to properly bind to this part, similar to how value indices work.
-        final ImmutableList.Builder<CorrelationIdentifier> placeholders = ImmutableList.builder();
-        placeholders.addAll(baseExpansion.getPlaceholderAliases());
 
         if (index.hasPredicate()) {
             final var filteredIndexPredicate = Objects.requireNonNull(index.getPredicate()).toPredicate(baseQuantifier.getFlowedObjectValue());

fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/KeyExpressionExpansionVisitor.java

@@ -35,6 +35,7 @@
 import com.apple.foundationdb.record.metadata.expressions.ThenKeyExpression;
 import com.apple.foundationdb.record.query.plan.cascades.KeyExpressionExpansionVisitor.VisitorState;
 import com.apple.foundationdb.record.query.plan.cascades.expressions.SelectExpression;
+import com.apple.foundationdb.record.query.plan.cascades.predicates.Placeholder;
 import com.apple.foundationdb.record.query.plan.cascades.predicates.PredicateWithValueAndRanges;
 import com.apple.foundationdb.record.query.plan.cascades.values.EmptyValue;
 import com.apple.foundationdb.record.query.plan.cascades.values.FieldValue;
@@ -48,6 +49,8 @@
 import java.util.ArrayDeque;
 import java.util.Deque;
 import java.util.List;
+import java.util.Objects;
+import java.util.stream.Collectors;
 
 /**
  * Expansion visitor that implements the shared logic between primary scan data access and value index access.
@@ -294,31 +297,18 @@ public GraphExpansion visitExpression(@Nonnull final NestingKeyExpression nestin
                     final SelectExpression selectExpression =
                             sealedBaseAndChildExpansion.buildSelect();
                     final Quantifier childQuantifier = Quantifier.forEach(Reference.initialOf(selectExpression));
-
-                    final var childExpansionValues =
-                            childExpansion.getResultColumns()
-                                    .stream()
-                                    .map(Column::getValue)
-                                    .collect(ImmutableList.toImmutableList());
                     final var childResultValue = selectExpression.getResultValue();
-                    final var pulledUpValuesMap =
-                            childResultValue.pullUp(childExpansionValues, EvaluationContext.empty(),
-                                    AliasMap.emptyMap(), ImmutableSet.of(), childQuantifier.getAlias());
-                    final ImmutableList<Column<? extends Value>> pulledUpExpansionColumns =
-                            childExpansionValues.stream()
-                                    .map(value -> {
-                                        if (!pulledUpValuesMap.containsKey(value)) {
-                                            throw new RecordCoreException("could not pull expansion value " + value)
-                                                    .addLogInfo(LogMessageKeys.VALUE, value);
-                                        }
-                                        return pulledUpValuesMap.get(value);
-                                    })
-                                    .map(Column::unnamedOf)
-                                    .collect(ImmutableList.toImmutableList());
 
-                    return sealedBaseAndChildExpansion
-                            .builderWithInheritedPlaceholders()
+                    final var pulledUpExpansionColumns =
+                            pullUpResultColumns(childExpansion, childResultValue, childQuantifier);
+
+                    final var pulledUpPlaceholders =
+                            pullUpPlaceholders(childExpansion, childResultValue, childQuantifier);
+
+                    return GraphExpansion.builder()
                             .addQuantifier(childQuantifier)
+                            .addAllPredicates(pulledUpPlaceholders)
+                            .addAllPlaceholders(pulledUpPlaceholders)
                             .addAllResultColumns(pulledUpExpansionColumns)
                             .build();
                 }
@@ -328,6 +318,65 @@ public GraphExpansion visitExpression(@Nonnull final NestingKeyExpression nestin
         }
     }
 
+    @Nonnull
+    private static ImmutableList<Placeholder> pullUpPlaceholders(@Nonnull final GraphExpansion childExpansion,
+                                                                 @Nonnull final Value childResultValue,
+                                                                 @Nonnull final Quantifier childQuantifier) {
+        final var childExpansionPlaceholderValuesMap =
+                childExpansion.getPlaceholders()
+                        .stream()
+                        .collect(Collectors.toMap(PredicateWithValueAndRanges::getValue,
+                                Placeholder::getParameterAlias,
+                                (l, r) -> {
+                                    if (l.equals(r)) {
+                                        return l;
+                                    }
+                                    throw new RecordCoreException("ambiguous values in placeholder map");
+                                },
+                                LinkedIdentityMap::new));
+        final var childExpansionPlaceholderValues = childExpansionPlaceholderValuesMap.keySet();
+        final var pulledUpPlaceholderValuesMap =
+                childResultValue.pullUp(childExpansionPlaceholderValues, EvaluationContext.empty(),
+                        AliasMap.emptyMap(), ImmutableSet.of(), childQuantifier.getAlias());
+        return childExpansionPlaceholderValues
+                .stream()
+                .map(value -> {
+                    if (!pulledUpPlaceholderValuesMap.containsKey(value)) {
+                        throw new RecordCoreException("could not pull expansion value " + value)
+                                .addLogInfo(LogMessageKeys.VALUE, value);
+                    }
+                    final var pulledUpValue = pulledUpPlaceholderValuesMap.get(value);
+                    final var parameterAlias =
+                            Objects.requireNonNull(childExpansionPlaceholderValuesMap.get(value));
+                    return Placeholder.newInstanceWithoutRanges(pulledUpValue, parameterAlias);
+                })
+                .collect(ImmutableList.toImmutableList());
+    }
+
+    @Nonnull
+    private static ImmutableList<Column<? extends Value>> pullUpResultColumns(@Nonnull final GraphExpansion childExpansion,
+                                                                              @Nonnull final Value childResultValue,
+                                                                              @Nonnull final Quantifier childQuantifier) {
+        final var childExpansionValues =
+                childExpansion.getResultColumns()
+                        .stream()
+                        .map(Column::getValue)
+                        .collect(ImmutableList.toImmutableList());
+        final var pulledUpValuesMap =
+                childResultValue.pullUp(childExpansionValues, EvaluationContext.empty(),
+                        AliasMap.emptyMap(), ImmutableSet.of(), childQuantifier.getAlias());
+        return childExpansionValues.stream()
+                .map(value -> {
+                    if (!pulledUpValuesMap.containsKey(value)) {
+                        throw new RecordCoreException("could not pull expansion value " + value)
+                                .addLogInfo(LogMessageKeys.VALUE, value);
+                    }
+                    return pulledUpValuesMap.get(value);
+                })
+                .map(Column::unnamedOf)
+                .collect(ImmutableList.toImmutableList());
+    }
+
     @Nonnull
     @Override
     public GraphExpansion visitExpression(@Nonnull final ThenKeyExpression thenKeyExpression) {