Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
83078c9
Add continuations to struct type name tests
arnaud-lacurie Nov 13, 2025
64d5a83
Add more tests
arnaud-lacurie Nov 13, 2025
2e9890b
Fix struct type metadata preservation in query results and continuations
arnaud-lacurie Nov 14, 2025
7a808ab
Add some tests around named structs
arnaud-lacurie Nov 14, 2025
b12aea9
Fix style and remove unnecessary sorting
arnaud-lacurie Nov 17, 2025
21eea73
Add additional tests to cover teamscale coverage gap
arnaud-lacurie Nov 17, 2025
c3f9c81
Add test coverage for ContinuedPhysicalQueryPlan.withExecutionContext
arnaud-lacurie Nov 17, 2025
f3307bf
Merge branch 'main' into struct_type_metadata_fix
arnaud-lacurie Nov 17, 2025
9435474
Extract type enrichment logic to TypeMetadataEnricher utility class
arnaud-lacurie Nov 17, 2025
a694dfc
Change TypeMetadataEnricher class to final
arnaud-lacurie Nov 17, 2025
5ce3daf
Remove deprecated `WITH CONTINUATION` syntax now that `EXECUTE CONTIN…
arnaud-lacurie Nov 18, 2025
cff2f13
Merge branch 'remove_with_continuation' into struct_type_metadata_fix
arnaud-lacurie Nov 20, 2025
6a66aaa
Merge branch 'struct_type_metadata_fix' of github.com:arnaud-lacurie/…
arnaud-lacurie Nov 20, 2025
5760a9d
Merge remote-tracking branch 'upstream/main' into struct_type_metadat…
arnaud-lacurie Nov 26, 2025
42bbe3c
Address teamscale issues
arnaud-lacurie Nov 27, 2025
9d1ba5e
Minor fix
arnaud-lacurie Nov 27, 2025
6af1e5c
Move getDataTypes into Expressions
arnaud-lacurie Nov 28, 2025
76e6542
Refactor semantic type capture to use StructType
arnaud-lacurie Nov 28, 2025
063bc47
Remove unused import
arnaud-lacurie Nov 28, 2025
4c122af
Remove unnecessary file
arnaud-lacurie Nov 28, 2025
93b5f87
Remove unused method
arnaud-lacurie Nov 28, 2025
1386af8
Merge remote-tracking branch 'upstream/main' into struct_type_metadat…
arnaud-lacurie Dec 1, 2025
be9e823
Rely solely on Relational metadata to construct (nested) ResultSetMet…
hatyo Dec 18, 2025
e62d708
Fix checkstyle
arnaud-lacurie Dec 18, 2025
b7af2d6
Merge remote-tracking branch 'upstream/main' into struct_type_metadat…
arnaud-lacurie Dec 19, 2025
c63c641
Fix issue and add more tests
arnaud-lacurie Dec 19, 2025
5b48f20
Merge remote-tracking branch 'upstream/main' into struct_type_metadat…
arnaud-lacurie Dec 19, 2025
00e6b4c
remove unused field
arnaud-lacurie Dec 19, 2025
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
Original file line number Diff line number Diff line change
Expand Up @@ -2588,7 +2588,6 @@ private static List<Field> normalizeFields(@Nullable final List<Field> fields) {
// If any field info is missing, the type that is about to be constructed comes from a constructing
// code path. We should be able to just set these field names and indexes as we wish.
//
Set<String> fieldNamesSeen = Sets.newHashSet();
final ImmutableList.Builder<Field> resultFieldsBuilder = ImmutableList.builder();
for (int i = 0; i < fields.size(); i++) {
final var field = fields.get(i);
Expand All @@ -2612,9 +2611,6 @@ private static List<Field> normalizeFields(@Nullable final List<Field> fields) {
Optional.of(fieldStorageName));
}

if (!(fieldNamesSeen.add(fieldToBeAdded.getFieldName()))) {
throw new RecordCoreException("fields contain duplicate field names");
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is there a reason this check on type was removed?

}
resultFieldsBuilder.add(fieldToBeAdded);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ final var record = (Type.Record) type;
case ENUM:
final var asEnum = (Type.Enum) type;
final var enumValues = asEnum.getEnumValues().stream().map(v -> DataType.EnumType.EnumValue.of(v.getName(), v.getNumber())).collect(Collectors.toList());
return DataType.EnumType.from(asEnum.getName() == null ? ProtoUtils.uniqueName("") : asEnum.getName(), enumValues, asEnum.isNullable());
return DataType.EnumType.from(asEnum.getName() == null ? ProtoUtils.uniqueName("id") : asEnum.getName(), enumValues, asEnum.isNullable());
default:
Assert.failUnchecked(String.format(Locale.ROOT, "unexpected type %s", type));
return null; // make compiler happy.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,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.Value;
import com.apple.foundationdb.relational.api.metadata.DataType;
import com.apple.foundationdb.relational.util.Assert;
import com.google.common.base.Verify;
import com.google.common.collect.ImmutableList;
Expand Down Expand Up @@ -207,6 +208,43 @@ public List<Type> underlyingTypes() {
return Streams.stream(underlying()).map(Value::getResultType).collect(ImmutableList.toImmutableList());
}

/**
* Returns a StructType representing the semantic types of all expressions with their field names.
*
* <p>This wraps the individual expression data types into a single StructType that is
* structurally equivalent to the planner's Type.Record output. The StructType includes:
* <ul>
* <li>Field names from the expressions (handles aliases, star expansion, etc.)</li>
* <li>Type structure from semantic analysis (preserves struct type names)</li>
* </ul>
*
* <p>This StructType can be used directly for result set metadata after enriching nested
* struct names from RecordMetaData descriptors.
*
* <p>Note: The StructType name is a generated UUID since this is a general-purpose method.
* Contexts that need a specific name (e.g., "QUERY_RESULT" for top-level queries) should
* wrap or recreate the StructType with an appropriate name.
*
* @return A StructType with field names and semantic types from expressions
*/
@Nonnull
public DataType.StructType getStructType() {
final ImmutableList.Builder<DataType.StructType.Field> fieldsBuilder = ImmutableList.builder();
int index = 0;
for (final Expression expression : underlying) {
// Use expression name if available, otherwise generate a name
final String fieldName = expression.getName()
.map(Identifier::toString)
.orElse("_" + index);
fieldsBuilder.add(DataType.StructType.Field.from(fieldName, expression.getDataType(), index));
index++;
}
// Use UUID-based name since this is a general-purpose method
// Top-level contexts (like query results) will override with "QUERY_RESULT"
final String generatedName = "id" + java.util.UUID.randomUUID().toString().replace("-", "_");
return DataType.StructType.from(generatedName, fieldsBuilder.build(), true);
}

@Nonnull
public Stream<Expression> stream() {
return underlying.stream();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
import com.apple.foundationdb.record.query.plan.cascades.SemanticException;
import com.apple.foundationdb.record.query.plan.cascades.StableSelectorCostModel;
import com.apple.foundationdb.record.query.plan.cascades.typing.TypeRepository;
import com.apple.foundationdb.record.query.plan.cascades.typing.Type;
import com.apple.foundationdb.record.query.plan.plans.RecordQueryPlan;
import com.apple.foundationdb.record.query.plan.serialization.DefaultPlanSerializationRegistry;
import com.apple.foundationdb.record.util.ProtoUtils;
Expand All @@ -42,10 +43,12 @@
import com.apple.foundationdb.relational.api.exceptions.ErrorCode;
import com.apple.foundationdb.relational.api.exceptions.RelationalException;
import com.apple.foundationdb.relational.api.exceptions.UncheckedRelationalException;
import com.apple.foundationdb.relational.api.metadata.DataType;
import com.apple.foundationdb.relational.api.metrics.RelationalMetric;
import com.apple.foundationdb.relational.continuation.CompiledStatement;
import com.apple.foundationdb.relational.continuation.TypedQueryArgument;
import com.apple.foundationdb.relational.recordlayer.ContinuationImpl;
import com.apple.foundationdb.relational.recordlayer.metadata.DataTypeUtils;
import com.apple.foundationdb.relational.recordlayer.metadata.RecordLayerSchemaTemplate;
import com.apple.foundationdb.relational.recordlayer.query.cache.PhysicalPlanEquivalence;
import com.apple.foundationdb.relational.recordlayer.query.cache.RelationalPlanCache;
Expand All @@ -54,6 +57,7 @@
import com.apple.foundationdb.relational.util.Assert;
import com.apple.foundationdb.relational.util.RelationalLoggingUtil;
import com.google.common.base.VerifyException;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.Maps;
import com.google.protobuf.InvalidProtocolBufferException;
import org.apache.logging.log4j.LogManager;
Expand All @@ -62,6 +66,7 @@
import javax.annotation.Nonnull;
import java.sql.SQLException;
import java.util.Arrays;
import java.util.List;
import java.util.Objects;
import java.util.Optional;
import java.util.Set;
Expand Down Expand Up @@ -326,12 +331,35 @@
planGenerationContext.setContinuation(continuationProto);
final var continuationPlanConstraint =
QueryPlanConstraint.fromProto(serializationContext, compiledStatement.getPlanConstraint());

final DataType.StructType semanticStructType;
if (compiledStatement.hasQueryMetadata()) {
semanticStructType = Assert.castUnchecked(DataTypeUtils.toRelationalType(Type.fromTypeProto(serializationContext, compiledStatement.getQueryMetadata())),
DataType.StructType.class);
} else {
final Type resultType = recordQueryPlan.getResultType().getInnerType();
if (resultType instanceof Type.Record) {
final Type.Record recordType = (Type.Record)resultType;
final List<DataType.StructType.Field> fields = recordType.getFields().stream()
.map(field -> DataType.StructType.Field.from(
field.getFieldName(),
DataTypeUtils.toRelationalType(field.getFieldType()),
field.getFieldIndex()))
.collect(java.util.stream.Collectors.toList());
semanticStructType = DataType.StructType.from("QUERY_RESULT", fields, true);
} else {
// Fallback for non-record types (shouldn't happen for SELECT results)
semanticStructType = DataType.StructType.from("QUERY_RESULT", ImmutableList.of(), true);

Check warning on line 352 in fdb-relational-core/src/main/java/com/apple/foundationdb/relational/recordlayer/query/PlanGenerator.java

View check run for this annotation

fdb.teamscale.io / Teamscale | Findings

fdb-relational-core/src/main/java/com/apple/foundationdb/relational/recordlayer/query/PlanGenerator.java#L352

Use `java.util.List.of` instead https://fdb.teamscale.io/findings/details/foundationdb-fdb-record-layer?t=FORK_MR%2F3753%2Farnaud-lacurie%2Fstruct_type_metadata_fix%3AHEAD&id=F67F589FF9D89DA0F30325FCB33F3553
}
}

return new QueryPlan.ContinuedPhysicalQueryPlan(recordQueryPlan, typeRepository,
continuationPlanConstraint,
planGenerationContext,
"EXECUTE CONTINUATION " + ast.getQueryCacheKey().getCanonicalQueryString(),
currentPlanHashMode,
serializedPlanHashMode);
serializedPlanHashMode,
semanticStructType);
}

private void resetTimer() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -128,14 +128,22 @@
@Nonnull
private final QueryExecutionContext queryExecutionContext;

/**
* Semantic type structure captured during semantic analysis.
* Complete StructType with field names and nested struct type names preserved.
*/
@Nonnull
private final DataType.StructType semanticStructType;

public PhysicalQueryPlan(@Nonnull final RecordQueryPlan recordQueryPlan,
@Nullable final StatsMaps plannerStatsMaps,
@Nonnull final TypeRepository typeRepository,
@Nonnull final QueryPlanConstraint constraint,
@Nonnull final QueryPlanConstraint continuationConstraint,
@Nonnull final QueryExecutionContext queryExecutionContext,
@Nonnull final String query,
@Nonnull final PlanHashMode currentPlanHashMode) {
@Nonnull final PlanHashMode currentPlanHashMode,
@Nonnull final DataType.StructType semanticStructType) {
super(query);
this.recordQueryPlan = recordQueryPlan;
this.plannerStatsMaps = plannerStatsMaps;
Expand All @@ -145,6 +153,7 @@
this.queryExecutionContext = queryExecutionContext;
this.currentPlanHashMode = currentPlanHashMode;
this.planHashSupplier = Suppliers.memoize(() -> recordQueryPlan.planHash(currentPlanHashMode));
this.semanticStructType = semanticStructType;
}

@Nonnull
Expand Down Expand Up @@ -192,7 +201,8 @@
return this;
}
return new PhysicalQueryPlan(recordQueryPlan, plannerStatsMaps, typeRepository, constraint,
continuationConstraint, queryExecutionContext, query, queryExecutionContext.getPlanHashMode());
continuationConstraint, queryExecutionContext, query, queryExecutionContext.getPlanHashMode(),
semanticStructType);
}

@Nonnull
Expand Down Expand Up @@ -404,10 +414,10 @@
parsedContinuation.getExecutionState(),
executeProperties));
final var currentPlanHashMode = OptionsUtils.getCurrentPlanHashMode(options);
final var dataType = (DataType.StructType) DataTypeUtils.toRelationalType(type);

return executionContext.metricCollector.clock(RelationalMetric.RelationalEvent.CREATE_RESULT_SET_ITERATOR, () -> {
final ResumableIterator<Row> iterator = RecordLayerIterator.create(cursor, messageFDBQueriedRecord -> new MessageTuple(messageFDBQueriedRecord.getMessage()));
return new RecordLayerResultSet(RelationalStructMetaData.of(dataType), iterator, connection,
return new RecordLayerResultSet(RelationalStructMetaData.of(semanticStructType), iterator, connection,
(continuation, reason) -> enrichContinuation(continuation,
currentPlanHashMode, reason));
});
Expand Down Expand Up @@ -450,8 +460,8 @@
i++;
}

compiledStatementBuilder.setPlanConstraint(getContinuationConstraint().toProto(serializationContext));

compiledStatementBuilder.setPlanConstraint(getContinuationConstraint().toProto(serializationContext))
.setQueryMetadata(DataTypeUtils.toRecordLayerType(semanticStructType).toTypeProto(serializationContext));
continuationBuilder.withCompiledStatement(compiledStatementBuilder.build());
}
return continuationBuilder.build();
Expand All @@ -470,15 +480,16 @@
@Nonnull
private final Supplier<Integer> serializedPlanHashSupplier;

public ContinuedPhysicalQueryPlan(@Nonnull final RecordQueryPlan recordQueryPlan,

Check warning on line 483 in fdb-relational-core/src/main/java/com/apple/foundationdb/relational/recordlayer/query/QueryPlan.java

View check run for this annotation

fdb.teamscale.io / Teamscale | Findings

fdb-relational-core/src/main/java/com/apple/foundationdb/relational/recordlayer/query/QueryPlan.java#L483

Method `ContinuedPhysicalQueryPlan` has 8 parameters but no more than 7 parameters are allowed https://fdb.teamscale.io/findings/details/foundationdb-fdb-record-layer?t=FORK_MR%2F3753%2Farnaud-lacurie%2Fstruct_type_metadata_fix%3AHEAD&id=83DD14390C33E4BA1F8C0CFB51EF6ED7
@Nonnull final TypeRepository typeRepository,
@Nonnull final QueryPlanConstraint continuationConstraint,
@Nonnull final QueryExecutionContext queryExecutionParameters,
@Nonnull final String query,
@Nonnull final PlanHashMode currentPlanHashMode,
@Nonnull final PlanHashMode serializedPlanHashMode) {
@Nonnull final PlanHashMode serializedPlanHashMode,
@Nonnull final DataType.StructType semanticStructType) {
super(recordQueryPlan, null, typeRepository, QueryPlanConstraint.noConstraint(),
continuationConstraint, queryExecutionParameters, query, currentPlanHashMode);
continuationConstraint, queryExecutionParameters, query, currentPlanHashMode, semanticStructType);
this.serializedPlanHashMode = serializedPlanHashMode;
this.serializedPlanHashSupplier = Suppliers.memoize(() -> recordQueryPlan.planHash(serializedPlanHashMode));
}
Expand All @@ -488,15 +499,29 @@
return serializedPlanHashMode;
}

@SuppressWarnings("PMD.CompareObjectsWithEquals")
/**
* Returns a plan with updated execution context.
*
* <p>Note: This method is never called in production because ContinuedPhysicalQueryPlan instances
* are not cached - each EXECUTE CONTINUATION deserializes the plan fresh from the continuation blob.
* However, we must override to satisfy the Plan interface contract.
*
* <p>TODO: Refactor the class hierarchy to eliminate this dead code. Potential approaches:
* <ul>
* <li>Collapse ContinuedPhysicalQueryPlan into PhysicalQueryPlan with a flag</li>
* <li>Use composition instead of inheritance</li>
* <li>Make Plan.withExecutionContext() optional with default implementation</li>
* </ul>
*
* @param queryExecutionContext The new execution context (ignored - never called)
* @return This instance (since continuation plans are never cached)
*/

Check warning on line 518 in fdb-relational-core/src/main/java/com/apple/foundationdb/relational/recordlayer/query/QueryPlan.java

View check run for this annotation

fdb.teamscale.io / Teamscale | Findings

fdb-relational-core/src/main/java/com/apple/foundationdb/relational/recordlayer/query/QueryPlan.java#L509-L518

TODO: Refactor the class hierarchy to eliminate this dead code. Potential approaches: https://fdb.teamscale.io/findings/details/foundationdb-fdb-record-layer?t=FORK_MR%2F3753%2Farnaud-lacurie%2Fstruct_type_metadata_fix%3AHEAD&id=A6376AE1BDD8DB9635F8755F5A4BF18B
@Override
@Nonnull
public PhysicalQueryPlan withExecutionContext(@Nonnull final QueryExecutionContext queryExecutionContext) {
if (queryExecutionContext == this.getQueryExecutionContext()) {
return this;
}
return new ContinuedPhysicalQueryPlan(getRecordQueryPlan(), getTypeRepository(), getContinuationConstraint(),
queryExecutionContext, query, queryExecutionContext.getPlanHashMode(), getSerializedPlanHashMode());
// This method is never called in production - continuation plans bypass the cache.
// Return this to avoid maintaining dead code.
return this;
}

@Override
Expand Down Expand Up @@ -549,18 +574,27 @@
@Nonnull
private final String query;

/**
* Semantic type structure captured during semantic analysis.
* Preserves struct type names - will be merged with planner field names after planning.
*/
@Nonnull
private final DataType.StructType semanticStructType;

@SuppressWarnings("OptionalUsedAsFieldOrParameterType")
@Nonnull
private Optional<PhysicalQueryPlan> optimizedPlan;

private LogicalQueryPlan(@Nonnull final RelationalExpression relationalExpression,
@Nonnull final MutablePlanGenerationContext context,
@Nonnull final String query) {
@Nonnull final String query,
@Nonnull final DataType.StructType semanticStructType) {
super(query);
this.relationalExpression = relationalExpression;
this.context = context;
this.optimizedPlan = Optional.empty();
this.query = query;
this.semanticStructType = semanticStructType;
}

@Override
Expand Down Expand Up @@ -609,7 +643,8 @@

optimizedPlan = Optional.of(
new PhysicalQueryPlan(minimizedPlan, statsMaps, builder.build(),
constraint, continuationConstraint, context, query, currentPlanHashMode));
constraint, continuationConstraint, context, query, currentPlanHashMode,
semanticStructType));
return optimizedPlan.get();
});
}
Expand Down Expand Up @@ -657,8 +692,9 @@
@Nonnull
public static LogicalQueryPlan of(@Nonnull final RelationalExpression relationalExpression,
@Nonnull final MutablePlanGenerationContext context,
@Nonnull final String query) {
return new LogicalQueryPlan(relationalExpression, context, query);
@Nonnull final String query,
@Nonnull final DataType.StructType semanticStructType) {
return new LogicalQueryPlan(relationalExpression, context, query, semanticStructType);
}

@Nonnull
Expand Down
Loading
Loading