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

Fix simple query string serialization conditional #38960

Merged
merged 1 commit into from
Feb 15, 2019

Conversation

javanna
Copy link
Member

@javanna javanna commented Feb 15, 2019

Looks like #21504 introduced a serialization conditional for lenientSet,
but the PR was never backported to 5.x hence whenever simple query
string is serialized between 6.x and 5.6, it can't be read/written properly causing errors on the transport layer.

This commit updates the conditional to only read/write the field against
6.0+ versions which are aware of it.

Closes #38889

Looks like elastic#21504 introduced a serialization conditional for lenientSet,
but the PR was never backported to 5.x hence whenever simple query
string is serialized between 6.x and 5.6, it can't be read/written properly causing errors on the transport layer.

This commit updates the conditional to only read/write the field against
6.0+ versions which are aware of it.

Closes elastic#38889
@javanna javanna added >bug :Search/Search Search-related issues that do not fall into other categories v6.7.0 v6.6.2 labels Feb 15, 2019
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-search

@javanna javanna requested a review from jimczi February 15, 2019 14:45
Copy link
Contributor

@jimczi jimczi left a comment

Choose a reason for hiding this comment

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

Ouch, thanks @javanna . I left one comment for a follow up to ensure that we have bwc tests for this query.

}
}

public void testReadFrom_5_6() throws IOException {
Copy link
Contributor

Choose a reason for hiding this comment

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

We should add a simple rest test for the simple_query_string as a follow up in master. This would have caught this bug if we had one.

Copy link
Member Author

Choose a reason for hiding this comment

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

right, this unit test would have failed if we had it, but it only catches the bug when reading on 6.7 something that was written on 5.6. The other way around is much harder to test in 6.x cause we don't really have the reading code from 5.6.

@javanna javanna merged commit a2e8bfa into elastic:6.7 Feb 15, 2019
javanna added a commit that referenced this pull request Feb 16, 2019
Looks like #21504 introduced a serialization conditional for lenientSet,
but the PR was never backported to 5.x hence whenever simple query
string is serialized between 6.x and 5.6, it can't be read/written properly causing errors on the transport layer.

This commit updates the conditional to only read/write the field against
6.0+ versions which are aware of it.

Closes #38889
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>bug :Search/Search Search-related issues that do not fall into other categories v6.6.2 v6.7.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants