Replies: 6 comments 8 replies
-
I'm having trouble understanding why changes to ad hoc data views necessitates changing the id. I understand it causes 'conflicts' but I don't understand what that means in this circumstance. I'd like to make sure the foundations of how we use ad hoc data views are serving us as best they can before continuing to build on them. Also, the generation of new ids is something that happens outside the scope of the service - it seems to me that it should be happening within the service if possible. That said, I have no familiarity with this usage of data views. |
Beta Was this translation helpful? Give feedback.
-
In both Discover and Lens we are creating dataviews using
This means that a new adHoc dataview will be created with a new id. In some cases this is very important:
In Lens it is important to have different ids per adhoc dataview as in each dataview the user needs to have different configuration. This makes sense, we are talking about different specs here, so having the same id is not correct The ES|QL case is different though. In ES|QL we do not have runtime fields, or field formatters or different timefields. In ES|QL we are using adhoc dataviews for technical reasons because Lens, dashboards and Discover need a dataview to work properly. (they shouldn't most possibly but this is what we have atm). In ES|QL an adhoc dataview that is used to create a saved search and quering the logstash indices will have the same spec with the adhoc dataview which is used to create an ES|QL visualization which queries the same indices. In this particular scenario using the same id is more performant and I think the correct path forward for now that we can only rely on adhoc dataviews. Also we don't introduce new flags on the dataview service that we possibly wont need in the future. |
Beta Was this translation helpful? Give feedback.
-
There are part of this that I do and don't understand so I'm going to try repeating this back which differs from what I've been told
Its my understanding that when this initially happens, you'll have the same data view in memory but since the data view was saved at two different points, you'll have different specs with the same id in each saved search. Yes, whichever data view is loaded first would override the second. Am I understanding correctly? Also, what problems would arise if we didn't save the id as part of an ad hoc data view? |
Beta Was this translation helpful? Give feedback.
-
Here's an idea - perhaps we shouldn't generate the data view id for ad hoc data views. Perhaps it should take the id of the saved object that persists it. Are there any saved objects that save more than one data view? I don't think so, but we could append Some changes would need to be made to data view api consumers that have special code around working with ad hoc data views. I'm not sure how much of this there is or what shape it takes. I'm not sure if this is avoidable. Do we have a way of determining an ad hoc data view from a persisted data view? If so, it doesn't seem to be part of the DataView service or classes. |
Beta Was this translation helpful? Give feedback.
-
I think we want to ensure id uniqueness among all data views saved as a part of other saved objects. While a difference in data view content necessitates it, lack of difference doesn't mean we benefit codewise from sharing a data view. Given a data view, would it be useful to know which saved object its part of? If you go from a saved object to discover, does the context of that saved object maintain any meaning or should we consider the discover context divorced from where we came? Is it possible to create a visualization, save a data data view inside it, alter the data view in discover - if we go back to the visualization, do we see the persisted data view or altered data view? Perhaps ad hoc data views should have a reference to the saved object that created them. Then when a given SO goes to save the data view it can determine if its the 'owner' of the data view. If not, its saved with a new id. |
Beta Was this translation helpful? Give feedback.
-
I started this discussion to evaluate a data view id by specification when ad hoc data views are generated , and I also did a quick POC to see what breaks if we do so: Working with it I figured out, that it wouldn't be a simple change mostly due to the way how our spec is generated today, taking default values into consideration, new spec fields would change the generated id, even when not being used. We can't be sure due to a change in the code, it wouldn't lead to different ids, even no relevant spec value changed. Anyway, for a defined minmal spec (so we determine which fields to use for it, rather then use all of it), like the one that's currently used in ES|QL it would make sense, during my exploration I figured out what would be solved by having a deterministic id:
To sum it up , it would resolve several issues with one approach , that would consume much more effort when targeted separately. Given, that further down the road we would change the way data views would be consumed, change the way ids are generated, this wouldn't block iterations. When an ad-hoc data view with an "old" id would be edited, a new id would be assigned. Let's say we decide that there can be a different timeField. Then we would need to take this field into consideration when the id is generated. Old ids would still be valid, but due to the change of spec a new ad-hoc data view with a different id would be created. However how we do this can be decided when the requirement is here. This is just to highlight, that I don't see a path forward being blocked by having a deterministic id approach. |
Beta Was this translation helpful? Give feedback.
-
History
In Discover and Lens, every time the configuration of an ad-hoc data view changes, we change its ID. This prevents having 2 same ad-hoc data views on a Dashboard with different configurations. Without this, e.g. users could create a Visualization in Lens, add it to a dashboard, use “Explore data in Discover”, which would use the same Ad-hoc Data View in Discover, change its configuration in Discover to a different index pattern, add it to the same dashboard, and now we would have 2 different data view specs with the same id on a dashboard.
Here is more context about this from Lens side
https://github.com/elastic/kibana/pull/140385/files#r967104452
To prevent conflicts, it's important to not re-use ad-hoc data view ids for different specs.
In Discover and Lens, every time the configuration of an ad-hoc data view changes, we change its ID. This prevents having 2 same ad-hoc data views on a Dashboard with different configurations. Without this, e.g. users could create a Visualization in Lens, add it to a dashboard, use “Explore data in Discover”, which would use the same Ad-hoc Data View in Discover, change its configuration in Discover to a different index pattern, add it to the same dashboard, and now we would have 2 different data view specs with the same id on a dashboard.
Here is more context about this from Lens side
https://github.com/elastic/kibana/pull/140385/files#r967104452
To prevent conflicts, it's important to not re-use ad-hoc data view ids for different specs.
In Discover it was implemented here
#141908
Problem statement
The creation of unique ids for every spec change prevents troubles, but there are side effects, that emerged during the ES|QL implementation.
Suggested solution
The PR #174736 proves that using data view with a static id using the current index pattern for ES|QL (
esql-${index-pattern}
) resolves 1. 2. 3.The generation of this id could be unified, baken into the data views service, e.g. by a helper method
Consuming this change would also resolve the “Unsaved changes” issue with non-ES|QL ad-hoc data views.
Given the convention how to generate the id would be aligned with #174736, this PR could move forward, and the data view changes could be applied later on
Further down the road
type
ofeqsql
in the PR, so this would need to be already considered in the spec to id generation, which also shows a downside of this approach, any new fields addons we add would create a new idBeta Was this translation helpful? Give feedback.
All reactions