Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR fixes a regression caused by #16551 which modified the
UnnestStorageAdapter
to return the output capabilities of the unnest column instead of the capabilities pre-unnest, which was totally cool and correct but was leaving out settingsetDictionaryValuesUnique
which is also used by unnest to check if it can use a dimension cursor.Though admittedly totally contrived, if used again in an UNNEST operator this would result in different behavior than prior to this change because the loss of
areDictionaryValuesUnique
would result in using the column value selector cursor instead of dimension selector based cursor which have different handling of null values due to the dimension cursors attempts to be compatible with the implicit unnest used by group-by and topN queries on mvds.While writing tests for this, I found another bug that could happen for any mvd rows with 0 size against any segment where the null value was not dictionary id 0 (such as realtime segments or segments without null values in the column). I fixed this by reporting 0 as the size of the unnested row if the underlying row is 0, which aligns behavior with the implicit unnest done by group-by and topN queries (even though they are not consistent with each other, see #5897).
Finally, another bug with MSQ frames which has some mismatch where the writers are hard-coded with multi-value true while the window 'rows and columns' readers are hard-coded to use false, which results in some errors. It looks like window stuff doesn't really support multi-value strings so maybe the solution there is to just explode? But i didn't do that for now, in this case i just made the field writer detect if the input is multi-value or not so it isn't hard-coded to true (which handle 0 length rows differently than single valued)