Repository navigation
Conversation
The df vector holds UINT64 values, but setValue(0, 1) deduces int and writes 4 bytes into the 8-byte slot, leaving the upper half as it was. It was zero in practice, so the stored df was right, but an engine check that setValue writes the vector's value size rejects the call. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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
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 work was produced with the help of language models.
FTSIndex::insertToTermsTableset a new term's df withdfVector.setValue(0, 1). The literal deducesint, so 4 bytes went into theUINT64slot and the upper half kept what the vector held before. That half was zero in practice, so stored dfs were right.LadybugDB/ladybug#1464 makes
ValueVector::setValue<T>assert thatThas the vector's value size. In runtime-check builds, which ladybug's CI uses (RUNTIME_CHECKS=1), this call then throws on every FTS insert or update that adds a new term. The df is now written asuint64_t. The change needs no engine change and has to merge before LadybugDB/ladybug#1464, which depends on it.Tests. No new test: nothing can observe the 4-byte write without that assertion, and the existing fts tests cover the call once it is in. Under the assertion, the fts e2e run on
mainfails 11 tests, and the half-applied inserts then crashQUERY_FTS_INDEXininsertion.test'sDeleteAndReCopy, a separate exception-safety problem. With this change the assertion never fires and 86 of the 87 tests pass.show_loaded_extensionsalso loads the duckdb extension, which the run did not build.