sst / kysely-data-api

79 stars 21 forks source link

Support for JSONB data types #3

Open jagregory opened 2 years ago

jagregory commented 2 years ago

Hey, thanks for this library, it's super promising. I've been using it for schema changes so far, but when I attempted to use it for an actual insert query I hit an issue with my JSONB columns. Is there a way to use JSONB at the moment or is it unsupported?

~I had a poke around (after receiving a wtf error from here šŸ˜…) and if I hacked the serialization for object I could make it work, though I'm not sure if it's actually the right way to do it:~

case "object":
  if (Buffer.isBuffer(value))
    return {
      blobValue: value,
    };
  else
    return {
      stringValue: JSON.stringify(value);
    };

~I also wondered about the typeHint property for the SqlParameter, which can be set to JSON to indicate the stringValue contains JSON. I didn't actually do this and it all worked fine anyway, but perhaps the serialize could be updated to return a SqlParameter with the hint rather than just the value itself.~

Edit: Ignore the above, it wasn't actually working against a real database, just locally. Ooops.

I'm interested to hear your thoughts on this. I'm happy to send a PR if you can give me some direction.

thdxr commented 2 years ago

Hey sorry I didn't see this until now, are you still having this issue? We're using json columns currently

fbergeret95 commented 2 years ago

From this discord conversation (note: I'm not the one who found this, credits to steebee#5616):

this seems related to the overall typeHint problem in kysely-data-api. It seems like the RDS documentation says that strings that have a specific encoding need to have a typeHint provided: https://docs.aws.amazon.com/rdsdataservice/latest/APIReference/API_SqlParameter.html

If I change this line in the query compiler:

return {
  value: {
      stringValue: JSON.stringify(value),
  }
};

To this everything works:

return {
  typeHint: "JSON",
  value: {
      stringValue: JSON.stringify(value),
  }
};

As he pointed out there is this PR: #5 pending to merge. Maybe I could give it a try later or perhaps @thdxr wants to give it more thought

kirylan commented 1 year ago

Currently this fix worked for me

dan-turner commented 1 year ago

Yep, also using the fix mentioned here. A similar solution was required for uuid columns.

https://github.com/kysely-org/kysely/issues/209#issuecomment-1306187965

export const json = <T>(value: T): RawBuilder<string> => {
  return sql`CAST(${JSON.stringify(value)} AS JSONB)`;
};

export const uuid = (value: string): RawBuilder<string> => {
  return sql`CAST (${value} AS UUID)`;
};

export const create = async (id: string, payload: unknown) => {
  const [result] = await DB.insertInto("event")
    .values({
      id: uuid(id),
      payload: json(payload),
    })
    .returningAll()
    .execute();
  return result;
};

Combined with this for deserialising on the way out:

https://github.com/kysely-org/kysely/blob/db8999b64af504eafd8a84ce9f28954bb5a7e15a/src/plugin/parse-json-results/parse-json-results-plugin.ts#L24

export const DB = new Kysely<Database>({
  dialect: new DataApiDialect({
    mode: "postgres",
    driver: {
      secretArn: RDS.db.secretArn,
      resourceArn: RDS.db.clusterArn,
      database: RDS.db.defaultDatabaseName,
      client: new RDSData({}),
    },
  }),
  plugins: [new ParseJSONResultsPlugin()],    // <-- Adding this parses JSONB columns in result sets
});

NOTE The ParseJSONResultsPlugin was only introduced in 0.26.0 which resulted in me encountering this issue: https://github.com/sst/kysely-data-api/issues/28

Workaround was to downgrade to 0.25.0 and copy/paste the ParseJSONResultsPlugin plugin in to my own codebase for now...