Skip to content

feat: Session support for mutateRows - #14644

Open
alexoneill wants to merge 2 commits into
googleapis:mainfrom
alexoneill:feat-session-mutate-rows
Open

alexoneill wants to merge 2 commits into
googleapis:mainfrom
alexoneill:feat-session-mutate-rows

Conversation

@alexoneill

Copy link
Copy Markdown
Contributor

No description provided.

@alexoneill
alexoneill requested a review from mutianf October 13, 2026 14:51
@alexoneill
alexoneill requested review from a team as code owners October 13, 2026 14:51

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request introduces support for mutateRows (bulk mutations) across the Bigtable client's async API and compatibility layers, including TableAsync, AuthorizedViewAsync, and TableBase, along with a new MutateRowsShim implementation. The review feedback suggests importing com.google.common.base.Preconditions and adding a defensive null check for the client parameter in the MutateRowsShim constructor to ensure object integrity.

Comment on lines +27 to +28
import com.google.cloud.bigtable.data.v2.models.TargetId;
import io.grpc.Deadline;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

Import com.google.common.base.Preconditions to support defensive null-checking of parameters in the constructor.

Suggested change
import com.google.cloud.bigtable.data.v2.models.TargetId;
import io.grpc.Deadline;
import com.google.cloud.bigtable.data.v2.models.TargetId;
import com.google.common.base.Preconditions;
import io.grpc.Deadline;

Comment on lines +37 to +38
public MutateRowsShim(Client client) {
tables =

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

Add a defensive null check for the client parameter in the constructor to ensure the object is initialized with a valid reference.

  public MutateRowsShim(Client client) {
    Preconditions.checkNotNull(client, "client is required");
    tables =
References
  1. Defensive null checks in constructors are appropriate to ensure object integrity, provided they are not redundant with upstream validation.

return f;
}

// TODO: get deadline from compatibility layer

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

this comment can be removed. My PR is removing it in all other places.

return true;
}

public static boolean isIdempotent(SessionMutateRowsRequest req) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can we remove this method?

on line 192 in tableBase you can do something like:

ImmutableList<Mutation> mutations = request.getEntriesList().stream().flatMap(entry -> entry.getMutationsList().stream()).collect(toImmutableList());
Util.isIdempotent(mutations);

private final SessionPoolMap<AuthorizedViewId, AuthorizedViewAsync> authViews;

public MutateRowsShim(Client client) {
tables =

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

we should consider share the pool with MutateRow.

See my PR (although is not approved yet 😂 )
https://github.com/googleapis/google-cloud-java/pull/14585/changes#diff-e47339d2678dd2991014d93d7eaa96d999bb53d535c753e67766f0582b2212f4

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants