Skip to content

clarify batch aggregation scrubbing - #4911

Open
tgeoghegan wants to merge 1 commit into
timg/leader-selected-collection-job-idempotency-key-collector-integration-interopfrom
timg/clarify-batch-aggregation-scrubbing
Open

tgeoghegan wants to merge 1 commit into
timg/leader-selected-collection-job-idempotency-key-collector-integration-interopfrom
timg/clarify-batch-aggregation-scrubbing

Conversation

@tgeoghegan

@tgeoghegan tgeoghegan commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor

Adds some doccomments on items related to scrubbed batch aggregations, as well as tidying up comments in the DB schema so they don't scroll off the right side of one's editor window.

@tgeoghegan tgeoghegan added the allow-changed-migrations Override the ci-migrations check to allow migrations that have changed. label Oct 14, 2026
@tgeoghegan
tgeoghegan force-pushed the timg/clarify-batch-aggregation-scrubbing branch 2 times, most recently from b7cc59a to f14dbe9 Compare October 19, 2026 17:13
Adds some doccomments on items related to scrubbed batch aggregations,
as well as tidying up comments in the DB schema so they don't scroll off
the right side of one's editor window.
@tgeoghegan
tgeoghegan force-pushed the timg/clarify-batch-aggregation-scrubbing branch from f14dbe9 to c7ea143 Compare October 19, 2026 18:18
@tgeoghegan
tgeoghegan changed the base branch from main to timg/leader-selected-collection-job-idempotency-key-collector-integration-interop October 19, 2026 18:18
@tgeoghegan
tgeoghegan added this pull request to stack #4914 October 19, 2026 18:18
@tgeoghegan
tgeoghegan marked this pull request as ready for review October 19, 2026 18:18
@tgeoghegan
tgeoghegan requested a review from a team as a code owner October 19, 2026 18:18
// ReportAggregationStateCode exists alongside the public ReportAggregationState because there is no
// apparent way to denote a Postgres enum literal without deriving FromSql/ToSql on a Rust enum
// type, but it is not possible to derive FromSql/ToSql on a non-C-style enum.
/// ReportAggregationStateCode corresponds to the `batch_aggregation_state` enum.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/// ReportAggregationStateCode corresponds to the `batch_aggregation_state` enum.
/// BatchAggregationStateCode corresponds to the `batch_aggregation_state` enum.

// type, but it is not possible to derive FromSql/ToSql on a non-C-style enum.
/// ReportAggregationStateCode corresponds to the `batch_aggregation_state` enum.
///
/// It exists alongside the public ReportAggregationState because there is no apparent way to denote

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/// It exists alongside the public ReportAggregationState because there is no apparent way to denote
/// It exists alongside the public BatchAggregationState because there is no apparent way to denote

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

allow-changed-migrations Override the ci-migrations check to allow migrations that have changed.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants