Skip to content

Commit

Permalink
* factor out Field usage and only rely on FieldPath.
Browse files Browse the repository at this point in the history
  • Loading branch information
@hatyo
hatyo committed Nov 7, 2022
1 parent 3ed2994 commit 849b37a
Show file tree
Hide file tree
Showing 32 changed files with 357 additions and 248 deletions.
2 changes: 1 addition & 1 deletion README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ The Record Layer is a Java API providing a record-oriented store on top of Found
across one or more record types, and a query planner capable of
automatic selection of indexes.
* **Many record stores, shared schema** - The Record Layer provides the
the ability to support many discrete record store instances, all with
ability to support many discrete record store instances, all with
a shared (and evolving) schema. For example, rather than modeling a
single database in which to store all users' data, each user can be
given their own record store, perhaps sharded across different FDB
Expand Down
7 changes: 4 additions & 3 deletions ...ava/com/apple/foundationdb/record/query/plan/cascades/AggregateIndexExpansionVisitor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ private Quantifier constructSelectWhere(@Nonnull final Quantifier.ForEach baseQu
// add an RCV column representing the grouping columns as the first result set column
final var groupingValue = RecordConstructorValue.ofColumns(groupingValues
.stream()
.map(v -> Column.of(((FieldValue)v).getLastField(), v))
.map(Column::unnamedOf) // REMOVE: name is important?
.collect(Collectors.toList()));

// flow all underlying quantifiers in their own QOV columns.
Expand All @@ -184,6 +184,7 @@ private Quantifier constructSelectWhere(@Nonnull final Quantifier.ForEach baseQu
return Quantifier.forEach(GroupExpressionRef.of(GraphExpansion.ofOthers(allExpansionsBuilder.build()).buildSelect()));
}

@SuppressWarnings("deprecation")
@Nonnull
private Quantifier constructGroupBy(@Nonnull final CorrelationIdentifier baseQuantifierCorrelationIdentifier,
@Nonnull final List<? extends Value> groupedValue,
Expand Down Expand Up @@ -229,12 +230,12 @@ private Pair<SelectExpression, List<CorrelationIdentifier>> constructSelectHavin
final var placeholder = v.asPlaceholder(CorrelationIdentifier.uniqueID(ValueComparisonRangePredicate.Placeholder.class));
placeholderAliases.add(placeholder.getAlias());
selectHavingGraphExpansionBuilder
.addResultColumn(Column.of(field.getLastField(), field))
.addResultColumn(Column.unnamedOf(field))
.addPlaceholder(placeholder)
.addPredicate(placeholder);
});
}
selectHavingGraphExpansionBuilder.addResultColumn(Column.of(aggregateValueReference.getLastField(), aggregateValueReference)); // TODO should we also add the aggregate reference as a placeholder?
selectHavingGraphExpansionBuilder.addResultColumn(Column.unnamedOf(aggregateValueReference)); // TODO should we also add the aggregate reference as a placeholder? // REMOVE: name is important?
return Pair.of(selectHavingGraphExpansionBuilder.build().buildSelect(), placeholderAliases.build());
}

Expand Down
13 changes: 7 additions & 6 deletions .../java/com/apple/foundationdb/record/query/plan/cascades/AggregateIndexMatchCandidate.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -353,17 +353,18 @@ private static Type reset(@Nonnull final Type type) {
private static void addCoveringField(@Nonnull IndexKeyValueToPartialRecord.Builder builder,
@Nonnull FieldValue fieldValue,
@Nonnull AvailableFields.FieldData fieldData) {
// TODO field names are for debugging purposes only, we should probably use field ordinals here instead.
final var simplifiedFieldValue = (FieldValue)fieldValue.simplify(AliasMap.emptyMap(), ImmutableSet.of());
for (final Type.Record.Field field : simplifiedFieldValue.getFieldPrefix()) {
Verify.verify(field.getFieldNameOptional().isPresent());
builder = builder.getFieldBuilder(field.getFieldName());
for (final var maybeFieldName : simplifiedFieldValue.getFieldPrefix().getFieldNames()) {
Verify.verify(maybeFieldName.isPresent());
builder = builder.getFieldBuilder(maybeFieldName.get());
}

// TODO not sure what to do with the null standing requirement

final Type.Record.Field field = simplifiedFieldValue.getLastField();
Verify.verify(field.getFieldNameOptional().isPresent());
final String fieldName = field.getFieldName();
final var maybeFieldName = simplifiedFieldValue.getLastFieldName();
Verify.verify(maybeFieldName.isPresent());
final String fieldName = maybeFieldName.get();
if (!builder.hasField(fieldName)) {
builder.addField(fieldName, fieldData.getSource(), fieldData.getCopyIfPredicate(), fieldData.getOrdinalPath());
}
Expand Down
14 changes: 7 additions & 7 deletions .../java/com/apple/foundationdb/record/query/plan/cascades/ValueIndexScanMatchCandidate.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -332,20 +332,20 @@ private static ScanComparisons toScanComparisons(@Nonnull final List<ComparisonR
private static boolean addCoveringField(@Nonnull IndexKeyValueToPartialRecord.Builder builder,
@Nonnull FieldValue fieldValue,
@Nonnull AvailableFields.FieldData fieldData) {
for (final Type.Record.Field field : fieldValue.getFieldPrefix()) {
if (field.getFieldNameOptional().isEmpty()) {
// TODO field names are for debugging purposes only, we should probably use field ordinals here instead.
for (final var maybeFieldName : fieldValue.getFieldPrefix().getFieldPrefix().getFieldNames()) {
if (maybeFieldName.isEmpty()) {
return false;
}
builder = builder.getFieldBuilder(field.getFieldName());
builder = builder.getFieldBuilder(maybeFieldName.get());
}

// TODO not sure what to do with the null standing requirement

final Type.Record.Field field = fieldValue.getLastField();
if (field.getFieldNameOptional().isEmpty()) {
final var maybeFieldName = fieldValue.getLastFieldName();
if (maybeFieldName.isEmpty()) {
return false;
}
final String fieldName = field.getFieldName();
final String fieldName = maybeFieldName.get();
if (!builder.hasField(fieldName)) {
builder.addField(fieldName, fieldData.getSource(), fieldData.getCopyIfPredicate(), fieldData.getOrdinalPath());
}
Expand Down
13 changes: 7 additions & 6 deletions ...va/com/apple/foundationdb/record/query/plan/cascades/WindowedIndexScanMatchCandidate.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -456,20 +456,21 @@ private static ScanComparisons toScanComparisons(@Nonnull final List<ComparisonR
private static boolean addCoveringField(@Nonnull IndexKeyValueToPartialRecord.Builder builder,
@Nonnull FieldValue fieldValue,
@Nonnull AvailableFields.FieldData fieldData) {
for (final Type.Record.Field field : fieldValue.getFieldPrefix()) {
if (field.getFieldNameOptional().isEmpty()) {
// TODO field names are for debugging purposes only, we should probably use field ordinals here instead.
for (final var maybeFieldName : fieldValue.getFieldPrefix().getFieldNames()) {
if (maybeFieldName.isEmpty()) {
return false;
}
builder = builder.getFieldBuilder(field.getFieldName());
builder = builder.getFieldBuilder(maybeFieldName.get());
}

// TODO not sure what to do with the null standing requirement

final Type.Record.Field field = fieldValue.getLastField();
if (field.getFieldNameOptional().isEmpty()) {
final var maybeFieldName = fieldValue.getLastFieldName();
if (maybeFieldName.isEmpty()) {
return false;
}
final String fieldName = field.getFieldName();
final String fieldName = maybeFieldName.get();
if (!builder.hasField(fieldName)) {
builder.addField(fieldName, fieldData.getSource(), t -> true, fieldData.getOrdinalPath());
}
Expand Down
28 changes: 22 additions & 6 deletions ...a/com/apple/foundationdb/record/query/plan/cascades/matching/structure/ValueMatchers.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,11 +54,13 @@ public static BindingMatcher<Value> anyValue() {
return typed(Value.class);
}

@Deprecated // should access fields by ordinals
@Nonnull
public static BindingMatcher<FieldValue> fieldValueWithFieldNames(@Nonnull final String fieldPathAsString) {
return fieldValueWithFieldNames(anyValue(), fieldPathAsString);
}

@Deprecated // should access fields by ordinals
@Nonnull
public static <V extends Value> BindingMatcher<FieldValue> fieldValueWithFieldNames(@Nonnull final BindingMatcher<V> downstreamValue,
@Nonnull final String fieldPathAsString) {
Expand All @@ -69,6 +71,7 @@ public static <V extends Value> BindingMatcher<FieldValue> fieldValueWithFieldNa
return fieldValueWithFieldNames(downstreamValue, exactly(fieldPathMatchers));
}

@Deprecated // should access fields by ordinals
@Nonnull
public static <V extends Value> BindingMatcher<FieldValue> fieldValueWithFieldNames(@Nonnull final BindingMatcher<V> downstreamValue,
@Nonnull final CollectionMatcher<String> downstreamFieldPath) {
Expand All @@ -86,21 +89,27 @@ public static <V extends Value> BindingMatcher<FieldValue> fieldValueWithFieldNa
matchingAllOf(FieldValue.class, ImmutableList.of(downstreamValueMatcher, downstreamFieldPathMatcher)));
}

@SuppressWarnings("UnstableApiUsage")
@Nonnull
public static <V extends Value> BindingMatcher<FieldValue> fieldValueWithFieldPath(@Nonnull final BindingMatcher<V> downstreamValue,
@Nonnull final CollectionMatcher<Type.Record.Field> downstreamFieldPath) {
@Nonnull final CollectionMatcher<Integer> downstreamFieldPathOrdinals,
@Nonnull final CollectionMatcher<Type> downstreamFieldPathTypes) {
final TypedMatcherWithExtractAndDownstream<FieldValue> downstreamValueMatcher =
typedWithDownstream(FieldValue.class,
Extractor.of(FieldValue::getChild, name -> "child(" + name + ")"),
downstreamValue);
final TypedMatcherWithExtractAndDownstream<FieldValue> downstreamFieldPathMatcher =
final TypedMatcherWithExtractAndDownstream<FieldValue> downstreamFieldPathOrdinalsMatcher =
typedWithDownstream(FieldValue.class,
Extractor.of(FieldValue::getFields, name -> "fieldPath(" + name + ")"),
downstreamFieldPath);
Extractor.of(f -> f.getFieldPathOrdinals().asList(), name -> "fieldPathOrdinals(" + name + ")"),
downstreamFieldPathOrdinals);
final TypedMatcherWithExtractAndDownstream<FieldValue> downstreamFieldPathTypesMatcher =
typedWithDownstream(FieldValue.class,
Extractor.of(FieldValue::getFieldPathTypes, name -> "fieldPathTypes(" + name + ")"),
downstreamFieldPathTypes);

return typedWithDownstream(FieldValue.class,
Extractor.identity(),
matchingAllOf(FieldValue.class, ImmutableList.of(downstreamValueMatcher, downstreamFieldPathMatcher)));
matchingAllOf(FieldValue.class, ImmutableList.of(downstreamValueMatcher, downstreamFieldPathOrdinalsMatcher, downstreamFieldPathTypesMatcher)));
}

public static <V extends Value> BindingMatcher<NumericAggregationValue.Sum> sumAggregationValue() {
Expand Down Expand Up @@ -144,6 +153,13 @@ public static BindingMatcher<QuantifiedObjectValue> currentObjectValue() {

@Nonnull
public static BindingMatcher<StreamableAggregateValue> streamableAggregateValue() {
return typed(StreamableAggregateValue.class);
return streamableAggregateValue(exactly(ImmutableList.of(anyValue())));
}

@Nonnull
public static BindingMatcher<StreamableAggregateValue> streamableAggregateValue(@Nonnull final CollectionMatcher<? extends Value> downstreamValues) {
return typedWithDownstream(StreamableAggregateValue.class,
Extractor.of(StreamableAggregateValue::getChildren, name -> "children(" + name + ")"),
downstreamValues);
}
}
2 changes: 1 addition & 1 deletion ...com/apple/foundationdb/record/query/plan/cascades/properties/DistinctRecordsProperty.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ public Boolean visitInComparandJoinPlan(@Nonnull final RecordQueryInComparandJoi
@Nonnull
@Override
public Boolean visitAggregateIndexPlan(@Nonnull final RecordQueryAggregateIndexPlan aggregateIndexPlan) {
return visitIndexPlan(aggregateIndexPlan.getIndexPlan());
return true;
}

@Nonnull
Expand Down
6 changes: 4 additions & 2 deletions ...n/java/com/apple/foundationdb/record/query/plan/cascades/properties/OrderingProperty.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@
import javax.annotation.Nonnull;
import javax.annotation.Nullable;
import java.util.List;
import java.util.Optional;
import java.util.function.BinaryOperator;
import java.util.stream.Stream;

Expand Down Expand Up @@ -110,6 +111,7 @@ public String toString() {
@API(API.Status.EXPERIMENTAL)
@SuppressWarnings("java:S3776")
public static class OrderingVisitor implements RecordQueryPlanVisitor<Ordering> {
@SuppressWarnings("deprecation")
@Nonnull
@Override
public Ordering visitPredicatesFilterPlan(@Nonnull final RecordQueryPredicatesFilterPlan predicatesFilterPlan) {
Expand All @@ -132,9 +134,9 @@ public Ordering visitPredicatesFilterPlan(@Nonnull final RecordQueryPredicatesFi
}

final var fieldValue = (FieldValue)valuePredicate.getValue();
if (fieldValue.getFields()
if (fieldValue.getFieldPathNamesMaybe()
.stream()
.anyMatch(field -> field.getFieldNameOptional().isEmpty())) {
.anyMatch(Optional::isEmpty)) {
return Stream.of();
}

Expand Down
2 changes: 1 addition & 1 deletion ...re/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/CountValue.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@
* A counting aggregate value.
*/
@API(API.Status.EXPERIMENTAL)
public class CountValue implements AggregateValue, StreamableAggregateValue, IndexableAggregateValue {
public class CountValue implements Value, AggregateValue, StreamableAggregateValue, IndexableAggregateValue {
private static final ObjectPlanHash BASE_HASH = new ObjectPlanHash("Count-Value");

@Nonnull
Expand Down

0 comments on commit 849b37a

Please sign in to comment.