Skip to content

fix: bound counts bins and enforce merged schema field limits - #86

Open
VibeSWE wants to merge 4 commits into
mainfrom
fix/query-schema-input-limits
Open

VibeSWE wants to merge 4 commits into
mainfrom
fix/query-schema-input-limits

Conversation

@VibeSWE

@VibeSWE VibeSWE commented Oct 30, 2026 •

Copy link
Copy Markdown

Description

Counts requests previously accepted arbitrarily large numBins, and ingestion checked the field limit before adding the request's columns. A request could therefore allocate enormous bin vectors or grow a stream past its configured schema limit.

  • Restrict explicit numBins to 1–10,000 on filtered and unfiltered counts requests. Check automatically chosen bins before allocation or manifest reads, and absorb remainders into the final bin instead of allocating an extra bin.
  • Enforce P_DATASET_FIELD_COUNT_LIMIT against distinct existing and incoming fields under the stream metadata lock. Include generated fields, header fields, and typed siblings; cover every user event processed through the shared HTTP, OTel, and Kafka path.
  • Check the union of partitioned request schemas before processing events. Return the existing HTTP 400 field-limit error with the actual count and configured limit.

Compatibility

The existing field-limit setting and default of 1,000 are unchanged. Existing over-limit streams remain loadable and readable, but ingestion is rejected even when it adds no fields. Schema recovery and query refresh stay unrestricted; internal streams retain their existing behavior.

New header fields on otherwise known-schema events are now committed for dynamic streams. Filtered counts keep their existing SQL interval behavior. Concurrent partitioned requests retain existing partial-ingestion semantics; commit-time checks still prevent the local schema from exceeding its limit.

Validation

Backend-only validation with four build jobs, after rebasing onto current main:

  • cargo test --locked --lib -- --test-threads=4: 1,240 passed, 3 ignored, no failures.
  • cargo clippy --locked --all-targets: passed with existing repository warnings.
  • Rustfmt checks on all five changed files and git diff --check.

Added 24 regression tests covering bin boundaries and extreme values, HTTP 400 responses, merged field counts, generated/header/typed-sibling fields, request unions, existing wide schemas, and concurrent schema writers.


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need.

VibeSWE and others added 4 commits October 30, 2026 13:23
Static schemas never grow on ingestion, but commit_ingested_schema and
check_ingested_schemas counted the generated p_ and header columns of
each event as new fields. A static stream at or near
dataset_fields_allowed_limit therefore rejected every event with
FieldsCountLimitExceeded. For static-schema streams (non-first events),
check only the stored schema size against the limit.

Proposal-ID: cd40db86dfe934fd6c6730efa7a5d2e3
The Kafka sink commits offsets whether or not a chunk is ingested and
has no way to report a rejection to the producer, so enforcing the
dataset field limit there silently dropped whole batches.

Split Event::process into a limit-parameterised helper and add
Event::process_without_field_limit, used by the Kafka sink. HTTP and
other callers of Event::process keep enforcing the limit. Also log the
dropped record count and committed offsets when a Kafka chunk fails.

Proposal-ID: 88b5c078c1f51f364ac1272c99430dd5
- Only check numBins on the manifest-based counts path. The SQL/grouped
  path (conditions set) works out its bin interval from the time range
  and ignores numBins, so clear the value there instead of returning
  a 400.
- When numBins is omitted, clamp an out-of-range server default to
  1..=10000 instead of failing with an error that blames a parameter
  the client never sent. In-range defaults are untouched, so existing
  bins don't change.
- Update the tests to match.

Proposal-ID: 0d8f3e51d61b60ccfd75ea3a08551950
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