Int8 as string

[TheNickmaster21]

Updated the types to set int8 columns as strings files to match the default behavior of the pg library.

[jawj]

Thanks, but this is complicated slightly by the fact that int8 values are represented as numbers when they come via JSON, as is the case with all the shortcut functions. So it will take a little more thought to fix ...

// CREATE TABLE int8test (num int8);

await (async () => {
  console.log('\n=== PR #68 ===\n');

  const insresult = await db.insert('int8test', { num: 12 }).run(pool);
  console.log(typeof insresult.num, insresult.num);  // number 12

  const selresult = await db.selectOne('int8test', db.all).run(pool);
  console.log(typeof selresult!.num, selresult!.num);  // number 12

  const manresult = await db.sql<s.int8test.SQL, s.int8test.Selectable[]>`SELECT * FROM ${"int8test"}`.run(pool);
  console.log(typeof manresult[0].num, manresult[0].num);  // string 12 -- but wrongly typed as number

  const jsonresult = await db.sql<s.int8test.SQL, { result: s.int8test.JSONSelectable[] }[]>`SELECT jsonb_agg(i.*) AS result FROM ${"int8test"} i`.run(pool);
  console.log(typeof jsonresult[0].result[0].num, jsonresult[0].result[0].num); // number 12
})();

[TheNickmaster21]

I think it's more important for the default SQL typings to be accurate, as they currently are not, than to account for a special condition where the return of the query is explicitly cast through a extraneous function.

However, as a solution, would you consider having the types of int8 fields be exported as a union of 'string' and 'number' to account for both situations? This seems like it might take some changes to how the files are generated but I'd be willing to look into it.

[jawj]

I think it's more important for the default SQL typings to be accurate, as they currently are not, than to account for a special condition where the return of the query is explicitly cast through a extraneous function.

My own use of the library is heavy on the shortcuts, so I wouldn't call that a special condition. But I certainly agree that having accurate types is important in both cases.

Having int8 fields be string | number everywhere is a possible workaround, but could cause a lot of annoying casting, so I'll see if I can treat the type separately for Selectable and JSONSelectable.

I guess for Insertable, Updatable and Whereable, string | number is probably fine.

[TheNickmaster21]

As I am starting to use the library more, I do see what you mean about the shortcuts. I am now having an issue where I am selecting from a table and a Date field has the string type. I believe I understand why, it's the JSON representation of the table actually being selected, but that means I have to update the type signature of my method to be for the JSON representation of the table which is hardly ideal. For my own fork, I will likely change the date field type to be Date | string but I can certainly see how that only further complicates this situation.

Do you happen to have an example repository using this library? I feel like the issues I am running into are fairly significant parts of how our code currently works and that is likely due to a paradigm shift between how zapatos is meant to be used and how we did our querying before.

[jawj]

The original project the library grew out of is proprietary, unfortunately, so all I can share for now is the examples in the docs (of which there are quite a lot), and those in the zapatos-demo repo (which is kind of my scratch space for trying things out and informal testing).

It's true that date columns coming to TS as strings is a bit of a pain. This might be the kind of thing that reflect-metadata could help with — see #61.

[jawj]

I haven't forgotten this, but I haven't got much free time at present, and this requires a bit of time for a small but fundamental refactor of the types generation (to allow for the TS type mapping to depend on context — in this case, whether we're going via JSON or not).

[TheNickmaster21]

I completely understand. For the time being, my project is just using my own fork of zapatos with this change.

[jawj]

OK, I think the changes now in master should have fixed this issue properly. int8 values should appear as db.Int8String (an alias of string) in a Selectable, number in a JSONSelectable, and number | db.Int8String everywhere else.

timestamp and related types get similar treatment (replacing the slightly hacky post-processing with JSONSelectableFromSelectable<Selectable> that was done before).

Are you able to test that this all works for you? To use master from your project you'll need to clone the repo, npm install, npm run build, and npm link, then in your project directory run npm link zapatos.

[TheNickmaster21]

I've been busy but I can give this a try tonight. Thank you!

[jawj]

@TheNickmaster21 Are we good to close this?

[TheNickmaster21]

Very sorry; work has been chaos. I can actually try to test it today.

[TheNickmaster21]

So far, it seems to work fine. I had some type mismatches from my own approach. I was using .Selectable interfaces in places that now need to use JSONSelectableForTable interfaces but other than that, it seems good to go. I just wish there was a more generic interface I could use for the table that matches for the JSONSelectableForTable or the Selectable or the Updatable. But for now, this should suffice! Thank you very much.

[jawj]

Great. This is now released in 3.5.

[johannesfritsch]

@jawj You're doing a great job with zapatos. Just wanted to let you know :-)