proposal: database/sql: export convertAssign as DefaultConvertAssign

[moznion]

As discussed in #24258 and #35697, but I think it would be nicer to have the exposed sql.convertAssign() function.
In the previous suggestion, @kardianos said at #24258 (comment):

I don't want the extra burden of maintaining convertAssign as a public API. I'm going to ask you either continue using the type assert or copy the one function out of database/sql and check on it every couple of years to see if you need to update it.

and @rsc also said as well at #35697 (comment), i.e. they suggested doing copy the sql.convertAssign() to the user's project.
At the date of the first suggestion, Apr 20, 2018, copying was a good solution because that function hadn't accessed the unexposed value and functions; ref: https://github.com/golang/go/blob/da24c95ce09668a0d977c208e8e610a21b98b019/src/database/sql/convert.go
However, that function touches the unexported items in the current implementation; for example,

go/src/database/sql/convert.go

Lines 319 to 347 in 9ac6b00

	case driver.Rows:
	switch d := dest.(type) {
	case *Rows:
	if d == nil {
	return errNilPtr
	}
	if rows == nil {
	return errors.New("invalid context to convert cursor rows, missing parent *Rows")
	}
	rows.closemu.Lock()
	*d = Rows{
	dc: rows.dc,
	releaseConn: func(error) {},
	rowsi: s,
	}
	// Chain the cancel function.
	parentCancel := rows.cancel
	rows.cancel = func() {
	// When Rows.cancel is called, the closemu will be locked as well.
	// So we can access rs.lasterr.
	d.close(rows.lasterr)
	if parentCancel != nil {
	parentCancel()
	}
	}
	rows.closemu.Unlock()
	return nil
	}
	}

I suppose this implies users have not been able to copy this function simply so copying would be no longer an effective way to satisfy the demand. Can we have a chance to reboot exposure for this sql.convertAssign() function?

[moznion]

FYI: I would like to use that function in my project and we've tried to copy'n'paste that, but we faced such issue in copying.
moznion/go-optional#32

[ianlancetaylor]

CC @kardianos

[ianlancetaylor]

What are the types that you want to pass to the proposed function?

[moznion]

@ianlancetaylor actually, the type would be any; like https://github.com/moznion/go-optional/blob/c95f59ccbec4a6dab276f0ad80349cf989f1907e/sql_driver.go#L15-L19

[ianlancetaylor]

I mean, what would the dynamic type be? What type would be stored in the interface?

What I'm trying to get at is: when would people want to use this? Do you have an example of code that is currently copying it? Thanks.

[moznion]

Actually, this is my use case but I'm developing the optional type library for golang and it would be nicer to implement database/sql/driver.Valuer and database/sql.Scanner to map the database nullable values to that optional types. ref: moznion/go-optional#27.
And then, a contributor suggested me it would be good to use sql.convertAssign on Scan to map the value passed by database to the Golang's type. The following patch is what we did, we tried to copy that function but it was hard for us as I mentioned: https://github.com/moznion/go-optional/pull/32/files
Now this implementation was reverted because of imperfect copied code due to the unexposed values, but if I could get a chance I would like to use the officially exposed function instead.

[majewsky]

I ran into the same problem. The addition of omitzero in encoding/json for 1.24 made me excited about finally implementing a proper Option type that hides its internals in a struct. But of course, I saw the same issue that @moznion described above, and so I also ended up going with the go:linkname hack.

The omission of sql.DefaultConvertAssign() or something to that effect, as a tool to implement sql.Scanner on generic types, feels particularly weird since the inverse already exists in std: The fallback behavior for driver.Valuer can be accessed through driver.DefaultParameterConverter. So going from custom data types to SQL values does not require any hacks, only going the other direction does.

[ianlancetaylor]

I think it would help if somebody could write the documentation for the proposed function. The current documentation for the unexported function is pretty short and does not explain why anybody would want to use the function. Thanks.

[majewsky]

I looked at where convertAssign is used throughout the database/sql package, and found that sql.Null.Scan() uses it pretty much directly, so I think we can go through this route instead.

@moznion moznion/go-optional#44