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

fix(bigtable): Allow nil condition in conditional mutation #11457

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

bhshkh
Copy link
Contributor

@bhshkh bhshkh commented Jan 16, 2025

When the test proxy receives a conditional mutation request with nil condition, it uses the PassAllFilter to call Apply method of Go library. Thus, the request sent by Go library to Cloud Bigtable service contains the field pass all filter.
TestCheckAndMutateRow_NoRetry_TrueMutations conformance test fails with error

=== RUN   TestCheckAndMutateRow_NoRetry_TrueMutations
    checkandmutaterow_test.go:138: diff found (-want +got):
          (*bigtablepb.CheckAndMutateRowRequest)(Inverse(protocmp.Transform, protocmp.Message{
          	"@type":            s"google.bigtable.v2.CheckAndMutateRowRequest",
        + 	"predicate_filter": s"{pass_all_filter:true}",
          	"row_key":          []uint8("row-01"),
          	"table_name":       string("projects/project/instances/instance/tables/table"),
          	"true_mutations":   []protocmp.Message{{"@type": s"google.bigtable.v2.Mutation", ...}, {"@type": s"google.bigtable.v2.Mutation", ...}},
          }))
--- FAIL: TestCheckAndMutateRow_NoRetry_TrueMutations (0.00s)

The Go library assumes that the field cond in Mutation being non-nil denotes conditional mutation but as per the above test we want to allow conditional mutation with no condition. So, a new variable isConditional is being added to Mutation to distinguish between conditional and non-conditional mutations.

After fix:

=== RUN   TestCheckAndMutateRow_NoRetry_TrueMutations
--- PASS: TestCheckAndMutateRow_NoRetry_TrueMutations (0.00s)

@bhshkh bhshkh requested a review from gkevinzheng January 16, 2025 01:55
@bhshkh bhshkh requested review from a team as code owners January 16, 2025 01:55
@product-auto-label product-auto-label bot added the api: bigtable Issues related to the Bigtable API. label Jan 16, 2025
@bhshkh bhshkh requested a review from andre-sampaio January 16, 2025 01:55
@bhshkh bhshkh enabled auto-merge (squash) January 16, 2025 01:58
@bhshkh bhshkh force-pushed the fix/cbt-TestCheckAndMutateRow_NoRetry_TrueMutations branch from af826d7 to 0397be0 Compare January 16, 2025 02:00
Copy link
Contributor

@gkevinzheng gkevinzheng left a comment

Choose a reason for hiding this comment

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

Where is m.isConditional ever set to false?

@bhshkh
Copy link
Contributor Author

bhshkh commented Jan 16, 2025

Where is m.isConditional ever set to false?

If it is not set, that means it is false. Default value of bool types is false

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: bigtable Issues related to the Bigtable API.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants