Document behavior of INSERT with RETURNING through tests

[mlafeldt]

This is essentially a port of duckdb/duckdb-rs#597 that I created after noticing some differences in behavior.

Note that the following patch would bring the Go client more in line with the Rust client:

diff --git duckdb_test.go duckdb_test.go
index 8e3f3be..d5b481b 100644
--- duckdb_test.go
+++ duckdb_test.go
@@ -352,7 +352,7 @@ func TestQuery(t *testing.T) {
 		require.NoError(t, err)
 		ra, err := res.RowsAffected()
 		require.NoError(t, err)
-		require.Equal(t, int64(100000), ra)
+		require.Equal(t, int64(0), ra)
 
 		r, err := db.Query(`SELECT i FROM integers ORDER BY i ASC`)
 		require.NoError(t, err)
diff --git statement.go statement.go
index 0834667..93b01ee 100644
--- statement.go
+++ statement.go
@@ -466,7 +466,7 @@ func (s *Stmt) ExecContext(ctx context.Context, nargs []driver.NamedValue) (driv
 	}
 	defer mapping.DestroyResult(res)
 
-	ra := mapping.ValueInt64(res, 0, 0)
+	ra := int64(mapping.RowsChanged(res))
 	return &result{ra}, nil
 }
 
@@ -490,7 +490,7 @@ func (s *Stmt) ExecBound(ctx context.Context) (driver.Result, error) {
 	}
 	defer mapping.DestroyResult(res)
 
-	ra := mapping.ValueInt64(res, 0, 0)
+	ra := int64(mapping.RowsChanged(res))
 	return &result{ra}, nil
 }
 
diff --git statement_test.go statement_test.go
index 8007924..2777245 100644
--- statement_test.go
+++ statement_test.go
@@ -889,13 +889,12 @@ func TestInsertWithReturningClause(t *testing.T) {
 	require.NoError(t, err)
 	require.Equal(t, int64(1), changes)
 
-	// INSERT with RETURNING using Exec
-	// FIXME: RowsAffected returns ID value, not affected rows
+	// INSERT with RETURNING using Exec - RowsAffected is 0 (known limitation)
 	res, err = db.Exec("INSERT INTO location (name) VALUES (?) RETURNING id", "test2")
 	require.NoError(t, err)
 	changes, err = res.RowsAffected()
 	require.NoError(t, err)
-	require.Equal(t, int64(2), changes)
+	require.Equal(t, int64(0), changes)
 
 	// Verify both rows were inserted
 	var count int64

[taniabogatsch]

I like the proposed change, I think it is in line with moving away from deprecated C API functionality, also:

/*!
**DEPRECATION NOTICE**: This method is scheduled for removal in a future release.

* @return The int64_t value at the specified location, or 0 if the value cannot be converted.
*/
DUCKDB_C_API int64_t duckdb_value_int64(duckdb_result *result, idx_t col, idx_t row);

However, would this be a breaking change, or rather just a bug fix, and the previous behavior was wrong? If it is a breaking change, then we might want to document it carefully.

[mlafeldt]

However, would this be a breaking change, or rather just a bug fix, and the previous behavior was wrong? If it is a breaking change, then we might want to document it carefully.

A bugfix IMO. Returning the first value as "rows affected" has always been wrong.

[taniabogatsch]

Yes, I agree. :) Let's add the changes.

[mlafeldt]

Yes, I agree. :) Let's add the changes.

Yeah, will do in a follow-up PR. Can we merge this one now?