release-21.1: colserde: fix the edge case with nulls handling #62915
Conversation
When serializing the data of Bool, Bytes, Int, and Float types when they don't have any nulls in the vector, we don't explicit specify the null bitmap. Previously, when deserializing such vectors with no nulls we would simply call `UnsetNulls` on the `coldata.Nulls` object that is currently present. However, it is possible that already present nulls object cannot support the desired batch length. This could lead to index out of bounds accesses. Note that in the vast majority of cases this likely doesn't happen in practice because we check `MaybeHasNulls`, and that would return `false` making us omit the null checking code. Release note (bug fix): Previously, CockroachDB could encounter an internal error in rare circumstances when executing queries via the vectorized engine that operate on columns of BOOL, BYTES, INT, and FLOAT types that have a mix of NULL and non-NULL values.
|
This change is |
|
I believe this PR could be justified for backport to 21.1.0 since it fixes a logic issue, yet the issue has been present in previous releases too, so I wonder whether we should hold off with the fix till 21.1.1. |
There was a problem hiding this comment.
Reviewed 8 of 8 files at r1.
Reviewable status:complete! 1 of 0 LGTMs obtained (waiting on @asubiotto)
returning incorrect results would also qualify as reason for a backport if that's what you meant by this |
|
Yeah, the impact of the bug is that the user might encounter an internal error (it is actually a panic which is converted into an error by our panic-catcher). I'll hold off on merging until 21.1.0 is done. |
|
sounds good thanks! |
Backport 1/1 commits from #62642.
/cc @cockroachdb/release
When serializing the data of Bool, Bytes, Int, and Float types when they
don't have any nulls in the vector, we don't explicit specify the null
bitmap. Previously, when deserializing such vectors with no nulls we
would simply call
UnsetNullson thecoldata.Nullsobject that iscurrently present. However, it is possible that already present nulls
object cannot support the desired batch length. This could lead to index
out of bounds accesses. Note that in the vast majority of cases this
likely doesn't happen in practice because we check
MaybeHasNulls, andthat would return
falsemaking us omit the null checking code.Fixes: #62636.
Release note (bug fix): Previously, CockroachDB could encounter an
internal error in rare circumstances when executing queries via the
vectorized engine that operate on columns of BOOL, BYTES, INT, and FLOAT
types that have a mix of NULL and non-NULL values.