sql: Arrays store wrong encoding type for DATE #19942
Comments
solongordon
added
the
C-bug
|
The kosher way to do this is to create a cluster migration function that looks at all the table schemas currently defined, and for each one that actually uses ARRAY[DATE] launch a column rewrite. I would be surprised however if people have been storing date arrays already since 1.1 was launched. @bdarnell thoughts? |
|
I agree that date arrays don't seem like an especially likely thing, but do any of the metrics we collect tell us about whether people are doing this? In any case, if we don't have any concrete plans to use these tags then we don't have to do anything right away - we could wait until we have a specific need before writing the implementation. (and in the meantime, we start writing the correct tag and maybe collect more metrics about whether this type is being used) |
|
As far as I can see we don't collect any metrics along these lines. And if we started collecting them now, it still couldn't tell us definitively that no one is using date arrays since not everyone has metrics reporting enabled. I agree it seems unlikely anyone is using date arrays at this point, so I'm inclined to just start writing the correct tag and add a warning comment in the code that there's a slim possibility there are incorrect headers out there. Let me know if that sounds reasonable. |
When we encode arrays, we were storing the wrong encoding type for dates (encoding.Time rather than encoding.Int). This is now fixed. This hasn't caused any problems so far since we don't use the `elementType` header value for anything. If we start using it in the future, there's a slim possibility there are persisted date arrays out there with the incorrect value, in which case we would need some special handling to detect and handle that scenario. Refers cockroachdb#19942 Release note: None
|
I'm closing this since we're now writing the correct tag, but feel free to reopen if anyone feels we should do more work w.r.t. metrics collection. |
When arrays are encoded, we store the elements' encoding type in the array header. We're currently storing the wrong type for DATE arrays:
encoding.Time(see sql/sqlbase/table.go#L1786) rather thanencoding.Int(see sql/sqlbase/table.go#L1823).Per @justinj, these header values aren't yet used for anything but may be in the future. This is trivial to fix, but it will create a potentially confusing discrepancy between values that were persisted before and after the fix.
@knz for his thoughts
The text was updated successfully, but these errors were encountered: