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

Update Index for Ferry Layers #3909

Closed
wants to merge 1 commit into from

Conversation

Nakaner
Copy link
Contributor

@Nakaner Nakaner commented Sep 27, 2019

Pull request #3727 introduced the new condition osm_id > 0 but did not update the condition for the indexes inteded to be used by the SQL queries of these layers.

Pull request gravitystorm#3727 introduced the new condition osm_id > 0 but did
not update the condition for the indexes inteded to be used by the SQL
queries of these layers.
@pnorman
Copy link
Collaborator

pnorman commented Sep 27, 2019

The old index will still work. I wouldn't expect many ways to match the old index but not the one proposed here, so I'd expect the performance difference to be small.

@jeisenbe
Copy link
Collaborator

jeisenbe commented Oct 3, 2019

@pnorman - your comments suggest that this change would make a small but positive improvement in the indexes. Do you approve of this change, or are there problems?

Copy link
Collaborator

@jeisenbe jeisenbe left a comment

Choose a reason for hiding this comment

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

Thank you, this change to the indexes should be an improvement.

@pnorman
Copy link
Collaborator

pnorman commented Oct 19, 2019

Do you approve of this change, or are there problems?

Operational costs of changing indexes

@jeisenbe
Copy link
Collaborator

Should it be merged now, for this release, or do you want this delayed until we have another PR that requires updating indexes? For example, we could merge it to the schema-changes branch.

@jeisenbe
Copy link
Collaborator

@pnorman - is it ok to merge this PR for the next release, or do we need to wait?

@pnorman
Copy link
Collaborator

pnorman commented Nov 25, 2019

I'm not sure why we need it, but if we're going to merge it, now is okay

@pnorman
Copy link
Collaborator

pnorman commented Jan 28, 2020

Let's ship it with the schema changes

@jeisenbe
Copy link
Collaborator

jeisenbe commented Feb 3, 2020

Plan is to merge this PR at the same time the schema_changes branch is merged to master - I hope it will be this month.

@pnorman
Copy link
Collaborator

pnorman commented Feb 13, 2020

I merged this in to schema_changes, so it'll get brought in to master when we merge that.

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.

None yet

3 participants