handling of db columns that have database specified default

[rmallah]

Hi , This is about DBIx backend.

In case a table column has default value specified at DB level and we do not specify that column in the item to be created , still Yancy seems to be attempting to specify that item while inserting into database.

This is problematic specially when the default value is a function in such case Yancy is attempting to insert it as the function definition body.

If it is really working like that , I think Its better to leave the handling of DB defaults to DB itself.

[rmallah]

by having read_schema => 0 , the problem could be averted.

[preaction]

Ah, there are two things happening here:

1. Yancy is defining a default value for the column when it's not appropriate to. What does Yancy end up thinking the default value is in the schema? Could you do ./myapp.pl eval -V 'app->yancy->schema' and paste the results to gist.github.com, please?
2. Yancy tells JSON::Validator to fill in default values.

These are probably both bugs. Yancy shouldn't define a default when it's not a simple value (except the "now" exception for date-time fields). Fixing that would probably solve this issue, but you're right in that if the database will fill in the default Yancy shouldn't. Yancy could instead use the default values in the JSON schema as a placeholder instead of a fill-in value, so that both the controller and the backend can know whether the field was actually filled-in, or if the user just wants to use the default.

Thanks for reporting this!

[rmallah]

Sorry i had missed this message.

it was short hence i pasted here.

./myapp.pl eval -V 'app->yancy->schema'

{
  "dbix_migration" => {
    "properties" => {
      "name" => {
        "type" => "string",
        "x-order" => 1
      },
      "value" => {
        "type" => [
          "string",
          "null"
        ],
        "x-order" => 2
      }
    },
    "required" => [
      "name"
    ],
    "type" => "object",
    "x-id-field" => "name"
  },
  "foo" => {
    "properties" => {
      "id" => {
        "type" => "integer",
        "x-order" => 1
      },
      "sno" => {
        "default" => '-752871115425358459',
        "type" => [
          "integer",
          "null"
        ],
        "x-order" => 2
      },
      "status" => {
        "type" => [
          "string",
          "null"
        ],
        "x-order" => 3
      }
    },
    "required" => [
      "id"
    ],
    "type" => "object"
  }
}

[rmallah]

cat sqlite/schema_1_up.sql

DROP TABLE IF EXISTS foo;
CREATE TABLE foo  (
        id integer primary key,
        sno integer default ( random() )  ,
        status  varchar(20)
);

[rmallah]

Please note

$ curl 'http://localhost:3000/yancy/api/foo' -X POST  --data-raw '{"id":"19","status":"HELLO"}'

does succeed ,

which makes me think that if introduce some kind of an option in the yancy ui to ignore the field or "use defaults" it will make the ui more usable.

also it currently fails because it expecting integer or null and we are sending ""

[preaction]

It definitely shouldn't be trying to send the empty string for an integer field, but I had thought I fixed that... Is it just generating the empty string in the "Raw" field?

On the default value, though, notice your pasted schema doesn't have ( random() ) as the default, but -752871115425358459. That's caused by this line in fixup_default of Yancy::Backend::Sqlite which is actually fetching a random number from SQLite to use as the default. Unfortunately, it's only fetching a random number once while reading the schema, and I don't think the fix for this should be running fixup_default for all random values when saving a row. I think the actual fix would be for that fixup_default function to return undef if it sees a function call (instead of going on to actually call that function to get a static value).

[rmallah]

thanks for the attention and discussion. I will try to put up a PR based on above and my understanding.

[rmallah]

It definitely shouldn't be trying to send the empty string for an integer field, but I had thought I fixed that... Is it just >>generating the empty string in the "Raw" field?

No its not just visual. it really sends the request like that.

ON:

I think the actual fix would be for that fixup_default function to return undef if it sees a function call (instead of going on >>to actually call that function to get a static value).

sending 'undef' would actually fill the column with NULL whereas we would want the underlying DB engine to evaluate the function and fill the result. hence IMHO, Should not we be omitting the column itself in the update ?

[rmallah]

@preaction

i have crafted a small fix based on your suggestion . This fixes for DBIC backend. could you please review #143