Skip to content

[Discover] Supports SQL query language#134429

Merged
stratoula merged 140 commits intoelastic:unified-search-text-based-langfrom
stratoula:text-based-lang-phase-1
Jul 20, 2022
Merged

[Discover] Supports SQL query language#134429
stratoula merged 140 commits intoelastic:unified-search-text-based-langfrom
stratoula:text-based-lang-phase-1

Conversation

@stratoula
Copy link
Copy Markdown
Contributor

@stratoula stratoula commented Jun 15, 2022

Summary

Note: This PR will be merged on a feature branch

This PR enables the SQL queries in Discover. The feature is available after an advanced setting is enabled.
image

The user can run sql queries and also save them as saved searches. These searches cant be used for creating an aggregation based visualization but can be imported on a dashboard.

sql_discover

sql_discover1

For the technical preview we are going to support the following features:

  • SQL integration, FROM clause bound to a data view (error if no match)
  • No autocomplete
  • No date histogram
  • Saving as part of the saved search
  • No switch to Lens
  • No filter by cell click
  • Time range integration
  • Dashboard filter integration
  • No async search / search sessions
  • No “add field” button
  • No document viewer / context viewer
  • Inline documentation

Checklist

Delete any items that are not applicable to this PR.

@stratoula stratoula changed the title Move the add dataview action above the dataview selection panel [Discover] Supports SQL query language Jun 15, 2022
@flash1293 flash1293 force-pushed the text-based-lang-phase-1 branch from c9fe22a to 1fdbde6 Compare July 19, 2022 10:27
Copy link
Copy Markdown
Contributor

@MichaelMarcialis MichaelMarcialis left a comment

Choose a reason for hiding this comment

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

Amazing work as usual, @stratoula! I spent some time with it yesterday and have put together an initial list of comments for your review. Let me know if you have any questions or need any design support to achieve any of the following:

Data view and text-based query language menu

  • On the "SQL" option in this menu, the mouse cursor changes from pointer to default when hovering the accompanying "Technical preview" badge. Can we fix this so that the mouse cursor remains a constant pointer when hovering the "SQL" option (including the badge portion)?

  • Both the "Add a field to this data view" and "Manage this data view" buttons should not be present in the menu when a text-based query language is currently selected.

image

  • In my wireframes, I had suggested that when a text-based query language was selected, we should mark each individual data view option with a warning icon and tooltip. On second thought, after playing with it here, that suggestion seems heavy-handed and creates a lot of potential redundancy. Would it be possible to instead move these individual warning icons and tooltips to a single warning icon and tooltip that prepends the "Data views" title? Doing so will cut down on the redundancy, but still provide the warning to those looking to switch from SQL mode to data view mode.

image

  • After selecting "SQL" and closing the menu, reopening the menu appears to apply focus styles to both the "SQL" option and the and the first data view simultaneously. The browser is only actually focusing the first data view, despite the styling being applied to "SQL" as well. I believe we should only be applying the focus styles to a single option at a time in this menu. Additionally, I imagine in the above example, the "SQL" is the option that should be focused by default when the menu is reopened.

image

  • Would it be possible to adjust the horizontal padding of selectable options in this menu so that they appear flush with the left/right edges of menu? I believe EUI has a popover example with this flush look.

image

Confirmation modal

  • When in SQL mode and attempting to switch back to data view mode, users are presented with a confirmation modal. Interacting with the close/cancel button currently closes the modal and acts as if the user chose to proceed to switch modes without saving (losing their query in the process). This is not what I would expect as a user. Can we instead change this so that interacting with the the close/cancel (X) button simply cancels the mode switch action and keeps the user in SQL mode with their query?

  • The "Don't show this warning again" checkbox isn't vertically centered with the opposing buttons that appear to its right side. Would it be possible to center align it vertically?

image

  • The "Switch without saving" and "Save and switch" buttons are not right aligned in the modal currently. Possible to do so?

image

  • The gutter space between the "Switch without saving" and "Save and switch" buttons can be reduced to 16px.

image

  • The "Switch without saving" button appears to be using custom styles. Could we simply use the standard EuiButton with color="warning" prop instead?

  • When choosing to "Save and switch", a second modal for saving the search appears on top of the confirmation modal. Could we replace the confirmation modal with the save modal, rather than stacking them? At that point, completing the save modal should then perform the switch. Canceling out of the save modal should be considered the user wanting to cancel and cancel the entire mode switch action.

image

Editor

General

  • Is there an alternative code highlighting theme that we can use which doesn't so prominently utilize blue colored text? I'm worried the current blue color is very similar to the color for links that we use across Kibana.

  • Would it be possible to add tooltips for enable/disable word wrap, expand/compact query editor, and SQL reference? Examples can be seen in the wireframes.

  • Clicking the SQL reference button once opens it. Clicking it again should close it, but the popover remains open here, requiring users to click elsewhere on the page to close. Would it be possible to change it so that clicking the button when the popover is open will close it?

  • The red underlining of errors within the editor appears odd currently. For example, when supplying an unavailable data view with the SQL FROM clause, only the R in FROM will be underlined in red. It seems odd that only a single character in the clause is being underlined. Further, would it be better to have the unavailable data view underlined here instead?

  • Can we reduce the width of the line number column in the current error list popover? It feels overly wide currently.

image

  • I had suggested in the wireframes that clicking an individual error in the error list popover should focus the user's cursor on that error in the editor. That doesn't appear to be implemented here. Would it be possible to add?

Compact/auto-sizing editor

  • Rather than using primary colored EuiButtonIcon for the expand and SQL reference buttons, can we instead style them to appear visually similar other EUI append and prepend buttons, with the light gray background and blue colored icon (similar to the calendar icon prepended to the date picker)?

image

Unfocused

  • Can we apply the standard rounded corners to the top-left and bottom-left corners of the editor to better match other EUI inputs?

image

  • Would it be possible to reduce the left and right padding of the unfocused editor to 12px?

image

  • The single visible line of query text is not vertically centered in its container currently. Would it be possible to vertically center this text?

  • When more than one line of SQL is entered into the editor and the editor is then unfocused, the original design expectation was that the query would be minified down into a single line (i.e. line breaks turned into spaces), with as much of the multi-line query being shown as possible until reaching the rightmost edge of the available space. Only at that point should the query text-overflow to ellipsis. However, what appears to be happening in the PR is that only the first line with appended ellipsis is shown when the editor is unfocused, regardless of length. Can we change to the original intended behavior?

image

  • The bottom border that appears below the editor to indicate either focus (blue) or invalid (red) states appears to be 1px here. Could we change these to be 2px to match the size of similar states in other EUI inputs?

image

Focused

  • Can we apply the standard rounded corners to the top-left, bottom-left, and bottom-right corners of the editor and footer to better match other EUI inputs?

image

  • Would it be possible to apply some vertical padding at the beginning and end of the focused editor? I'm thinking 8px will be sufficient.

image

  • Can we add a light gray background behind the line numbers on the left of the editor for added separation between them and the query?

image

  • When the editor is focused, a scrollbar track for the editor appears to the right, even if the maximum height for the editor has yet to be exceeded. Could we change this behavior to hide the scrollbar track until the user has the need/ability to scroll the editor?

image

  • It looks like the footer container is positioned slightly incorrectly, appearing a few pixels too far to the right. Can this be corrected to align with the editor above?

image

  • Can we add additional padding to the footer area (that contains line count, error count, and run query shortcut)? I'm thinking a total of 8px vertically and 16px horizontally should look nice.

  • Can we change the font size for text within the footer (with line count, error count, and run query shortcut) be 12px instead of the current 14px?

  • Let's change the "⌘ + Enter" shortcut from being in an EuiBadge to EuiCode (to better match how we display keyboard shortcuts elsewhere in Kibana).

Expanded/manual sizing editor

  • Can we apply the standard rounded corners to the top-left, bottom-left, top-right, and bottom-right corners of the editor's header and footer to better match other EUI inputs?

image

  • Can we add additional padding to both the header (word wrap, compact, SQL reference) and footer (line count, error count, and run query shortcut) areas? I'm thinking a total of 8px vertically and 16px horizontally should look nice for both.

  • Both the the header (word wrap, compact, SQL reference) and footer (line count, error count, and run query shortcut) areas appear misaligned on the right side. Can this be fixed?

image

  • The word wrap button is currently being given the fill treatment when disabled. Can we remove that fill styling? I believe the icon change and addition of a tooltip should be sufficient in indicating what state the word wrap setting is in.

image

  • The bottom border that typically appears below EUI inputs denoting focus and invalid states is missing when the editor is expanded. Can we add those in?

image

  • It appears that a custom resizer is being used when the editor is in expanded mode. Would it be possible to switch to using EUI's resizable container components to ensure consistent styling?

Additional notes

  • When SQL errors are present and the query is run, the current error messaging doesn't currently appear as suggested in the wireframes. The field list still shows (even in situations where a non-existing data view is being referenced) and the only option provided to users is to show the error message in a toast. Would it be possible to change this to come closer to the what the wireframes suggest?

image

  • When making the initial switch from data view mode to SQL mode, a scrollbar that only scrolls ~1px vertically appears for some reason. I recall similar scrollbar issues occurring in fixed viewport apps in the past, so something we're doing here is causing a regression. Would it be possible to investigate and fix?

  • Should the Discover tour be updated or otherwise altered when taking the tour in text-based query language mode? Asking because the first step in the tour speaks of using the plus button on the field list to add fields to the table, but I believe that only works for data view mode.

image

@kertal
Copy link
Copy Markdown
Member

kertal commented Jul 19, 2022

  • Should the Discover tour be updated or otherwise altered when taking the tour in text-based query language mode? Asking because the first step in the tour speaks of using the plus button on the field list to add fields to the table, but I believe that only works for data view mode.

@MichaelMarcialis I think it should be hidden for this case

@flash1293
Copy link
Copy Markdown
Contributor

When making the initial switch from data view mode to SQL mode, a scrollbar that only scrolls ~1px vertically appears for some reason. I recall similar scrollbar issues occurring in fixed viewport apps in the past, so something we're doing here is causing a regression. Would it be possible to investigate and fix?

It's around for a long time, but this hasn't been fixed for Monaco - see #93587

Should the Discover tour be updated or otherwise altered when taking the tour in text-based query language mode? Asking because the first step in the tour speaks of using the plus button on the field list to add fields to the table, but I believe that only works for data view mode.

Agree with @kertal here, let's just hide the tour

@stratoula
Copy link
Copy Markdown
Contributor Author

It appears that a custom resizer is being used when the editor is in expanded mode. Would it be possible to switch to using EUI's resizable container components to ensure consistent styling?

I cant use it unfortunately because the code editor lives in the unified search plugin and Discover page is on the discover plugin.

@MichaelMarcialis
Copy link
Copy Markdown
Contributor

I cant use it unfortunately because the code editor lives in the unified search plugin and Discover page is on the discover plugin.

Forgive the potentially silly question on my part, but could you elaborate on this point? Not sure I'm following on why we can't use the EUI component.

@stratoula
Copy link
Copy Markdown
Contributor Author

stratoula commented Jul 19, 2022

Of course! If I understand correctly how the EuiResizer works

<EuiResizableContainer direction="vertical">
  {(EuiResizablePanel, EuiResizableButton) => (
    <>
      <EuiResizablePanel initialSize={50} minSize="20%">
        <EuiText>
          <p>{text}</p>
        </EuiText>
      </EuiResizablePanel>

      <EuiResizableButton />

      <EuiResizablePanel initialSize={50} minSize="20%">
        <EuiText>
          <p>{text}</p>
        </EuiText>
      </EuiResizablePanel>
    </>
  )}
</EuiResizableContainer>

you need to wrap the CodeEditor and the rest of the page into the <EuiResizablePanel , right?
These 2 components are not on the same plugin. So I cant think of a way to do it. I could use the EuiResizableButton in order to give the same look and feel but unfortunately it is not exported from EUI.

@kibana-ci
Copy link
Copy Markdown

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

  • [job] [logs] FTR Configs #22 / apis uptime feature controls spaces "after all" hook for "user_1 can't access APIs in space_2"
  • [job] [logs] FTR Configs #22 / apis uptime feature controls spaces "before all" hook for "user_1 can access APIs in space_1"

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
apm 1302 1303 +1
canvas 1141 1142 +1
cloudSecurityPosture 269 270 +1
controls 172 173 +1
dashboard 416 417 +1
dashboardEnhanced 93 94 +1
data 514 516 +2
dataViewManagement 175 176 +1
dataVisualizer 365 366 +1
discover 590 593 +3
discoverEnhanced 71 72 +1
fleet 747 748 +1
graph 216 217 +1
infra 952 953 +1
inputControlVis 106 107 +1
lens 930 931 +1
lists 303 304 +1
maps 865 866 +1
ml 1624 1625 +1
monitoring 504 505 +1
observability 480 481 +1
osquery 324 325 +1
securitySolution 3154 3155 +1
stackAlerts 137 138 +1
synthetics 886 887 +1
timelines 252 253 +1
transform 319 320 +1
unifiedSearch 196 212 +16
upgradeAssistant 197 198 +1
visTypeTimelion 106 107 +1
visTypeVega 229 230 +1
visualizations 363 364 +1
total +50

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id before after diff
@kbn/es-query 159 168 +9
data 2402 2405 +3
discover 63 65 +2
unifiedSearch 78 82 +4
total +18

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
cloudSecurityPosture 219.0KB 219.0KB +61.0B
controls 429.1KB 429.2KB +61.0B
dashboard 380.2KB 380.7KB +531.0B
dataViewManagement 143.6KB 143.7KB +57.0B
dataVisualizer 545.2KB 545.2KB +57.0B
discover 492.4KB 497.1KB +4.6KB
discoverEnhanced 42.6KB 43.2KB +531.0B
infra 1023.2KB 1023.2KB +61.0B
lens 1.2MB 1.2MB +531.0B
ml 3.3MB 3.3MB +61.0B
transform 383.2KB 383.2KB +61.0B
unifiedSearch 179.0KB 224.1KB +45.1KB
visTypeTimelion 107.7KB 107.8KB +61.0B
visTypeVega 1.7MB 1.7MB +61.0B
visualizations 240.5KB 240.6KB +85.0B
total +51.9KB

Canvas Sharable Runtime

The Canvas "shareable runtime" is an bundle produced to enable running Canvas workpads outside of Kibana. This bundle is included in third-party webpages that embed canvas and therefor should be as slim as possible.

id before after diff
module count 4967 4969 +2

Public APIs missing exports

Total count of every type that is part of your API that should be exported but is not. This will cause broken links in the API documentation system. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats exports for more detailed information.

id before after diff
kibana 578 579 +1
unifiedSearch 13 16 +3
total +4

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
data 421.9KB 422.8KB +935.0B
discover 28.7KB 28.9KB +192.0B
maps 80.5KB 80.5KB +4.0B
navigation 9.6KB 9.6KB +56.0B
securitySolution 253.4KB 253.4KB +61.0B
timelines 258.9KB 259.0KB +61.0B
unifiedSearch 44.0KB 46.9KB +2.9KB
total +4.2KB

Saved Objects .kibana field count

Every field in each saved object type adds overhead to Elasticsearch. Kibana needs to keep the total field count below Elasticsearch's default limit of 1000 fields. Only specify field mappings for the fields you wish to search on or query. See https://www.elastic.co/guide/en/kibana/master/development-plugin-saved-objects.html#_mappings

id before after diff
search 13 14 +1
Unknown metric groups

API count

id before after diff
@kbn/es-query 213 222 +9
data 3075 3078 +3
discover 79 81 +2
unifiedSearch 82 96 +14
total +28

async chunk count

id before after diff
unifiedSearch 11 14 +3

ESLint disabled line counts

id before after diff
@kbn/es-query 8 9 +1
discover 36 37 +1
unifiedSearch 16 19 +3
total +5

Total ESLint disabled count

id before after diff
@kbn/es-query 9 10 +1
discover 38 39 +1
unifiedSearch 16 19 +3
total +5

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@stratoula stratoula marked this pull request as ready for review July 20, 2022 05:58
@stratoula stratoula requested review from a team as code owners July 20, 2022 05:58
@stratoula stratoula merged commit 95da4fc into elastic:unified-search-text-based-lang Jul 20, 2022
stratoula added a commit that referenced this pull request Jul 26, 2022
* [Discover] Supports SQL query language (#134429)

* Move the add dataview action above the dataview selection panel

* Implements a new selectable on the dataview picker for the text based languages

* Implementation of the transition modal when on SQL mode and select a dataview

* Fix es lint

* Change switch modal button modal icon

* Lazy load components

* Small changes on the styling of the switch without saving button

* Initialization of mocaco editor

* Change to the type

* Fixes types checks

* New submit button for query mode

* Implememtation of the expanded mode of the editor

* Implement documentation

* Implementation of the oneliner mode with ellipsis

* Some  fixes on the resizer

* Implementation of the errors layout, WIP

* Fetch SQL data in Discover

* Fix expression test

* Fix editor zIndex

* Fix types error

* Fix type check in Discover

* Fix more types

* some CI fixes

* Fixes

* Cleanup after merge

* Remove from state

* Connect search errors with the unified search editor

* Add error mrkers in unified search editor

* Save and open saved searches

* Filter out saved searches from text based languages

* Some fixes

* Fix unit tests

* Fix checks

* On save and exit modal implementation

* Add shortcut on the editor for submit query

* Fix wrong condition

* Initial types change

* Use regex to find the index pattern string

* Fix some types and cleanup

* Fix types

* Fix some types

* Further fixes

* More fixes

* More fixes

* Fix visualize types

* more

* More fixes

* Fixes more types

* Fix dashboard types

* Fix dashboard types

* Controls plugin types

* Fix Lens types

* Fix data plugin types

* Fix types in Lens 2

* buildEsConfig type fixes

* Fix observability types

* Fix maps types

* data visualizer types

* Fix ml types

* xpack rest types

* Fix jest test

* Fix

* Move helper functions to es config

* fix bug on breadcrumb click

* Fix time field bug

* Add enableSql advanced setting to discover for enabling the sql mode

* Make the documentation component more dynamic

* Add some comments, improvements

* Enhance storybook with the textbased languages

* Update storybook with the error state of the editor

* Adds a readme for the editor and fixes the modal mobile version

* [Discover] improve test and storybook for new data type

* [Discover] add functional tests

* Add aggregate functions to the documentation

* [Discover] fix tests

* Add some unit tests

* [Discover] fix linting

* [Discover] update linting

* More unti tests

* Dataview picker unit tests

* Fix a bug on the dataview picker

* Add unit tests for the editor

* Fix jest test

* [Discover] apply suggestions

* [Discover] adjust styles

* Fix some bugs and select columns in the sql mode

* [Discover] fix eslint and tests

* [Discover] update unit tests

* Fix bug on transitioning from sql mode to dataview mode

* [Discover] fix tests

* Design fixes on the errors messages

* [Discover] fix ci

* Update the columns only if the query changes

* [Discover] change isPlainRecord retrieval method

* Fix bug on cleanup

* Fix bug on opening a saved search

* [Discover] fix comments

* [Discover] fix bug with browser refresh

* [Discover] fix functional

* [Discover] fix another functional

* Fix ordering lost when the user refreshes the browser

* [Discover] revert use_discover_state

* [Discover] revert functional impl

* Fix security solution types

* Casting dashboard plugin

* Revert change

* type param

* [CI] Auto-commit changed files from 'node scripts/precommit_hook.js --ref HEAD~1..HEAD --fix'

* Revert types changes

* More reverts

* Types fixes

* Fix Discover jest test

* Fix context app jest test

* Final types changes

* Fixes unit test

Co-authored-by: Dzmitry Tamashevich <diaamnj@mail.ru>
Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
Co-authored-by: Joe Reuter <johannes.reuter@elastic.co>

* Fix types

* Fix jest test

* More design fixes

* Update advanced setting description

* Further design changes

* [Discover] Remove document explorer header column edit data view field functionality (#136743)

* remove Edit data view field for SQL

* Fix the fix

* [Discover] Implement SQL data fetching for embeddable (#136793)

* remove Edit data view field for SQL

* Fix the fix

* Implement SQL for embeddable

* Fix non-saved-search embeddables

* Fix reporting bundle size

* Allow filters on dashboard level for sql searches

* Fix the radius on the editor

* Add vertical padding on the editor

* Change the theme

* Address PR comments

* Fix types

* Address some of the comments

* Fix bug on transitioning from SQL to dataview mode with the modal dismissed

* More types fixes

* Design review comments

* Discovery team review comments

* Fix jest tests

* Fix bug on navigating from the SQL mode to the dataview mode and back in sql mode by clicking the breadcrumb

* Update src/plugins/discover/public/application/main/hooks/use_discover_state.ts

Co-authored-by: Matthias Wilhelm <matthias.wilhelm@elastic.co>

* Add padding to the top of the editor without creating any bug

* Add some padding to the bottom without creating any bug

* Fixes undo bug

* Fix confusing naming of variable

* Fix nested selects

* Update texts for transition modal and warning

* Make it work with dashboard Query

* Address some of the comments

Co-authored-by: Dzmitry Tamashevich <diaamnj@mail.ru>
Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
Co-authored-by: Joe Reuter <johannes.reuter@elastic.co>
Co-authored-by: Matthias Wilhelm <matthias.wilhelm@elastic.co>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants