Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[IMP] util.explode_query_range: replace problematic parallel_filter #142

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

Pirols
Copy link
Contributor

@Pirols Pirols commented Sep 26, 2024

… formatting

The usage of str.format to inject the parallel filter used to explode queries is not robust to the presence of other curly braces. Examples:

  1. JSON strings (typically to leverage their mapping capabilities):

see 79f3d71, where a query had to be modified to accomodate that.

  1. Hardcoded sets of curly braces:
>>> "UPDATE t SET c = '{usage as literal characters}' WHERE {parallel_filter}".format(parallel_filter="…")
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
KeyError: 'usage as literal characters'

Which can be (unelegantly) solved adding even more braces, leveraging one side-effect of .format:

>>> "UPDATE t SET c = '{{usage as literal characters}}' WHERE {parallel_filter}".format(parallel_filter="…")
"UPDATE t SET c = '{usage as literal characters}' WHERE …"
  1. Hardcoded curly braces (AFAICT no way to solve this):
>>> "UPDATE t SET c = 'this is an open curly brace = {' WHERE {parallel_filter}".format(parallel_filter="…")
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
ValueError: unexpected '{' in field name
>>> "UPDATE t SET c = 'this is a close brace = }' WHERE {parallel_filter}".format(parallel_filter="…")
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
ValueError: Single '}' encountered in format string

@robodoo
Copy link
Contributor

robodoo commented Sep 26, 2024

Pull request status dashboard

@Pirols Pirols force-pushed the master-remove_explode_query_range_format-pied branch from bf79fa6 to 96a474e Compare September 26, 2024 10:06
@Pirols Pirols requested review from a team, filisbits, aj-fuentes and KangOl September 26, 2024 10:10
@Pirols Pirols force-pushed the master-remove_explode_query_range_format-pied branch 2 times, most recently from 210ef0a to e800de2 Compare September 26, 2024 11:03
src/util/pg.py Outdated Show resolved Hide resolved
@Pirols Pirols force-pushed the master-remove_explode_query_range_format-pied branch 2 times, most recently from 2b341f2 to fd3ad98 Compare September 26, 2024 11:18
src/util/pg.py Outdated Show resolved Hide resolved
@Pirols Pirols force-pushed the master-remove_explode_query_range_format-pied branch 4 times, most recently from 1e35f62 to 8bbcc71 Compare September 26, 2024 15:02
… formatting

The usage of `str.format` to inject the parallel filter used to explode queries is not
robust to the presence of other curly braces. Examples:

1. `JSON` strings (typically to leverage their mapping capabilities):

see 79f3d71, where a query had to be modified to
accomodate that.

2. Hardcoded sets of curly braces:

```python
>>> "UPDATE t SET c = '{usage as literal characters}' WHERE {parallel_filter}".format(parallel_filter="…")
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
KeyError: 'usage as literal characters'
```

Which can be (unelegantly) solved adding even more braces, leveraging one side-effect of
`.format`:

```python
>>> "UPDATE t SET c = '{{usage as literal characters}}' WHERE {parallel_filter}".format(parallel_filter="…")
"UPDATE t SET c = '{usage as literal characters}' WHERE …"
```

3. Hardcoded curly braces (AFAICT no way to solve this):

```python
>>> "UPDATE t SET c = 'this is an open curly brace = {' WHERE {parallel_filter}".format(parallel_filter="…")
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
ValueError: unexpected '{' in field name
```

```python
>>> "UPDATE t SET c = 'this is a close brace = }' WHERE {parallel_filter}".format(parallel_filter="…")
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
ValueError: Single '}' encountered in format string
```
@Pirols Pirols force-pushed the master-remove_explode_query_range_format-pied branch from 8bbcc71 to 6299ef9 Compare September 26, 2024 15:03
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.

nits

src/util/pg.py Outdated Show resolved Hide resolved
src/util/pg.py Outdated Show resolved Hide resolved
@Pirols Pirols force-pushed the master-remove_explode_query_range_format-pied branch from 6299ef9 to faf43c0 Compare September 27, 2024 09:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants