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

detect_noop now understands null as a valid value #11210

Merged
merged 1 commit into from May 19, 2015

Conversation

nik9000
Copy link
Member

@nik9000 nik9000 commented May 18, 2015

If the source contrains a null value for a field then detect_noop should
consider setting it to null again to be a noop.

Closes #11208

modified = true;
continue;
}
modified = !old.equals(changesEntry.getValue());
modified = old == null ? changesEntry.getValue() != null : !old.equals(changesEntry.getValue());
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe this could be simplified to modified = !java.util.Objects.equal(old, changesEntry.getValue())?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah. I forget about that one.

@clintongormley clintongormley added >bug review :Distributed/CRUD A catch all label for issues around indexing, updating and getting a doc by id. Not search. labels May 18, 2015
@jpountz jpountz self-assigned this May 18, 2015
@jpountz
Copy link
Contributor

jpountz commented May 18, 2015

This looks good to me, I'll merge it tomorrow if nobody else has objections.

@nik9000
Copy link
Member Author

nik9000 commented May 18, 2015

Updated based on your comment. I can squash the changes if you want.

@jpountz
Copy link
Contributor

jpountz commented May 18, 2015

That would be great. :-)

If the source contrains a null value for a field then detect_noop should
consider setting it to null again to be a noop.

Closes elastic#11208
@nik9000
Copy link
Member Author

nik9000 commented May 18, 2015

Squashed.

jpountz added a commit that referenced this pull request May 19, 2015
detect_noop now understands null as a valid value
@jpountz jpountz merged commit f0d6dbb into elastic:master May 19, 2015
@kevinkluge kevinkluge removed the review label May 19, 2015
@clintongormley clintongormley changed the title detect_noop now understands null as a valid value detect_noop now understands null as a valid value Jun 7, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>bug :Distributed/CRUD A catch all label for issues around indexing, updating and getting a doc by id. Not search. v1.5.3 v1.6.0 v2.0.0-beta1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

detect_noop doesn't work with null values
4 participants