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)
Area: ingest · api — bug (silent data loss / correctness) · found via PR review (#219 follow-up)
Expected: With dedupe on, a row whose
id_fieldis present butnull({"event_id": null}) is treated like a missing id — counted bywavehouse_ingest_dedupe_missing_id_totaland rejected underrequire_id: true(else published un-deduped).Actual:
internal/api/ingest.goprocessRecorddoesidVal, ok := data[h.IDField]; an explicit JSONnulldecodes to a present key, sook == trueand 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_idnever fires and the metric never counts them.Impact: Silent data loss whenever dedupe is on and a producer emits
nullfor the id (common when a serializer writes null for an unset field). Therequire_idtripwire — 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 bothrequire_idmodes. Decide in the same fix whether empty-string""should also count as missing (same collision class). Separate latent issue:fmt.Sprintkeys collide across types (123vs"123").Related: #219 (the missing-id handling this extends)