PostgreSQL driver: feed DEFAULT to columns instead of NULL.

[kolypto]

Using the following model

var Upload = db.define('Upload', {
        id: Number,
        name: String
    }, {
        id: 'id',
        table: 'uploads'
    });

Against the following schema:

CREATE SEQUENCE "uploads_id_seq";
CREATE TABLE "uploads" (
    "id" int NOT NULL DEFAULT nextval('uploads_id_seq'),
    "name" varchar(255) NOT NULL
);
ALTER SEQUENCE "uploads_id_seq" OWNED BY uploads.id;
CREATE UNIQUE INDEX "idx:uploads:id" ON "uploads" ("id");

I try the following:

Upload.create({}, function(){...});
Upload.create({ id: undefined }, function(){...});

All give the following error:

error: null value in column "id" violates not-null constraint

This happens because PostgreSQL, when said to insert NULL, inserts as requested. To insert the default value, one should put DEFAULT there, or omit the column when inserting.

---

In general, a NOT NULL column will never allow NULL, and for NULL columns -- this value is the default default value, so to say.

I'm suggesting to always use DEFAULT when inserting nulls into PostgreSQL, or at least handle undefined this way.

[kcphysics]

Given the docs at http://www.postgresql.org/docs/9.3/static/ddl-default.html, it should be safe to send 'default' instead of null/undefined except for one important case. That case is when there is a default defined and a null should be inserted.

It seems to me that you would have to know whether a default was defined in the database before safely making the judgement to replace null with default.

For instance, lets say you have a table as follows:

CREATE TABLE table1(
    id SERIAL NOT NULL,
    stuff TEXT DEFAULT 'Items',
    CONSTRAINT table1_pk PRIMARY KEY (id)
);

The operations:

INSERT INTO table1 VALUES (DEFAULT, DEFAULT);

and

INSERT INTO table1 VALUES (DEFAULT, NULL);

Are very different, and could be meaningful to that database.

Therefore the only way to safely flag where NULL and DEFAULT can be interchangeable is by having some sort of default set in the schema.

Now for having undefined be handled in this way is ok, however, it is important to note that the user must be made aware of this. Most drivers handle undefined and null in a similar way. What you are suggesting here is that undefined be treated as default.

The consequence is that most of the time default and null are the same (as most fields in a database do not have defaults defined). However, it is that rare case where a default IS defined that undefined and null will behave differently.

I will go ahead and see if I can make the change. The other issue, and I need some guidance here as I am new to the OSS scene, is that the change is not in ORM2, but sql-query, a package ORM depends on. It is here, in the PostgreSQL dialects folder that the change would have to be implemented.

[kcphysics]

So a pull-request has been generated that contains the appropriate changes to allow a DEFAULT value to be passed to PostgreSQL. If these changes are accepted, your table should like this:

CREATE TABLE table1(
    id SERIAL NOT NULL,
    stuff TEXT,
    CONSTRAINT table1_pk PRIMARY KEY (id)
);

Your model definition should look like this:

var table1 = db.define('table1', {
    id     : {type: "number", defaultValue: undefined},
    stuff : {type: "text"}
},{
    id: 'id',
    table: 'table1'
});

And your create should look like this:

table1.create([
   {
       stuff: "Text String goes here"
   },
   {
       id: undefined,
       stuff: "This will also work"
   }
], function(err, items){});

This will allow the series that is created for the id column in PostgreSQL to be used. It will be used if the value is omitted or if the value is undefined. If you don't have a server side default, that's alright too, as PostgreSQL assigns a default value of null.

If you want null in id (in this case PostgreSQL would throw an error), you have to explicitly assign null.

The only changes to node-orm2 should be a test that tests for this. If the pull-request is accepted on node-sql-query, I will work on this.

[kolypto]

@lselvy, what about sending NULL for nulls, and DEFAULT for undefineds?

[kcphysics]

Its possible to do so. null for NULL is already implemented. If you put in a null in a field, node-orm will pass this down to the database, which will validate it on its own.

If you're asking, can we use undefined to indicate that a DEFAULT should be put into the database, that is what I am suggesting above, and have submitted the above pull request. That pull request was merged in this afternoon, so you should be able to use undefined in your model instance creation (if you pull the head of node-sql-query).

Does that address your question?

[kolypto]

@lselvy, sorry for posting before thinking :) yes, it addresses precisely

[kcphysics]

This is my first foray into working on an Open Source project, so it could
have been my explanation.

Thanks!

On Thu, Feb 20, 2014 at 8:01 PM, Mark notifications@github.com wrote:

@lselvy https://github.com/lselvy, sorry for posting before thinking :)
yes, it addresses precisely

Reply to this email directly or view it on GitHubhttps://github.com//issues/391#issuecomment-35688596
.

"Trust in the Lord with all thine heart and lean not on thine own
understanding, but in all thy ways acknowledge Him and He shall direct thy
path."
-Proverbs 3:5-6

[dxg]

Since the PR has been merged, I'm gonna close this.
It'll be released as part of 2.1.4

[naturalethic]

This doesn't appear to be working in 2.1.4 -- i don't in fact see that the orm package is even using node-sql-query.

[naturalethic]

#485 is required to allow undefined to pass through to the generator.

[cjnqt]

@dxg This is still an issue (as far as I can see).

If we have defined a DEFAULT value in the database table but not in the model, we can't insert stuff and use those DEFAULT values.

An example:

Let's say we have a Postgres table with two columns:

CREATE TABLE "user" ("first_name" TEXT, "random_string" TEXT DEFAULT gen_random_bytes(16))

the default value for the column random_string is get_random_bytes - a postgres function.

The query INSERT INTO "user" ("first_name") VALUES ("PeterPan") will generate a row with both first_name and random_string populated as expected.

But using User.create({'first_name': 'PeterPan' }, ... creates the SQL-query
INSERT INTO "user" ("first_name", "random_string") VALUES ("PeterPan", NULL) and this inserts a row with only first_name populated.

I would suggest (if it doesn't break other things...) that if a table column is not included in a model.create, it should be added to the INSERT query only if it has a defaultValue specified in the model. Otherwise it shouldn't be included in the INSERT-query at all.