Skip to content

Wrap legacy --fbin vector index instead of panicking past the end of the file - #192

Open
RaphaelFakhri wants to merge 1 commit into
qdrant:devfrom
RaphaelFakhri:fix/legacy-fbin-wrap-index
Open

RaphaelFakhri wants to merge 1 commit into
qdrant:devfrom
RaphaelFakhri:fix/legacy-fbin-wrap-index

Conversation

@RaphaelFakhri

Copy link
Copy Markdown

Fixes #191

All Submissions:

  • Contributions should target the dev branch. Did you create your branch from dev?
  • Have you followed the guidelines in our Contributing document?
  • Have you checked to ensure there aren't other open Pull Requests for the same update/change?

Changes to Core Features:

  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your core changes, as applicable?
  • Have you successfully ran tests with your changes locally?

What this changes

Before this change, LegacyGenerator::make_point passed the point index straight to FBinReader::read_vector. When --num-vectors, --offset or --max-id goes past the number of vectors in the --fbin file, the upload panicked with an out-of-range slice index. The default --num-vectors is 100000, so any smaller file triggered the panic.

The legacy path now takes the index modulo the file's vector count. This matches what source: {type: file, strategy: from_start} already does for YAML uploads and for query generation.

Tests

generators::legacy::tests::fbin_vectors_wrap_past_the_end_of_the_file builds a two-vector fbin file and checks points 0, 1, 2 and 5. The test fails on dev with the out-of-range panic and passes with this change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant