feat(postgrest): allow select() chaining on write builders#1383
feat(postgrest): allow select() chaining on write builders#1383Dodothereal wants to merge 1 commit intosupabase:mainfrom
Conversation
|
@olirice @silentworks could you please review this enhancement for #1271 when you have time? Thanks. |
📝 WalkthroughWalkthroughThe PR extends the fluent builder pattern in both sync and async request builders, adding a Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/postgrest/src/postgrest/_async/request_builder.py`:
- Around line 36-46: In select (the async QueryBuilder.select implementation)
the code currently appends "return=representation" to the Prefer header which
can leave an existing "return=minimal" first and be ignored by PostgREST; change
the logic to parse self.request.headers.get("Prefer") and if any existing
"return=" directive exists replace that fragment with "return=representation"
(preserving other comma-separated preferences and whitespace), otherwise add
"return=representation" as before; apply the same change to the
SyncQueryRequestBuilder.select implementation so both _async/request_builder.py
and _sync/request_builder.py consistently replace any existing "return="
preference instead of appending.
| def select(self: QueryBuilderT, *columns: str) -> QueryBuilderT: | ||
| _, params, _, _ = pre_select(*columns, count=None) | ||
| self.request.params = self.request.params.add("select", params["select"]) | ||
| prefer_header = self.request.headers.get("Prefer") | ||
| if not prefer_header: | ||
| self.request.headers["Prefer"] = "return=representation" | ||
| elif "return=representation" not in [ | ||
| value.strip() for value in prefer_header.split(",") | ||
| ]: | ||
| self.request.headers["Prefer"] = f"{prefer_header},return=representation" | ||
| return self |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
PostgREST Prefer header precedence return=minimal return=representation multiple directives
💡 Result:
In PostgREST, Prefer follows RFC 7240. PostgREST supports Prefer: return=minimal (default for writes) and Prefer: return=representation (return the mutated rows). [1][2]
If you send multiple return directives in the same request, e.g.
Prefer: return=minimal, return=representation(or across multiplePrefer:header lines)
then only the first occurrence is considered and later duplicates should be ignored, per RFC 7240 (“If any preference is specified more than once, only the first instance is to be considered.”). So the leftmost return=... wins. [2]
Also, RFC 7240 notes return=minimal and return=representation are mutually exclusive, so including both is generally a client bug. [2]
Sources: [1] PostgREST Prefer header docs, [2] RFC 7240.
🌐 Web query:
PostgREST RFC 7240 Prefer header handling duplicate directives
💡 Result:
RFC 7240 defines two relevant rules for Prefer:
- Multiple
Preferheader fields are allowed and are equivalent to a single header whose value is the comma-separated concatenation of all preference tokens. If an HTTP client/library “merges” multiplePreferheaders into one comma-separated header, that’s still compliant. [1] - Duplicate preference tokens should not be sent, but if they are, only the first occurrence is considered; later duplicates should be ignored without error (ordering becomes significant only in this duplicate case). [1]
PostgREST follows this RFC 7240 behavior when parsing request headers: if a preference is set more than once (even across multiple Prefer headers), PostgREST uses the first value and ignores subsequent duplicates. Example from PostgREST’s own fromHeaders docs: Prefer: tx=commit, tx=rollback results in tx=commit being used. [2]
Separately, PostgREST’s Prefer: handling=strict is about invalid/unrecognized/unfulfillable preferences (error vs ignore), not about duplicates—duplicates are simply handled as “first wins” per RFC 7240. [3]
Sources: RFC 7240 Prefer header specification [1]; PostgREST preference parsing (fromHeaders) behavior [2]; PostgREST Prefer header docs / handling=strict [3].
[1] https://www.rfc-editor.org/rfc/rfc7240
[2] https://hackage.haskell.org/package/postgrest/docs/PostgREST-Request-Preferences.html
[3] https://docs.postgrest.org/en/v12/references/api/preferences.html
🏁 Script executed:
# Search for SyncQueryRequestBuilder.select to verify if logic is mirrored
fd -type f -name "*.py" | xargs rg -l "SyncQueryRequestBuilder" | head -5Repository: supabase/supabase-py
Length of output: 444
🏁 Script executed:
# Find and examine the SyncQueryRequestBuilder.select method
rg -A 15 "class SyncQueryRequestBuilder" --type pyRepository: supabase/supabase-py
Length of output: 1614
Replace return= directives in Prefer header instead of appending to avoid return=minimal overriding the intended behavior.
PostgREST follows RFC 7240, which specifies that when a preference appears multiple times, only the first occurrence is honored. If Prefer already contains return=minimal and select() appends ,return=representation, the header becomes return=minimal,return=representation—and PostgREST will use return=minimal (the first), not the intended return=representation. This breaks embedding behavior.
Replace any existing return= preference instead of appending to ensure return=representation is always used:
Required fix
- prefer_header = self.request.headers.get("Prefer")
- if not prefer_header:
- self.request.headers["Prefer"] = "return=representation"
- elif "return=representation" not in [
- value.strip() for value in prefer_header.split(",")
- ]:
- self.request.headers["Prefer"] = f"{prefer_header},return=representation"
+ prefer_header = self.request.headers.get("Prefer")
+ if not prefer_header:
+ self.request.headers["Prefer"] = "return=representation"
+ else:
+ values = [v.strip() for v in prefer_header.split(",") if v.strip()]
+ values = [v for v in values if not v.lower().startswith("return=")]
+ values.append("return=representation")
+ self.request.headers["Prefer"] = ",".join(values)Apply the same fix to SyncQueryRequestBuilder.select in src/postgrest/src/postgrest/_sync/request_builder.py to keep them synchronized.
🤖 Prompt for AI Agents
In `@src/postgrest/src/postgrest/_async/request_builder.py` around lines 36 - 46,
In select (the async QueryBuilder.select implementation) the code currently
appends "return=representation" to the Prefer header which can leave an existing
"return=minimal" first and be ignored by PostgREST; change the logic to parse
self.request.headers.get("Prefer") and if any existing "return=" directive
exists replace that fragment with "return=representation" (preserving other
comma-separated preferences and whitespace), otherwise add
"return=representation" as before; apply the same change to the
SyncQueryRequestBuilder.select implementation so both _async/request_builder.py
and _sync/request_builder.py consistently replace any existing "return="
preference instead of appending.
Summary
select(*columns)to sync/async query builders used by write operationsinsert(),upsert(),update(), anddelete()select()is used on write builders, ensurePrefer: return=representationis present without duplicating itreturn=minimaloverride behaviorWhy
Issue #1271 requests support for resource embedding on write operations via the
selectquery parameter. This change adds first-class builder support without requiring manual mutation of internal params.Testing
uv run --package postgrest pytest tests/_sync/test_request_builder.py tests/_async/test_request_builder.py(run insrc/postgrest)make postgrest.mypyuv run ruff check src/postgrest/src/postgrest/_sync/request_builder.py src/postgrest/src/postgrest/_async/request_builder.py src/postgrest/tests/_sync/test_request_builder.py src/postgrest/tests/_async/test_request_builder.pyCloses #1271
Summary by CodeRabbit
New Features
select()method to query builders, enabling users to specify which columns to retrieve from database operations.Tests