Make the drop_null_columns configurable in ES|QL search strategies (#176236)

## Summary

Fixes https://github.com/elastic/kibana/issues/176227

As not everyone is using expressions and the `?drop_null_columns`
significantly changing the output, I am making this configurable. So for
the expressions users it will always be true but for anyone else it
would be false unless they specifically configure it.

This PR is fixing it but Nathan is going to follow up:

- maps are setting the drop_null_columns to true (as it makes the
response smaller, which is always a benefit)
- add some FTs to test the output

---------

Co-authored-by: Julia Rechkunova <julia.rechkunova@gmail.com>
This commit is contained in:
Stratoula Kalafateli 2024-02-06 16:28:08 +02:00 committed by GitHub
parent 92b6fd64cd
commit 43d93baf29
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
6 changed files with 25 additions and 11 deletions

View file

@ -678,4 +678,5 @@ export interface ESQLSearchParams {
query: string;
filter?: unknown;
locale?: string;
dropNullColumns?: boolean;
}

View file

@ -194,7 +194,10 @@ export const getEsqlFn = ({ getStartDependencies }: EsqlFnArguments) => {
return search<
IKibanaSearchRequest<ESQLSearchParams>,
IKibanaSearchResponse<ESQLSearchReponse>
>({ params }, { abortSignal, strategy: ESQL_ASYNC_SEARCH_STRATEGY }).pipe(
>(
{ params: { ...params, dropNullColumns: true } },
{ abortSignal, strategy: ESQL_ASYNC_SEARCH_STRATEGY }
).pipe(
catchError((error) => {
if (!error.attributes) {
error.message = `Unexpected error from Elasticsearch: ${error.message}`;

View file

@ -123,7 +123,7 @@ describe('ES|QL async search strategy', () => {
it('sets transport options on POST requests', async () => {
const transportOptions = { maxRetries: 1 };
mockApiCaller.mockResolvedValueOnce(mockAsyncResponse);
const params = {};
const params = { query: 'from logs' };
const esSearch = esqlAsyncSearchStrategyProvider(mockSearchConfig, mockLogger);
await firstValueFrom(
@ -139,6 +139,7 @@ describe('ES|QL async search strategy', () => {
keep_alive: '60000ms',
wait_for_completion_timeout: '100ms',
keep_on_completion: false,
query: 'from logs',
},
}),
expect.objectContaining({ maxRetries: 1, meta: true, signal: undefined })

View file

@ -11,6 +11,7 @@ import { catchError, tap } from 'rxjs/operators';
import { getKbnServerError } from '@kbn/kibana-utils-plugin/server';
import { SqlQueryRequest } from '@elastic/elasticsearch/lib/api/typesWithBodyKey';
import { SqlGetAsyncResponse } from '@elastic/elasticsearch/lib/api/types';
import type { ESQLSearchParams } from '@kbn/es-types';
import {
getCommonDefaultAsyncSubmitParams,
getCommonDefaultAsyncGetParams,
@ -22,12 +23,18 @@ import { IKibanaSearchRequest, IKibanaSearchResponse, pollSearch } from '../../.
import { toAsyncKibanaSearchResponse } from './response_utils';
import { SearchConfigSchema } from '../../../../config';
// `drop_null_columns` is going to change the response
// now we get `all_columns` and `columns`
// `columns` contain only columns with data
// `all_columns` contain everything
type ESQLQueryRequest = ESQLSearchParams & SqlQueryRequest['body'];
export const esqlAsyncSearchStrategyProvider = (
searchConfig: SearchConfigSchema,
logger: Logger,
useInternalUser: boolean = false
): ISearchStrategy<
IKibanaSearchRequest<SqlQueryRequest['body']>,
IKibanaSearchRequest<ESQLQueryRequest>,
IKibanaSearchResponse<SqlGetAsyncResponse>
> => {
function cancelAsyncSearch(id: string, esClient: IScopedClusterClient) {
@ -46,12 +53,12 @@ export const esqlAsyncSearchStrategyProvider = (
}
function asyncSearch(
{ id, ...request }: IKibanaSearchRequest<SqlQueryRequest['body']>,
{ id, ...request }: IKibanaSearchRequest<ESQLQueryRequest>,
options: IAsyncSearchOptions,
{ esClient, uiSettingsClient }: SearchStrategyDependencies
) {
const client = useInternalUser ? esClient.asInternalUser : esClient.asCurrentUser;
const { dropNullColumns, ...requestParams } = request.params ?? {};
const search = async () => {
const params = id
? {
@ -63,7 +70,7 @@ export const esqlAsyncSearchStrategyProvider = (
}
: {
...(await getCommonDefaultAsyncSubmitParams(searchConfig, options)),
...request.params,
...requestParams,
};
const { body, headers, meta } = id
? await client.transport.request<SqlGetAsyncResponse>(
@ -79,7 +86,7 @@ export const esqlAsyncSearchStrategyProvider = (
method: 'POST',
path: `/_query/async`,
body: params,
querystring: 'drop_null_columns',
querystring: dropNullColumns ? 'drop_null_columns' : '',
},
{ ...options.transport, signal: options.abortSignal, meta: true }
);

View file

@ -32,12 +32,16 @@ export const esqlSearchStrategyProvider = (
const search = async () => {
try {
const { terminateAfter, ...requestParams } = request.params ?? {};
// `drop_null_columns` is going to change the response
// now we get `all_columns` and `columns`
// `columns` contain only columns with data
// `all_columns` contain everything
const { terminateAfter, dropNullColumns, ...requestParams } = request.params ?? {};
const { headers, body, meta } = await esClient.asCurrentUser.transport.request(
{
method: 'POST',
path: `/_query`,
querystring: 'drop_null_columns',
querystring: dropNullColumns ? 'drop_null_columns' : '',
body: {
...requestParams,
},

View file

@ -428,7 +428,6 @@ export default function ({ getService }: FtrProviderContext) {
expect(jsonBody[0].result.requestParams).to.eql({
method: 'POST',
path: '/_query',
querystring: 'drop_null_columns',
});
});
@ -457,7 +456,6 @@ export default function ({ getService }: FtrProviderContext) {
expect(jsonBody[0].error.attributes.requestParams).to.eql({
method: 'POST',
path: '/_query',
querystring: 'drop_null_columns',
});
});
});