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

Returning last inserted id #112

Closed
vladfaust opened this issue Sep 21, 2017 · 10 comments
Closed

Returning last inserted id #112

vladfaust opened this issue Sep 21, 2017 · 10 comments

Comments

@vladfaust
Copy link
Contributor

Refer to

last_insert_id: 0_i64 # postgres doesn't support this

But there is an option with RETURNING; maybe this could be implemented?

@will
Copy link
Owner

will commented Sep 21, 2017

I'm not sure how the driver here would know to add RETURNING automatically, because not every statement is an insert, and not every id column is necessarily called "id"

Also, it looks like it's used in this struct https://github.com/crystal-lang/crystal-db/blob/385cf70a8a5d41374cce988168e82495743ee459/src/db.cr#L80 But I often have tables where the ID is not an Int64, often they're UUIDs or something else, so I'm not sure it's generalizable at all. This might be something that comes from mysql or something?

@vladfaust
Copy link
Contributor Author

@will I agree that last_insert_id : Int64 is idiomatically wrong. Maybe it worths opening an issue in crystal-db? Like, change to Int64 | String?

I'm not sure, but aren't you able to check whether is RETURNING present in a query? If yes, maybe parse the first value... It add some covenants, though.

@vladfaust
Copy link
Contributor Author

rows_affected equals to 0 too.

@nicferrier
Copy link

nicferrier commented Dec 29, 2017

I can't even get RETURNING to work, the code gets tangled up. It looks like support was there in the frame.cr code once (DataRow) but it doesn't work now.

Other issues report that db.scalar works but it doesn't with the latest release - possibly because of the breaking crystal change that broke the rest?

Below is the attempted fix with a PR #121

nicferrier added a commit to nicferrier/crystal-pg that referenced this issue Dec 30, 2017
@vladfaust
Copy link
Contributor Author

So, could anyone make RETURNING work?

@RX14
Copy link
Contributor

RX14 commented May 26, 2018

RETURNING has always worked for me

@vladfaust
Copy link
Contributor Author

vladfaust commented May 28, 2018

Hm, yes. It works if doing db.query("INSERT ... RETURNING *")... So the issue regrads to ExecResult, which has these fields always nil. It's Crystal::DB architectural failure, I suppose?

@RX14
Copy link
Contributor

RX14 commented May 28, 2018

Thats just how postgres works. ExecResult is useless for postgres. What if your primary key wasn't an Int64?

ExecResult should probably be removed as too db-specific

@vladfaust
Copy link
Contributor Author

Yep. Closing this one then.

@waghanza
Copy link
Contributor

Does RETURNING works for exec ? or only for query ?

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

5 participants