Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Top-level type parser #315

Open
kyscott18 opened this issue Sep 12, 2024 · 4 comments
Open

Top-level type parser #315

kyscott18 opened this issue Sep 12, 2024 · 4 comments

Comments

@kyscott18
Copy link

It would be great to be able to set a custom type parser for postgres data types at an instance level in addition to the query level. I'd be happy to work on this pr, but wanted to get some input on how it should look first.

API

import { PGlite, types } from '@electric-sql/pglite';

const pg = await PGlite.create({
 parsers: {
    [types.TEXT]: (value) => value.toUpperCase(),
  },
});

// or 

const pg = await PGlite.create();
pg.setParser(types.TEXT, (value) => value.toUpperCase());

It should probably be one or the other, which one do you think fits in best?

Relevant examples

node-postgres

var types = require('pg').types;
types.setTypeParser(20, function(val) {
  return parseInt(val, 10)
});

drizzle-orm

const users = pgTable('users', {
  // set custom column type with ".$type"
  id: serial('id').$type<UserId>().primaryKey(),
});
@Ericnr
Copy link

Ericnr commented Jan 12, 2025

this is very important for people trying to use PgLite for testing. We need to be able to match the way PgLite parses with the production driver

@jamesarosen
Copy link

const pg = await PGlite.create({
 parsers: {
    [types.TEXT]: (value) => value.toUpperCase(),
  },
});

One reason not to choose this approach is that whoever is creating the PGlite instance needs to know about type parsers and why they might override them:

// test-setup.ts
const client = await PGlite.create({
 parsers: {
    [types.TEXT]: (value) => value.toUpperCase(),
  },
});
const db = drizzle({ client });

For this reason, I recommend

pg.setParser(types.TEXT, (value) => value.toUpperCase());

or even mimic the node-postgres API directly given how popular that project is:

pg.setTypeParser(types.TEXT, (value) => value.toUpperCase());

That way, libraries like Drizzle can use the API to work around incompatibilities without having to push the work to the application developer.

@jadejr
Copy link

jadejr commented Feb 6, 2025

Doesn't this already exist? https://pglite.dev/docs/api or am I misunderstanding? It doesn't seem to have an instance method though as suggested. If that is what is really wanted, then the issue title should be adjusted.

@jamesarosen
Copy link

Great callout, @jadejr!

And although there's no instance method, there is a public setter. This fixes the issue with Drizzle:

const pglite = await PGlite.create({...})
const db = drizzle({ client });

// Fixes https://github.com/drizzle-team/drizzle-orm/issues/3997
client.serializers[types.JSON] = (value) => value;
client.serializers[types.JSONB] = (value) => value;

but this does not:

const pglite = await PGlite.create({
  serializers: {
    [types.JSON]: (value) => value,
    [types.JSONB]: (value) => value,
  }
})
const db = drizzle({ client })

because Drizzle (incorrectly) overrides the parsers. That's a Drizzle bug, but it shows the importance of flexibility here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants