Generate valid MSSQL upserts for multi-table persistence by fubhy · Pull Request #7005 · Effect-TS/effect

fubhy · 2026-08-04T18:25:15Z

Summary

Multi-table writes configured for MSSQL generate unsupported ON CONFLICT syntax and therefore cannot execute on MSSQL.

Important

This PR starts with focused failing reproduction tests. Add the implementation fix to this same branch; CI is expected to fail until that fix is included.

MSSQL multi-table writes use SQLite upsert syntax

Module: Persistence
Audit ID: unstable-state-p-5
Severity / confidence: high / high

What happens

Multi-table writes configured for MSSQL generate unsupported ON CONFLICT syntax and therefore cannot execute on MSSQL.

Why it happens

The upsert dialect switch handles PostgreSQL and MySQL explicitly, then sends every other dialect through a SQLite ON CONFLICT(id) fallback. MSSQL requires different syntax.

Expected behavior

layerBackingSqlMultiTable supports the MSSQL dialect selected elsewhere in the same layer.

Relevant implementation

These links and excerpts are pinned to audit base c9b56ab507f224426ee8388dc450da447ec4715f.

packages/effect/src/unstable/persistence/Persistence.ts:354-371

View problematic code at packages/effect/src/unstable/persistence/Persistence.ts:354-371

      const upsert = sql.onDialectOrElse({
        pg: (): UpsertFn => (entries) =>
          sql`
            INSERT INTO ${table} ${sql.insert(entries)}
            ON CONFLICT (id) DO UPDATE SET value=EXCLUDED.value, expires=EXCLUDED.expires
          `.unprepared,
        mysql: (): UpsertFn => (entries) =>
          sql`
            INSERT INTO ${table} ${sql.insert(entries)}
            ON DUPLICATE KEY UPDATE value=VALUES(value), expires=VALUES(expires)
          `.unprepared,
        // sqlite
        orElse: (): UpsertFn => (entries) =>
          sql`
            INSERT INTO ${table} ${sql.insert(entries)}
            ON CONFLICT(id) DO UPDATE SET value=excluded.value, expires=excluded.expires
          `.unprepared
      })

View exact lines on GitHub

Reproduction

pnpm test --run packages/effect/test/unstable/persistence/KeyValueStore.test.ts

Observed failure: The MSSQL-selected generated statement contained ON CONFLICT and no MERGE.

Implementation handoff

The initial reproduction tests on this branch are the regression specification for the implementation fix that should follow in this PR.

Start with the pinned implementation excerpts and the Why it happens analysis above.
Change the implementation so it satisfies the stated Expected behavior; do not weaken or remove the reproduction assertions.
Run the focused reproduction command(s) and confirm the observed failures become passing tests:

pnpm test --run packages/effect/test/unstable/persistence/KeyValueStore.test.ts

Run the affected package's existing tests, then the repository lint and type checks before requesting review.

Audit provenance

Audit base: c9b56ab507f224426ee8388dc450da447ec4715f
Reproduction base: c9b56ab507f224426ee8388dc450da447ec4715f
Findings: unstable-state-p-5
Initial patch: focused reproduction tests; implementation fix pending

Closes EFF-442

changeset-bot · 2026-08-04T18:25:20Z

🦋 Changeset detected

Latest commit: 197c4b9

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 30 packages

Name	Type
effect	Patch
@effect/opentelemetry	Patch
@effect/platform-browser	Patch
@effect/platform-bun	Patch
@effect/platform-deno	Patch
@effect/platform-node-shared	Patch
@effect/platform-node	Patch
@effect/vitest	Patch
@effect/ai-anthropic	Patch
@effect/ai-openai-compat	Patch
@effect/ai-openai	Patch
@effect/ai-openrouter	Patch
@effect/atom-react	Patch
@effect/atom-solid	Patch
@effect/atom-vue	Patch
@effect/sql-clickhouse	Patch
@effect/sql-d1	Patch
@effect/sql-libsql	Patch
@effect/sql-mssql	Patch
@effect/sql-mysql2	Patch
@effect/sql-pg	Patch
@effect/sql-pglite	Patch
@effect/sql-sqlite-bun	Patch
@effect/sql-sqlite-do	Patch
@effect/sql-sqlite-node	Patch
@effect/sql-sqlite-react-native	Patch
@effect/sql-sqlite-wasm	Patch
@effect/docgen	Patch
@effect/doctest	Patch
@effect/openapi-generator	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

pullfrog

Important

This PR currently adds the regression test but not the implementation fix that the title promises. The focused test fails on the branch because layerBackingSqlMultiTable still emits SQLite ON CONFLICT syntax for the MSSQL dialect.

Reviewed changes

This run reviewed the single commit on this branch.

Added regression test: packages/effect/test/unstable/persistence/KeyValueStore.test.ts adds a mocked SqlClient test that asserts the MSSQL-generated upsert contains MERGE and excludes ON CONFLICT.
Missing implementation fix: packages/effect/src/unstable/persistence/Persistence.ts was not modified in this diff, so layerBackingSqlMultiTable still has no mssql branch in its upsert dialect switch.

Note: 1 inline comment(s) dropped because they did not anchor to lines inside the PR diff:

packages/effect/src/unstable/persistence/Persistence.ts:366 (RIGHT) — file not in PR diff

^{｜ ⚠️ this action is pinned to a commit SHA, which freezes the cleanup step — switch to @v0 or keep the SHA fresh with Dependabot ｜ Fix it ➔ ｜ View workflow run ｜ Using Kimi K2 (free via Pullfrog for OSS) ｜ 𝕏}

tim-smart · 2026-08-04T23:12:29Z

@@ -88,6 +89,36 @@ export const testLayer = <E>(layer: Layer.Layer<KeyValueStore.KeyValueStore, E>)

 describe("KeyValueStore / layerMemory", () => testLayer(KeyValueStore.layerMemory))

+describe("Persistence / layerBackingSqlMultiTable", () => {


Remove this test and enable the intergration tests for mssql

pullfrog

ℹ️ No critical issues — one coverage suggestion inline.

Reviewed changes

This run reviewed the two commits pushed since the prior Pullfrog review (569faf3).

Implemented the MSSQL upsert branch in packages/effect/src/unstable/persistence/Persistence.ts: multi-table persistence now emits a MERGE ... USING (...) AS source statement per entry, matching the existing single-table backend.
Replaced the mocked regression test with real integration tests: added packages/sql/mssql/test/Persistence.integration.test.ts and a shared MssqlContainer helper, and added the MSSQL image to the CI pre-pull step.
Updated changeset for effect.

^{｜ ⚠️ this action is pinned to a commit SHA, which freezes the cleanup step — switch to @v0 or keep the SHA fresh with Dependabot ｜ Fix all ➔ ｜ Fix 👍s ➔ ｜ View workflow run ｜ Using Kimi K2 (free via Pullfrog for OSS) ｜ 𝕏}

pullfrog

ℹ️ No critical issues — the table-existence fix looks correct. The prior suggestion to retain a lightweight unit regression test remains open.

Reviewed changes

This run reviewed the one commit pushed since the prior Pullfrog review (68b8dbb35).

Fixed MSSQL persistence table-existence checks in packages/effect/src/unstable/persistence/Persistence.ts: the multi-table backend now builds the table name as a plain string and compares sysobjects.name = ${tableName}, and the single-table backend compares against ${"effect_persistence"}. Both replace the previous ${table} identifier fragment, which generated invalid T-SQL in the WHERE predicate.

^{｜ ⚠️ this action is pinned to a commit SHA, which freezes the cleanup step — switch to @v0 or keep the SHA fresh with Dependabot ｜ Fix it ➔ ｜ View workflow run ｜ Using Kimi K2 (free via Pullfrog for OSS) ｜ 𝕏}

github-actions · 2026-08-05T00:20:58Z

Bundle Size Analysis

Generated from PR build output; treat the content below as untrusted.

File Name	Current Size	Previous Size	Difference
`basic.ts`	7.06 KB	7.06 KB	0.00 KB (0.00%)
`batching.ts`	9.86 KB	9.86 KB	0.00 KB (0.00%)
`brand.ts`	6.34 KB	6.34 KB	0.00 KB (0.00%)
`cache.ts`	10.71 KB	10.71 KB	0.00 KB (0.00%)
`config.ts`	20.60 KB	20.60 KB	0.00 KB (0.00%)
`differ.ts`	20.20 KB	20.20 KB	0.00 KB (0.00%)
`http-client.ts`	21.54 KB	21.53 KB	+0.01 KB (+0.05%)
`logger.ts`	10.84 KB	10.84 KB	0.00 KB (0.00%)
`metric.ts`	8.98 KB	8.98 KB	0.00 KB (0.00%)
`optic.ts`	7.18 KB	7.18 KB	0.00 KB (0.00%)
`pubsub.ts`	14.99 KB	14.99 KB	0.00 KB (0.00%)
`queue.ts`	11.66 KB	11.66 KB	0.00 KB (0.00%)
`schedule.ts`	10.83 KB	10.83 KB	0.00 KB (0.00%)
`schema-class.ts`	19.14 KB	19.14 KB	0.00 KB (0.00%)
`schema-fromJsonSchemaDocument.ts`	28.96 KB	28.96 KB	0.00 KB (0.00%)
`schema-representation-roundtrip.ts`	25.29 KB	25.29 KB	0.00 KB (0.00%)
`schema-string-transformation.ts`	13.38 KB	13.38 KB	0.00 KB (0.00%)
`schema-string.ts`	10.94 KB	10.94 KB	0.00 KB (0.00%)
`schema-template-literal.ts`	15.17 KB	15.17 KB	0.00 KB (0.00%)
`schema-toArbitraryLazy.ts`	21.94 KB	21.94 KB	0.00 KB (0.00%)
`schema-toCodeDocument.ts`	24.34 KB	24.34 KB	0.00 KB (0.00%)
`schema-toCodecJson.ts`	19.18 KB	19.18 KB	0.00 KB (0.00%)
`schema-toEquivalence.ts`	19.01 KB	19.01 KB	0.00 KB (0.00%)
`schema-toFormatter.ts`	18.87 KB	18.87 KB	0.00 KB (0.00%)
`schema-toJsonSchemaDocument.ts`	22.60 KB	22.60 KB	0.00 KB (0.00%)
`schema-toRepresentation.ts`	19.52 KB	19.52 KB	0.00 KB (0.00%)
`schema.ts`	18.41 KB	18.41 KB	0.00 KB (0.00%)
`stm.ts`	12.63 KB	12.63 KB	0.00 KB (0.00%)
`stream.ts`	9.80 KB	9.80 KB	0.00 KB (0.00%)

Generate valid MSSQL upserts for multi-table persistence

569faf3

fubhy added the audit Findings originating from the Effect runtime correctness audit label Aug 4, 2026

effect-slopcop Bot added 4.0 bug Something isn't working labels Aug 4, 2026

pullfrog Bot reviewed Aug 4, 2026

View reviewed changes

Fix MSSQL multi-table persistence upserts

2188b7e

tim-smart reviewed Aug 4, 2026

View reviewed changes

Enable MSSQL persistence integration tests

68b8dbb

tim-smart enabled auto-merge (squash) August 4, 2026 23:29

pullfrog Bot reviewed Aug 4, 2026

View reviewed changes

Comment thread packages/sql/mssql/test/Persistence.integration.test.ts

Fix MSSQL persistence table existence checks

197c4b9

pullfrog Bot reviewed Aug 4, 2026

View reviewed changes

tim-smart merged commit ae4cf7b into main Aug 5, 2026
18 of 20 checks passed

tim-smart deleted the audit/repro-unstable-persistence-persistence-mssql-multitable-upsert branch August 5, 2026 00:07

Navigation Menu

Uh oh!

Uh oh!

Generate valid MSSQL upserts for multi-table persistence - #7005

Generate valid MSSQL upserts for multi-table persistence#7005
tim-smart merged 4 commits into
mainfrom
audit/repro-unstable-persistence-persistence-mssql-multitable-upsert

fubhy commented Aug 4, 2026 •

edited by tim-smart

Loading

Uh oh!

changeset-bot Bot commented Aug 4, 2026 •

edited

Loading

Uh oh!

pullfrog Bot left a comment

Uh oh!

tim-smart Aug 4, 2026

Uh oh!

pullfrog Bot left a comment

Uh oh!

Uh oh!

pullfrog Bot left a comment

Uh oh!

Uh oh!

github-actions Bot commented Aug 5, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		@@ -88,6 +89,36 @@ export const testLayer = <E>(layer: Layer.Layer<KeyValueStore.KeyValueStore, E>)

		describe("KeyValueStore / layerMemory", () => testLayer(KeyValueStore.layerMemory))

		describe("Persistence / layerBackingSqlMultiTable", () => {

Uh oh!

Uh oh!

Conversation

fubhy commented Aug 4, 2026 • edited by tim-smart Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

MSSQL multi-table writes use SQLite upsert syntax

What happens

Why it happens

Expected behavior

Relevant implementation

Reproduction

Implementation handoff

Audit provenance

Uh oh!

changeset-bot Bot commented Aug 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🦋 Changeset detected

Uh oh!

pullfrog Bot left a comment

Choose a reason for hiding this comment

Uh oh!

tim-smart Aug 4, 2026

Choose a reason for hiding this comment

Uh oh!

pullfrog Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

pullfrog Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

github-actions Bot commented Aug 5, 2026

Bundle Size Analysis

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

fubhy commented Aug 4, 2026 •

edited by tim-smart

Loading

changeset-bot Bot commented Aug 4, 2026 •

edited

Loading