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

Fix Postgres array syntax for UPDATE #644

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

raxod502
Copy link

This PR should address #579 by formatting UPDATE queries using the same rules as INSERT queries. Regression tests for the new case have been added, courtesy of @vfonte91.

A side effect of this fix is that I discovered PyPika has inconsistent handling of null versus NULL. In these queries, null is used:

SELECT * FROM "abc" WHERE "foo"=null
UPDATE "abc" SET "foo"=null

But in these queries, NULL is used:

INSERT INTO "abc" VALUES (NULL)
INSERT INTO "abc" VALUES (1) RETURNING NULL
SELECT NULL "abcdef"

This is because there are two different code paths for formatting null values:

return "null"

super().__init__("NULL", alias)

Both of these code paths used to return lowercase null circa 2018; this was changed in a seemingly unrelated commit 3bfee98.

Some other keywords (true, false) seem to be lowercase by convention in PyPika, whereas others (SYSTEM_TIME) seem to be uppercase by convention, so it's not entirely clear to me what the expected standardization is. On the plus side, since SQL is fully case-insensitive, it doesn't really matter too much.

In this PR, I've updated all code paths and tests to use lowercase null. Let me know if that's wrong.

@levintrix
Copy link

would love this to be merged :(

@foxx
Copy link

foxx commented Mar 23, 2022

@ Maintainers - any chance of getting this merged?

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