Skip to content

Let an explode declare the shape of the value it flows #4650

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
g31pranjal merged 3 commits into from
Sep 25, 2026
Merged

Let an explode declare the shape of the value it flows #4650

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d46370d
Let an explode declare the shape of the value it flows
g31pranjal Sep 22, 2026
ccd9163
update
g31pranjal Sep 24, 2026
91320b1
address comments
g31pranjal Sep 25, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
115 changes: 96 additions & 19 deletions ...java/com/apple/foundationdb/record/query/plan/cascades/expressions/ExplodeExpression.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,8 @@

import com.apple.foundationdb.annotation.API;
import com.apple.foundationdb.record.EvaluationContext;
import com.apple.foundationdb.record.query.plan.cascades.AliasMap;

Check notice on line 25 in fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/expressions/ExplodeExpression.java

View workflow job for this annotation

GitHub Actions / coverage 

File coverage: 97.3% (73/75 lines) | Changed lines: 100.0% (32/32 lines)
import com.apple.foundationdb.record.query.plan.cascades.Column;
import com.apple.foundationdb.record.query.plan.cascades.ComparisonRange;
import com.apple.foundationdb.record.query.plan.cascades.Compensation;
import com.apple.foundationdb.record.query.plan.cascades.CorrelationIdentifier;
Expand All @@ -35,6 +36,7 @@
import com.apple.foundationdb.record.query.plan.cascades.typing.Type;
import com.apple.foundationdb.record.query.plan.cascades.values.FieldValue;
import com.apple.foundationdb.record.query.plan.cascades.values.QueriedValue;
import com.apple.foundationdb.record.query.plan.cascades.values.RecordConstructorValue;
import com.apple.foundationdb.record.query.plan.cascades.values.Value;
import com.apple.foundationdb.record.query.plan.cascades.values.translation.PullUp;
import com.apple.foundationdb.record.query.plan.cascades.values.translation.TranslationMap;
Expand All @@ -57,6 +59,9 @@
* <p>In the {@code WITH ORDINALITY} variant, it also generates ordinals of the field values. In this case it
* produces a struct with two anonymous fields—the element and the ordinal—instead of the bare element. The ordinals
* are 1-based per the SQL standard, unless the expression is created {@linkplain #isZeroBasedOrdinality() 0-based}.
*
* <p>An explode that {@linkplain #flowsRecordConstructorValue() flows a record constructor} produces a struct in the
* plain variant as well, with the element as its only field.
*/
@API(API.Status.EXPERIMENTAL)
public class ExplodeExpression extends AbstractRelationalExpressionWithoutChildren implements InternalPlannerGraphRewritable {
Expand Down Expand Up @@ -85,15 +90,35 @@
@Nonnull
private final Type explodeResultType;

/**
* Whether the value this expression flows is a record constructor of the element—and the ordinal, for the
* {@code WITH ORDINALITY} variant—rather than one opaque value.
*/
private final boolean flowsRecordConstructorValue;

/**
* The result value of the explode.
*/
@Nonnull
private final Value resultValue;

public ExplodeExpression(@Nonnull final Value collectionValue, final boolean withOrdinality,
final boolean zeroBasedOrdinality) {
final boolean zeroBasedOrdinality, final boolean flowsRecordConstructorValue) {

@robert-brunel robert-brunel Sep 24, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also verify that zeroBasedOrdinality == true implies flowRecordConstructorValue == true?

@g31pranjal g31pranjal Sep 24, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This, does not hold currently. While the zeroBasedOrdinality is enforced everywhere by #4633, flowRecordConstructorValue will be enforced by #4651. After that, the implication will hold true - and will enforce that we are really covered everywhere.

@robert-brunel robert-brunel Sep 24, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK

Verify.verify(withOrdinality || !zeroBasedOrdinality, "cannot base ordinals that are not produced");
this.collectionValue = collectionValue;
this.withOrdinality = withOrdinality;
this.zeroBasedOrdinality = zeroBasedOrdinality;
Verify.verify(collectionValue.getResultType().isArray());
this.elementType = Objects.requireNonNull(((Type.Array)collectionValue.getResultType()).getElementType());
this.explodeResultType = explodeResultType(elementType, withOrdinality);
this.flowsRecordConstructorValue = flowsRecordConstructorValue;
this.explodeResultType = explodeResultType(elementType, withOrdinality, flowsRecordConstructorValue);
this.resultValue = explodeResultValue(elementType, withOrdinality, flowsRecordConstructorValue);
Verify.verify(explodeResultType.equals(resultValue.getResultType()));

@robert-brunel robert-brunel Sep 24, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We’re computing explodeResultType() twice, once here and once in explodeResultValue().

But since this equality holds, we probably don’t have to actually compute and store this.explodeResultType here anymore in addition to this.resultValue.

Also in the RecordQueryExplodePlan class.

@g31pranjal g31pranjal Sep 24, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think there is definitely merit to do this, and we should do this but probably at one of the cleanup stages, where type directly yields homogenous value. I'll create a follow cleanup PR that accommodates this.

@robert-brunel robert-brunel Sep 24, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK

}

public ExplodeExpression(@Nonnull final Value collectionValue, final boolean withOrdinality,
final boolean zeroBasedOrdinality) {
this(collectionValue, withOrdinality, zeroBasedOrdinality, false);
}

public ExplodeExpression(@Nonnull final Value collectionValue, final boolean withOrdinality) {
Expand All @@ -113,18 +138,28 @@
}

/**
* Returns the type of the explode result. For the {@code WITH ORDINALITY} variant, builds an anonymous-field
* struct result type holding the element and the ordinal.
* Returns the type of the explode result: the element type itself, or an anonymous-field struct holding the
* element and, for the {@code WITH ORDINALITY} variant, the ordinal. The {@code WITH ORDINALITY} variant needs the
* struct to carry the ordinal; the plain variant takes it only when the explode
* {@linkplain #flowsRecordConstructorValue() flows a record constructor}.
*
* @param elementType the element type of the collection being exploded
* @param withOrdinality whether ordinals are produced alongside the elements
* @param flowsRecordConstructorValue whether the explode flows a record constructor
* @return the result type of such an explode
*/
@Nonnull
public static Type explodeResultType(@Nonnull final Type elementType, boolean withOrdinality) {
if (withOrdinality) {
return Type.Record.fromFields(ImmutableList.of(
Type.Record.Field.of(elementType, Optional.empty()),
Type.Record.Field.of(Type.primitiveType(Type.TypeCode.INT, false), Optional.empty())));
} else {
public static Type explodeResultType(@Nonnull final Type elementType, final boolean withOrdinality,
final boolean flowsRecordConstructorValue) {
if (!withOrdinality && !flowsRecordConstructorValue) {

@alecgrieser alecgrieser Sep 25, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the design makes sense for now, but I did want to ask one thing about the future. In the future, for ExplodeExpressions without ordinality, are we going to return an RCV with a single element, or is this going to return just the element? Do we gain something by always wrapping? There may actually be advantages--for example, it may be easier for the planner to reason about a query for just the value that skips the ordinal if the expression is a projection of an expression on both fields

There are a number of places where this PR needed to update the wrapping logic to accommodate the new state of !withOrdinality && flowsRecordConstructorValue. If we were planning on banning that state long term (i.e., only wrapping if there's an ordinal), then in theory we could save some churn.

@g31pranjal g31pranjal Sep 25, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We will ultimately wrap for both the cases (i.e., with and without ordinals) - this is what I am aiming for. In short , the reason for it is to have homogeneity between the two case.

Let me explain it with what triggered this. We need the ability to match the query-side unnestings with a candidate side graph for leveraging the UnnestedRecordType. Note that the query-side could have EXPLODE with or without ordinals, but the candidate-side will always have it WITH ORIDINAL (because the ordinals form the primary key and descriptor of synthetic type). With this assumption, I figured out that its relatively difficult match a without-ordinal EXPLODE's QueriedValue from query-side to the RCV-wrapped WITH ORDINAL EXPLODE in the candidate-side. The easiest way, I feel, around this is to homogenize the output of the EXPLODE throughout the system so that both sides are actually FieldValue in the RCV. Having said that, I am still in the process of experimentally verify that the matching is correct - that is, we may have to do more changes around this.

Converting bare Value to RCV of that value has lots of ripple effects throughout, hence I am trying to fix those in #4623. Some of these are simply about making the consumer of the EXPLODE to understand the new shape. While others are more extensive, namely ImplementInJoin and ImplementInUnion that worked under the assumption that the "output" of EXPLODE is always the underlying array value. Still making my way through it, but I think doable.

Unfortunately, we will have to take the longer route, if we decide to move away from the plan this PR proposes. I'd love to discuss this more, if you have ideas related to it.

I am also planning to enable this in 2 steps - 1) only for WITH ORDINAL (smaller blast-radius, sufficient to match single unnested UnnestedRecordType-based indexes), and then, 2) all EXPLODEs, (this affects all plans that have EXPLODE).

return elementType;
}
final var fields = ImmutableList.<Type.Record.Field>builder();
fields.add(Type.Record.Field.of(elementType, Optional.empty()));
if (withOrdinality) {
fields.add(Type.Record.Field.of(Type.primitiveType(Type.TypeCode.INT, false), Optional.empty()));
}
return Type.Record.fromFields(fields.build());
}

/**
Expand All @@ -135,10 +170,35 @@
return explodeResultType;
}

/**
* Returns the value an explode of {@code elementType} flows, of the
* {@linkplain #explodeResultType(Type, boolean, boolean) explode result type} either way: one opaque
* {@link QueriedValue}, or a {@link RecordConstructorValue} whose columns are the element and, for the
* {@code WITH ORDINALITY} variant, the ordinal.
*
* @param elementType the element type of the collection being exploded
* @param withOrdinality whether ordinals are produced alongside the elements
* @param flowsRecordConstructorValue whether the element and the ordinal are flowed as a record constructor
* @return the value flowed by such an explode
*/
@Nonnull
public static Value explodeResultValue(@Nonnull final Type elementType, final boolean withOrdinality,
final boolean flowsRecordConstructorValue) {
if (!flowsRecordConstructorValue) {
return new QueriedValue(explodeResultType(elementType, withOrdinality, false));
}
final var columns = ImmutableList.<Column<? extends Value>>builder();
columns.add(Column.unnamedOf(new QueriedValue(elementType)));
if (withOrdinality) {
columns.add(Column.unnamedOf(new QueriedValue(Type.primitiveType(Type.TypeCode.INT, false))));
}
return RecordConstructorValue.ofColumns(columns.build(), true);
Comment thread
alecgrieser marked this conversation as resolved.
Resolved
}

@Nonnull
@Override
public Value getResultValue() {
return new QueriedValue(getExplodeResultType());
return resultValue;
}

@Nonnull
Expand All @@ -154,6 +214,15 @@
return zeroBasedOrdinality;
}

/**
* Returns whether the value this expression flows is a record constructor of the element—and the ordinal, for the
* {@code WITH ORDINALITY} variant—rather than one opaque value. The plan implementing the expression takes this
* over.
*/
public boolean flowsRecordConstructorValue() {
return flowsRecordConstructorValue;
}

@Nonnull
@Override
public List<? extends Quantifier> getQuantifiers() {
Expand All @@ -177,21 +246,28 @@
return collectionValue.semanticEquals(other.getCollectionValue(), equivalencesMap) &&
isWithOrdinality() == other.isWithOrdinality() &&
isZeroBasedOrdinality() == other.isZeroBasedOrdinality() &&
flowsRecordConstructorValue() == other.flowsRecordConstructorValue() &&
semanticEqualsForResults(otherExpression, equivalencesMap);
}
return false;
}

@Override
public int computeHashCodeWithoutChildren() {
// Note: This is written in a way that preserves pre-existing hashes for `withOrdinality=false` and for
// (`withOrdinality=true` and `zeroBasedOrdinality=false`)
if (!withOrdinality) {
return Objects.hash(collectionValue);
// Note: This is written in a way that preserves the hashes of every combination that could be expressed before
// an explode could flow a record constructor: that flag only ever appends to what was hashed before, so with
// the flag unset nothing is appended at all.
final var hashedObjects = ImmutableList.builder().add(collectionValue);
if (withOrdinality) {
hashedObjects.add(true);
if (zeroBasedOrdinality) {
hashedObjects.add(true);
}
}
if (flowsRecordConstructorValue) {
hashedObjects.add("flowsRecordConstructorValue");
}
return zeroBasedOrdinality
? Objects.hash(collectionValue, true, true)
: Objects.hash(collectionValue, true);
return Objects.hash(hashedObjects.build().toArray());
}

@Nonnull
Expand All @@ -205,7 +281,8 @@
collectionValue.translateCorrelations(translationMap, shouldSimplifyValues);
// this is ok since there are no new quantifiers
if (translatedCollectionValue != collectionValue) {
return new ExplodeExpression(translatedCollectionValue, withOrdinality, zeroBasedOrdinality);
return new ExplodeExpression(translatedCollectionValue, withOrdinality, zeroBasedOrdinality,
flowsRecordConstructorValue);
Comment thread
alecgrieser marked this conversation as resolved.
Resolved
}
return this;
}
Expand Down
16 changes: 11 additions & 5 deletions ...ava/com/apple/foundationdb/record/query/plan/cascades/properties/DerivationsProperty.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -350,12 +350,18 @@
final Type elementType = explodePlan.getElementType();
// Use `FirstOrDefaultValue` as a representative for “some” element of the array.
final Value first = new FirstOrDefaultValue(collectionValue, new ThrowsValue(elementType));
final Value representative;

Check notice on line 353 in fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/properties/DerivationsProperty.java

View workflow job for this annotation

GitHub Actions / coverage 

File coverage: 88.5% (363/410 lines) | Changed lines: 100.0% (7/7 lines)
if (explodePlan.isWithOrdinality()) {
final Type ordinalType = Type.primitiveType(Type.TypeCode.INT, false);
representative = RecordConstructorValue.ofColumns(List.of(
Column.of(Type.Record.Field.of(elementType, Optional.empty()), first),
Column.of(Type.Record.Field.of(ordinalType, Optional.empty()), new ThrowsValue(ordinalType))));
if (explodePlan.isWithOrdinality() || explodePlan.flowsRecordConstructorValue()) {
Comment thread
alecgrieser marked this conversation as resolved.
Resolved
// The plan flows the element wrapped in a struct, so the representative has to be wrapped the same way:
// accesses of the element are written against the wrapper.
final var columns = ImmutableList.<Column<? extends Value>>builder();
columns.add(Column.of(Type.Record.Field.of(elementType, Optional.empty()), first));
if (explodePlan.isWithOrdinality()) {
final Type ordinalType = Type.primitiveType(Type.TypeCode.INT, false);
columns.add(Column.of(Type.Record.Field.of(ordinalType, Optional.empty()),
new ThrowsValue(ordinalType)));
}
representative = RecordConstructorValue.ofColumns(columns.build(), true);
} else {
representative = first;
}
Expand Down
4 changes: 3 additions & 1 deletion ...in/java/com/apple/foundationdb/record/query/plan/cascades/rules/ImplementExplodeRule.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,9 @@
final ExplodeExpression explodeExpression = call.get(root);
final Value collectionValue = explodeExpression.getCollectionValue();
final boolean isWithOrdinality = explodeExpression.isWithOrdinality();
final boolean isZeroBasedOrdinality = explodeExpression.isZeroBasedOrdinality();

Check notice on line 52 in fdb-record-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/rules/ImplementExplodeRule.java

View workflow job for this annotation

GitHub Actions / coverage 

File coverage: 100.0% (10/10 lines) | Changed lines: 100.0% (2/2 lines)
call.yieldPlan(new RecordQueryExplodePlan(collectionValue, isWithOrdinality, isZeroBasedOrdinality));
final boolean flowsRecordConstructorValue = explodeExpression.flowsRecordConstructorValue();
call.yieldPlan(new RecordQueryExplodePlan(collectionValue, isWithOrdinality, isZeroBasedOrdinality,
flowsRecordConstructorValue));
}
}
Loading
Loading