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

Return inserted ids MySQL #279

Closed
wants to merge 1 commit into from

Conversation

robertomiranda
Copy link

@robertomiranda robertomiranda commented May 30, 2016

ref #94

[number_of_inserts, []]
if first_inserted_id
last_inserted_id = first_inserted_id + number_of_inserts - 1
ids = (first_inserted_id..last_inserted_id).to_a
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@zdennis what do you think about this approach?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@robertomiranda, thank you for contributing to ar-import! With regard to your question above I have the same concerns as @tonic20 and @jkowens. I'd want to ensure that we however this implemented we can guarantee reliability and avoid unsuspecting issues when the returned IDs are wrong.

@jkowens
Copy link
Collaborator

jkowens commented May 31, 2016

The database table would need to be InnoDB with innodb_autoinc_lock_mode set to 0 or 1 (default) to guarantee sequential id values for batch inserts. If it is set to 2 (interleaved) or is MyISAM, sequential ids cannot be guaranteed.

else
value_sets = ::ActiveRecord::Import::ValueSetsBytesParser.parse(values,
reserved_bytes: sql_size,
max_bytes: max)
value_sets.each do |value_set|
number_of_inserts += 1
sql2insert = base_sql + value_set.join( ',' ) + post_sql
insert( sql2insert, *args )
ids << insert( sql2insert, *args )
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will result in missing ids because this is a batch operation.

@philipgiuliani
Copy link

philipgiuliani commented Jun 29, 2016

Would be nice to have this feature. It will work fine for all users with INNODB + innodb_autoinc_lock_mode set to 0 or 1. Of course we'll have to mention that somewhere.

@philipgiuliani
Copy link

This will also not work when there are duplicates and you use on_duplicate_key_update, right? Than it returns wrong id's probably.

@jkowens
Copy link
Collaborator

jkowens commented Oct 9, 2016

@philipgiuliani you're correct, this would not work with on_duplicate_key_update.

@jkowens
Copy link
Collaborator

jkowens commented Oct 25, 2016

I'm going to close this PR for now because it doesn't seem like MySQL provides a good solution for this problem at the moment. With the need to support ON DUPLICATE KEY UPDATE and with UUID keys gaining momentum, I think we'll have to wait and hope MySQL eventually returns IDs like PostgreSQL.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants