diff --git a/AGENTS.md b/AGENTS.md
index 8154b0cf..21f463c2 100644
--- a/AGENTS.md
+++ b/AGENTS.md
@@ -54,7 +54,7 @@ The invariant index — what must stay true. Full narrative and rationale live i
5. **Per-table batching** — the worker groups events by table and bulk-INSERTs in schema column order; each table's batch is independent.
6. **Dead Letter Queue** — failed batch inserts publish to `WAVEHOUSE_DLQ` (`dlq.
`), gated by `dlq.enabled`. No silent data loss.
7. **Auth: always on, fail-loud, decoupled from authz (security)** — the JWT middleware always runs (no `auth.enabled`/`dev_mode` flag); it verifies with HMAC **or** JWKS (not both), with accepted `alg` pinned to the active verifier and checked before any key is used (rejects `alg:none` and cross-family confusion). No/invalid/expired token → empty role → policy `default_role`, with the bad-token reason stashed so a denying gate returns a loud `401`, not a bare `403`. Elevated access needs a valid granted role. Detail: architecture.md § `api/` + `internal/auth`; see also #11, §Security Considerations.
-8. **Optional dedup** — opt-in via `dedupe.enabled`; `dedupe.id_field` selects the JSON key.
+8. **Optional dedup** — opt-in via `dedupe.enabled`; `dedupe.id_field`/`require_id` are global defaults, overridable per table via `dedupe.tables` (an `id_field` of `""` opts a table out). Dedup keys are namespaced by table (`table` + NUL + id) so equal ids in different tables never collide — preserve when touching `internal/dedupe` (#222).
9. **Singleflight** — `TieredCache` coalesces concurrent misses (`x/sync/singleflight`) to prevent cache stampede.
10. **Active Sweeper** — purges NATS messages that are both ACKed (written to CH) and older than the gap window; SSE gap-fill uses `DeliverByStartTime`, no in-process ring buffer.
11. **Hasura-style access control: fail-closed (security)** — `policy.IsAdmin` (role == `admin_role`, **exact case-sensitive**, default `"admin"`) is the single admin check, shared by `Evaluate`/`ResolveRole`/`Validate`/the `/v1/admin` gate/`RoleAllowed`. Empty/absent role matches nothing (no `"*"` wildcard); `Validate` rejects empty role keys; a `nil` policy (deleted) denies **everyone incl. admin** — a total lockout, so bootstrap from the policy file, never an implicit admin grant. `default_role` is the one sanctioned roleless exception (`ResolveRole` maps empty → it pre-eval); `default_role == admin_role` is permitted but dev-only and loudly warned (`policy.DefaultRoleGrantsAdmin`). Preserve when touching `internal/policy` (policy twin of #13; see #159). Detail: architecture.md § `policy/`.
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 15b9ada6..d26dd39c 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -11,6 +11,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
### Added
+- **Per-table dedupe `id_field` and `require_id` overrides** (`internal/config/config.go`, `internal/api/ingest.go`, `cmd/wavehouse/main.go`, `config.yaml`, `docs/src/content/docs/configuration.mdx`, plus tests in `internal/api/ingest_test.go`): closes #222. The dedupe `id_field` and `require_id` were single deployment-wide settings, forcing every table onto one id-field name and one strictness level. A new optional `dedupe.tables` map (YAML-only; the flat `dedupe.*` settings stay the defaults) overrides `id_field` and/or `require_id` per table — so one table can require a strict id while another stays lenient, or dedupe on a different field; setting a table's `id_field` to `""` skips deduplication for it. A table absent from the map inherits the defaults, so existing `dedupe.*` config is unchanged. Pairs with the table-namespaced dedupe keyspace fix (also #222, see Fixed).
+
- **Missing-dedupe-id observability + optional strict mode** (`internal/api/ingest.go`, `internal/api/ingest_test.go`, `internal/config/config.go`, `cmd/wavehouse/main.go`, `config.yaml`, `deployments/compose/standalone.yaml`, `docs/src/content/docs/configuration.mdx`, `docs/src/content/docs/api.md`, `docs/src/content/docs/architecture.md`): closes #219. With dedupe enabled, a row missing the configured `id_field` can't be deduped — previously it was published with idempotency silently disabled and *no* log or metric, so a producer bug that dropped the id turned off the guarantee for those rows unnoticed. Now every such row is logged at `WARN` and counted by a new `wavehouse_ingest_dedupe_missing_id_total` counter (labeled by `table`), making the loss observable server-side. A new opt-in `dedupe.require_id` (`WH_DEDUPE_REQUIRE_ID`, default `false`) turns that signal into enforcement: a row missing the id is rejected (`400` for a single insert; a per-record failure in a batch) instead of published — a tripwire for producers that must guarantee the id (complements the client-side [#202](https://github.com/Wave-RF/WaveHouse/issues/202)). Default behavior is unchanged.
- **"Durability & Storage" operations guide** (`docs/src/content/docs/durability.md` (new), `docs/src/config/sidebar.ts`, `docs/src/content/docs/reverse-proxy.mdx`, `docs/src/content/docs/configuration.mdx`, `docs/src/content/docs/deployment.md`): documents #84. A new Operations page making the embedded-JetStream durability contract explicit before the docs site publishes: a `200` from `POST /v1/ingest` means the event has been `fsync`'d to disk on the node (the server runs with `SyncAlways: true` in `internal/mq/embedded.go`), which makes the storage substrate's `fsync` tail the ingest latency floor. Covers the contract (and how it differs from JetStream's default page-cache-then-periodic-sync mode), why a slow `fsync` tail manifests as `create stream: ... context deadline exceeded` and `503` backpressure, a where-it's-cheap-vs-expensive substrate table (managed cloud block storage and PLP NVMe vs. ZFS-without-SLOG / qcow2-on-`ext4` / spinning disks), an `fio` recipe + verdict bands to measure your own storage (with the macOS `F_FULLFSYNC` honesty caveat), and the symptom checklist. Forward-references the configurable group-commit interval (`mq.sync_interval`, [#139](https://github.com/Wave-RF/WaveHouse/issues/139)) and the planned `wavehouse storage-check` preflight ([#84](https://github.com/Wave-RF/WaveHouse/issues/84)) without claiming either exists yet. Cross-linked from Configuration (Message Queue), Deployment (Persistent Storage), and the Ingest Pipeline's worker-side ack section; no code changes.
- **"Behind a reverse proxy" deployment guide** (`docs/src/content/docs/reverse-proxy.mdx` (new), `docs/src/config/sidebar.ts`, `docs/src/content/docs/deployment.md`, `docs/src/content/docs/configuration.mdx`, `docs/src/content/docs/api.md`, `internal/api/stream.go`, `internal/api/stream_test.go`): closes #241. A new Operations page for the common "WaveHouse behind nginx / Caddy / Cloudflare Tunnel" setup, since several behaviors only matter behind a proxy and weren't documented together. Covers: TLS termination (WaveHouse serves plain HTTP and manages no certs); the request-body size limits and the division of responsibility (WaveHouse ships fixed in-code memory-safety backstops — 1 MiB control / 16 MiB ingest — while the proxy is the tunable *outer* limit, so a missing/loose proxy limit can't OOM the server); Server-Sent Events buffering + idle-timeout tuning (WaveHouse sends a `: connected` comment on open plus a periodic `:` keepalive comment so quiet streams survive proxy idle timeouts, [#226](https://github.com/Wave-RF/WaveHouse/issues/226)); the `?token=` / `since` / `Last-Event-ID` forwarding streams need; `X-Forwarded-For` trust (don't expose `:8080` directly — it's honored, so a direct client could spoof it); and which health paths to expose (`/livez`/`/readyz` internal-optional, `/v1/health` must stay public). Ships full example nginx, Caddy, and Cloudflare-Tunnel configs, and is cross-linked from Deployment, Configuration, and the API reference. One small code change lands with it: the SSE endpoint (`GET /v1/stream`) now sets `X-Accel-Buffering: no` so nginx-class proxies stream events without buffering out of the box (nginx strips the header before the client sees it; Caddy/Cloudflare ignore it). The health-probe guidance is also upgraded from "optional" to a recommendation — keep the bare `/livez`/`/readyz`/`/healthz` paths internal (a public `/readyz` turns each hit into a ClickHouse `Ping`) and expose only `/v1/health` publicly.
@@ -28,6 +30,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- **Dependabot npm updates moved to the pnpm workspace root, fixing recurring `ERR_PNPM_OUTDATED_LOCKFILE` CI failures on dependency PRs** (`.github/dependabot.yml`, `docs/src/content/docs/development.md`, `SECURITY.md`): the three per-member npm update configs (`directory: /docs`, `/clients/ts`, `/tests/e2e/sdk`) are replaced by a single config at the workspace root (`directory: /`, group `npm-deps`, prefix `deps`). The repo has one root `pnpm-lock.yaml` (the #190 consolidation), and Dependabot only updates a lockfile co-located with the manifest it targets — so a per-member config bumped a member's `package.json` without regenerating the root lockfile, and every such PR (e.g. #211, #337) then failed CI's `pnpm install --frozen-lockfile` with `ERR_PNPM_OUTDATED_LOCKFILE`; no rebase could fix it, because a rebase never regenerates the lockfile. Pointing at the root lets Dependabot read `pnpm-workspace.yaml`, walk every member, and update the one lockfile within the PR, and as a bonus brings the root `package.json`'s own devDeps (biome, markdownlint, nyc) under Dependabot. Trade-off: one combined weekly npm PR with a single `deps:` prefix instead of three per-area PRs (`docs:` / `deps(sdk):` / `deps(tests):`).
+### Fixed
+
+- **Dedupe no longer collapses explicit-null `id_field` values onto one key, and `require_id` now rejects them** (`internal/api/ingest.go`, `internal/api/ingest_test.go`, `docs/src/content/docs/configuration.mdx`, `docs/src/content/docs/api.md`, `docs/src/content/docs/architecture.md`): closes #370. The dedupe check keyed on field *presence*, so an explicit JSON `null` id (`{"event_id": null}`) — a present key with no value — was treated as a real id and deduped on `fmt.Sprint(nil)`, the literal string `""`. Every null-id row across every table collided onto that one key (all but the first silently dropped as duplicates), and the row was invisible to both `require_id` strict mode and the `wavehouse_ingest_dedupe_missing_id_total` counter. A `null` `id_field` is now treated exactly like a missing one — logged, counted, and rejected under `require_id` (or published un-deduped otherwise). Scoped to `null`; an empty string stays a real value that dedupes. Reachable whenever the `id_field` column is `Nullable(...)` or has a `DEFAULT`, which `discovery.Validate` lets a null pass through.
+- **Dedupe keys are now namespaced by table, closing a silent cross-table collision** (`internal/dedupe/dedupe.go`, `internal/dedupe/embedded.go`, `internal/api/ingest.go`, plus tests in `internal/dedupe/embedded_test.go`, `internal/testutil/mocks.go`): part of #222. `CheckAndMark` keyed Pebble on the raw id value with no table prefix, so the same id *value* ingested into two different tables collided — the second row was silently dropped as a "duplicate." The key is now `table\x00id` (a ClickHouse identifier can't contain a NUL byte, so the table prefix can't be forged by an id value). **Breaking:** the on-disk key format changes, so existing dedupe state no longer matches after upgrade — every event is treated as new for one round. Best-effort idempotency means no stored data is lost, but a duplicate submitted across the upgrade boundary won't be caught.
+
### Security
- **Policy `_in` is now enforced on both the row-`filter` and insert-`check` paths, closing a fail-open row-security gap** (`internal/policy/policy.go`, `internal/api/ingest.go`, `docs/src/content/docs/access-control.mdx`, plus tests in `internal/policy/policy_test.go`, `internal/api/ingest_test.go`): closes #224. The `Filter` schema accepted `_in` but the engine never read it: on the row-`filter`/SELECT path `resolveFilters` had no `_in` branch, so a row-security filter like `tenant_id: { _in: … }` produced **no `WHERE` predicate** and the role saw every row instead of its tenant subset (a fail-open, same family as #223); on the `check`/INSERT path only `_eq` was honored, silently dropping any other operator. `_in` now takes a single claim that resolves to a JSON **array** (the multi-tenant case — a token's `tenant_ids` list) and emits `col IN (?, …)` with one bound param per element; a scalar claim is a one-element set, and an empty/absent claim matches **no rows** (fail-closed) rather than widening to all of them. On the insert path an `_in` check requires the column be present and one of the set — there is no single value to auto-inject as `_eq` does, so an omitted column is rejected (`403 check failed`). The comparison operators are enforced on `filter` (`_eq`/`_neq`/`_gt`/`_lt`/`_in` all produce predicates now, so nothing is rejected there) and, on `check`, `_neq`/`_gt`/`_lt` become a loud config-load rejection (no insert-time semantics; `check` honors `_eq` + `_in`). The `_in` value stays a single templated string in the wire schema (Go `Filter.In`, SDK `PolicyFilter._in`), matching the established "set = array" shape of the caller-query `in` operator.
diff --git a/cmd/wavehouse/main.go b/cmd/wavehouse/main.go
index 698de535..fb6ad8d9 100644
--- a/cmd/wavehouse/main.go
+++ b/cmd/wavehouse/main.go
@@ -331,6 +331,13 @@ func run() int {
ingestHandler.Dedup = dedup
ingestHandler.IDField = cfg.Dedupe.IDField
ingestHandler.RequireID = cfg.Dedupe.RequireID
+ if len(cfg.Dedupe.Tables) > 0 {
+ overrides := make(map[string]api.DedupeOverride, len(cfg.Dedupe.Tables))
+ for t, o := range cfg.Dedupe.Tables {
+ overrides[t] = api.DedupeOverride{IDField: o.IDField, RequireID: o.RequireID}
+ }
+ ingestHandler.DedupeTables = overrides
+ }
}
var dlqHandler *api.DLQHandler
diff --git a/config.yaml b/config.yaml
index 1f4cb47e..e371da3d 100644
--- a/config.yaml
+++ b/config.yaml
@@ -73,6 +73,16 @@ dedupe:
# Reject rows missing id_field instead of publishing them un-deduped. Off by
# default (they are logged + counted via wavehouse_ingest_dedupe_missing_id_total).
require_id: false
+ # Per-table overrides of id_field / require_id (YAML-only; the settings above
+ # are the defaults). A table not listed inherits the defaults; set a table's
+ # id_field to "" to skip dedupe for it. (#222)
+ # tables:
+ # payments:
+ # require_id: true # payments must carry an id or be rejected
+ # page_views:
+ # id_field: view_id # dedupe this table on view_id instead of event_id
+ # audit_log:
+ # id_field: "" # skip dedupe for this table entirely
cache:
l1_max_cost: 67108864
diff --git a/docs/src/content/docs/api.md b/docs/src/content/docs/api.md
index 13b1608f..38db7970 100644
--- a/docs/src/content/docs/api.md
+++ b/docs/src/content/docs/api.md
@@ -225,7 +225,7 @@ The body is a **flat JSON object** whose keys must match column names in the tar
| ------ | ---- | ----- |
| 400 | `{"error":"invalid json"}` | Malformed request body |
| 400 | `{"error":"unknown column ... for table ..."}` (also: `missing required column ...`, `type mismatch for column ...`, `null value for non-nullable column ...`) | Schema validation failure (unknown fields, type mismatches, missing required columns, null in a non-nullable column). The body is the validator's message verbatim — there is no `validation failed:` prefix. |
-| 400 | `{"error":"missing dedupe id field \"event_id\""}` | Only when dedupe is enabled with `dedupe.require_id: true` and the row lacks the configured `id_field`. With `require_id: false` (the default) the row is instead published un-deduped. Either way — reject or publish — the row is logged at `WARN` and counted by `wavehouse_ingest_dedupe_missing_id_total`. In a batch this is a per-record failure, not a whole-request error. |
+| 400 | `{"error":"missing dedupe id field \"\""}` (`` is the table's effective dedupe field — the `dedupe.id_field` default, e.g. `event_id`, or a `dedupe.tables` per-table override) | Only when dedupe is enabled and `require_id` is in effect for the table (the global `dedupe.require_id`, or a per-table `dedupe.tables` override) and the row lacks that `id_field` — whether the field is missing or sent as an explicit JSON `null`. With `require_id` off (the default) the row is instead published un-deduped. Either way — reject or publish — the row is logged at `WARN` and counted by `wavehouse_ingest_dedupe_missing_id_total`. In a batch this is a per-record failure, not a whole-request error. |
| 401 | `{"error":"invalid token"}` / `{"error":"token expired"}` | A present-but-invalid/expired token was supplied and denied (the gate surfaces the token reason rather than silently falling back to `default_role`) |
| 403 | `{"error":"forbidden"}` (empty-role variant: `forbidden: request has no role and no public default_role is configured`) | The resolved role lacks `insert` on the table |
| 404 | `{"error":"unknown table: ..."}` | Table not found in ClickHouse schema |
diff --git a/docs/src/content/docs/architecture.md b/docs/src/content/docs/architecture.md
index 3865d6d3..575bd9e9 100644
--- a/docs/src/content/docs/architecture.md
+++ b/docs/src/content/docs/architecture.md
@@ -74,7 +74,7 @@ The API layer uses [Chi](https://github.com/go-chi/chi) for routing with Request
- **policy.go** — CRUD handler for access control policies (`/v1/admin/policy`).
- **pipes.go** — Named query pipe handlers: admin CRUD and execution with parameter binding.
- **structured_query.go** — Handler for `POST /v1/query?table={table}`: validates query AST, enforces permissions, builds and executes SQL.
-- **ingest.go** — Accepts flat JSON body for `POST /v1/ingest?table={table}`, validates against discovered schema, optional dedup, publishes to NATS subject `ingest.{table}`. When dedup is on, a row missing the configured `id_field` can't be deduped: it is logged at `WARN` and counted by `wavehouse_ingest_dedupe_missing_id_total` (labeled by `table`), then published un-deduped — or rejected when `dedupe.require_id` is set (#219).
+- **ingest.go** — Accepts flat JSON body for `POST /v1/ingest?table={table}`, validates against discovered schema, optional dedup, publishes to NATS subject `ingest.{table}`. When dedup is on, a row missing the configured `id_field` (or sending it as an explicit `null`) can't be deduped: it is logged at `WARN` and counted by `wavehouse_ingest_dedupe_missing_id_total` (labeled by `table`), then published un-deduped — or rejected when `dedupe.require_id` is set (#219). The `id_field` and `require_id` are resolved per table (global defaults plus `dedupe.tables` overrides), and the dedup key is namespaced by table so equal id values in different tables never collide (#222).
- **query.go** — Proxies raw SQL for `POST /v1/admin/query` straight to ClickHouse's HTTP interface. **Not cached** — sets `Cache-Control: no-store` so every request hits ClickHouse; DateTime is rendered ISO-8601 via `date_time_output_format=iso` (the Go-side type conversion lives in the structured-query / pipes path, not here).
- **stream.go** — Real-time streaming via SSE. Callers select a table with the `?table=` query parameter. Each connection registers one `Subscriber` (the `stream/` package) with both the event `Hub` (under its `(topic, role)`) and the shared keepalive wheel, then drains both from a single byte-pump — so idle streams keep emitting `:` keepalive comments (surviving reverse-proxy idle timeouts) while live events arrive already projected and serialized. Per-event projection/serialization happens **once per role** in the `Hub`, not once per subscriber ([#294](https://github.com/Wave-RF/WaveHouse/issues/294)). Gap-fill replay from NATS JetStream (`DeliverByStartTime`) stays per-connection (low-volume, one-time on connect).
- **schema.go** — Schema discovery API: list all schemas, get one table, trigger refresh.
@@ -108,8 +108,8 @@ The SSE fan-out, factored out of `api/` so the delivery hot path ([#294](https:/
### `dedupe/` — Deduplication (Optional)
-- **dedupe.go** — `Deduplicator` interface: `CheckAndMark(ctx, eventID) (bool, error)`.
-- **embedded.go** — Uses [Pebble](https://github.com/cockroachdb/pebble) (embedded key-value store). Key = event ID.
+- **dedupe.go** — `Deduplicator` interface: `CheckAndMark(ctx, table, eventID) (bool, error)`.
+- **embedded.go** — Uses [Pebble](https://github.com/cockroachdb/pebble) (embedded key-value store). Key is namespaced by table (`table` + NUL + event ID) so equal id values in different tables never collide (#222).
### `discovery/` — Schema Discovery & Validation
@@ -168,7 +168,8 @@ Client POST /v1/ingest?table={table}
→ Policy column rules + check clauses (disallowed columns rejected;
claim-derived values enforced or injected)
→ Optional deduplication check (configurable ID field; a row missing that
- field is published un-deduped + logged/counted, or rejected under require_id)
+ field — or carrying it as an explicit null — is published un-deduped +
+ logged/counted, or rejected under require_id)
→ Publish to NATS JetStream (ingest.{table})
→ 200 OK returned immediately
→ (If NATS stream is full: 503 + Retry-After header)
diff --git a/docs/src/content/docs/configuration.mdx b/docs/src/content/docs/configuration.mdx
index a46f06f9..ef04ad2a 100644
--- a/docs/src/content/docs/configuration.mdx
+++ b/docs/src/content/docs/configuration.mdx
@@ -129,7 +129,24 @@ WaveHouse's per-role caps are sent as per-query `SETTINGS` on its connection, so
| --- | --- | ------- | ----------- |
| `dedupe.enabled` | `WH_DEDUPE_ENABLED` | `false` | Enable event deduplication. When enabled, the ingest handler checks for duplicates using the configured ID field. |
| `dedupe.id_field` | `WH_DEDUPE_ID_FIELD` | `event_id` | JSON field name in the ingest body used as the dedup key. |
-| `dedupe.require_id` | `WH_DEDUPE_REQUIRE_ID` | `false` | With dedupe enabled, controls what happens to a row missing `id_field` (which can't be deduped, so idempotency wouldn't apply to it). Such a row is always logged at `WARN` and counted by `wavehouse_ingest_dedupe_missing_id_total`, in both modes. Default (`false`): it is then published un-deduped. Set `true` to reject it instead (`400` for a single insert; a per-record failure in a batch) — a server-side tripwire for producers that must guarantee the id. |
+| `dedupe.require_id` | `WH_DEDUPE_REQUIRE_ID` | `false` | With dedupe enabled, controls what happens to a row whose `id_field` is missing or an explicit JSON `null` (which can't be deduped, so idempotency wouldn't apply to it). Such a row is always logged at `WARN` and counted by `wavehouse_ingest_dedupe_missing_id_total`, in both modes. Default (`false`): it is then published un-deduped. Set `true` to reject it instead (`400` for a single insert; a per-record failure in a batch) — a server-side tripwire for producers that must guarantee the id. |
+| `dedupe.tables` | *(YAML only)* | *(none)* | Per-table overrides of `id_field` / `require_id`, keyed by table name. A table absent here inherits the global defaults above; set a table's `id_field` to `""` to skip deduplication for it. |
+
+Each table's dedup keyspace is independent — the same id value ingested into two different tables is **not** treated as a collision. The flat `dedupe.*` settings are the defaults; `dedupe.tables` overrides them per table (YAML-only — there is no env var for per-table config):
+
+```yaml
+dedupe:
+ enabled: true
+ id_field: event_id # default for tables not listed below
+ require_id: false
+ tables:
+ payments:
+ require_id: true # payments must carry an id or be rejected
+ page_views:
+ id_field: view_id # dedupe this table on view_id instead
+ audit_log:
+ id_field: "" # skip dedupe for this table entirely
+```
### Cache
diff --git a/internal/api/ingest.go b/internal/api/ingest.go
index f283c59a..64920719 100644
--- a/internal/api/ingest.go
+++ b/internal/api/ingest.go
@@ -36,13 +36,14 @@ const maxReportedResults = 10000
// IngestHandler handles POST /v1/ingest?table={table}
type IngestHandler struct {
- Registry *discovery.SchemaRegistry
- Dedup dedupe.Deduplicator // nil if dedup disabled
- IDField string // dedup key field name (e.g. "event_id")
- RequireID bool // reject rows missing IDField instead of publishing un-deduped (dedupe.require_id)
- Publisher mq.Publisher
- PolicyStore *policy.Store
- logger *slog.Logger
+ Registry *discovery.SchemaRegistry
+ Dedup dedupe.Deduplicator // nil if dedup disabled
+ IDField string // default dedup key field name (e.g. "event_id"); DedupeTables overrides it per table
+ RequireID bool // default strict mode: reject rows missing the id (dedupe.require_id); DedupeTables overrides it per table
+ DedupeTables map[string]DedupeOverride
+ Publisher mq.Publisher
+ PolicyStore *policy.Store
+ logger *slog.Logger
// maxRequestBytes optionally overrides the default inbound request body cap
// (maxRequestBodyBytes). When 0, the default applies. Exists so same-package
@@ -55,6 +56,28 @@ func NewIngestHandler(registry *discovery.SchemaRegistry, pub mq.Publisher, logg
return &IngestHandler{Registry: registry, Publisher: pub, logger: logger}
}
+type DedupeOverride struct {
+ IDField *string
+ RequireID *bool
+}
+
+// dedupeFieldsFor resolves the effective dedupe id field and strict-mode flag
+// for a table: the IDField/RequireID defaults, with any per-table override from
+// DedupeTables applied. An empty id field means dedupe is skipped for the
+// table.
+func (h *IngestHandler) dedupeFieldsFor(table string) (idField string, requireID bool) {
+ idField, requireID = h.IDField, h.RequireID
+ if o, ok := h.DedupeTables[table]; ok {
+ if o.IDField != nil {
+ idField = *o.IDField
+ }
+ if o.RequireID != nil {
+ requireID = *o.RequireID
+ }
+ }
+ return idField, requireID
+}
+
var dedupeMissingIDCounter, _ = otel.Meter("wavehouse-ingest").Int64Counter(
"wavehouse_ingest_dedupe_missing_id_total",
metric.WithDescription("Ingested records missing the configured dedupe id_field (idempotency skipped)"),
@@ -401,29 +424,40 @@ func (h *IngestHandler) processRecord(
}
}
- // Optional deduplication.
- if h.Dedup != nil && h.IDField != "" {
- idVal, ok := data[h.IDField]
- if !ok {
- dedupeMissingIDCounter.Add(ctx, 1, metric.WithAttributes(attribute.String("table", table)))
- if h.RequireID {
- h.logger.WarnContext(ctx, "dedupe id_field missing; rejecting", "id_field", h.IDField, "table", table)
- return false, &recordReject{
- Status: http.StatusBadRequest,
- Message: fmt.Sprintf("missing dedupe id field %q", h.IDField),
- }, nil
- }
- h.logger.WarnContext(ctx, "dedupe id_field missing; publishing without idempotency", "id_field", h.IDField, "table", table)
- } else {
- eventID := fmt.Sprint(idVal)
- dup, err := h.Dedup.CheckAndMark(ctx, eventID)
- if err != nil {
- h.logger.ErrorContext(ctx, "dedupe check failed", "error", err, "event_id", eventID)
- return false, nil, &requestAbort{Status: http.StatusInternalServerError, Message: "dedupe failed"}
- }
- if dup {
- h.logger.InfoContext(ctx, "duplicate event skipped", "event_id", eventID)
- return true, nil, nil
+ // Optional deduplication. The id field and strict-mode flag are resolved per
+ // table — global defaults with per-table overrides — and an empty id field
+ // skips dedupe for this table.
+ if h.Dedup != nil {
+ idField, requireID := h.dedupeFieldsFor(table)
+ if idField != "" {
+ // A present key with an explicit JSON null (ok == true, idVal == nil)
+ // carries no usable id — semantically the same as the field being
+ // absent. Treat it as missing so it isn't deduped on the
+ // fmt.Sprint(nil) sentinel "" (which collides every null-id row
+ // onto one key, dropping all but the first as duplicates) and so
+ // require_id still enforces.
+ idVal, ok := data[idField]
+ if !ok || idVal == nil {
+ dedupeMissingIDCounter.Add(ctx, 1, metric.WithAttributes(attribute.String("table", table)))
+ if requireID {
+ h.logger.WarnContext(ctx, "dedupe id_field missing or null; rejecting", "id_field", idField, "table", table)
+ return false, &recordReject{
+ Status: http.StatusBadRequest,
+ Message: fmt.Sprintf("missing dedupe id field %q", idField),
+ }, nil
+ }
+ h.logger.WarnContext(ctx, "dedupe id_field missing or null; publishing without idempotency", "id_field", idField, "table", table)
+ } else {
+ eventID := fmt.Sprint(idVal)
+ dup, err := h.Dedup.CheckAndMark(ctx, table, eventID)
+ if err != nil {
+ h.logger.ErrorContext(ctx, "dedupe check failed", "error", err, "event_id", eventID)
+ return false, nil, &requestAbort{Status: http.StatusInternalServerError, Message: "dedupe failed"}
+ }
+ if dup {
+ h.logger.InfoContext(ctx, "duplicate event skipped", "event_id", eventID)
+ return true, nil, nil
+ }
}
}
}
diff --git a/internal/api/ingest_test.go b/internal/api/ingest_test.go
index 129aa965..0ffae029 100644
--- a/internal/api/ingest_test.go
+++ b/internal/api/ingest_test.go
@@ -530,6 +530,125 @@ func TestIngest_Dedup_RequireID_Rejects(t *testing.T) {
assert.NotNil(t, pub.LastMessage(), "a record carrying the id is still accepted")
}
+// TestIngest_Dedup_NullIDField_RequireID_Rejects covers the null-id hole in
+// strict mode: an explicit JSON null is a present key but carries no usable id,
+// so require_id must reject it exactly like a missing field rather than let it
+// slip through and dedupe on the fmt.Sprint(nil) "" sentinel.
+func TestIngest_Dedup_NullIDField_RequireID_Rejects(t *testing.T) {
+ t.Parallel()
+ pub := &testutil.MockPublisher{}
+ h := NewIngestHandler(testRegistry(), pub, testutil.NopLogger())
+ h.Dedup = testutil.NewMockDeduplicator()
+ h.IDField = "event_id"
+ h.RequireID = true
+
+ w := httptest.NewRecorder()
+ h.Handle(w, ingestRequest(t, "clicks", map[string]any{"page": "/home", "event_id": nil}))
+ assert.Equal(t, http.StatusBadRequest, w.Code)
+ assert.Contains(t, w.Body.String(), "missing dedupe id field")
+ testutil.AssertJSONErrorResponse(t, w)
+ assert.Nil(t, pub.LastMessage(), "must not publish a row whose dedupe id is an explicit null under require_id")
+}
+
+// TestIngest_Dedup_NullIDField_ObserveMode_PublishesEach is the regression for
+// the null-id collision: with require_id off, a null id is treated as missing
+// and published un-deduped, so two distinct null-id rows both publish. Before
+// the fix they collapsed onto the fmt.Sprint(nil) "" key and the second was
+// silently dropped as a duplicate.
+func TestIngest_Dedup_NullIDField_ObserveMode_PublishesEach(t *testing.T) {
+ t.Parallel()
+ pub := &testutil.MockPublisher{}
+ h := NewIngestHandler(testRegistry(), pub, testutil.NopLogger())
+ h.Dedup = testutil.NewMockDeduplicator()
+ h.IDField = "event_id"
+
+ for _, page := range []string{"/a", "/b"} {
+ w := httptest.NewRecorder()
+ h.Handle(w, ingestRequest(t, "clicks", map[string]any{"page": page, "event_id": nil}))
+ assert.Equal(t, http.StatusOK, w.Code)
+ var resp map[string]bool
+ require.NoError(t, json.Unmarshal(w.Body.Bytes(), &resp))
+ assert.False(t, resp["duplicate"], "a null-id row must not be treated as a duplicate")
+ }
+ assert.Len(t, pub.Messages, 2, "both null-id rows publish un-deduped; neither collapses onto the key")
+}
+
+func ptr[T any](v T) *T { return &v }
+
+// TestIngest_Dedup_PerTable_RequireIDOverride verifies a per-table override of
+// require_id: strict mode is off by default, but the "clicks" table turns it on,
+// so a clicks row missing the id is rejected (#222).
+func TestIngest_Dedup_PerTable_RequireIDOverride(t *testing.T) {
+ t.Parallel()
+ pub := &testutil.MockPublisher{}
+ h := NewIngestHandler(testRegistry(), pub, testutil.NopLogger())
+ h.Dedup = testutil.NewMockDeduplicator()
+ h.IDField = "event_id"
+ h.RequireID = false // global default: lenient
+ h.DedupeTables = map[string]DedupeOverride{
+ "clicks": {RequireID: ptr(true)}, // this table is strict
+ }
+
+ w := httptest.NewRecorder()
+ h.Handle(w, ingestRequest(t, "clicks", map[string]any{"page": "/home"})) // no event_id
+ assert.Equal(t, http.StatusBadRequest, w.Code, "clicks overrides require_id=true, so a missing id is rejected")
+ assert.Contains(t, w.Body.String(), "missing dedupe id field")
+ testutil.AssertJSONErrorResponse(t, w)
+ assert.Nil(t, pub.LastMessage())
+}
+
+// TestIngest_Dedup_PerTable_IDFieldOverride verifies a per-table override of the
+// dedupe id field: "clicks" dedupes on org_id instead of the global event_id, so
+// two rows sharing an org_id collapse to one (#222).
+func TestIngest_Dedup_PerTable_IDFieldOverride(t *testing.T) {
+ t.Parallel()
+ pub := &testutil.MockPublisher{}
+ h := NewIngestHandler(testRegistry(), pub, testutil.NopLogger())
+ h.Dedup = testutil.NewMockDeduplicator()
+ h.IDField = "event_id"
+ h.DedupeTables = map[string]DedupeOverride{
+ "clicks": {IDField: ptr("org_id")},
+ }
+
+ // Two rows share org_id (the overridden key) but carry different event_ids →
+ // the second is a duplicate because clicks keys on org_id, not event_id.
+ w := httptest.NewRecorder()
+ h.Handle(w, ingestRequest(t, "clicks", map[string]any{"page": "/a", "org_id": "org-1", "event_id": "e1"}))
+ assert.Equal(t, http.StatusOK, w.Code)
+
+ w = httptest.NewRecorder()
+ h.Handle(w, ingestRequest(t, "clicks", map[string]any{"page": "/b", "org_id": "org-1", "event_id": "e2"}))
+ assert.Equal(t, http.StatusOK, w.Code)
+ var resp map[string]bool
+ require.NoError(t, json.Unmarshal(w.Body.Bytes(), &resp))
+ assert.True(t, resp["duplicate"], "second row with the same org_id is a duplicate under the id_field override")
+ assert.Len(t, pub.Messages, 1)
+}
+
+// TestIngest_Dedup_PerTable_OptOut verifies that a per-table id_field of ""
+// disables dedupe for that table: two rows with the same event_id both publish
+// even though dedupe is globally enabled (#222).
+func TestIngest_Dedup_PerTable_OptOut(t *testing.T) {
+ t.Parallel()
+ pub := &testutil.MockPublisher{}
+ h := NewIngestHandler(testRegistry(), pub, testutil.NopLogger())
+ h.Dedup = testutil.NewMockDeduplicator()
+ h.IDField = "event_id"
+ h.DedupeTables = map[string]DedupeOverride{
+ "clicks": {IDField: ptr("")}, // opt clicks out of dedupe
+ }
+
+ for _, n := range []string{"first", "second"} {
+ w := httptest.NewRecorder()
+ h.Handle(w, ingestRequest(t, "clicks", map[string]any{"page": "/" + n, "event_id": "same-id"}))
+ assert.Equal(t, http.StatusOK, w.Code)
+ var resp map[string]bool
+ require.NoError(t, json.Unmarshal(w.Body.Bytes(), &resp))
+ assert.False(t, resp["duplicate"], "dedupe is off for clicks, so no row is a duplicate")
+ }
+ assert.Len(t, pub.Messages, 2, "both rows publish; clicks is opted out of dedupe")
+}
+
// TestIngest_NDJSON_RequireID_Rejects confirms strict mode is per-record: the
// missing-id line fails while the rest of the batch is published.
func TestIngest_NDJSON_RequireID_Rejects(t *testing.T) {
diff --git a/internal/config/config.go b/internal/config/config.go
index b14d8ff4..7a3dd22d 100644
--- a/internal/config/config.go
+++ b/internal/config/config.go
@@ -133,9 +133,18 @@ type MQ struct {
}
type Dedupe struct {
- Enabled bool `yaml:"enabled" env:"WH_DEDUPE_ENABLED" env-default:"false"`
- IDField string `yaml:"id_field" env:"WH_DEDUPE_ID_FIELD" env-default:"event_id"`
- RequireID bool `yaml:"require_id" env:"WH_DEDUPE_REQUIRE_ID" env-default:"false"`
+ Enabled bool `yaml:"enabled" env:"WH_DEDUPE_ENABLED" env-default:"false"`
+ IDField string `yaml:"id_field" env:"WH_DEDUPE_ID_FIELD" env-default:"event_id"`
+ RequireID bool `yaml:"require_id" env:"WH_DEDUPE_REQUIRE_ID" env-default:"false"`
+ Tables map[string]DedupeTable `yaml:"tables"`
+}
+
+// DedupeTable overrides the global dedupe defaults for one table. A nil
+// field inherits the corresponding global default; an id_field of "" skips
+// deduplication for the table.
+type DedupeTable struct {
+ IDField *string `yaml:"id_field"`
+ RequireID *bool `yaml:"require_id"`
}
type Cache struct {
diff --git a/internal/dedupe/dedupe.go b/internal/dedupe/dedupe.go
index 01594220..20da5b68 100644
--- a/internal/dedupe/dedupe.go
+++ b/internal/dedupe/dedupe.go
@@ -5,8 +5,9 @@ import "context"
// Deduplicator checks whether an event has been seen before and marks it.
type Deduplicator interface {
// CheckAndMark returns true if the event was already seen (duplicate).
- // If not seen, it atomically marks the event as seen.
- CheckAndMark(ctx context.Context, eventID string) (isDuplicate bool, err error)
+ // If not seen, it atomically marks the event as seen. The key is namespaced
+ // by table so equal id values in different tables never collide (#222).
+ CheckAndMark(ctx context.Context, table, eventID string) (isDuplicate bool, err error)
Stats() map[string]int64
diff --git a/internal/dedupe/embedded.go b/internal/dedupe/embedded.go
index 6ba2a13a..da1dadb4 100644
--- a/internal/dedupe/embedded.go
+++ b/internal/dedupe/embedded.go
@@ -25,8 +25,11 @@ func NewEmbedded(dir string) (*EmbeddedDeduplicator, error) {
}
// CheckAndMark returns true if the event was already seen.
-func (d *EmbeddedDeduplicator) CheckAndMark(_ context.Context, eventID string) (bool, error) {
- key := []byte(eventID)
+func (d *EmbeddedDeduplicator) CheckAndMark(_ context.Context, table, eventID string) (bool, error) {
+ // Namespace the key by table so equal id values in different tables don't
+ // collide. A ClickHouse identifier can't contain a NUL byte, so the
+ // first NUL unambiguously separates the table prefix from the id.
+ key := []byte(table + "\x00" + eventID)
_, closer, err := d.db.Get(key)
if err == nil {
diff --git a/internal/dedupe/embedded_test.go b/internal/dedupe/embedded_test.go
index 9851164b..5f612237 100644
--- a/internal/dedupe/embedded_test.go
+++ b/internal/dedupe/embedded_test.go
@@ -24,19 +24,38 @@ func TestEmbeddedDeduplicator_FirstSeenThenDuplicate(t *testing.T) {
d := newEmbedded(t)
ctx := context.Background()
- dup, err := d.CheckAndMark(ctx, "event-1")
+ dup, err := d.CheckAndMark(ctx, "clicks", "event-1")
require.NoError(t, err)
assert.False(t, dup, "first occurrence must not be a duplicate")
- dup, err = d.CheckAndMark(ctx, "event-1")
+ dup, err = d.CheckAndMark(ctx, "clicks", "event-1")
require.NoError(t, err)
assert.True(t, dup, "second occurrence of the same id must be a duplicate")
- dup, err = d.CheckAndMark(ctx, "event-2")
+ dup, err = d.CheckAndMark(ctx, "clicks", "event-2")
require.NoError(t, err)
assert.False(t, dup, "distinct ids are independent")
}
+func TestEmbeddedDeduplicator_TableNamespacing(t *testing.T) {
+ t.Parallel()
+
+ d := newEmbedded(t)
+ ctx := context.Background()
+
+ dup, err := d.CheckAndMark(ctx, "clicks", "shared-id")
+ require.NoError(t, err)
+ assert.False(t, dup, "first table's id is new")
+
+ dup, err = d.CheckAndMark(ctx, "purchases", "shared-id")
+ require.NoError(t, err)
+ assert.False(t, dup, "same id value in a different table must not be a duplicate")
+
+ dup, err = d.CheckAndMark(ctx, "clicks", "shared-id")
+ require.NoError(t, err)
+ assert.True(t, dup, "same id in the same table is still a duplicate")
+}
+
func TestEmbeddedDeduplicator_Stats(t *testing.T) {
t.Parallel()
diff --git a/internal/observability/metrics_test.go b/internal/observability/metrics_test.go
index 15d5368b..959014f8 100644
--- a/internal/observability/metrics_test.go
+++ b/internal/observability/metrics_test.go
@@ -20,9 +20,11 @@ type stubDeduplicator struct {
stats map[string]int64
}
-func (s *stubDeduplicator) CheckAndMark(context.Context, string) (bool, error) { return false, nil }
-func (s *stubDeduplicator) Stats() map[string]int64 { return s.stats }
-func (s *stubDeduplicator) Close() error { return nil }
+func (s *stubDeduplicator) CheckAndMark(context.Context, string, string) (bool, error) {
+ return false, nil
+}
+func (s *stubDeduplicator) Stats() map[string]int64 { return s.stats }
+func (s *stubDeduplicator) Close() error { return nil }
func TestRegisterSystemMetrics_NilInputs(t *testing.T) {
// No t.Parallel(): all three TestRegisterSystemMetrics_* tests mutate the
diff --git a/internal/testutil/mocks.go b/internal/testutil/mocks.go
index eb7ab654..2cc0cfbd 100644
--- a/internal/testutil/mocks.go
+++ b/internal/testutil/mocks.go
@@ -97,16 +97,17 @@ func (m *MockDeduplicator) Stats() map[string]int64 {
}
}
-func (m *MockDeduplicator) CheckAndMark(_ context.Context, eventID string) (bool, error) {
+func (m *MockDeduplicator) CheckAndMark(_ context.Context, table, eventID string) (bool, error) {
if m.Err != nil {
return false, m.Err
}
m.mu.Lock()
defer m.mu.Unlock()
- if m.seen[eventID] {
+ key := table + "\x00" + eventID
+ if m.seen[key] {
return true, nil
}
- m.seen[eventID] = true
+ m.seen[key] = true
return false, nil
}
diff --git a/tests/e2e/fixtures/config.yaml b/tests/e2e/fixtures/config.yaml
index da387be5..f2b54eaf 100644
--- a/tests/e2e/fixtures/config.yaml
+++ b/tests/e2e/fixtures/config.yaml
@@ -17,6 +17,9 @@ mq:
dedupe:
enabled: true
id_field: event_id
+ tables:
+ events_ingest:
+ require_id: true
# JWT validation. The suite signs test tokens with this fixed dev secret;
# don't reuse outside the e2e rig.