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

defaultsTo and required key do not reflect in database schema for postgresql #6801

Open
krisalay opened this issue Jun 28, 2019 · 3 comments
Labels
helpful info or workaround orm Related to models, datastores, orm config, Waterline, sails-hook-orm, etc. postgresql Issue only occurs when using PostgreSQL

Comments

@krisalay
Copy link

Node version: 10.16.0
Sails version (sails): 1.2.3
ORM hook version (sails-hook-orm): ^2.1.1
Sockets hook version (sails-hook-sockets): ^2.0.0
DB adapter & version (sails-postgresql): ^1.0.2


I have an existing postgresql database, and I want to replicate the same database using the sails-postgresql. But I am facing the following issues:

  1. defaultsTo value does not reflect in the database schema. (eg. The isDeleted field in the attribute is of type boolean which defaults to false, but the schema does not reflect the same. Simillarly the meta field in the attribute is of type jsonb which defaults to empty object, but no reflection in the db schema)

  2. Ideally, when I add the field required: true, then the db schema should show not null value in the Nullable column in the schema, but it shows nothing. (eg. the name field has required: true but the Nullable column is empty for the same)

  3. The createdAt column is of type ref with column type timestamp(0) with time zone. The requirement is that, it should add the current timestamp (similar as default value NOW() in postgres). But autoUpdatedAt: true is not helping me with the same.

module.exports = {
  schema: true,
  tableName: 'organization',
  attributes: {
    id: {
      type: 'number',
      unique: true,
      columnName: 'id',
      autoIncrement: true
    },
    isDeleted: {
      type: 'boolean',
      defaultsTo: false,
      columnName: 'is_deleted'
    },
    name: {
      type: 'ref',
      columnName: 'name',
      columnType: 'text',
      required: true
    },
    createdAt: {
      type: 'ref',
      columnName: 'created_at',
      columnType: 'timestamp(0) with time zone',
      autoUpdatedAt: true,
    },
    meta: {
      type: 'ref',
      columnName: 'meta',
      columnType: 'jsonb',
      defaultsTo: {},
    },
  }
};

Schema built using sailsjs:
Screenshot from 2019-06-28 12-42-04
Original existing Schema:
Screenshot from 2019-06-28 12-53-37

Can you please let me know if I am making some mistake, or sailsjs is designed like this only???

@sailsbot
Copy link

@krisalay Thanks for posting! We'll take a look as soon as possible.

In the mean time, there are a few ways you can help speed things along:

  • look for a workaround. (Even if it's just temporary, sharing your solution can save someone else a lot of time and effort.)
  • tell us why this issue is important to you and your team. What are you trying to accomplish? (Submissions with a little bit of human context tend to be easier to understand and faster to resolve.)
  • make sure you've provided clear instructions on how to reproduce the bug from a clean install.
  • double-check that you've provided all of the requested version and dependency information. (Some of this info might seem irrelevant at first, like which database adapter you're using, but we ask that you include it anyway. Oftentimes an issue is caused by a confluence of unexpected factors, and it can save everybody a ton of time to know all the details up front.)
  • read the code of conduct.
  • if appropriate, ask your business to sponsor your issue. (Open source is our passion, and our core maintainers volunteer many of their nights and weekends working on Sails. But you only get so many nights and weekends in life, and stuff gets done a lot faster when you can work on it during normal daylight hours.)
  • let us know if you are using a 3rd party plugin; whether that's a database adapter, a non-standard view engine, or any other dependency maintained by someone other than our core team. (Besides the name of the 3rd party package, it helps to include the exact version you're using. If you're unsure, check out this list of all the core packages we maintain.)

Please remember: never post in a public forum if you believe you've found a genuine security vulnerability. Instead, disclose it responsibly.

For help with questions about Sails, click here.

@nahanil
Copy link

nahanil commented Jun 28, 2019

Unfortunately there is quite a disconnect between how Waterline and the underlying database handle your data validations & constraints, with Waterline favouring to do it all itself.

I think for those fields that you defined with type ref you can hint to PG what the default should be, ie

    updatedAt: {
      type: 'ref',
      columnName: 'created_at',
      columnType: 'timestamptz not null default now()',
      autoUpdatedAt: true,
    },
    previousEmailAddresses: {
        type: 'ref',
        columnName: 'previous_email',
        columnType: `text[] default '{}'`, // Default array enforced @ db level
        defaultsTo: [], // Default empty array enforced @ JS level
    },

That previousEmailAddresses is just a PG text array I pulled from a working model of ours but might give you some inspiration. Using the above we end up with something like this:

            Column            |           Type           | Collation | Nullable |             Default
------------------------------+--------------------------+-----------+----------+----------------------------------
 updated_at                   | timestamp with time zone |           | not null | now()
 previous_email               | text[]                   |           |          | '{}'::text[]
...

Buyer beware: It's not The Sails Way, and will probably barf using the local disk adapter in tests / if you change to another underlying ORM / if Waterline changes how it handles these kind of attribute definitions in future.

Introducing things like "real" foreign keys at the database level can/will-most-likely cause the Waterline migrations/table alterations to go boom, if you ever end up that far down the rabbit hole.

Also you probably want to use autoCreatedAt: true on your createdAt attribute - it'll set the "now" timestamp in JS land before sending the INSERT to the database, and autoUpdatedAt will do some vooodo when you update an existing record.

@johnabrams7 johnabrams7 added orm Related to models, datastores, orm config, Waterline, sails-hook-orm, etc. postgresql Issue only occurs when using PostgreSQL labels Jun 28, 2019
@whichking
Copy link
Contributor

Hi, @krisalay—if you're using the safe migration setting (migrate: 'safe—this should be used in production), then changes won't be made to your database when you run sails lift. To apply the changes, you can either set migrate: alter or migrate: drop (again, not for use in production), or you can manually update the database. When we migrate production data, we typically do so with a custom script (check out the relevant docs). For more on migration, see this docs page.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
helpful info or workaround orm Related to models, datastores, orm config, Waterline, sails-hook-orm, etc. postgresql Issue only occurs when using PostgreSQL
Development

No branches or pull requests

5 participants