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.

Sign up for GitHub

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

Jump to bottom

fix(bigquery): dependency detection on proto conversion #8566

Merged
merged 4 commits into from
Sep 19, 2023
Merged

fix(bigquery): dependency detection on proto conversion #8566

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
2dc139d
fix(bigquery): dependency detection on proto conversion
alvarowolfx Sep 13, 2023
e0e2f90
test: convert reuse submessage test to table driven format
alvarowolfx Sep 13, 2023
dfb8345
test(bigquery): better names for nested reused message
alvarowolfx Sep 15, 2023
f1fc4d4
Merge branch 'main' into bq-fix-adapt-dep-detection
alvarowolfx Sep 18, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
25 changes: 23 additions & 2 deletions bigquery/storage/managedwriter/adapt/protoconversion.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -185,8 +185,8 @@ func storageSchemaToDescriptorInternal(inSchema *storagepb.TableSchema, scope st
if foundDesc != nil {
// check to see if we already have this in current dependency list
haveDep := false
for _, curDep := range deps {
if foundDesc.ParentFile().FullName() == curDep.FullName() {
for _, dep := range deps {
if messageDependsOnFile(foundDesc, dep) {
haveDep = true
break
}
Expand Down Expand Up @@ -279,6 +279,27 @@ func storageSchemaToDescriptorInternal(inSchema *storagepb.TableSchema, scope st
return found.(protoreflect.MessageDescriptor), nil
}

// messageDependsOnFile checks if the given message descriptor already belongs to the file descriptor.
// To check for that, first we check if the message descriptor parent file is the same as the file descriptor.
// If not, check if the message descriptor belongs is contained as a child of the file descriptor.
func messageDependsOnFile(msg protoreflect.MessageDescriptor, file protoreflect.FileDescriptor) bool {
parentFile := msg.ParentFile()
parentFileName := parentFile.FullName()
if parentFileName != "" {
if parentFileName == file.FullName() {
return true
}
}
fileMessages := file.Messages()
for i := 0; i < fileMessages.Len(); i++ {
childMsg := fileMessages.Get(i)
if msg.FullName() == childMsg.FullName() {
return true
}
}
return false
}

// tableFieldSchemaToFieldDescriptorProto builds individual field descriptors for a proto message.
//
// For proto3, in cases where the mode is nullable we use the well known wrapper types.
Expand Down
169 changes: 169 additions & 0 deletions bigquery/storage/managedwriter/adapt/protoconversion_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -360,6 +360,175 @@ func TestSchemaToProtoConversion(t *testing.T) {
},
},
},
{
description: "nested with reused submessage in different levels",
bq: &storagepb.TableSchema{
Fields: []*storagepb.TableFieldSchema{
{
Name: "bartest1",
Type: storagepb.TableFieldSchema_STRUCT,
Mode: storagepb.TableFieldSchema_REQUIRED,
Fields: []*storagepb.TableFieldSchema{
{
Name: "barid",
Type: storagepb.TableFieldSchema_STRING,
Mode: storagepb.TableFieldSchema_REQUIRED,
},
},
},
{
Name: "fullprofile",
Type: storagepb.TableFieldSchema_STRUCT,
Mode: storagepb.TableFieldSchema_REQUIRED,
Fields: []*storagepb.TableFieldSchema{
{
Name: "biztest1",
Type: storagepb.TableFieldSchema_STRUCT,
Mode: storagepb.TableFieldSchema_REQUIRED,
Fields: []*storagepb.TableFieldSchema{
{
Name: "bizid",
Type: storagepb.TableFieldSchema_STRING,
Mode: storagepb.TableFieldSchema_REQUIRED,
},
},
},
{
Name: "bartest2",
Type: storagepb.TableFieldSchema_STRUCT,
Mode: storagepb.TableFieldSchema_REQUIRED,
Fields: []*storagepb.TableFieldSchema{
{
Name: "barid",
Type: storagepb.TableFieldSchema_STRING,
Mode: storagepb.TableFieldSchema_REQUIRED,
},
},
},
{
Name: "bartest3",
Type: storagepb.TableFieldSchema_STRUCT,
Mode: storagepb.TableFieldSchema_REQUIRED,
Fields: []*storagepb.TableFieldSchema{
{
Name: "barid",
Type: storagepb.TableFieldSchema_STRING,
Mode: storagepb.TableFieldSchema_REQUIRED,
},
},
},
},
},
},
},
wantProto2: &descriptorpb.DescriptorProto{
Name: proto.String("root"),
Field: []*descriptorpb.FieldDescriptorProto{
{
Name: proto.String("bartest1"),
Number: proto.Int32(1),
Type: descriptorpb.FieldDescriptorProto_TYPE_MESSAGE.Enum(),
TypeName: proto.String(".root__bartest1"),
Label: descriptorpb.FieldDescriptorProto_LABEL_REQUIRED.Enum(),
},
{
Name: proto.String("fullprofile"),
Number: proto.Int32(2),
Type: descriptorpb.FieldDescriptorProto_TYPE_MESSAGE.Enum(),
TypeName: proto.String(".root__fullprofile"),
Label: descriptorpb.FieldDescriptorProto_LABEL_REQUIRED.Enum(),
},
},
},
wantProto2Normalized: &descriptorpb.DescriptorProto{
Name: proto.String("root"),
Field: []*descriptorpb.FieldDescriptorProto{
{
Name: proto.String("bartest1"),
Number: proto.Int32(1),
Type: descriptorpb.FieldDescriptorProto_TYPE_MESSAGE.Enum(),
TypeName: proto.String("root__bartest1"),
Label: descriptorpb.FieldDescriptorProto_LABEL_REQUIRED.Enum(),
},
{
Name: proto.String("fullprofile"),
Number: proto.Int32(2),
Type: descriptorpb.FieldDescriptorProto_TYPE_MESSAGE.Enum(),
TypeName: proto.String("root__fullprofile"),
Label: descriptorpb.FieldDescriptorProto_LABEL_REQUIRED.Enum(),
},
},
NestedType: []*descriptorpb.DescriptorProto{
{
Name: proto.String("root__bartest1"),
Field: []*descriptorpb.FieldDescriptorProto{
{
Name: proto.String("barid"),
Number: proto.Int32(1),
Type: descriptorpb.FieldDescriptorProto_TYPE_STRING.Enum(),
Label: descriptorpb.FieldDescriptorProto_LABEL_REQUIRED.Enum(),
},
},
},
{
Name: proto.String("root__fullprofile__biztest1"),
Field: []*descriptorpb.FieldDescriptorProto{
{
Name: proto.String("bizid"),
Number: proto.Int32(1),
Type: descriptorpb.FieldDescriptorProto_TYPE_STRING.Enum(),
Label: descriptorpb.FieldDescriptorProto_LABEL_REQUIRED.Enum(),
},
},
},
{
Name: proto.String("root__fullprofile"),
Field: []*descriptorpb.FieldDescriptorProto{
{
Name: proto.String("biztest1"),
Number: proto.Int32(1),
Type: descriptorpb.FieldDescriptorProto_TYPE_MESSAGE.Enum(),
TypeName: proto.String("root__fullprofile__biztest1"),
Label: descriptorpb.FieldDescriptorProto_LABEL_REQUIRED.Enum(),
},
{
Name: proto.String("bartest2"),
Number: proto.Int32(2),
Type: descriptorpb.FieldDescriptorProto_TYPE_MESSAGE.Enum(),
TypeName: proto.String("root__bartest1"),
Label: descriptorpb.FieldDescriptorProto_LABEL_REQUIRED.Enum(),
},
{
Name: proto.String("bartest3"),
Number: proto.Int32(3),
Type: descriptorpb.FieldDescriptorProto_TYPE_MESSAGE.Enum(),
TypeName: proto.String("root__bartest1"),
Label: descriptorpb.FieldDescriptorProto_LABEL_REQUIRED.Enum(),
},
},
},
},
},
wantProto3: &descriptorpb.DescriptorProto{
Name: proto.String("root"),
Field: []*descriptorpb.FieldDescriptorProto{
{
Name: proto.String("bartest1"),
Number: proto.Int32(1),
Type: descriptorpb.FieldDescriptorProto_TYPE_MESSAGE.Enum(),
TypeName: proto.String(".root__bartest1"),
Label: descriptorpb.FieldDescriptorProto_LABEL_OPTIONAL.Enum(),
},
{
Name: proto.String("fullprofile"),
Number: proto.Int32(2),
Type: descriptorpb.FieldDescriptorProto_TYPE_MESSAGE.Enum(),
TypeName: proto.String(".root__fullprofile"),
Label: descriptorpb.FieldDescriptorProto_LABEL_OPTIONAL.Enum(),
},
},
},
},
{
description: "multiple nesting levels",
bq: &storagepb.TableSchema{
Expand Down