feat(clp-s): Support per-archive group-by count aggregation output to stdout and the results cache.#2379
Draft
davemarco wants to merge 68 commits into
Draft
feat(clp-s): Support per-archive group-by count aggregation output to stdout and the results cache.#2379davemarco wants to merge 68 commits into
davemarco wants to merge 68 commits into
Conversation
…e results cache. Allows --count and --count-by-time to be used with the results-cache output handler, writing aggregation results directly to MongoDB via _id-keyed atomic $inc upserts so that partial results from multiple search processes merge correctly without a reducer.
…he search usage text outputs to the reducer.
…nation and simplify comments.
…dout. Adds `--count` and `--count-by-time` to the stdout output handler, emitting per-archive results as newline-delimited JSON. The options may be given without naming the `stdout` handler (e.g. `clp-s s <archives> <query> --count`), and the kv-ir search path now reports these aggregations as unsupported, consistent with the reducer and results-cache handlers.
… results cache. Adds --min/--max <field> aggregations, extracting the field's numeric value from each matched record's marshalled JSON. Unifies the results-cache count handlers into a single AggregationToResultsCacheOutputHandler handling count, count-by-time, min, and max.
…ers. - Take string_view for the results-cache handler uri/collection/dataset/archive_id params and materialize std::string only at the mongocxx call sites. - Drop the redundant ::clp_s:: qualifier on the count handlers' base class. - Rename the count-by-time write timestamp param to timestamp_ms to match the bucket size unit.
…n handler. Merges the results-cache count work and applies the same review fixes to AggregationToStdoutOutputHandler: take string_view for archive_id, drop the redundant ::clp_s:: qualifier on the base class, rename the count-by-time bucket size to count_by_time_bucket_size_ms, and rename the count-by-time write timestamp param to timestamp_ms.
Also wrap an IntFloatCompare comment and drop a stray blank line.
…reducer dispatch, rename aggregation error. - CountByTimeAggregation rejects non-positive bucket sizes. - ResultsCacheSink clears its buffer after a successful flush to avoid duplicate inserts. - Reducer output dispatch handles count-by-time explicitly and throws on unsupported aggregations. - Rename KvIrSearchErrorEnum::CountSupportNotImplemented to AggregationSupportNotImplemented.
…er clang-tidy. Make connect_to_results_cache a non-template function defined in ResultsCacheUtils.cpp instead of a header-only template. The template existed only to throw each caller's nested OperationFailed, but the call sites are caught as std::exception and TraceableException::what() ignores the exception type, so collapsing them into a single ResultsCacheConnectionError is behavior-preserving. This also gives the header a sibling .cpp, so clang-tidy resolves the mongocxx include path (header-only templates fall back to a non-mongo compile command and fail with "mongocxx/client.hpp file not found"). Removes the now-dead nested OperationFailed classes and the reducer parse-time comment.
…e results cache.
Add a --unique FIELD aggregation that collects the distinct scalar values of a field across matched records, emitting one {field, value} result document per value through the existing aggregation sinks. Factor the shared field-path tokenization and JSON navigation out of MinMaxAggregation into reusable helpers, and rewrite the reducer output-handler restriction as a whitelist.
Add bool to AggregationValue so --unique treats booleans as distinct scalar values, and flush ResultsCacheSink in batches so high-cardinality aggregations don't buffer every document, surfacing mid-stream database errors.
- Rename the per-record aggregator classes and their variant from `*Aggregation`/`Aggregation` to `*Aggregator`/`Aggregator`, keeping "aggregation" for the operation's data/output (value, result, sink, output handler). - Rename ambiguous `_ms` identifiers to `_millisecs`. - Use `<clp_s/...>` includes, brace-init constexpr members, and tidy doc-comments (possessive wording, hoist implementation notes). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… category comments.
Contributor
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
# Conflicts: # components/core/src/clp_s/AggregationSink.cpp # components/core/src/clp_s/AggregationSink.hpp # components/core/src/clp_s/CommandLineArguments.cpp # components/core/src/clp_s/CommandLineArguments.hpp # components/core/src/clp_s/aggregators.cpp # components/core/src/clp_s/aggregators.hpp # components/core/src/clp_s/archive_constants.hpp
davemarco
changed the title
# Conflicts: # components/core/src/clp_s/aggregators.cpp
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Adds a --count-by search aggregation, grouping matched records by the value of a
field and counting each group, writing the results to stdout or the results cache. Continues
the aggregation work from #2326, #2342, #2345, and #2367.
Outputs one document per distinct value, for both destinations:
{archive_id, field, value, count}
Changes
per-value counts in a std::map.
Checklist
breaking change.
Validation performed
TBU