drizzle-team / drizzle-orm

Headless TypeScript ORM with a head. Runs on Node, Bun and Deno. Lives on the Edge and yes, it's a JavaScript ORM too 😅
https://orm.drizzle.team
Apache License 2.0
23.59k stars 579 forks source link

[BUG]: orderBy causes relational query to fail #1249

Open joshdchang opened 1 year ago

joshdchang commented 1 year ago

What version of drizzle-orm are you using?

0.28.6

What version of drizzle-kit are you using?

n/a

Describe the Bug

Hi Drizzle Team, thank you for making an awesome library! I think I found a bug, and I would love to help pin down what's causing it.

I have a simple schema that represents a file system, with objects and folders.

The schema:

```ts export const folders = mysqlTable("folder", { id: varchar("id", { length: 255 }).notNull().primaryKey(), name: varchar("name", { length: 255 }), parentId: varchar("parentId", { length: 255 }), updatedAt: timestamp("updatedAt", { mode: "date" }).defaultNow().onUpdateNow().notNull(), }); export const foldersRelations = relations(folders, ({ one, many }) => ({ parentFolder: one(folders, { fields: [folders.parentId], references: [folders.id], relationName: "parentFolder", }), childFolders: many(folders, { relationName: "parentFolder", }), childObjects: many(objects), })); export const objects = mysqlTable("object", { id: varchar("id", { length: 255 }).notNull().primaryKey(), name: varchar("name", { length: 255 }).notNull(), contentType: varchar("contentType", { length: 255 }).notNull(), parentId: varchar("parentId", { length: 255 }), updatedAt: timestamp("updatedAt", { mode: "date" }).defaultNow().onUpdateNow().notNull(), }); export const objectsRelations = relations(objects, ({ one, many }) => ({ parentFolder: one(folders, { fields: [objects.parentId], references: [folders.id], }), })); ```

However, when I run the following query using the Query API, I get a runtime error (TypeScript is totally clear and happy).

// this leads to an error at runtime

const folder = await db.query.folders.findFirst({
  where: (folders, { eq }) => eq(folders.id, folderId),
  with: {
    childFolders: {
      orderBy: (childFolders, { desc }) => desc(childFolders.updatedAt),
    },
    childObjects: {
      orderBy: (objects, { desc }) => desc(objects.updatedAt),
    },
    parentFolder: true,
  },
});

Here is the error message (it is a bit unwieldy, most of it is the sql query run by Drizzle)

Internal server error: target: pubflow.-.primary: vttablet: rpc error: code = Unknown desc = Window name 'id' is not defined. (errno 3579) (sqlstate HY000) (CallerID: jhm173senda5tl9p8jx7): Sql: "select id, `name`, parentId, updatedAt, coalesce((select json_arrayagg(json_array(id, `name`, parentId, updatedAt)) from (select folders_childFolders.id, folders_childFolders.`name`, folders_childFolders.parentId, folders_childFolders.updatedAt, row_number() over ( order by folders_childFolders.updatedAt desc) from folder as folders_childFolders where folders_childFolders.parentId = folders.id) as folders_childFolders), json_array()) as childFolders, coalesce((select json_arrayagg(json_array(id, `name`, contentType, parentId, updatedAt)) from (select folders_childObjects.id, folders_childObjects.contentType, folders_childObjects.parentId, folders_childObjects.`name`, folders_childObjects.updatedAt, row_number() over ( id order by folders_childObjects.updatedAt desc) from object as folders_childObjects where folders_childObjects.parentId = folders.id) as folders_childObjects), json_array()) as childObjects, (select json_array(id, `name`, parentId) from (select folders_parentFolder.id, folders_parentFolder.`name`, folders_parentFolder.parentId, folders_parentFolder.updatedAt from folder as folders_parentFolder where folders_parentFolder.id = folders.parentId limit :vtg1 /* INT64 */) as folders_parentFolder) as parentFolder from folder as folders where folders.id = :folders_id /* VARCHAR */ limit :vtg2 /* INT64 */", BindVars: {REDACTED}

However, when I remove the orderBy call on the child folders it all works perfectly (except for the ordering of the child folders, of course). Also, it's only necessary to remove the orderBy on the child folders (a relation to the same table), but not on child objects (which is a different table).

// now this code works!

const folder = await db.query.folders.findFirst({
  where: (folders, { eq }) => eq(folders.id, folderId),
  with: {
    childFolders: {
      // orderBy: (childFolders, { desc }) => desc(childFolders.updatedAt),
    },
    childObjects: {
      orderBy: (objects, { desc }) => desc(objects.updatedAt),
    },
    parentFolder: true,
  },
});

Seems to be an issue with orderBy in self-referential queries.

I'm using the PlanetScale MySQL adapter, but my guess is that it's not an issue with that because the second example works.

Thanks for your help, and please let me know if you have any questions.

Expected behavior

I expected the query to return successfully

Environment & setup

Local development in Node

joshdchang commented 1 year ago

Update:

I got the query to work by adding a where explicitly saying that the folder should have a parentId of the current folderId. However, this shouldn't be necessary because the the schema (included above) already specifies this, and the documentation does not suggest that this would be needed, and it isn't needed for the non self-referential case.

const folder = await db.query.folders.findFirst({
  where: (folders, { eq }) => eq(folders.id, folderId),
  with: {
    childFolders: {
      orderBy: (childFolders, { desc }) => desc(childFolders.updatedAt),
      where: (childFolders, { eq }) => eq(childFolders.parentId, folderId), // adding this makes the query work
    },
    childObjects: {
      orderBy: (objects, { desc }) => desc(objects.updatedAt),
    },
    parentFolder: true,
  },
});

This is a good workaround for now, and I'm sure building this in would not be too difficult.

danielsharvey commented 7 months ago

Unfortunately, the workaround does not work for me. My workaround is to sort out of the database - annoying but not crazy for my application.

This is a good workaround for now, and I'm sure building this in would not be too difficult.

yasseralsaidi commented 2 months ago

same bug any fix?