Fix test-unit.c so the C unit tests compile and pass#305
Open
stumpylog wants to merge 2 commits into
Open
Conversation
Feature functions were spliced into other functions' bodies by mis-anchored IVF/DiskANN hunks, with mismatched #if/#endif guards and an unterminated #ifdef in main(). Regroup each test back into a whole function under the correct feature guard, restoring the rescore tests from their last-clean revision and reassembling the IVF/DiskANN tests. Also align the IVF test guard (SQLITE_VEC_ENABLE_IVF) with the implementation flag (SQLITE_VEC_EXPERIMENTAL_IVF_ENABLE) so the binary links, and add the sqlite-vec.h / $(prefix) prerequisites to the test-unit target. Compiles and links across the flag matrix. The suite does not yet pass at runtime: the rescore int8 assertions are stale (separate pre-existing bug).
…tions The test's hand-maintained struct mirror in sqlite-vec-internal.h had drifted from sqlite-vec.c: Vec0RescoreConfig was missing the oversample_search field (8 vs 12 bytes), shifting ivf/diskann by 4 bytes so the diskann parse tests read the wrong fields. The header also didn't default RESCORE/DISKANN on (the extension does), so a no-flag build disagreed on struct sizes and smashed the stack. Sync the mirror, default the flags to match sqlite-vec.c, drop a duplicated rescore definition, and switch the feature guards from #ifdef to #if so explicit -DFLAG=0 builds stay consistent. Rewrite test_rescore_quantize_float_to_int8 against the actual quantizer (a fixed [-1,1] -> [-128,127] mapping); the old assertions expected a range-normalizing quantizer that no longer exists. All unit tests pass across the flag matrix (defaults, RESCORE, RESCORE+DISKANN, RESCORE+DISKANN+IVF, and explicit all-off).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
tests/test-unit.chasn't compiled since the IVF/DiskANN commits, and no CI job builds it, so it rotted unnoticed. Three defects:#if/#endifand an unterminated#ifdefinmain().SQLITE_VEC_ENABLE_IVFbut the implementation usesSQLITE_VEC_EXPERIMENTAL_IVF_ENABLE, so theivf_*symbols don't link.sqlite-vec-internal.hwas missingVec0RescoreConfig.oversample_search, shiftingivf/diskannby 4 bytes so the DiskANN parse tests read the wrong fields.Fix: regroup the tests into whole functions under correct guards, sync the struct mirror and default the flags to match
sqlite-vec.c, align the IVF guard, rewrite the stale int8 quantizer assertions, and add the missingtest-unitmake prerequisites.make test-unitpasses, and the suite compiles, links, and passes across the flag matrix (defaults, RESCORE, RESCORE+DISKANN, RESCORE+DISKANN+IVF, all-off).