Skip to content

Conversation

Pirols
Copy link
Contributor

@Pirols Pirols commented Sep 24, 2024

Otherwise errors (such as the following) will ensue.

2024-09-24 12:11:34,164 2158 ERROR matt-purchase odoo.upgrade.base.tests.test_util: ERROR: TestRecords.test_rename_xmlid
Traceback (most recent call last):
  File "/upgrade-util/src/base/tests/test_util.py", line 1030, in test_rename_xmlid
    res = util.rename_xmlid(cr, "base.TX1", "base.TX2", on_collision="merge")
  File "/upgrade-util/src/util/records.py", line 751, in rename_xmlid
    replace_record_references(cr, (model, old_id), (model, new_id), replace_xmlid=False)
  File "/upgrade-util/src/util/records.py", line 1308, in replace_record_references
    return replace_record_references_batch(cr, {old[1]: new[1]}, old[0], new[0], replace_xmlid)
  File "/upgrade-util/src/util/records.py", line 1490, in replace_record_references_batch
    explode_execute(cr, query, table=ir.table)
  File "/upgrade-util/src/util/pg.py", line 352, in explode_execute
    explode_query_range(cr, query, table, alias=alias, bucket_size=bucket_size),
  File "/upgrade-util/src/util/pg.py", line 259, in explode_query_range
    cr.execute(format_query(cr, "SELECT min(id), max(id) FROM {}", table))
  File "/tmp/tmpinhjowz1/odoo/pr/153463/odoo/sql_db.py", line 354, in execute
    res = self._obj.execute(query, params)
psycopg2.errors.UndefinedTable: relation "res.partner" does not exist
LINE 1: SELECT min(id), max(id) FROM "res.partner"

Otherwise errors (such as the following) will ensue.

```
2024-09-24 12:11:34,164 2158 ERROR matt-purchase odoo.upgrade.base.tests.test_util: ERROR: TestRecords.test_rename_xmlid
Traceback (most recent call last):
  File "/upgrade-util/src/base/tests/test_util.py", line 1030, in test_rename_xmlid
    res = util.rename_xmlid(cr, "base.TX1", "base.TX2", on_collision="merge")
  File "/upgrade-util/src/util/records.py", line 751, in rename_xmlid
    replace_record_references(cr, (model, old_id), (model, new_id), replace_xmlid=False)
  File "/upgrade-util/src/util/records.py", line 1308, in replace_record_references
    return replace_record_references_batch(cr, {old[1]: new[1]}, old[0], new[0], replace_xmlid)
  File "/upgrade-util/src/util/records.py", line 1490, in replace_record_references_batch
    explode_execute(cr, query, table=ir.table)
  File "/upgrade-util/src/util/pg.py", line 352, in explode_execute
    explode_query_range(cr, query, table, alias=alias, bucket_size=bucket_size),
  File "/upgrade-util/src/util/pg.py", line 259, in explode_query_range
    cr.execute(format_query(cr, "SELECT min(id), max(id) FROM {}", table))
  File "/tmp/tmpinhjowz1/odoo/pr/153463/odoo/sql_db.py", line 354, in execute
    res = self._obj.execute(query, params)
psycopg2.errors.UndefinedTable: relation "res.partner" does not exist
LINE 1: SELECT min(id), max(id) FROM "res.partner"
```
@Pirols Pirols requested a review from aj-fuentes September 24, 2024 13:58
@robodoo
Copy link
Contributor

robodoo commented Sep 24, 2024

Pull request status dashboard

@KangOl
Copy link
Contributor

KangOl commented Sep 24, 2024

upgradeci retry with always only purchase

Copy link
Contributor

@aj-fuentes aj-fuentes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@KangOl
Copy link
Contributor

KangOl commented Sep 24, 2024

There are still some weird errors: https://upgradeci.odoo.com/upgradeci/run/165581

@Pirols
Copy link
Contributor Author

Pirols commented Sep 24, 2024

There are still some weird errors: https://upgradeci.odoo.com/upgradeci/run/165581

Yes, I'm debugging those.

@aj-fuentes
Copy link
Contributor

aj-fuentes commented Sep 24, 2024

From DM discussion, these lines

query = cr.mogrify(
format_query(
cr,
"""
UPDATE {table}
SET {column} = (
SELECT jsonb_object_agg(key, COALESCE((%s::jsonb->>value)::int, value::int))
FROM jsonb_each_text({column})
)
WHERE {column} IS NOT NULL
AND {column} @? %s
AND {{parallel_filter}}
""",
table=ir.table,
column=ir.res_id,
),
[Json(id_mapping), "$.* ? ({})".format(" || ".join(map("@ == {}".format, id_mapping)))],
).decode()

Should be something like:

                query = cr.mogrify(
                    format_query(
                        cr,
                        """
                        UPDATE {table}
                           SET {column} = (
                                   SELECT jsonb_object_agg(key, COALESCE(
                                                                   jsonb_object(%s::text[], %s::text[])->>value)::int,
                                                                   value::int
                                                                 ))
                                   FROM jsonb_each_text({column})
                               )
                         WHERE {column} IS NOT NULL
                           AND {column} @? %s
                           AND {{parallel_filter}}
                        """,
                        table=ir.table,
                        column=ir.res_id,
                    ),
                    [
                        list(id_mapping.keys()),
                        list(id_mapping.values(), 
                        "$.* ? ({})".format(" || ".join(map("@ == {}".format, id_mapping))),
                    ],
                ).decode()

The key idea: pass the keys and values from id_mapping, to be used as jsonb in the query. Avoiding in the process explicit jsonb dicts in the form {"key1": value1, ...} which causes issues for the query formatter.

@Pirols Pirols force-pushed the master-use_table_name-pied branch from 2b28336 to 508d69e Compare September 24, 2024 16:56
UPDATE {table}
SET {column} = (
SELECT jsonb_object_agg(key, COALESCE((%s::jsonb->>value)::int, value::int))
SELECT jsonb_object(ARRAY_AGG(key), %s)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
SELECT jsonb_object(ARRAY_AGG(key), %s)
SELECT jsonb_object_agg(key, (jsonb_object(%s::text[], %s::text[])->>value)::int, value::int)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this version avoids having literal { in the query.

Let's try it over my patch.

column=ir.res_id,
),
[Json(id_mapping), "$.* ? ({})".format(" || ".join(map("@ == {}".format, id_mapping)))],
[list(map(str, id_mapping.values())), "$.* ? ({})".format(" || ".join(map("@ == {}".format, id_mapping)))],
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
[list(map(str, id_mapping.values())), "$.* ? ({})".format(" || ".join(map("@ == {}".format, id_mapping)))],
[list(id_mapping.keys()), list(id_mapping.values()), "$.* ? ({})".format(" || ".join(map("@ == {}".format, id_mapping)))],

@Pirols Pirols force-pushed the master-use_table_name-pied branch from 508d69e to a518368 Compare September 24, 2024 17:06
@KangOl KangOl force-pushed the master-use_table_name-pied branch 2 times, most recently from 81e900d to aeb3308 Compare September 24, 2024 17:51
Correctly handle changes in company dependent jsonb columns.

Use the function `jsonb_object` instead of a casted string, so
we don't get literal `{` and `}` into the query. Such characters
needs to be escaped to allow the query to be exploded.

Also use `cr.mogrify` instead of manual formatting of the jsonpath
argument of the query.

Co-authored-by: Edoardo Piroli <[email protected]>
Co-authored-by: Alvaro Fuentes <[email protected]>
@KangOl KangOl force-pushed the master-use_table_name-pied branch from aeb3308 to e186ed5 Compare September 24, 2024 18:02
Copy link
Contributor

@KangOl KangOl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@robodoo rebase-ff r+

@robodoo
Copy link
Contributor

robodoo commented Sep 24, 2024

Merge method set to rebase and fast-forward.

robodoo pushed a commit that referenced this pull request Sep 24, 2024
Otherwise errors (such as the following) will ensue.

```
2024-09-24 12:11:34,164 2158 ERROR matt-purchase odoo.upgrade.base.tests.test_util: ERROR: TestRecords.test_rename_xmlid
Traceback (most recent call last):
  File "/upgrade-util/src/base/tests/test_util.py", line 1030, in test_rename_xmlid
    res = util.rename_xmlid(cr, "base.TX1", "base.TX2", on_collision="merge")
  File "/upgrade-util/src/util/records.py", line 751, in rename_xmlid
    replace_record_references(cr, (model, old_id), (model, new_id), replace_xmlid=False)
  File "/upgrade-util/src/util/records.py", line 1308, in replace_record_references
    return replace_record_references_batch(cr, {old[1]: new[1]}, old[0], new[0], replace_xmlid)
  File "/upgrade-util/src/util/records.py", line 1490, in replace_record_references_batch
    explode_execute(cr, query, table=ir.table)
  File "/upgrade-util/src/util/pg.py", line 352, in explode_execute
    explode_query_range(cr, query, table, alias=alias, bucket_size=bucket_size),
  File "/upgrade-util/src/util/pg.py", line 259, in explode_query_range
    cr.execute(format_query(cr, "SELECT min(id), max(id) FROM {}", table))
  File "/tmp/tmpinhjowz1/odoo/pr/153463/odoo/sql_db.py", line 354, in execute
    res = self._obj.execute(query, params)
psycopg2.errors.UndefinedTable: relation "res.partner" does not exist
LINE 1: SELECT min(id), max(id) FROM "res.partner"
```

Part-of: #141
Signed-off-by: Christophe Simonis (chs) <[email protected]>
robodoo pushed a commit that referenced this pull request Sep 24, 2024
Correctly handle changes in company dependent jsonb columns.

Use the function `jsonb_object` instead of a casted string, so
we don't get literal `{` and `}` into the query. Such characters
needs to be escaped to allow the query to be exploded.

Also use `cr.mogrify` instead of manual formatting of the jsonpath
argument of the query.

closes #141

Signed-off-by: Christophe Simonis (chs) <[email protected]>
Co-authored-by: Edoardo Piroli <[email protected]>
Co-authored-by: Alvaro Fuentes <[email protected]>
robodoo pushed a commit that referenced this pull request Sep 24, 2024
Otherwise errors (such as the following) will ensue.

```
2024-09-24 12:11:34,164 2158 ERROR matt-purchase odoo.upgrade.base.tests.test_util: ERROR: TestRecords.test_rename_xmlid
Traceback (most recent call last):
  File "/upgrade-util/src/base/tests/test_util.py", line 1030, in test_rename_xmlid
    res = util.rename_xmlid(cr, "base.TX1", "base.TX2", on_collision="merge")
  File "/upgrade-util/src/util/records.py", line 751, in rename_xmlid
    replace_record_references(cr, (model, old_id), (model, new_id), replace_xmlid=False)
  File "/upgrade-util/src/util/records.py", line 1308, in replace_record_references
    return replace_record_references_batch(cr, {old[1]: new[1]}, old[0], new[0], replace_xmlid)
  File "/upgrade-util/src/util/records.py", line 1490, in replace_record_references_batch
    explode_execute(cr, query, table=ir.table)
  File "/upgrade-util/src/util/pg.py", line 352, in explode_execute
    explode_query_range(cr, query, table, alias=alias, bucket_size=bucket_size),
  File "/upgrade-util/src/util/pg.py", line 259, in explode_query_range
    cr.execute(format_query(cr, "SELECT min(id), max(id) FROM {}", table))
  File "/tmp/tmpinhjowz1/odoo/pr/153463/odoo/sql_db.py", line 354, in execute
    res = self._obj.execute(query, params)
psycopg2.errors.UndefinedTable: relation "res.partner" does not exist
LINE 1: SELECT min(id), max(id) FROM "res.partner"
```

Part-of: #141
Signed-off-by: Christophe Simonis (chs) <[email protected]>
@robodoo robodoo closed this in 79f3d71 Sep 24, 2024
@robodoo robodoo added the 17.5 label Sep 24, 2024
@Pirols Pirols deleted the master-use_table_name-pied branch September 26, 2024 16:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants