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

rowexec: implement deep copies for EncDatumRow #43182

Closed
asubiotto opened this issue Dec 16, 2019 · 1 comment
Closed

rowexec: implement deep copies for EncDatumRow #43182

asubiotto opened this issue Dec 16, 2019 · 1 comment

Comments

@asubiotto
Copy link
Contributor

Throughout the row execution engine, there are interfaces that specify that a row may only be reused until the next call to a specific interface method. For example Next in RowSource or Row in RowIterator. Callers that want to buffer rows call CopyRow with the returned row for reuse.

CopyRow only performs a shallow copy. This is fine most of the time because processors tend to only reuse the EncDatum object, rather than any of its fields. However, #43145 ran into problems when trying to reuse the encoded field since the slice is not copied in CopyRow. This is dangerous because this could result in a row changing under the caller even after a CopyRow. To satisfy the interface contract, we need to implement deep copies for EncDatumRows.

To provide deep copies of EncDatum, we need to also add a Copy method to the Datum interface with implementations for each type.

@asubiotto
Copy link
Contributor Author

From the Next contract:

<...>
	// EncDatumRows returned by Next() are only valid until the next call to
	// Next(), although the EncDatums inside them stay valid forever.
	//
<...>
	Next() (sqlbase.EncDatumRow, *execinfrapb.ProducerMetadata)

So the current behavior is acceptable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant