[francoisjehl]

Context

On DDL like ALTER TABLE ADD COLUMN, SingleStore reports the table's row count as affected rows.
Issue is, this is 32 bits in ADO.NET and thus once a table passes 2^31 rows, it throws OverflowException.
We faced that at Pigment on a 4B row table.

Implementation

• Let's return (int) Math.Min(recordsAffected, int.MaxValue). This keeps the int contract of DbDataReader.RecordsAffected, saturates instead of throwing, and documents the cap in the XML remarks.
• A simple test that reproduces our OverflowError

[cursor]

PR Summary

Low Risk
Narrow behavior change on an edge case (overflow); normal row counts are unchanged and the fix prevents failures on large DDL.

Overview
RecordsAffected no longer throws when the server reports more than int.MaxValue affected rows (e.g. DDL such as ALTER TABLE on very large tables, where SingleStore can return the table row count). The reader now saturates at int.MaxValue via Math.Min instead of checked cast, and the XML remarks document that cap.

A regression test runs ExecuteNonQuery against a fake server OK packet with 4 billion rows affected and expects int.MaxValue. The fake server handler was extended to emit that response for the test SQL.

^{Reviewed by Cursor Bugbot for commit 4576eb6. Configure here.}

[okramarenko]

thanks for the fix!

one thing to keep in mind is that previously the OverflowException made the caller aware that the count didn't fit into int, now there's no way a caller who gets int.MaxValue can't tell whether they actually did get int.MaxValue or more.

that's why I think it makes sense to make RealRecordsAffected public with internal set and add a mention of it in the remarks, so the users know where to look for it.

got this idea from Npsql connector -- https://github.com/npgsql/npgsql/blob/af6676e70ea51699df13b9f97db8f02bb9b6f629/src/Npgsql/NpgsqlDataReader.cs#L885-L905

[francoisjehl]

You're welcome and good call!

[okramarenko]

nit: this duplicates WriteOk except for the value of affected-rows field. an optional ulong affectedRows = 0 parameter in WriteOk might be a better choice to avoid duplications

[francoisjehl]

WriteOk is passed as a method group in several places, so an optional parameter didn't want to compile.
Thus I thought an overload would do the job? You tell me.

[okramarenko]

good point! optional parameters aren't considered in method group conversion, so an overload
is the right call. lets have WriteOk(BinaryWriter) delgating to new WriteOk(BinaryWriter, ulong affectedRows)

[okramarenko]

thanks for updating these!

one detail though: ExecuteNonQuery() creates and disposes the reader internally, so users can't access RealRecordsAffected. could we add this info here and in ExecuteNonQueryAsync()? smth like:

If the number of affected rows exceeds int.MaxValue, the return value is int.MaxValue. To obtain the exact count, use ExecuteReader(), call SingleStoreDataReader.NextResult until it returns false, then read SingleStoreDataReader.RealRecordsAffected

for the async version we should use ExecuteReaderAsync() and NextResultAsync()