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

ES API date format strict_date_optional_time is not supported #5460

Closed
kuzaxak opened this issue Sep 28, 2024 · 2 comments
Closed

ES API date format strict_date_optional_time is not supported #5460

kuzaxak opened this issue Sep 28, 2024 · 2 comments
Labels
bug Something isn't working

Comments

@kuzaxak
Copy link
Collaborator

kuzaxak commented Sep 28, 2024

Describe the bug

After PR #5208 ES API responds with error when OSD sends a request like:

"query": {
  "bool": {
    "must": [],
    "filter": [
      {
        "match_all": {}
      },
      {
        "range": {
          "time": {
            "gte": "2024-09-28T10:07:55.797Z",
            "lte": "2024-09-28T10:22:55.797Z",
            "format": "strict_date_optional_time"
          }
        }
      }
    ],
    "should": [],
    "must_not": []
  }
}

to _search endpoint returns an error:

{
    "status": 400,
    "error": {
        "caused_by": null,
        "reason": "Failed to parse date time: a character literal was not valid",
        "stack_trace": null,
        "type": null
    }
}

Expected behavior

Formats from ES should be supported.

Configuration:
Please provide:

  1. Output of quickwit --version
    Quickwit 0.8.0 (aarch64-unknown-linux-gnu 2024-09-20T09:00:32Z 634a1bc)
  2. The index_config.yaml
    Default OTEL traces index.
@kuzaxak kuzaxak added the bug Something isn't working label Sep 28, 2024
kuzaxak added a commit that referenced this issue Sep 28, 2024
ES uses [own names][1] for date formats. I implemented the ones that I was able to find in OSD.

Test cases taken from [doc][1] and [OS test][2].

This PR is fixing [issue](#5460) and restore compatibility with OSD.

[1]: https://opensearch.org/docs/latest/field-types/supported-field-types/date/
[2]: https://github.com/opensearch-project/OpenSearch/blob/main/server/src/test/java/org/opensearch/common/time/DateFormattersTests.java
kuzaxak added a commit that referenced this issue Sep 28, 2024
ES uses [own names][1] for date formats. I implemented the ones that I was able to find in OSD.

Test cases taken from [doc][1] and [OS test][2].

This PR is fixing [issue](#5460) and restore compatibility with OSD.

[1]: https://opensearch.org/docs/latest/field-types/supported-field-types/date/
[2]: https://github.com/opensearch-project/OpenSearch/blob/main/server/src/test/java/org/opensearch/common/time/DateFormattersTests.java
kuzaxak added a commit that referenced this issue Sep 28, 2024
ES uses [own names][1] for date formats. I implemented the ones that I was able to find in OSD.

Test cases taken from [doc][1] and [OS test][2].

This PR is fixing [issue](#5460) and restore compatibility with OSD.

[1]: https://opensearch.org/docs/latest/field-types/supported-field-types/date/
[2]: https://github.com/opensearch-project/OpenSearch/blob/main/server/src/test/java/org/opensearch/common/time/DateFormattersTests.java
@kuzaxak
Copy link
Collaborator Author

kuzaxak commented Sep 28, 2024

It looks like a duplicate for already reported:
#5228
#5109

And seems it will be fixed in PR

kuzaxak added a commit that referenced this issue Sep 29, 2024
This test covers that we actually can call ES API endpoint with named alias as a formatter.

#5460
kuzaxak added a commit that referenced this issue Sep 29, 2024
This test covers that we actually can call ES API endpoint with named alias as a formatter.

#5460
fulmicoton added a commit that referenced this issue Sep 30, 2024
* fix(es): add java date format support

* Adding rest integration test

* Bugfix: expressing date in rfc3339. unit test + integration test

* fix: add week patterns

* Add support for variable zone offset type

ES supports 3 formats at the same time. To do the same we will use
`First` format item with multiple options.

* Replace regexp tokenizer with recursion parser

In the current implementation, the RegexTokenizer uses regular
expressions to tokenize the format string. However, regular expressions
are not well-suited for parsing nested structures like nested brackets
because they cannot match patterns with recursive depth.

To address this, we'll write a custom recursive parser that can handle
nested optional brackets. This parser will process the format string
character by character, building the format items and managing the
nesting of optional components.

That change allowed me to cover `strict_date_optional_time` which has a
nested depth according to the examples in [OS repo][2]

Additional tests taken from [the doc][1]

[1]: https://opensearch.org/docs/latest/field-types/supported-field-types/date/
[2]: https://github.com/opensearch-project/OpenSearch/blob/main/server/src/test/java/org/opensearch/common/time/DateFormattersTests.java

* Move StrptimeParser and Java related parsers to a dedicated file

This way it is easier to keep it manageable and clearly separate QW API DatParser logic and ES API related parser.

Tests moved together with the code base.

* Add additional test to cover initially reported issue

This test covers that we actually can call ES API endpoint with named alias as a formatter.

#5460

---------

Co-authored-by: Eugene Tolbakov <[email protected]>
Co-authored-by: Paul Masurel <[email protected]>
@kuzaxak
Copy link
Collaborator Author

kuzaxak commented Sep 30, 2024

Implemented in #5462

@kuzaxak kuzaxak closed this as completed Sep 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

1 participant