Skip to content

Commit

Permalink
Fixes #2806: add Support for nested DynamicMessage in ConstantObjectV…
Browse files Browse the repository at this point in the history
…alue (#2807)
  • Loading branch information
g31pranjal authored Jul 11, 2024
1 parent 675e019 commit ae4f343
Show file tree
Hide file tree
Showing 6 changed files with 162 additions and 7 deletions.
2 changes: 1 addition & 1 deletion docs/ReleaseNotes.md
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ Starting with version [3.4.455.0](#344550), the semantics of `UnnestedRecordType
* **Feature** Feature 2 [(Issue #NNN)](https://github.com/FoundationDB/fdb-record-layer/issues/NNN)
* **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)
* **Feature** add Support for nested DynamicMessage in ConstantObjectValue [(Issue #2806)](https://github.com/FoundationDB/fdb-record-layer/issues/2806)
* **Breaking change** Change 1 [(Issue #NNN)](https://github.com/FoundationDB/fdb-record-layer/issues/NNN)
* **Breaking change** Change 2 [(Issue #NNN)](https://github.com/FoundationDB/fdb-record-layer/issues/NNN)
* **Breaking change** Change 3 [(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 @@ -58,6 +58,7 @@
import com.google.protobuf.DescriptorProtos.DescriptorProto;
import com.google.protobuf.DescriptorProtos.FieldDescriptorProto;
import com.google.protobuf.Descriptors;
import com.google.protobuf.DynamicMessage;
import com.google.protobuf.Message;
import org.apache.commons.lang3.StringUtils;

Expand Down Expand Up @@ -591,6 +592,9 @@ static Type fromObject(@Nullable final Object object) {
}
return new Type.Array(Type.fromListObject((List<?>)object));
}
if (object instanceof DynamicMessage) {
return Record.fromDescriptor(((DynamicMessage) object).getDescriptorForType());
}
final var typeCode = typeCodeFromPrimitive(object);
if (typeCode == TypeCode.NULL) {
return Type.nullType();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@
import com.google.auto.service.AutoService;
import com.google.common.base.Verify;
import com.google.common.collect.ImmutableList;
import com.google.protobuf.DynamicMessage;
import com.google.protobuf.Message;

import javax.annotation.Nonnull;
Expand Down Expand Up @@ -137,10 +136,6 @@ public <M extends Message> Object eval(@Nonnull final FDBRecordStoreBase<M> stor
Verify.verify(getResultType().isNullable());
return null;
}
if (obj instanceof DynamicMessage) {
// TODO: run coercion for proper promotion, and if that fails then bailout.
return obj;
}
final var objType = Type.fromObject(obj);
final var promotionNeeded = PromoteValue.isPromotionNeeded(objType, getResultType());
if (!promotionNeeded) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -422,7 +422,7 @@ public static boolean isPromotionNeeded(@Nonnull final Type inType, @Nonnull fin
}
if (inType.isRecord() && promoteToType.isRecord()) {
final List<Type> inTypeElements = Objects.requireNonNull(((Type.Record) inType).getElementTypes());
final List<Type> promoteToTypeElements = Objects.requireNonNull(((Type.Record) inType).getElementTypes());
final List<Type> promoteToTypeElements = Objects.requireNonNull(((Type.Record) promoteToType).getElementTypes());
SemanticException.check(inTypeElements.size() == promoteToTypeElements.size(), SemanticException.ErrorCode.INCOMPATIBLE_TYPE);
for (int i = 0; i < inTypeElements.size(); i++) {
SemanticException.check(!isPromotionNeeded(inTypeElements.get(i), promoteToTypeElements.get(i)), SemanticException.ErrorCode.INCOMPATIBLE_TYPE);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,141 @@
/*
* ConstantObjectValueTest.java
*
* This source file is part of the FoundationDB open source project
*
* Copyright 2015-2024 Apple Inc. and the FoundationDB project authors
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package com.apple.foundationdb.record.query.plan.cascades;

import com.apple.foundationdb.record.Bindings;
import com.apple.foundationdb.record.EvaluationContext;
import com.apple.foundationdb.record.query.plan.cascades.typing.Type;
import com.apple.foundationdb.record.query.plan.cascades.typing.TypeRepository;
import com.apple.foundationdb.record.query.plan.cascades.values.ConstantObjectValue;
import com.google.protobuf.DynamicMessage;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.Arguments;
import org.junit.jupiter.params.provider.MethodSource;

import javax.annotation.Nonnull;
import javax.annotation.Nullable;
import java.util.HashMap;
import java.util.List;
import java.util.Objects;
import java.util.Optional;
import java.util.stream.Stream;

/**
* Tests for {@link ConstantObjectValueTest}.
*/
public class ConstantObjectValueTest {

private static Stream<Arguments> argumentsProvider() {
final var type1 = Type.Record.fromFieldsWithName("str_long_long", true, List.of(
Type.Record.Field.of(Type.primitiveType(Type.TypeCode.STRING), Optional.of("f0")),
Type.Record.Field.of(Type.primitiveType(Type.TypeCode.LONG), Optional.of("f1"))
));
final var type2 = Type.Record.fromFieldsWithName("str_int_int", true, List.of(
Type.Record.Field.of(Type.primitiveType(Type.TypeCode.STRING), Optional.of("f0")),
Type.Record.Field.of(Type.primitiveType(Type.TypeCode.INT), Optional.of("f1"))
));
final var typeRepository = TypeRepository.newBuilder().addAllTypes(List.of(type1, type2)).build();
final var descriptor1 = Objects.requireNonNull(typeRepository.getMessageDescriptor(type1));
final var message1 = DynamicMessage.newBuilder(descriptor1)
.setField(descriptor1.findFieldByName("f0"), "blah1")
.setField(descriptor1.findFieldByName("f1"), 1L)
.build();
final var descriptor2 = Objects.requireNonNull(typeRepository.getMessageDescriptor(type2));
final var message2 = DynamicMessage.newBuilder(descriptor2)
.setField(descriptor2.findFieldByName("f0"), "blah1")
.setField(descriptor2.findFieldByName("f1"), 1)
.build();
return Stream.of(
// null
Arguments.of(Type.nullType(), true, null, null),

// primitive type
Arguments.of(Type.primitiveType(Type.TypeCode.INT), true, 10, 10),
Arguments.of(Type.primitiveType(Type.TypeCode.LONG, false), true, 10L, 10L),
Arguments.of(Type.primitiveType(Type.TypeCode.LONG, false), true, 10, 10L),
Arguments.of(Type.primitiveType(Type.TypeCode.FLOAT, false), true, 10, 10.0f),
Arguments.of(Type.primitiveType(Type.TypeCode.FLOAT, false), true, 10L, 10.0f),
Arguments.of(Type.primitiveType(Type.TypeCode.FLOAT, false), true, 10.0f, 10.0f),
Arguments.of(Type.primitiveType(Type.TypeCode.DOUBLE, false), true, 10, 10.0),
Arguments.of(Type.primitiveType(Type.TypeCode.DOUBLE, false), true, 10L, 10.0),
Arguments.of(Type.primitiveType(Type.TypeCode.DOUBLE, false), true, 10.0f, 10.0),
Arguments.of(Type.primitiveType(Type.TypeCode.DOUBLE, false), true, 10.0, 10.0),

// DynamicMessage
Arguments.of(Type.Record.fromFields(List.of(
Type.Record.Field.of(Type.primitiveType(Type.TypeCode.STRING), Optional.of("f0")),
Type.Record.Field.of(Type.primitiveType(Type.TypeCode.LONG), Optional.of("f1"))
)), true, message1, message1),
// Different shape of DynamicMessage is not allowed
Arguments.of(Type.Record.fromFields(List.of(
Type.Record.Field.of(Type.primitiveType(Type.TypeCode.LONG), Optional.of("f1")),
Type.Record.Field.of(Type.primitiveType(Type.TypeCode.STRING), Optional.of("f0"))
)), false, message1, null),
// Promotion in DynamicMessage is not allowed
Arguments.of(Type.Record.fromFields(List.of(
Type.Record.Field.of(Type.primitiveType(Type.TypeCode.STRING), Optional.of("f0")),
Type.Record.Field.of(Type.primitiveType(Type.TypeCode.LONG), Optional.of("f1"))
)), false, message2, null),

// List type
Arguments.of(new Type.Array(Type.primitiveType(Type.TypeCode.LONG)), true, List.of(1L, 2L, 3L), List.of(1L, 2L, 3L)),
Arguments.of(new Type.Array(Type.primitiveType(Type.TypeCode.INT)), true, List.of(1, 2, 3), List.of(1, 2, 3)),
Arguments.of(new Type.Array(Type.primitiveType(Type.TypeCode.DOUBLE)), true, List.of(1.0, 2.0, 3.0), List.of(1.0, 2.0, 3.0)),
Arguments.of(new Type.Array(Type.primitiveType(Type.TypeCode.FLOAT)), true, List.of(1.0f, 2.0f, 3.0f), List.of(1.0f, 2.0f, 3.0f)),
Arguments.of(new Type.Array(Type.primitiveType(Type.TypeCode.STRING)), true, List.of("1", "2", "3"), List.of("1", "2", "3")),
Arguments.of(new Type.Array(Type.primitiveType(Type.TypeCode.BOOLEAN)), true, List.of(true, false, true), List.of(true, false, true)),
// List of DynamicMessage
Arguments.of(new Type.Array(Type.Record.fromFields(List.of(
Type.Record.Field.of(Type.primitiveType(Type.TypeCode.STRING), Optional.of("f0")),
Type.Record.Field.of(Type.primitiveType(Type.TypeCode.LONG), Optional.of("f1"))
))), true, List.of(message1, message1), List.of(message1, message1)),
// promotion in Array is not allowed
Arguments.of(new Type.Array(Type.primitiveType(Type.TypeCode.LONG)), false, List.of(1, 2, 3), null),
Arguments.of(new Type.Array(Type.primitiveType(Type.TypeCode.DOUBLE)), false, List.of(1, 2, 3), null),
Arguments.of(new Type.Array(Type.primitiveType(Type.TypeCode.FLOAT)), false, List.of(1, 2, 3), null),
Arguments.of(new Type.Array(Type.Record.fromFields(List.of(
Type.Record.Field.of(Type.primitiveType(Type.TypeCode.STRING), Optional.of("f0")),
Type.Record.Field.of(Type.primitiveType(Type.TypeCode.LONG), Optional.of("f1"))
))), false, List.of(message2), null),
Arguments.of(new Type.Array(Type.Record.fromFields(List.of(
Type.Record.Field.of(Type.primitiveType(Type.TypeCode.LONG), Optional.of("f1")),
Type.Record.Field.of(Type.primitiveType(Type.TypeCode.STRING), Optional.of("f0"))
))), false, List.of(message1), null)
);
}

@ParameterizedTest
@MethodSource("argumentsProvider")
public void testEval(@Nonnull Type covResultType, boolean expectedSuccess, @Nullable Object bindingObject, @Nullable Object expectedObject) {
final var alias = Bindings.Internal.CONSTANT.bindingName("blah");
final var bindingMap = new HashMap<String, Object>();
bindingMap.put("key", bindingObject);
final var ctx = EvaluationContext.forBindings(Bindings.newBuilder().set(alias, bindingMap).build());
final var cov = ConstantObjectValue.of(CorrelationIdentifier.of("blah"), "key", covResultType);
if (expectedSuccess) {
final var actualValue = cov.eval(null, ctx);
Assertions.assertEquals(expectedObject, actualValue);
} else {
Assertions.assertThrows(Throwable.class, () -> cov.eval(null, ctx));
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,14 @@ public Stream<? extends Arguments> provideArguments(final ExtensionContext conte
listOfNullsAndNonNulls.add(null);
listOfNullsAndNonNulls.add(42);
listOfNullsAndNonNulls.add(43);

final var descriptor = TestRecords4Proto.ReviewerStats.getDescriptor();
final var actualMessage = DynamicMessage.newBuilder(descriptor)
.setField(descriptor.findFieldByName("school_name"), "blah")
.setField(descriptor.findFieldByName("start_date"), 34343L)
.setField(descriptor.findFieldByName("hometown"), "blah2")
.build();

return Stream.of(

// Typed objects
Expand Down Expand Up @@ -255,6 +263,13 @@ public Stream<? extends Arguments> provideArguments(final ExtensionContext conte
Arguments.of(List.of(List.of(ByteString.copyFrom("bar", Charset.defaultCharset().name())),
List.of(ByteString.copyFrom("bar", Charset.defaultCharset().name()))), new Type.Array(new Type.Array(Type.primitiveType(Type.TypeCode.BYTES, false))), false),

// DynamicMessage
Arguments.of(actualMessage, Type.Record.fromFields(List.of(
Type.Record.Field.of(Type.primitiveType(Type.TypeCode.LONG), Optional.of("start_date")),
Type.Record.Field.of(Type.primitiveType(Type.TypeCode.STRING), Optional.of("school_name")),
Type.Record.Field.of(Type.primitiveType(Type.TypeCode.STRING), Optional.of("hometown"))
))),

// Unsupported cases
Arguments.of(new int[] {1, 2, 3}, Type.any()), // primitive Arrays are not supported
Arguments.of(new Integer[] {1, 2, 3}, Type.any()), // object Arrays are not supported
Expand Down

0 comments on commit ae4f343

Please sign in to comment.