Should `allowNull: false` be equivalent to `notNull: true`?

[andresakata]

Currently, allowNull: false is not equivalent to notNull: true. It doesn't add a NOT NULL constraint. I think it can lead to mistakes. Is it intended to function in that manner?

[Shinigami92]

allowNull exists in alterColumn and in alterDomain. Do you mean one specific? Or is the request equivalent for both?

[andresakata]

I was thinking of alterColumn, but it looks like it applies to both.

[brenoepics]

I agree, it can be a bit confusing, but IMO adding a breaking change like that in such a widely used function can be tricky.

We could either

• Add runtime guardrails in alterColumn: If allowNull === false and notNull !== true, warn: allowNull: false is a no‑op; use notNull: true to add NOT NULL. If both notNull and allowNull are provided, warn about the conflict.

• Tighten TypeScript types: Maybe using a union type allowing only { notNull: true } | { notNull: false } | { allowNull: true } | {}... Mark allowNull: false as deprecated.

[Shinigami92]

https://www.postgresql.org/docs/current/sql-altertable.html#SQL-ALTERTABLE-DESC-SET-DROP-NOT-NULL

ALTER [ COLUMN ] column_name { SET | DROP } NOT NULL

https://www.postgresql.org/docs/current/sql-alterdomain.html

ALTER DOMAIN name { SET | DROP } NOT NULL

My personal preference would be to align with the official documentation as near as possible. They use SET/DROP for NOT NULL. So if we change that and even aiming for a breaking change, we could introduce a notNull: 'SET'/notNull: 'DROP'.

But yes, for the upcoming v10 we should first introduce the mentioned warning and also add the additional new values for notNull and deprecate allowNull.

@brenoepics Please sanity-check me so I don't have missed something fundamentally 😅 Also we should try this out in a PR and see how the integration tests react.