cockroachdb / cockroach

CockroachDB — the cloud native, distributed SQL database designed for high availability, effortless scale, and control over data placement.
https://www.cockroachlabs.com
Other
30.1k stars 3.81k forks source link

"could not determine data type of placeholder" with string arguments and psycopg3 #91396

Open timgraham opened 2 years ago

timgraham commented 2 years ago

Describe the problem

Running Django's tests with CockroachDB and psycopg3, there are some "could not determine data type of placeholder" errors with string arguments.

To Reproduce

The simplest reproducer seems to be:

import psycopg
conn = psycopg.connect(...)
cursor = conn.cursor()
cursor.execute('SELECT %s', ['1'])

It works with PostgreSQL w/psycopg and PostgreSQL & CockroachDB w/psycopg2, but not with CockroachDB & psycopg.

Expected behavior

Is it a bug or a limitation we have to try to work around, at least temporarily? @rafiss says, "not sure why that would be new with psycopg3. the underlying problem is https://github.com/cockroachdb/cockroach/issues/41558"

Environment:

Additional context

See https://github.com/django/django/pull/15687#issuecomment-1294303652 for an example from Django's test suite.

Jira issue: CRDB-21245

blathers-crl[bot] commented 2 years ago

Hello, I am Blathers. I am here to help you get the issue triaged.

Hoot - a bug! Though bugs are the bane of my existence, rest assured the wretched thing will get the best of care here.

I have CC'd a few people who may be able to assist you:

If we have not gotten back to your issue within a few business days, you can try the following:

:owl: Hoot! I am a Blathers, a bot for CockroachDB. My owner is otan.

rafiss commented 2 years ago

this seems like a duplicate of https://github.com/cockroachdb/cockroach/issues/41558

timgraham commented 1 year ago

Quite a few of the failures in Django's test suite involve string literals in CASE expressions, e.g.

SELECT ...
       CASE
           WHEN "expressions_case_casetestmodel"."integer" = %s THEN %s
           WHEN "expressions_case_casetestmodel"."integer" = %s THEN %s
           ELSE 'other'
       END AS "test"
FROM ...
args=(1, 'one',  2,  'two', 'other');
...
Error: could not determine data type of placeholder $2

Again, the same query works with psycopg2. If this isn't something that can easily be fixed in CockroachDB, is there a suggested workaround on the client side?

rafiss commented 1 year ago

The suggested client side workaround is to add a type hint or a SQL-level cast to the placeholder. for a CASE expression, it should be sufficient to add the hint/cast to just one of the placeholders.

jordanlewis commented 1 year ago

Is psycopg3 no longer sending type hints with its prepared statements? That seems like the most likely issue here that would distinguish 3 from 2.

timgraham commented 1 year ago

Right. See https://github.com/django/django/pull/15687#discussion_r1027371722 for some relevent discussion. The line mentioned there (ctx.register_dumper(str, StrDumper)) was removed before that PR was merged, but adding it back fixes the affected queries on CockroachDB. It seems like there's a few cases where PostgreSQL doesn't require the type hint for string arguments but they don't work on CockroachDB.

jordanlewis commented 1 year ago

Ugh, that's a troublesome problem - do you have insight into whether that decision is final? It would be better to keep sending type hints - even with Postgres, it can lead to undesirable results.

Playing with the type system is very delicate, we may be able to fix this (and we should given our compatibility goals) but it may be challenging.

timgraham commented 1 year ago

I don't have a complete understanding of the issue, but Django compatibility with psycopg3 & PostgreSQL seems to be working well, so I'm not sure what decision would be revisited.

As part of psycopg3 compatibility, Django added some casts to fix "could not determine data type of placeholder" errors on PostgreSQL, but there are some remaining cases where CockroachDB doesn't behave like PostgreSQL.

The affected queries work with client-side-binding cursors (which Django defaults to) but not server-side cursors (which I was originally testing with when I created this issue). (Incidentally, there are also some queries that work with server-side but not client-side binding, e.g. https://github.com/cockroachdb/django-cockroachdb/issues/21).