cli icon indicating copy to clipboard operation
cli copied to clipboard

cli appending "public" schema in front of database name during migrations.

Open buildjosh opened this issue 8 years ago • 22 comments

So I'm using the latest cli and latest sequelize.

Originally I had my migration file look like this:

'use strict';

module.exports = {
  up: function (queryInterface, Sequelize) {
    return queryInterface.addColumn('accounts', 'agentId', {
      type: Sequelize.STRING,
      allowNull: false,
      defaultValue: 'NOT_TRACKED'
    });
  },

  down: function (queryInterface, Sequelize) {
    /*
      Add reverting commands here.
      Return a promise to correctly handle asynchronicity.

      Example:
      return queryInterface.dropTable('users');
    */
  }
};

But the SQL query was ignoring my database name and using "public.accounts":

{ [SequelizeDatabaseError: relation "public.accounts" does not exist]

So I tried changing the database to "crm.public.accounts" in the migration file, but that resulted in the following.

Config file looks like this:

{
  "development": {
    "use_env_variable": "DATABASE_URL",
    "dialect": "postgres"
  },
  "production": {
    "use_env_variable": "DATABASE_URL",
    "dialect": "postgres"
  }
}

DATABASE_URL looks like:

postgres://josh@localhost:5432/crm

When trying to run migrations:

$  ./node_modules/.bin/sequelize db:migrate

I'm getting this:

{ [SequelizeDatabaseError: relation "public.crm.public.accounts" does not exist]
  name: 'SequelizeDatabaseError',
  message: 'relation "public.crm.public.accounts" does not exist',
  parent: 
   { [error: relation "public.crm.public.accounts" does not exist]
     name: 'error',
     length: 117,
     severity: 'ERROR',
     code: '42P01',
     detail: undefined,
     hint: undefined,
     position: undefined,
     internalPosition: undefined,
     internalQuery: undefined,
     where: undefined,
     schema: undefined,
     table: undefined,
     column: undefined,
     dataType: undefined,
     constraint: undefined,
     file: 'namespace.c',
     line: '413',
     routine: 'RangeVarGetRelidExtended',
     sql: 'ALTER TABLE "public"."crm.public.accounts" ADD COLUMN "agentId" VARCHAR(255) NOT NULL DEFAULT \'NOT_TRACKED\';' },
  original: 
   { [error: relation "public.crm.public.accounts" does not exist]
     name: 'error',
     length: 117,
     severity: 'ERROR',
     code: '42P01',
     detail: undefined,
     hint: undefined,
     position: undefined,
     internalPosition: undefined,
     internalQuery: undefined,
     where: undefined,
     schema: undefined,
     table: undefined,
     column: undefined,
     dataType: undefined,
     constraint: undefined,
     file: 'namespace.c',
     line: '413',
     routine: 'RangeVarGetRelidExtended',
     sql: 'ALTER TABLE "public"."crm.public.accounts" ADD COLUMN "agentId" VARCHAR(255) NOT NULL DEFAULT \'NOT_TRACKED\';' },
  sql: 'ALTER TABLE "public"."crm.public.accounts" ADD COLUMN "agentId" VARCHAR(255) NOT NULL DEFAULT \'NOT_TRACKED\';' }

How do I get the cli to respect my database when running migrations?

buildjosh avatar Apr 07 '16 16:04 buildjosh

The original migration should work. Are you sure you are using the public schema?

Americas avatar Apr 11 '16 07:04 Americas

Same error here

lopezjurip avatar Jun 09 '16 03:06 lopezjurip

Similar problem here

onlyurei avatar Jul 25 '16 21:07 onlyurei

same :/

benbonnet avatar Aug 02 '16 09:08 benbonnet

solution?

bayarja avatar Oct 24 '16 03:10 bayarja

If anyone is struggling, when I removed schema: 'public' option in createTable, it stopped prepending 'public.'

tsuz avatar Jan 21 '17 13:01 tsuz

Same issue here. I don't have schema:public on create table, but the issue still persists.

saurabhverma2892 avatar Jul 18 '17 22:07 saurabhverma2892

I have the same problem, which makes it virtually impossible to use a schema other than public. Is there any solution now?

frankthelen avatar Sep 21 '17 14:09 frankthelen

Same issue here, It should support defining of schema in option!

adeelhussain avatar Oct 07 '17 07:10 adeelhussain

Same issue here. What should we do to fix it?

prokopsimek avatar Nov 10 '17 15:11 prokopsimek

The problem is somewhere inside of Sequelize itself. Here https://github.com/sequelize/cli/blob/master/src/core/migrator.js#L27 is still the config with the schema, so I think that it must be implemented in Sequelize.

prokopsimek avatar Nov 10 '17 16:11 prokopsimek

same issue here, is there help coming?

stonedraider avatar Dec 05 '17 13:12 stonedraider

This is still an issue. Can we get support for a simple configuration here?!

jpwiddy avatar Mar 13 '19 16:03 jpwiddy

No solution to this problem yet?

renanbirlem avatar Aug 07 '19 20:08 renanbirlem

I have two schemas in my postgres DB and I solved the problem just added the name of the specific schema to the migration. It looks like this:

module.exports = {
  up: (queryInterface, Sequelize) => {
    return queryInterface.sequelize.transaction((t) => {
      return Promise.all([
        queryInterface.addColumn(
          {tableName: 'investor', schema: "Name"},
          'new_column', {type: Sequelize.STRING}, { transaction: t })
      ])
    })
  },

  down: (queryInterface, Sequelize) => {
    return queryInterface.sequelize.transaction((t) => {
      return Promise.all([
        queryInterface.removeColumn(
          {tableName: 'investor', schema: "Name"},
          'new_column', { transaction: t })
      ])
    })
  }
};

rmalkevy avatar Sep 11 '19 15:09 rmalkevy

The mistake I made was thinking model name == table name and not understanding there is a distinction (the later is pulralized by default). I suggest changing the example migration to "Persons" to be consistent with the default behaviour for those who are skimming the docs.

bostelk avatar Sep 13 '19 16:09 bostelk

This is definitely a real problem rather than just a typo as was the case for @bostelk. It appears to be a problem with the addColumn migration. I've used most of the other migration methods and not had a problem with any of them, but for some reason addColumn ignores the defined schema and uses public. The workaround suggested by @rmalkevy works, but isn't really a practical solution when you want to run the same migrations on many multiple schemas.

If anyone has any thoughts on possible solutions that would be great! :)

ghost avatar Nov 09 '19 21:11 ghost

Ran into this issue while attempting to run the migrations for an app which uses sequelize in a schema other than public. Was a bit of a mystery as to what was actually going on, so spent some time today trying to trace all the different steps to try to at least get a coherent explanation for what is happening.

To me it looks like there are places in the code where the schema will always resolve to or is hard-coded to public. So even if as a developer you attempt to control the schema externally via search_path or another mechanism, you can't change it. From what I've been able to find, I think this happens in 3 places.

1. resolving relation names

The base class QueryGenerator has a method extractTableDetails which is used by the dialect specific QueryGenerator class instances to determine the full name of the relation i.e. schema.table.

https://github.com/sequelize/sequelize/blob/f3110e32b5b35390e956158339cd9e1d2c67f007/lib/dialects/abstract/query-generator.js#L41-L49

For postgres it is used in 4 places in the file lib/dialects/postgres/query-generator.js. As an example explaining the issue @Chuckatron had, if we look at it being used in the addColumnQuery function below, we see it is not called with an options parameter.

https://github.com/sequelize/sequelize/blob/f3110e32b5b35390e956158339cd9e1d2c67f007/lib/dialects/postgres/query-generator.js#L246-L260

Therefore, the schema will always resolve to public as per the line schema: tableName.schema || options.schema || 'public',. Unless you know about the workaround @rmalkevy shared (which is actually the intended behaviour) and you specify your tables in migration files as { tableName, schema } objects.

2. showTablesQuery

Here public is simply hard-coded, which would not be desirable if you wanted to use a different schema. https://github.com/sequelize/sequelize/blob/f3110e32b5b35390e956158339cd9e1d2c67f007/lib/dialects/postgres/query-generator.js#L117-L119

3. describeTableQuery

This method is used many times in the code, sometimes with the schema supplied sometimes not. https://github.com/sequelize/sequelize/blob/f3110e32b5b35390e956158339cd9e1d2c67f007/lib/dialects/postgres/query-generator.js#L121-L144

I didn't have time to fully trace the call path for this one because it was in so many different places. But for me I did have to hard-code the schema to something else to get my migrations to complete.

possible fixes

The base class QueryGenerator is initialised whenever new Sequelize(options) is called. These options are available in the constructor as seen in the below snippet. https://github.com/sequelize/sequelize/blob/f3110e32b5b35390e956158339cd9e1d2c67f007/lib/dialects/abstract/query-generator.js#L28-L39

When using Sequelize in our apps, we can specify a { schema: 'schema-name' } option (docs link here). The same is actually possible when doing migrations with the sequelize-cli. The function getSequelizeInstance takes care of reading in the config, and initialising a new Sequelize instance. If you add "schema": "schema-name" to database.json this is passed along to the Sequelize instance, making it available to the base class QueryGenerator. e.g.

{
  "development": {
    "use_env_variable": "DATABASE_URL",
    "dialect": "postgres",
    "logging": "true",
    "schema": "schema-name"
  }
}

note: this also seems to be an alias to the option migrationStorageTableSchema, so it will also put your meta migration table in new-schema too. I don't think I saw this behaviour documented anywhere.

I suggest that it is here in the class QueryGenerator we make use of this.options.schema in all of its methods to resolve the schema value if it is supplied before falling back to public. For example,

  extractTableDetails(tableName, options) {
    options = options || {};
    tableName = tableName || {};
    return {
      schema: tableName.schema || options.schema || this.options.schema || 'public',

avoiding the problem?

If you don't work with databases directly, the schema concept in postgres could be a surprise. So it's good there here, as in many ORMs, this is attempted to be abstracted away. Although I do think there there are a couple of places where this abstraction leaks, and there is a mixture of allowing explicit schema control, hidden hard coding of the schema and silent fallbacks to the postgres database using public by default internally.

Personally I like to avoid the whole thing by shifting as much work to the DB as possible. I often make dedicated schemas for applications, and also dedicated users for the schemas with their search_path set to only that schema. So all this is done at the DB connection level, allowing sql code to be portable to any schema. There is an open PR #622 trying to do this via the connection url, but I think it's better as a setting on the database users' permissions, to minimise further database specific forks in logic/complexity.

Knowing the schema may be required in the case of some of the queries which target the postgres system catalogue tables. So perhaps, just one top top level config for DEFAULT_SCHMEA can be set and propagated everywhere in the code.

sorry for the long message! Wanted as much as anything else to not forget all the things I'd found out while tracing my steps through the code. Hopefully this serves as a comprehensive record. Also, in no way meant to be critical of the package. Making an ORM, is a really really big undertaking. As always, thanks to all maintainers and contributors on OSS projects.

HUSSTECH avatar Jan 11 '20 20:01 HUSSTECH

Fantastic writeup @HUSSTECH - you certainly got much further than me when I made an attempt to get to the bottom of it. Your suggested fixes make a lot of sense, at least with my limited understanding of the internals of Sequelize. I'm particularly grateful for you making the suggestion of limiting a user to a particular schema, I too tend to have a direct 1:1 user<=>schema relationship, so this would work around the problem for me. :+1: Thank you!

ghost avatar Jan 14 '20 14:01 ghost

I may have a potential fix coming for schemas. That is, I encountered an issue with schemas (including existing) and their permissions.

Background

I came across Sequelize while looking for a similar ORM to Rails ActiveRecord, which could perform command-line migrations like sequelize-cli. In my PostgreSQL setup, I wanted the DBA to control the public schema and the sequelize-connected account to only have access w/in an assigned schema, but not the ability to create schemas, which would grant permission to create unlimited schemas.

Problems

With my custom db-permission scheme, I learned Sequelize:

  1. breaks when it encounters an existing schema (it tries to re-create even if it already exists -- really, the IF NOT EXISTS doesn't matter to Postgres)
  2. Sequelize handles schema from config okay for the migration file, but does not reuse that for table creation

There are a few other items, but not necessary to list out here as you get the point.

Afterwards

I've modified a few files in node_modules sequelize and sequelize-cli directories. While I'm JavaScript strong 💪 I'm not a seasoned node developer and still in the process of figuring out how the npm package aligns with the git-repo -- so that I can take the changes I've made locally and apply them to the github repo (for a PR).

I still need to:

  1. review my sequelize changes to see if they're a breaking change for sequlize-cli
  2. write tests and learn how to run them in these node projects -- it doesn't look too difficult but that's famous last words
  3. submit the PR
  4. see how it applies to this specific issue (or if it can be amended)

If I don't get distracted, we can expect this w/in the week. If I get distracted it may never come 😄 (lots of balls in the air).

mike-usa avatar May 12 '22 19:05 mike-usa

That's great :) Looking forward to it!

ephys avatar May 13 '22 12:05 ephys

Hey everyone,

I have created a PR over in the sequelize project with some code changes aimed to address the points that @HUSSTECH outlined above.

There is a request for me to add tests to this PR. I am pretty busy and will try to get to them this week, but if anyone else would like to add these tests for me, please reach out and I will make sure you have permissions to add to the PR.

nholmes3 avatar Jun 14 '22 15:06 nholmes3