Commit process of UPDATE clause fails

[hyagi96]

If you try to commit an UPDATE (and DELETE) clause for an existing row, you will get an error.
This issue(#121) may be relevant.
A minimal example is shown below.

Environment details

• Programming language: Python
• OS: Ubuntu 20.04, macOS 12.0.1
• Language runtime version: 3.9.9
• Package version:
  • sqlalchemy-spanner 1.0.0
  • SQLAlchemy 1.23.1
    • further information can be found in: https://github.com/hyagi96/spanner-minimum-case

Steps to reproduce

1. clone above repository and run init.sh, create virtualenv using pipenv.
2. $ pipenv shell to enter virtualenv.
3. $ cp .env.example .env and $ vi .env to add environment variables such as project ID, spanner instance ID, database ID, and credential json file path.
   You need to create a Spanner instance beforehand and then create a database using Google standard SQL.
4. $ ./run.py .
   The first time you run it, it will only perform INSERT, so no error will occur.
5. $ ./run.py again.
   An error occurs in session.commit to existing row (because rowcount is not implemented).

cf. Error log when running run.py for the second time in my environment

/[current_dir]/.venv/lib/python3.9/site-packages/sqlalchemy/engine/default.py:1237: UserWarning: The `rowcount` property is non-operational. Request resulting rows are streamed by the `fetch*()` methods and can't be counted before they are all streamed.
  return self.cursor.rowcount
Traceback (most recent call last):
  File "/[current_dir]/./run.py", line 55, in <module>
    session.commit()
  File "/[current_dir]/.venv/lib/python3.9/site-packages/sqlalchemy/orm/session.py", line 1046, in commit
    self.transaction.commit()
  File "/[current_dir]/.venv/lib/python3.9/site-packages/sqlalchemy/orm/session.py", line 504, in commit
    self._prepare_impl()
  File "/[current_dir]/.venv/lib/python3.9/site-packages/sqlalchemy/orm/session.py", line 483, in _prepare_impl
    self.session.flush()
  File "/[current_dir]/.venv/lib/python3.9/site-packages/sqlalchemy/orm/session.py", line 2540, in flush
    self._flush(objects)
  File "/[current_dir]/.venv/lib/python3.9/site-packages/sqlalchemy/orm/session.py", line 2682, in _flush
    transaction.rollback(_capture_exception=True)
  File "/[current_dir]/.venv/lib/python3.9/site-packages/sqlalchemy/util/langhelpers.py", line 68, in __exit__
    compat.raise_(
  File "/[current_dir]/.venv/lib/python3.9/site-packages/sqlalchemy/util/compat.py", line 182, in raise_
    raise exception
  File "/[current_dir]/.venv/lib/python3.9/site-packages/sqlalchemy/orm/session.py", line 2642, in _flush
    flush_context.execute()
  File "/[current_dir]/.venv/lib/python3.9/site-packages/sqlalchemy/orm/unitofwork.py", line 422, in execute
    rec.execute(self)
  File "/[current_dir]/.venv/lib/python3.9/site-packages/sqlalchemy/orm/unitofwork.py", line 586, in execute
    persistence.save_obj(
  File "/[current_dir]/.venv/lib/python3.9/site-packages/sqlalchemy/orm/persistence.py", line 230, in save_obj
    _emit_update_statements(
  File "/[current_dir]/.venv/lib/python3.9/site-packages/sqlalchemy/orm/persistence.py", line 998, in _emit_update_statements
    rows += c.rowcount
TypeError: unsupported operand type(s) for +=: 'int' and 'NoneType'

[skuruppu]

@IlyaFaer if you can please take a look today, that would be awesome.

[IlyaFaer]

The thing is that we don't support rowcount - we're raising a warning in case one is trying to use it (see for more details).

Still, it looks like at some points SQLAlchemy ignores the fact we've set rowcount feature as unsupported in the dialect requirements and uses it.

The problem is that this warning can be shown in cases like this one - user executes an update, they don't even know rowcount is used under the hood, but the warning will show up. It can be confusing.

Maybe the best solution will be just to use the property docstrings to explain it's not valid, and always return -1.

[hyagi96]

Thank you for your quick response, analysis and confirming my assumption.

I've also investigated the issue in detail in other repositories, and noticed that there is a similar issue(googleapis/python-spanner-django#740) in another repository for Django.
I've further traced the cause of this and found that, as shown in the above issue, this commit (googleapis/python-spanner@b5a567f1db) that incorporated in version 3.12.0.
The direct cause of this problem is that this commit (8762802182a3319c16b6456bb208d8) has been changed to not return values.
In fact, I commented out warnings.warn in the relevant part (Line 137) of [project_dir]/.venv/lib/python3.9/site-packages/google/cloud/spanner_dbapi/cursor.py and did return -1, and the problem was solved.
The same result was also obtained by using v3.11.1.

In summary, the cause is the degradation of python-spanner (PyPI: google-cloud-spanner) v3.12.0.
I'm going to apply a workaround that explicitly uses v3.11.1 until this issue is resolved.

[IlyaFaer]

The PR with a fix for this problem is merged