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

Better support for non-primitive types #94

Closed
ivan-kleshnin opened this issue Sep 4, 2019 · 4 comments
Closed

Better support for non-primitive types #94

ivan-kleshnin opened this issue Sep 4, 2019 · 4 comments

Comments

@ivan-kleshnin
Copy link

ivan-kleshnin commented Sep 4, 2019

What I find odd about this (overall great) library is that, while it's built on top of PG, it sometimes feels even more low-level.

For example, when you read a timestamp field, PG returns a Date object while Slonik returns a number:

let PG = require("pg")
let {sql, createPool} = require("slonik")

let pgPool = new PG.Pool({
  host: "localhost",
  user: "postgres",
  database: "dbname",
})

let slonikPool = createPool("postgres://postgres:@localhost:5432/dbname", {
  idleTimeout: 1
})

pgPool.query(`SELECT "createdAt" FROM "post" LIMIT 1`)
  .then(({rows}) => console.log(rows[0].createdAt)) // 2016-11-02T22:00:00.000Z (date obj)

slonikPool.oneFirst(sql`SELECT "createdAt" FROM "post" LIMIT 1`)
  .then(r => console.log(r)) // 1478124000000 (number)

The same happens when you write a Date object. PG accepts it and Slonik doesn't.

The same happens with JSON objects and arrays, though I remember your concerns about them.

undefined values are problematic (see below).

All that requires to immediately wrap Slonik in an additional layer of data transformations. Preferences may vary, but I personally don't like to import a bunch of helpers for even the simplest maintenance scripts when I've just installed a specialized library.


Probably a topic for another issue but...

undefined values resulting in Unexpected value expression are very hard to debug.
Multiple times already I had to comment out SQL lines and model fields gradually, one by one, to discover where the problem really is. You can say "use TS", but that's not always possible.

In one case I ended up with a loop which replaced all undefineds with nulls because the first gave me obscure parsing errors from Slonik while the latter gave me clear runtime errors from Postgres. Didn't have such problems with PG where undefined and null are simply treated in the same way.

moved to #95

@camflan
Copy link

camflan commented Sep 13, 2019

I am testing this lib out and am having the same issue with timestamp fields. It appears that this should be working already - it's the goal of the typeParsers, right?

@camflan
Copy link

camflan commented Sep 13, 2019

@ivan-kleshnin ok, I got it... I misread what the the default typeParsers do, which is the opposite of what I (and I think you) want.

You can remove the timestamp(tz) typeParsers when creating the pool so that you get Dates back instead.

here I've removed all default slonik parsers and added the transformer for column names

const pool = createPool(
  `postgres://${username}:${password}@${host}:${port}/${dbname}`,
  {
    interceptors: [
      createFieldNameTransformationInterceptor({
        format: "CAMEL_CASE"
      })
    ],
    typeParsers: []
  }
);

@ivan-kleshnin
Copy link
Author

ivan-kleshnin commented Sep 13, 2019

@camflan thanks man! I confirm that typeParsers: [] disable timestamp conversion. It's strange that it's enabled by default.

Btw. I have all my PG fields in camel case – I find it more bullet-proof with custom (think GraphQL) data filters like id_in – no collisions as db fields have no underscores. I don't write as many SQL queries daily to be bored by extra quotes :)

@gajus
Copy link
Owner

gajus commented Oct 23, 2019

With regards to Date, I will open a separate issue. Beyond the mentioned Date, I don't see other types that would be worth adding special handling of.

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

No branches or pull requests

3 participants