Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixing onConflict case with existing column used #3137

Merged
merged 1 commit into from
Nov 24, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 7 additions & 7 deletions .github/workflows/site.yml
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,11 @@ jobs:
if: ${{ github.event_name == 'pull_request' }}
steps:
- name: Git Checkout
uses: actions/checkout@v4.2.2
uses: actions/checkout@v3.3.0
with:
fetch-depth: '0'
- name: Setup Scala
uses: actions/setup-java@v4.5.0
uses: actions/setup-java@v3.9.0
with:
distribution: temurin
java-version: 17
Expand All @@ -39,11 +39,11 @@ jobs:
if: ${{ ((github.event_name == 'release') && (github.event.action == 'published')) || (github.event_name == 'workflow_dispatch') }}
steps:
- name: Git Checkout
uses: actions/checkout@v4.2.2
uses: actions/checkout@v3.3.0
with:
fetch-depth: '0'
- name: Setup Scala
uses: actions/setup-java@v4.5.0
uses: actions/setup-java@v3.9.0
with:
distribution: temurin
java-version: 17
Expand All @@ -63,12 +63,12 @@ jobs:
if: ${{ (github.event_name == 'push') || ((github.event_name == 'release') && (github.event.action == 'published')) }}
steps:
- name: Git Checkout
uses: actions/checkout@v4.2.2
uses: actions/checkout@v3.3.0
with:
ref: ${{ github.head_ref }}
fetch-depth: '0'
- name: Setup Scala
uses: actions/setup-java@v4.5.0
uses: actions/setup-java@v3.9.0
with:
distribution: temurin
java-version: 17
Expand All @@ -82,7 +82,7 @@ jobs:
git add README.md
git commit -m "Update README.md" || echo "No changes to commit"
- name: Create Pull Request
uses: peter-evans/create-pull-request@v7.0.5
uses: peter-evans/create-pull-request@v4.2.3
with:
body: |-
Autogenerated changes after running the `sbt docs/generateReadme` command of the [zio-sbt-website](https://zio.dev/zio-sbt) plugin.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,32 @@ trait OnConflictSupport {
strategy: NamingStrategy,
idiomContext: IdiomContext
): Tokenizer[OnConflict] = {
val entityAlias = "t"

val customEntityTokenizer = Tokenizer[Entity] { case Entity.Opinionated(name, _, _, renameable) =>
stmt"INTO ${renameable.fixedOr(name.token)(strategy.table(name).token)} AS t"
stmt"INTO ${renameable.fixedOr(name.token)(strategy.table(name).token)} AS ${entityAlias.token}"
}

val customAstTokenizer =
Tokenizer.withFallback[Ast](self.astTokenizer(_, strategy, idiomContext)) {
case Property(_: OnConflict.Excluded, value) => stmt"EXCLUDED.${value.token}"

// At first glance it might be hard to understand why this is doing `case OnConflict.Existing(a) => stmt"${entityAlias}"`
// but consider that this is a situation where multiple aliases are used in multiple update clauses e.g. the `tt` in the below example
// wouldn't even exist as a variable because in the query produced (also below) it would not even exist
// i.e. since the table will be aliased as the first `existing table` variable i.e. `t`.
// The second one `tt` wouldn't even exist.
// ins.onConflictUpdate(_.i, _.s)(
// (t, e) => t.l -> foo(t.l, e.l), (tt, ee) => tt.l -> bar(tt.l, ee.l)
// )
// This doesn't exist!!
// v
// > INSERT INTO TestEntity AS t (s,i,l,o,b) VALUES (?, ?, ?, ?, ?) ON CONFLICT (i,s) DO UPDATE SET l = foo(t.l, EXCLUDED.l), l = bar(tt.l, EXCLUDED.l)
//
// So instead of the situation we use the original alias of the table as it was computed by Quill i.e. `t`.
// See the "cols target - update + infix" example for more detail
case _: OnConflict.Excluded => stmt"EXCLUDED"
case OnConflict.Existing(a) => stmt"${a.token}"
case _: OnConflict.Existing => stmt"${entityAlias.token}"
case a: Action =>
self.actionTokenizer(customEntityTokenizer)(actionAstTokenizer, strategy, idiomContext).token(a)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,14 @@ trait OnConflictSpec extends Spec {
def `cols target - update`(ins: Quoted[Insert[TestEntity]]) = quote {
ins.onConflictUpdate(_.i, _.s)((t, e) => t.l -> (t.l + e.l) / 2, _.s -> _.s)
}
// In this situation the `tt` variable that the "existing" row is pointing to (in the second clause) wouldn't
// even be created so clearly the variable name itself must be dropped and only the column reference should be used
def `cols target - update + infix`(ins: Quoted[Insert[TestEntity]]) = quote {
ins.onConflictUpdate(_.i, _.s)(
(t, e) => t.l -> sql"foo(${t.l}, ${e.l})".as[Long],
(tt, ee) => tt.l -> sql"bar(${tt.l}, ${ee.l})".as[Long]
)
}
def insBatch = quote(liftQuery(List(e, TestEntity("s2", 1, 2L, Some(1), true))))

def `no target - ignore batch` = quote {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,5 +62,9 @@ class PostgresDialectSpec extends OnConflictSpec {
ctx.run(`cols target - update`(i)).string mustEqual
"INSERT INTO TestEntity AS t (s,i,l,o,b) VALUES (?, ?, ?, ?, ?) ON CONFLICT (i,s) DO UPDATE SET l = ((t.l + EXCLUDED.l) / 2), s = EXCLUDED.s"
}
"cols target - update + infix" in {
ctx.run(`cols target - update + infix`(i)).string mustEqual
"INSERT INTO TestEntity AS t (s,i,l,o,b) VALUES (?, ?, ?, ?, ?) ON CONFLICT (i,s) DO UPDATE SET l = foo(t.l, EXCLUDED.l), l = bar(t.l, EXCLUDED.l)"
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -53,5 +53,9 @@ class SqliteDialectSpec extends OnConflictSpec {
ctx.run(`cols target - update`(i)).string mustEqual
"INSERT INTO TestEntity AS t (s,i,l,o,b) VALUES (?, ?, ?, ?, ?) ON CONFLICT (i,s) DO UPDATE SET l = ((t.l + EXCLUDED.l) / 2), s = EXCLUDED.s"
}
"cols target - update + infix" in {
ctx.run(`cols target - update + infix`(i)).string mustEqual
"INSERT INTO TestEntity AS t (s,i,l,o,b) VALUES (?, ?, ?, ?, ?) ON CONFLICT (i,s) DO UPDATE SET l = foo(t.l, EXCLUDED.l), l = bar(t.l, EXCLUDED.l)"
}
}
}
Loading