Skip to content

Escape quotes and quote times in QueryString.__str__ - #1447

Merged
dantownsend merged 1 commit into
piccolo-orm:masterfrom
youdie006:check-constraint-string-quotes
Oct 28, 2026
Merged

dantownsend merged 1 commit into
piccolo-orm:masterfrom
youdie006:check-constraint-string-quotes

Conversation

@youdie006

@youdie006 youdie006 commented Oct 27, 2026 •

Copy link
Copy Markdown
Contributor

Check constraints get their DDL from QueryString.__str__ (via get_condition_str), which puts a string argument between single quotes without doubling the quotes inside it, and leaves a datetime.time unquoted because only datetime and date are handled:

class Band(Table):
    genre = Varchar()
    check_genre = Check(genre != "rock'n'roll")


class Shop(Table):
    opens = Time()
    check_opens = Check(opens >= datetime.time(9, 0))
ALTER TABLE "band" ADD CONSTRAINT check_genre CHECK ("genre" != 'rock'n'roll')
ALTER TABLE "shop" ADD CONSTRAINT check_opens CHECK ("opens" >= 09:00:00)

PostgreSQL's parser (checked with pglast / libpg_query) rejects both, so create_table() and the migration fail. With this change they become 'rock''n''roll' and '09:00:00', which parse. Strings without a quote and other types print as before; the only other visible change is that print(query) now shows '' for a quote inside a string.

Two tests are added to tests/query/test_querystring.py; they fail on master. The non-integration tests (sqlite) and scripts/lint.sh pass; the Postgres jobs need a server I didn't run.

Written with AI assistance (Claude); I have reviewed the change.

Check constraints build their DDL from QueryString.__str__, which put a
string argument between single quotes without doubling the quotes inside
it and left datetime.time values unquoted. A condition such as
genre != "rock'n'roll" or opens >= time(9, 0) produced DDL that
PostgreSQL rejects. Double embedded quotes and quote times like dates.
@dantownsend

Copy link
Copy Markdown
Member

I've tested this locally, and it works well. Thanks for the fix 👍

I realised we might also have problems with timedelta too, and we could do with more test coverage for check constraints. That can be in a future PR though.

@dantownsend
dantownsend merged commit fb9d18e into piccolo-orm:master Oct 28, 2026
41 checks passed
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.

2 participants