Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

proposal: database/sql: Support for generics with NullTypes Interface #56439

Closed
timdadd opened this issue Oct 26, 2022 · 9 comments
Closed

proposal: database/sql: Support for generics with NullTypes Interface #56439

timdadd opened this issue Oct 26, 2022 · 9 comments

Comments

@timdadd
Copy link

timdadd commented Oct 26, 2022

To support handling of generics, add an interface that includes all the null types
type NullTypes interface { NullString | NullInt64 | NullTime | NullBool ... }

@gopherbot gopherbot added this to the Proposal milestone Oct 26, 2022
@seankhliao
Copy link
Member

what does the interface enable? why can't you define the interface in the code that's using it?

@seankhliao seankhliao added the WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. label Oct 27, 2022
@seankhliao seankhliao changed the title proposal: affected/package: sql Support for generics with NullTypes Interface proposal: database/sql: Support for generics with NullTypes Interface Oct 27, 2022
@rsc
Copy link
Contributor

rsc commented Nov 9, 2022

This proposal has been added to the active column of the proposals project
and will now be reviewed at the weekly proposal review meetings.
— rsc for the proposal review group

@joedian joedian removed the WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. label Nov 15, 2022
@rsc
Copy link
Contributor

rsc commented Nov 16, 2022

It's still unclear what this would enable us to do.

@timdadd
Copy link
Author

timdadd commented Nov 16, 2022

The only reason for adding this is because the sql package defines all the null types. As it's generic code in the software using the SQL package, it would be better if the interface is defined in the SQL package because if the SQL package adds another type in the future then the generic will not work unless the interface defined in the software using the SQL package updates their interface.

So if someone doesn't want to support all the types in their generic code then they define their own interface. But if, as I have, the written code works with all types then it would be nice, if a future release of the sql package added the new type to a published interface instead of the current risk that something breaks.

@rsc
Copy link
Contributor

rsc commented Nov 30, 2022

@timdadd I don't understand how you would use the interface. Do you have concrete code you can share that would make use of this interface?

@timdadd
Copy link
Author

timdadd commented Nov 30, 2022

@rsc I have to unmarshall json messages that have an unknown structure but follow the sql.null format because they were marshalled from SQL structs. Being SQL null they are of the format xxx/valid. I want to save writing code for each SQL Null type, so instead I build the functions using a single generic function.

Here is my generic function that builds 3 real functions

// interfaceToType takes a json Interface and unmarshalls to the SQL type given
func interfaceToType[V sql.NullString | sql.NullInt64 | sql.NullTime](m interface{}, t V) (res V, err error) {
	var buf []byte
	if buf, err = json.Marshal(m); err != nil {
		return res, logErrF("couldn't marshall the map %w:%v", m, err)
	}
	logf("interfaceToType", string(buf))
	if err = json.Unmarshal(buf, &res); err != nil {
		return res, logErrF("couldn't unmarshall the map %s:%v", buf, err)
	}
	logf("interfaceToType", res)
	return
}

The calling code is like this:

	if an.DisplayName, err = interfaceToType(attrNameMap["display_name"], an.DisplayName); err != nil {
		return
	}

If an interface exists then I can use instead of [] and then if you add a new type then my code just needs recompilation.

Here's a bit of JSON I have to unmarshall, but I can't use a struct because the JSON structure is inconsistent:

			"attr_type": "T",
			"attr_type_format_id": {
				"Int64": 0,
				"Valid": false
			},
			"format_value": "",
			"import": false,
			"optional": false,
			"normalise": true,
			"display_name": {
				"String": "",
				"Valid": true
			},

Of course you might have a much better way of doing all this, if so then please let me know.

@rsc
Copy link
Contributor

rsc commented Dec 7, 2022

I don't see anything in interfaceToType that is specific to SQL. It seems like a shorter function would be:

func unmarshal[T any](m interface{}) (res T, err error) {
	var buf []byte
	if buf, err = json.Marshal(m); err != nil {
		return res, logErrF("couldn't marshall the map %w:%v", m, err)
	}
	logf("interfaceToType", string(buf))
	if err = json.Unmarshal(buf, &res); err != nil {
		return res, logErrF("couldn't unmarshall the map %s:%v", buf, err)
	}
	logf("interfaceToType", res)
	return
}

and then used as

	if an.DisplayName, err = unmarshal[sql.NullString](attrNameMap["display_name"]); err != nil {
		return
	}

This wouldn't need the null interface constraint at all.

@rsc
Copy link
Contributor

rsc commented Dec 14, 2022

Based on the discussion above, this proposal seems like a likely decline.
— rsc for the proposal review group

@rsc
Copy link
Contributor

rsc commented Dec 21, 2022

No change in consensus, so declined.
— rsc for the proposal review group

@rsc rsc closed this as completed Dec 21, 2022
@golang golang locked and limited conversation to collaborators Dec 21, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants