Conversation
…te linking to spec records. Updated forecast name and designator handling to split concatenated names.
…onflicts for TS migration
| and forecast_date = old_ts.forecast_date | ||
| and issue_date = old_ts.issue_date | ||
| fetch next 1 row only; | ||
| l_new_inst.issue_date_time := l_version_date; |
There was a problem hiding this comment.
Wouldn't we need to create an AT_FCST_INST row with an ISSUE_DATE_TIME for each time series VERSION_DATE in this case, not just the first one we found.
There was a problem hiding this comment.
wouldn't that make it where there were multiple forecasts given the different ts version vs. still having a single forecast with multiple data?
There was a problem hiding this comment.
My comment was insufficient.
We should only create an AT_FCST_INST row when the old structure has multiple AT_FORECAST_TS records for the same AT_FORECAST_SPEC, TS_CODE, FORECAST_DATE, and ISSUE_DATE, differing only by VERSION_DATE (i.e., multiple version dates for a single time series in a forecast instance). The tradeoff is between:
- creating forecast issue dates that didn't previously exist
- disassociating existing forecast time series from any forecast
This would require a more complex test when iterating over AT_FORECAST_TS to identify whether this condition exists. If we can verify that this condition doesn't exist in existing data, then we could drop this requirement.
| -- cwms_ts.CHANGE_VERSION_DATE(l_ts_row.ts_code, l_ts_row.version_date, old_ts.issue_date, l_ts_start, l_ts_end); | ||
| dbms_output.put_line('change version date from: ' || l_ts_row.forecast_date || ' to ' || old_ts.issue_date); | ||
| end if; | ||
| if l_distinct_count > 1 then |
There was a problem hiding this comment.
Replace the two if blocks with a single if/elsif block or a case block to make it clear that it is a single decision block.
| from av_tsv | ||
| where ts_code = l_ts_row.ts_code; | ||
| -- cwms_ts.CHANGE_VERSION_DATE(l_ts_row.ts_code, l_ts_row.version_date, old_ts.issue_date, l_ts_start, l_ts_end); | ||
| dbms_output.put_line('change version date from: ' || l_ts_row.forecast_date || ' to ' || old_ts.issue_date); |
There was a problem hiding this comment.
Don't we need to actually change the version date and not just output that it should be changed?
There was a problem hiding this comment.
I would agree it should change. Or at least be duplicated (probably don't want to delete the old time series data.)
We should also make use of the blob/text notes/key->value pairs in the new system to record exactly what we did. Entirely possible what we do here will be effectively wrong, and we definitely want that record, especially if we're going to delete the old tables.
There was a problem hiding this comment.
Looks like a remnant from my local testing. I've added back the CHANGE_VERSION_DATE call and removed the DBMS output line.
Mike, are you referring to the blob description text? Also, what information should be included in that record?
There was a problem hiding this comment.
I would avoid using the AT_FCST_INST.BLOB_FILE for this for instances that already have a non-null entry. To be consistent we could use AT_FCST_INST.NOTES if the conversion notes can always be contained in 256 characters; otherwise we should use the AT_FCST_INFO table with a key of 'CONVERSION NOTES' or something similar.
| execute immediate | ||
| 'insert into at_fcst_location (fcst_spec_code, location_code, sort_order)' || | ||
| 'values (:1, :2, :3)' | ||
| using l_new_spec.fcst_spec_code, old_spec.target_location_code, 0; |
There was a problem hiding this comment.
Sort order should be -1 if location_code == AT_FORECAST_SPEC.TARGET_LOCATION_CODE
There was a problem hiding this comment.
In this case, would we be checking the AT_FCST_SPEC table for existing entries of the same location code? Or just using -1 for all entries, since the inserted location code is the TARGET_LOCATION_CODE?
There was a problem hiding this comment.
Sorry - you're correct on the way it's currently written. What we should do is:
- Move the location insertion block to inside the time series insertion block
- Get the location code for each ts code
- Merge that location (using
mergeorinsertwith exception handling for duplicates), setting the sort order to -1 if it's the same asold.spec.target_location_codeand 0 otherwise.
MikeNeilson
left a comment
MikeNeilson
left a comment
There was a problem hiding this comment.
Okay, I agree with the overall logic.
And the basic mechanisms of traceability in the changes.
But we are forcibly altering some time series (appropriately, but we're still doing it.)
So, in additional to all that. I think we should also extract and output the data as-is. E.g. just like a simple extract that's just "csv export from the original views of what everything would be and dumped in."
Doesn't have to be stored in the database itself, on the disk for when it's run is fine.
And yes I am just being paranoid.
…to feature/forecast_api_migration
3 participants
Script to migrate data from
AT_FORECAST_*toAT_FCST_*tables.Resolves #192