Skip to content

proposal: database/sql: ScannerContext, ValuerContext support #40511

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

Closed
jinzhu opened this issue Jul 31, 2020 · 9 comments
Closed

proposal: database/sql: ScannerContext, ValuerContext support #40511

jinzhu opened this issue Jul 31, 2020 · 9 comments

Comments

@jinzhu
Copy link
Contributor

jinzhu commented Jul 31, 2020

Currently when using Scanner, Valuer interface with database/sql, it can't behave differently based on the current context

For example, in a multi-tenant system, each tenant has a different encryption key and would like to save the encrypted value and retrieve the decrypted value automatically.

If we can support ScannerContext and ValuerContext interface in the package database/sql, it would be much easier to implement those cases.

// option 1
type ScannerContext interface {
  Scan(ctx context.Context, src interface{}) error
}

type ValuerContext interface {
	Value(ctx context.Context) (Value, error)
}

// option 2
type ScannerContext interface {
  ScanContext(ctx context.Context, src interface{}) error
}

type ValuerContext interface {
	ValueContext(ctx context.Context) (Value, error)
}
@gopherbot gopherbot added this to the Proposal milestone Jul 31, 2020
@ianlancetaylor
Copy link
Member

CC @kardianos

@kardianos
Copy link
Contributor

@jinzhu I strongly suspect that this isn't going to fly. But as of yet I don't see a complete example. Perhaps you can flush it out a bit with how you would call this so I can fully understand what you are trying to do.

@jinzhu
Copy link
Contributor Author

jinzhu commented Aug 11, 2020

Hello @kardianos

Thank you for your reply, we have many applications have similar requirements.

The application need to save multiple fields of some structs as encrypted data into the database, for each organization, it has an encryption key, we will store the encryption key into context for coming API requests.

There are many services using the data have to encrypt/decrypt those fields one by one everywhere, it is easy to make a mistake when processing the data, e.g, double or forget to encrypt a field that corrupted the data, which will make the application less maintainable.

If Go supports ScannerContext and ValuerContext, we can move the encrypt/decrypt logic to the interface to avoid those issues, which helps deliver a more maintainable product without breaking the go1 compatibility.

@kardianos
Copy link
Contributor

@jinzhu Thank you for the description. Can you write some code as you would envision using this? As before, I doubt this proposal will be acceptable, so set expectations accordingly.

@jinzhu
Copy link
Contributor Author

jinzhu commented Aug 12, 2020

@kardianos If it is supported, then we can write our application like this:

type User struct {
	ID      string
	Name    string
	Email   EncryptedString
	Phone   EncryptedString
	Address EncryptedString
}

type Message struct {
	ID   uint
	From uint
	To   uint
	Body EncryptedString
}

type EncryptedString struct {
	encryptedValue string
	decryptedValue string
}

func (es *EncryptedString) ScanContext(ctx context.Context, src interface{}) (err error) {
	if encryptionKey, ok := ctx.Value("TenantEncryptionKey").(string); ok {
		ns := sql.NullString{}
		if err = ns.Scan(src); err == nil && ns.String != "" {
			es.decryptedValue, err = Decrypt(ns.String, encryptionKey)
		}
	} else {
		return errors.New("invalid encryption key")
	}
	return err
}

func (es EncryptedString) ValueContext(ctx context.Context) (_ driver.Value, err error) {
	if encryptionKey, ok := ctx.Value("TenantEncryptionKey").(string); ok {
		if es.encryptedValue != "" {
			return es.encryptedValue, nil
		}

		if es.decryptedValue != "" {
			es.encryptedValue, err = Encrypt(es.decryptedValue, encryptionKey)
		}
	} else {
		return nil, errors.New("invalid encryption key")
	}
	return es.encryptedValue, err
}

func (es *EncryptedString) UnmarshalJSON(b []byte) error {
	json.Unmarshal(b, &es.decryptedValue)
	return nil
}

func (es *EncryptedString) MarshalJSON() ([]byte, error) {
	return []byte(es.decryptedValue), nil
}

and remove the encrypt/decrypt logic which scattered everywhere right now

@jinzhu
Copy link
Contributor Author

jinzhu commented Aug 26, 2020

Hello @kardianos

Any plan, can I start to implement this feature?

@kardianos
Copy link
Contributor

I don't feel great about this feature request. While the context is usually available in a Scan, it isn't obvious what it is used by.

You can probably achieve a similar result by creating a helper scan stub:

type UpdateWithContext interface {
    Update(ctx context.Context) error
}
func MyAppScan(ctx context.Context, scanner Scanner, values ...interface{}) error {
    err := scanner.Scan(values...)
    if err != nil {
        return err
    }
    for _, v := range values {
        if ok, v := v.(UpdateWithContext); ok {
            err = v.Update(ctx)
            if err != nil {
                return err
            }
        }
    }
    return nil
}

I would like to have a story for this in a v2 with better custom scanning / marshaling, but not in v1.

@rsc
Copy link
Contributor

rsc commented Sep 2, 2020

Based on the discussion above, this seems like a likely decline.

@rsc
Copy link
Contributor

rsc commented Sep 16, 2020

No change in consensus, so declined.

@rsc rsc closed this as completed Sep 16, 2020
@golang golang locked and limited conversation to collaborators Sep 16, 2021
@rsc rsc moved this to Declined in Proposals Aug 10, 2022
@rsc rsc added this to Proposals Aug 10, 2022
@rsc rsc removed this from Proposals Oct 19, 2022
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