-
Notifications
You must be signed in to change notification settings - Fork 25.7k
ESQL - Tdigest data type #139151
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
ESQL - Tdigest data type #139151
Conversation
…type' into tdigest-data-type
Conflicts: server/src/main/resources/transport/upper_bounds/9.3.csv
Conflicts: server/src/main/resources/transport/upper_bounds/9.3.csv x-pack/plugin/esql/qa/testFixtures/src/main/java/org/elasticsearch/xpack/esql/EsqlTestUtils.java x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/nulls/Coalesce.java
…type' into tdigest-data-type
|
Pinging @elastic/es-storage-engine (Team:StorageEngine) |
dnhatn
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.
One comment, but LGTM. Thanks Mark!
| this(encodeCentroidsAndCounts(centroids, counts), min, max, sum, valueCount); | ||
| } | ||
|
|
||
| public TDigestHolder(StreamInput in) throws IOException { |
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.
We usually place readFrom and writeTo next to each other so they can be easily scanned and compared.
Conflicts: server/src/main/resources/transport/upper_bounds/9.3.csv
This introduces an ESQL data type for TDigest fields. I've attempted to keep this as minimal as possible, so there's a lot of stuff that we expect data types to support that doesn't work yet (
Case,Is Null, etc). That will come with future PRs.The biggest change here is making
TDigestHolderwritable, which is required for using it inLiteral. That in turn is required for the ESQL tests.