-
-
Notifications
You must be signed in to change notification settings - Fork 24
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
model.create
should/could needResponse
#21
Comments
You're probably right that it should return the created object... I checked and mongoose, nedb, & linvodb all return the newly created object, so it would make sense for Trilogy to do the same if we want to follow somewhat similar semantics. One thing to note though is that sqlite doesn't support So then we can ask... is that sufficient? Maybe there's a better way of grabbing the last inserted object. edit: or if nothing else, Trilogy could add an Also if we do change this, it would be a breaking change. I'll leave it up for discussion for now on whether it's warranted in this case - I mean, it's an RC but if we're going to hit 1.0 we should hit it with a stable API. I'd rather not jump to 2.0 next week 😄 |
Playing/hacking a bit I got the last inserted id with: // this is inside runQuery()
db.exec("select last_insert_rowid();") |
I'm not able to test right now, does that work with both sqlite & sql.js backends? I'd assume it does since it's a sqlite function but we need to maintain parity between the two. |
Wow, what a pain to install this thing (sqlite3). Giving up for now. |
😆 and that my friends is why Trilogy exists. Thanks for trying! I'll get at it too when I have the time |
Finally the light came and I get this beast working. Yeah, instance.knex.raw('select seq from sqlite_sequence where name="some_table"') Both — sql.js and sqlite3 — work well. |
I can get the newly object created with this: // with sql.js -- hacking inside runQuery()
const created = parseResponse(db.exec('select seq from sqlite_sequence where name="some_table"'));
const info = parseResponse(db.exec('PRAGMA table_info("some_table")'));
// find the primary key name
let key;
info.some(each => {
if (each.pk && each.notnull && each.type === 'integer') {
key = each.name;
return true;
}
});
const obj_ = {};
obj_[key] = created[0].seq;
const doc = instance.findOne(name, obj_);
doc.then(r => console.log(r)); Do you think this can turn into a PR, obviously, after some polish? |
Does that only work for edit: Yep, looks like this is
If Based on what I'm finding there, I think your first solution is the one we should chase. Thoughts? |
@citycide I'm in a tight deadline and will return as soon as possible. |
So @citycide let's try to finish this one. To sumarize a desired PR:
|
@jonataswalker well since neither seem absolute, a hybrid option might be necessary. In this order:
This adds a couple queries but covers as many cases as we can. If we want to avoid the extra queries, we could go back to the original behavior of simply using part 3 from above. It only doesn't work reliably when the schema doesn't have a unique or primary key - and most should. Edited to say reliably because it would always select and return an object meeting those criteria. |
PR #22 created, not sure if you'll like new changes. I'm going to get back tomorrow to finish it. |
Released in v1.0.0.rc-3. |
Right after creating a new object it would be helpful if created object returns.
Much better if
model.create
usereturning('id')
, otherwise how we can get last inserted record?The text was updated successfully, but these errors were encountered: