How to update column value with mutation to NULL

[langpavel]

Hi @calebmer. I'm challenging problem. How I can set value to NULL?

[langpavel]

One possible solution is to add boolean fields into Input shape for each nullable column,
like newImageFeatured for setting value
and imageFeaturedSetNull or nullImageFeatured for database NULL.

[calebmer]

This definitely looks like a bug. I'll investigate 👍

[langpavel]

@calebmer Can you point out some relevant pieces of code for me? I can prepare PR for this, but I feel there will be more discussion about this... So, any ideas? I really want this feature 😈

[calebmer]

For sure 😊

This statement will be what you want to look at. Also, here where setClauses gets defined.

Also a quick crash-course on SQLBuilder (it was a quick and dirty solution, there’s a much cleaner SQL builder in the next branch 😉). Counting up (think $1, $2, $3, etc.) like in normal prepared statements is hard to keep track of and very difficult to compose. So SQLBuilder allows you to just use $ and pass the array of ordered values like you normally would. Then SQLBuilder will compose those $ to have the correct index.

The bug may be here or it may be as easy a fix as changing this to:

if (value === null) {
  setClauses.push(`"${column.name}" = null`)
}
else {
  setClauses.push(`"${column.name}" = $`)
  setValues.push(value)
}

(pg may not like nulls 😊)

Let me know how it goes 👍 🎉

[langpavel]

Thanks! Will look. Btw do you like way I'm using template strings in my pg-async library?

[calebmer]

It depends, how much do you support? 😉 I didn’t see any documentation when I looked quickly.

I really don’t like SQLBuilder (it was the best I could do at the time…), so I’m going to talk more about the PostGraphQL 2 SQL builder.

Some requirements for a SQL builder used by PostGraphQL is:

• Supports identifier variables. So something like "forum_example"."person"."id" can be added as so sql.query${sql.identifier('forum_example', 'person', 'id')} = 1``.
• Allows for both eager and lazy values. Therefore you could do column = ${sql.value(1)} or column = ${sql.placeholder('num')} and then inject a value for num later. In both cases though we still need to use PostgreSQL placeholders with auto-incrementing “names” ($1, $2, $3, etc).
• Must be type safe. This is more a design constraint of PostGraphQL 2 though.

The implementation I landed on is also pretty fast. At the end of the days it’s just some string concatenation. Under the hood everything is an object though, it takes the strings and turns it into an array, so a query like:

sql.query`where ${sql.identifier('a', 'b', 'c')} = ${sql.value(value)}`

Turns into:

[{ type: 'RAW', text: 'where ' }, { type: 'IDENTIFIER', names: ['a', 'b', 'c'] }, { type: 'RAW', text: ' = ' }, { type: 'VALUE', value: value }]

I really like the API/implementation I ended up with, but I’m always open to new ideas 😊

[calebmer]

After some more research, I remembered that there is no idea of “null” for inputs in GraphQL. See this issue graphql/graphql-js#133 and this RFC pull request graphql/graphql-spec#83

For now if you want to continue with your proposal (imageFeaturedSetNull etc) go for it, just know it will probably be deprecated when we get support for the null literal in GraphQL 👍

Another option is we could add a removedColumns array, but I like the idea of imageFeaturedSetNull better more personally.

[langpavel]

Hi. I just finished new PR graphql/graphql-js#544 extending GraphQL with null literal in GraphQL language itself so RFC will be fulfilled soon I hope :-)

But it looks that null values are supported already in graphql-js@master.

What is state of this issue? Are explicit nulls still unhandled?

[calebmer]

I’ve been watching the progress, great job 😊

I’ll have to check and see if null values are supported, but I’ll definitely make sure everything works as expected when graphql/graphql-js#544 lands 👍

[benjie]

Looks like graphql/graphql-js#544 just got merged!

[calebmer]

🎉 now just waiting for it to get released in 0.8.0 then I’ll test to make sure everything works as expected.

Also see #202

[ferdinandsalis]

@calebmer what is the status of this?

[calebmer]

@ferdinandsalis I can confirm this works, just make sure you have a version of GraphQL above 0.8.0 👍

Submit a PR to add tests and prevent regression, once that gets merged we can close this issue: #255