search

ropensci / dittodb

dittodb: A Test Environment for DB Queries in R
https://dittodb.jonkeane.com/
Other
82 stars 15 forks source link

possible bug: bigint type from postgres not captured corectly #165

Closed majazaloznik closed 1 year ago

majazaloznik commented 2 years ago

It seems that the bigint type for postgres is not captured correctly in the fixtures.

Instead of the correct response (which R parses as int64), I am getting empty mocks.

A workaround is to transform the data type in the query already e.g. CAST("id" AS NUMERIC) AS "id" which however presumably only works up to a point...

jonkeane commented 2 years ago

Ah indeed that could be an issue. Are you using RPostgres as your driver? It looks like that's using {bit64} to represent large integers, and I suspect those aren't being roundtripped in the dput() output.

And of course if you're interested in submitting a PR, I would be happy to review + provide some pointers. If not, I can try and take a stab at it myself.

majazaloznik commented 1 year ago

Sorry for dropping off, yes I am using RPostgres as the driver. I'm afraid I wouldn't know where to begin with a PR unfortunately :)

I've now just been fixing the fixtures manually...

I also manually fix the fixtures for with another problem I have with non-standard characters such as č, š, ž, which when returned from the db either work on my machine and don't work on git hub actions, or vice versa. So I'm glad you mentioned dput() above, since that gave me the clue that I had to use ASCII characters only, so I now manually change them to \u0161 or whatever.

(Actually that might also be useful to mention somewhere in the documentation)

jonkeane commented 1 year ago

Ok, lemme see if I can take a stab at a PR for the bigint issue.

So I'm glad you mentioned dput() above, since that gave me the clue that I had to use ASCII characters only, so I now manually change them to \u0161 or whatever.

(Actually that might also be useful to mention somewhere in the documentation)

Definitely agreed on that — would you be interested in make a PR to add those to the docs (or at least a separate issue for that...)?

jonkeane commented 1 year ago

Ok, I just put up a PR that I think will address this bug. Would you mind giving it a try and letting me know if it resolves your issue here?

majazaloznik commented 1 year ago

I coded so defensively around this, I can barely recreate the original issue 🤣 , but looks to me that the mocks where bigint values should be returned are working correctly and returning integer64s, thanks!

I'll have a look at the docs and try to articulate what I think is the issue, otherwise I'll open an new issue, cheers!

jonkeane commented 1 year ago

Thanks! I might cut a release soon since there's this + a handful of other issues that would be good to have on CRAN too.

majazaloznik commented 1 year ago

That would be great!