SMF icon indicating copy to clipboard operation
SMF copied to clipboard

pacman, pg & mysql: Adding a NOT NULL column to an existing table...

Open sbulen opened this issue 3 years ago • 6 comments

Description

Problem 1: Pg & mysql behave differently with NOT NULL and defaults on text fields. Mysql will NOT allow you to specify a default on a TEXT column. pg will allow it. So... Right there it is possible to create a mod that will install on one but not the other.

Problem 2: If you try to create a column with NOT NULL and NO default, on a table that has existing records, pg will complain, because it feels it must give a value to the column for the existing rows, and you haven't specified one.

So, even if you tried to be consistent, and specified NOT NULL with no default, it wouldn't work.

Note we have had a similar issue with the installer/upgrader earlier. Pacman faces the same issue: https://github.com/SimpleMachines/SMF2.1/pull/7376

Possible solutions? Thinking aloud here...

  • Give an error when you see a default on a text column, forcing pg & mysql to have the same definition
  • If rows exist, in pg, the update must be done in 2 steps, as in the PR above

Environment (complete as necessary)

  • Version/Git revision: Current
  • Database Type: pg & mysql

Additional information/references

NOTE that if submitting a PR, we will want to see proof of testing on pg & mysql. Before & after screenshots.

sbulen avatar Apr 07 '22 03:04 sbulen

I'm not sure of a solution here. The first problem is a issue with mysql. However this stackoverflow says 8.0.13 allows it in a weird way: https://stackoverflow.com/questions/3466872/why-cant-a-text-column-have-a-default-value-in-mysql

I would go on saying that we detect if we are trying to add a default to a text column and not allow it for mysql. But that may not work since it may expect data if its a not null column. There is no win here.

The second problem, I am guessing for new tables, it just wants that column to have a value then on insert? Again, this has a no win scenario for us. We would need a default value to give it a valid setup. Maybe we just force it for both mysql and postgresql that you can not have a default of none and a not null when you do an add column? I think Postgresql code does a slow standup if you look at its add_column function and this would have to be stopped early.

jdarwood007 avatar Apr 09 '22 00:04 jdarwood007

In order to have the table structures consistent across pg & mysql, I think we have to do 2 things:

  • Support the lowest common denominator... Do not allow defaults on text columns. OR, maybe only allow '', since that is mysql's implicit default... & then ignore it for mysql.
  • Changes to pg need to be in 2 steps (just like the pg solution in PR #7376 ) - Issue two commands not one. The first has a default (to fix existing rows) & the second does not. We could possibly narrow that to pg tables that have existing rows.

Today, the only solution that works consistently across pg & mysql is to have columns that are nullable and have a default of null. Any other combination will either yield different db structures or outright fail under some circumstances.

OR... We approve customizations for pg & mysql independently.

sbulen avatar Apr 09 '22 00:04 sbulen

The second solution still requires a default or we have to make an assumed default based on the type (empty string for strings, for numerical, :: for inet, no idea on binary).

jdarwood007 avatar Apr 09 '22 03:04 jdarwood007

FYI my PR #7424 does not address this issue. Since we don't have a conclusion and this isn't a issue with our design, but a issue with how to support 2 database engines that are incompatible with one another on a single feature so completely differently.

jdarwood007 avatar Apr 09 '22 03:04 jdarwood007

in my eyes the conclusion exists, when you want to support the wrong behavior of mysql, you need to simulate this on pg side.

albertlast avatar Apr 09 '22 09:04 albertlast

Again, we have to assume some default then and it may be the wrong data.

I would go with a not allow it. Fail and error out. The mod author can first create the field with a default and change it. Which we would assume the author is going to handle fixing any records to defaults they assume.

jdarwood007 avatar Apr 09 '22 15:04 jdarwood007