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

Datatypes for nodes table #70

Closed
mooreryan opened this issue Dec 21, 2024 · 2 comments
Closed

Datatypes for nodes table #70

mooreryan opened this issue Dec 21, 2024 · 2 comments

Comments

@mooreryan
Copy link

I was going through the taxonomizr code and I noticed that while the taxonomy IDs are generally treated as integers (eg in the accessionTaxa.taxa column, names.id column, and nodes.id column), in the nodes table, the parent column uses as.numeric (here) and the resulting sqlite datatype becomes REAL rather than INTEGER (as observed when I dump the schema).

I'm wondering if REAL was chosen for that datatype rather than INTEGER for a specific reason?

I haven't checked to see if it makes a performance difference on a join or not, and in fact I'm not even sure if sqlite would do any conversion between REAL and INTEGER before comparing (see 4.2. Type Conversions Prior To Comparison in https://www.sqlite.org/datatype3.html).

Anyway, I'm curious about your thoughts on it.

@sherrillmix
Copy link
Owner

Good catch. I don't think any special reason to be REAL and it just slipped by. The code has been working so some conversion must be going on in the background hence why this wasn't noticed. For example, sqlite does seem to compare INT and REAL without erroring out e.g.:

CREATE TABLE A (a int, b real);  
INSERT INTO A(a,b) VALUES (4,4);
INSERT INTO A(a,b) VALUES (4,4.0);
INSERT INTO A(a,b) VALUES (4,3.999999);
INSERT INTO A(a,b) VALUES (4,4.000001);
SELECT * FROM A WHERE a=b;

returning:

4|4.0
4|4.0

Might as well fix it since it seems an easy change. Should be updated on github now and I'll push to CRAN after the holidays. Thanks!

@sherrillmix
Copy link
Owner

The package should be on its way to CRAN now. Thanks!

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

2 participants