[FIX] pg: avoid dropping not null constraints in pg18

[Pirols]

In PG18 not null constraint names follow the '{table}_{column}not_null' pattern as opposed to the '[0-9]+_not_null' one followed until pg17.

Our utils must be adapted to cope with that, otherwise the util will attempt to drop not null constraints - failing against those of primary keys.

The following traceback is achieved when trying to upgrade a new demo db in PG18 with mrp installed, from 17.0 to 18.0:

2025-11-04 10:23:38,126 28873 ERROR test_mrp_17_18.0 odoo.sql_db: bad query: b'ALTER TABLE "mail_push" DROP CONSTRAINT IF EXISTS "mail_notification_web_push_id_not_null" '
ERROR: column "id" is in a primary key

2025-11-04 10:23:38,127 28873 WARNING test_mrp_17_18.0 odoo.modules.loading: Transient module states were reset
2025-11-04 10:23:38,129 28873 ERROR test_mrp_17_18.0 odoo.modules.registry: Failed to load registry
2025-11-04 10:23:38,129 28873 CRITICAL test_mrp_17_18.0 odoo.service.server: Failed to initialize database `test_mrp_17_18.0`.
Traceback (most recent call last):
  File "/home/odoo/src/odoo/18.0/odoo/service/server.py", line 1366, in preload_registries
    registry = Registry.new(dbname, update_module=update_module)
  File "<decorator-gen-13>", line 2, in new
  File "/home/odoo/src/odoo/18.0/odoo/tools/func.py", line 97, in locked
    return func(inst, *args, **kwargs)
  File "/home/odoo/src/odoo/18.0/odoo/modules/registry.py", line 129, in new
    odoo.modules.load_modules(registry, force_demo, status, update_module)
  File "/home/odoo/src/odoo/18.0/odoo/modules/loading.py", line 485, in load_modules
    processed_modules += load_marked_modules(env, graph,
  File "/home/odoo/src/odoo/18.0/odoo/modules/loading.py", line 365, in load_marked_modules
    loaded, processed = load_module_graph(
  File "/home/odoo/src/odoo/18.0/odoo/modules/loading.py", line 182, in load_module_graph
    migrations.migrate_module(package, 'pre')
  File "/home/odoo/src/odoo/18.0/odoo/modules/migration.py", line 222, in migrate_module
    exec_script(self.cr, installed_version, pyfile, pkg.name, stage, stageformat[stage] % version)
  File "/home/odoo/src/odoo/18.0/odoo/modules/migration.py", line 259, in exec_script
    mod.migrate(cr, installed_version)
  File "/home/odoo/src/upgrade/migrations/mail/saas~17.1.1.16/pre-migrate.py", line 30, in migrate
    util.rename_model(cr, "mail.notification.web.push", "mail.push")
  File "/home/odoo/src/upgrade-util/src/util/models.py", line 308, in rename_model
    pg_rename_table(cr, old_table, new_table)
  File "/home/odoo/src/upgrade-util/src/util/pg.py", line 1354, in rename_table
    remove_constraint(cr, new_table, const, warn=False)
  File "/home/odoo/src/upgrade-util/src/util/pg.py", line 853, in remove_constraint
    cr.execute(format_query(cr, "ALTER TABLE {} DROP CONSTRAINT IF EXISTS {} {}", table, name, cascade))
  File "/home/odoo/src/odoo/18.0/odoo/sql_db.py", line 357, in execute
    res = self._obj.execute(query, params)
psycopg2.errors.InvalidTableDefinition: column "id" is in a primary key

[robodoo]

[KangOl]

upgradeci retry with always only mrp

[aj-fuentes]

What about #337 ?

Note that in principle we can drop not_null constraints and let the ORM re-recreate them, the only blocking one is the one generated for PKs in PG18... that's IIUC the approach in 337

EDIT: it looks like what we actually need is to skip digits and underscore for PG<18 and table_name_(any_column)_not_null after.

[Pirols]

What about #337 ?

Didn't check for it. 🤦‍♂️

Note that in principle we can drop not_null constraints and let the ORM re-recreate them,

Well yes, but I find it undesirable to have different behavior for constraints created in PG17- vs PG18+.

so the only blocking one is the one generated for PKs in PG18... that's IIUC the approach in 337

From what I see, yes that's roughly the approach in there. With 2 limitations I can think of:

1. Would crash if the primary key column name does not end with id.
2. Does not rename the associated constraint, which remains {old_table}_{column}_not_null.

All of that said, both should work alright in most scenarios. Let me know which one you prefer, I can of course close this one.

[aj-fuentes]

The idea looks correct.

[aj-fuentes]

Does not rename the associated constraint, which remains {old_table}_{column}_not_null.

Yes, this is the primary reason why I think this is useful. If we create a table and rename it in PG18 the constraint name keeps the original table name. This shouldn't be an issue in Odoo but it's anyway confusing. Thus the new renaming scheme you propose here may be better.

[aj-fuentes]

Well yes, but I find it undesirable to have different behavior for constraints created in PG17- vs PG18+.

The difference in behavior is still there, even if our code is the same :)

[Pirols]

Well yes, but I find it undesirable to have different behavior for constraints created in PG17- vs PG18+.

The difference in behavior is still there, even if our code is the same :)

What do you mean?

My commit aims to keep avoiding dropping any not null constraint in PG18+, which AFAICT is what we've been doing forever.

EDIT: it looks like what we actually need is to skip digits and underscore for PG<18 and table_name_(any_column)_not_null after.

Yes. I simplified a bit the condition in https://github.com/odoo/upgrade-util/pull/348/files#diff-11084359fec4e491106b8dde1a63810eb34f95320aa49c3b961d9d25c4f8605fR1343 because the intention seemed to skip all not null constraints. If preferred I can explode it into the 2 explicit ones instead.

[aj-fuentes]

What do you mean?

I meant that we can check the server version and have a different (adapted) query accordingly. Instead of the current approach --which I'm not against, just for the record-- of having the same query dealing with all cases.

My commit aims to keep avoiding dropping any not null constraint in PG18+, which AFAICT is what we've been doing forever.

The former code was removing explicit not null constraints added by the ORM in the form table_name_something_extra.

That being said, I agree with the approach here of keeping and renaming all ..._not_null constraints as long as they start with the table name.

@KangOl wdty?