romeerez/orchid-orm

Query builder crash when selecting non-existent field in subquery

Closed this issue · 3 comments

Steps to reproduce

Run a query with a subquery selecting a non-existent field.

await db.user.select("id", {
  posts: user => user.posts.select("id", "no_such_field"),
})

Expected

The SQL query runs, and fails with database error: column post.no_such_field does not exist.

This is exactly what happens when selecting that field in the root query:

await db.post.select("id", "no_such_field") // proper SQL generated, crashes in Postgres

Alternatively (although less preferred), the query builder crashes with meaningful error, something like "no_such_field is not a field or relation in PostTable".

Actual result

The query builder fails to generate SQL and crashes with cryptic error:

file:///Users/is/test/node_modules/.pnpm/pqb@0.23.5/node_modules/pqb/dist/index.mjs:1844
  return isSubQuery && column.data.name ? setColumnData(column, "name", void 0) : column;
                              ^

TypeError: Cannot read properties of undefined (reading 'data')
    at maybeUnNameColumn (file:///Users/is/test/node_modules/.pnpm/pqb@0.23.5/node_modules/pqb/src/queryMethods/select.ts:493:51)
    at addColumnToShapeFromSelect (file:///Users/is/test/node_modules/.pnpm/pqb@0.23.5/node_modules/pqb/src/queryMethods/select.ts:487:26)
    at getShapeFromSelect (file:///Users/is/test/node_modules/.pnpm/pqb@0.23.5/node_modules/pqb/src/queryMethods/select.ts:423:9)
    at queryFrom (file:///Users/is/test/node_modules/.pnpm/pqb@0.23.5/node_modules/pqb/src/queryMethods/from.ts:114:20)
    at queryWrap (file:///Users/is/test/node_modules/.pnpm/pqb@0.23.5/node_modules/pqb/src/queryMethods/queryMethods.utils.ts:12:5)
    at queryJson (file:///Users/is/test/node_modules/.pnpm/pqb@0.23.5/node_modules/pqb/src/queryMethods/json.utils.ts:16:13)
    at Db.json (file:///Users/is/test/node_modules/.pnpm/pqb@0.23.5/node_modules/pqb/src/queryMethods/json.ts:328:12)
    at processSelectArg (file:///Users/is/test/node_modules/.pnpm/pqb@0.23.5/node_modules/pqb/src/queryMethods/select.ts:302:25)
    at file:///Users/is/test/node_modules/.pnpm/pqb@0.23.5/node_modules/pqb/src/queryMethods/select.ts:517:41
    at Array.map (<anonymous>)

Complete reproduction

Complete reproduction
import process from "node:process"

import { createBaseTable, orchidORM, testTransaction } from "orchid-orm"

const BaseTable = createBaseTable()

class UserTable extends BaseTable {
  readonly table = "user"

  columns = this.setColumns(t => ({
    id: t.identity().primaryKey(),
  }))

  relations = {
    posts: this.hasMany(() => PostTable, {
      required: true,
      columns: ["id"],
      references: ["user_id"],
    }),
  }
}

class PostTable extends BaseTable {
  readonly table = "post"

  columns = this.setColumns(t => ({
    id: t.identity().primaryKey(),
    user_id: t.integer().foreignKey("user", "id").index(),
  }))
}

const db = orchidORM(
  { databaseURL: process.env.DATABASE_URL, log: true },
  {
    user: UserTable,
    post: PostTable,
  },
)

await testTransaction.start(db)

await db.$query`
  create table "user" (
    id serial primary key
  );
  create table post (
    id serial primary key,
    user_id integer not null
  );
`

// SELECT "post"."id", "post"."no_such_field" FROM "post"
// Error: column user.no_such_field does not exist
await db.post.select("id", "no_such_field")

// Doesn't prepare any SQL, crashes with meaningless error
await db.user.select("id", {
  posts: user => user.posts.select("id", "no_such_field"),
})

await testTransaction.close(db)

I appreciate your complete reproduction, though in this case, it's excessive.

It was crashing because of an unknown column before it could execute the query, that's why it was a different error.

I added ? to where the error happens and now it will execute the query and will be a clear db error saying there is no such column, published this fix.

Why wasn't it caught up by TS error?

In general, for the sake of micro-optimizations, when something can be caught by a TS error, I'm trying to keep as few JS checks in the logic as possible. That means there might be more cases like this one, but all should be covered with type-system so shouldn't be a problem.

Why wasn't it caught up by TS error?

It would have been caught if Orchid was used directly in user-land code, however, I'm also using orchid as an underlying query builder engine for the GraphQL query builder (orchid-graphql). I didn't yet come up with full type transparency through the entire GraphQL request-resolver-query builder-response stack (and I'm not sure it's possible at all) so such errors only happen in runtime.

Cool project!

Yeah, the same in my code in this repo - no type-safety in the guts, dirty type-casts everywhere. Definitely not our fault, and I believe not a fault of TS, because library code is dynamic and tricky by its nature, and use cases go beyond what probably any type-system could support.