Add index for update metric calculation spec reporting metrics in create metrics

[tristanvuong2021]

No description provided.

[wfa-reviewable]

This change is 

[SanjayVas]

Reviewed 2 of 2 files at r1, all commit messages.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @tristanvuong2021)

a discussion (no related file):
I'm not quite understanding what this index is for.

Part of this is because I don't quite understand the MetricCalculationSpecReportingMetrics table. It appears to be mapping MetricCalculationSpecs to Metrics within the context of a Report. What's odd to me is that CreateMetricRequestId is part of the primary key rather than MetricId. MetricId is also nullable. Is the idea that this row is created separately from Metric creation, and the Metric is created later? Therefore the (MeasurementConsumerId, CreateMetricRequestId) tuple uniquely identifies a future or existing Metric? Why does it have ReportingSetId as part of its primary key then, as each Metric is tied to exactly one ReportingSet?

---

---

src/main/resources/reporting/postgres/add-index-on-metric-calculation-spec-reporting-metrics-table-for-update.sql line 40 at r1 (raw file):

-- changeset tristanvuong2021:add-metric-calculation-spec-reporting-metrics-create-metric-request-id-index dbms:postgresl
CREATE INDEX metric_calculation_spec_reporting_metrics_create_metric_request_id

Index names tend to be UpperCamelCase.

Suggestion:

MetricCalculationSpecReportingMetricsByCreateMetricRequestId

---

src/main/resources/reporting/postgres/changelog-v2.yaml line 48 at r1 (raw file):

- include:
    file: add-index-on-metric-calculation-spec-reporting-metrics-table-for-update.sql
    relativeToChangeLogFile: true    

nit: trailing whitespace

Code quote:

····

[tristanvuong2021]

Reviewable status: 1 of 3 files reviewed, 2 unresolved discussions (waiting on @SanjayVas)

a discussion (no related file):

Previously, SanjayVas (Sanjay Vasandani) wrote…

I'm not quite understanding what this index is for.

Part of this is because I don't quite understand the MetricCalculationSpecReportingMetrics table. It appears to be mapping MetricCalculationSpecs to Metrics within the context of a Report. What's odd to me is that CreateMetricRequestId is part of the primary key rather than MetricId. MetricId is also nullable. Is the idea that this row is created separately from Metric creation, and the Metric is created later? Therefore the (MeasurementConsumerId, CreateMetricRequestId) tuple uniquely identifies a future or existing Metric? Why does it have ReportingSetId as part of its primary key then, as each Metric is tied to exactly one ReportingSet?

Metrics are created separately from the Report and the rows in this table are first created when creating a Report before being updated later once the metrics are created.

ReportingSetId isn't needed as part of the primary key. Removed it.

---

---

src/main/resources/reporting/postgres/add-index-on-metric-calculation-spec-reporting-metrics-table-for-update.sql line 40 at r1 (raw file):

Previously, SanjayVas (Sanjay Vasandani) wrote…

Index names tend to be UpperCamelCase.

In postgres, index names tend to use this convention because of lack of case sensitivity

[SanjayVas]

Reviewed 2 of 2 files at r2, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @tristanvuong2021)

---

src/main/resources/reporting/postgres/drop-reporting-set-id-from-metric-calculation-spec-reporting-metrics-table-primary-key.sql line 39 at r2 (raw file):

--           └── MetricCalculationSpecReportingMetrics

-- changeset tristanvuong2021:drop-reporting-set-id-column-metric-calculation-spec-reporting-metrics-table-primary-key dbms:postgresql

nit: you don't need to be this verbose in the file/changeset name. You can add a -- comment: line to the changeset to explain what's happening.

[stevenwarejones]

Reviewed 2 of 2 files at r2, 1 of 1 files at r3, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @tristanvuong2021)