PhilWaldmann / openrecord

Make ORMs great again!
https://openrecord.js.org
MIT License
486 stars 38 forks source link

ID conversion problem #97

Closed idma88 closed 4 years ago

idma88 commented 5 years ago

Hi! I use the String type for IDs in the sqlite3 database (because JavaScript does not support BIGINT). When use such code, get the wrong record.

var res = await store.Model ('Guild'). find ("583946876706095100");
console.log (res);

<Guild {id: "411600256049086460" cmdPrefix: "+"}>

Note that ID is different.

If change the ID so that they begin NOT with a number (for example, "_583946876706095100"), then everything works as intended.

I tried to add to the model

static definition () {
   this.attribute ('id', String);
}

but that did not solve the problem.

PhilWaldmann commented 5 years ago

Hi @idma88 ,

please start your test with env DEBUG=openrecord:* which should log all queries. What query will be executed with the false . find("583946876706095100") ?

What's your table schema? Is the id field defined as primary?

idma88 commented 5 years ago

Hi @PhilWaldmann

I have a table:

CREATE TABLE guilds (
     id STRING PRIMARY KEY
               UNIQUE
               NOT NULL,
     cmdPrefix STRING
);

with two lines

id cmdPrefix
411600256049086460 +
583946876706095100 null

Next, I set DEBUG=openrecord:* and execute .find("583946876706095100") as you asked. The following line appears in the console:

openrecord:exec [0.56ms] select * from `guilds` where 583946876706095100 limit 1 +0ms

As I wrote earlier, I get an entry with a different id

<Guild {id: "411600256049086460" cmdPrefix: "+"}>
PhilWaldmann commented 5 years ago

The id column is not configured right. Did you turn off autoAttributes? The sql query should be select * fromguildswhere id = 583946876706095100 limit 1. If autoAttributes is tuned off, you need to configure your fields manually.

this.attribute('id', 'string', {primary: true, persistent: true, notnull: true})
idma88 commented 5 years ago

No, I have not disabled autoAttributes (using default behavior). If I force set autoAttributes to true, then the problem is solved. I also tried to add the code you wrote. This solves the problem.

I still do not understand why the column “id” was not defined as “main”. autoAttributes is by default set totrue?

PhilWaldmann commented 5 years ago

interesting! openrecord will check if autoAttributes is equal false. So setting it to true shouldn't make any difference.

It would be awesome if you could write a small test case (singe .js + .sqlite3 file) and attach it to this issue. So that I can investigate this issue.

Thanks, Philipp

idma88 commented 5 years ago

Example as you requested

sample.zip

PhilWaldmann commented 5 years ago

Thanks!

I've tested your code with node v10.13.0 and v8.12.0, openrecord@2.10.0 and sqlite3@4.0.8 . I got the same result with or without autoLoad: true

$ DEBUG=openrecord:* node index.js 
<Guild [id, cmdPrefix]>
  openrecord:exec [4.78ms] select * from `guilds` where `guilds`.`id` = '411600256049086494' limit 1 +0ms
[ itemA ]   <Guild {id:"411600256049086460" cmdPrefix:"+"}>
  openrecord:save update `guilds` set `cmdPrefix` = '-' where `id` = '411600256049086460' +0ms
  openrecord:transaction commit +0ms
  openrecord:exec [0.87ms] select * from `guilds` where `guilds`.`id` = '411600256049086494' limit 1 +14ms
[ itemA ]   <Guild {id:"411600256049086460" cmdPrefix:"+"}>
  openrecord:exec [0.51ms] select * from `guilds` where `guilds`.`id` = '583946876706095100' limit 1 +2ms
[ itemB ]   <Guild {id:"583946876706095100" cmdPrefix:null}>

What versions do you use?

idma88 commented 5 years ago

I use node v10.15.3, openrecord@2.10.0 and sqlite3@4.0.8.

I do not know why my SQL query was not executed correctly, but now it works on my PC.

But still I want to note that by requesting an entry with ID 411600256049086494 I get an entry with ID 411600256049086460. But there is no record 411600256049086460 in the database. So the ID is assigned incorrectly for the JS object.

This behavior is typical for JS when we work with large numbers. But in my case for the ID string is used.

PhilWaldmann commented 5 years ago

Sorry for the late reply! It seems it's a knex bug. The following code automatically converts the id to an integer. I'll need to dig deeper, but will open an issue on the knex project if it's a knex bug.

const knex = require('knex')({
  client: 'sqlite3',
  connection: {
    filename: "./storage.sqlite3"
  }
});

knex('guilds')
.then(result => {
  console.log('>>', result)
})