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

Add support for a 'format' option in fields retrieval. #57855

Merged
merged 4 commits into from
Jun 9, 2020

Conversation

jtibshirani
Copy link
Contributor

@jtibshirani jtibshirani commented Jun 9, 2020

The new format option allows for passing a custom date format:

POST logs-*/_search
{
  "fields": [
    "file.*",
    {
      "field": "event.timestamp",
      "format": "epoch_millis"
    },
    ...
  ]
}

Other API notes:

  • We use the same syntax as docvalue_fields for consistency. Under the hood, both fields and docvalue_fields use the same FieldAndFormat object to share serialization logic.
  • Only date and date_range fields support formatting currently. I really wasn't sure how helpful it was to support a decimal format on numeric fields (as we do with docvalues_fields), and didn't want to add pre-emptive functionality. I'm very happy for feedback on this point.

Implementation notes:

  • While the design is very simple, a downside is that custom date formatter object needs to be re-created for every document.
  • I looked into re-using each field's DocValueFormat to perform the formatting, but this didn't seem like the right way to go. DocValueFormat is specifically designed for translating between on-disk docvalues and human-readable values, and it felt like an abuse to start using it for formatting source values.

@jtibshirani jtibshirani added >enhancement :Search Foundations/Mapping Index mappings, including merging and defining field types labels Jun 9, 2020
@jtibshirani jtibshirani requested a review from nik9000 June 9, 2020 01:26
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-search (:Search/Mapping)

@elasticmachine elasticmachine added the Team:Search Meta label for search team label Jun 9, 2020

DateFormatter dateTimeFormatter = fieldType().dateTimeFormatter();
if (format != null) {
dateTimeFormatter = DateFormatter.forPattern(format).withLocale(dateTimeFormatter.locale());
Copy link
Member

Choose a reason for hiding this comment

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

I'd prefer to avoid this one day, but for now it is cool. And, I think it'd be a bit overboard to try and avoid it now. If I did want to avoid it maybe something like:

public Function<Object, String> sourceParser(String format);

I don't know. Just thinking out loud. Not needed now.

Copy link
Contributor Author

@jtibshirani jtibshirani Jun 9, 2020

Choose a reason for hiding this comment

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

I agree, instead of a function we could even introduce an object like SourceValueParser. This could also help clean up some of the fragility around the dual lookupValues and parseSourceValue methods. It did feel like overkill to do this refactor now, though.

@jtibshirani
Copy link
Contributor Author

Thanks @nik9000 for the review. Any thoughts on this comment?

Only date and date_range fields support formatting currently. I really wasn't sure how helpful it was to support a decimal format on numeric fields (as we do with docvalues_fields), and didn't want to add pre-emptive functionality.

@nik9000
Copy link
Member

nik9000 commented Jun 9, 2020

Thanks @nik9000 for the review. Any thoughts on this comment?

I'm super ok doing it just for dates now.

@jtibshirani jtibshirani merged commit 4788875 into elastic:field-retrieval Jun 9, 2020
@jtibshirani jtibshirani deleted the field-format branch June 9, 2020 17:43
jtibshirani added a commit that referenced this pull request Jun 17, 2020
The new `format` option allows for passing a custom date format:

```
POST logs-*/_search
{
  "fields": [
    "file.*",
    {
      "field": "event.timestamp",
      "format": "epoch_millis"
    },
    ...
  ]
}
```

Other API notes:
* We use the same syntax as `docvalue_fields` for consistency. Under the hood,
both `fields` and `docvalue_fields` use the same `FieldAndFormat` object to
share serialization logic.
* Only `date` and `date_range` fields support formatting currently.
jtibshirani added a commit that referenced this pull request Jun 26, 2020
The new `format` option allows for passing a custom date format:

```
POST logs-*/_search
{
  "fields": [
    "file.*",
    {
      "field": "event.timestamp",
      "format": "epoch_millis"
    },
    ...
  ]
}
```

Other API notes:
* We use the same syntax as `docvalue_fields` for consistency. Under the hood,
both `fields` and `docvalue_fields` use the same `FieldAndFormat` object to
share serialization logic.
* Only `date` and `date_range` fields support formatting currently.
jtibshirani added a commit that referenced this pull request Jun 30, 2020
The new `format` option allows for passing a custom date format:

```
POST logs-*/_search
{
  "fields": [
    "file.*",
    {
      "field": "event.timestamp",
      "format": "epoch_millis"
    },
    ...
  ]
}
```

Other API notes:
* We use the same syntax as `docvalue_fields` for consistency. Under the hood,
both `fields` and `docvalue_fields` use the same `FieldAndFormat` object to
share serialization logic.
* Only `date` and `date_range` fields support formatting currently.
jtibshirani added a commit that referenced this pull request Jun 30, 2020
The new `format` option allows for passing a custom date format:

```
POST logs-*/_search
{
  "fields": [
    "file.*",
    {
      "field": "event.timestamp",
      "format": "epoch_millis"
    },
    ...
  ]
}
```

Other API notes:
* We use the same syntax as `docvalue_fields` for consistency. Under the hood,
both `fields` and `docvalue_fields` use the same `FieldAndFormat` object to
share serialization logic.
* Only `date` and `date_range` fields support formatting currently.
jtibshirani added a commit that referenced this pull request Jul 8, 2020
The new `format` option allows for passing a custom date format:

```
POST logs-*/_search
{
  "fields": [
    "file.*",
    {
      "field": "event.timestamp",
      "format": "epoch_millis"
    },
    ...
  ]
}
```

Other API notes:
* We use the same syntax as `docvalue_fields` for consistency. Under the hood,
both `fields` and `docvalue_fields` use the same `FieldAndFormat` object to
share serialization logic.
* Only `date` and `date_range` fields support formatting currently.
jtibshirani added a commit that referenced this pull request Jul 14, 2020
The new `format` option allows for passing a custom date format:

```
POST logs-*/_search
{
  "fields": [
    "file.*",
    {
      "field": "event.timestamp",
      "format": "epoch_millis"
    },
    ...
  ]
}
```

Other API notes:
* We use the same syntax as `docvalue_fields` for consistency. Under the hood,
both `fields` and `docvalue_fields` use the same `FieldAndFormat` object to
share serialization logic.
* Only `date` and `date_range` fields support formatting currently.
jtibshirani added a commit that referenced this pull request Jul 15, 2020
The new `format` option allows for passing a custom date format:

```
POST logs-*/_search
{
  "fields": [
    "file.*",
    {
      "field": "event.timestamp",
      "format": "epoch_millis"
    },
    ...
  ]
}
```

Other API notes:
* We use the same syntax as `docvalue_fields` for consistency. Under the hood,
both `fields` and `docvalue_fields` use the same `FieldAndFormat` object to
share serialization logic.
* Only `date` and `date_range` fields support formatting currently.
jtibshirani added a commit that referenced this pull request Jul 18, 2020
The new `format` option allows for passing a custom date format:

```
POST logs-*/_search
{
  "fields": [
    "file.*",
    {
      "field": "event.timestamp",
      "format": "epoch_millis"
    },
    ...
  ]
}
```

Other API notes:
* We use the same syntax as `docvalue_fields` for consistency. Under the hood,
both `fields` and `docvalue_fields` use the same `FieldAndFormat` object to
share serialization logic.
* Only `date` and `date_range` fields support formatting currently.
jtibshirani added a commit that referenced this pull request Jul 21, 2020
The new `format` option allows for passing a custom date format:

```
POST logs-*/_search
{
  "fields": [
    "file.*",
    {
      "field": "event.timestamp",
      "format": "epoch_millis"
    },
    ...
  ]
}
```

Other API notes:
* We use the same syntax as `docvalue_fields` for consistency. Under the hood,
both `fields` and `docvalue_fields` use the same `FieldAndFormat` object to
share serialization logic.
* Only `date` and `date_range` fields support formatting currently.
jtibshirani added a commit that referenced this pull request Jul 23, 2020
The new `format` option allows for passing a custom date format:

```
POST logs-*/_search
{
  "fields": [
    "file.*",
    {
      "field": "event.timestamp",
      "format": "epoch_millis"
    },
    ...
  ]
}
```

Other API notes:
* We use the same syntax as `docvalue_fields` for consistency. Under the hood,
both `fields` and `docvalue_fields` use the same `FieldAndFormat` object to
share serialization logic.
* Only `date` and `date_range` fields support formatting currently.
jtibshirani added a commit to jtibshirani/elasticsearch that referenced this pull request Jul 27, 2020
The new `format` option allows for passing a custom date format:

```
POST logs-*/_search
{
  "fields": [
    "file.*",
    {
      "field": "event.timestamp",
      "format": "epoch_millis"
    },
    ...
  ]
}
```

Other API notes:
* We use the same syntax as `docvalue_fields` for consistency. Under the hood,
both `fields` and `docvalue_fields` use the same `FieldAndFormat` object to
share serialization logic.
* Only `date` and `date_range` fields support formatting currently.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>enhancement :Search Foundations/Mapping Index mappings, including merging and defining field types Team:Search Meta label for search team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants