Skip to content

fix(bigtable): populate Value type in _format_execute_query_view_params#17547

Open
ad548 wants to merge 1 commit into
googleapis:mainfrom
ad548:fix-view-params-value-type
Open

fix(bigtable): populate Value type in _format_execute_query_view_params#17547
ad548 wants to merge 1 commit into
googleapis:mainfrom
ad548:fix-view-params-value-type

Conversation

@ad548

@ad548 ad548 commented Jun 23, 2026

Copy link
Copy Markdown
Member

Ensures that string view parameters construct Value objects with both string_value and explicit string_type populated.

Fixes issue where view parameters created Value objects missing the explicit type_ field.

@ad548 ad548 requested a review from a team as a code owner June 23, 2026 20:18

@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 updates the view parameter formatting for Bigtable queries to explicitly include the string type when constructing Value objects. It also adds unit tests to verify this behavior. The reviewer noted that comparing proto-plus message objects directly to dictionaries in the new test assertions will fail, and suggested converting the messages to dictionaries first using to_dict.

request = execute_query_mock.call_args[0][0]
assert "user_id" in request.view_parameters
assert request.view_parameters["user_id"].string_value == "alice"
assert request.view_parameters["user_id"].type_ == {"string_type": {}}

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.

high

In proto-plus, comparing a message object directly to a dictionary using == will evaluate to False because proto-plus does not support direct equality comparison with dicts. To perform this assertion correctly, convert the message to a dictionary first using type(message).to_dict(message).

Suggested change
assert request.view_parameters["user_id"].type_ == {"string_type": {}}
assert type(request.view_parameters["user_id"].type_).to_dict(request.view_parameters["user_id"].type_) == {"string_type": {}}

request = execute_query_mock.call_args[0][0]
assert "user_id" in request.view_parameters
assert request.view_parameters["user_id"].string_value == "alice"
assert request.view_parameters["user_id"].type_ == {"string_type": {}}

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.

high

In proto-plus, comparing a message object directly to a dictionary using == will evaluate to False because proto-plus does not support direct equality comparison with dicts. To perform this assertion correctly, convert the message to a dictionary first using type(message).to_dict(message).

Suggested change
assert request.view_parameters["user_id"].type_ == {"string_type": {}}
assert type(request.view_parameters["user_id"].type_).to_dict(request.view_parameters["user_id"].type_) == {"string_type": {}}

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.

1 participant