optimizise prov summary for larger imports - #668
Conversation
Not up to standards ⛔🔴 Issues
|
| Category | Results |
|---|---|
| Compatibility | 25 high |
🟢 Metrics 0 complexity
Metric Results Complexity 0
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
There was a problem hiding this comment.
Code Review
This pull request optimizes the provenance summary generation by aggregating observations per time series directly in Spanner before transferring the data to BigQuery, replacing the temp_obs_flat table with temp_series_summary. Corresponding tests and a new helper SQL script are also updated. The review feedback correctly identifies a critical GoogleSQL syntax error across multiple files where columns specified in the USING clause of a JOIN are incorrectly qualified with a table alias, which would lead to query compilation failures.
| '''SELECT | ||
| variable_measured, | ||
| entity1, | ||
| extra_entities_id, | ||
| facet_id, | ||
| provenance, | ||
| observation_period, | ||
| measurement_method, | ||
| unit, | ||
| facet, | ||
| date, | ||
| value | ||
| FROM TimeSeries | ||
| JOIN Observation USING (variable_measured, entity1, extra_entities_id, facet_id) | ||
| WHERE provenance IN ({provenances_str}) '''); | ||
| ts.variable_measured, | ||
| ts.entity1 AS observation_about, | ||
| ts.facet_id, | ||
| ts.provenance, | ||
| ANY_VALUE(ts.observation_period) AS observation_period, | ||
| ANY_VALUE(ts.measurement_method) AS measurement_method, | ||
| ANY_VALUE(ts.unit) AS unit, | ||
| ANY_VALUE(JSON_VALUE(ts.facet, '$.scalingFactor')) AS scaling_factor, | ||
| ANY_VALUE(SAFE_CAST(JSON_VALUE(ts.facet, '$.isDcAggregate') AS BOOL)) AS is_dc_aggregate, | ||
| MIN(obs.date) AS min_date, | ||
| MAX(obs.date) AS max_date, | ||
| MIN(SAFE_CAST(obs.value AS FLOAT64)) AS min_value, | ||
| MAX(SAFE_CAST(obs.value AS FLOAT64)) AS max_value, | ||
| COUNT(*) AS observation_count | ||
| FROM TimeSeries AS ts | ||
| JOIN Observation AS obs | ||
| USING (variable_measured, entity1, extra_entities_id, facet_id) | ||
| WHERE ts.provenance IN ({provenances_str}) | ||
| GROUP BY | ||
| ts.variable_measured, | ||
| ts.entity1, | ||
| ts.extra_entities_id, | ||
| ts.facet_id, | ||
| ts.provenance '''); |
There was a problem hiding this comment.
In GoogleSQL (used by both BigQuery and Spanner), columns specified in the USING clause of a JOIN (such as variable_measured, entity1, extra_entities_id, and facet_id) cannot be qualified with a table alias (like ts.) in the SELECT list or GROUP BY clause. Doing so will result in a query compilation error. Please remove the ts. prefix from these columns.
'''SELECT
variable_measured,
entity1 AS observation_about,
facet_id,
ts.provenance,
ANY_VALUE(ts.observation_period) AS observation_period,
ANY_VALUE(ts.measurement_method) AS measurement_method,
ANY_VALUE(ts.unit) AS unit,
ANY_VALUE(JSON_VALUE(ts.facet, '$.scalingFactor')) AS scaling_factor,
ANY_VALUE(SAFE_CAST(JSON_VALUE(ts.facet, '$.isDcAggregate') AS BOOL)) AS is_dc_aggregate,
MIN(obs.date) AS min_date,
MAX(obs.date) AS max_date,
MIN(SAFE_CAST(obs.value AS FLOAT64)) AS min_value,
MAX(SAFE_CAST(obs.value AS FLOAT64)) AS max_value,
COUNT(*) AS observation_count
FROM TimeSeries AS ts
JOIN Observation AS obs
USING (variable_measured, entity1, extra_entities_id, facet_id)
WHERE ts.provenance IN ({provenances_str})
GROUP BY
variable_measured,
entity1,
extra_entities_id,
facet_id,
ts.provenance ''';| '''SELECT | ||
| variable_measured, | ||
| entity1, | ||
| facet_id, | ||
| provenance, | ||
| observation_period, | ||
| measurement_method, | ||
| unit, | ||
| facet, | ||
| date, | ||
| value | ||
| FROM TimeSeries | ||
| JOIN Observation USING (variable_measured, entity1, extra_entities_id, facet_id) | ||
| WHERE provenance IN ({provenances_str}) '''); | ||
| ts.variable_measured, | ||
| ts.entity1 AS observation_about, | ||
| ts.facet_id, | ||
| ts.provenance, | ||
| ANY_VALUE(ts.observation_period) AS observation_period, | ||
| ANY_VALUE(ts.measurement_method) AS measurement_method, | ||
| ANY_VALUE(ts.unit) AS unit, | ||
| ANY_VALUE(JSON_VALUE(ts.facet, '$.scalingFactor')) AS scaling_factor, | ||
| ANY_VALUE(SAFE_CAST(JSON_VALUE(ts.facet, '$.isDcAggregate') AS BOOL)) AS is_dc_aggregate, | ||
| MIN(obs.date) AS min_date, | ||
| MAX(obs.date) AS max_date, | ||
| MIN(SAFE_CAST(obs.value AS FLOAT64)) AS min_value, | ||
| MAX(SAFE_CAST(obs.value AS FLOAT64)) AS max_value, | ||
| COUNT(*) AS observation_count | ||
| FROM TimeSeries AS ts | ||
| JOIN Observation AS obs | ||
| USING (variable_measured, entity1, extra_entities_id, facet_id) | ||
| WHERE ts.provenance IN ({provenances_str}) | ||
| GROUP BY | ||
| ts.variable_measured, | ||
| ts.entity1, | ||
| ts.extra_entities_id, | ||
| ts.facet_id, | ||
| ts.provenance '''); |
There was a problem hiding this comment.
In GoogleSQL (used by both BigQuery and Spanner), columns specified in the USING clause of a JOIN (such as variable_measured, entity1, extra_entities_id, and facet_id) cannot be qualified with a table alias (like ts.) in the SELECT list or GROUP BY clause. Doing so will result in a query compilation error. Please remove the ts. prefix from these columns.
'''SELECT
variable_measured,
entity1 AS observation_about,
facet_id,
ts.provenance,
ANY_VALUE(ts.observation_period) AS observation_period,
ANY_VALUE(ts.measurement_method) AS measurement_method,
ANY_VALUE(ts.unit) AS unit,
ANY_VALUE(JSON_VALUE(ts.facet, '$.scalingFactor')) AS scaling_factor,
ANY_VALUE(SAFE_CAST(JSON_VALUE(ts.facet, '$.isDcAggregate') AS BOOL)) AS is_dc_aggregate,
MIN(obs.date) AS min_date,
MAX(obs.date) AS max_date,
MIN(SAFE_CAST(obs.value AS FLOAT64)) AS min_value,
MAX(SAFE_CAST(obs.value AS FLOAT64)) AS max_value,
COUNT(*) AS observation_count
FROM TimeSeries AS ts
JOIN Observation AS obs
USING (variable_measured, entity1, extra_entities_id, facet_id)
WHERE ts.provenance IN ({provenances_str})
GROUP BY
variable_measured,
entity1,
extra_entities_id,
facet_id,
ts.provenance ''';| '''SELECT | ||
| ts.variable_measured, | ||
| ts.entity1 AS observation_about, | ||
| ts.facet_id, | ||
| ts.provenance, | ||
| ANY_VALUE(ts.observation_period) AS observation_period, | ||
| ANY_VALUE(ts.measurement_method) AS measurement_method, | ||
| ANY_VALUE(ts.unit) AS unit, | ||
| ANY_VALUE(JSON_VALUE(ts.facet, '$.scalingFactor')) AS scaling_factor, | ||
| ANY_VALUE(SAFE_CAST(JSON_VALUE(ts.facet, '$.isDcAggregate') AS BOOL)) AS is_dc_aggregate, | ||
| MIN(obs.date) AS min_date, | ||
| MAX(obs.date) AS max_date, | ||
| MIN(SAFE_CAST(obs.value AS FLOAT64)) AS min_value, | ||
| MAX(SAFE_CAST(obs.value AS FLOAT64)) AS max_value, | ||
| COUNT(*) AS observation_count | ||
| FROM TimeSeries AS ts | ||
| JOIN Observation AS obs | ||
| USING (variable_measured, entity1, extra_entities_id, facet_id) | ||
| WHERE ts.provenance = 'dc/base/CensusACS5YearSurvey' | ||
| GROUP BY | ||
| ts.variable_measured, | ||
| ts.entity1, | ||
| ts.extra_entities_id, | ||
| ts.facet_id, | ||
| ts.provenance''' |
There was a problem hiding this comment.
In GoogleSQL (used by both BigQuery and Spanner), columns specified in the USING clause of a JOIN (such as variable_measured, entity1, extra_entities_id, and facet_id) cannot be qualified with a table alias (like ts.) in the SELECT list or GROUP BY clause. Doing so will result in a query compilation error. Please remove the ts. prefix from these columns.
'''SELECT
variable_measured,
entity1 AS observation_about,
facet_id,
ts.provenance,
ANY_VALUE(ts.observation_period) AS observation_period,
ANY_VALUE(ts.measurement_method) AS measurement_method,
ANY_VALUE(ts.unit) AS unit,
ANY_VALUE(JSON_VALUE(ts.facet, '$.scalingFactor')) AS scaling_factor,
ANY_VALUE(SAFE_CAST(JSON_VALUE(ts.facet, '$.isDcAggregate') AS BOOL)) AS is_dc_aggregate,
MIN(obs.date) AS min_date,
MAX(obs.date) AS max_date,
MIN(SAFE_CAST(obs.value AS FLOAT64)) AS min_value,
MAX(SAFE_CAST(obs.value AS FLOAT64)) AS max_value,
COUNT(*) AS observation_count
FROM TimeSeries AS ts
JOIN Observation AS obs
USING (variable_measured, entity1, extra_entities_id, facet_id)
WHERE ts.provenance = 'dc/base/CensusACS5YearSurvey'
GROUP BY
variable_measured,
entity1,
extra_entities_id,
facet_id,
ts.provenance'''
No description provided.