fix(spanner): row mismatch in SelectAll using custom type#12222
fix(spanner): row mismatch in SelectAll using custom type#12222
Conversation
| return rows.Do(func(row *Row) error { | ||
| sliceItem := reflect.New(itemType) | ||
| if isFirstRow && !isPrimitive { | ||
| if !isPrimitive { |
There was a problem hiding this comment.
Do I understand it correctly that we could, instead of the current change, also do the following:
- Keep the if statements as is.
- Add a
defer func() { ... set all pointers to nil ...}to this function.
That would mean that we are re-using the slice of pointers, but they are reset to nil for each new row. Would that be a slightly more efficient solution?
If not, could we otherwise just move the entire declaration of var pointers into this function, as I dont' think we are using it outside of the function.
There was a problem hiding this comment.
I think re-using the slice of pointers will be more optimal here and that was why we initially added this
There was a problem hiding this comment.
Can we do benchmark test also like here
#9206
To confirm we are not adding regression
|
@rahul2393 @olavloite I changed the approach and set the value using reflection instead of re-initializing memory. Benchmark results |
…#12222) * fix(spanner): row mismatch in SelectAll using custom type * change else if to else * update approach to improve performance
Fixes #11101
Why is it an issue?
We are creating a new pointers for each columns only once while decoding the first row and reusing it in subsequent rows. In the decoding, if customer didn't set certain values, it's picking up values from previous column since pointers always same when DecodeSpanner is called. When we try to call DecodeSpanner, we need to make sure we have to use new memory instead of reusing it.