Skip to content

feat(postgrest): add defaultToNull to insert and upsert - #1311

Draft
grdsdev wants to merge 2 commits into
mainfrom
guilhermesouza/sdk-1613-postgrest-no-defaulttonull-prefer-missingdefault-on-insert
Draft

grdsdev wants to merge 2 commits into
mainfrom
guilhermesouza/sdk-1613-postgrest-no-defaulttonull-prefer-missingdefault-on-insert

Conversation

@grdsdev

@grdsdev grdsdev commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Why

A bulk payload sends the union of every row's keys as the columns parameter, so a column that only some rows carry is written as null for the rows that omit it — overwriting its database DEFAULT. PostgREST's Prefer: missing=default asks for the default instead, and Swift had no first-class way to send it. supabase-js exposes this as defaultToNull.

defaultToNull: Bool = true on the legacy insert and upsert; false appends missing=default to Prefer.

Important

Draft — blocked on the spec PR that documents this capability: supabase/sdk#130 (awaiting CODEOWNERS). Opening as a draft so CI can answer the ABI question below while that review happens.

The ABI question this PR exists to answer

SDK-1613 flags that adding even a defaulted parameter to a public function can be reported as an API breakage: because the mangled name changes — and this is a shipped surface (the legacy builder, present in v2.55.1), so it matters.

I could not answer it locally. scripts/check-for-breaking-api-changes.sh reports ❌ Breaking API changes detected on a tree byte-identical to the baseline — all 8 modules fail to load with a libsecp256k1 module redefinition, so every baseline has zero symbols and the non-zero exit is a false positive (rm -rf .build first does not help). The gate is green in CI, so it is a local-environment problem, not a broken gate.

So api-stability.yml on this PR is the verdict. If it flags the parameter, the fix is to swap it for a new overload rather than carry a ! and a V3_MIGRATION.md entry for a purely additive option — say the word and I will reshape it.

Two deviations from the issue, both deliberate

1. The header is not gated on the payload shape. The issue suggested gating it on the existing [[String: Any]] array check "for exact parity". postgrest-js does the opposite — if (!defaultToNull) headers.append('Prefer', 'missing=default') runs unconditionally, and Array.isArray gates only the columns parameter. Gating would therefore break parity, so the header is sent for a single row too. Harmless there: with no columns parameter, omitted columns already take their default.

2. Scope is insert + upsert only. PostgREST applies missing to PATCH as well, but supabase-js exposes defaultToNull only on these two, so update is untouched.

Unplanned fallout worth a reviewer's eye

Adding the parameter renames both methods in DocC, which broke nine pre-existing symbol links across PostgrestRequestBuilder.swift, PostgrestClient.swift and Types.swift./scripts/test-docs.sh failed until each gained defaultToNull:. That is most of the non-test diff and the reason it touches four source files instead of one.

I also dropped a cross-link I had added from insert to upsert: I wrote it against the PostgrestQueryBuilder typealias path, which DocC does not resolve (the type is PostgrestRequestBuilder). Replaced with a prose mention — a hardcoded symbol path that must be re-edited on every signature change is not worth the link.

Test plan

New tests in PostgrestQueryBuilderTests, each asserting the exact full Prefer header via the existing curl inline snapshots (a contains check cannot catch a missing preference):

Test Prefer
insertDefaultToNullFalseSendsMissingDefault return=minimal,missing=default
insertDefaultToNullFalseComposesWithReturningAndCount return=minimal,count=estimated,missing=default
upsertDefaultToNullFalseKeepsEveryOtherPreference existing=value,resolution=merge-duplicates,return=minimal,count=estimated,missing=default
upsertDefaultToNullFalseComposesWithIgnoreDuplicates resolution=ignore-duplicates,return=representation,missing=default

The third is the one that matters most: it proves the new preference composes with a caller-supplied value, resolution=, return= and count= without dropping any — the failure mode that made #1308 (SDK-1626) a silent data bug.

"Option off" is covered by the pre-existing insert / upsert / upsertIgnoreDuplicates snapshots, which still assert their exact Prefer values with no missing=. That they pass unchanged is also the evidence for "existing call sites unchanged".

  • swift test — 1441 tests in 148 suites passed (12 known issues)
  • ./scripts/format.sh, ./scripts/spell-check.sh, ./scripts/test-docs.sh all clean
  • api-stability.yml — the open question; see above
  • sdk-compliance.yaml unchanged — it lists symbols without parameter labels (PostgrestRequestBuilder.insert), and no new symbol lands

Fixes SDK-1613

A bulk payload sends the union of every row's keys as `columns`, so a column
some rows omit was always written as `null` — overwriting its database DEFAULT.
`defaultToNull: false` sends `Prefer: missing=default` instead.

Sent whenever the option is false, for a single row or an array, matching
supabase-js: there the payload-shape check gates only the `columns` parameter.
The preference is appended to `Prefer` rather than replacing it, so it composes
with `resolution=`, `return=`, `count=` and any caller-supplied value.

Scope matches supabase-js: insert and upsert only. PostgREST applies `missing`
to PATCH too, but `update` is left alone.

Adding the parameter renames both symbols in DocC, so nine existing symbol
links across four files gained the new label.

Fixes SDK-1613
@coderabbitai

coderabbitai Bot commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

  • Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions

github-actions Bot commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

⚠️ Potential Breaking API Changes Detected

This PR appears to contain breaking API changes. Please review the changes below:

API Check Output

If this is intentional, please update your PR title or commit message to include:

  • ! after the type (e.g., feat!: remove deprecated method)
  • Or include BREAKING CHANGE: in the commit body

If this is a false positive, you can safely ignore this warning.

@grdsdev

grdsdev commented Sep 1, 2026

Copy link
Copy Markdown
Contributor Author

ABI verdict: not breaking ✅

api-stability passed on a9504453, the commit carrying the signature change:

completed  success  a9504453   api-stability.yml

So adding a defaulted parameter to a public function on a shipped surface does not trip swift package diagnose-api-breaking-changes, despite the mangled name changing. That was the open question in the description, and it resolves the clean way:

  • no ! on the title
  • no V3_MIGRATION.md entry
  • no need to swap the parameter for a new overload

Useful precedent for the next option added to these builders.

Remaining block-merge failure is expected and self-clearing — the workflow is "Block WIP/Draft Merges" and its log says ❌ This PR is blocked from merging: Draft PR. It goes green when this is marked ready for review.

Still a draft pending supabase/sdk#130 (the spec), which is the only thing gating this now.

@coveralls

Copy link
Copy Markdown

Coverage Report for CI Build 33515206474

Coverage increased (+0.2%) to 87.54%

Details

  • Coverage increased (+0.2%) from the base build.
  • Patch coverage: 6 of 6 lines across 1 file are fully covered (100%).
  • No coverage regressions found.

Uncovered Changes

No uncovered changes found.

Coverage Regressions

No coverage regressions found.


Coverage Stats

Coverage Status
Relevant Lines: 11003
Covered Lines: 9632
Line Coverage: 87.54%
Coverage Strength: 178.53 hits per line

💛 - Coveralls

@github-actions

github-actions Bot commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

⚠️ Capability matrix drift detected

The following capabilities are marked implemented in the matrix but could not be found in swift:

  • client.session_management.persist_session → expected symbol: AuthLocalStorage.defaultLocalStorage
  • functions.invocation.streaming_response → expected symbol: FunctionsClient._invokeWithStreamedResponse

The following capabilities are marked implemented in swift but have no registered symbols to verify:

  • auth.passkey.register_passkey (no symbols list — cannot confirm implementation exists)
  • auth.passkey.sign_in_with_passkey (no symbols list — cannot confirm implementation exists)
  • client.observability.trace_propagation (no symbols list — cannot confirm implementation exists)
  • database.using_modifiers.request_cancellation (no symbols list — cannot confirm implementation exists)
  • functions.invocation.request_cancellation (no symbols list — cannot confirm implementation exists)
  • storage.file_buckets.url_cache_nonce (no symbols list — cannot confirm implementation exists)

These may have been renamed, removed, or never registered. Please update the capability matrix.
See: https://github.com/supabase/sdk/blob/main/packages/capability-matrix/docs/capability-matrix.md

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants