fix(orm): parenthesize an inlined computed field expression - #2796

Merged
ymc9 merged 1 commit into
zenstackhq:devfrom
evgenovalov:fix/parenthesize-inlined-computed-field
Aug 14, 2026
Merged

fix(orm): parenthesize an inlined computed field expression#2796
ymc9 merged 1 commit into
zenstackhq:devfrom
evgenovalov:fix/parenthesize-inlined-computed-field

Conversation

@evgenovalov

@evgenovalovevgenovalov commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

Fixes#2795

Motivation

A computed field is inlined into larger expressions — a boolean filter renders it as <expr> = $n. If the implementation's top-level node is a bare comparison, the operator precedence leaks into the surrounding query:

computedFields: {post: {isMine: (eb)=>eb('authorId','=',1),},},
awaitdb.post.findMany({where: {isMine: true}});
error: syntax error at or near "="
-- beforeselect"Post"."id"as"id", "authorId"= $1as"isMine"from"public"."Post"where"authorId"= $2= $3-- afterselect"Post"."id"as"id", ("authorId"= $1) as"isMine"from"public"."Post"where ("authorId"= $2) = $3

Postgres rejects the chained = outright. SQLite and MySQL parse it left-associatively as (a = $2) = $3, which happens to be the intended semantics, so the bug is invisible there — it surfaced only on the Postgres CI matrix of #2789.

Changes

  • BaseCrudDialect.fieldRef() wraps the implementation's expression in eb.parens() when inlining a computed field.
  • e2e test in computed-fields.test.ts (runs on every provider in the matrix): a bare-comparison field asserted through where: { isMine: true } / { isMine: false } / NOT, plus an eb.or-based field as a no-regression guard.

The wrap is unconditional because Kysely skips it for an expression that is already parenthesized, so eb.or / eb.and and subquery implementations compile byte-for-byte as before; only bare comparisons, unary operations and raw fragments change. Verified against the compiled SQL for each shape.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes
    • Improved filtering for computed fields that use comparisons or combined logical expressions.
    • Ensured computed-field expressions are grouped correctly when used in larger queries.
    • Preserved support for true, false, NOT, and OR filtering behavior.

@coderabbitai

coderabbitaiBot commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: e19e607e-15de-4b9b-9fc5-53141392c015

📥 Commits

Reviewing files that changed from the base of the PR and between c67c8a2 and 318018f.

📒 Files selected for processing (2)
  • packages/orm/src/client/crud/dialects/base-dialect.ts
  • tests/e2e/orm/client-api/computed-fields.test.ts
🚧 Files skipped from review as they are similar to previous changes (2)
  • packages/orm/src/client/crud/dialects/base-dialect.ts
  • tests/e2e/orm/client-api/computed-fields.test.ts

📝 Walkthrough

Walkthrough

The ORM now parenthesizes computed-field expressions before embedding them in larger SQL expressions. End-to-end tests cover direct, negated, and logical boolean filters while confirming computed-field reads.

Changes

Computed field filtering

Layer / File(s)Summary
Parenthesized computed expressions and boolean filter coverage
packages/orm/src/client/crud/dialects/base-dialect.ts, tests/e2e/orm/client-api/computed-fields.test.ts
fieldRef now wraps computed-field expressions in parentheses and continues forwarding query-time arguments. End-to-end tests cover direct, NOT, and OR boolean filters and computed-field reads.

Estimated code review effort: 2 (Simple) | ~10 minutes

Mergeability Score:⚪ Minimal · up to 31801

The PR makes a localized SQL-expression parenthesization change and adds coverage for the affected computed-field cases; no actionable merge-blocking risk remains beyond normal checks and review.

Possibly related PRs

  • zenstackhq/zenstack#2744: Both PRs modify BaseCrudDialect.fieldRef; this PR adds parenthesization while the related PR adds argument forwarding.
  • zenstackhq/zenstack#2762: This PR builds on related computed-field handling in fieldRef.
  • zenstackhq/zenstack#2789: Both PRs modify computed-field handling in base-dialect.ts; this PR addresses the SQL parenthesization issue.

Suggested reviewers:ymc9

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check nameStatusExplanation
Description Check✅ PassedCheck skipped - CodeRabbit’s high-level summary is enabled.
Title check✅ PassedThe title clearly describes the main fix: parenthesizing inlined computed-field expressions.
Linked Issues check✅ PassedThe implementation and tests satisfy issue #2795 by parenthesizing computed expressions and covering boolean, NOT, OR, and AND filtering.
Out of Scope Changes check✅ PassedAll changes directly support issue #2795 and add focused regression coverage without unrelated code changes.
Docstring Coverage✅ PassedNo functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

packages/orm/src/client/crud/dialects/base-dialect.ts

ESLint skipped: missing config or dependency (missing-dependency). The ESLint configuration references a package that is not available in the sandbox.


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.

A computed field is inlined into larger expressions — a boolean filter renders it as
`<expr> = $n` — so an implementation whose top-level node is a bare comparison produced
a chained `"authorId" = $2 = $3`. Postgres rejects that as a syntax error; sqlite and
mysql only parse it correctly by accident of left associativity.
Wrap the implementation's expression in parens so its operator precedence stays
contained. Kysely doesn't double-wrap an already parenthesized expression, so `eb.or`
/`eb.and`-based and subquery implementations compile unchanged.
Fixeszenstackhq#2795
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@evgenovalov
evgenovalovforce-pushed the fix/parenthesize-inlined-computed-field branch from c67c8a2 to 318018fCompareAugust 13, 2026 09:37
@ymc9
ymc9 merged commit 1390aa0 into zenstackhq:devAug 14, 2026
8 checks passed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Computed field expression is not parenthesized when inlined, breaking boolean filters on Postgres

2 participants

@evgenovalov@ymc9
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content

fix(orm): parenthesize an inlined computed field expression - #2796

Merged
ymc9 merged 1 commit into
zenstackhq:devfrom
evgenovalov:fix/parenthesize-inlined-computed-field
Aug 14, 2026
Merged

fix(orm): parenthesize an inlined computed field expression#2796
ymc9 merged 1 commit into
zenstackhq:devfrom
evgenovalov:fix/parenthesize-inlined-computed-field

Conversation

@evgenovalov

@evgenovalovevgenovalov commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

Fixes#2795

Motivation

A computed field is inlined into larger expressions — a boolean filter renders it as <expr> = $n. If the implementation's top-level node is a bare comparison, the operator precedence leaks into the surrounding query:

computedFields: {post: {isMine: (eb)=>eb('authorId','=',1),},},
awaitdb.post.findMany({where: {isMine: true}});
error: syntax error at or near "="
-- beforeselect"Post"."id"as"id", "authorId"= $1as"isMine"from"public"."Post"where"authorId"= $2= $3-- afterselect"Post"."id"as"id", ("authorId"= $1) as"isMine"from"public"."Post"where ("authorId"= $2) = $3

Postgres rejects the chained = outright. SQLite and MySQL parse it left-associatively as (a = $2) = $3, which happens to be the intended semantics, so the bug is invisible there — it surfaced only on the Postgres CI matrix of #2789.

Changes

  • BaseCrudDialect.fieldRef() wraps the implementation's expression in eb.parens() when inlining a computed field.
  • e2e test in computed-fields.test.ts (runs on every provider in the matrix): a bare-comparison field asserted through where: { isMine: true } / { isMine: false } / NOT, plus an eb.or-based field as a no-regression guard.

The wrap is unconditional because Kysely skips it for an expression that is already parenthesized, so eb.or / eb.and and subquery implementations compile byte-for-byte as before; only bare comparisons, unary operations and raw fragments change. Verified against the compiled SQL for each shape.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes
    • Improved filtering for computed fields that use comparisons or combined logical expressions.
    • Ensured computed-field expressions are grouped correctly when used in larger queries.
    • Preserved support for true, false, NOT, and OR filtering behavior.

@coderabbitai

coderabbitaiBot commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: e19e607e-15de-4b9b-9fc5-53141392c015

📥 Commits

Reviewing files that changed from the base of the PR and between c67c8a2 and 318018f.

📒 Files selected for processing (2)
  • packages/orm/src/client/crud/dialects/base-dialect.ts
  • tests/e2e/orm/client-api/computed-fields.test.ts
🚧 Files skipped from review as they are similar to previous changes (2)
  • packages/orm/src/client/crud/dialects/base-dialect.ts
  • tests/e2e/orm/client-api/computed-fields.test.ts

📝 Walkthrough

Walkthrough

The ORM now parenthesizes computed-field expressions before embedding them in larger SQL expressions. End-to-end tests cover direct, negated, and logical boolean filters while confirming computed-field reads.

Changes

Computed field filtering

Layer / File(s)Summary
Parenthesized computed expressions and boolean filter coverage
packages/orm/src/client/crud/dialects/base-dialect.ts, tests/e2e/orm/client-api/computed-fields.test.ts
fieldRef now wraps computed-field expressions in parentheses and continues forwarding query-time arguments. End-to-end tests cover direct, NOT, and OR boolean filters and computed-field reads.

Estimated code review effort: 2 (Simple) | ~10 minutes

Mergeability Score:⚪ Minimal · up to 31801

The PR makes a localized SQL-expression parenthesization change and adds coverage for the affected computed-field cases; no actionable merge-blocking risk remains beyond normal checks and review.

Possibly related PRs

  • zenstackhq/zenstack#2744: Both PRs modify BaseCrudDialect.fieldRef; this PR adds parenthesization while the related PR adds argument forwarding.
  • zenstackhq/zenstack#2762: This PR builds on related computed-field handling in fieldRef.
  • zenstackhq/zenstack#2789: Both PRs modify computed-field handling in base-dialect.ts; this PR addresses the SQL parenthesization issue.

Suggested reviewers:ymc9

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check nameStatusExplanation
Description Check✅ PassedCheck skipped - CodeRabbit’s high-level summary is enabled.
Title check✅ PassedThe title clearly describes the main fix: parenthesizing inlined computed-field expressions.
Linked Issues check✅ PassedThe implementation and tests satisfy issue #2795 by parenthesizing computed expressions and covering boolean, NOT, OR, and AND filtering.
Out of Scope Changes check✅ PassedAll changes directly support issue #2795 and add focused regression coverage without unrelated code changes.
Docstring Coverage✅ PassedNo functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

packages/orm/src/client/crud/dialects/base-dialect.ts

ESLint skipped: missing config or dependency (missing-dependency). The ESLint configuration references a package that is not available in the sandbox.


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.

A computed field is inlined into larger expressions — a boolean filter renders it as
`<expr> = $n` — so an implementation whose top-level node is a bare comparison produced
a chained `"authorId" = $2 = $3`. Postgres rejects that as a syntax error; sqlite and
mysql only parse it correctly by accident of left associativity.
Wrap the implementation's expression in parens so its operator precedence stays
contained. Kysely doesn't double-wrap an already parenthesized expression, so `eb.or`
/`eb.and`-based and subquery implementations compile unchanged.
Fixeszenstackhq#2795
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@evgenovalov
evgenovalovforce-pushed the fix/parenthesize-inlined-computed-field branch from c67c8a2 to 318018fCompareAugust 13, 2026 09:37
@ymc9
ymc9 merged commit 1390aa0 into zenstackhq:devAug 14, 2026
8 checks passed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Computed field expression is not parenthesized when inlined, breaking boolean filters on Postgres

2 participants

@evgenovalov@ymc9
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

fix(orm): parenthesize an inlined computed field expression - #2796

Merged
ymc9 merged 1 commit into
zenstackhq:devfrom
evgenovalov:fix/parenthesize-inlined-computed-field
Aug 14, 2026
Merged

fix(orm): parenthesize an inlined computed field expression#2796
ymc9 merged 1 commit into
zenstackhq:devfrom
evgenovalov:fix/parenthesize-inlined-computed-field

Conversation

@evgenovalov

@evgenovalovevgenovalov commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

Fixes#2795

Motivation

A computed field is inlined into larger expressions — a boolean filter renders it as <expr> = $n. If the implementation's top-level node is a bare comparison, the operator precedence leaks into the surrounding query:

computedFields: {post: {isMine: (eb)=>eb('authorId','=',1),},},
awaitdb.post.findMany({where: {isMine: true}});
error: syntax error at or near "="
-- beforeselect"Post"."id"as"id", "authorId"= $1as"isMine"from"public"."Post"where"authorId"= $2= $3-- afterselect"Post"."id"as"id", ("authorId"= $1) as"isMine"from"public"."Post"where ("authorId"= $2) = $3

Postgres rejects the chained = outright. SQLite and MySQL parse it left-associatively as (a = $2) = $3, which happens to be the intended semantics, so the bug is invisible there — it surfaced only on the Postgres CI matrix of #2789.

Changes

  • BaseCrudDialect.fieldRef() wraps the implementation's expression in eb.parens() when inlining a computed field.
  • e2e test in computed-fields.test.ts (runs on every provider in the matrix): a bare-comparison field asserted through where: { isMine: true } / { isMine: false } / NOT, plus an eb.or-based field as a no-regression guard.

The wrap is unconditional because Kysely skips it for an expression that is already parenthesized, so eb.or / eb.and and subquery implementations compile byte-for-byte as before; only bare comparisons, unary operations and raw fragments change. Verified against the compiled SQL for each shape.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes
    • Improved filtering for computed fields that use comparisons or combined logical expressions.
    • Ensured computed-field expressions are grouped correctly when used in larger queries.
    • Preserved support for true, false, NOT, and OR filtering behavior.

@coderabbitai

coderabbitaiBot commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: e19e607e-15de-4b9b-9fc5-53141392c015

📥 Commits

Reviewing files that changed from the base of the PR and between c67c8a2 and 318018f.

📒 Files selected for processing (2)
  • packages/orm/src/client/crud/dialects/base-dialect.ts
  • tests/e2e/orm/client-api/computed-fields.test.ts
🚧 Files skipped from review as they are similar to previous changes (2)
  • packages/orm/src/client/crud/dialects/base-dialect.ts
  • tests/e2e/orm/client-api/computed-fields.test.ts

📝 Walkthrough

Walkthrough

The ORM now parenthesizes computed-field expressions before embedding them in larger SQL expressions. End-to-end tests cover direct, negated, and logical boolean filters while confirming computed-field reads.

Changes

Computed field filtering

Layer / File(s)Summary
Parenthesized computed expressions and boolean filter coverage
packages/orm/src/client/crud/dialects/base-dialect.ts, tests/e2e/orm/client-api/computed-fields.test.ts
fieldRef now wraps computed-field expressions in parentheses and continues forwarding query-time arguments. End-to-end tests cover direct, NOT, and OR boolean filters and computed-field reads.

Estimated code review effort: 2 (Simple) | ~10 minutes

Mergeability Score:⚪ Minimal · up to 31801

The PR makes a localized SQL-expression parenthesization change and adds coverage for the affected computed-field cases; no actionable merge-blocking risk remains beyond normal checks and review.

Possibly related PRs

  • zenstackhq/zenstack#2744: Both PRs modify BaseCrudDialect.fieldRef; this PR adds parenthesization while the related PR adds argument forwarding.
  • zenstackhq/zenstack#2762: This PR builds on related computed-field handling in fieldRef.
  • zenstackhq/zenstack#2789: Both PRs modify computed-field handling in base-dialect.ts; this PR addresses the SQL parenthesization issue.

Suggested reviewers:ymc9

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check nameStatusExplanation
Description Check✅ PassedCheck skipped - CodeRabbit’s high-level summary is enabled.
Title check✅ PassedThe title clearly describes the main fix: parenthesizing inlined computed-field expressions.
Linked Issues check✅ PassedThe implementation and tests satisfy issue #2795 by parenthesizing computed expressions and covering boolean, NOT, OR, and AND filtering.
Out of Scope Changes check✅ PassedAll changes directly support issue #2795 and add focused regression coverage without unrelated code changes.
Docstring Coverage✅ PassedNo functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

packages/orm/src/client/crud/dialects/base-dialect.ts

ESLint skipped: missing config or dependency (missing-dependency). The ESLint configuration references a package that is not available in the sandbox.


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.

A computed field is inlined into larger expressions — a boolean filter renders it as
`<expr> = $n` — so an implementation whose top-level node is a bare comparison produced
a chained `"authorId" = $2 = $3`. Postgres rejects that as a syntax error; sqlite and
mysql only parse it correctly by accident of left associativity.
Wrap the implementation's expression in parens so its operator precedence stays
contained. Kysely doesn't double-wrap an already parenthesized expression, so `eb.or`
/`eb.and`-based and subquery implementations compile unchanged.
Fixeszenstackhq#2795
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@evgenovalov
evgenovalovforce-pushed the fix/parenthesize-inlined-computed-field branch from c67c8a2 to 318018fCompareAugust 13, 2026 09:37
@ymc9
ymc9 merged commit 1390aa0 into zenstackhq:devAug 14, 2026
8 checks passed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Computed field expression is not parenthesized when inlined, breaking boolean filters on Postgres

2 participants

@evgenovalov@ymc9
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

fix(orm): parenthesize an inlined computed field expression - #2796

Merged
ymc9 merged 1 commit into
zenstackhq:devfrom
evgenovalov:fix/parenthesize-inlined-computed-field
Aug 14, 2026
Merged

fix(orm): parenthesize an inlined computed field expression#2796
ymc9 merged 1 commit into
zenstackhq:devfrom
evgenovalov:fix/parenthesize-inlined-computed-field

Conversation

@evgenovalov

@evgenovalovevgenovalov commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

Fixes#2795

Motivation

A computed field is inlined into larger expressions — a boolean filter renders it as <expr> = $n. If the implementation's top-level node is a bare comparison, the operator precedence leaks into the surrounding query:

computedFields: {post: {isMine: (eb)=>eb('authorId','=',1),},},
awaitdb.post.findMany({where: {isMine: true}});
error: syntax error at or near "="
-- beforeselect"Post"."id"as"id", "authorId"= $1as"isMine"from"public"."Post"where"authorId"= $2= $3-- afterselect"Post"."id"as"id", ("authorId"= $1) as"isMine"from"public"."Post"where ("authorId"= $2) = $3

Postgres rejects the chained = outright. SQLite and MySQL parse it left-associatively as (a = $2) = $3, which happens to be the intended semantics, so the bug is invisible there — it surfaced only on the Postgres CI matrix of #2789.

Changes

  • BaseCrudDialect.fieldRef() wraps the implementation's expression in eb.parens() when inlining a computed field.
  • e2e test in computed-fields.test.ts (runs on every provider in the matrix): a bare-comparison field asserted through where: { isMine: true } / { isMine: false } / NOT, plus an eb.or-based field as a no-regression guard.

The wrap is unconditional because Kysely skips it for an expression that is already parenthesized, so eb.or / eb.and and subquery implementations compile byte-for-byte as before; only bare comparisons, unary operations and raw fragments change. Verified against the compiled SQL for each shape.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes
    • Improved filtering for computed fields that use comparisons or combined logical expressions.
    • Ensured computed-field expressions are grouped correctly when used in larger queries.
    • Preserved support for true, false, NOT, and OR filtering behavior.

@coderabbitai

coderabbitaiBot commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: e19e607e-15de-4b9b-9fc5-53141392c015

📥 Commits

Reviewing files that changed from the base of the PR and between c67c8a2 and 318018f.

📒 Files selected for processing (2)
  • packages/orm/src/client/crud/dialects/base-dialect.ts
  • tests/e2e/orm/client-api/computed-fields.test.ts
🚧 Files skipped from review as they are similar to previous changes (2)
  • packages/orm/src/client/crud/dialects/base-dialect.ts
  • tests/e2e/orm/client-api/computed-fields.test.ts

📝 Walkthrough

Walkthrough

The ORM now parenthesizes computed-field expressions before embedding them in larger SQL expressions. End-to-end tests cover direct, negated, and logical boolean filters while confirming computed-field reads.

Changes

Computed field filtering

Layer / File(s)Summary
Parenthesized computed expressions and boolean filter coverage
packages/orm/src/client/crud/dialects/base-dialect.ts, tests/e2e/orm/client-api/computed-fields.test.ts
fieldRef now wraps computed-field expressions in parentheses and continues forwarding query-time arguments. End-to-end tests cover direct, NOT, and OR boolean filters and computed-field reads.

Estimated code review effort: 2 (Simple) | ~10 minutes

Mergeability Score:⚪ Minimal · up to 31801

The PR makes a localized SQL-expression parenthesization change and adds coverage for the affected computed-field cases; no actionable merge-blocking risk remains beyond normal checks and review.

Possibly related PRs

  • zenstackhq/zenstack#2744: Both PRs modify BaseCrudDialect.fieldRef; this PR adds parenthesization while the related PR adds argument forwarding.
  • zenstackhq/zenstack#2762: This PR builds on related computed-field handling in fieldRef.
  • zenstackhq/zenstack#2789: Both PRs modify computed-field handling in base-dialect.ts; this PR addresses the SQL parenthesization issue.

Suggested reviewers:ymc9

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check nameStatusExplanation
Description Check✅ PassedCheck skipped - CodeRabbit’s high-level summary is enabled.
Title check✅ PassedThe title clearly describes the main fix: parenthesizing inlined computed-field expressions.
Linked Issues check✅ PassedThe implementation and tests satisfy issue #2795 by parenthesizing computed expressions and covering boolean, NOT, OR, and AND filtering.
Out of Scope Changes check✅ PassedAll changes directly support issue #2795 and add focused regression coverage without unrelated code changes.
Docstring Coverage✅ PassedNo functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

packages/orm/src/client/crud/dialects/base-dialect.ts

ESLint skipped: missing config or dependency (missing-dependency). The ESLint configuration references a package that is not available in the sandbox.


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.

A computed field is inlined into larger expressions — a boolean filter renders it as
`<expr> = $n` — so an implementation whose top-level node is a bare comparison produced
a chained `"authorId" = $2 = $3`. Postgres rejects that as a syntax error; sqlite and
mysql only parse it correctly by accident of left associativity.
Wrap the implementation's expression in parens so its operator precedence stays
contained. Kysely doesn't double-wrap an already parenthesized expression, so `eb.or`
/`eb.and`-based and subquery implementations compile unchanged.
Fixeszenstackhq#2795
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@evgenovalov
evgenovalovforce-pushed the fix/parenthesize-inlined-computed-field branch from c67c8a2 to 318018fCompareAugust 13, 2026 09:37
@ymc9
ymc9 merged commit 1390aa0 into zenstackhq:devAug 14, 2026
8 checks passed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Computed field expression is not parenthesized when inlined, breaking boolean filters on Postgres

2 participants

@evgenovalov@ymc9
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content

fix(orm): parenthesize an inlined computed field expression - #2796

Merged
ymc9 merged 1 commit into
zenstackhq:devfrom
evgenovalov:fix/parenthesize-inlined-computed-field
Aug 14, 2026
Merged

fix(orm): parenthesize an inlined computed field expression#2796
ymc9 merged 1 commit into
zenstackhq:devfrom
evgenovalov:fix/parenthesize-inlined-computed-field

Conversation

@evgenovalov

@evgenovalovevgenovalov commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

Fixes#2795

Motivation

A computed field is inlined into larger expressions — a boolean filter renders it as <expr> = $n. If the implementation's top-level node is a bare comparison, the operator precedence leaks into the surrounding query:

computedFields: {post: {isMine: (eb)=>eb('authorId','=',1),},},
awaitdb.post.findMany({where: {isMine: true}});
error: syntax error at or near "="
-- beforeselect"Post"."id"as"id", "authorId"= $1as"isMine"from"public"."Post"where"authorId"= $2= $3-- afterselect"Post"."id"as"id", ("authorId"= $1) as"isMine"from"public"."Post"where ("authorId"= $2) = $3

Postgres rejects the chained = outright. SQLite and MySQL parse it left-associatively as (a = $2) = $3, which happens to be the intended semantics, so the bug is invisible there — it surfaced only on the Postgres CI matrix of #2789.

Changes

  • BaseCrudDialect.fieldRef() wraps the implementation's expression in eb.parens() when inlining a computed field.
  • e2e test in computed-fields.test.ts (runs on every provider in the matrix): a bare-comparison field asserted through where: { isMine: true } / { isMine: false } / NOT, plus an eb.or-based field as a no-regression guard.

The wrap is unconditional because Kysely skips it for an expression that is already parenthesized, so eb.or / eb.and and subquery implementations compile byte-for-byte as before; only bare comparisons, unary operations and raw fragments change. Verified against the compiled SQL for each shape.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes
    • Improved filtering for computed fields that use comparisons or combined logical expressions.
    • Ensured computed-field expressions are grouped correctly when used in larger queries.
    • Preserved support for true, false, NOT, and OR filtering behavior.

@coderabbitai

coderabbitaiBot commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: e19e607e-15de-4b9b-9fc5-53141392c015

📥 Commits

Reviewing files that changed from the base of the PR and between c67c8a2 and 318018f.

📒 Files selected for processing (2)
  • packages/orm/src/client/crud/dialects/base-dialect.ts
  • tests/e2e/orm/client-api/computed-fields.test.ts
🚧 Files skipped from review as they are similar to previous changes (2)
  • packages/orm/src/client/crud/dialects/base-dialect.ts
  • tests/e2e/orm/client-api/computed-fields.test.ts

📝 Walkthrough

Walkthrough

The ORM now parenthesizes computed-field expressions before embedding them in larger SQL expressions. End-to-end tests cover direct, negated, and logical boolean filters while confirming computed-field reads.

Changes

Computed field filtering

Layer / File(s)Summary
Parenthesized computed expressions and boolean filter coverage
packages/orm/src/client/crud/dialects/base-dialect.ts, tests/e2e/orm/client-api/computed-fields.test.ts
fieldRef now wraps computed-field expressions in parentheses and continues forwarding query-time arguments. End-to-end tests cover direct, NOT, and OR boolean filters and computed-field reads.

Estimated code review effort: 2 (Simple) | ~10 minutes

Mergeability Score:⚪ Minimal · up to 31801

The PR makes a localized SQL-expression parenthesization change and adds coverage for the affected computed-field cases; no actionable merge-blocking risk remains beyond normal checks and review.

Possibly related PRs

  • zenstackhq/zenstack#2744: Both PRs modify BaseCrudDialect.fieldRef; this PR adds parenthesization while the related PR adds argument forwarding.
  • zenstackhq/zenstack#2762: This PR builds on related computed-field handling in fieldRef.
  • zenstackhq/zenstack#2789: Both PRs modify computed-field handling in base-dialect.ts; this PR addresses the SQL parenthesization issue.

Suggested reviewers:ymc9

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check nameStatusExplanation
Description Check✅ PassedCheck skipped - CodeRabbit’s high-level summary is enabled.
Title check✅ PassedThe title clearly describes the main fix: parenthesizing inlined computed-field expressions.
Linked Issues check✅ PassedThe implementation and tests satisfy issue #2795 by parenthesizing computed expressions and covering boolean, NOT, OR, and AND filtering.
Out of Scope Changes check✅ PassedAll changes directly support issue #2795 and add focused regression coverage without unrelated code changes.
Docstring Coverage✅ PassedNo functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

packages/orm/src/client/crud/dialects/base-dialect.ts

ESLint skipped: missing config or dependency (missing-dependency). The ESLint configuration references a package that is not available in the sandbox.


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.

A computed field is inlined into larger expressions — a boolean filter renders it as
`<expr> = $n` — so an implementation whose top-level node is a bare comparison produced
a chained `"authorId" = $2 = $3`. Postgres rejects that as a syntax error; sqlite and
mysql only parse it correctly by accident of left associativity.
Wrap the implementation's expression in parens so its operator precedence stays
contained. Kysely doesn't double-wrap an already parenthesized expression, so `eb.or`
/`eb.and`-based and subquery implementations compile unchanged.
Fixeszenstackhq#2795
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@evgenovalov
evgenovalovforce-pushed the fix/parenthesize-inlined-computed-field branch from c67c8a2 to 318018fCompareAugust 13, 2026 09:37
@ymc9
ymc9 merged commit 1390aa0 into zenstackhq:devAug 14, 2026
8 checks passed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Computed field expression is not parenthesized when inlined, breaking boolean filters on Postgres

2 participants

@evgenovalov@ymc9
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

fix(orm): parenthesize an inlined computed field expression - #2796

Merged
ymc9 merged 1 commit into
zenstackhq:devfrom
evgenovalov:fix/parenthesize-inlined-computed-field
Aug 14, 2026
Merged

fix(orm): parenthesize an inlined computed field expression#2796
ymc9 merged 1 commit into
zenstackhq:devfrom
evgenovalov:fix/parenthesize-inlined-computed-field

Conversation

@evgenovalov

@evgenovalovevgenovalov commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

Fixes#2795

Motivation

A computed field is inlined into larger expressions — a boolean filter renders it as <expr> = $n. If the implementation's top-level node is a bare comparison, the operator precedence leaks into the surrounding query:

computedFields: {post: {isMine: (eb)=>eb('authorId','=',1),},},
awaitdb.post.findMany({where: {isMine: true}});
error: syntax error at or near "="
-- beforeselect"Post"."id"as"id", "authorId"= $1as"isMine"from"public"."Post"where"authorId"= $2= $3-- afterselect"Post"."id"as"id", ("authorId"= $1) as"isMine"from"public"."Post"where ("authorId"= $2) = $3

Postgres rejects the chained = outright. SQLite and MySQL parse it left-associatively as (a = $2) = $3, which happens to be the intended semantics, so the bug is invisible there — it surfaced only on the Postgres CI matrix of #2789.

Changes

  • BaseCrudDialect.fieldRef() wraps the implementation's expression in eb.parens() when inlining a computed field.
  • e2e test in computed-fields.test.ts (runs on every provider in the matrix): a bare-comparison field asserted through where: { isMine: true } / { isMine: false } / NOT, plus an eb.or-based field as a no-regression guard.

The wrap is unconditional because Kysely skips it for an expression that is already parenthesized, so eb.or / eb.and and subquery implementations compile byte-for-byte as before; only bare comparisons, unary operations and raw fragments change. Verified against the compiled SQL for each shape.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes
    • Improved filtering for computed fields that use comparisons or combined logical expressions.
    • Ensured computed-field expressions are grouped correctly when used in larger queries.
    • Preserved support for true, false, NOT, and OR filtering behavior.

@coderabbitai

coderabbitaiBot commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: e19e607e-15de-4b9b-9fc5-53141392c015

📥 Commits

Reviewing files that changed from the base of the PR and between c67c8a2 and 318018f.

📒 Files selected for processing (2)
  • packages/orm/src/client/crud/dialects/base-dialect.ts
  • tests/e2e/orm/client-api/computed-fields.test.ts
🚧 Files skipped from review as they are similar to previous changes (2)
  • packages/orm/src/client/crud/dialects/base-dialect.ts
  • tests/e2e/orm/client-api/computed-fields.test.ts

📝 Walkthrough

Walkthrough

The ORM now parenthesizes computed-field expressions before embedding them in larger SQL expressions. End-to-end tests cover direct, negated, and logical boolean filters while confirming computed-field reads.

Changes

Computed field filtering

Layer / File(s)Summary
Parenthesized computed expressions and boolean filter coverage
packages/orm/src/client/crud/dialects/base-dialect.ts, tests/e2e/orm/client-api/computed-fields.test.ts
fieldRef now wraps computed-field expressions in parentheses and continues forwarding query-time arguments. End-to-end tests cover direct, NOT, and OR boolean filters and computed-field reads.

Estimated code review effort: 2 (Simple) | ~10 minutes

Mergeability Score:⚪ Minimal · up to 31801

The PR makes a localized SQL-expression parenthesization change and adds coverage for the affected computed-field cases; no actionable merge-blocking risk remains beyond normal checks and review.

Possibly related PRs

  • zenstackhq/zenstack#2744: Both PRs modify BaseCrudDialect.fieldRef; this PR adds parenthesization while the related PR adds argument forwarding.
  • zenstackhq/zenstack#2762: This PR builds on related computed-field handling in fieldRef.
  • zenstackhq/zenstack#2789: Both PRs modify computed-field handling in base-dialect.ts; this PR addresses the SQL parenthesization issue.

Suggested reviewers:ymc9

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check nameStatusExplanation
Description Check✅ PassedCheck skipped - CodeRabbit’s high-level summary is enabled.
Title check✅ PassedThe title clearly describes the main fix: parenthesizing inlined computed-field expressions.
Linked Issues check✅ PassedThe implementation and tests satisfy issue #2795 by parenthesizing computed expressions and covering boolean, NOT, OR, and AND filtering.
Out of Scope Changes check✅ PassedAll changes directly support issue #2795 and add focused regression coverage without unrelated code changes.
Docstring Coverage✅ PassedNo functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

packages/orm/src/client/crud/dialects/base-dialect.ts

ESLint skipped: missing config or dependency (missing-dependency). The ESLint configuration references a package that is not available in the sandbox.


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.

A computed field is inlined into larger expressions — a boolean filter renders it as
`<expr> = $n` — so an implementation whose top-level node is a bare comparison produced
a chained `"authorId" = $2 = $3`. Postgres rejects that as a syntax error; sqlite and
mysql only parse it correctly by accident of left associativity.
Wrap the implementation's expression in parens so its operator precedence stays
contained. Kysely doesn't double-wrap an already parenthesized expression, so `eb.or`
/`eb.and`-based and subquery implementations compile unchanged.
Fixeszenstackhq#2795
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@evgenovalov
evgenovalovforce-pushed the fix/parenthesize-inlined-computed-field branch from c67c8a2 to 318018fCompareAugust 13, 2026 09:37
@ymc9
ymc9 merged commit 1390aa0 into zenstackhq:devAug 14, 2026
8 checks passed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Computed field expression is not parenthesized when inlined, breaking boolean filters on Postgres

2 participants

@evgenovalov@ymc9
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

fix(orm): parenthesize an inlined computed field expression - #2796

Merged
ymc9 merged 1 commit into
zenstackhq:devfrom
evgenovalov:fix/parenthesize-inlined-computed-field
Aug 14, 2026
Merged

fix(orm): parenthesize an inlined computed field expression#2796
ymc9 merged 1 commit into
zenstackhq:devfrom
evgenovalov:fix/parenthesize-inlined-computed-field

Conversation

@evgenovalov

@evgenovalovevgenovalov commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

Fixes#2795

Motivation

A computed field is inlined into larger expressions — a boolean filter renders it as <expr> = $n. If the implementation's top-level node is a bare comparison, the operator precedence leaks into the surrounding query:

computedFields: {post: {isMine: (eb)=>eb('authorId','=',1),},},
awaitdb.post.findMany({where: {isMine: true}});
error: syntax error at or near "="
-- beforeselect"Post"."id"as"id", "authorId"= $1as"isMine"from"public"."Post"where"authorId"= $2= $3-- afterselect"Post"."id"as"id", ("authorId"= $1) as"isMine"from"public"."Post"where ("authorId"= $2) = $3

Postgres rejects the chained = outright. SQLite and MySQL parse it left-associatively as (a = $2) = $3, which happens to be the intended semantics, so the bug is invisible there — it surfaced only on the Postgres CI matrix of #2789.

Changes

  • BaseCrudDialect.fieldRef() wraps the implementation's expression in eb.parens() when inlining a computed field.
  • e2e test in computed-fields.test.ts (runs on every provider in the matrix): a bare-comparison field asserted through where: { isMine: true } / { isMine: false } / NOT, plus an eb.or-based field as a no-regression guard.

The wrap is unconditional because Kysely skips it for an expression that is already parenthesized, so eb.or / eb.and and subquery implementations compile byte-for-byte as before; only bare comparisons, unary operations and raw fragments change. Verified against the compiled SQL for each shape.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes
    • Improved filtering for computed fields that use comparisons or combined logical expressions.
    • Ensured computed-field expressions are grouped correctly when used in larger queries.
    • Preserved support for true, false, NOT, and OR filtering behavior.

@coderabbitai

coderabbitaiBot commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: e19e607e-15de-4b9b-9fc5-53141392c015

📥 Commits

Reviewing files that changed from the base of the PR and between c67c8a2 and 318018f.

📒 Files selected for processing (2)
  • packages/orm/src/client/crud/dialects/base-dialect.ts
  • tests/e2e/orm/client-api/computed-fields.test.ts
🚧 Files skipped from review as they are similar to previous changes (2)
  • packages/orm/src/client/crud/dialects/base-dialect.ts
  • tests/e2e/orm/client-api/computed-fields.test.ts

📝 Walkthrough

Walkthrough

The ORM now parenthesizes computed-field expressions before embedding them in larger SQL expressions. End-to-end tests cover direct, negated, and logical boolean filters while confirming computed-field reads.

Changes

Computed field filtering

Layer / File(s)Summary
Parenthesized computed expressions and boolean filter coverage
packages/orm/src/client/crud/dialects/base-dialect.ts, tests/e2e/orm/client-api/computed-fields.test.ts
fieldRef now wraps computed-field expressions in parentheses and continues forwarding query-time arguments. End-to-end tests cover direct, NOT, and OR boolean filters and computed-field reads.

Estimated code review effort: 2 (Simple) | ~10 minutes

Mergeability Score:⚪ Minimal · up to 31801

The PR makes a localized SQL-expression parenthesization change and adds coverage for the affected computed-field cases; no actionable merge-blocking risk remains beyond normal checks and review.

Possibly related PRs

  • zenstackhq/zenstack#2744: Both PRs modify BaseCrudDialect.fieldRef; this PR adds parenthesization while the related PR adds argument forwarding.
  • zenstackhq/zenstack#2762: This PR builds on related computed-field handling in fieldRef.
  • zenstackhq/zenstack#2789: Both PRs modify computed-field handling in base-dialect.ts; this PR addresses the SQL parenthesization issue.

Suggested reviewers:ymc9

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check nameStatusExplanation
Description Check✅ PassedCheck skipped - CodeRabbit’s high-level summary is enabled.
Title check✅ PassedThe title clearly describes the main fix: parenthesizing inlined computed-field expressions.
Linked Issues check✅ PassedThe implementation and tests satisfy issue #2795 by parenthesizing computed expressions and covering boolean, NOT, OR, and AND filtering.
Out of Scope Changes check✅ PassedAll changes directly support issue #2795 and add focused regression coverage without unrelated code changes.
Docstring Coverage✅ PassedNo functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

packages/orm/src/client/crud/dialects/base-dialect.ts

ESLint skipped: missing config or dependency (missing-dependency). The ESLint configuration references a package that is not available in the sandbox.


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.

A computed field is inlined into larger expressions — a boolean filter renders it as
`<expr> = $n` — so an implementation whose top-level node is a bare comparison produced
a chained `"authorId" = $2 = $3`. Postgres rejects that as a syntax error; sqlite and
mysql only parse it correctly by accident of left associativity.
Wrap the implementation's expression in parens so its operator precedence stays
contained. Kysely doesn't double-wrap an already parenthesized expression, so `eb.or`
/`eb.and`-based and subquery implementations compile unchanged.
Fixeszenstackhq#2795
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@evgenovalov
evgenovalovforce-pushed the fix/parenthesize-inlined-computed-field branch from c67c8a2 to 318018fCompareAugust 13, 2026 09:37
@ymc9
ymc9 merged commit 1390aa0 into zenstackhq:devAug 14, 2026
8 checks passed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Computed field expression is not parenthesized when inlined, breaking boolean filters on Postgres

2 participants

@evgenovalov@ymc9
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content

fix(orm): parenthesize an inlined computed field expression - #2796

Merged
ymc9 merged 1 commit into
zenstackhq:devfrom
evgenovalov:fix/parenthesize-inlined-computed-field
Aug 14, 2026
Merged

fix(orm): parenthesize an inlined computed field expression#2796
ymc9 merged 1 commit into
zenstackhq:devfrom
evgenovalov:fix/parenthesize-inlined-computed-field

Conversation

@evgenovalov

@evgenovalovevgenovalov commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

Fixes#2795

Motivation

A computed field is inlined into larger expressions — a boolean filter renders it as <expr> = $n. If the implementation's top-level node is a bare comparison, the operator precedence leaks into the surrounding query:

computedFields: {post: {isMine: (eb)=>eb('authorId','=',1),},},
awaitdb.post.findMany({where: {isMine: true}});
error: syntax error at or near "="
-- beforeselect"Post"."id"as"id", "authorId"= $1as"isMine"from"public"."Post"where"authorId"= $2= $3-- afterselect"Post"."id"as"id", ("authorId"= $1) as"isMine"from"public"."Post"where ("authorId"= $2) = $3

Postgres rejects the chained = outright. SQLite and MySQL parse it left-associatively as (a = $2) = $3, which happens to be the intended semantics, so the bug is invisible there — it surfaced only on the Postgres CI matrix of #2789.

Changes

  • BaseCrudDialect.fieldRef() wraps the implementation's expression in eb.parens() when inlining a computed field.
  • e2e test in computed-fields.test.ts (runs on every provider in the matrix): a bare-comparison field asserted through where: { isMine: true } / { isMine: false } / NOT, plus an eb.or-based field as a no-regression guard.

The wrap is unconditional because Kysely skips it for an expression that is already parenthesized, so eb.or / eb.and and subquery implementations compile byte-for-byte as before; only bare comparisons, unary operations and raw fragments change. Verified against the compiled SQL for each shape.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes
    • Improved filtering for computed fields that use comparisons or combined logical expressions.
    • Ensured computed-field expressions are grouped correctly when used in larger queries.
    • Preserved support for true, false, NOT, and OR filtering behavior.

@coderabbitai

coderabbitaiBot commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: e19e607e-15de-4b9b-9fc5-53141392c015

📥 Commits

Reviewing files that changed from the base of the PR and between c67c8a2 and 318018f.

📒 Files selected for processing (2)
  • packages/orm/src/client/crud/dialects/base-dialect.ts
  • tests/e2e/orm/client-api/computed-fields.test.ts
🚧 Files skipped from review as they are similar to previous changes (2)
  • packages/orm/src/client/crud/dialects/base-dialect.ts
  • tests/e2e/orm/client-api/computed-fields.test.ts

📝 Walkthrough

Walkthrough

The ORM now parenthesizes computed-field expressions before embedding them in larger SQL expressions. End-to-end tests cover direct, negated, and logical boolean filters while confirming computed-field reads.

Changes

Computed field filtering

Layer / File(s)Summary
Parenthesized computed expressions and boolean filter coverage
packages/orm/src/client/crud/dialects/base-dialect.ts, tests/e2e/orm/client-api/computed-fields.test.ts
fieldRef now wraps computed-field expressions in parentheses and continues forwarding query-time arguments. End-to-end tests cover direct, NOT, and OR boolean filters and computed-field reads.

Estimated code review effort: 2 (Simple) | ~10 minutes

Mergeability Score:⚪ Minimal · up to 31801

The PR makes a localized SQL-expression parenthesization change and adds coverage for the affected computed-field cases; no actionable merge-blocking risk remains beyond normal checks and review.

Possibly related PRs

  • zenstackhq/zenstack#2744: Both PRs modify BaseCrudDialect.fieldRef; this PR adds parenthesization while the related PR adds argument forwarding.
  • zenstackhq/zenstack#2762: This PR builds on related computed-field handling in fieldRef.
  • zenstackhq/zenstack#2789: Both PRs modify computed-field handling in base-dialect.ts; this PR addresses the SQL parenthesization issue.

Suggested reviewers:ymc9

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check nameStatusExplanation
Description Check✅ PassedCheck skipped - CodeRabbit’s high-level summary is enabled.
Title check✅ PassedThe title clearly describes the main fix: parenthesizing inlined computed-field expressions.
Linked Issues check✅ PassedThe implementation and tests satisfy issue #2795 by parenthesizing computed expressions and covering boolean, NOT, OR, and AND filtering.
Out of Scope Changes check✅ PassedAll changes directly support issue #2795 and add focused regression coverage without unrelated code changes.
Docstring Coverage✅ PassedNo functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

packages/orm/src/client/crud/dialects/base-dialect.ts

ESLint skipped: missing config or dependency (missing-dependency). The ESLint configuration references a package that is not available in the sandbox.


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.

A computed field is inlined into larger expressions — a boolean filter renders it as
`<expr> = $n` — so an implementation whose top-level node is a bare comparison produced
a chained `"authorId" = $2 = $3`. Postgres rejects that as a syntax error; sqlite and
mysql only parse it correctly by accident of left associativity.
Wrap the implementation's expression in parens so its operator precedence stays
contained. Kysely doesn't double-wrap an already parenthesized expression, so `eb.or`
/`eb.and`-based and subquery implementations compile unchanged.
Fixeszenstackhq#2795
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@evgenovalov
evgenovalovforce-pushed the fix/parenthesize-inlined-computed-field branch from c67c8a2 to 318018fCompareAugust 13, 2026 09:37
@ymc9
ymc9 merged commit 1390aa0 into zenstackhq:devAug 14, 2026
8 checks passed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Computed field expression is not parenthesized when inlined, breaking boolean filters on Postgres

2 participants

@evgenovalov@ymc9