[Discover] Support custom logic to insert time filter based on dataset type #8932

joshuali925 · 2024-11-26T21:31:12Z

Description

Continue on #8917

This PR

allows dataset to override hideDatePicker per language for custom logic to insert time filter into query
disables submit button if time field is not selected in dataset configurator

Issues Resolved

closes #8917

Screenshot

Testing the changes

Changelog

feat: Support custom logic to insert time filter based on dataset type

Check List

All tests pass
- yarn test:jest
- yarn test:jest_integration
New functionality includes testing.
New functionality has been documented.
Update CHANGELOG.md
Commits are signed per the DCO using --signoff

Signed-off-by: Joshua Li <[email protected]>

codecov · 2024-11-26T21:46:55Z

Codecov Report

Attention: Patch coverage is 85.71429% with 4 lines in your changes missing coverage. Please review.

Project coverage is 60.88%. Comparing base (073a9ff) to head (7f6b2fc).
Report is 18 commits behind head on main.

Files with missing lines	Patch %	Lines
...s/data/public/ui/dataset_selector/configurator.tsx	81.25%	2 Missing and 1 partial ⚠️
...ta/public/ui/query_editor/query_editor_top_row.tsx	91.66%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #8932   +/-   ##
=======================================
  Coverage   60.87%   60.88%           
=======================================
  Files        3802     3802           
  Lines       91072    91092   +20     
  Branches    14375    14380    +5     
=======================================
+ Hits        55444    55462   +18     
- Misses      32086    32088    +2     
  Partials     3542     3542

Flag	Coverage Δ
Linux_1	`29.01% <0.00%> (-0.01%)`	⬇️
Linux_2	`56.39% <ø> (ø)`
Linux_3	`37.91% <85.71%> (+0.01%)`	⬆️
Linux_4	`28.99% <0.00%> (-0.01%)`	⬇️
Windows_1	`29.03% <0.00%> (-0.01%)`	⬇️
Windows_2	`56.34% <ø> (ø)`
Windows_3	`37.92% <85.71%> (+0.01%)`	⬆️
Windows_4	`28.99% <0.00%> (-0.01%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Signed-off-by: Joshua Li <[email protected]>

ruanyl · 2024-11-27T05:27:46Z

src/plugins/data/public/query/query_string/dataset_service/types.ts

@@ -43,6 +43,13 @@ export interface DatasetTypeConfig {
  id: string;
  /** Human-readable title for the dataset type */
  title: string;
+  languageOverrides?: {


Is there an example of how or when languageOverrides will be used?

currently no. this is for example if PPL needs different logic to insert the time filter for a specific dataset. Once the dataset overrides this option, PPL will not insert time filter in pplSearchInterceptor, but will pass the timeRange to params.body, and the dataset search strategy would be able to access the user query and timeRange to insert the time filter

for example

languageOverrides: { PPL: { hideDatePicker: false, }, },

and in search strategy, the request will contain

const query: Query = request.body.query; const timeRange: TimeRange = request.body.timeRange;

ruanyl · 2024-11-27T05:32:25Z

src/plugins/query_enhancements/public/search/ppl_search_interceptor.ts

    const dataset = query.dataset;
    if (!dataset || !dataset.timeFieldName) return query;
+    const datasetService = queryString.getDatasetService();
+    if (datasetService.getType(dataset.type)?.languageOverrides?.PPL?.hideDatePicker === false)


It seems when hideDatePicker === false, timeRange is added to params.body, otherwise, the time filter is encapsulated in the PPL query on line 102. The time filter will anyway be added, is this by intention?

yes it's by intention, if hideDatePicker === false, then it's not automatically inserted. This allows custom logic to insert the time filter

I see, so when timeRange is added to params.body, then it's up to server side search strategy to decide how to use it, did I get it right?

ruanyl

Approved with some non-blocking questions :)

ruanyl · 2024-12-04T03:32:29Z

src/plugins/query_enhancements/public/search/sql_search_interceptor.ts

@@ -62,6 +63,16 @@ export class SQLSearchInterceptor extends SearchInterceptor {
        .getDatasetService()
        .getType(datasetType);
      strategy = datasetTypeConfig?.getSearchOptions?.().strategy ?? strategy;
+
+      if (datasetTypeConfig?.languageOverrides?.SQL?.hideDatePicker === false) {


Is this just for consistency? Because I don't see it add time range in the query automatically in sql search interceptor.

the idea is that if there is a language override that sets hideDatePicker: false, then the search interceptor will not automatically insert the timeRange into query, but will pass timeRange to search strategy. Passing timeRange happens regardless of whether the search interceptor has logic to insert timeRange

Currently we don't have SQL query parser + builder so it's hard to insert time range to SQL, but if the search API for a specific dataset takes a SQL query and a separate time range parameter, then this will be useful

opensearch-trigger-bot · 2024-12-06T21:27:31Z

The backport to 2.x failed:

The process '/usr/bin/git' failed with exit code 128

To backport manually, run these commands in your terminal:

# Navigate to the root of your repository
cd $(git rev-parse --show-toplevel)
# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add ../.worktrees/OpenSearch-Dashboards/backport-2.x 2.x
# Navigate to the new working tree
pushd ../.worktrees/OpenSearch-Dashboards/backport-2.x
# Create a new branch
git switch --create backport/backport-8932-to-2.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 7df73ddeea9eb8f0c462cc8a099dc32f49d14692
# Push it to GitHub
git push --set-upstream origin backport/backport-8932-to-2.x
# Go back to the original working tree
popd
# Delete the working tree
git worktree remove ../.worktrees/OpenSearch-Dashboards/backport-2.x

Then, create a pull request where the base branch is 2.x and the compare/head branch is backport/backport-8932-to-2.x.

joshuali925 · 2024-12-06T21:34:53Z

backport 2.x depends on #8901

…t type (opensearch-project#8932) * Pass time filter if language overrides hideDatePicker --------- Signed-off-by: Joshua Li <[email protected]> Co-authored-by: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com> Signed-off-by: Federico Silva <[email protected]>

joshuali925 added 2 commits November 26, 2024 21:09

Pass time filter if language overrides hideDatePicker

5d97d3c

Signed-off-by: Joshua Li <[email protected]>

update outdated comments

0ff21eb

Signed-off-by: Joshua Li <[email protected]>

github-actions bot added the all-star-contributor label Nov 26, 2024

opensearch-changeset-bot bot added a commit to joshuali925/OpenSearch-Dashboards that referenced this pull request Nov 26, 2024

Changeset file for PR opensearch-project#8932 created/updated

38165a2

joshuali925 mentioned this pull request Nov 26, 2024

[discover] data type overrides language config and if includes time filter then don't modify query #8917

Closed

7 tasks

update tests

a0ea032

Signed-off-by: Joshua Li <[email protected]>

joshuali925 force-pushed the fix-time-field branch from 38165a2 to a0ea032 Compare November 26, 2024 21:59

joshuali925 marked this pull request as ready for review November 26, 2024 21:59

joshuali925 requested review from sejli and huyaboo as code owners November 26, 2024 21:59

joshuali925 changed the title ~~Support custom logic to insert time filter based on dataset type~~ [Discover] Support custom logic to insert time filter based on dataset type Nov 26, 2024

joshuali925 added the backport 2.x label Nov 26, 2024

Changeset file for PR opensearch-project#8932 created/updated

7f6b2fc

ruanyl reviewed Nov 27, 2024

View reviewed changes

ruanyl approved these changes Dec 4, 2024

View reviewed changes

ananzh approved these changes Dec 6, 2024

View reviewed changes

ananzh merged commit 7df73dd into opensearch-project:main Dec 6, 2024
68 of 69 checks passed

opensearch-trigger-bot bot added the failed backport label Dec 6, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Discover] Support custom logic to insert time filter based on dataset type #8932

[Discover] Support custom logic to insert time filter based on dataset type #8932

joshuali925 commented Nov 26, 2024

codecov bot commented Nov 26, 2024 •

edited

Loading

ruanyl Nov 27, 2024

joshuali925 Dec 3, 2024

ruanyl Nov 27, 2024

joshuali925 Dec 3, 2024

ruanyl Dec 4, 2024

joshuali925 Dec 4, 2024

ruanyl left a comment

ruanyl Dec 4, 2024

joshuali925 Dec 4, 2024

opensearch-trigger-bot bot commented Dec 6, 2024

joshuali925 commented Dec 6, 2024

[Discover] Support custom logic to insert time filter based on dataset type #8932

[Discover] Support custom logic to insert time filter based on dataset type #8932

Conversation

joshuali925 commented Nov 26, 2024

Description

Issues Resolved

Screenshot

Testing the changes

Changelog

Check List

codecov bot commented Nov 26, 2024 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ruanyl left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

opensearch-trigger-bot bot commented Dec 6, 2024

joshuali925 commented Dec 6, 2024

codecov bot commented Nov 26, 2024 •

edited

Loading