Batch inserts use "undefined" instead of falling back on default value

[nrathi]

When doing batch inserts (multiple values in a single INSERT), missing values are being replaced with DEFAULT values, but undefined values are not. Single inserts handle both missing and undefined values correctly.

Edit: The original repro wasn't great; the real repro is here: https://kyse.link/0feir

[koskimas]

Can't reproduce this. You can see here that DEFAULT is correctly used https://kyse.link/bNf8s

[koskimas]

By the way, you should never have optional fields in your table interface. Mark nullable columns as nullable. Also mark columns generated by the database using Generated.

interface TestTable {
  id: number
  created_at: Generated<Date> // generated by DB --> use Generated
  optional_field: string | null // nullable, NOT optional
}

Optionality is automatically determined by Kysely.

You should also never use the TestTable as a row type. As explained in the getting started section, you can get the entity types like this:

type Test = Selectable<TestTable> 
// { id: number, created_at: Date, optional_field: string | null }
type NewTest = Insertable<TestTable>
// { id: number, created_at?: Date, optional_field?: string | null }
type TestUpdate = Updateable<TestTable>
// { id?: number, created_at?: Date, optional_field?: string | null }

[nrathi]

Hi,

I agree that I can't replicate in the playground. This is a better example that matches what I have, but the generated SQL when I run it locally uses "undefined" instead of "DEFAULT".

This makes me wonder if there's a missing middleware or a version mismatch or something like that. I am using Postgres 15.4 and Kysley 0.27.4. Could you help me understand what the playground uses?

Furthermore, if I explicitly filter out "undefined" key/value pairs, then it works locally (I get DEFAULT instead of undefined):

 inventoryItemChunk.map(
            (item) =>
              Object.fromEntries(
                Object.entries(item).filter(([_, v]) => v !== undefined),
              ) as any,
          ),

PostgreSQL 15.4 (Debian 15.4-1.pgdg120+1) on aarch64-unknown-linux-gnu, compiled by gcc (Debian 12.2.0-14) 12.2.0, 64-bit

[igalklebanov]

Hey 👋

Can you provide a reproduction repository?

[nrathi]

Sure, do you have a starter repo I can fork?

Here's another interesting thing I'm noticing. I tried to write a test, and I can't get it to to fail. It looks correct just like with the playground. Both the kysley-playground and my testing environment use:

  const db = new Kysely<Database>({
    dialect: {
      createAdapter: () => new PostgresAdapter(),
      createDriver: () => new DummyDriver(),
      createIntrospector: (a) => new PostgresIntrospector(a),
      createQueryCompiler: () => new PostgresQueryCompiler(),
    },
  });

Whereas my actual app uses new PostgresDialect. Do you think it could be something in there? The typeOverries is just this (handling enums, not related to this boolean issue): brianc/node-pg-types#56

  const pool = new pg.Pool({
    host: databaseConfiguration.host,
    port: databaseConfiguration.port,
    user: databaseConfiguration.username,
    password: databaseConfiguration.password,
    database: databaseConfiguration.database,
    ssl: databaseConfiguration.ssl,
    application_name: isMainThread ? "kysely-main" : `kysely-worker`,
    min: 10,
    max: 20,
    types: typeOverrides,
  });
  const dialect = new PostgresDialect({
    pool,
  });
  return new Kysely<Database>({
    dialect,
  });

[nrathi]

To summarize:

The Kysley playground and my own test environment both work as expected. They both use:

const db = new Kysely<Database>({
    dialect: {
      createAdapter: () => new PostgresAdapter(),
      createDriver: () => new DummyDriver(),
      createIntrospector: (a) => new PostgresIntrospector(a),
      createQueryCompiler: () => new PostgresQueryCompiler(),
    },
  });

However, in my app, I have to either:

1. Insert rows 1 by 1 (chunkSize = 1)
2. If batch inserting (chunkSize > 1), strip all key/value pairs where value === undefined

My app uses kysley 0.27.4, Postgres 15.4, and:

  const pool = new pg.Pool({
    host: databaseConfiguration.host,
    port: databaseConfiguration.port,
    user: databaseConfiguration.username,
    password: databaseConfiguration.password,
    database: databaseConfiguration.database,
    ssl: databaseConfiguration.ssl,
    application_name: isMainThread ? "kysely-main" : `kysely-worker`,
    min: 10,
    max: 20,
    types: typeOverrides,
  });
  const dialect = new PostgresDialect({
    pool,
  });
  return new Kysely<Database>({
    dialect,
  });

I have logged the generated SQL for both chunk sizes, and when it works (chunkSize = 1) I see "DEFAULT", but when it doesn't work (chunkSize > 1) I see "undefined". It's the same exact code besides the chunk size.

[igalklebanov]

@nrathi Sure, do you have a starter repo I can fork?

We don't.

Copy over something minimal that 100% reproduces it. Something in your setup (e.g. bundler), dependencies or environment is causing this and I cannot help you if I can't reproduce it on my machine.

[koskimas]

I agree that I can't replicate in the playground.

I also can't replicate this any other way. Our test suite has tests for this case.

kysely/test/node/src/insert.test.ts

Line 689 in 67e413e

it('should insert multiple rows while falling back to default values in partial rows - missing columns', async () => {

[nrathi]

Finally got the repro!!!

https://kyse.link/Z8Pt5

[igalklebanov]

Yeah, that seems like a bug. Thank you! 🙏

[nrathi]

So it seems like if the insert contains any OTHER column that's nullable without a default, that's when the bug hits.

This can be "number | undefined" or "number | null", same bug:

nullable_field: ColumnType<
    number | null,
    number | null,
    number | null
  >;

[nrathi]

Actually, looks like any unrelated undefined value will do it. Another repro: https://kyse.link/0feir

[igalklebanov]

It's probably a bug in the values argument parser - plain JavaScript edge case not being covered.

[naorpeled]

Hi @nrathi,
I'd love to take a crack at this one if you're okay with it! 🙏 But if you'd prefer to work on it yourself, no worries at all—happy to step aside. 😊

[nrathi]

@naorpeled I wasn't taking a crack at all, but perhaps @igalklebanov has already made progress?

[igalklebanov]

I haven't touched this one.

[nrathi]

Thanks for confirming @igalklebanov and thanks for offering to help @naorpeled!

[igalklebanov]

@nrathi you want this one?

[nrathi]

I do not; I lost a few days to debugging already so I was planning on unblocking myself with:

        items.map(
            (item) =>
              Object.fromEntries(
                Object.entries(item).filter(([_, v]) => v !== undefined),
              ) as any,
          ),

Now that I comfortably understand the core issue.

@naorpeled I appreciate you jumping in and offering to take a look!