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

Apply bbox to part of "addresses" query #3942

Merged
merged 1 commit into from
Oct 23, 2019

Conversation

nexces
Copy link

@nexces nexces commented Oct 21, 2019

Adding parenthesis around additional conditions allows to use index and prevents from selecting all rows from DB containing "addr:housename" or "addr:unit".

Fixes #3937

Changes proposed in this pull request:

  • Adding parenthesis around additional conditions

Test rendering with links to the example places:
No changes in rendering

Adding parenthesis around additional conditions allows to use index and prevents from selecting all rows from DB containing "addr:housename" or "addr:unit".
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 @nexces. I have tested and approve these changes. It's hard to see the performance difference with a moderate-sized extract (Latvia.osm.pbf) but I suspect it would be more significant with a larger extract or whole planet file.

@nexces
Copy link
Author

nexces commented Oct 22, 2019

You can perform further checks with pure query grabbed from PostgreSQL slow query log. Run "EXPLAIN" on it and you will find how costs are distributed. Also original query returns points that are outside of bounding box which is irrelevant for rendering but has massive impact on data gathering. For Europe+Russia extract data collection went from ~45 seconds to 0.4 seconds

@jeisenbe
Copy link
Collaborator

Thank you for that advice (I need to learn more about PostgreSQL), and for submitting for this PR.

It would be very helpful if we had an automated or semi-automated way to test performance, to prevent bugs like this in the future. There's an open issue #1287 - perhaps you have an idea about solving this?

@jeisenbe jeisenbe merged commit e66889e into gravitystorm:master Oct 23, 2019
@nexces nexces deleted the fix-3937 branch June 5, 2020 11:09
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.

Part of query is not restricted by bounding box
3 participants