Skip to content

Downgrade script is wrong. Data was lost. #183

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

Open
KES777 opened this issue Mar 26, 2025 · 5 comments · May be fixed by #184
Open

Downgrade script is wrong. Data was lost. #183

KES777 opened this issue Mar 26, 2025 · 5 comments · May be fixed by #184

Comments

@KES777
Copy link
Contributor

KES777 commented Mar 26, 2025

Upgrade is correct:

diff --git a/share/migrations/PostgreSQL/upgrade/4-5/001-auto.sql b/share/migrations/Post>
new file mode 100644
index 000000000..69d445035
--- /dev/null
+++ b/share/migrations/PostgreSQL/upgrade/4-5/001-auto.sql
@@ -0,0 +1,15 @@
+-- Convert schema '/home/kes/work/projects/bot/app/share/migrations/_source/deploy/4/001>
+
+;
+BEGIN;
+
+;
+ALTER TABLE "tracker" RENAME COLUMN "me_id" TO "tracker_user_xid";
+
+;
+ALTER TABLE "user" RENAME COLUMN "telegram_xid" TO "telegram_user_xid";
+
+;
+
+COMMIT;
+

For downgrade I also expect RENAME COLUMN but DROP is used, which cause data loss.

diff --git a/share/migrations/PostgreSQL/downgrade/5-4/001-auto.sql b/share/migrations/Po>
new file mode 100644
index 000000000..7ee5f5780
--- /dev/null
+++ b/share/migrations/PostgreSQL/downgrade/5-4/001-auto.sql
@@ -0,0 +1,21 @@
+-- Convert schema '/home/kes/work/projects/bot/app/share/migrations/_source/deploy/5/001>
+
+;
+BEGIN;
+
+;
+ALTER TABLE "tracker" DROP COLUMN "tracker_user_xid";
+
+;
+ALTER TABLE "tracker" ADD COLUMN "me_id" integer NOT NULL;
+
+;
+ALTER TABLE "user" DROP COLUMN "telegram_user_xid";
+
+;
+ALTER TABLE "user" ADD COLUMN "telegram_xid" integer NOT NULL;
+
+;
+
+COMMIT;
+

How to reproduce:

diff --git a/lib/Db/Result/Tracker.pm b/lib/Db/Result/Tracker.pm
index 7f082852a..3f933fbab 100644
--- a/lib/Db/Result/Tracker.pm
+++ b/lib/Db/Result/Tracker.pm
@@ -34,8 +34,11 @@ $X->add_columns(
         data_type =>  'text',
     },
     # User id at tracker
-    me_id =>  {
+    tracker_user_xid =>  {
         data_type =>  'int',
+        extra     =>  {
+            renamed_from => 'me_id',
+        },
     },
     options =>  {
         data_type        =>  'json',
diff --git a/lib/Db/Result/User.pm b/lib/Db/Result/User.pm
index a5b92e7bd..d5db08b08 100644
--- a/lib/Db/Result/User.pm
+++ b/lib/Db/Result/User.pm
@@ -22,8 +22,11 @@ $X->add_columns(
     passw =>  {
         data_type =>  'text',
     },
-    telegram_xid => {
+    telegram_user_xid => {
         data_type => 'int',
+        extra     =>  {
+            renamed_from => 'telegram_xid',
+        },
     },
 );
@rabbiveesh
Copy link
Contributor

rabbiveesh commented Mar 26, 2025 via email

@rabbiveesh
Copy link
Contributor

Actually, i think the issue is not here; just reviewed the code; the problem is likely in https://github.com/frioux/DBIx-Class-DeploymentHandler, b/c i just realized that SQLT doesn't do anything magical for a "downgrade", it just diffs it as expected.
Would you be able to share a repo to simply reproduce the bug? b/c i suspect that DBICDH is the one not accounting for needing to reverse the renamed_from for the context of the downgrade

@KES777
Copy link
Contributor Author

KES777 commented Mar 26, 2025

I do not have that repo at that moment, but I would be happy to work with you and implement reproduction step or even test case for that.

Give me please a couple of days. Thanks.

@rabbiveesh
Copy link
Contributor

rabbiveesh commented Mar 26, 2025 via email

@KES777
Copy link
Contributor Author

KES777 commented Mar 27, 2025

Done, but tests were not extended. I do not see the right place where to implement that. The best place could be 30sqlt-new-diff-pgsql.t, but I do not understand how to describe there ->extra->{renamed_from}.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants