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

[SQLite] Fix onConflict targets #475

Merged
merged 4 commits into from
Apr 18, 2023
Merged

Conversation

wkunert
Copy link
Contributor

@wkunert wkunert commented Apr 17, 2023

As reported in #410 upserts are failing when using SQLite. @krzysztofciombor already identified the root cause (missing parentheses around the target), this PR delivers the proposed fix and corresponding tests.

wkunert added 3 commits April 17, 2023 20:45
The indexed column(s) must be wrapped in parentheses (cp.
https://www.sqlite.org/lang_UPSERT.html).
The indexed column(s) must be wrapped in parentheses (cp.
https://www.sqlite.org/lang_UPSERT.html).
Examples are taken from the pg-core documentation.
@AndriiSherman
Copy link
Member

@wkunert thanks a lot!
Also thanks for getting tests for each driver. Looks good to be merged

Right after @dankochetov will check - will include in in main and next release we will have

@AndriiSherman AndriiSherman merged commit 3c4693f into drizzle-team:main Apr 18, 2023
@hirvesh
Copy link

hirvesh commented Apr 25, 2023

Hey guys, I think there's an issue here with composite primary keys where now we end up with double parentheses. See example schema and query generated below and associated error:

export const posts = sqliteTable("posts", {
  id: text("id").notNull(),
  siteId: text("siteId").notNull().references(() => sites.id, { onDelete: "cascade" }),
  title: text("title"),
  content: text("content"),
  slug: text("slug").notNull(),
  createdAt: integer("createdAt", { mode: "timestamp_ms" }),
  updatedAt: integer("updatedAt", { mode: "timestamp_ms" }),
  readyToPublish: integer("readyToPublish").default(0),
  publishDate: integer("publishDate", { mode: "timestamp_ms" }),
}, (posts) => ({
  compositePk: primaryKey(posts.id, posts.siteId)
}));
db.insert(posts)
.values({
  id: postIds[index],
  siteId: siteWithAccount.sites.id,
  title: getPageTitle(post.value),
  content: JSON.stringify(post.value),
  slug: getSlug(siteWithAccount.sites.id, getPageTitle(post.value)),
})
.onConflictDoUpdate({
  target: [posts.id, posts.siteId],
  set: {
    title: getPageTitle(post.value),
    content: JSON.stringify(post.value),
    slug: getSlug(siteWithAccount.sites.id, getPageTitle(post.value)),
  }
})
.run()

insert into "posts" ("id", "siteId", "title", "content", "slug") values (?, ?, ?, ?, ?) on conflict (("posts"."id", "posts"."siteId")) do update set "title" = ?, "content" = ?, "slug" = ?

See double parentheses added above, which causes error: ON CONFLICT clause does not match any PRIMARY KEY or UNIQUE constraint.

Opened bug report: #518

@wkunert
Copy link
Contributor Author

wkunert commented Apr 26, 2023

Thank you @hirvesh for the analysis – I created a PR with a fix (#521).

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.

4 participants