Skip to content

avoid tag-translating in filter optimizer, fixes crash when filters contains a non-existent tag #363

Merged
tyrasd merged 5 commits into
masterfrom
fix-filtering-nonexistant-tags
Mar 23, 2021
Merged

avoid tag-translating in filter optimizer, fixes crash when filters contains a non-existent tag #363
tyrasd merged 5 commits into
masterfrom
fix-filtering-nonexistant-tags

Conversation

@tyrasd
Copy link
Copy Markdown
Member

@tyrasd tyrasd commented Mar 23, 2021

Description

A oshdb filter goes through some optimization steps for increased performance, e.g. for simple key=value queries, it will use a more efficient code path than for example a filter like key1=value1 or key2=value2. The "fast" path crashed, when a non-existent tag is contained in the filter: The code tries to convert the tag from the OSHDB-internal representation back to strings. This normally works fine, but not for tags which don't exist.

This fixes the bug by replacing the conversion from tag ids to strings by a newly added private method for using tag ids directly for filtering.

This PR also fixes a few minor typos (entitiy -> entity) and refactors some of the osmTag filters slightly (to also use the new helper method and be more similar to each other).

Corresponding issue

Closes GIScience/ohsome-api#154

Checklist

@tyrasd tyrasd added the bug Something isn't working as expected label Mar 23, 2021
@tyrasd tyrasd added this to the release 0.7.0 milestone Mar 23, 2021
@tyrasd tyrasd changed the title avoid tag-translating in filter optimizer, fixes crash when filters contains a non-existent tag 🚧 avoid tag-translating in filter optimizer, fixes crash when filters contains a non-existent tag Mar 23, 2021
@tyrasd tyrasd added the comments welcome Indicates that the creator of this issue/PR is open for early review comments label Mar 23, 2021
@tyrasd tyrasd force-pushed the fix-filtering-nonexistant-tags branch 4 times, most recently from 205aaac to f7ed1ca Compare March 23, 2021 11:39
@tyrasd tyrasd changed the title 🚧 avoid tag-translating in filter optimizer, fixes crash when filters contains a non-existent tag avoid tag-translating in filter optimizer, fixes crash when filters contains a non-existent tag Mar 23, 2021
@tyrasd tyrasd added waiting for review This pull request needs a code review and removed comments welcome Indicates that the creator of this issue/PR is open for early review comments labels Mar 23, 2021
Copy link
Copy Markdown
Member

@joker234 joker234 left a comment

Choose a reason for hiding this comment

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

So basically, apart from some reformatting and prettyfying the code, the change is to use OSHDBTag and OSHDBTagKey directly instead of translating it to OSMTag or OSMTagKey objects first? This prevents the translator to fail and we can return an empty response?

Comment thread oshdb-api/src/main/java/org/heigit/ohsome/oshdb/api/mapreducer/MapReducer.java Outdated
@tyrasd
Copy link
Copy Markdown
Member Author

tyrasd commented Mar 23, 2021

So basically […] the change is to use OSHDBTag and OSHDBTagKey directly instead of translating it to OSMTag or OSMTagKey objects first? This prevents the translator to fail and we can return an empty response?

Exactly.

joker234
joker234 previously approved these changes Mar 23, 2021
tyrasd added 4 commits March 23, 2021 20:00
to avoid deprecation warning where it is used internally, and will not be removed from the class
@tyrasd tyrasd force-pushed the fix-filtering-nonexistant-tags branch from f7ed1ca to 61ed94a Compare March 23, 2021 19:01
@tyrasd tyrasd force-pushed the fix-filtering-nonexistant-tags branch from 456f850 to 8ff4de5 Compare March 23, 2021 19:10
@tyrasd tyrasd merged commit e54fb5f into master Mar 23, 2021
@tyrasd tyrasd deleted the fix-filtering-nonexistant-tags branch March 23, 2021 19:17
tyrasd added a commit that referenced this pull request Mar 24, 2021
@tyrasd tyrasd removed the waiting for review This pull request needs a code review label Mar 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working as expected

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Query crashes when using non-existent tag in filter

2 participants