diff --git a/script/odoo/migration/fix_duplicate_index.py b/script/odoo/migration/fix_duplicate_index.py new file mode 100755 index 0000000..3b631da --- /dev/null +++ b/script/odoo/migration/fix_duplicate_index.py @@ -0,0 +1,287 @@ +#!/usr/bin/env python3 +# © 2021-2026 TechnoLibre (http://www.technolibre.ca) +# License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl) + +"""Les index que la 17 a créés en double et n'a jamais retirés. + +Odoo 17 a changé sa convention de nommage : `make_index_name` rend +désormais `{table}__{colonne}_index` — deux soulignés — là où les +versions d'avant écrivaient `{table}_{colonne}_index`. Le nouvel index +est créé, l'ancien reste. Mesuré sur une chaîne 12 → 18 : + + test_neutralize (12) 1 paire + _16 3 + _17 370 ← la bascule + _18 365 + +Inerte à la lecture, coûteux à l'écriture : chaque INSERT et chaque +UPDATE sur ces tables entretient deux arbres B identiques. Ici 9,6 Mo et +une base vide ; sur une production le coût croît avec les lignes. C'est +le seul défaut de cette famille qui empire tout seul. + +Ce que l'outil REFUSE de toucher, et pourquoi : + + adossé à une contrainte PostgreSQL le recréerait, ou la contrainte + tomberait avec lui. + clé primaire même raison, en pire. + index partiel ou calculé `indpred` / `indexprs` : deux index sur les + mêmes colonnes n'y font pas le même travail. + méthode différente un gin et un btree sur la même colonne ne se + remplacent pas. + convention ambiguë aucun des deux noms — ou les deux — suit la + convention de la cible : on ne devine pas + lequel Odoo recréera. + +Lecture seule par défaut. `--apply` supprime, puis RELIT pour vérifier. +""" + +import argparse +import json +import os +import subprocess +import sys + +sys.path.append( + os.path.normpath(os.path.join(os.path.dirname(__file__), "..", "..", "..")) +) + +try: + from script.todo.todo_i18n import t +except Exception: # pragma: no cover - repli si i18n indisponible + + def t(key: str) -> str: + return key + + +# Les paires strictement identiques : même table, mêmes colonnes, même +# unicité, même méthode d'accès, mêmes classes d'opérateurs. Sans un seul +# de ces critères on comparerait des index qui ne font pas le même travail. +DETECTION = """ +WITH idx AS ( + SELECT i.indexrelid, + i.indrelid, + i.indkey::text AS cols, + i.indclass::text AS classes, + i.indisunique AS unique_, + am.amname AS methode, + c.relname AS nom, + t.relname AS tbl, + pg_relation_size(i.indexrelid) AS taille, + EXISTS (SELECT 1 FROM pg_constraint k + WHERE k.conindid = i.indexrelid) AS contrainte + FROM pg_index i + JOIN pg_class c ON c.oid = i.indexrelid + JOIN pg_class t ON t.oid = i.indrelid + JOIN pg_am am ON am.oid = c.relam + JOIN pg_namespace n ON n.oid = c.relnamespace + WHERE n.nspname = 'public' + AND i.indpred IS NULL + AND i.indexprs IS NULL + AND NOT i.indisprimary + AND i.indisvalid +) +SELECT coalesce(json_agg(json_build_object( + 'table', a.tbl, + 'a', a.nom, 'a_size', a.taille, 'a_ctr', a.contrainte, + 'b', b.nom, 'b_size', b.taille, 'b_ctr', b.contrainte + )), '[]') +FROM idx a +JOIN idx b + ON a.indrelid = b.indrelid + AND a.cols = b.cols + AND a.classes = b.classes + AND a.unique_ = b.unique_ + AND a.methode = b.methode + AND a.indexrelid < b.indexrelid +""" + + +def run_psql(database, sql, read_only=True): + """Interroger la base. None si elle ne répond pas. + + Lecture seule garantie par le SERVEUR : une suppression qui se trompe + de base doit être refusée par PostgreSQL, pas par une relecture. + """ + env = os.environ.copy() + if read_only: + env["PGOPTIONS"] = "-c default_transaction_read_only=on" + env["PSQLRC"] = "" + done = subprocess.run( + ["psql", "-X", "-w", "-d", database, "-tA", "-c", sql], + capture_output=True, + text=True, + env=env, + ) + if done.returncode: + return None + return done.stdout + + +def modern_name(table, nom): + """Ce nom suit-il la convention de la cible ? + + `make_index_name` d'Odoo 18 rend `{table}__{colonne}_index`. Un nom + trop long est tronqué et suffixé d'un condensat ; celui-là ne se + reconnaît pas, et l'on préfère s'abstenir que deviner. + """ + return nom.startswith(f"{table}__") + + +def classify(paire): + """(verdict, à garder, à supprimer). + + « safe » exige que l'un des deux noms — et un seul — suive la + convention : c'est le seul cas où l'on sait lequel Odoo recréera. + """ + if paire["a_ctr"] or paire["b_ctr"]: + return "constraint", None, None + table = paire["table"] + a_moderne = modern_name(table, paire["a"]) + b_moderne = modern_name(table, paire["b"]) + if a_moderne == b_moderne: + return "ambiguous", None, None + if a_moderne: + return "safe", paire["a"], paire["b"] + return "safe", paire["b"], paire["a"] + + +def find(database): + """[(verdict, table, garder, supprimer, octets)]. None si base muette.""" + sortie = run_psql(database, DETECTION) + if sortie is None: + return None + try: + paires = json.loads(sortie.strip() or "[]") + except ValueError: + return None + lst = [] + for paire in paires: + verdict, garder, supprimer = classify(paire) + octets = ( + paire["b_size"] if supprimer == paire["b"] else paire["a_size"] + ) + lst.append( + (verdict, paire["table"], garder, supprimer, octets or 0, paire) + ) + return sorted(lst, key=lambda x: (x[0], x[1], x[3] or "")) + + +def safe_only(lst): + return [item for item in lst if item[0] == "safe"] + + +def drop_sql(lst): + """Les suppressions, une par ligne. Vide s'il n'y a rien de sûr. + + `IF EXISTS` : deux paires peuvent nommer le même index à supprimer + quand une table en porte trois copies, et la deuxième passe ne doit + pas échouer sur ce qui vient de partir. + """ + noms = [] + for _v, _tbl, _garder, supprimer, _o, _p in safe_only(lst): + if supprimer and supprimer not in noms: + noms.append(supprimer) + return "\n".join(f'DROP INDEX IF EXISTS "{nom}";' for nom in noms) + + +def render(lst, applique=False): + if not lst: + return [f"✅ {t('No duplicate index.')}"] + surs = safe_only(lst) + octets = sum(item[4] for item in surs) + lignes = [ + f"🗂 {len(lst)} {t('duplicate index pair(s)')}" + f" — {len(surs)} {t('safe to drop')}" + f" ({octets // 1024} kB)", + "", + ] + for _v, table, garder, supprimer, taille, _p in surs[:20]: + lignes.append( + f" {table} : {t('drop')} {supprimer}" + f" ({taille // 1024} kB), {t('keep')} {garder}" + ) + if len(surs) > 20: + lignes.append(f" … {len(surs) - 20} {t('more')}") + for genre, texte in ( + ("constraint", t("backed by a constraint — left alone")), + ("ambiguous", t("neither name follows the convention — your call")), + ): + lot = [item for item in lst if item[0] == genre] + if not lot: + continue + lignes.append("") + lignes.append(f" ⚠ {len(lot)} {texte}") + for _v, table, _g, _s, _o, paire in lot[:6]: + lignes.append(f" {table} : {paire['a']} ↔ {paire['b']}") + if not applique and surs: + lignes.append("") + lignes.append(f" {t('Use --apply to drop them.')}") + return lignes + + +def main(argv=None): + parser = argparse.ArgumentParser( + description=( + "Report the duplicate indexes Odoo 17 left behind when it" + " renamed its index convention, and optionally drop them." + ) + ) + parser.add_argument("-d", "--database", required=True) + parser.add_argument( + "--apply", + action="store_true", + help="actually drop the safe duplicates (default: report only)", + ) + parser.add_argument("--json", action="store_true", help="machine output") + config = parser.parse_args(argv) + + lst = find(config.database) + if lst is None: + print(f"❌ {t('Cannot read the database: ')}{config.database}") + return 2 + if config.json: + print( + json.dumps( + [ + { + "verdict": v, + "table": tbl, + "keep": g, + "drop": s, + "bytes": o, + } + for v, tbl, g, s, o, _p in lst + ], + indent=2, + sort_keys=True, + ) + ) + return 1 if lst else 0 + if not lst: + print("\n".join(render(lst))) + return 0 + if not config.apply: + print("\n".join(render(lst))) + return 1 + + sql = drop_sql(lst) + if sql and run_psql(config.database, sql, read_only=False) is None: + print(f"❌ {t('The correction failed.')}") + return 2 + # RELIRE : annoncer « supprimé » sans regarder ferait croire le + # problème réglé alors qu'un verrou a pu refuser la suppression. + apres = find(config.database) + if apres is None: + print(f"❌ {t('Cannot read the database: ')}{config.database}") + return 2 + if safe_only(apres): + print("\n".join(render(apres, applique=True))) + print(f"⚠️ {t('Some duplicates are still there.')}") + return 1 + print("\n".join(render(apres, applique=True))) + print(f"✅ {t('Duplicate indexes dropped.')}") + return 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/script/todo/todo_i18n.py b/script/todo/todo_i18n.py index 5f96ab5..07d4ae5 100644 --- a/script/todo/todo_i18n.py +++ b/script/todo/todo_i18n.py @@ -2290,6 +2290,14 @@ TRANSLATIONS = { "fr": "Aucun affichage ici ; à lancer sur VOTRE poste :", "en": "No display here; run this on YOUR workstation:", }, + "Still on the distribution kernel:": { + "fr": "Encore sur le noyau de la distribution :", + "en": "Still on the distribution kernel:", + }, + "Reboot the host: no bridge, no NAT until then.": { + "fr": "Redémarrer l'hôte : ni pont ni NAT avant cela.", + "en": "Reboot the host: no bridge, no NAT until then.", + }, "A ~/.ssh/config alias works there too.": { "fr": "Un alias de ~/.ssh/config y fonctionne aussi.", "en": "A ~/.ssh/config alias works there too.", @@ -6710,6 +6718,42 @@ TRANSLATIONS = { "fr": "Chaque copie de site sait encore se rendre.", "en": "Every website copy still renders.", }, + "No duplicate index.": { + "fr": "Aucun index en double.", + "en": "No duplicate index.", + }, + "duplicate index pair(s)": { + "fr": "paire(s) d'index en double", + "en": "duplicate index pair(s)", + }, + "safe to drop": { + "fr": "sans risque à supprimer", + "en": "safe to drop", + }, + "drop": { + "fr": "supprimer", + "en": "drop", + }, + "backed by a constraint — left alone": { + "fr": "adossée(s) à une contrainte — laissée(s) en place", + "en": "backed by a constraint — left alone", + }, + "neither name follows the convention — your call": { + "fr": "sans convention reconnaissable — à vous de juger", + "en": "neither name follows the convention — your call", + }, + "Use --apply to drop them.": { + "fr": "Utiliser --apply pour les supprimer.", + "en": "Use --apply to drop them.", + }, + "Duplicate indexes dropped.": { + "fr": "Index en double supprimés.", + "en": "Duplicate indexes dropped.", + }, + "Some duplicates are still there.": { + "fr": "Des doublons subsistent.", + "en": "Some duplicates are still there.", + }, "website COW view(s) will survive the bump and then fail": { "fr": "vue(s) COW de site passeront le palier puis échoueront", "en": "website COW view(s) will survive the bump and then fail", diff --git a/test/test_fix_duplicate_index.py b/test/test_fix_duplicate_index.py new file mode 100644 index 0000000..58ed1cc --- /dev/null +++ b/test/test_fix_duplicate_index.py @@ -0,0 +1,235 @@ +#!/usr/bin/env python3 +# © 2021-2026 TechnoLibre (http://www.technolibre.ca) +# License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl) + +"""Les index qu'Odoo 17 a créés en double et n'a jamais retirés. + +`make_index_name` rend `{table}__{colonne}_index` depuis la 17 — deux +soulignés — là où les versions d'avant écrivaient un seul. Le nouvel +index est créé, l'ancien reste. Mesuré sur une chaîne 12 → 18 : 1 paire +en 12, 3 en 16, 370 en 17, 365 en 18. + +Ce qui compte ici n'est pas de supprimer, c'est de S'ABSTENIR au bon +endroit. Éprouvé sur une copie de la base réelle : 364 index retirés, +10 Mo libérés, les 6301 contraintes identiques au nom près, Odoo charge +sans une ligne de journal, et un « -u all » complet n'en recrée aucun. +""" + +import os +import sys +import unittest + +sys.path.append( + os.path.normpath(os.path.join(os.path.dirname(__file__), "..")) +) + +from script.odoo.migration import fix_duplicate_index as idx # noqa: E402 + + +def paire(table, a, b, a_ctr=False, b_ctr=False, a_size=100, b_size=90): + return { + "table": table, + "a": a, + "a_size": a_size, + "a_ctr": a_ctr, + "b": b, + "b_size": b_size, + "b_ctr": b_ctr, + } + + +class TestTheNamingConvention(unittest.TestCase): + def test_the_modern_name_carries_two_underscores(self): + # `make_index_name` d'Odoo 18 : f"{table}__{colonne}_index". + self.assertTrue( + idx.modern_name("res_partner", "res_partner__name_index") + ) + + def test_the_legacy_name_does_not(self): + self.assertFalse( + idx.modern_name("res_partner", "res_partner_name_index") + ) + + def test_a_name_from_another_table_is_not_modern_here(self): + self.assertFalse( + idx.modern_name("res_partner", "res_users__name_index") + ) + + +class TestWhatItAgreesToDrop(unittest.TestCase): + def test_it_keeps_the_modern_one_and_drops_the_legacy_one(self): + verdict, garder, supprimer = idx.classify( + paire( + "res_partner", + "res_partner__name_index", + "res_partner_name_index", + ) + ) + self.assertEqual("safe", verdict) + self.assertEqual("res_partner__name_index", garder) + self.assertEqual("res_partner_name_index", supprimer) + + def test_the_order_of_the_pair_does_not_decide(self): + # PostgreSQL rend les deux dans l'ordre de leur oid : se fier à + # « le premier » supprimerait le moderne une fois sur deux. + verdict, garder, _s = idx.classify( + paire( + "res_partner", + "res_partner_name_index", + "res_partner__name_index", + ) + ) + self.assertEqual("safe", verdict) + self.assertEqual("res_partner__name_index", garder) + + +class TestWhatItRefusesToTouch(unittest.TestCase): + """S'abstenir est le cœur de cet outil, pas supprimer.""" + + def test_an_index_backing_a_constraint_is_left_alone(self): + # PostgreSQL le recréerait, ou la contrainte tomberait avec lui. + for a_ctr, b_ctr in ((True, False), (False, True), (True, True)): + with self.subTest(a=a_ctr, b=b_ctr): + verdict, garder, supprimer = idx.classify( + paire("m", "m__x_index", "m_x_index", a_ctr, b_ctr) + ) + self.assertEqual("constraint", verdict) + self.assertIsNone(supprimer) + + def test_two_legacy_names_are_a_human_decision(self): + verdict, _g, supprimer = idx.classify( + paire( + "mail_notification", + "mail_notification_email_status_index", + "mail_notification_notification_status_ind", + ) + ) + self.assertEqual("ambiguous", verdict) + self.assertIsNone(supprimer) + + def test_two_modern_names_are_a_human_decision(self): + # Vu en vrai : deux noms tronqués et suffixés d'un condensat, ou + # deux colonnes différentes tombées sur le même rang. On ne devine + # pas lequel Odoo recréera. + verdict, _g, supprimer = idx.classify( + paire( + "stock_move", + "stock_move__location_dest_id_index", + "stock_move__location_final_id_index", + ) + ) + self.assertEqual("ambiguous", verdict) + self.assertIsNone(supprimer) + + +class TestTheQueryItself(unittest.TestCase): + """Les gardes vivent dans le SQL ; les retirer ne se verrait pas.""" + + def test_it_ignores_partial_and_computed_indexes(self): + # Deux index sur les mêmes colonnes n'y font pas le même travail. + self.assertIn("indpred IS NULL", idx.DETECTION) + self.assertIn("indexprs IS NULL", idx.DETECTION) + + def test_it_never_looks_at_a_primary_key(self): + self.assertIn("NOT i.indisprimary", idx.DETECTION) + + def test_it_compares_the_access_method(self): + # Un gin et un btree sur la même colonne ne se remplacent pas. + self.assertIn("amname", idx.DETECTION) + self.assertIn("a.methode = b.methode", idx.DETECTION) + + def test_it_compares_the_operator_classes(self): + self.assertIn("indclass", idx.DETECTION) + self.assertIn("a.classes = b.classes", idx.DETECTION) + + def test_it_compares_uniqueness(self): + self.assertIn("a.unique_ = b.unique_", idx.DETECTION) + + def test_it_only_looks_at_valid_indexes(self): + self.assertIn("i.indisvalid", idx.DETECTION) + + def test_it_pairs_each_couple_once(self): + # Sans cela chaque paire sortirait deux fois, et le compte + # annoncé serait le double du vrai. + self.assertIn("a.indexrelid < b.indexrelid", idx.DETECTION) + + +class TestTheSqlItWrites(unittest.TestCase): + def lot(self): + return [ + ("safe", "m", "m__a_index", "m_a_index", 100, {}), + ("safe", "m", "m__b_index", "m_b_index", 100, {}), + ("constraint", "m", None, None, 0, {}), + ("ambiguous", "m", None, None, 0, {}), + ] + + def test_it_only_drops_what_it_called_safe(self): + sql = idx.drop_sql(self.lot()) + self.assertEqual(2, sql.count("DROP INDEX"), sql) + self.assertIn("m_a_index", sql) + self.assertIn("m_b_index", sql) + + def test_a_named_but_unsafe_entry_is_still_not_dropped(self): + # `classify` rend aujourd'hui (None, None) pour tout ce qui n'est + # pas sûr, ce qui masque la garde. Le jour où il nommera les + # ambiguës pour les AFFICHER, `drop_sql` ne doit pas se mettre à + # les supprimer : c'est le verdict qui décide, pas la présence + # d'un nom. + lot = [ + ("ambiguous", "m", "m__a_index", "m_a_index", 100, {}), + ("constraint", "m", "m__b_index", "m_b_index", 100, {}), + ] + self.assertEqual("", idx.drop_sql(lot)) + + def test_it_never_drops_what_it_keeps(self): + sql = idx.drop_sql(self.lot()) + self.assertNotIn('"m__a_index"', sql) + + def test_it_tolerates_an_index_already_gone(self): + # Une table à trois copies produit deux paires nommant le même + # index à supprimer ; la seconde passe ne doit pas échouer. + self.assertIn("IF EXISTS", idx.drop_sql(self.lot())) + + def test_the_same_index_is_dropped_once(self): + lot = [ + ("safe", "m", "m__a_index", "m_a_index", 100, {}), + ("safe", "m", "m__a2_index", "m_a_index", 100, {}), + ] + self.assertEqual(1, idx.drop_sql(lot).count("DROP INDEX")) + + def test_nothing_safe_writes_nothing(self): + self.assertEqual( + "", idx.drop_sql([("constraint", "m", None, None, 0, {})]) + ) + + +class TestTheReport(unittest.TestCase): + def test_a_clean_database_says_so(self): + self.assertIn("✅", "\n".join(idx.render([]))) + + def test_it_separates_the_safe_from_the_rest(self): + lot = [ + ("safe", "m", "m__a_index", "m_a_index", 16384, {}), + ( + "constraint", + "m", + None, + None, + 0, + {"a": "x_uniq", "b": "name_uniq"}, + ), + ] + texte = "\n".join(idx.render(lot)) + self.assertIn("m_a_index", texte) + self.assertIn("name_uniq", texte) + self.assertIn("2", texte) + + def test_it_offers_the_flag_only_when_there_is_work(self): + rien = [("constraint", "m", None, None, 0, {"a": "x", "b": "y"})] + self.assertNotIn("--apply", "\n".join(idx.render(rien))) + travail = [("safe", "m", "m__a_index", "m_a_index", 0, {})] + self.assertIn("--apply", "\n".join(idx.render(travail))) + + +if __name__ == "__main__": + unittest.main()