Pg: Why Has default Nullable constraint been switched to Not Null By Default?

[eparziale]

@vmihailenco Can you please provide a little more context on the reasoning behind the pull request here: https://github.com/go-pg/pg/pull/1309/files?

From my team's perspective (and I imagine many other teams) this is a significantly backwards-incompatible change that breaks existing code, and requires significant manual changes and testing to ensure successful integration with existing projects using go-pg.

Our project has 80+ table definitions living in go files, many similiar to the following table
````
type Email struct {
Id int64
FromField string
ToField string
Cc string
Bcc string

Subject string
Body    string

Success    bool
ErrMessage string

CreatedAt time.Time

tableName struct{} `pg:",discard_unknown_columns"`

}
````

After the above changes, we are going to change every single one of our 80+ table definitions to add nullable tags where appropriate.

``` type Email struct { Id int64 FromField stringsql:",nullable" ToField stringsql:",nullable" Cc stringsql:",nullable" Bcc stringsql:",nullable"`

Subject string `sql:",nullable"`
Body    string `sql:",nullable"`

Success    bool `sql:",nullable"`
ErrMessage string `sql:",nullable"`

CreatedAt time.Time `sql:",nullable"`

tableName struct{} `pg:",discard_unknown_columns"`

}
````

It seems non-intuitive for go-pg to drop the notnull tag and instead require developers to litter their table definitions with nullable tags.

All SQL database systems that i'm familiar with default column definitions to nullable, and require explicit 'not null' to override that default behavior.

i.e. W3 Schools info on not null: https://www.w3schools.com/sql/sql_notnull.asp

Can you please provide additional color on the reasoning behind the linked PR, or roll back the changes that dropped not-null behavior?

Thanks!

[vmihailenco]

sql:",nullable" only affects CreateTable and you likely don't use it in existing project (it is mainly for bootstraping / tests). But you will need to add pg:",zeroable" if you want '' instead of NULL for empty strings.

NOT NULL by default is because in my projects most columns are not nullable. Together with default "" -> NULL conversion it allow you to make sure you don't forget to set some fields.

[eparziale]

@vmihailenco Thanks for the extra details.

Understood that sql:",nullable" only affects CreateTable, so it won't impact our current application in prod.

However, we do use CreateTable in our project when running tests (We do a full drop and rebuild of database schema using go-pg CreatTable in order to run end-to-end integration tets) so if this PR stands, we will need to add sql:",nullable" to almost every single column in our existing 80+ tables...

I don't have any problem with the pg:",zeroable" changes. I think that tag is a valuable new feature enabling apps to save '' instaed of NULL for empty strings.

Here is the most important point to discuss:
You say:

NOT NULL by default is because in my projects most columns are not nullable. Together with default"" -> NULL conversion it allow you to make sure you don't forget to set some fields.

In the above statement you say that you made this change because in _your_ projects most columns are not nullable. Even though _your_ projects regularly enforce all table columns to be not null, shouldn't the change be made in terms of usability and applicability of column definitions to _go-pg integrations as a whole_? i.e. column default null vs not-null should be decided as it pertains to how Postgres is normally used.

See here: https://www.w3resource.com/PostgreSQL/not-null.php
The reverse of NOT NULL constraint is NULL, but it is not necessary to mention NULL to create a table, _the default is NULL_, which does not mean that the column must contain NULL, it means that the column might contain NULL value.

I really like using go-pg, and my team has been very pleased with how it works. Part of what I like is that the model definitions (https://github.com/go-pg/pg/wiki/Model-Definition) are in line with standard and familiar Postgres features.

I feel like making columns NOT NULL by default in go-pg breaks part of natural correspondance between go-pg and postgres.

Even though most columns in _your projects_ are not nullable, I don't think that is a good enough reason to make columns not null by default across all of go-pg.

Perhaps you know other go-pg power users who can provide an opinion on this matter, but I strongly feel that columns should be nullable by default (Just like standard Postgres and other SQL database table definitions).

Please please keep go-pg standard table definitions mostly in line with existing Postgres standards. This change feels unnatural.

[andrewdbase]

I feel like making columns NOT NULL by default in go-pg breaks part of natural correspondance between go-pg and postgres.

I agree here with @eparziale . Parity between Postgres and go-pg is paramount. @vmihailenco personal preferences for his particular projects probably shouldn't be the deciding factor for a change like this. That would be quite arbitrary.

[djvt]

we are having the same issue. thanks for reporting this. hopefully it is resolved soon.

[vmihailenco]

we will need to add sql:",nullable" to almost every single column in our existing 80+ tables...

How much will that take? 30 minutes at most? It is annoying but it is not that big task if you just add it to the every field. And most users are supposed to have NOT NULL on most of their columns so they actually will remove a bunch of sql:".notnull" tags.

In the above statement you say that you made this change because in your projects most columns are not nullable

I've made these changes because of

• https://github.com/go-pg/pg/issues/1306
• https://github.com/go-pg/pg/issues/1304

It is impossible to fix them without breaking current behavior of sql:",notnull". So while breaking existing behaviour I thought it makes sense to pick better defaults.

The good thing about sql:",nullable" is that it helps to understand pg:",zeroable". First allows SQL NULL and second "allows" Go 0, "", [] etc.

Overall until yesterday sql:",notnull" was mostly used to control how go-pg generates NULL and DEFAULT values. It was also used in UpdateNotNull but no more.

I personally don't use CreateTable so I don't care much about what is the default. But if we remove sql:",nullable" we probably should also rename pg:",zeroable" since it becomes even harder to undertand. pg:",allowzero" or pg:",usezero" are possible candidates.

What do you think?

[eparziale]

Thanks @vmihailenco

Agree it won't take more than 30 mins to add nullable constraint to our table columns.

However when you say:

most users are supposed to have NOT NULL on most of their columns so they actually will remove a bunch of sql:".notnull" tags.

That is a poor reason for making not null the default behavior of go-pg column definitions. First - If most users are 'supposed' to have NOT NULL on most of their columns, then why do all major SQL Databases default columns to nullable? Second - As mentioned above, making columns not null by default breaks parity between go-pg and postgres table definitions.

_How about this_ - Here's a middle ground that solves both #1306, and #1304, is backwards-compatible, keeps parity between go-pg and Postgres, and is straightforward to implement.

1. By default, columns should be nullable (Just like Postgres default). How it used to be in go-pg.

2. By default, go-pg should convert 0, empty strings, and nil objects to SQL NULL. When reading this data back, SQL NULL should be marshaled to respective 0, empty string, or nil object.

3. Developers should be able to add sql:",notnull" tag to add not-null constraint to specific columns as part of createTable command. (sql:",notnull" should ONLY mean SQL NOT NULL constraint and nothing more) With this point and point 2 above, go-pg will enforce that no golang 0, empty string, or nil objects get inserted into a specific table column that has requirement of being not null. (This solves #1304)

4. As you mention, we should rename the pg:",zeroable" tag to pg:",usezero" as it's clearer for developers. Sometimes teams may want to save 0 and empty strings as-is in the database. However, the documentation should provide a warning that if both zeroable and notnull tags are used in the same column definition, then updating/inserting empty values in go structs will not throw not null constraint DB errors because empty go-values (0, '') will be saved as-is in the DB.

5. Keep the changes to rename and tweak UpdateNotNull to UpdateNonZero here: https://github.com/go-pg/pg/pull/1307/files#diff-e806fe1161547db6adc83097b0672ff6 (This solves #1306)

With the above points, we solve the two issues, we keep parity between go-pg and postgres regarding notnull, and the changes are really easy to implement

All you need to do to make the changes are the following:

1. Roll back PR 1309 https://github.com/go-pg/pg/pull/1309/files

@vmihailenco Please let me know if this summary makes sense.

Thanks!

[eparziale]

Awesome! Thanks a bunch @vmihailenco

[vmihailenco]

You are welcome!