Skip to content

Conversation

@aol-nnov
Copy link
Contributor

This PR allows to define the default value for a field by the means of database server. It is especially useful when the default value is calculated by a stored procedure.

Example usage is as follows:
simpleUsage.json

{
    "name": "simpleModel",
    "properties": {
        "creationDate": {
            "type": "Date",
            "postgresql": {
                "dbDefault": "now()"
            }
        }
    }
}

Another, a more complex usecase:
myModel.json

{
    "name": "myModel",
    "base": "PersistedModel",
    "options": {
        "idInjection": false,
        "postgresql": {
            "schema": "store",
            "table": "item"
        }
    },
    "properties": {
        "id": {
            "type": "String",
            "id": true,
            "postgresql": {
                "dataType": "uuid",
                "dbDefault": "uuid_generate_v4()"
            }
        }
    }
}

I quite agree that in many cases the same behaviour can be achieved by Loopback hooks, but still there are cases exist where default value is calculated by custom stored procedure on the DB side.

Feel free to contact me if you need more comments on this PR.

@slnode
Copy link

slnode commented Jan 21, 2015

Can one of the admins verify this patch? To accept patch and trigger a build add comment ".ok\W+to\W+test."

@raymondfeng
Copy link
Contributor

Would you please add some test cases?

@aol-nnov
Copy link
Contributor Author

@raymondfeng, no problem, but it'll take some time, because I'm not familiar with test-suite, my bad...

@raymondfeng
Copy link
Contributor

BTW, this feature can be possibly ported to other connectors too.

@aol-nnov
Copy link
Contributor Author

yeah, I know that :)
Many (if not all) RDBMs support function calls as column default values. At least MySQL and PostgreSQL do.

aol-nnov added a commit to aol-nnov/loopback-connector-postgresql that referenced this pull request Jan 21, 2015
@aol-nnov
Copy link
Contributor Author

@raymondfeng, just added basic tests for this PR. If it's not enough, please advice more test-cases to be considered.

@raymondfeng
Copy link
Contributor

Great, maybe two more tests:

  1. Invalid default for DB
  2. Try to create a record and read it back to see if the DB default is effective.

@aol-nnov
Copy link
Contributor Author

Okay. Will do it tomorrow! Way too late over here now.

aol-nnov added a commit to aol-nnov/loopback-connector-postgresql that referenced this pull request Jan 22, 2015
@aol-nnov
Copy link
Contributor Author

@raymondfeng, just added more tests.
You need to integrate #51 first to pass 'should create a record with default value' though..

@aol-nnov
Copy link
Contributor Author

this PR now longer depends on #51. All tests are passing correctly.

@aol-nnov
Copy link
Contributor Author

Well, It was a headlong verdict about @aars's fix replacement. :)
As he pointed in comment to my commit bfb3f00, there will be inconsistences between column names and values. With small extra tweaks it is possible to find a way around.
But!
His fix does not pass loopback-database-juggler tests either (at least they fail on my computer). This is the general problem and my solution won't pass them too. And the root-cause is: if all model fields are not initialised with values, wrong sql query will be generated.

I'm not yet sure how to run only partial tests, so here is the command line that I was using:

aol@Andreys-MBP:~/develop/web/loopback-connector-postgresql$ DEBUG=loopback:connector:postgresql ./node_modules/.bin/mocha --require ./test/init.js node_modules/loopback-datasource-juggler/test/relations.test.js

on your master branch I have the following debug output in 'can be declared in short form' testcase:

  loopback:connector:postgresql SQL: INSERT INTO "public"."book" ("name","type") SELECT $1,$2  RETURNING "id"
Parameters: , +2ms

(note two empty parameters!)

And with @aar's patch applied, this query becomes:

  loopback:connector:postgresql SQL: INSERT INTO "public"."book" () SELECT   RETURNING "id"

It seems to me, that it happens if new object is saved and it's fields are not initialised.

So, it seems, we need to invent another approach to handle undefined values.
From one side, we can't substitute undefined keys of data with NULLs (to utilise database default values or for leaving columns intact when they do not have any constraints for values), from the other side, the latter query is meaningless and erroneous as column list and values list are empty.

Please, let me know, if any of my thoughts are correct.

@aol-nnov
Copy link
Contributor Author

Seems, this is the way out. All previously failing tests passed!

@raymondfeng raymondfeng merged commit a7cd59a into loopbackio:master Jan 23, 2015
raymondfeng pushed a commit that referenced this pull request Jan 23, 2015
1.4.0

 * Remove the usage of `CREATE SCHEMA IF NOT EXISTS' for compatibility (Raymond Feng)

 * one-line fix for #51 (Andrey Loukhnov)

 * basic tests for PR #53 (Andrey Loukhnov)

 * basic tests for PR #54 (Andrey Loukhnov)

 * provide database column default values via Loopback model description (Andrey Loukhnov)

 * autocreate schema if it does not exist during migration/update (Andrey Loukhnov)

 * Use connection pooling from the driver (Raymond Feng)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants