Skip to content

Support bigint#126

Merged
cnathe merged 5 commits into
LabKey:developfrom
bbimber:fb_bigint
Jun 29, 2026
Merged

Support bigint#126
cnathe merged 5 commits into
LabKey:developfrom
bbimber:fb_bigint

Conversation

@bbimber

@bbimber bbimber commented May 20, 2026

Copy link
Copy Markdown
Contributor

This is related to this ticket: https://www.labkey.org/ONPRC/Support%20Tickets/issues-details.view?issueId=54682

I dont recall how LabKey does testing on RLabkey; however, it would be a good idea to add a test case with some kind of large integer (e.g., 3.157524392E9) and SelectRows.

Comment thread Rlabkey/DESCRIPTION Outdated
Comment thread Rlabkey/R/makeDF.R Outdated
@bbimber

bbimber commented Jun 25, 2026

Copy link
Copy Markdown
Contributor Author

@cnathe: I think the current changes should pass for you. the query API tests are working locally for me.

@cnathe

cnathe commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

@cnathe: I think the current changes should pass for you. the query API tests are working locally for me.

Yes, that works much better. I think this approach to "only use bigint when needed" makes sense for this first pass. Thanks.

@cnathe

cnathe commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

I'm going to get this merges so that I can create a new Rlabkey version with this change and the other change that was just merged to develop last week.

@cnathe cnathe merged commit 9eb0907 into LabKey:develop Jun 29, 2026
@bbimber

bbimber commented Jul 6, 2026

Copy link
Copy Markdown
Contributor Author

Hey @cnathe: Having used this a little more, I want to point out some suboptimal behaviors in R. I dont think there is a perfect solution, since R doesnt suport 64-bit integers well (as far as I can see).

Take this simple example:

library(bit64)

int_vec <- as.integer64(c(1,2,3,4))

# This loop returns the following, rather than 1-4:
#[1] 4.940656e-324
#[1] 9.881313e-324
#[1] 1.482197e-323
#[1] 1.976263e-323
for (y in int_vec) {
  print(y)
}

# This returns the correct values
lapply(int_vec, function(x) {
  print(x)
})

There doesnt seem to be a good solution for this problem. Because the final version of this PR only uses integer64 when necessary, the impact of this problem is scoped only to situations that would have failed in the prior version anyway. Without integer64, all large integers would be reported as NAs, so neither approach is seamless.

I'm not advocating changes now, but I think if this becomes a problem, we could drop bit64::integer64 and use R's numeric() class:

return(bit64::as.integer64(x))

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

Successfully merging this pull request may close these issues.

2 participants