Skip to content
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.

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

Temporary tables #2962

Merged
merged 18 commits into from
Nov 12, 2024
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
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
2 changes: 1 addition & 1 deletion docs/ReleaseNotes.md
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ Starting with version [3.4.455.0](#344550), the semantics of `UnnestedRecordType
* **Performance** Improvement 4 [(Issue #NNN)](https://github.com/FoundationDB/fdb-record-layer/issues/NNN)
* **Performance** Improvement 5 [(Issue #NNN)](https://github.com/FoundationDB/fdb-record-layer/issues/NNN)
* **Feature** Add more lucene exception handling tests [(Issue #2939)](https://github.com/FoundationDB/fdb-record-layer/issues/2939)
* **Feature** Feature 2 [(Issue #NNN)](https://github.com/FoundationDB/fdb-record-layer/issues/NNN)
* **Feature** Table queues [(Issue #2962)](https://github.com/FoundationDB/fdb-record-layer/pull/2962)
* **Feature** Feature 3 [(Issue #NNN)](https://github.com/FoundationDB/fdb-record-layer/issues/NNN)
* **Feature** Feature 4 [(Issue #NNN)](https://github.com/FoundationDB/fdb-record-layer/issues/NNN)
* **Feature** Feature 5 [(Issue #NNN)](https://github.com/FoundationDB/fdb-record-layer/issues/NNN)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,17 +44,18 @@ public class Bindings {
/**
* Bindings slots used internally by plan operators.
*/
public enum Internal {
public enum BindingType {
Copy link
Contributor

Choose a reason for hiding this comment

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

I agree that Internal is too short for what is now a fairly wide variety of special bindings. But BindingType suggests to me that all bindings would be of one of these types, when these are still just the special ones and normally named parameters aren't one of these types. Also, the serialized enum is PBindingKind. It seems that perhaps a longer consistent name is needed.

Copy link
Contributor

Choose a reason for hiding this comment

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

The only bindings that do not have an Internal are named bindings coming from the heuristic planner. I think everything in the cascades planner actually has a corresponding Internal (mostly CORRELATION or CONSTANT so far).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I removed the newly added enum value TABLE_QUEUE effectively reverting the change, but I renamed Internal to BindingKind (not BindingType) to align with the corresponding PB definition in record_query_plan.proto:

Copy link
Contributor

Choose a reason for hiding this comment

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

Although one might prefer to build a ConstantObjectValue and/or CONSTANT binding, from some higher level query data structure, it works today to call CascadesPlanner with a RecordQuery containing a ParameterComparison and run that plan with an EvaluationContext with a Bindings with a corresponding named parameter. In other words, an external binding kind. These untyped / neutral bindings are not specific to the heuristic planner.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I reverted the name back to how it was before. It looks like introducing an External binding could cause wider code changes that are outside the scope of this PR. Let's discuss this as a follow up item.

IN("__in_"),
RANK("__rank_"),
CORRELATION("__corr_"),
CONSTANT("__const_");
CONSTANT("__const_"),
TABLE_QUEUE("__tq_");

public static final String PREFIX = "__";
private final String value;


Internal(String value) {
BindingType(String value) {
this.value = value;
}

Expand Down Expand Up @@ -87,14 +88,16 @@ public PBindingKind toProto(@Nonnull final PlanSerializationContext serializatio
return PBindingKind.CONSTANT;
case CORRELATION:
return PBindingKind.CORRELATION;
case TABLE_QUEUE:
return PBindingKind.TABLE_QUEUE; // TEMP_TABLE
default:
throw new RecordCoreException("unknown binding mapping. did you forget to map it?");
}
}

@Nonnull
@SuppressWarnings("unused")
public static Internal fromProto(@Nonnull final PlanSerializationContext serializationContext, @Nonnull final PBindingKind bindingKindProto) {
public static BindingType fromProto(@Nonnull final PlanSerializationContext serializationContext, @Nonnull final PBindingKind bindingKindProto) {
switch (bindingKindProto) {
case IN:
return IN;
Expand All @@ -104,6 +107,8 @@ public static Internal fromProto(@Nonnull final PlanSerializationContext seriali
return CONSTANT;
case CORRELATION:
return CORRELATION;
case TABLE_QUEUE:
return TABLE_QUEUE;
default:
throw new RecordCoreException("unknown binding mapping. did you forget to map it?");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -127,11 +127,13 @@ public Object getBinding(@Nonnull String name) {
/**
* Get the value bound to a single parameter.
*
* @param type the type of the parameter
* @param alias the correlation identifier
*
* @return the value bound to the given parameter
*/
public Object getBinding(@Nonnull CorrelationIdentifier alias) {
return bindings.get(Bindings.Internal.CORRELATION.bindingName(alias.getId()));
public Object getBinding(@Nonnull Bindings.BindingType type, @Nonnull CorrelationIdentifier alias) {
return bindings.get(type.bindingName(alias.getId()));
}

/**
Expand All @@ -144,7 +146,7 @@ public Object getBinding(@Nonnull CorrelationIdentifier alias) {
@SuppressWarnings("unchecked")
@Nullable
public Object dereferenceConstant(@Nonnull final CorrelationIdentifier alias, @Nonnull final String constantId) {
final var constantsMap = (Map<String, ?>)bindings.get(Bindings.Internal.CONSTANT.bindingName(alias.getId()));
final var constantsMap = (Map<String, ?>)bindings.get(Bindings.BindingType.CONSTANT.bindingName(alias.getId()));
if (constantsMap == null) {
throw new RecordCoreException("could not find constant in the evaluation context")
.addLogInfo(LogMessageKeys.KEY, "'" + alias.getId() + "' - '" + constantId + "'");
Expand Down Expand Up @@ -201,11 +203,13 @@ public EvaluationContext withBinding(@Nonnull String bindingName, @Nullable Obje
* context included all bindings except that it will bind an additional
* parameter to an additional value.
*
* @param type the type of the binding.
* @param alias the alias determining the binding name to add
* @param value the value to bind the name to
*
* @return a new <code>EvaluationContext</code> with the new binding
*/
public EvaluationContext withBinding(@Nonnull CorrelationIdentifier alias, @Nullable Object value) {
return childBuilder().setBinding(Bindings.Internal.CORRELATION.bindingName(alias.getId()), value).build(typeRepository);
public EvaluationContext withBinding(final Bindings.BindingType type, @Nonnull CorrelationIdentifier alias, @Nullable Object value) {
return childBuilder().setBinding(type.bindingName(alias.getId()), value).build(typeRepository);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -96,11 +96,11 @@ public EvaluationContextBuilder setBinding(@Nonnull String name, @Nullable Objec
* @return this <code>EvaluationContextBuilder</code>
*/
public EvaluationContextBuilder setBinding(@Nonnull CorrelationIdentifier alias, @Nullable Object value) {
return setBinding(Bindings.Internal.CORRELATION.bindingName(alias.getId()), value);
return setBinding(Bindings.BindingType.CORRELATION.bindingName(alias.getId()), value);
}

public EvaluationContextBuilder setConstant(@Nonnull CorrelationIdentifier alias, @Nullable Object value) {
return setBinding(Bindings.Internal.CONSTANT.bindingName(alias.getId()), value);
return setBinding(Bindings.BindingType.CONSTANT.bindingName(alias.getId()), value);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@

package com.apple.foundationdb.record.cursors.aggregate;

import com.apple.foundationdb.record.Bindings;
import com.apple.foundationdb.record.EvaluationContext;
import com.apple.foundationdb.record.RecordCursorResult;
import com.apple.foundationdb.record.provider.foundationdb.FDBRecordStoreBase;
Expand Down Expand Up @@ -183,13 +184,13 @@ private void finalizeGroup(Object nextGroup) {
}

private void accumulate(@Nullable Object currentObject) {
EvaluationContext nestedContext = context.withBinding(alias, currentObject);
EvaluationContext nestedContext = context.withBinding(Bindings.BindingType.CORRELATION, alias, currentObject);
final Object partial = aggregateValue.evalToPartial(store, nestedContext);
accumulator.accumulate(partial);
}

private Object evalGroupingKey(@Nullable final Object currentObject) {
final EvaluationContext nestedContext = context.withBinding(alias, currentObject);
final EvaluationContext nestedContext = context.withBinding(Bindings.BindingType.CORRELATION, alias, currentObject);
return Objects.requireNonNull(groupingKeyValue).eval(store, nestedContext);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ public <M extends Message> boolean shouldIndexThisRecord(@Nonnull FDBRecordStore
CorrelationIdentifier objectQuantifier = Quantifier.current();
QueryPredicate queryPredicate = getQueryPredicate(store.getRecordMetaData(), savedRecord.getRecordType(), objectQuantifier);

String bindingName = Bindings.Internal.CORRELATION.bindingName(objectQuantifier.getId());
String bindingName = Bindings.BindingType.CORRELATION.bindingName(objectQuantifier.getId());
Bindings bindings = Bindings.newBuilder().set(bindingName, QueryResult.ofComputed(savedRecord.getRecord())).build();

return Boolean.TRUE.equals(queryPredicate.eval(store, EvaluationContext.forBindings(bindings)));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1150,19 +1150,19 @@ public abstract static class ParameterComparisonBase implements ComparisonWithPa
@Nonnull
protected final String parameter;
@Nullable
protected final Bindings.Internal internal;
protected final Bindings.BindingType bindingType;
@Nonnull
protected final ParameterRelationshipGraph parameterRelationshipGraph;
@Nonnull
protected final Supplier<Integer> hashCodeSupplier;

protected ParameterComparisonBase(@Nonnull Type type, @Nonnull String parameter,
@Nullable Bindings.Internal internal,
@Nullable Bindings.BindingType bindingType,
@Nonnull ParameterRelationshipGraph parameterRelationshipGraph) {
checkInternalBinding(parameter, internal);
checkInternalBinding(parameter, bindingType);
this.type = type;
this.parameter = parameter;
this.internal = internal;
this.bindingType = bindingType;
if (type.isUnary()) {
throw new RecordCoreException("Unary comparison type " + type + " cannot be bound to a parameter");
}
Expand All @@ -1183,15 +1183,15 @@ public Type getType() {
}

public boolean isCorrelation() {
return internal == Bindings.Internal.CORRELATION;
return bindingType == Bindings.BindingType.CORRELATION;
}

@Override
public boolean isCorrelatedTo(@Nonnull final CorrelationIdentifier alias) {
if (!isCorrelation()) {
return false;
}
return Bindings.Internal.CORRELATION.identifier(getParameter()).equals(alias.getId());
return Bindings.BindingType.CORRELATION.identifier(getParameter()).equals(alias.getId());
}

@Nullable
Expand All @@ -1212,7 +1212,7 @@ public Object getComparand(@Nullable FDBRecordStoreBase<?> store, @Nullable Eval
@SuppressWarnings("PMD.CompareObjectsWithEquals")
public Comparison translateCorrelations(@Nonnull final TranslationMap translationMap) {
if (isCorrelation()) {
final var alias = CorrelationIdentifier.of(Bindings.Internal.CORRELATION.identifier(parameter));
final var alias = CorrelationIdentifier.of(Bindings.BindingType.CORRELATION.identifier(parameter));
final var quantifiedObjectValue = QuantifiedObjectValue.of(alias,
com.apple.foundationdb.record.query.plan.cascades.typing.Type.any());

Expand Down Expand Up @@ -1309,7 +1309,7 @@ public CorrelationIdentifier getAlias() {
if (!isCorrelation()) {
throw new IllegalStateException("caller should check for type of binding before calling this method");
}
return CorrelationIdentifier.of(Bindings.Internal.CORRELATION.identifier(parameter));
return CorrelationIdentifier.of(Bindings.BindingType.CORRELATION.identifier(parameter));
}

@Override
Expand Down Expand Up @@ -1359,10 +1359,10 @@ public int queryHash(@Nonnull final QueryHashKind hashKind) {
}

@Nonnull
private static String checkInternalBinding(@Nonnull String parameter, @Nullable Bindings.Internal internal) {
if (internal == null && Bindings.Internal.isInternal(parameter)) {
private static String checkInternalBinding(@Nonnull String parameter, @Nullable Bindings.BindingType bindingType) {
if (bindingType == null && Bindings.BindingType.isInternal(parameter)) {
throw new RecordCoreException(
"Parameter is internal, parameters cannot start with \"" + Bindings.Internal.PREFIX + "\"");
"Parameter is internal, parameters cannot start with \"" + Bindings.BindingType.PREFIX + "\"");
}
return parameter;
}
Expand All @@ -1373,17 +1373,17 @@ private static String checkInternalBinding(@Nonnull String parameter, @Nullable
*/
public static class ParameterComparison extends ParameterComparisonBase {
protected ParameterComparison(@Nonnull Type type, @Nonnull String parameter,
@Nullable Bindings.Internal internal,
@Nullable Bindings.BindingType bindingType,
@Nonnull ParameterRelationshipGraph parameterRelationshipGraph) {
super(type, parameter, internal, parameterRelationshipGraph);
super(type, parameter, bindingType, parameterRelationshipGraph);
}

public ParameterComparison(@Nonnull Type type, @Nonnull String parameter) {
this(type, parameter, null, ParameterRelationshipGraph.unbound());
}

public ParameterComparison(@Nonnull Type type, @Nonnull String parameter, @Nullable Bindings.Internal internal) {
this(type, parameter, internal, ParameterRelationshipGraph.unbound());
public ParameterComparison(@Nonnull Type type, @Nonnull String parameter, @Nullable Bindings.BindingType bindingType) {
this(type, parameter, bindingType, ParameterRelationshipGraph.unbound());
}

@Nonnull
Expand All @@ -1392,23 +1392,23 @@ public Comparison withType(@Nonnull final Type newType) {
if (type == newType) {
return this;
}
return new ParameterComparison(newType, parameter, internal, parameterRelationshipGraph);
return new ParameterComparison(newType, parameter, bindingType, parameterRelationshipGraph);
}

@Nonnull
@Override
protected ParameterComparisonBase withTranslatedCorrelation(@Nonnull CorrelationIdentifier translatedAlias) {
return new ParameterComparison(type,
Bindings.Internal.CORRELATION.bindingName(translatedAlias.getId()),
Bindings.Internal.CORRELATION,
Bindings.BindingType.CORRELATION.bindingName(translatedAlias.getId()),
Bindings.BindingType.CORRELATION,
parameterRelationshipGraph);
}

@Nonnull
@Override
public Comparison withParameterRelationshipMap(@Nonnull final ParameterRelationshipGraph parameterRelationshipGraph) {
Verify.verify(this.parameterRelationshipGraph.isUnbound());
return new ParameterComparison(type, parameter, internal, parameterRelationshipGraph);
return new ParameterComparison(type, parameter, bindingType, parameterRelationshipGraph);
}

@Nonnull
Expand All @@ -1418,8 +1418,8 @@ public PParameterComparison toProto(@Nonnull final PlanSerializationContext seri
.setType(type.toProto(serializationContext))
.setParameter(parameter);

if (internal != null) {
builder.setInternal(internal.toProto(serializationContext));
if (bindingType != null) {
builder.setInternal(bindingType.toProto(serializationContext));
}
return builder.build();
}
Expand All @@ -1433,15 +1433,15 @@ public PComparison toComparisonProto(@Nonnull final PlanSerializationContext ser
@Nonnull
public static ParameterComparison fromProto(@Nonnull final PlanSerializationContext serializationContext,
@Nonnull final PParameterComparison parameterComparisonProto) {
final Bindings.Internal internal;
final Bindings.BindingType bindingType;
if (parameterComparisonProto.hasInternal()) {
internal = Bindings.Internal.fromProto(serializationContext, Objects.requireNonNull(parameterComparisonProto.getInternal()));
bindingType = Bindings.BindingType.fromProto(serializationContext, Objects.requireNonNull(parameterComparisonProto.getInternal()));
} else {
internal = null;
bindingType = null;
}
return new ParameterComparison(Type.fromProto(serializationContext, Objects.requireNonNull(parameterComparisonProto.getType())),
Objects.requireNonNull(parameterComparisonProto.getParameter()),
internal);
bindingType);
}

/**
Expand Down
Loading