Speed up postgrest JSON validation#1487
Conversation
AmaLS367
left a comment
There was a problem hiding this comment.
Thanks for this PR — the performance improvement is impressive and the change is clean. The core fix looks correct.
One suggestion on the test test_json_adapter_uses_pydantic_json_value_schema:
assert JSONAdapter.core_schema["schema"]["schema_ref"].startswith(
"pydantic.types.JsonValue:"
)The intent is good (guarding the fast path), but the nested ["schema"]["schema_ref"] path and the hardcoded "pydantic.types.JsonValue:" string tie the test to Pydantic's internal schema structure. A more resilient alternative would be to compare against a reference adapter:
def test_json_adapter_schema_matches_json_value():
from pydantic.types import JsonValue as PydanticJsonValue
from pydantic import TypeAdapter
assert JSONAdapter.core_schema == TypeAdapter(PydanticJsonValue).core_schemaThis guards the same regression (someone reverting to the slow TypeAliasType path) without hardcoding internal schema details. Just a suggestion though — not a blocker.
|
Thanks for the suggestion. I updated the test to compare Verified locally:
|
Fixes #1486
Summary
postgrest.types.JSONAdapterto Pydantic's optimizedJsonValueschemaJSONtype alias unchanged for request typingJsonValueadapter path and nested JSON byte validationLocal validation
JSONAdapter.validate_json(payload)averaged44.78 ms;TypeAdapter(JsonValue)averaged3.93 msJSONAdapter.validate_json(payload)averaged3.56 ms;TypeAdapter(JsonValue)averaged3.74 msuv run --project src/postgrest pytest src/postgrest/tests/test_types.py -quv run --project src/postgrest pytest src/postgrest/tests/test_types.py src/postgrest/tests/test_utils.py src/postgrest/tests/_sync/test_request_builder.py src/postgrest/tests/_async/test_request_builder.py -quv run --project src/postgrest ruff check src/postgrest/src/postgrest/types.py src/postgrest/tests/test_types.pyNotes
src/postgrest/testssuite locally, but it did not finish before the 4-minute timeout in my Windows environment. The targeted request-builder/type tests above completed successfully.uv run --lockedcurrently reports that the existing workspace lockfile needs an update, so the PR intentionally does not include lockfile changes.