-
Notifications
You must be signed in to change notification settings - Fork 25.7k
ESQL: Add exponential histogram CSV Tests #137619
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
Conversation
|
Pinging @elastic/es-analytical-engine (Team:Analytics) |
x-pack/plugin/esql/qa/testFixtures/src/main/resources/exponential_histogram.csv-spec
Outdated
Show resolved
Hide resolved
nik9000
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks fine.
JSON in these is pretty gnarly. For date_ranges we're doing the rust-like 2025-01-01..2026-01-01. We'll do that for ip and numeric ranges too. But these complex things already have json parsing code and it's compelling to reuse it. A local minima, maybe.
Maybe we should do a """ to support un-escaped ". It'd be easier to embed json, I guess.
x-pack/plugin/esql/qa/testFixtures/src/main/resources/exponential_histogram.csv-spec
Outdated
Show resolved
Hide resolved
Indeed, but I think querying raw exponential histograms will rarely happen in the CSV tests. We'll rather look at the aggregations (e.g.
Is this some CSV standard or are you thinking of a custom implementation? I'm only aware of the We could also make our CSV-test matching more lenient and omit the quotes completely IINM. |
Adds a very basic CSV test for exponential histograms.
The dataset consists of some pathological dummy values alongside with a few real response time histograms.
Adapts the CSV test infrastructure to work with exponential histograms and hopefully doesn't break any bwc / release tests. Please take a closer look at that, as I'm really new to the ESQL testing world.
We don't have a ES test node feature for support of the
exponential_histogramfield type in elasticsearch, as that is hidden behind a feature flag aswell. But I think using the ES|QL capability should be a good enough surrogate, as we are planning on lifting those at the same time (or at least closely one after another).