diff --git a/src/ab-testing/entities/experiment-variant.entity.ts b/src/ab-testing/entities/experiment-variant.entity.ts index 00c68e36..cf7ab129 100644 --- a/src/ab-testing/entities/experiment-variant.entity.ts +++ b/src/ab-testing/entities/experiment-variant.entity.ts @@ -8,14 +8,46 @@ import { ManyToOne, OneToMany, VersionColumn, + Index, } from 'typeorm'; import { Experiment } from './experiment.entity'; import { VariantMetric } from './variant-metric.entity'; /** * Represents the experiment Variant entity. + * + * ## Indexes (#1223) + * + * Every real query path against this entity loads variants through the + * `experiment` relation (`experimentRepository.findOne({ relations: [...] })` + * across `ab-testing.service.ts`, `experiments/experiment.service.ts`, + * `analysis/statistical-analysis.service.ts`, + * `automation/automated-decision.service.ts`, and + * `reporting/ab-testing-reports.service.ts`), then filters the loaded array + * in-memory for the control variant (`v.isControl`) or the winning variant + * (`v.isWinner`) — every one of those service methods does this. There is + * no direct `variantRepository.find({ where: ... })` call anywhere in this + * codebase filtering on anything else. + * + * The two composite indexes below match that access shape directly — + * `(experiment, isControl)` and `(experiment, isWinner)` — rather than a + * plain single-column index on `experiment` alone: leftmost-prefix lookup + * means either composite already serves a plain "all variants for this + * experiment" query just as well as a dedicated `experiment`-only index + * would, so adding one of those *in addition* would only be a redundant, + * un-selective duplicate (`isControl`/`isWinner` are booleans — indexing + * either alone, without the `experiment` prefix, has essentially no + * selectivity and no query in this codebase would use it that way). + * + * See `src/migrations/1802000000000-add-experiment-variant-indexes.ts` for + * the migration that creates the same two indexes on an existing database + * (`synchronize` is `false` in `src/config/datasource.ts`, so the + * `@Index` decorators below are the schema's documentation of intent, not + * what actually creates the indexes). */ @Entity({ name: 'experiment_variants' }) +@Index(['experiment', 'isControl']) +@Index(['experiment', 'isWinner']) export class IExperimentVariant { @PrimaryGeneratedColumn('uuid') id: string; diff --git a/src/migrations/1802000000000-add-experiment-variant-indexes.ts b/src/migrations/1802000000000-add-experiment-variant-indexes.ts new file mode 100644 index 00000000..cc66c70a --- /dev/null +++ b/src/migrations/1802000000000-add-experiment-variant-indexes.ts @@ -0,0 +1,35 @@ +import { MigrationInterface, QueryRunner } from 'typeorm'; + +/** + * #1223 — src/ab-testing/entities/experiment-variant.entity.ts declared no + * indexes. Every real query path against `experiment_variants` loads + * variants through the `experiment` foreign key (via the `experiment` + * relation on `IExperimentVariant`, e.g. + * `experimentRepository.findOne({ relations: [...] })` across the + * ab-testing services), then filters in-memory for the control variant + * (`isControl`) or the winning variant (`isWinner`). These two composite + * indexes match that shape directly. See the `@Index` decorators and their + * accompanying comment on `IExperimentVariant` for the full rationale, + * including why this is two composites rather than three separate indexes + * (a plain `experimentId`-only index would be redundant given either + * composite already serves that lookup via its leftmost column). + */ +export class AddExperimentVariantIndexes1802000000000 implements MigrationInterface { + public async up(queryRunner: QueryRunner): Promise { + await queryRunner.query( + 'CREATE INDEX IF NOT EXISTS "IDX_experiment_variants_experiment_isControl" ON "experiment_variants" ("experimentId", "isControl")', + ); + await queryRunner.query( + 'CREATE INDEX IF NOT EXISTS "IDX_experiment_variants_experiment_isWinner" ON "experiment_variants" ("experimentId", "isWinner")', + ); + } + + public async down(queryRunner: QueryRunner): Promise { + await queryRunner.query( + 'DROP INDEX IF EXISTS "IDX_experiment_variants_experiment_isWinner"', + ); + await queryRunner.query( + 'DROP INDEX IF EXISTS "IDX_experiment_variants_experiment_isControl"', + ); + } +}