skoshx / pentagon

Prisma-like ORM built on top of Deno KV. Allows you to write your database schemas and relations using Zod schemas, and run queries using familiar syntax from Prisma.
https://dash.deno.com/playground/pentagon-demo
MIT License
153 stars 4 forks source link

[Bug]: Table 'bookingReference' can't use a non-unique index without a primary index #32

Open waptik opened 1 year ago

waptik commented 1 year ago

So i came across some serious bug which i think it is. I was moving forward with my project when i noticed an error while querying an index field. I thought it was some code error on my part but i found no error after hours of searching. So i updated my demo example and tried it there after updating to latest version. At this location, https://github.com/waptik/test-deno-pentagon/blob/main/main.ts#L67, you can replace the value of meetingId with crypto.randomUUID() and you'll still get the error found in the screenshot. If you remove .describe('index') from https://github.com/waptik/test-deno-pentagon/blob/main/db.ts#L80 and save the file, the code will run perfectly without errors.

image

skoshx commented 1 year ago

Hello, thanks for opening this issue, will investigate!

waptik commented 1 year ago

Hello, thanks for opening this issue, will investigate!

Okay, looking forward to a positive outcome

skoshx commented 1 year ago

Hello,

I'm curious to understand your use of index for meetingId in this example.

export const BookingReference = z
  .object({
    id: z
      .string()
      .uuid()
      .describe("primary")
      .default(() => generateUUID()),
    type: z.string().min(1),
    bookingId: z.string().uuid().optional(),
    meetingId: z.string().uuid().describe("index").optional(),
    meetingUrl: z.string().url().optional(),
    meetingPassword: z.string().optional(),
  })
  .merge(WithDefautTimestamps);

Why is the meetingId indexed, when it can also be optional? The use of indexed keys is basically to improve querying speed of many different entries that have the matching indexed key value. So for example; if you had a million users with a indexed key favoriteColor, and indexed key would make it faster to search the users that have eg favoriteColor: 'yellow'

So I'm curious to understand the use-case here, with an optional indexed key?

waptik commented 1 year ago

Okay so I was working on a simple paid booking app prior to using a subset of the db as a demo. The meetingId is an index obviously to make querying faster and it’s optional because I first link the booking id to the newly created reference and when payment has been made and the third party meeting call link has been created, it also get linked to the reference. That’s why the meetingId is not a required field

On Sun, 30 Jul 2023 at 20:20, Skosh @.***> wrote:

Hello,

I'm curious to understand your use of index for meetingId in this example.

export const BookingReference = z .object({ id: z .string() .uuid() .describe("primary") .default(() => generateUUID()), type: z.string().min(1), bookingId: z.string().uuid().optional(), meetingId: z.string().uuid().describe("index").optional(), meetingUrl: z.string().url().optional(), meetingPassword: z.string().optional(), }) .merge(WithDefautTimestamps);

Why is the meetingId indexed, when it can also be optional? The use of indexed keys is basically to improve querying speed of many different entries that have the matching indexed key value. So for example; if you had a million users with a indexed key favoriteColor, and indexed key would make it faster to search the users that have eg favoriteColor: 'yellow'

So I'm curious to understand the use-case here, with an optional indexed key?

— Reply to this email directly, view it on GitHub https://github.com/skoshx/pentagon/issues/32#issuecomment-1657259011, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAM3773XMBJSAYERTY25XHLXS26ZRANCNFSM6AAAAAA2ZXQ23A . You are receiving this because you authored the thread.Message ID: @.***>

skoshx commented 1 year ago

Okay, I see. I think in this case the meetingId shouldn't be an indexed key, because it doesn't really help in performance unless there are many bookings with the same meetingId (I think it would have to be hundreds of bookings to make performance difference).

If everything works fine without the index, I think then just removing it is the best solution for now!

waptik commented 1 year ago

Okay, i've removed the index for now after opening this issue but i still think this should be looked into. Right now, there's no way for the app to have hundreds of bookings and meetingId (event ids) so it's no big deal for now when index isn't present.