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.

Sign up for GitHub

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

Jump to bottom

Accelerated rock search - PLEASE UPDATE YOUR SECTORS #279

Merged
merged 2 commits into from
Dec 21, 2021
Merged

Accelerated rock search - PLEASE UPDATE YOUR SECTORS #279

merged 2 commits into from
Dec 21, 2021

Conversation

paetz
Copy link
Collaborator

@paetz paetz commented Dec 20, 2021

Resolves #276

chrgernoe
chrgernoe requested changes Dec 20, 2021
View reviewed changes
Copy link
Member

@chrgernoe chrgernoe left a comment

Choose a reason for hiding this comment

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

Why is the Unicode character fast? Please add a code comment.

@chrgernoe
Copy link
Member

I tried the rock search before the patch. I don't see a speed issue? What are the conditions?

@paetz
Copy link
Collaborator Author

paetz commented Dec 20, 2021 •
edited
Loading

Why is the Unicode character fast? Please add a code comment.

What do yo mean by "fast"? "\u0000" (the old/current separator between german and czech name in our db) is interpreted as some error case in sql, hence every name part behind this marker can not be detected by sql command LIKE

@paetz
Copy link
Collaborator Author

paetz commented Dec 20, 2021 •
edited
Loading

I tried the rock search before the patch. I don't see a speed issue? What are the conditions?

I did some timing measurements:
For instance, if only Sächsische and Böhmische Schweiz are locally available, global rock search takes approx.

  • 100-200ms with the old approach
  • ~10ms with the new approach

paetz
paetz commented Dec 20, 2021
View reviewed changes
docs/release-notes/v0.5.0.md Outdated
@@ -0,0 +1,12 @@
## Important notice

* To ensure correct display of rocks/routes and a working rock/route search,
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Should be in German, I guess

Copy link
Member

Choose a reason for hiding this comment

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

Normally we do all the release notes in English but this case is a bit special. German is also fine.

@chrgernoe
Copy link
Member

Why is the Unicode character fast? Please add a code comment.

What do yo mean by "fast"? "\u0000" (the old/current separator between german and czech name in our db) is interpreted as some error case in sql, hence every name part behind this marker can not be detected by sql command LIKE

Understand. Please at this as comment to the code.

@paetz paetz requested a review from chrgernoe December 21, 2021 09:43
@paetz paetz merged commit 06f25cd into master Dec 21, 2021
@paetz paetz deleted the feature/276_accelerate_rock_search_-_PLEASE_UPDATE_YOUR_SECTORS branch December 21, 2021 10:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@chrgernoe chrgernoe chrgernoe approved these changes

@elmogit elmogit Awaiting requested review from elmogit

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Accelerate rock search
2 participants
@paetz @chrgernoe