Skip to content

Commit

Permalink
* refactoring.
Browse files Browse the repository at this point in the history
  • Loading branch information
@hatyo
hatyo committed Oct 17, 2022
1 parent 5a6d051 commit ca24264
Show file tree
Hide file tree
Showing 12 changed files with 75 additions and 57 deletions.
8 changes: 4 additions & 4 deletions ...rd-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/AvailableFields.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -76,12 +76,12 @@ public boolean containsAll(@Nonnull Collection<KeyExpression> requiredFields) {
}

@Nullable
public IndexKeyValueToPartialRecord.Builder buildIndexKeyValueToPartialRecord(@Nonnull RecordType recordType) {
public IndexKeyValueToPartialRecord.Builder<String> buildIndexKeyValueToPartialRecord(@Nonnull RecordType recordType) {
if (fields == null) {
return null;
}

final IndexKeyValueToPartialRecord.Builder builder = IndexKeyValueToPartialRecord.newBuilder(recordType);
final IndexKeyValueToPartialRecord.Builder<String> builder = IndexKeyValueToPartialRecord.newBuilder(recordType);
for (Map.Entry<KeyExpression, FieldData> entry : fields.entrySet()) {
if (!addCoveringField(entry.getKey(), entry.getValue(), builder)) {
return null;
Expand Down Expand Up @@ -143,7 +143,7 @@ public static AvailableFields fromIndex(@Nonnull RecordType recordType,
keyFields.addAll(primaryKeys);

Map<KeyExpression, FieldData> fields = new HashMap<>();
final IndexKeyValueToPartialRecord.Builder builder = IndexKeyValueToPartialRecord.newBuilder(recordType);
final IndexKeyValueToPartialRecord.Builder<String> builder = IndexKeyValueToPartialRecord.newBuilder(recordType);
for (int i = 0; i < keyFields.size(); i++) {
KeyExpression keyField = keyFields.get(i);
FieldData fieldData = FieldData.of(IndexKeyValueToPartialRecord.TupleSource.KEY, i);
Expand Down Expand Up @@ -172,7 +172,7 @@ public static AvailableFields fromIndex(@Nonnull RecordType recordType,

public static boolean addCoveringField(@Nonnull KeyExpression requiredExpr,
@Nonnull FieldData fieldData,
@Nonnull IndexKeyValueToPartialRecord.Builder builder) {
@Nonnull IndexKeyValueToPartialRecord.Builder<String> builder) {
if (fieldData.source == IndexKeyValueToPartialRecord.TupleSource.OTHER) {
return true;
}
Expand Down
14 changes: 8 additions & 6 deletions .../src/main/java/com/apple/foundationdb/record/query/plan/IndexKeyValueToPartialRecord.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,7 @@ public int hashCode() {
*/
@Deprecated
public static Builder<String> newBuilder(@Nonnull RecordType recordType) {
return new Builder<>(recordType, FieldAccessor.byName());
return newBuilderUsingFieldName(recordType);
}

public static Builder<String> newBuilderUsingFieldName(@Nonnull RecordType recordType) {
Expand All @@ -246,6 +246,7 @@ public static Builder<Integer> newBuilderUsingFieldIndex(@Nonnull RecordType rec

/**
* A builder for {@link IndexKeyValueToPartialRecord}.
* @param <T> The type of the field-accessing identifier, usually either a {@code String} or {@code Integer}.
*/
public static class Builder<T extends Comparable<T>> {
@Nonnull
Expand Down Expand Up @@ -276,7 +277,7 @@ public boolean hasField(@Nonnull T field) {
return fields.containsKey(field) || nestedBuilders.containsKey(field);
}

public Builder addField(@Nonnull T field, @Nonnull TupleSource source, int index) {
public Builder<T> addField(@Nonnull T field, @Nonnull TupleSource source, int index) {
final Descriptors.FieldDescriptor fieldDescriptor = fieldAccessor.getField(field, recordDescriptor);
if (fieldDescriptor == null) {
throw new MetaDataException("field not found: " + field);
Expand Down Expand Up @@ -381,7 +382,7 @@ public boolean equals(final Object o) {
return false;
}
final var that = (BoundFieldAccessorImpl<?>)o;
return modifier == that.modifier;
return modifier.equals(that.modifier);
}

@Override
Expand Down Expand Up @@ -410,11 +411,12 @@ public Descriptors.FieldDescriptor getField(@Nonnull final Descriptors.Descripto
}

}
Descriptors.FieldDescriptor getField(@Nonnull final T modifier, @Nonnull Descriptors.Descriptor record);

T getFieldIdentifier(@Nonnull final Descriptors.FieldDescriptor fieldDescriptor);
Descriptors.FieldDescriptor getField(@Nonnull T modifier, @Nonnull Descriptors.Descriptor record);

T getFieldIdentifier(@Nonnull Descriptors.FieldDescriptor fieldDescriptor);

Bound bind(@Nonnull final T modifier);
Bound bind(@Nonnull T modifier);

class ByIndex implements FieldAccessor<Integer> {
@Override
Expand Down
3 changes: 3 additions & 0 deletions ...rd-layer-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/Column.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,9 @@
* @param <V> type parameter for the {@link Value}
*/
public class Column<V extends Value> {

// (yhatem) it is unclear whether we should this be replaced by {@code FieldDelegate}.
// it seems a {@code Field} is used as a named _alias_ of the {@code Column}'s underlying child.
@Nonnull
private final Field field;
@Nonnull
Expand Down
23 changes: 13 additions & 10 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 @@ -240,7 +240,7 @@ private Optional<RelationalExpression> tryFetchCoveringIndexScan(@Nonnull final
}

final RecordType recordType = Iterables.getOnlyElement(queriedRecordTypes);
final IndexKeyValueToPartialRecord.Builder builder = IndexKeyValueToPartialRecord.newBuilder(recordType);
final IndexKeyValueToPartialRecord.Builder<Integer> builder = IndexKeyValueToPartialRecord.newBuilderUsingFieldIndex(recordType);
final Value baseObjectValue = QuantifiedObjectValue.of(baseAlias, baseType);
for (int i = 0; i < indexKeyValues.size(); i++) {
final Value keyValue = indexKeyValues.get(i);
Expand Down Expand Up @@ -321,25 +321,28 @@ private static ScanComparisons toScanComparisons(@Nonnull final List<ComparisonR
return builder.build();
}

private static boolean addCoveringField(@Nonnull IndexKeyValueToPartialRecord.Builder builder,
private static boolean addCoveringField(@Nonnull IndexKeyValueToPartialRecord.Builder<Integer> builder,
@Nonnull FieldValue fieldValue,
@Nonnull AvailableFields.FieldData fieldData) {
for (final Type.Record.Field field : fieldValue.getFieldPrefix()) {
if (field.getFieldNameOptional().isEmpty()) {

// TODO this method is duplicated in `WindowedIndexScanMatchCandidate` and should be unified.

for (final FieldValue.FieldDelegate fieldDelegate : fieldValue.getFieldPrefix()) {
if (fieldDelegate.getFieldIndexOptional().isEmpty()) {
return false;
}
builder = builder.getFieldBuilder(field.getFieldName());
builder = builder.getFieldBuilder(fieldDelegate.getFieldIndex());
}

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

final Type.Record.Field field = fieldValue.getLastField();
if (field.getFieldNameOptional().isEmpty()) {
final FieldValue.FieldDelegate lastFieldDelegate = fieldValue.getLastField();
if (lastFieldDelegate.getFieldIndexOptional().isEmpty()) {
return false;
}
final String fieldName = field.getFieldName();
if (!builder.hasField(fieldName)) {
builder.addField(fieldName, fieldData.getSource(), fieldData.getIndex());
final var fieldIndex = lastFieldDelegate.getFieldIndex();
if (!builder.hasField(fieldIndex)) {
builder.addField(fieldIndex, fieldData.getSource(), fieldData.getIndex());
}
return true;
}
Expand Down
23 changes: 13 additions & 10 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 @@ -376,7 +376,7 @@ private Optional<RelationalExpression> tryFetchCoveringIndexScan(@Nonnull final
}

final RecordType recordType = Iterables.getOnlyElement(queriedRecordTypes);
final IndexKeyValueToPartialRecord.Builder builder = IndexKeyValueToPartialRecord.newBuilder(recordType);
final IndexKeyValueToPartialRecord.Builder<Integer> builder = IndexKeyValueToPartialRecord.newBuilderUsingFieldIndex(recordType);
final Value baseObjectValue = QuantifiedObjectValue.of(baseAlias, baseRecordType);
for (int i = 0; i < indexKeyValues.size(); i++) {
final Value keyValue = indexKeyValues.get(i);
Expand Down Expand Up @@ -445,25 +445,28 @@ private static ScanComparisons toScanComparisons(@Nonnull final List<ComparisonR
return builder.build();
}

private static boolean addCoveringField(@Nonnull IndexKeyValueToPartialRecord.Builder builder,
private static boolean addCoveringField(@Nonnull IndexKeyValueToPartialRecord.Builder<Integer> builder,
@Nonnull FieldValue fieldValue,
@Nonnull AvailableFields.FieldData fieldData) {
for (final Type.Record.Field field : fieldValue.getFieldPrefix()) {
if (field.getFieldNameOptional().isEmpty()) {

// TODO this method is duplicated in `ValueIndexScanMatchCandidate` and should be unified.

for (final FieldValue.FieldDelegate fieldDelegate : fieldValue.getFieldPrefix()) {
if (fieldDelegate.getFieldIndexOptional().isEmpty()) {
return false;
}
builder = builder.getFieldBuilder(field.getFieldName());
builder = builder.getFieldBuilder(fieldDelegate.getFieldIndex());
}

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

final Type.Record.Field field = fieldValue.getLastField();
if (field.getFieldNameOptional().isEmpty()) {
final FieldValue.FieldDelegate lastFieldDelegate = fieldValue.getLastField();
if (lastFieldDelegate.getFieldIndexOptional().isEmpty()) {
return false;
}
final String fieldName = field.getFieldName();
if (!builder.hasField(fieldName)) {
builder.addField(fieldName, fieldData.getSource(), fieldData.getIndex());
final var fieldIndex = lastFieldDelegate.getFieldIndex();
if (!builder.hasField(fieldIndex)) {
builder.addField(fieldIndex, fieldData.getSource(), fieldData.getIndex());
}
return true;
}
Expand Down
2 changes: 1 addition & 1 deletion ...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 @@ -132,7 +132,7 @@ public Ordering visitPredicatesFilterPlan(@Nonnull final RecordQueryPredicatesFi
final var fieldValue = (FieldValue)valuePredicate.getValue();
if (fieldValue.getFields()
.stream()
.anyMatch(field -> field.getFieldNameOptional().isEmpty())) {
.anyMatch(field -> field.getFieldIndexOptional().isEmpty())) {
return Stream.of();
}

Expand Down
15 changes: 10 additions & 5 deletions ...re/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/FieldValue.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -221,13 +221,13 @@ public static FieldValue ofFieldDelegates(@Nonnull Value childValue, @Nonnull fi
return new FieldValue(childValue, new FieldPath(fieldDelegates));
}

public static FieldValue ofFieldsAndFuseIfPossible(@Nonnull Value childValue, @Nonnull final List<Field> fields) {
public static FieldValue ofFieldsAndFuseIfPossible(@Nonnull Value childValue, @Nonnull final List<FieldDelegate> fields) {
if (childValue instanceof FieldValue) {
final var childFieldValue = (FieldValue)childValue;
return FieldValue.ofFieldDelegates(childFieldValue.getChild(),
ImmutableList.<FieldDelegate>builder().addAll(childFieldValue.getFields()).addAll(fields.stream().map(FieldDelegate::of).collect(Collectors.toList())).build());
ImmutableList.<FieldDelegate>builder().addAll(childFieldValue.getFields()).addAll(fields).build());
}
return FieldValue.ofFields(childValue, fields);
return FieldValue.ofFieldDelegates(childValue, fields);
}

@Nonnull
Expand All @@ -236,8 +236,8 @@ public static FieldValue ofOrdinalNumber(@Nonnull Value childValue, final int or
}

@Nonnull
public static Optional<List<Field>> stripFieldPrefixMaybe(@Nonnull List<Field> fieldPath,
@Nonnull List<Field> potentialPrefixPath) {
public static Optional<List<FieldDelegate>> stripFieldPrefixMaybe(@Nonnull List<FieldDelegate> fieldPath,
@Nonnull List<FieldDelegate> potentialPrefixPath) {
if (fieldPath.size() < potentialPrefixPath.size()) {
return Optional.empty();
}
Expand Down Expand Up @@ -373,10 +373,15 @@ private FieldDelegate(@Nonnull final Field field) {
this.field = field;
}

@Nonnull
public Optional<Integer> getFieldIndexOptional() {
return field.getFieldIndexOptional();
}

public Integer getFieldIndex() {
return field.getFieldIndex();
}

@Nonnull
public Type getFieldType() {
return field.getFieldType();
Expand Down
18 changes: 9 additions & 9 deletions ...rc/main/java/com/apple/foundationdb/record/query/plan/cascades/values/TransformValue.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -304,7 +304,7 @@ private static TrieNode computeTrieForFieldPaths(@Nonnull final FieldPath prefix
orderedFieldPathIterator.next();
return new TrieNode(Verify.verifyNotNull(transformMap.get(prefix)), null);
}
final var childrenMapBuilder = ImmutableMap.<Type.Record.Field, TrieNode>builder();
final var childrenMapBuilder = ImmutableMap.<FieldValue.FieldDelegate, TrieNode>builder();
while (orderedFieldPathIterator.hasNext()) {
final var fieldPath = orderedFieldPathIterator.peek();
if (!prefix.isPrefixOf(fieldPath)) {
Expand All @@ -313,7 +313,7 @@ private static TrieNode computeTrieForFieldPaths(@Nonnull final FieldPath prefix

final var prefixFields = prefix.getFields();
final var currentField = fieldPath.getFields().get(prefixFields.size());
final var nestedPrefix = new FieldPath(ImmutableList.<Type.Record.Field>builder()
final var nestedPrefix = new FieldPath(ImmutableList.<FieldValue.FieldDelegate>builder()
.addAll(prefixFields)
.add(currentField)
.build());
Expand Down Expand Up @@ -356,11 +356,11 @@ public static class TrieNode {
@Nullable
private final Value value;
@Nullable
private final Map<Type.Record.Field, TrieNode> childrenMap;
private final Map<FieldValue.FieldDelegate, TrieNode> childrenMap;
@Nullable
private final Map<Integer, Type.Record.Field> fieldIndexToFieldMap;
private final Map<Integer, FieldValue.FieldDelegate> fieldIndexToFieldMap;

public TrieNode(@Nullable final Value value, @Nullable final Map<Type.Record.Field, TrieNode> childrenMap) {
public TrieNode(@Nullable final Value value, @Nullable final Map<FieldValue.FieldDelegate, TrieNode> childrenMap) {
this.value = value;
this.childrenMap = childrenMap == null ? null : ImmutableMap.copyOf(childrenMap);
this.fieldIndexToFieldMap = childrenMap == null ? null : computeFieldIndexToFieldMap(this.childrenMap);
Expand All @@ -372,18 +372,18 @@ public Value getValue() {
}

@Nullable
public Map<Type.Record.Field, TrieNode> getChildrenMap() {
public Map<FieldValue.FieldDelegate, TrieNode> getChildrenMap() {
return childrenMap;
}

@Nullable
public Map<Integer, Type.Record.Field> getFieldIndexToFieldMap() {
public Map<Integer, FieldValue.FieldDelegate> getFieldIndexToFieldMap() {
return fieldIndexToFieldMap;
}

@Nonnull
private static Map<Integer, Type.Record.Field> computeFieldIndexToFieldMap(@Nonnull final Map<Type.Record.Field, TrieNode> childrenMap) {
final var resultBuilder = ImmutableMap.<Integer, Type.Record.Field>builder();
private static Map<Integer, FieldValue.FieldDelegate> computeFieldIndexToFieldMap(@Nonnull final Map<FieldValue.FieldDelegate, TrieNode> childrenMap) {
final var resultBuilder = ImmutableMap.<Integer, FieldValue.FieldDelegate>builder();
for (final var entry : childrenMap.entrySet()) {
resultBuilder.put(entry.getKey().getFieldIndex(), entry.getKey());
}
Expand Down
3 changes: 2 additions & 1 deletion ...r-core/src/main/java/com/apple/foundationdb/record/query/plan/cascades/values/Values.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,8 @@ public static Set<Value> primitiveAccessorsForType(@Nonnull final Type type,
final var fields = recordType.getFields();

for (final var field : fields) {
primitiveAccessorsForType(field.getFieldType(), () -> FieldValue.ofFieldsAndFuseIfPossible(baseValueSupplier.get(), ImmutableList.of(field)), constantAliases).stream()
primitiveAccessorsForType(field.getFieldType(), () -> FieldValue.ofFieldsAndFuseIfPossible(baseValueSupplier.get(),
ImmutableList.of(FieldValue.FieldDelegate.of(field))), constantAliases).stream()
.map(orderingValue -> orderingValue.simplify(DefaultValueSimplificationRuleSet.ofSimplificationRules(), AliasMap.emptyMap(), constantAliases))
.forEach(orderingValuesBuilder::add);
}
Expand Down
3 changes: 2 additions & 1 deletion ...ndb/record/query/plan/cascades/values/simplification/CompensateRecordConstructorRule.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import com.apple.foundationdb.annotation.API;
import com.apple.foundationdb.record.query.plan.cascades.LinkedIdentityMap;
import com.apple.foundationdb.record.query.plan.cascades.matching.structure.BindingMatcher;
import com.apple.foundationdb.record.query.plan.cascades.values.FieldValue;
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.simplification.MatchOrCompensateFieldValueRule.FieldValueCompensation;
Expand Down Expand Up @@ -84,7 +85,7 @@ public void onMatch(@Nonnull final ValueComputationRuleCall<Iterable<? extends V
final var argumentValue = childValueEntry.getKey();
final var argumentValueCompensation = childValueEntry.getValue();
newMatchedValuesMap.put(argumentValue,
new FieldValueCompensation(ImmutableList.of(column.getField()), argumentValueCompensation));
new FieldValueCompensation(ImmutableList.of(FieldValue.FieldDelegate.of(column.getField())), argumentValueCompensation));
}
return newMatchedValuesMap;
})
Expand Down
10 changes: 5 additions & 5 deletions ...ndb/record/query/plan/cascades/values/simplification/MatchOrCompensateFieldValueRule.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -108,22 +108,22 @@ public void onMatch(@Nonnull final ValueComputationRuleCall<Iterable<? extends V
*/
public static class FieldValueCompensation implements Function<Value, Value> {
@Nonnull
private final List<Type.Record.Field> fieldPath;
private final List<FieldValue.FieldDelegate> fieldPath;

@Nonnull
private final Function<Value, Value> downstreamCompensation;

public FieldValueCompensation(@Nonnull final List<Type.Record.Field> fieldPath) {
public FieldValueCompensation(@Nonnull final List<FieldValue.FieldDelegate> fieldPath) {
this(fieldPath, Function.identity());
}

public FieldValueCompensation(@Nonnull final List<Type.Record.Field> fieldPath, @Nonnull final Function<Value, Value> downstreamCompensation) {
public FieldValueCompensation(@Nonnull final List<FieldValue.FieldDelegate> fieldPath, @Nonnull final Function<Value, Value> downstreamCompensation) {
this.fieldPath = ImmutableList.copyOf(fieldPath);
this.downstreamCompensation = downstreamCompensation;
}

@Nonnull
public List<Type.Record.Field> getFieldPath() {
public List<FieldValue.FieldDelegate> getFieldPath() {
return fieldPath;
}

Expand All @@ -133,7 +133,7 @@ public Value apply(final Value value) {
return downstreamCompensation.apply(FieldValue.ofFieldsAndFuseIfPossible(value, fieldPath));
}

public Function<Value, Value> withSuffix(@Nonnull final List<Type.Record.Field> suffixFieldPath) {
public Function<Value, Value> withSuffix(@Nonnull final List<FieldValue.FieldDelegate> suffixFieldPath) {
if (suffixFieldPath.isEmpty()) {
return downstreamCompensation;
}
Expand Down

0 comments on commit ca24264

Please sign in to comment.