Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove FK from continuous_agg_migrate_plan #5664

Merged

Conversation

fabriziomello
Copy link
Contributor

@fabriziomello fabriziomello commented May 5, 2023

During the cagg_migrate execution if the user set the drop_old parameter to true the routine will drop the old Continuous Aggregate leading to an inconsistent state because the catalog code don't handle this table as a normal catalog table so the records are not removed when dropping a Continuous Aggregate. The same problem will happen if you manually drop the old Continuous Aggregate after the migration.

Fixed it by removing the useless Foreign Key and also adding another column named user_view_definition to the main plan table just to store the original user view definition for troubleshooting purposes.

Disable-check: force-changelog-changed

Fixed #5662

@github-actions
Copy link

github-actions bot commented May 5, 2023

@lkshminarayanan, @sb230132: please review this pull request.

Powered by pull-review

@codecov
Copy link

codecov bot commented May 5, 2023

Codecov Report

Merging #5664 (623d257) into main (8e69a99) will increase coverage by 0.36%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main    #5664      +/-   ##
==========================================
+ Coverage   90.97%   91.34%   +0.36%     
==========================================
  Files         230      228       -2     
  Lines       54473    51920    -2553     
==========================================
- Hits        49557    47424    -2133     
+ Misses       4916     4496     -420     

see 132 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@fabriziomello fabriziomello force-pushed the remove_fk_from_cagg_migrate_plan branch 2 times, most recently from daa5478 to c6c586e Compare May 5, 2023 18:14
During the `cagg_migrate` execution if the user set the `drop_old`
parameter to `true` the routine will drop the old Continuous Aggregate
leading to an inconsistent state because the catalog code don't handle
this table as a normal catalog table so the records are not removed
when dropping a Continuous Aggregate. The same problem will happen if
you manually drop the old Continuous Aggregate after the migration.

Fixed it by removing the useless Foreign Key and also adding another
column named `user_view_definition` to the main plan table just to store
the original user view definition for troubleshooting purposes.

Fixed timescale#5662
@fabriziomello fabriziomello force-pushed the remove_fk_from_cagg_migrate_plan branch from c6c586e to 623d257 Compare May 8, 2023 12:19
@konskov konskov self-requested a review May 10, 2023 07:16
@fabriziomello fabriziomello merged commit f250eaa into timescale:main May 10, 2023
41 checks passed
@timescale-automation
Copy link

Automated backport to 2.10.x not done: cherry-pick failed.

Git status

HEAD detached at origin/2.10.x
You are currently cherry-picking commit f250eaa6.
  (fix conflicts and run "git cherry-pick --continue")
  (use "git cherry-pick --skip" to skip this patch)
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)

Changes to be committed:
	modified:   sql/cagg_migrate.sql
	modified:   sql/pre_install/tables.sql
	modified:   tsl/test/expected/cagg_migrate.out
	modified:   tsl/test/expected/cagg_migrate_dist_ht.out
	modified:   tsl/test/sql/include/cagg_migrate_common.sql

Unmerged paths:
  (use "git add <file>..." to mark resolution)
	both modified:   sql/updates/latest-dev.sql
	both modified:   sql/updates/reverse-dev.sql


Job log

@timescale-automation timescale-automation added the auto-backport-not-done Automated backport of this PR has failed non-retriably (e.g. conflicts) label May 10, 2023
@fabriziomello fabriziomello added disable-auto-backport Do not automatically backport this PR or fix of this issue and removed disable-auto-backport Do not automatically backport this PR or fix of this issue labels May 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-backport-not-done Automated backport of this PR has failed non-retriably (e.g. conflicts) continuous_aggregate disable-auto-backport Do not automatically backport this PR or fix of this issue Team: Core Database
Projects
None yet
4 participants