Skip to content

Support synthetic source for date fields when ignore_malformed is used #109410

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

Merged
merged 10 commits into from
Jun 10, 2024
Merged

Support synthetic source for date fields when ignore_malformed is used #109410

Changes from 1 commit
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
6bcf63a
Support synthetic source for date fields when ignore_malformed is used
lkts Jun 5, 2024
3cc4107
style
lkts Jun 5, 2024
258394a
fix testSyntheticSourceIgnoreMalformedExamples
lkts Jun 5, 2024
58bd4df
docs
lkts Jun 6, 2024
110330b
iter
lkts Jun 6, 2024
51ccb29
iter
lkts Jun 6, 2024
54a026d
style
lkts Jun 6, 2024
bd0b74b
iter
lkts Jun 6, 2024
cd6bd29
Update docs/changelog/109410.yaml
lkts Jun 7, 2024
09994aa
Address feedback
lkts Jun 7, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
style
  • Loading branch information
@lkts
lkts committed Jun 5, 2024
commit 3cc4107b08093c60c0d36f8923a205a98e257064
12 changes: 6 additions & 6 deletions server/src/test/java/org/elasticsearch/index/mapper/DateFieldMapperTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
import org.elasticsearch.script.DateFieldScript;
import org.elasticsearch.script.ScriptService;
import org.elasticsearch.search.DocValueFormat;
import org.elasticsearch.test.ESTestCase;
import org.elasticsearch.test.index.IndexVersionUtils;
import org.elasticsearch.xcontent.XContentBuilder;

Expand All @@ -34,9 +33,7 @@
import java.util.ArrayList;
import java.util.Comparator;
import java.util.List;
import java.util.Map;
import java.util.function.Function;
import java.util.function.Supplier;
import java.util.stream.Stream;

import static org.elasticsearch.index.mapper.DateFieldMapper.DEFAULT_DATE_TIME_FORMATTER;
Expand Down Expand Up @@ -156,9 +153,12 @@ protected List<ExampleMalformedValue> exampleMalformedValues() {
exampleMalformedValue("2016-03-99").mapping(mappingWithFormat("strict_date_optional_time||epoch_millis"))
.errorMatches("failed to parse date field [2016-03-99] with format [strict_date_optional_time||epoch_millis]"),
exampleMalformedValue("-522000000").mapping(mappingWithFormat("date_optional_time")).errorMatches("long overflow"),
Copy link
Contributor Author

@lkts lkts Jun 6, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

One note here is that, due to the way this mapper is implemented, trying to f.e. index an object actually hard fails and does not go via malformed code path. This feels like a bug since it works in other mappers. Fixing it is a breaking change though i imagine.

Copy link
Contributor

Choose a reason for hiding this comment

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

Hm why is it a breaking change, we always return an error?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

When ignore_malformed is true we currently return an error but we'll stop if we fix the behavior.

Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't this the expectation? I'm trying to understand if it's a breaking change or a bug.

Copy link
Member

Choose a reason for hiding this comment

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

I don't think this is a breaking change. If we ignore_malformed is set to true and we fail because of malformed data in whatever form then that was a bug.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds good, i'll create a bug for this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

#109539

exampleMalformedValue("2020").mapping(mappingWithFormat("strict_date")).errorMatches("failed to parse date field [2020] with format [strict_date]"),
exampleMalformedValue("hello world").mapping(mappingWithFormat("strict_date_optional_time")).errorMatches("failed to parse date field [hello world]"),
exampleMalformedValue("true").mapping(mappingWithFormat("strict_date_optional_time")).errorMatches("failed to parse date field [true]")
exampleMalformedValue("2020").mapping(mappingWithFormat("strict_date"))
.errorMatches("failed to parse date field [2020] with format [strict_date]"),
exampleMalformedValue("hello world").mapping(mappingWithFormat("strict_date_optional_time"))
.errorMatches("failed to parse date field [hello world]"),
exampleMalformedValue("true").mapping(mappingWithFormat("strict_date_optional_time"))
.errorMatches("failed to parse date field [true]")
);
}

Expand Down