jjn1056 / DBIx-Class-Migration

Use DBIC::DeploymentHandler and DBIC::Fixtures together for a sane database versioning workflow
33 stars 41 forks source link

Downgrade script is wrong. Data was lost. #145

Open KES777 opened 2 years ago

KES777 commented 2 years ago

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 commented 2 years ago

Hey, could you cross post this as an issue in SQL::Translator? That's what generates the diffs.

On Sat, Jul 30, 2022, 23:56 Eugen Konkov @.***> wrote:

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',+ }, }, );

— Reply to this email directly, view it on GitHub https://github.com/jjn1056/DBIx-Class-Migration/issues/145, or unsubscribe https://github.com/notifications/unsubscribe-auth/AFURPKQ5IGLAJWGV4FZEU53VWWJILANCNFSM55DZCPSA . You are receiving this because you are subscribed to this thread.Message ID: @.***>

KES777 commented 2 years ago

@rabbiveesh https://rt.cpan.org/Ticket/Display.html?id=143916

rabbiveesh commented 2 years ago

I meant on github; RT is .... RT. The interface is confusing enough that most tickets are accidentally filed on 0.05 rather than the thing you started by filing against.

I'm moving my discussion there, b/c as far as DBICM is concerned, it warns you that you should review the auto-generated diffs.