Skip to content

Fixup/pr/5881 - Add source filtering#5981

Closed
spalger wants to merge 26 commits intoelastic:masterfrom
spalger:fixup/pr/5881
Closed

Fixup/pr/5881 - Add source filtering#5981
spalger wants to merge 26 commits intoelastic:masterfrom
spalger:fixup/pr/5881

Conversation

@spalger
Copy link
Contributor

@spalger spalger commented Jan 22, 2016

In order to apply final fixes before merging, moving this from #5881 so we can iterate faster.


  • fetch field if:
    • explicitly selected in discover
    • show in a table
    • used in an aggregate

close #4366

I am not sure if this PR is a complete solution for the issue, but I believe it is a part of it.

This PR adds a configuration on the index pattern level that allows to define which field to exclude using the source filtering API. There is a new tab in which you can set the JSON to specify what to exclude.
The fields that are actually retrieved are indicated on the Fields tab as an additional column: if it is retrieved, the row is checked.

2016-01-12-104207_1920x1848_scrot

To test it:

  • Create a small index:
curl -XPUT 'http://localhost:9200/country/Country/1' -d '
{
  "name": "Italy",
  "description": "pasta"
}'
  • Add the country index and go to the Retrieved Fields tab. Exclude the description field from being retrieved. Add the following object to do so:
{
 "exclude": "description"
}

2016-01-12-105353_1920x1848_scrot

  • The description is now unchecked, meaning it is not retrieved.

2016-01-12-103741_1920x1848_scrot

  • Create a saved search for the countries and then add a search visualisation to a dashboard. You will see that the field description does not appear.

2016-01-12-104756_1920x1848_scrot


What is not provided in this PR but could be necessary: a way to display an excluded field for a document. Do you see this mandatory, or is the proposed solution good enough ?

@jbudz
Copy link
Contributor

jbudz commented Jan 25, 2016

Looking good!

  1. As brought up outside, can we add a note saying this isn't a security feature?
  2. If I add a column in discover for example, and then exclude it, all the values show up as nulls. Thoughts on this potentially being misleading?
    image

@jbudz jbudz assigned spalger and unassigned jbudz Jan 25, 2016
@spalger
Copy link
Contributor Author

spalger commented Jan 25, 2016

Talked with @rashidkpc about this and we agree that this is fine for now. If someone configures the index pattern to exclude that field then that's what they get. If it turns out that isn't sufficient we can cross that bridge.

@scampi
Copy link
Contributor

scampi commented Jan 26, 2016

I tried the branch, and there is a little left-over from the previous PR.

@jbudz @spalger thanks for taking care of this. If you need me to do something, please ping me.

@epixa epixa added needs updates and removed review labels Feb 8, 2016
@jccq
Copy link

jccq commented Feb 8, 2016

Hi guys i would reccomend an option (To go with this new feature) to show the 'hidden" field when browsing the specific record e.g. in the result table.

one should see something like "there are 3 fileds that have not been loaded by default e.g. "full description" "comment lists" etc.. and then a button that retrieves it for the operator that is interested in actually looking at them..

i feel this extra feature is really needed so not to "lose sight of data" (conceptually) ever. What do you think?

@spalger
Copy link
Contributor Author

spalger commented Feb 9, 2016

I'm inclined to agree @jccq. Unfortunately it's much more difficult/complex to implement a solution where discover or other views have control over what fields are being filtered.

@w33ble w33ble assigned w33ble and spalger and unassigned w33ble Apr 5, 2016
@spalger spalger assigned w33ble and unassigned spalger Apr 6, 2016
@w33ble
Copy link
Contributor

w33ble commented Apr 6, 2016

screenshot 2016-04-05 17 18 42

Much better!

@w33ble
Copy link
Contributor

w33ble commented Apr 6, 2016

With the latest commits, this LGTM. Passing to @Bargs for final confirmation, since he's looked at this before.

@w33ble w33ble assigned Bargs and unassigned w33ble Apr 6, 2016
@Bargs
Copy link
Contributor

Bargs commented Apr 6, 2016

Should this work on meta fields? I tried excluding _id, and it matched, but it didn't actually get exluded from discover.

screen shot 2016-04-06 at 4 24 12 pm

@Bargs
Copy link
Contributor

Bargs commented Apr 6, 2016

Typos:

pr5981typo

foo

@Bargs
Copy link
Contributor

Bargs commented Apr 6, 2016

Given that this is a brand new feature, it probably deserves some doc updates now that we're responsible for documentation. Especially because even I don't understand where all this filtering is expected to take place. The original PR description mentions filtering in the dashboard, but that no longer appears to happen.

Are there any places besides discover where this filtering is currently expected to take place?

@Bargs
Copy link
Contributor

Bargs commented Apr 6, 2016

How difficult would it be to hide the filter bar from this tab, since it doesn't do anything?

screen shot 2016-04-06 at 4 48 06 pm

@Bargs
Copy link
Contributor

Bargs commented Apr 6, 2016

It matches on scripted fields but doesn't filter them:

screen shot 2016-04-06 at 4 55 35 pm

@Bargs Bargs assigned spalger and unassigned Bargs Apr 6, 2016
@Bargs Bargs added needs updates and removed review labels Apr 6, 2016
@scampi
Copy link
Contributor

scampi commented Jun 9, 2016

fixes will be done in #7402
This can be closed

@spalger spalger closed this Jun 22, 2016
@spalger spalger deleted the fixup/pr/5881 branch October 18, 2019 17:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Loading huge documents

8 participants

Comments