drizzle-orm icon indicating copy to clipboard operation
drizzle-orm copied to clipboard

[BUG]: Duplicate migration when using AWS Data API

Open emarref opened this issue 1 year ago • 5 comments

What version of drizzle-orm are you using?

0.25.1

What version of drizzle-kit are you using?

N/A

Describe the Bug

Running migrate() when using a AWS Data API connection fails, as it attempts to re-run the most recently applied migration.

During migrate, the pg-core migrate function retrieves the most recent migration from the __drizzle_migrations table. The result of this is expected to be an object of shape {id: number; hash: string; created_at: string;}[]. However when I console.log("migrations", dbMigrations.length, dbMigrations);, the console displays:

|  +658ms  migrations undefined {
|  +659ms    '$metadata': {
|  +659ms      httpStatusCode: 200,
|  +659ms      requestId: 'e8782039-053f-44bb-9ce5-b21f60c6f6c1',
|  +659ms      extendedRequestId: undefined,
|  +659ms      cfId: undefined,
|  +659ms      attempts: 1,
|  +659ms      totalRetryDelay: 0
|  +659ms    },
|  +659ms    numberOfRecordsUpdated: 0,
|  +659ms    records: [ [ [Object], [Object], [Object] ] ]
|  +659ms  }

So dbMigrations.length is undefined, and dbMigrations is an object that does not fit the expected data shape. Which means that const lastDbMigration = dbMigrations[0]; is undefined. When iterating over the migrations to check whether or not to run the migration, it is truthy and incorrectly runs the migration.

I believe this is due to a bug in the AWS Data API package. When the execute() function returns early if there are no fields to map, it incorrectly returns the parent result of the query, rather than the records returned by the query. Modifying this block to return the records property fixes the issue;

    if (!fields) {
      const result = await client.send(rawQuery);
      return result.records; // <-- Return the value of the records property, rather than the containing result object.
    }

Expected behavior

I should be able to run the migrate() function multiple times, and only those migrations that have not already been executed should be applied.

Environment & setup

Running a migration for the second time when using AWS Data API.

emarref avatar Apr 24 '23 03:04 emarref

I've added a Pull Request with my suggested change at https://github.com/drizzle-team/drizzle-orm/pull/510

emarref avatar Apr 24 '23 03:04 emarref

The pull request I created has not fixed the issue.

In the PgDialect.migrate() method, the following query is executed:

    const dbMigrations = await session.all<{
      id: number;
      hash: string;
      created_at: string;
    }>(
      sql`select id, hash, created_at from "drizzle"."__drizzle_migrations" order by created_at desc limit 1`
    );

The result of which is expected to be an array of the provided type argument { id: number; hash: string; created_at: string; }. However, the result of this query is actually very different:

[
    { longValue: number },
    { stringValue: string },
    { longValue: number }
][]

Which means the result of assigning the lastDbMigration variable is:

[
    { longValue: 2 },
    {
        stringValue: 'b3157b3854d6fa3af5f27456e35ab4b63950c154a5f4d8104cd7c978d9e85a08'
    },
    { longValue: 1682297028889 }
]

The above is the representation of the most recently executed migration. It should be:

{
    id: 2,
    hash: "b3157b3854d6fa3af5f27456e35ab4b63950c154a5f4d8104cd7c978d9e85a08",
    created_at: "1682297028889"
}

So it is failing to determine which migrations to run:

        if (
          !lastDbMigration ||
          Number(lastDbMigration.created_at) < migration.folderMillis
        ) {

Because lastDbMigration is an array with three entries, the first clause !lastDbMigration is untrue so it moves on to the timestamp comparison. In the timestamp comparison, lastDbMigration.created_at is undefined, which makes Number(lastDbMigration.created_at) NaN, and NaN < 1682297028889 results in false.

So the end result is that no migrations are run.

emarref avatar Apr 27 '23 03:04 emarref

Thanks! I guess I know the reason here. Will work on it

AndriiSherman avatar Apr 27 '23 04:04 AndriiSherman

Thanks. Let me know if I can contribute. I couldn't figure it out, but could help if you can point me in the right direction.

emarref avatar Apr 27 '23 05:04 emarref

Ran into this as well. For now, I've pulled out the migrator:

import path from 'node:path';
import { sql } from 'drizzle-orm';
import { readMigrationFiles } from 'drizzle-orm/migrator';
import { RDSData } from '@aws-sdk/client-rds-data';
import { drizzle } from 'drizzle-orm/aws-data-api/pg';

const { RDS_SECRET_ARN, RDS_CLUSTER_ARN, DATABASE_NAME } = process.env;
const missingRdsEnvVars = !RDS_SECRET_ARN || !RDS_CLUSTER_ARN || !DATABASE_NAME;

if (missingRdsEnvVars) {
  throw new Error('RDS environment variables not set');
}

export const db = drizzle(new RDSData({}), {
  database: DATABASE_NAME!,
  resourceArn: RDS_CLUSTER_ARN!,
  secretArn: RDS_SECRET_ARN!,
  logger: true,
});

type DBMigrationQueryResult = [
  { longValue: number }, // id
  { stringValue: string }, // hash
  { longValue: number }, // created_at
];

const run = async () => {
  const migrationsFolder = path.join(__dirname, '..', '..', 'core', 'migrations');

  console.log(`Running migrations from ${migrationsFolder}...`);

  const migrations = readMigrationFiles({ migrationsFolder });

  const migrationTableCreate = sql`
    CREATE TABLE IF NOT EXISTS "drizzle"."__drizzle_migrations" (
    	id SERIAL PRIMARY KEY,
    	hash text NOT NULL,
    	created_at bigint
    )
  `;
  await db.execute(sql`CREATE SCHEMA IF NOT EXISTS "drizzle"`);
  await db.execute(migrationTableCreate);

  const dbMigrations = (await db.execute(
    sql`select id, hash, created_at from "drizzle"."__drizzle_migrations" order by created_at desc limit 1`,
  )) as unknown as DBMigrationQueryResult[];

  const [lastDbMigration] = dbMigrations;
  const createdAt = lastDbMigration && lastDbMigration[2] ? lastDbMigration[2].longValue : 0;
  await db.transaction(async tx => {
    for await (const migration of migrations) {
      if (!lastDbMigration || createdAt < migration.folderMillis) {
        for (const stmt of migration.sql) {
          await tx.execute(sql.raw(stmt));
        }
        await tx.execute(
          sql`insert into "drizzle"."__drizzle_migrations" ("hash", "created_at") values(${migration.hash}, ${migration.folderMillis})`,
        );
      }
    }
  });

  console.log('Finished running migrations');
};

run();

michaelgmcd avatar Apr 29 '23 19:04 michaelgmcd

@michaelgmcd you're a lifesaver. The wheels were spinning for a bit there but your fix worked 👍. I owe you a 🍺

shawngustaw avatar Jul 14 '23 01:07 shawngustaw

This also worked for me, thanks @michaelgmcd

atjeff avatar Aug 14 '23 13:08 atjeff

@michaelgmcd just weighing in with additional thanks--this worked for me as well 🙏 If you are ever in Denver HMU for drinks on me 🍻

travishaby avatar Apr 08 '24 23:04 travishaby

Should work well in the latest versions. Try to upgrade and check. If you still see the issue - feel free to reopen this one

AndriiSherman avatar Jun 02 '24 15:06 AndriiSherman