googleapis / googleapis/google-cloud-python

Issue: Inconsistency in `Transaction.execute_update` Documentation Regarding `param_types`

オープン
#15,902 コメント 1 件 リアクション 2 件 担当者 1 名 @larkee が担当を希望しています GitHub で見る
api: spanner
主要言語
Python
スター
5.4k
フォーク
1.8k
平均マージ
3日 4時間
マージ済み PR(30日)
122

説明

#### Overview

While reading through the `Transaction` class documentation, I noticed a potential inconsistency or missing implementation regarding the `param_types` argument in the `execute_update` method.

The documentation states the following about param_types:
```py
(Optional) maps explicit types for one or more param values;
required if parameters are passed.
```

Additionally, the docstring for the helper method `_make_params_pb`, which is used by `execute_update`, provides this information:

```py
"""Helper for :meth:`execute_update`.

:type params: dict, {str -> column value}
:param params: values for parameter replacement. Keys must match
the names used in ``dml``.

:type param_types: dict[str -> Union[dict, .types.Type]]
:param param_types:
(Optional) maps explicit types for one or more param values;
required if parameters are passed.

:rtype: Union[None, :class:`Struct`]
:returns: a struct message for the passed params, or None
:raises ValueError:
If ``param_types`` is None but ``params`` is not None.
:raises ValueError:
If ``params`` is None but ``param_types`` is not None.
"""
```

Contrary to the documentation, the` _make_params_pb` method does not appear to include any conditions to raise a ValueError. Furthermore, when I run the following code without explicitly providing `param_types`, no error is raised, and the update works as expected.

#### Question
Is the documentation incorrect in stating that `param_types` is required when params are passed, or is the implementation missing the `ValueError` raises described in the docstring?

#### Environment details

- OS type and version: macOS Sequoia 15.2
- Python version: `3.11.9`
- pip version: `24.0`
- `google-cloud-spanner` version: `3.51.0`

#### Steps to reproduce
1. Use the `Transaction.execute_update` method with the following parameters:
- @new_val = 1 and @table_id = 1.
- Do not include `param_types`.

2. Observe that no error is raised, despite the documentation indicating that `param_types` is required when `params` are provided.

#### Code example

```python
transaction.execute_update(
"""
UPDATE Table
SET Col = @new_val
WHERE TableId = @table_id
""",
params={
"new_val": new_val,
"table_id": table_id
},
)
```

#### Expected behavior
According to the documentation, param_types is required when params are passed. If param_types is not provided, a ValueError should be raised.

#### Observed behavior
No error is raised, and the query executes successfully.

#### Stack trace

No error or exception is raised, so no stack trace is available.

### Suggestions

**1. Add a `ValueError` in the implementation:**
Update the `_make_params_pb` method to raise a `ValueError` if `param_types` is `None` while params is not `None`. For example:
```py
@staticmethod
def _make_params_pb(params, param_types):
"""Helper for :meth:`execute_update`.

:type params: dict, {str -> column value}
:param params: values for parameter replacement. Keys must match
the names used in ``dml``.

:type param_types: dict[str -> Union[dict, .types.Type]]
:param param_types:
(Optional) maps explicit types for one or more param values;
required if parameters are passed.

:rtype: Union[None, :class:`Struct`]
:returns: a struct message for the passed params, or None
:raises ValueError:
If ``param_types`` is None but ``params`` is not None.
:raises ValueError:
If ``params`` is None but ``param_types`` is not None.
"""
if (params is not None and param_types is None):
raise ValueError("Both `params` and `param_types` must be provided together.")
if params:
return Struct(
fields={key: _make_value_pb(value) for key, value in params.items()}
)

return {}
```

**Update the documentation to reflect the actual behavior**
Modify the docstring in `_make_params_pb` and the `Transaction.execute_update` method to clarify that `param_types` is optional, even when params are passed, and that no error will be raised if only one of them is provided. For example:
```py
(Optional) maps explicit types for one or more param values.
This argument is optional, even when parameters are provided.
```

コントリビューションガイド

コントリビューションガイドを開く

評価

この issue はまだ評価されていません。

新しい issue をメールで受け取る

初心者向けの GitHub issue を短くまとめたダイジェスト。