Add variant_combinations table for efficient matview grouping

[mstaeble]

Summary

• Introduces a variant_combinations table that assigns a SERIAL integer ID to each unique prow_jobs.variants array
• A BEFORE INSERT OR UPDATE trigger on prow_jobs automatically maintains the FK, requiring no Go loader changes
• Denormalizes variant_combination_id into test_daily_summaries for single-stage matview aggregation
• Updates the testReportMatView definition to GROUP BY variant_combination_id (int4) directly from test_daily_summaries, eliminating the previous two-stage aggregation (per-job → per-variant-combo collapse)

Benchmark results (staging, 18K jobs, 45M daily summary rows)

Metric	Before	After	Change
Matview query (7d)	~158s	~69s	56% faster
Pre-agg intermediate rows	8.7M	4.8M (direct)	Eliminated collapse step

Migration details

• Creates variant_combinations table + trigger + backfill (~4.5s on staging)
• Truncates test_daily_summaries (repopulated on next refresh with variant_combination_id set)
• Matview schema auto-detected as changed and recreated by syncPostgresMaterializedViews

Test plan

• Migration up/down/up cycle verified on seed and staging databases
• go build ./... and go test ./pkg/db/... pass
• Full refresh on staging completes successfully (daily summaries + all matviews)
• Matview data verified identical to pre-change (zero row count, value, or key differences on staging)
• Verify sippy-ng frontend renders test reports correctly after matview change

🤖 Generated with Claude Code

@coderabbitai ignore

[openshift-merge-bot]

Pipeline controller notification
This repo is configured to use the pipeline controller. Second-stage tests will be triggered either automatically or after lgtm label is added, depending on the repository configuration. The pipeline controller will automatically detect which contexts are required and will utilize /test Prow commands to trigger the second stage.

For optional jobs, comment /test ? to see a list of all defined jobs. To trigger manually all jobs from second stage use /pipeline required command.

This repository is configured in: automatic mode

[openshift-ci]

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

[coderabbitai]

Walkthrough

Introduces a variant_combinations lookup table that deduplicates prow_jobs.variants arrays into stable integer IDs. A PL/pgSQL trigger maintains the FK on prow_jobs, existing rows are backfilled, and test_daily_summaries receives the same column. Go models, daily summary aggregation SQL, and materialized view definitions are updated to use variant_combination_id instead of raw variants arrays.

Changes

variant_combinations normalization

Layer / File(s)	Summary
variant_combinations schema, trigger, and backfill pkg/db/migrations/000003_create_variant_combinations.up.sql, pkg/db/migrations/000003_create_variant_combinations.down.sql	Creates the variant_combinations table with unique index; adds nullable variant_combination_id to prow_jobs with a BEFORE INSERT OR UPDATE trigger that upserts into variant_combinations and writes the FK back; backfills all existing rows; indexes prow_jobs(variant_combination_id); adds variant_combination_id to test_daily_summaries and truncates it. Down migration reverses all steps in dependency order.
Go model fields for variant_combination_id pkg/db/models/prow.go	Adds nullable VariantCombinationID *uint with gorm:"column:variant_combination_id" tags to both ProwJob and TestDailySummary structs.
Daily summary aggregation keyed on variant_combination_id pkg/db/dailysummary/dailysummary.go	Extends valueColumns to include variant_combination_id; updates buildInsertSQL() to SELECT and GROUP BY pj.variant_combination_id, changing how conflict keys are resolved on upsert.
Materialized view SQL and index columns rewired pkg/db/views.go	Replaces variants with variant_combination_id in IndexColumns for both 7d and 2d matviews; switches pre_agg grouping from prow_job_id to variant_combination_id; changes metric aggregation from SUM() to direct bigint casts; adds LEFT JOIN variant_combinations vc to resolve variants from the lookup table rather than joining prow_jobs.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

• openshift/sippy#3659: Modifies the same daily-summary pipeline and matview logic in pkg/db/dailysummary/dailysummary.go and related view SQL, directly overlapping with this PR's changes to test_daily_summaries aggregation and matview definitions.

Suggested labels

ready-for-human-review

Suggested reviewers

• neisw
• smg247
• xueqzhan
• petr-muller

🚥 Pre-merge checks | ✅ 18 | ❌ 3

❌ Failed checks (3 warnings)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Test Coverage For New Features	⚠️ Warning	No tests cover the new variant_combination SQL/schema logic; the existing tests only exercise refresh control flow and matview ordering.	Add unit tests for the new dailysummary SQL builders and coverage for the variant_combination migration/trigger and matview schema changes, or refactor SQL into testable helpers.
Feature Documentation	⚠️ Warning	docs/features has only job-analysis-symptoms.md; no feature doc was added/updated for the new variant_combination_id test-report flow.	Add or update a docs/features page for the test-report/variant-combination change, covering the new lookup table, trigger, and matview aggregation.

✅ Passed checks (18 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly describes the main change: adding a variant_combinations table to support more efficient materialized-view grouping.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Go Error Handling	✅ Passed	Changed Go code avoids ignored errors/panics, wraps returned errors with context, and guards pointer dereferences where applicable.
Sql Injection Prevention	✅ Passed	PASS: New SQL uses parameterized Exec for runtime values; added migrations/views are static or interpolate internal schema names, not user input.
Excessive Css In React Should Use Styles	✅ Passed	No React/JSX/TSX components or inline style objects are touched; the PR only changes Go and SQL backend files.
Single Responsibility And Clear Naming	✅ Passed	New names are specific and aligned to existing entities; no broad or generic packages, structs, or methods were added.
Stable And Deterministic Test Names	✅ Passed	No test-title changes were introduced: the PR touches only SQL/Go model/view files, and pkg/db test files contain no Ginkgo-style titles.
Test Structure And Quality	✅ Passed	The PR only changes db Go/SQL files; no Ginkgo spec files or test setup code were modified, so the test-quality checklist is not applicable.
Microshift Test Compatibility	✅ Passed	Changed files are DB/model/view/migration code only; no Ginkgo e2e tests or MicroShift-unsupported API references were added.
Single Node Openshift (Sno) Test Compatibility	✅ Passed	No new Ginkgo e2e tests were added; the PR only changes DB migrations/models/views and the new identifiers appear in no *_test.go files.
Topology-Aware Scheduling Compatibility	✅ Passed	Only DB migration/model/view files changed; no deployment manifests, operators, or controllers were modified, so no scheduling constraints were introduced.
Ote Binary Stdout Contract	✅ Passed	Changed files are DB/migration/view code; the only process entrypoint touched is scripts/updatesuites.go, and its diff adds no stdout writes.
Ipv6 And Disconnected Network Test Compatibility	✅ Passed	PR only changes DB migrations/models/views; no Ginkgo e2e tests or IPv4/external-network assumptions were found in touched files.
No-Weak-Crypto	✅ Passed	No weak crypto primitives, custom crypto, or secret/token comparisons appear in the changed DB/schema files; the PR is unrelated to crypto.
Container-Privileges	✅ Passed	No PR changes touch container/K8s manifests, and repo-wide searches found no privileged:true, allowPrivilegeEscalation:true, hostPID/Network/IPC, or SYS_ADMIN settings.
No-Sensitive-Data-In-Logs	✅ Passed	Touched code only adds generic status logs; no new logging of secrets, PII, hostnames, or customer data appears in the PR files.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[openshift-ci]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: mstaeble
Once this PR has been reviewed and has the lgtm label, please assign sosiouxme for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@pkg/db/migrations/000003_create_variant_combinations.up.sql`:
- Around line 18-19: Add a real foreign key for the new variant_combination_id
column in the migration so prow_jobs references variant_combinations(id)
directly. Update the CREATE/ALTER logic around the prow_jobs schema change to
include the FK constraint, keeping the column nullable if needed for truly NULL
variants, and make sure the related migration that builds the lookup/index still
works with the enforced relationship.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository YAML (base), Central YAML (inherited)

Review profile: CHILL

Plan: Enterprise

Run ID: cbd519f7-b82a-4f16-b95b-f25a03d4c4e1

📥 Commits

Reviewing files that changed from the base of the PR and between 9eff376 and 8eb2e21.

📒 Files selected for processing (5)

• pkg/db/dailysummary/dailysummary.go
• pkg/db/migrations/000003_create_variant_combinations.down.sql
• pkg/db/migrations/000003_create_variant_combinations.up.sql
• pkg/db/models/prow.go
• pkg/db/views.go

[openshift-merge-bot]

Scheduling required tests:
/test e2e

[openshift-merge-bot]

Scheduling required tests:
/test e2e

[openshift-ci]

@mstaeble: all tests passed!

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.