Skip to content

bug(ingest): explicit null id_field slips past require_id and dedupes every row onto "<nil>" #370

Closed

Description

@taitelee

Area: ingest · api — bug (silent data loss / correctness) · found via PR review (#219 follow-up)

Expected: With dedupe on, a row whose id_field is present but null ({"event_id": null}) is treated like a missing id — counted by wavehouse_ingest_dedupe_missing_id_total and rejected under require_id: true (else published un-deduped).

Actual: internal/api/ingest.go processRecord does idVal, ok := data[h.IDField]; an explicit JSON null decodes to a present key, so ok == true and the record skips the missing-id branch. fmt.Sprint(nil) yields the literal "<nil>", and the dedup key is un-namespaced (internal/dedupe/embedded.go:29, key := []byte(eventID) — no table/scope prefix), so every null-id row across every table collides on the single key "<nil>": the first is ingested, the rest are dropped as duplicates. require_id never fires and the metric never counts them.

Impact: Silent data loss whenever dedupe is on and a producer emits null for the id (common when a serializer writes null for an unset field). The require_id tripwire — whose whole point is guaranteeing a usable id — is defeated, and the omission is invisible in metrics.

Scope: treat a nil value as missing (if !ok || idVal == nil) so null flows through the existing missing-id path; add tests for {"event_id": null} in both require_id modes. Decide in the same fix whether empty-string "" should also count as missing (same collision class). Separate latent issue: fmt.Sprint keys collide across types (123 vs "123").

Related: #219 (the missing-id handling this extends)

Activity

  1. added
    area/ingestIngest pipeline (Bento, batching, DLQ)
    area/apiHTTP handlers, routing, middleware
    on Jul 6, 2026
  2. self-assigned this
    on Jul 6, 2026
  3. coderabbitai commented on Jul 6, 2026

    @coderabbitai
    ⚠️ Possible Duplicate Issue(s)
    🔗 Related PRs

    #122 - fix(ingest): break infinite-Nak loop on permanent delete errors [merged]
    #182 - refactor: full api --> ingest --> clickhouse --> dlq refactor [closed]


    🧪 Issue enrichment is currently in open beta.

    You can configure auto-planning by selecting labels in the issue_enrichment configuration.

    To disable automatic issue enrichment, add the following to your .coderabbit.yaml:

    issue_enrichment:
      auto_enrich:
        enabled: false

    💬 Have feedback or questions? Drop into our discord!

  4. EricAndrechek commented on Jul 6, 2026

    @EricAndrechek
    Member

    Cross-linking #222 — the cross-table dimension of this shares a root cause with it.

    The global "<nil>" collision described here happens because the dedup key is un-namespaced (internal/dedupe/embedded.go:29, key := []byte(eventID) — no table/scope prefix), which is exactly item 2 of #222 ("namespace the dedupe key by table"). So #222's keyspace fix resolves the cross-table half of this null collision on its own; what stays unique to this issue is then just treating a present-but-null (and missing) id_field as "no id" in processRecord (if !ok || idVal == nil).

    Flagging so the keyspace change gets coordinated across both rather than patched twice.

  5. moved this from Backlog to Ready in WaveHouse Task Boardon Jul 6, 2026
  6. moved this from Ready to In progress in WaveHouse Task Boardon Jul 6, 2026
  7. moved this from In progress to In review in WaveHouse Task Boardon Jul 7, 2026
  8. moved this from In review to Backlog in WaveHouse Task Boardon Jul 9, 2026
  9. moved this from Backlog to In progress in WaveHouse Task Boardon Aug 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

area/apiHTTP handlers, routing, middlewarearea/ingestIngest pipeline (Bento, batching, DLQ)

Type

No type

Projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions