Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 36 additions & 0 deletions src/base/tests/test_util.py
Original file line number Diff line number Diff line change
Expand Up @@ -768,6 +768,42 @@ def test_pg_text2html(self, value, expected):
result = cr.fetchone()[0]
self.assertEqual(result, expected)

@parametrize(
[
("{parallel_filter}", "…"),
("{{parallel_filter}}", "{parallel_filter}"),
("{}", "{}"),
("{0}", "{0}"),
("{{0}}", "{0}"),
("{x}", "{x}"),
("{{x}}", "{x}"),
("{{}}", "{}"),
("{{", "{"),
("test", "test"),
("", ""),
("WHERE {parallel_filter} AND true", "WHERE … AND true"),
("WHERE {parallel_filter} AND {other}", "WHERE … AND {other}"),
("WHERE {parallel_filter} AND {other!r}", "WHERE … AND {other!r}"),
("WHERE {parallel_filter} AND {{other}}", "WHERE … AND {other}"),
("WHERE {parallel_filter} AND {}", "WHERE … AND {}"),
("WHERE {parallel_filter} AND {{}}", "WHERE … AND {}"),
("WHERE {parallel_filter} AND {parallel_filter}", "WHERE … AND …"),
("using { with other things inside } and {parallel_filter}", "using { with other things inside } and …"),
]
)
def test_ExplodeFormatter(self, value, expected):
formatted = util.pg._ExplodeFormatter().format(value, parallel_filter="…")
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
formatted = util.pg._ExplodeFormatter().format(value, parallel_filter="")
formatted = util.pg._ExplodeFormatter().format(value, parallel_filter="...")

np: I can imagine someone trying to add a test case being confused by the unicode character here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It was copied from the commit message stolen from #142

Copy link
Contributor

Choose a reason for hiding this comment

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

It did look familiar. :p
For what it's worth, I have since removed the vim abbrev rule that replaced triple dots with that one.

self.assertEqual(formatted, expected)
# retro-compatibility test
try:
std_formatted = value.format(parallel_filter="…")
except (IndexError, KeyError):
# ignore string that weren't valid
pass
else:
# assert that the new formatted output match the old one.
self.assertEqual(formatted, std_formatted)

def _get_cr(self):
cr = self.registry.cursor()
self.addCleanup(cr.close)
Expand Down
47 changes: 43 additions & 4 deletions src/util/pg.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import logging
import os
import re
import string
import threading
import time
import uuid
Expand Down Expand Up @@ -220,6 +221,43 @@ def wrap(arg):
return SQLStr(sql.SQL(query).format(*args, **kwargs).as_string(cr._cnx))


class _ExplodeFormatter(string.Formatter):
"""
Retro-compatible parallel filter formatter.

Any input that didn't fail before satisfies:
1. There is no replacement in the string other than `{parallel_filter}`.
2. Any literal brace was escaped --by doubling them.

For any input that didn't fail before this new implementation returns the same output
as `str.format`.

The main change here, and the goal of this class, is to now make former invalid input
valid. Thus this formatter will _only_ replace `{parallel_filter}` while keeping any
other `{str}` or `{int}` elements. Double braces will still be formatted into
single ones.

:meta private: exclude from online docs
"""

def parse(self, format_string):
for literal_text, field_name, format_spec, conversion in super(_ExplodeFormatter, self).parse(format_string):
if field_name is not None and field_name != "parallel_filter":
yield literal_text + "{", None, None, None
composed = (
field_name
+ (("!" + conversion) if conversion else "")
+ ((":" + format_spec) if format_spec else "")
+ "}"
)
yield composed, None, None, None
else:
yield literal_text, field_name, format_spec, conversion


_explode_format = _ExplodeFormatter().format


def explode_query(cr, query, alias=None, num_buckets=8, prefix=None):
"""
Explode a query to multiple queries that can be executed in parallel.
Expand Down Expand Up @@ -256,7 +294,7 @@ def explode_query(cr, query, alias=None, num_buckets=8, prefix=None):
if num_buckets < 1:
raise ValueError("num_buckets should be greater than zero")
parallel_filter = "mod(abs({prefix}id), %s) = %s".format(prefix=prefix)
query = query.replace("%", "%%").format(parallel_filter=parallel_filter)
query = _explode_format(query.replace("%", "%%"), parallel_filter=parallel_filter)
return [cr.mogrify(query, [num_buckets, index]).decode() for index in range(num_buckets)]


Expand Down Expand Up @@ -294,7 +332,7 @@ def explode_query_range(cr, query, table, alias=None, bucket_size=DEFAULT_BUCKET
# Even if there are any records, return one query to be executed to validate its correctness and avoid
# scripts that pass the CI but fail in production.
parallel_filter = "{alias}.id IS NOT NULL".format(alias=alias)
return [query.format(parallel_filter=parallel_filter)]
return [_explode_format(query, parallel_filter=parallel_filter)]
else:
return []

Expand Down Expand Up @@ -335,10 +373,11 @@ def explode_query_range(cr, query, table, alias=None, bucket_size=DEFAULT_BUCKET
# Still, since the query may only be valid if there is no split, we force the usage of `prefix` in the query to
# validate its correctness and avoid scripts that pass the CI but fail in production.
parallel_filter = "{alias}.id IS NOT NULL".format(alias=alias)
return [query.format(parallel_filter=parallel_filter)]
return [_explode_format(query, parallel_filter=parallel_filter)]

parallel_filter = "{alias}.id BETWEEN %(lower-bound)s AND %(upper-bound)s".format(alias=alias)
query = query.replace("%", "%%").format(parallel_filter=parallel_filter)
query = _explode_format(query.replace("%", "%%"), parallel_filter=parallel_filter)

return [
cr.mogrify(query, {"lower-bound": ids[i], "upper-bound": ids[i + 1] - 1}).decode() for i in range(len(ids) - 1)
]
Expand Down