Skip to content

Commit

Permalink
Fix query condition regression introduced by #5417 (#5461)
Browse files Browse the repository at this point in the history
#5417 made several changes to
`SparseGlobalOrderReader::merge_result_cell_slabs`. One of these changes
erroneously caused coordinates which do not pass the query condition to
be emitted. This pull request restores the correct behavior.

The crux of the change is
```
            length = 1;
```
to
```
            length = to_process.max_slab_length(1);
```

The affected line of code occurs during coordinate de-duplication.
`merge_result_cell_slabs` puts coordinates from the leading tile of each
fragment into a priority queue and pops them off. If the coordinate at
the head of the queue matches what we just pulled off, then we continue
popping until there is no longer a match.

This de-duplication can cause a fragment to run out of loaded tiles, in
which case it is necessary to end the loop to get more data so that we
do not emit coordinates out of order. That's how we get to this `length
= 1` code, which signals that we should emit a result slab of length 1.
In contrast, `max_slab_length(1)` checks the query condition bitmap to
see if the duplicated coordinate actually belongs in the query result.

This was found by a customer data set which has many fragments which are
essentially copies of the whole coordinate domain. This scenario
de-duplicates many coordinates and is likely to hit the condition
described. The new test `Sparse global order reader: fragment full copy
1d` simulates this set-up.

A large fraction of the changes in this PR are the test scaffolding
changes to support evaluating query conditions upon our `FxRun1D` and
`FxRun2D` structures, so we can continue to check whether the expected
and actual results match in the presence of a query condition.

---
TYPE: BUG
DESC: Fix query condition regression introduced by #5417
  • Loading branch information
rroelke authored Feb 18, 2025
1 parent d0348dc commit 022af98
Show file tree
Hide file tree
Showing 10 changed files with 784 additions and 166 deletions.
5 changes: 3 additions & 2 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -169,12 +169,13 @@ if (MSVC)
add_compile_definitions("$<IF:$<CONFIG:Debug>,DEBUG,NDEBUG>")
add_compile_options(
# /Od: Disable optimizations
# /bigobj: Increase number of sections in .obj file
"$<$<CONFIG:Debug>:/Od;/bigobj>"
"$<$<CONFIG:Debug>:/Od>"
# /Ox: Enable most speed optimizations
"$<$<CONFIG:Release,RelWithDebInfo>:/Ox>"
# /Zi: Generate debug info in a separate .pdb file
"$<$<CONFIG:Debug,RelWithDebInfo>:/Zi>")
# /bigobj: increase number of sections in .obj file
add_compile_options("/bigobj")
else()
add_compile_options(-Wall -Wextra)
if (TILEDB_WERROR)
Expand Down
Loading

0 comments on commit 022af98

Please sign in to comment.