Skip to content
This repository was archived by the owner on May 9, 2024. It is now read-only.

Fix arrays fetch in ResultSetRegistry. #629

Merged
merged 1 commit into from
Aug 16, 2023
Merged

Conversation

ienkovich
Copy link
Contributor

Currently, NULL value for fixed-length arrays in TargetValue can be expressed in two different ways - nullopt and zero-length array. Only arrow converter supports both cases, fetch and contentToString don't cover both. This patch forces nullopt to be always used.

Also, this patch fixes the fetch of varlen arrays with lazy fetch.

Resolves #627

@@ -1055,7 +1055,7 @@ size_t ResultSet::computeVarLenOffsets(size_t col_idx, int32_t* offsets) const {
}

// Translate varlen value to its length. Return -1 for NULLs.
auto slot_val_to_length = [this, lazy_fetch, col_idx, type, target_info](
auto slot_val_to_length = [this, lazy_fetch, col_idx, type, target_info, arr_elem_size](
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd change the name since you've modified it's meaning.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the name still correctly describes the method's meaning. It gets a slot value and returns the corresponding array length in bytes.

@ienkovich ienkovich merged commit 81de792 into main Aug 16, 2023
@ienkovich ienkovich deleted the ienkovich/issue-627 branch August 16, 2023 17:49
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

QueryBuilderTest.FixedArrayInRes produces incorrect results with allow_lazy_fetch=true
2 participants