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

Deserialization does not respect optional fields for special types for raw queries #998

Closed
AstreaTSS opened this issue Aug 8, 2024 · 1 comment
Labels
bug/2-confirmed We have confirmed that this is a bug. kind/bug A reported bug. topic: client Related to the Client API

Comments

@AstreaTSS
Copy link
Contributor

AstreaTSS commented Aug 8, 2024

Bug description

If a field can be null/None and is specially handled with the DESERIALIZERS variable found in prisma/_raw_query (ie BigInt, Decimal, Json), then any raw queries which has said field and is null will error out.

This only happens on 0.14.0 and above, as raw queries were reworked for that version.

How to reproduce

  1. Define a schema with an optional BigInt/Decimal/Json value:
model PrismaBugExample {
  id          Int      @id @default(autoincrement())
  bad_var     BigInt?
}
  1. Set up the tables and your Python program, and create an entry with a null/None value for bad_var:
await PrismaBugExample.prisma().create(data={"bad_var": None})
  1. Make a raw query getting that entry:
data = await PrismaBugExample.prisma().query_first("SELECT * FROM PrismaBugExample")

The bug should occur at this step.

Expected behavior

For raw queries including these nullable types to work.

Prisma information

Schema: https://github.com/AstreaTSS/PYTHIA/blob/8873fd94c770c9faa503c70201c01af3a0390943/schema.prisma
Models (there is some post-generation adjustments that I do to them - it has no effect on this bug but I might as well include everything): https://github.com/AstreaTSS/PYTHIA/blob/8873fd94c770c9faa503c70201c01af3a0390943/common/models.py
Query:

Environment & setup

  • OS: Windows - WSL, Ubuntu
  • Database: PostgreSQL
  • Python version: 3.12.3
  • Prisma version:
prisma                  : 5.17.0
prisma client python    : 0.14.0
platform                : debian-openssl-1.1.x
expected engine version : 393aa359c9ad4a4bb28630fb5613f9c281cde053
installed extras        : []
install path            : /home/astrea/Documents/PYTHIA/env/lib/python3.12/site-packages/prisma
binary cache dir        : /home/astrea/.cache/prisma-python/binaries/5.17.0/393aa359c9ad4a4bb28630fb5613f9c281cde053

Internal cause

Doing some investigation on my own, I found the culprit of the bug - _deserialize_prisma_object in src/prisma/_raw_query.py:

def _deserialize_prisma_object(
fields: list[object],
*,
result: RawQueryResult,
for_model: bool,
model: type[BaseModelT] | None = None,
) -> BaseModelT | dict[str, Any]:
# create a local reference to avoid performance penalty of global
# lookups on some python versions
_deserializers = DESERIALIZERS
new_obj: dict[str, Any] = {}
for i, field in enumerate(fields):
key = result.columns[i]
prisma_type = result.types[i]
if prisma_type.endswith('-array'):
if field is None:
# array fields can stil be `None`
new_obj[key] = None
continue
if not isinstance(field, list):
raise TypeError(
f'Expected array data for {key} column with internal type {prisma_type}',
)
item_type, _ = prisma_type.split('-')
new_obj[key] = [
_deserializers[item_type](value, for_model)
#
if item_type in _deserializers
else value
for value in field
]
else:
value = field
new_obj[key] = _deserializers[prisma_type](value, for_model) if prisma_type in _deserializers else value
if model is not None:
return model_parse(model, new_obj)
return new_obj

More specifically, focusing in on the non-array handling (essentially a single line), we get this:

new_obj[key] = _deserializers[prisma_type](value, for_model) if prisma_type in _deserializers else value

From my understanding, what this does is:

  • Sees if the prisma_type (which I think is gathered from the model or from Prisma proper - point is, it's gotten independently of the actual query result) is in the DESERIALIZERS mentioned earlier (though aliased under _deserializers).
  • If it is, then it throws the gotten value directly into the deserializer.

Note that this makes no check to see if the value is, say, None. Interestingly enough, arrays do check for None, meaning it won't occur there.

Anyways, a potential fix is just to hoist that None check for arrays to above the conditional statement, making it run for all values regardless of type. I was unsure if that fixes the issue in every case, and I'm unsure if that's necessarily the way it should be fixed, hence this issue.

@RobertCraigie RobertCraigie added bug/1-repro-available A reproduction exists and needs to be confirmed. kind/bug A reported bug. topic: client Related to the Client API labels Aug 8, 2024
@RobertCraigie
Copy link
Owner

Thanks for the detailed bug report, I agree with your suggested fix. The None check should be hoisted.

@RobertCraigie RobertCraigie added bug/2-confirmed We have confirmed that this is a bug. and removed bug/1-repro-available A reproduction exists and needs to be confirmed. labels Aug 9, 2024
RobertCraigie added a commit that referenced this issue Aug 9, 2024
## Change Summary

See #998.

## Agreement

By submitting this pull request, I confirm that you can use, modify,
copy and redistribute this contribution, under the terms of your choice.

---------

Co-authored-by: Robert Craigie <robert@craigie.dev>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug/2-confirmed We have confirmed that this is a bug. kind/bug A reported bug. topic: client Related to the Client API
Projects
None yet
Development

No branches or pull requests

2 participants