Skip to content

Reject nested lift at compile time and run nothing for empty batches - #818

Merged
khajavi merged 4 commits into
zio:masterfrom
mberndt123:nested-lift-empty-batch
Oct 29, 2026
Merged

khajavi merged 4 commits into
zio:masterfrom
mberndt123:nested-lift-empty-batch

Conversation

@mberndt123

@mberndt123 mberndt123 commented Oct 27, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

Solution

The first commit builds against the zio-quill master snapshot that contains zio/zio-quill#3455, #3456 and #3457. The flat join tests in CriticalIssuesSpec (#396, #239, #335) need #3456, and #3457 changes the expected batch update RETURNING in BatchUpdateValuesMirrorSpec from ps.age to p.age.

The second commit is #728 by @li-nkSN, unchanged apart from the package of the new CriticalIssuesSpec:

  • LiftMacro rejects a lifted expression that is itself a lift.
  • QueryExecutionBatchIteration returns no batch groups for an empty collection, so nothing is executed.

The third commit narrows the nested lift check. It matched any expression ending in a method named unquote, so lift(w.unquote) for a user-defined unquote method was rejected as a nested lift, although it compiled before. It now only matches when unquote is called on a Planter.

Notes

  • Supersedes fix: compile-time nested lift detection (#412) and empty batch guard (#431) #728, which can be closed once this is merged.
  • With an empty collection, ctx.translate of a batch action now returns no statements instead of one.
  • Verified: Test/compile of all modules, quill-sql/test (280 tests), quill-sql-tests/test (673 tests), and the Postgres tests of quill-jdbc (148) and quill-jdbc-zio (43) against the docker-compose Postgres. The #412 test fails without the nested lift check, and a lift(w.unquote) with a user-defined unquote compiles with the third commit.

Checklist

  • Unit test all changes
  • Update README.md if applicable
  • Add [WIP] to the pull request title if it's work in progress
  • Squash commits that aren't meaningful changes

@getquill/maintainers

🤖 Generated with Claude Code

mberndt123 and others added 3 commits October 28, 2026 14:44
…#3456 and #3457

zio/zio-quill#3457 qualifies batch update RETURNING with the updated
table's alias instead of the VALUES alias.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- zio#412: lift(lift(x)) previously compiled but threw RuntimeException at
  runtime ("Unquotation can only be done from a quoted block."). Now
  detected at compile time in LiftMacro.apply by inspecting the
  expression tree for .unquote calls from an inner lift expansion.
  Uses a custom AST unwrapper since underlyingArgument doesn't strip
  Inlined nodes with non-empty bindings.

- zio#431: liftQuery(List.empty[T]).foreach(...) produced a batch group
  with SQL but no prepare rows, leading to invalid execution. Now
  returns Nil (no batch groups) when perRowLifts is empty.

Tests for zio#396, zio#239, and zio#335 are included in CriticalIssuesSpec but
will fail until zio/zio-quill#3378 is merged by @getquill/maintainers
and the quill-engine dependency version is bumped.

Authored-by: Colin K. Williams (li-nkSN) <colin@li-nk.org>
Refs: zio#412, zio#431
(cherry picked from commit b0fe910)
The check matched any expression ending in a method named unquote, so it
also rejected lifting values like w.unquote for a user-defined unquote
method, which compiled before.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@mberndt123
mberndt123 force-pushed the nested-lift-empty-batch branch from 9ae5679 to 9f72d76 Compare October 28, 2026 13:45
@mberndt123

Copy link
Copy Markdown
Collaborator Author

@khajavi,
this is the last one that I'd like to get merged before the release

@khajavi
khajavi merged commit ff6ec88 into zio:master Oct 29, 2026
6 checks passed
Sign up for free to 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.

3 participants