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

Waterline - I cannot set null on the "one" side of a one-to-many when using id columnType "bigserial" - postgres #6876

Open
Noitidart opened this issue Oct 14, 2019 · 16 comments
Labels
helpful info or workaround orm Related to models, datastores, orm config, Waterline, sails-hook-orm, etc.

Comments

@Noitidart
Copy link

Node version: v12.10.0
Sails version (sails): 1.2.2
ORM hook version (sails-hook-orm): 2.1.1
Sockets hook version (sails-hook-sockets): 2.0.0
Organics hook version (sails-hook-organics): 0.16.0
Grunt hook version (sails-hook-grunt): UNINSTALLED
Uploads hook version (sails-hook-uploads): NOT INSTALLED
DB adapter & version (e.g. sails-mysql@5.55.5): sails-postgresql@1.0.2
Skipper adapter & version (e.g. skipper-s3@5.55.5): NOT INSTALLED

I posted this on Stackoverflow too - https://stackoverflow.com/questions/58379618/nullable-value-for-a-one-to-many-relation

I cannot set null on the "one" side of a one-to-many

I have a Payment model, and I want it to optionally be related to a DonationBox. Meaning, Payments can be created without specifying a DonationBox.

Example I want to do this:

Payment.create({
    donationBox: null
})

I have a DonationBox model with this relation:

payments: {
  collection: 'payment',
  via: 'donationBox'
},

I then have a Payment model with this relation:

// Optional because some payments are not for donation boxes
donationBox: {
  model: 'donationbox',
  allowNull: true
},

However lifting with allowNull causes error:

The attribute donationBox on the payment model contains invalid properties. The allowNull flag may not be used on attributes that represent associations. For singular associations null is allowed by default.

If I remove that, then the Payment.create({ donationBox: null }) gives error:

AdapterError: Unexpected error from database adapter: null value in column "donationBox" violates not-null constraint

This error is opposite of the error given when I tried to add allowNull to the model attribute. It said that "null is allowed by default", but it's not the case.

Anyone know how I can set null here?

@sailsbot
Copy link

@Noitidart 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.

@johnabrams7 johnabrams7 added the orm Related to models, datastores, orm config, Waterline, sails-hook-orm, etc. label Oct 14, 2019
@whichking
Copy link
Contributor

Hey, @Noitidart!

This certainly is perplexing. Any chance you can share a pared-down repro repo containing only the models relevant to the one-to-many association? Thanks!

@whichking whichking added the repro please Could you reproduce this in a repository for us? label Oct 14, 2019
@Noitidart
Copy link
Author

Noitidart commented Oct 15, 2019 via email

@sailsbot sailsbot removed the repro please Could you reproduce this in a repository for us? label Oct 15, 2019
@Noitidart Noitidart changed the title Waterline - I cannot set null on the "one" side of a one-to-many Waterline - I cannot set null on the "one" side of a one-to-many when using id columnType "bigserial" - postgres Oct 15, 2019
@Noitidart
Copy link
Author

Noitidart commented Oct 15, 2019

In creating the simple test case I found the exact issue. It's when I use columnType of "bigserial" then it fails.

To see it fail I created a repo, using web app template:

Initialize

  1. cd so working dir is the repo you checked out.
  2. docker-compose up -d

Failing test

  1. Checkout this repo - https://github.com/Noitidart/sails-one-side-null-bigserial-bug
  2. Then run npm run test-backend.
  3. You will see it fail.

Passing test

  1. To see it not fail please go to config/models.js and change id: { type: 'string', columnType: 'bigserial', autoIncrement: true, }, to id: { type: 'number', autoIncrement: true, },
  2. Then run npm run test-backend again.
  3. It will pass

Shutdown

  1. docker-compose down

I updated the title to indicate that if we use columnType of "bigserial" it is causing the problem.

@Noitidart
Copy link
Author

By any chance, would anyone have a solution I can use until a fix for this lands? Moving away from bigserial back to number I can't do. :(

@Noitidart
Copy link
Author

Just a bump, was kind of stuck on this one, anyone have any ideas of a workaround?

@Noitidart
Copy link
Author

Bumpity :(

@alxndrsn
Copy link

Have you looked at the db schema that is generated?

# \d+ payment
                                                       Table "public.payment"
   Column    |  Type  | Collation | Nullable |                    Default                     | Storage | Stats target | Description 
-------------+--------+-----------+----------+------------------------------------------------+---------+--------------+-------------
 createdAt   | bigint |           |          |                                                | plain   |              | 
 updatedAt   | bigint |           |          |                                                | plain   |              | 
 id          | bigint |           | not null | nextval('payment_id_seq'::regclass)            | plain   |              | 
 donationBox | bigint |           | not null | nextval('"payment_donationBox_seq"'::regclass) | plain   |              | 
Indexes:
    "payment_pkey" PRIMARY KEY, btree (id)

donationBox should be nullable, but is not. I'd guess this is a bug in the schema generation.

@alxndrsn
Copy link

alxndrsn commented Nov 12, 2019

For a dirty fix in your test:

  it('can create a payment with null donationBox', async () => {                                                                                                        
    await sails.getDatastore().sendNativeQuery(`ALTER TABLE payment ALTER COLUMN "donationBox" DROP NOT NULL;`);                                                             
                                                                                                                                                                             
    await expect(Payment.create()).to.eventually.be.fulfilled                                                                                                                
                                                                                                                                                                             
    console.log(require('util').inspect(await Payment.find({}).populate('donationBox'), false, null, true));                                                                 
                                                                                                                                                                             
  }); 

So to fix this in your project, add this line to config/bootstrap.js for each relevant table:

await sails
    .getDatastore()
    .sendNativeQuery(`
      ALTER TABLE payment
        ALTER COLUMN "donationBox"
        DROP NOT NULL;
     `);

@Noitidart
Copy link
Author

Noitidart commented Nov 12, 2019

@alxndrsn thank you so so much! Just tested this and it's awesome it works! I didn't test in main project yet.

@Noitidart
Copy link
Author

I was checkng the schemas now @alxndrsn and found something interesting. I'm new to postgres so learning a ton from your help thank you!

On the right side, is where I'm using "number" for "id". And on the left side I'm using "bigserial" for "id":

image

We see on the right, for "donationBox", "Nullable" column has no value, which you showed me above. However my question is on the "Default" column. On the right the default is blank, but on the left the default value is nextval('"payment_donationBox_seq"'::regclass). What does this mean?

Thanks!

@nahanil
Copy link

nahanil commented Nov 12, 2019

@Noitidart serial and friends are just a shortcut type - https://www.postgresql.org/docs/9.1/datatype-numeric.html#DATATYPE-SERIAL

In the case of bigserial your actual column becomes a bigint and a sequence (something starting at some number and incrementing by some number each time you call nextval on it) is created, then used as the default value for the bigint if you don't explicitly specify something when you insert new stuff.

As for a columns default values in general, it'll be whatever data you want in a given column for the new row if nothing is specified when you insert a new record. So you could specify a column as type boolean default false, or a date/time column as timestamp default CURRENT_TIMESTAMP.

Not sure if it helps/makes sense & sorry for butting in 😝

@Noitidart
Copy link
Author

Oooh thanks @nahanil !!

@Noitidart
Copy link
Author

@alxndrsn is there a place other then bootstrap that I can do this? Because bootstrap runs after migration, so migration is failing:


On Sun Nov 24 2019 12:25:44 GMT-0800 (Pacific Standard Time), Sails attempted to auto-migrate
using the `alter` strategy, but was unable to transform all of your
existing development data automatically.  This temporary file was created 
for your convenience, as a way of holding on to the unmigrated records that
were originally stored in the `payment` model.
(Otherwise, this data would have been lost forever.)

================================
Recovered data (`payment`):

@alxndrsn
Copy link

@Noitidart you might be able to do this with sails hooks.

However, I'm guessing the migration is done by the sails ORM hook, and there is no guarantee of hook load order, so maybe this won't work.

There's a discussion about hook load order relating to modifying domain objects at #6799 which might be of interest.

@Noitidart
Copy link
Author

Noitidart commented Nov 26, 2019 via email

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.
Development

No branches or pull requests

6 participants