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

The query_string cache should returned cloned Query instances. #6733

Closed
wants to merge 1 commit into from
Closed

The query_string cache should returned cloned Query instances. #6733

wants to merge 1 commit into from

Conversation

martijnvg
Copy link
Member

The query string cache can't return the same instance, since Query is mutable changing the query else where in the execution path changes the instance in the cache too.

Fixes #2542

@martijnvg
The query string cache can't return the same instance, since Query is…
033d0e8 
… mutable changing the query else where in the execution path changes the instance in the cache too.

Instead the query parser cache should return a cloned instances.

Closes elastic#2542
@rmuir
Copy link
Contributor

rmuir commented Jul 4, 2014

+1, looks good

@jpountz
Copy link
Contributor

jpountz commented Jul 4, 2014

LGTM

1 similar comment
@s1monw
Copy link
Contributor

s1monw commented Jul 4, 2014

LGTM

@martijnvg martijnvg changed the title The query string cache should returned clones Query instances. Core: The query string cache should returned clones Query instances. Jul 4, 2014
martijnvg added a commit that referenced this pull request Jul 4, 2014
@martijnvg
The query string cache can't return the same instance, since Query is…
9800a64 
… mutable changing the query else where in the execution path changes the instance in the cache too.

Instead the query parser cache should return a cloned instances.

Closes #2542
Closes #6733
@martijnvg martijnvg closed this in 730b83c Jul 4, 2014
martijnvg added a commit that referenced this pull request Jul 4, 2014
@martijnvg
The query string cache can't return the same instance, since Query is…
1f78967 
… mutable changing the query else where in the execution path changes the instance in the cache too.

Instead the query parser cache should return a cloned instances.

Closes #2542
Closes #6733
@clintongormley clintongormley changed the title Core: The query string cache should returned clones Query instances. Search: The query string cache should returned clones Query instances. Jul 9, 2014
@clintongormley clintongormley changed the title Search: The query string cache should returned clones Query instances. Search: The query string cache should returned cloned Query instances. Jul 9, 2014
@clintongormley clintongormley changed the title Search: The query string cache should returned cloned Query instances. Search: The query_string cache should returned cloned Query instances. Jul 9, 2014
@martijnvg martijnvg deleted the bugs/resident_query_string_cache_changing_boost branch May 18, 2015 23:31
@clintongormley clintongormley added the :Search/Search Search-related issues that do not fall into other categories label Jun 7, 2015
@clintongormley clintongormley changed the title Search: The query_string cache should returned cloned Query instances. The query_string cache should returned cloned Query instances. Jun 7, 2015
mute pushed a commit to mute/elasticsearch that referenced this pull request Jul 29, 2015
@martijnvg
The query string cache can't return the same instance, since Query is…
08fd3e3 
… mutable changing the query else where in the execution path changes the instance in the cache too.

Instead the query parser cache should return a cloned instances.

Closes elastic#2542
Closes elastic#6733
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
>bug :Search/Search Search-related issues that do not fall into other categories v1.2.2 v1.3.0 v2.0.0-beta1
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Issue with boost settings
5 participants
@martijnvg @rmuir @jpountz @s1monw @clintongormley