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

Query requests return with sample size defined in advanced setting #9356

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions changelogs/fragments/9356.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
feat:
- Remove sample size in default query string but use advanced setting ([#9356](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/9356))
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ const queriesTestSuite = () => {
// Default SQL query should be set
cy.waitForLoader(true);
cy.getElementByTestId(`osdQueryEditor__multiLine`).contains(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

js file 😭

`SELECT * FROM ${INDEX_WITH_TIME_1}* LIMIT 10`
`SELECT * FROM ${INDEX_WITH_TIME_1}`
);
cy.getElementByTestId(`queryResultCompleteMsg`).should('be.visible');

Expand Down Expand Up @@ -125,7 +125,7 @@ const queriesTestSuite = () => {
// Default PPL query should be set
cy.waitForLoader(true);
cy.getElementByTestId(`osdQueryEditor__multiLine`).contains(
`source = ${INDEX_WITH_TIME_1}*`
`source = ${INDEX_WITH_TIME_1}* | head 500`
);
cy.waitForSearch();
cy.getElementByTestId(`queryResultCompleteMsg`).should('be.visible');
Expand Down
2 changes: 1 addition & 1 deletion cypress/utils/apps/query_enhancements/recent_queries.js
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ export const generateRecentQueriesTestConfiguration = (dataset, datasetType, lan
PPL: 'OpenSearch SQL',
'OpenSearch SQL': 'PPL',
};
const defaultQuery = language.name === 'PPL' ? '' : ' LIMIT 10';
const defaultQuery = language.name === 'PPL' ? 'head 500' : '';
const customDatasetType = RecentQueriesDataTypes[datasetType].name;
return {
dataset,
Expand Down
2 changes: 1 addition & 1 deletion cypress/utils/apps/query_enhancements/shared.js
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ export const getDefaultQuery = (datasetName, language) => {
case QueryLanguages.PPL.name:
return `source = ${datasetName}`;
case QueryLanguages.SQL.name:
return `SELECT * FROM ${datasetName} LIMIT 10`;
return `SELECT * FROM ${datasetName}`;
}
};

Expand Down
1 change: 1 addition & 0 deletions src/plugins/data/common/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -111,4 +111,5 @@ export const UI_SETTINGS = {
SEARCH_QUERY_LANGUAGE_BLOCKLIST: 'search:queryLanguageBlocklist',
NEW_HOME_PAGE: 'home:useNewHomePage',
DATA_WITH_LONG_NUMERALS: 'data:withLongNumerals',
DISCOVER_SAMPLE_SIZE: 'discover:sampleSize',
} as const;
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ export const indexPatternTypeConfig: DatasetTypeConfig = {
title: i18n.translate('data.indexPatternType.sampleQuery.basicSQLQuery', {
defaultMessage: 'Sample query for SQL',
}),
query: `SELECT * FROM ${dataset.title} LIMIT 10`,
query: `SELECT * FROM ${dataset.title}`,
},
];
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ export const indexTypeConfig: DatasetTypeConfig = {
title: i18n.translate('data.indexType.sampleQuery.basicSQLQuery', {
defaultMessage: 'Sample query for SQL',
}),
query: `SELECT * FROM ${dataset.title} LIMIT 10`,
query: `SELECT * FROM ${dataset.title}`,
},
];
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
* SPDX-License-Identifier: Apache-2.0
*/
import { EuiIconProps } from '@elastic/eui';
import { CoreStart } from 'opensearch-dashboards/public';
import {
Dataset,
DatasetField,
Expand Down Expand Up @@ -117,5 +118,5 @@ export interface DatasetTypeConfig {
/**
* Returns the initial query that is added to the query editor when a dataset is selected.
*/
getInitialQueryString?: (query: Query) => string | void;
getInitialQueryString?: (query: Query, uiSettings?: CoreStart['uiSettings']) => string | void;
}
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,8 @@ export class QueryStringManager {
}

return (
typeConfig?.getInitialQueryString?.(query) ?? (languageConfig?.getQueryString(query) || '')
typeConfig?.getInitialQueryString?.(query, this.uiSettings) ??
(languageConfig?.getQueryString(query) || '')
);
}

Expand Down
20 changes: 17 additions & 3 deletions src/plugins/query_enhancements/public/datasets/s3_type.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

import { i18n } from '@osd/i18n';
import { trimEnd } from 'lodash';
import { HttpSetup, SavedObjectsClientContract } from 'opensearch-dashboards/public';
import { CoreStart, HttpSetup, SavedObjectsClientContract } from 'opensearch-dashboards/public';
import semver from 'semver';
import {
DATA_STRUCTURE_META_TYPES,
Expand All @@ -15,6 +15,7 @@
DataStructureCustomMeta,
Dataset,
DatasetField,
Query,
} from '../../../data/common';
import { DatasetTypeConfig, IDataPluginServices, OSD_FIELD_TYPES } from '../../../data/public';
import {
Expand Down Expand Up @@ -131,7 +132,7 @@
title: i18n.translate('queryEnhancements.s3Type.sampleQuery.basicPPLQuery', {
defaultMessage: 'Sample query for PPL',
}),
query: `source = ${dataset.title}`,
query: `source = ${dataset.title} | head 500`,
},
];
case 'SQL':
Expand All @@ -140,11 +141,24 @@
title: i18n.translate('queryEnhancements.s3Type.sampleQuery.basicSQLQuery', {
defaultMessage: 'Sample query for SQL',
}),
query: `SELECT * FROM ${dataset.title} LIMIT 10`,
abbyhu2000 marked this conversation as resolved.
Show resolved Hide resolved
query: `SELECT * FROM ${dataset.title} LIMIT 500`,
},
];
}
},

getInitialQueryString: (query: Query, uiSettings?: CoreStart['uiSettings']) => {
switch (query.language) {
case 'PPL':
return `source = ${query.dataset?.title} | head ${

Check warning on line 153 in src/plugins/query_enhancements/public/datasets/s3_type.ts

View check run for this annotation

Codecov / codecov/patch

src/plugins/query_enhancements/public/datasets/s3_type.ts#L153

Added line #L153 was not covered by tests
uiSettings?.get('discover:sampleSize') ?? 500
}`;
case 'SQL':
return `SELECT * FROM ${query.dataset?.title} LIMIT ${

Check warning on line 157 in src/plugins/query_enhancements/public/datasets/s3_type.ts

View check run for this annotation

Codecov / codecov/patch

src/plugins/query_enhancements/public/datasets/s3_type.ts#L157

Added line #L157 was not covered by tests
uiSettings?.get('discover:sampleSize') ?? 500
}`;
}
},
};

const fetch = async (
Expand Down
12 changes: 8 additions & 4 deletions src/plugins/query_enhancements/public/plugin.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
import { i18n } from '@osd/i18n';
import { BehaviorSubject } from 'rxjs';
import { CoreSetup, CoreStart, Plugin, PluginInitializerContext } from '../../../core/public';
import { DataStorage } from '../../data/common';
import { DataStorage, UI_SETTINGS } from '../../data/common';
import {
createEditor,
DefaultInput,
Expand Down Expand Up @@ -65,7 +65,12 @@ export class QueryEnhancementsPlugin
startServices: core.getStartServices(),
usageCollector: data.search.usageCollector,
}),
getQueryString: (currentQuery: Query) => `source = ${currentQuery.dataset?.title}`,
// TODO: we should remove sample size for default PPL query here
// once fetch_size is working: https://github.com/opensearch-project/OpenSearch-Dashboards/issues/9385
getQueryString: (currentQuery: Query) =>
`source = ${currentQuery.dataset?.title} | head ${core.uiSettings.get(
UI_SETTINGS.DISCOVER_SAMPLE_SIZE
)}`,
fields: { sortable: false, filterable: false, visualizable: false },
docLink: {
title: i18n.translate('queryEnhancements.pplLanguage.docLink', {
Expand Down Expand Up @@ -130,8 +135,7 @@ export class QueryEnhancementsPlugin
startServices: core.getStartServices(),
usageCollector: data.search.usageCollector,
}),
getQueryString: (currentQuery: Query) =>
`SELECT * FROM ${currentQuery.dataset?.title} LIMIT 10`,
getQueryString: (currentQuery: Query) => `SELECT * FROM ${currentQuery.dataset?.title}`,
fields: { sortable: false, filterable: false, visualizable: false },
docLink: {
title: i18n.translate('queryEnhancements.sqlLanguage.docLink', {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,9 @@ export const pplSearchStrategyProvider = (
return {
search: async (context, request: any, options) => {
try {
// TODO: PPL does not support fetch_size yet due to https://github.com/opensearch-project/sql/issues/3314
// We can add the below back for https://github.com/opensearch-project/OpenSearch-Dashboards/issues/9385
// request.body.fetch_size = await context.core.uiSettings.client.get('discover:sampleSize');
const query: Query = request.body.query;
const aggConfig: QueryAggConfig | undefined = request.body.aggConfig;
const rawResponse: any = await pplFacet.describeQuery(context, request);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,15 @@ describe('sqlSearchStrategyProvider', () => {
let logger: Logger;
let client: ILegacyClusterClient;
let usage: SearchUsage;
const emptyRequestHandlerContext = ({} as unknown) as RequestHandlerContext;
const emptyRequestHandlerContext = ({
core: {
uiSettings: {
client: {
get: jest.fn(),
},
},
},
} as unknown) as RequestHandlerContext;

beforeEach(() => {
config$ = of({} as SharedGlobalConfig);
Expand Down Expand Up @@ -69,7 +77,6 @@ describe('sqlSearchStrategyProvider', () => {
{ name: 'field1', type: 'long' },
{ name: 'field2', type: 'text' },
]);

const strategy = sqlSearchStrategyProvider(config$, logger, client, usage);
const result = await strategy.search(
emptyRequestHandlerContext,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,12 @@ export const sqlSearchStrategyProvider = (
search: async (context, request: any, options) => {
try {
const query: Query = request.body.query;
const rawResponse: any = await sqlFacet.describeQuery(context, request);
request.body.fetch_size = await context.core.uiSettings.client.get('discover:sampleSize');
const requestWithFetchSize = {
...request,
body: { ...request.body, fetch_size: request.body.fetch_size },
};
const rawResponse: any = await sqlFacet.describeQuery(context, requestWithFetchSize);

if (!rawResponse.success) throwFacetError(rawResponse);

Expand Down
1 change: 1 addition & 0 deletions src/plugins/query_enhancements/server/utils/facet.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ export class Facet {
body: {
query: query.query,
...(meta?.name && { datasource: meta.name }),
...(request.body?.fetch_size && { fetch_size: request.body.fetch_size }),
...(meta?.sessionId && {
sessionId: meta.sessionId,
}),
Expand Down
Loading