Skip to content

fix: handle indexes without string conversion - #202

Merged
joamaki merged 1 commit into
cilium:mainfrom
immanuwell:fix-query-without-from-string
Sep 24, 2026
Merged

joamaki merged 1 commit into
cilium:mainfrom
immanuwell:fix-query-without-from-string

Conversation

@immanuwell

@immanuwell immanuwell commented Sep 13, 2026 •

Copy link
Copy Markdown
Contributor

Indexes can omit FromString, but db/get calls it anyway and panics

Repro:

  1. Register an index without FromString
  2. Run db/get test 1
  3. Current main panics in Index.fromString

The fix: return a clear query error instead

Tests: make

AIL:1

@immanuwell
immanuwell requested a review from a team as a code owner September 13, 2026 16:06
@immanuwell
immanuwell requested review from tommyp1ckles and removed request for a team September 13, 2026 16:06
@joamaki

joamaki commented Sep 14, 2026

Copy link
Copy Markdown
Contributor

See my comment at #199 (review). The commit description and the AI influence level disclaimer is missing. I'll add a PR template soon to have a checklist...

Comment thread part_index.go
Indexes can omit FromString, but string-based queries through AnyTable call it unconditionally and panic. Return a query error for these indexes and cover the db/get path with a regression test.

Signed-off-by: immanuwell <pchpr.00@list.ru>
@immanuwell
immanuwell force-pushed the fix-query-without-from-string branch from 5175e0b to 776617d Compare September 24, 2026 12:28
@immanuwell

immanuwell commented Sep 24, 2026 •

Copy link
Copy Markdown
Contributor Author

@joamaki I am sorry, added both commit description and AIL (but I thought trivial
use need not be declared (?))

@immanuwell

Copy link
Copy Markdown
Contributor Author

@tommyp1ckles everything is addressed, please take a look

@joamaki
joamaki merged commit 830d469 into cilium:main Sep 24, 2026
1 check passed
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.

3 participants