Skip to content

Commit

Permalink
testing(bigquery/storage/managewriter): confirm packed behaviors with…
Browse files Browse the repository at this point in the history 
… normalization (#6327)

* testing(bigquery/storage/managewriter): confirm packed behaviors with normalization

It turns out that packed repeated values are backwards compatible, so
reading values doesn't require parity with the packed option in the
writer.

This PR adds tests for normalizing a proto3 descriptor of the various
repeated scalar types, and adds a validation test to ensure packing
behavior is as expected.
  • Loading branch information
@shollyman
shollyman authored Jul 12, 2022
1 parent 989cd0e commit 6d1e9fe
Show file tree
Hide file tree
Showing 2 changed files with 208 additions and 0 deletions.
130 changes: 130 additions & 0 deletions bigquery/storage/managedwriter/adapt/protoconversion_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1084,6 +1084,136 @@ func TestNormalizeDescriptor(t *testing.T) {
},
},
},
{
description: "ValidationP3PackedRepeated",
in: (&testdata.ValidationP3PackedRepeated{}).ProtoReflect().Descriptor(),
want: &descriptorpb.DescriptorProto{
Name: proto.String("testdata_ValidationP3PackedRepeated"),
Field: []*descriptorpb.FieldDescriptorProto{
{
Name: proto.String("id"),
JsonName: proto.String("id"),
Number: proto.Int32(1),
Type: descriptorpb.FieldDescriptorProto_TYPE_INT64.Enum(),
Label: descriptorpb.FieldDescriptorProto_LABEL_OPTIONAL.Enum(),
},
{
Name: proto.String("double_repeated"),
JsonName: proto.String("doubleRepeated"),
Number: proto.Int32(2),
Type: descriptorpb.FieldDescriptorProto_TYPE_DOUBLE.Enum(),
Label: descriptorpb.FieldDescriptorProto_LABEL_REPEATED.Enum(),
},
{
Name: proto.String("float_repeated"),
JsonName: proto.String("floatRepeated"),
Number: proto.Int32(3),
Type: descriptorpb.FieldDescriptorProto_TYPE_FLOAT.Enum(),
Label: descriptorpb.FieldDescriptorProto_LABEL_REPEATED.Enum(),
},
{
Name: proto.String("int32_repeated"),
JsonName: proto.String("int32Repeated"),
Number: proto.Int32(4),
Type: descriptorpb.FieldDescriptorProto_TYPE_INT32.Enum(),
Label: descriptorpb.FieldDescriptorProto_LABEL_REPEATED.Enum(),
},
{
Name: proto.String("int64_repeated"),
JsonName: proto.String("int64Repeated"),
Number: proto.Int32(5),
Type: descriptorpb.FieldDescriptorProto_TYPE_INT64.Enum(),
Label: descriptorpb.FieldDescriptorProto_LABEL_REPEATED.Enum(),
},
{
Name: proto.String("uint32_repeated"),
JsonName: proto.String("uint32Repeated"),
Number: proto.Int32(6),
Type: descriptorpb.FieldDescriptorProto_TYPE_UINT32.Enum(),
Label: descriptorpb.FieldDescriptorProto_LABEL_REPEATED.Enum(),
},
{
Name: proto.String("sint32_repeated"),
JsonName: proto.String("sint32Repeated"),
Number: proto.Int32(7),
Type: descriptorpb.FieldDescriptorProto_TYPE_SINT32.Enum(),
Label: descriptorpb.FieldDescriptorProto_LABEL_REPEATED.Enum(),
},
{
Name: proto.String("sint64_repeated"),
JsonName: proto.String("sint64Repeated"),
Number: proto.Int32(8),
Type: descriptorpb.FieldDescriptorProto_TYPE_SINT64.Enum(),
Label: descriptorpb.FieldDescriptorProto_LABEL_REPEATED.Enum(),
},
{
Name: proto.String("fixed32_repeated"),
JsonName: proto.String("fixed32Repeated"),
Number: proto.Int32(9),
Type: descriptorpb.FieldDescriptorProto_TYPE_FIXED32.Enum(),
Label: descriptorpb.FieldDescriptorProto_LABEL_REPEATED.Enum(),
},
{
Name: proto.String("sfixed32_repeated"),
JsonName: proto.String("sfixed32Repeated"),
Number: proto.Int32(10),
Type: descriptorpb.FieldDescriptorProto_TYPE_SFIXED32.Enum(),
Label: descriptorpb.FieldDescriptorProto_LABEL_REPEATED.Enum(),
},
{
Name: proto.String("sfixed64_repeated"),
JsonName: proto.String("sfixed64Repeated"),
Number: proto.Int32(11),
Type: descriptorpb.FieldDescriptorProto_TYPE_SFIXED64.Enum(),
Label: descriptorpb.FieldDescriptorProto_LABEL_REPEATED.Enum(),
},

{
Name: proto.String("bool_repeated"),
JsonName: proto.String("boolRepeated"),
Number: proto.Int32(12),
Type: descriptorpb.FieldDescriptorProto_TYPE_BOOL.Enum(),
Label: descriptorpb.FieldDescriptorProto_LABEL_REPEATED.Enum(),
},
{
Name: proto.String("enum_repeated"),
JsonName: proto.String("enumRepeated"),
Number: proto.Int32(13),
Type: descriptorpb.FieldDescriptorProto_TYPE_ENUM.Enum(),
TypeName: proto.String("testdata_Proto3ExampleEnum_E.Proto3ExampleEnum"),
Label: descriptorpb.FieldDescriptorProto_LABEL_REPEATED.Enum(),
},
},
NestedType: []*descriptorpb.DescriptorProto{
{
Name: proto.String("testdata_Proto3ExampleEnum_E"),
EnumType: []*descriptorpb.EnumDescriptorProto{
{
Name: proto.String("Proto3ExampleEnum"),
Value: []*descriptorpb.EnumValueDescriptorProto{
{
Name: proto.String("P3_UNDEFINED"),
Number: proto.Int32(0),
},
{
Name: proto.String("P3_THING"),
Number: proto.Int32(1),
},
{
Name: proto.String("P3_OTHER_THING"),
Number: proto.Int32(2),
},
{
Name: proto.String("P3_THIRD_THING"),
Number: proto.Int32(3),
},
},
},
},
},
},
},
},
}

for _, tc := range testCases {
Expand Down
78 changes: 78 additions & 0 deletions bigquery/storage/managedwriter/validation_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,10 @@ import (
"cloud.google.com/go/bigquery/storage/managedwriter/adapt"
"cloud.google.com/go/bigquery/storage/managedwriter/testdata"
"google.golang.org/protobuf/proto"
"google.golang.org/protobuf/reflect/protodesc"
"google.golang.org/protobuf/reflect/protoreflect"
"google.golang.org/protobuf/types/descriptorpb"
"google.golang.org/protobuf/types/dynamicpb"
)

func TestValidation_Values(t *testing.T) {
Expand Down Expand Up @@ -445,3 +449,77 @@ func TestValidation_Values(t *testing.T) {
validateTableConstraints(ctx, t, bqClient, testTable, tc.description, tc.constraints...)
}
}

func TestValidationRoundtripRepeated(t *testing.T) {
// This test exists to confirm packed values are backwards compatible.
// We create a message using a packed descriptor, and normalize it which
// loses the packed option, and confirm we can decode the values using the
// normalized descriptor.
input := &testdata.ValidationP3PackedRepeated{
Id: proto.Int64(2022),
Int64Repeated: []int64{1, 2, 4, -88},
}
b, err := proto.Marshal(input)
if err != nil {
t.Fatalf("proto.Marshal: %v", err)
}

// Verify original packed option (proto3 is default packed)
origDescriptor := input.ProtoReflect().Descriptor()
origFD := origDescriptor.Fields().ByName(protoreflect.Name("int64_repeated"))
if !origFD.IsPacked() {
t.Errorf("expected original field to be packed, wasn't")
}

// Normalize and use it to get a new descriptor.
normalized, err := adapt.NormalizeDescriptor(input.ProtoReflect().Descriptor())
if err != nil {
t.Fatalf("NormalizeDescriptor: %v", err)
}
fdp := &descriptorpb.FileDescriptorProto{
MessageType: []*descriptorpb.DescriptorProto{normalized},
Name: proto.String("lookup"),
Syntax: proto.String("proto2"),
}
fds := &descriptorpb.FileDescriptorSet{
File: []*descriptorpb.FileDescriptorProto{fdp},
}
files, err := protodesc.NewFiles(fds)
if err != nil {
t.Fatalf("protodesc.NewFiles: %v", err)
}
found, err := files.FindDescriptorByName("testdata_ValidationP3PackedRepeated")
if err != nil {
t.Fatalf("FindDescriptorByName: %v", err)
}
md := found.(protoreflect.MessageDescriptor)

// Use the new, normalized descriptor to unmarshal the bytes and verify.
msg := dynamicpb.NewMessage(md)
if err := proto.Unmarshal(b, msg); err != nil {
t.Fatalf("Unmarshal: %v", err)
}

//
int64FD := msg.Descriptor().Fields().ByName(protoreflect.Name("int64_repeated")) // int64_repeated again
if int64FD == nil {
t.Fatalf("failed to get field")
}
if int64FD.IsPacked() {
t.Errorf("expected normalized descriptor to be un-packed, but it was packed")
}
// Ensure we got the expected values out the other side.
list := msg.Get(int64FD).List()
wantLen := 4
if list.Len() != wantLen {
t.Errorf("wanted %d values, got %d", wantLen, list.Len())
}
// Confirm the same values out the other side.
wantVals := []int64{1, 2, 4, -88}
for i := 0; i < list.Len(); i++ {
got := list.Get(i).Int()
if got != wantVals[i] {
t.Errorf("expected elem %d to be %d, was %d", i, wantVals[i], got)
}
}
}

0 comments on commit 6d1e9fe

Please sign in to comment.