ESQL: define the `tables` parameter #2706

nik9000 · 2024-07-10T19:48:41Z

Adds support for the tables parameter and documents the profile option.

nik9000 · 2024-07-10T20:04:33Z

I'm not sure what's up with the failure here. It looks like it doesn't like:

    tables?: Dictionary<
      string,
      Dictionary<string, SingleKeyDictionary<TableValuesType, any>>
    >

The SingleKeyDictionary<TableValuesType, any> doesn't feel quite right, but I'm not sure how tightly you need it here. That could be: {"long": [long|[long]]}/{"int": [int|[int]]}/{"keyword": [string|[string]]}/{"double": [double|[double]]}. But I don't know how to write that.

flobernd · 2024-07-11T06:43:37Z

I would model it like this:

// the naming should probably get improved
export type TableValuesData = long | int | string | double
export type TableValuesDataPayload = TableValuesData | TableValuesData[]

tables?: Dictionary<
   string,
   Dictionary<string, SingleKeyDictionary<TableValuesType, TableValuesDataPayload[]>>
>

Btw: any is invalid in the specification (note the failed validation). Instead, the UserDefinedValue might be used.

nik9000 · 2024-07-11T13:08:35Z

Btw: any is invalid in the specification (note the failed validation). Instead, the UserDefinedValue might be used.

Gotcha. I'll do it like you said without any. That's just better and not hard.

nik9000 · 2024-07-11T19:50:34Z

@flobernd have a look now!

github-actions · 2024-07-11T19:51:42Z

Following you can find the validation results for the APIs you have changed.

API	Status	Request	Response
`esql.async_query_get`	🟠	Missing type	Missing type
`esql.async_query`	🟠	Missing type	Missing type
`esql.query`	🔴	233/233	12/233

You can validate these APIs yourself by using the make validate target.

nik9000 · 2024-07-11T20:48:08Z

esql.query

Hey! I was testing that with make validate api=esql.query type=request branch=main locally. That does seem better. Though the response is defined as an ArrayBuffer so I think that should be skipped.

github-actions · 2024-07-11T20:50:30Z

Following you can find the validation results for the APIs you have changed.

API	Status	Request	Response
`esql.async_query_get`	🟠	Missing type	Missing type
`esql.async_query`	🟠	Missing type	Missing type
`esql.query`	🔴	233/233	12/233

You can validate these APIs yourself by using the make validate target.

flobernd · 2024-07-16T11:49:38Z

specification/esql/_types/TableValuesType.ts

+export interface TableValuesInteger {
+  integer: TableValuesIntegerValue[]
+}
+type TableValuesIntegerValue = integer | integer[]
+export interface TableValuesKeyword {
+  keyword: TableValuesKeywordValue[]
+}
+type TableValuesKeywordValue = string | string[]
+export interface TableValuesLong {
+  long: TableValuesLongValue[]
+}
+type TableValuesLongValue = long | long[]
+export interface TableValuesDouble {
+  double: TableValuesLongDouble[]
+}
+type TableValuesLongDouble = double | double[]


Since these types are mutually exclusive, we should consider modelling them as a container like:

/** @variants container */ export interface TableValues { integer?: integer | integer[] keyword?: string | string[] long?: long | long[] double?: double | double[] }

Would that work with your code-generators @Anaethelion @l-trotta ?

I did it this way because you can't specify more than one key in the object. Is that what @container does?

nit: for it to be a proper container all the fields should be optional.

To explain further the @container was designed primarily for query in search where the fields you set defines the type of query you use. Thus each QueryContainer can only have one field populated. link to modeling guide

I think it fits the intent.

@nik9000

I did it this way because you can't specify more than one key in the object. Is that what @container does?

Yes, the keys inside the container are mutually exclusive. I updated the above code with the missing optional ? modifier.

I updated the above code

I don't see a push. I'm willing to make the update myself if you'd like. Just getting out from other stuff.

Oh sorry, I meant the code with the container example in the above comment.

Cool. I've pushed some updates.

flobernd · 2024-07-16T11:50:48Z

specification/esql/query/QueryRequest.ts

    profile?: boolean
    /**
     * The ES|QL query API accepts an ES|QL query string in the query parameter, runs it, and returns the results.
     */
    query: string
+    /**
+     * Tables to use with the LOOKUP operation.


We should probably document the dictionary keys a little bit more in detail. I assume the outer key is the table name and the inner one the column name?

github-actions · 2024-07-17T15:14:40Z

Following you can find the validation results for the APIs you have changed.

API	Status	Request	Response
`esql.async_query_get`	🟠	Missing type	Missing type
`esql.async_query`	🟠	Missing type	Missing type
`esql.query`	🔴	233/233	12/233

You can validate these APIs yourself by using the make validate target.

nik9000

I've pushed another update based on suggestions.

nik9000 · 2024-07-17T15:15:35Z

specification/esql/query/QueryRequest.ts

    profile?: boolean
    /**
     * The ES|QL query API accepts an ES|QL query string in the query parameter, runs it, and returns the results.
     */
    query: string
+    /**
+     * Tables to use with the LOOKUP operation.


github-actions · 2024-07-17T15:16:48Z

Following you can find the validation results for the APIs you have changed.

API	Status	Request	Response
`esql.async_query_get`	🟠	Missing type	Missing type
`esql.async_query`	🟠	Missing type	Missing type
`esql.query`	🔴	233/233	12/233

You can validate these APIs yourself by using the make validate target.

flobernd · 2024-07-18T10:18:52Z

specification/esql/_types/TableValues.ts

+import { double, integer, long } from '@_types/Numeric'
+
+/** @variants container */
+export interface TableValues {


Suggested change

export interface TableValues {

export interface TableValueContainer {

Just a naming suggestion to be consistent with the existing specification, LGTM otherwise!

(or TableValuesContainer - not sure if singular or plural makes more sense here)

I think plural does. I'll make the change.

flobernd · 2024-07-18T10:19:09Z

specification/esql/query/QueryRequest.ts

+     * Tables to use with the LOOKUP operation. The top level key is the table
+     * name and the next level key is the column name.
+     */
+    tables?: Dictionary<string, Dictionary<string, TableValues>>


Suggested change

tables?: Dictionary<string, Dictionary<string, TableValues>>

tables?: Dictionary<string, Dictionary<string, TableValueContainer>>

github-actions · 2024-07-18T14:29:58Z

Following you can find the validation results for the APIs you have changed.

API	Status	Request	Response
`esql.async_query_get`	🟠	Missing type	Missing type
`esql.async_query`	🟠	Missing type	Missing type
`esql.query`	🔴	233/233	12/233

You can validate these APIs yourself by using the make validate target.

github-actions · 2024-07-18T14:46:48Z

Following you can find the validation results for the APIs you have changed.

API	Status	Request	Response
`esql.async_query_get`	🟠	Missing type	Missing type
`esql.async_query`	🟠	Missing type	Missing type
`esql.query`	🔴	233/233	12/233

You can validate these APIs yourself by using the make validate target.

nik9000 · 2024-07-18T14:51:01Z

Renamed. I've marked this auto-merge because I think that's what we want. I think I'm waiting on a final review.

github-actions · 2024-07-18T19:26:22Z

Following you can find the validation results for the APIs you have changed.

API	Status	Request	Response
`esql.async_query_get`	🟠	Missing type	Missing type
`esql.async_query`	🟠	Missing type	Missing type
`esql.query`	🔴	233/233	12/233

You can validate these APIs yourself by using the make validate target.

flobernd

LGTM! Thanks for the effort 🙂

nik9000 · 2024-07-19T11:58:20Z

Thanks for the feedback folks!

ESQL: define the tables parameter

bad9e4d

nik9000 requested review from pquentin, Anaethelion and l-trotta July 10, 2024 19:48

github-actions bot added the specification label Jul 10, 2024

Format

f12827d

nik9000 added 2 commits July 11, 2024 15:38

Better

6ad17e6

This way!

b28272b

Merge branch 'main' into define_tables

3fb43ee

flobernd reviewed Jul 16, 2024

View reviewed changes

Move

1d01d9b

Docs

f760f07

nik9000 commented Jul 17, 2024

View reviewed changes

flobernd reviewed Jul 18, 2024

View reviewed changes

nik9000 added 2 commits July 18, 2024 10:25

Move

015cef1

Update

0ec8d8f

Merge branch 'main' into define_tables

1065ab4

nik9000 enabled auto-merge (squash) July 18, 2024 14:49

nik9000 requested a review from flobernd July 18, 2024 14:49

Merge branch 'main' into define_tables

bd957f3

flobernd approved these changes Jul 19, 2024

View reviewed changes

nik9000 merged commit a003a41 into main Jul 19, 2024
6 checks passed

nik9000 deleted the define_tables branch July 19, 2024 10:02

	export interface TableValues {
	export interface TableValueContainer {

	tables?: Dictionary<string, Dictionary<string, TableValues>>
	tables?: Dictionary<string, Dictionary<string, TableValueContainer>>

ESQL: define the tables parameter #2706

ESQL: define the tables parameter #2706

Uh oh!

Conversation

nik9000 commented Jul 10, 2024

Uh oh!

nik9000 commented Jul 10, 2024

Uh oh!

flobernd commented Jul 11, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nik9000 commented Jul 11, 2024

Uh oh!

nik9000 commented Jul 11, 2024

Uh oh!

github-actions bot commented Jul 11, 2024

Uh oh!

nik9000 commented Jul 11, 2024

Uh oh!

github-actions bot commented Jul 11, 2024

Uh oh!

flobernd Jul 16, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Jul 17, 2024

Uh oh!

nik9000 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Jul 17, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Jul 18, 2024

Uh oh!

github-actions bot commented Jul 18, 2024

Uh oh!

nik9000 commented Jul 18, 2024

Uh oh!

github-actions bot commented Jul 18, 2024

Uh oh!

flobernd left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

nik9000 commented Jul 19, 2024

Uh oh!

Uh oh!

ESQL: define the `tables` parameter #2706

ESQL: define the `tables` parameter #2706

flobernd commented Jul 11, 2024 •

edited

Loading

flobernd Jul 16, 2024 •

edited

Loading