`ResultProxy.rowcount` returns -1 regardless of modified rows

[aderk]

https://docs.sqlalchemy.org/en/13/core/connections.html#sqlalchemy.engine.ResultProxy.rowcount

Environment details

• Programming language: Python 3.9
• OS: Debian
• Language runtime version:
• Package version:

Steps to reproduce

from sqlalchemy import Column, MetaData, Table, Text, create_engine

project_id = 'foo'
instance_id = 'bar'
database = 'baz'

spanner_uri = f'spanner:///projects/{project_id}/instances/{instance_id}/databases/{database}'

# Create a table with one `Text`-type column
engine = create_engine(spanner_uri)
metadata = MetaData(bind=engine)
t = Table("TestTable", metadata, Column("test_column", Text, nullable=False))
metadata.create_all(engine)

test_value = 'foo1'

with engine.begin() as conn:
 #conn.execute(t.insert(None).values(test_column=test_value))
 res = conn.execute(t.update(None).where(t.c.test_column == test_value).values(test_column='bar2'))

assert res.rowcount == 1, f"Incorrect rowcount. want 1, got {res.rowcount}"

Actual vs Expected Behavior

res.rowcount returns -1, but should return 1.

[aderk]

Have you started working on this? I could take a stab at it, if you like

[IlyaFaer]

@aderk, well, actually, we didn't implement it earlier, because it looks not very possible. The thing is that rowcount, according to PEP-249, represents a number of rows, produced by the last execute() method call. Under the hood of the execute() method we're in fact only retrieving a StreamedResultSet() object, not the rows themselves. As the name of the object hints, it streams the resulting rows (when you're calling fetch() methods). It means after execute() method call we don't really know, how many resulting rows we'll get, as they are not streamed yet, we only got a pipe to get them. So, preliminary, the existing API doesn't give us an ability to solve the issue (I remember we have comments about it somewhere in the code, at least in Django related Spanner API).

Still, before giving a straight answer to the issue I'd like to take a closer look at all of it once again, to ensure I'm not missing new functionality (like response metadata) or something. Let's say we'll resolve the issue during this week.

[aderk]

Thanks @IlyaFaer! That makes sense. I'll rewrite our application code to not use rowcount. Feel free to close or deprioritize this issue. Cheers

[IlyaFaer]

@larkee, do you have any thoughts here?
I've taken a look at responses to different requests and metadatas, and didn't find a way to provide resulting rows count even for half of our cases.

Should we raise NotImplemented for rowcount property to explain why it's not supported?

[aderk]

Should we raise NotImplemented for rowcount property?

+1

…

On Fri, Sep 24, 2021 at 3:23 AM Ilya Gurov ***@***.***> wrote: @larkee <https://github.com/larkee>, do you have any thoughts here? I've taken a look at responses to different requests and didn't find a way to provide resulting rows count. Should we raise NotImplemented for rowcount property? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#121 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AOZKJVRQBNV5KRGGCAU2NOLUDRGQ3ANCNFSM5EA4L7RA> .

[IlyaFaer]

The PR with related changes merged