Regression in Rows.Scan: incorrect handling of sql.Scanner implementation in v5.7.2

[gazerro]

The Rows.Scan method of the pgx package is documented as follows:

// Scan reads the values from the current row into dest values positionally.
// dest can include pointers to core types, values implementing the Scanner
// interface, and nil. nil will skip the value entirely. It is an error to
// call Scan without first calling Next() and checking that it returned true.

According to the documentation, it accepts as arguments:

1. pointers to core types,
2. values implementing the Scanner interface,
3. nil.

Starting from version v5.7.2, point (2) is no longer true because:

1. It no longer accepts non-pointer values implementing the sql.Scanner interface.
2. It accepts pointer values that do not implement sql.Scanner but point to types that do.

Both changes contradict the method's documentation. In particular:

• The first change breaks existing programs that relied on the documented behavior.
• The second change allows invalid values to pass through, potentially hiding parameter-passing errors.

These issues were introduced in commit 5c9b565. The issue #2146, which this commit intended to resolve, did not need fixing in the first place because the original behavior was correct. The example code in the issue is incorrect. Specifically:

var r Row  
err := rows.Scan(&r.ID, &r.Data)  
...  
type Row struct {  
	ID   int        `json:"id"`  
	Data *Timestamp `json:"data"` // <- this is a pointer  
}  
...  
func (t *Timestamp) Scan(value interface{}) error { ... }  

The second argument to rows.Scan has type **T, but **T does not implement sql.Scanner. Therefore, the Scan method has no Scan implementation to invoke.

The example code should be corrected as follows:

err := rows.Scan(&r.ID, r.Data)  

Here, r.Data must not be passed as a pointer.

As a result, commit 5c9b565 should be reverted to restore the correct behavior, which was consistent with both the documentation and expected usage.

[jackc]

dest can include pointers to core types, values implementing the Scanner interface, and nil

It looks like the docs for Rows.Scan that you mention are somewhat out of date. It looks like those comments were added for v3. They are still true, but there are many more things that Rows.Scan handles now.

It no longer accepts non-pointer values implementing the sql.Scanner interface.

Do you have an example of this?

It accepts pointer values that do not implement sql.Scanner but point to types that do.

It think this matches current database/sql behavior.

As far as **T vs *T in general, pgx supports **T. This allows NULL being scanned into a nil. If not NULL, then pgx allocates a T and follows the normal scanning logic.