Skip to content
This repository was archived by the owner on Apr 1, 2026. It is now read-only.
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions google/cloud/bigtable/data/_async/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -657,6 +657,7 @@ async def execute_query(
DeadlineExceeded,
ServiceUnavailable,
),
column_info: dict[str, Any] | None = None,

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 use something other than Any here?

It seems like the value is only used if it's a Message/EnumTypeWrapper?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done!

) -> "ExecuteQueryIteratorAsync":
"""
Executes an SQL query on an instance.
Expand Down Expand Up @@ -705,6 +706,13 @@ async def execute_query(
If None, defaults to prepare_operation_timeout.
prepare_retryable_errors: a list of errors that will be retried if encountered during prepareQuery.
Defaults to 4 (DeadlineExceeded) and 14 (ServiceUnavailable)
column_info: Dictionary with mappings between column names and additional column information.
An object where column names as keys and custom objects as corresponding
values for deserialization. It's specifically useful for data types like
protobuf where deserialization logic is on user-specific code. When provided,
the custom object enables deserialization of backend-received column data.
If not provided, data remains serialized as bytes for Proto Messages and
integer for Proto Enums.

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 is a bit confusing to me. will users know how to construct this dict? Is there a docs link we can add here or something?

The first and second sentences also seem a bit contradictory, but maybe I'm reading it wrong

@trollyxia trollyxia Sep 5, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I expanded the docstring. Hopefully it makes more sense now. Please take another look thanks!

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.

That looks good to me!

Returns:
ExecuteQueryIteratorAsync: an asynchronous iterator that yields rows returned by the query
Raises:
Expand Down Expand Up @@ -771,6 +779,7 @@ async def execute_query(
attempt_timeout,
operation_timeout,
retryable_excs=retryable_excs,
column_info=column_info,
)

@CrossSync.convert(sync_name="__enter__")
Expand Down
9 changes: 9 additions & 0 deletions google/cloud/bigtable/data/_sync_autogen/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -485,6 +485,7 @@ def execute_query(
DeadlineExceeded,
ServiceUnavailable,
),
column_info: dict[str, Any] | None = None,
) -> "ExecuteQueryIterator":
"""Executes an SQL query on an instance.
Returns an iterator to asynchronously stream back columns from selected rows.
Expand Down Expand Up @@ -532,6 +533,13 @@ def execute_query(
If None, defaults to prepare_operation_timeout.
prepare_retryable_errors: a list of errors that will be retried if encountered during prepareQuery.
Defaults to 4 (DeadlineExceeded) and 14 (ServiceUnavailable)
column_info: Dictionary with mappings between column names and additional column information.
An object where column names as keys and custom objects as corresponding
values for deserialization. It's specifically useful for data types like
protobuf where deserialization logic is on user-specific code. When provided,
the custom object enables deserialization of backend-received column data.
If not provided, data remains serialized as bytes for Proto Messages and
integer for Proto Enums.
Returns:
ExecuteQueryIterator: an asynchronous iterator that yields rows returned by the query
Raises:
Expand Down Expand Up @@ -592,6 +600,7 @@ def execute_query(
attempt_timeout,
operation_timeout,
retryable_excs=retryable_excs,
column_info=column_info,
)

def __enter__(self):
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@ def __init__(
operation_timeout: float,
req_metadata: Sequence[Tuple[str, str]] = (),
retryable_excs: Sequence[type[Exception]] = (),
column_info: Dict[str, Any] | None = None,
) -> None:
"""
Collects responses from ExecuteQuery requests and parses them into QueryResultRows.
Expand All @@ -107,6 +108,8 @@ def __init__(
Failed requests will be retried within the budget
req_metadata: metadata used while sending the gRPC request
retryable_excs: a list of errors that will be retried if encountered.
column_info: dict with mappings between column names and additional column information
for protobuf deserialization.
Raises:
{NO_LOOP}
:class:`ValueError <exceptions.ValueError>` as a safeguard if data is processed in an unexpected state
Expand Down Expand Up @@ -135,6 +138,7 @@ def __init__(
exception_factory=_retry_exception_factory,
)
self._req_metadata = req_metadata
self._column_info = column_info
try:
self._register_instance_task = CrossSync.create_task(
self._client._register_instance,
Expand Down Expand Up @@ -202,7 +206,9 @@ async def _next_impl(self) -> CrossSync.Iterator[QueryResultRow]:
raise ValueError(
"Error parsing response before finalizing metadata"
)
results = self._reader.consume(batches_to_parse, self.metadata)
results = self._reader.consume(
batches_to_parse, self.metadata, self._column_info
)
if results is None:
continue

Expand Down
116 changes: 105 additions & 11 deletions google/cloud/bigtable/data/execute_query/_query_result_parsing_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,12 @@
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
from __future__ import annotations

from typing import Any, Callable, Dict, Type

from google.protobuf.message import Message
from google.protobuf.internal.enum_type_wrapper import EnumTypeWrapper

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.

is this safe to use? (I see internal in there)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

from google.cloud.bigtable.data.execute_query.values import Struct
from google.cloud.bigtable.data.execute_query.metadata import SqlType
from google.cloud.bigtable_v2 import Value as PBValue
Expand All @@ -30,24 +34,36 @@
SqlType.Struct: "array_value",
SqlType.Array: "array_value",
SqlType.Map: "array_value",
SqlType.Proto: "bytes_value",
SqlType.Enum: "int_value",
}


def _parse_array_type(value: PBValue, metadata_type: SqlType.Array) -> Any:
def _parse_array_type(
value: PBValue,
metadata_type: SqlType.Array,
column_name: str | None,
column_info: dict[str, Any] | None = None,
) -> Any:

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.

I know you didn't touch this part, but maybe this should be list[Any]?

"""
used for parsing an array represented as a protobuf to a python list.
"""
return list(
map(
lambda val: _parse_pb_value_to_python_value(
val, metadata_type.element_type
val, metadata_type.element_type, column_name, column_info
),
value.array_value.values,
)
)


def _parse_map_type(value: PBValue, metadata_type: SqlType.Map) -> Any:
def _parse_map_type(
value: PBValue,
metadata_type: SqlType.Map,
column_name: str | None,
column_info: dict[str, Any] | None = None,
) -> Any:

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.

same as above: dict[Any, Any]?

"""
used for parsing a map represented as a protobuf to a python dict.

Expand All @@ -64,10 +80,16 @@ def _parse_map_type(value: PBValue, metadata_type: SqlType.Map) -> Any:
map(
lambda map_entry: (
_parse_pb_value_to_python_value(
map_entry.array_value.values[0], metadata_type.key_type
map_entry.array_value.values[0],
metadata_type.key_type,
f"{column_name}.key" if column_name is not None else None,
column_info,
),
_parse_pb_value_to_python_value(
map_entry.array_value.values[1], metadata_type.value_type
map_entry.array_value.values[1],
metadata_type.value_type,
f"{column_name}.value" if column_name is not None else None,
column_info,
),
),
value.array_value.values,
Expand All @@ -77,7 +99,12 @@ def _parse_map_type(value: PBValue, metadata_type: SqlType.Map) -> Any:
raise ValueError("Invalid map entry - less or more than two values.")


def _parse_struct_type(value: PBValue, metadata_type: SqlType.Struct) -> Struct:
def _parse_struct_type(
value: PBValue,
metadata_type: SqlType.Struct,
column_name: str | None,
column_info: dict[str, Any] | None = None,
) -> Struct:
"""
used for parsing a struct represented as a protobuf to a
google.cloud.bigtable.data.execute_query.Struct
Expand All @@ -88,29 +115,96 @@ def _parse_struct_type(value: PBValue, metadata_type: SqlType.Struct) -> Struct:
struct = Struct()
for value, field in zip(value.array_value.values, metadata_type.fields):
field_name, field_type = field
struct.add_field(field_name, _parse_pb_value_to_python_value(value, field_type))
nested_column_name: str | None
if column_name is None:
nested_column_name = None
else:
# qualify the column name for nested lookups
nested_column_name = (
f"{column_name}.{field_name}" if field_name else column_name
Comment thread
jackdingilian marked this conversation as resolved.
Outdated
)
struct.add_field(
field_name,
_parse_pb_value_to_python_value(
value, field_type, nested_column_name, column_info
),
)

return struct


def _parse_timestamp_type(
value: PBValue, metadata_type: SqlType.Timestamp
value: PBValue,
metadata_type: SqlType.Timestamp,
column_name: str | None,
column_info: dict[str, Any] | None = None,
) -> DatetimeWithNanoseconds:
"""
used for parsing a timestamp represented as a protobuf to DatetimeWithNanoseconds
"""
return DatetimeWithNanoseconds.from_timestamp_pb(value.timestamp_value)


_TYPE_PARSERS: Dict[Type[SqlType.Type], Callable[[PBValue, Any], Any]] = {
def _parse_proto_type(
value: PBValue,
metadata_type: SqlType.Proto,
column_name: str | None,
column_info: dict[str, Any] | None = None,
) -> Message | bytes:
"""
Parses a serialized protobuf message into a Message object.
"""

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.

I think some more context here could be helpful.

It seems like it just falls back to value.bytes_value if parsing fails? Is that an expected state, or does that mean something went wrong?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done!

Yes, that's correct. If column_info is not provided or the provided column_info doesn't contain the required type information, then it is expected that we return value.bytes_value. However, if parsing fails then that does mean something went wrong (i.e., the user provided the wrong type information).

In either case, we fall back to return the bytes. This approach/design mirrors the spanner's implementation in googleapis/python-spanner#1084.

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.

Ok, yeah I was going to question if it would be better to raise an exception on parse failure, but that could go either way. If spanner does it this way, it's probably good to follow suit

if (
column_name is not None
and column_info is not None
and column_info.get(column_name) is not None
):
default_proto_message = column_info.get(column_name)
if isinstance(default_proto_message, Message):

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.

I wonder of all of these nested conditionals could be simplified by using exception handling instead? (i.e, just assume everything exists, and return value.bytes_value if a parsing exception is thrown)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I tried to refactor the code using a try-except block as you described, but this got me into several mypy errors as below:

google/cloud/bigtable/data/execute_query/_query_result_parsing_utils.py:175: error: Item "None" of "Optional[Dict[str, Union[Message, EnumTypeWrapper]]]" has no attribute "get"  [union-attr]
google/cloud/bigtable/data/execute_query/_query_result_parsing_utils.py:175: error: Argument 1 to "get" of "dict" has incompatible type "Optional[str]"; expected "str"  [arg-type]
google/cloud/bigtable/data/execute_query/_query_result_parsing_utils.py:176: error: Missing positional argument "enum_type" in call to "EnumTypeWrapper"  [call-arg]
google/cloud/bigtable/data/execute_query/_query_result_parsing_utils.py:176: error: Cannot instantiate type "Type[None]"  [misc]
google/cloud/bigtable/data/execute_query/_query_result_parsing_utils.py:177: error: Item "EnumTypeWrapper" of "Union[Message, EnumTypeWrapper, Any]" has no attribute "ParseFromString"  [union-attr]
google/cloud/bigtable/data/execute_query/_query_result_parsing_utils.py:178: error: Incompatible return value type (got "Union[Message, EnumTypeWrapper, Any]", expected "Union[Message, bytes]")  [return-value]

I'm not super familiar with Python language, so I'd appreciate any further advice/suggestion you could provide!

@daniel-sanche daniel-sanche Sep 5, 2025

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.

Hmm, you could resolve that using typing.cast, to tell mypy to assume it's a specific type:

try:
    proto_enum = cast(dict[str, EnumTypeWrapper], column_info).get(column_name)
    return cast(EnumTypeWrapper, proto_enum).Name(value.int_value)
except AttributeError:
    return value.int_value

That said, you might still have to do a None check for column_name, because passing .get(None) doesn't seem to raise an exception

This might be getting into "overly clever" territory though, the original if statements worked fine too. Exception handling was something that came to mind to consider as I was reviewing, but maybe it's not worth it here

proto_message = type(default_proto_message)()
proto_message.ParseFromString(value.bytes_value)
return proto_message
return value.bytes_value


def _parse_enum_type(
value: PBValue,
metadata_type: SqlType.Enum,
column_name: str | None,
column_info: dict[str, Any] | None = None,
) -> int | Any:

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.

does this have to be Any?

I looked up EnumTypeWrapper.Name, and it seems like it's supposed to be a string?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good catch. Done!

"""
Parses an integer value into a Protobuf enum.
"""
if (
column_name is not None
and column_info is not None
and column_info.get(column_name) is not None
):
proto_enum = column_info.get(column_name)
if isinstance(proto_enum, EnumTypeWrapper):
return proto_enum.Name(value.int_value)
return value.int_value


_TYPE_PARSERS: Dict[
Type[SqlType.Type], Callable[[PBValue, Any, str | None, dict[str, Any] | None], Any]

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.

nit: it might be good to make a type alias for Callable[[PBValue, Any, str | None, dict[str, Any] | None], Any]. Maybe call it ParserCallable or something?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The usage of TypeAlias seems to break several checks :(

Could you please let me know what I've done wrong?

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.

Oh right, I don't think all versions we support work with TypeAlias

I think you can just make it implicit by removing the TypeAlias annotation though:

ParserCallable = Callable[
    [PBValue, Any, Optional[str], Optional[Dict[str, Union[Message, EnumTypeWrapper]]]],
    Any,
]

That seems to be what we did for other aliases in this library. Let me know if that causes any issues

] = {
SqlType.Timestamp: _parse_timestamp_type,
SqlType.Struct: _parse_struct_type,
SqlType.Array: _parse_array_type,
SqlType.Map: _parse_map_type,
SqlType.Proto: _parse_proto_type,
SqlType.Enum: _parse_enum_type,
}


def _parse_pb_value_to_python_value(value: PBValue, metadata_type: SqlType.Type) -> Any:
def _parse_pb_value_to_python_value(
value: PBValue,
metadata_type: SqlType.Type,
column_name: str | None,
column_info: dict[str, Any] | None = None,
) -> Any:
"""
used for converting the value represented as a protobufs to a python object.
"""
Expand All @@ -126,7 +220,7 @@ def _parse_pb_value_to_python_value(value: PBValue, metadata_type: SqlType.Type)

if kind in _TYPE_PARSERS:
parser = _TYPE_PARSERS[kind]
return parser(value, metadata_type)
return parser(value, metadata_type, column_name, column_info)
elif kind in _REQUIRED_PROTO_FIELDS:
field_name = _REQUIRED_PROTO_FIELDS[kind]
return getattr(value, field_name)
Expand Down
29 changes: 24 additions & 5 deletions google/cloud/bigtable/data/execute_query/_reader.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,10 @@
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
from __future__ import annotations

from typing import (
Any,
List,
TypeVar,
Generic,
Expand Down Expand Up @@ -54,7 +56,10 @@ class _Reader(ABC, Generic[T]):

@abstractmethod
def consume(
self, batches_to_consume: List[bytes], metadata: Metadata
self,
batches_to_consume: List[bytes],
metadata: Metadata,
column_info: dict[str, Any] | None = None,
) -> Optional[Iterable[T]]:
"""This method receives a list of batches of bytes to be parsed as ProtoRows messages.
It then uses the metadata to group the values in the parsed messages into rows. Returns
Expand All @@ -64,6 +69,8 @@ def consume(
:meth:`google.cloud.bigtable.byte_cursor._ByteCursor.consume`
method.
metadata: metadata used to transform values to rows
column_info: (Optional) dict with mappings between column names and additional column information
for protobuf deserialization.

Returns:
Iterable[T] or None: Iterable if gathered values can form one or more instances of T,
Expand All @@ -89,7 +96,10 @@ def _parse_proto_rows(self, bytes_to_parse: bytes) -> Iterable[PBValue]:
return proto_rows.values

def _construct_query_result_row(
self, values: Sequence[PBValue], metadata: Metadata
self,
values: Sequence[PBValue],
metadata: Metadata,
column_info: dict[str, Any] | None = None,
) -> QueryResultRow:
result = QueryResultRow()
columns = metadata.columns
Expand All @@ -99,20 +109,29 @@ def _construct_query_result_row(
), "This function should be called only when count of values matches count of columns."

for column, value in zip(columns, values):
parsed_value = _parse_pb_value_to_python_value(value, column.column_type)
parsed_value = _parse_pb_value_to_python_value(
value, column.column_type, column.column_name, column_info
)
result.add_field(column.column_name, parsed_value)
return result

def consume(
self, batches_to_consume: List[bytes], metadata: Metadata
self,
batches_to_consume: List[bytes],
metadata: Metadata,
column_info: dict[str, Any] | None = None,
) -> Optional[Iterable[QueryResultRow]]:
num_columns = len(metadata.columns)
rows = []
for batch_bytes in batches_to_consume:
values = self._parse_proto_rows(batch_bytes)
for row_data in batched(values, n=num_columns):
if len(row_data) == num_columns:
rows.append(self._construct_query_result_row(row_data, metadata))
rows.append(
self._construct_query_result_row(
row_data, metadata, column_info
)
)
else:
raise ValueError(
"Unexpected error, recieved bad number of values. "
Expand Down
Loading
Loading