RangeFieldType.rangeQuery now defaults to mappings format(parser)

[mP1]

• previously the format given in the mapping was ignored while parsing
  date in responses, fixed.
• range filter on date_range datatype should use the format defined in the mapping #29282
• added more tests for some combinations of range queries in RangeFieldTypeTests.

• Have you signed the contributor license agreement?
  YES

• Have you followed the contributor guidelines?
  YES

• If submitting code, have you built your formula locally prior to submission with gradle check?
  YES - passed

• If submitting code, is your pull request against master? Unless there is a good reason otherwise, we prefer pull requests against master and will backport as needed.
  rebased against master 2018/4/30

• If submitting code, have you checked that your submission is for an OS that we support?
  OSX (n/a)

• If you are submitting this code for a class then read our policy for that.
  N/A

[elasticmachine]

Pinging @elastic/es-search-aggs

[cbuescher]

@mP1 thanks for opening this PR. You might not have seen it in the original issue (#29282), but this issue already has an open PR that is currently under review (#29310). The fix looks identical to me, I'd like to continue working on the tests on that PR when the author comes back with it. Hope you don't mind if we don't go into more deeper reviews for this PR only if #29310 doesn't get resolved.

[mP1]

@cbuescher

Well this PR

• better code coverage in tests
• tests each permutation of inputs with nice expectations.
• tests need to try and test not only the bug but reasonable possible inputs both good and bad, other pr doesnt do that.
• other pr has been idle for a month, we can wrap this up much more quickly.

[cbuescher]

we can wrap this up much more quickly

Its not so much about speed, but about letting contributers continue with PRs they started working on. If you open an issue that you know is already being worked on elsewhere, please be so nice as to let us and the user know about it. If you need something to go in badly lets discuss that on the issue.
I was reviewing the other PR so I knew it was already worked on, but somebody else wouldn’t have known, so please add a note when doing this, so we avoid needless review cycles.

[mP1]

@cbuescher

Its not so much about speed, but about letting contributers continue with PRs they started working on.

Over a month for a reply shows they have abandoned this review. The time limit for replies on PRs must be something reasonable so commits stay relevant to the current system. The longer time advances, and the more changes the system sees, old commits will have so many conflicts and everything will be so different, the original will require almost a total rewrite.

There needs to be a policy that if people dont advance a review after a week or two, then that stale PR is closed, so other people can get it done. Just abandoning and not replying to review comments isnt good enough. Theres no point waiting forever for someone who is not going to reply...

This is one of them.

[colings86]

@mP1 We should at least give people the chance to pick up those PRs instead of opening a new PR with the same code. People do sometimes forget about PRs they are working on and I think it is only right to see if they still want to work on them before calling them abandoned and creating a new PR.

In this case the user has replied very quickly after being asked if they would like to progress their PR so I think its only right that we allow them some time.

[cbuescher]

Since #29310 has been merged, which solves the underlying issue, I'm also closing this PR.