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

Optimize insert query loading #15404

Merged
merged 2 commits into from
Jun 23, 2017

Conversation

Ladas
Copy link
Contributor

@Ladas Ladas commented Jun 20, 2017

Optimize insert query loading, using a Returning clause properly. This way we can spare another query for loading back the data. We need to load back the inserted data, to get their primary keys.

@Ladas
Copy link
Contributor Author

Ladas commented Jun 20, 2017

@miq-bot assign @agrare

@Ladas
Copy link
Contributor Author

Ladas commented Jun 20, 2017

@miq-bot add_label enhancement

Add Returning clause if the IC has dependees
Use INSERT query result if possible. If we are sure that all
ids of inserted records are in the result, we can use it
and spare time loading the records back from the db.
@Ladas Ladas force-pushed the optimize_insert_query_loading branch from b3be700 to b4330a8 Compare June 20, 2017 13:10
@miq-bot
Copy link
Member

miq-bot commented Jun 20, 2017

Checked commits Ladas/manageiq@74c72f8~...b4330a8 with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
2 files checked, 1 offense detected

app/models/manager_refresh/save_collection/saver/sql_helper.rb

# we test if the number of results matches the expected batch size. Then if the counts do not match, the only
# safe option is to query all the data from the DB, using the unique_indexes. The batch size will also not match
# for every remainders(a last batch in a stream of batches)
if !supports_remote_data_timestamp?(all_attribute_keys) || result.count == batch_size
Copy link
Member

Choose a reason for hiding this comment

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

@Ladas is it possible the count could be the same as the batch size but still not return the "correct" ids?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it should not be, we check for uniqueness at least like on 2 places

@agrare agrare merged commit b4330a8 into ManageIQ:master Jun 23, 2017
agrare added a commit that referenced this pull request Jun 23, 2017
@agrare agrare added this to the Sprint 64 Ending Jul 3, 2017 milestone Jun 23, 2017
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.

3 participants