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

[Lens] Remove lens_multi_table #74643

Closed
flash1293 opened this issue Aug 10, 2020 · 7 comments
Closed

[Lens] Remove lens_multi_table #74643

flash1293 opened this issue Aug 10, 2020 · 7 comments
Labels
Feature:Lens Team:Visualizations Visualization editors, elastic-charts and infrastructure technical debt Improvement of the software architecture and operational architecture

Comments

@flash1293
Copy link
Contributor

Based on the discussion in #68457 , the lens_multi_table type should be removed in favor of using variables like this:

kibana
| kibana_context
| var_set name="table_a" value={esaggs ... | mapColumn name="columnId" expression={getCell "columnId" | replace "\W+" "_" }} name="table_b" value={esaggs ...}
| lens_xy_chart table="table_a" table="table_b"

Changes:

  • Frame doesn't create a multi table, but stores the result of each datasource layer expression in a seperate variable named by the layer
  • All visualizations are rewritten to take a list of tables via argument and ignore the context
@flash1293 flash1293 added Team:Visualizations Visualization editors, elastic-charts and infrastructure Feature:Lens labels Aug 10, 2020
@flash1293 flash1293 added this to Long-term goals in Lens via automation Aug 10, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-app (Team:KibanaApp)

@flash1293 flash1293 moved this from Long-term goals to Tech Debt in Lens Aug 10, 2020
@flash1293 flash1293 added the technical debt Improvement of the software architecture and operational architecture label Aug 10, 2020
@ppisljar
Copy link
Member

could we still use context for the main table and only provide additional datatables via arguments ?

@flash1293
Copy link
Contributor Author

There's not really a "main table" - they aren't ranked (at least not in the general case)

@ppisljar
Copy link
Member

revisiting this discussion:

it seems multitable is not something lens specific but will be needed in other places:

  • if maps were to use expressions they need layered data (multiple tables as well)
  • we might have single data fetching functions that need to return multiple tables (for example graph data, one table for nodes one for links)
kibana
| kibana_context
| create_multitable name="table_a" value={esaggs ... | mapColumn name="columnId" expression={getCell "columnId" | replace "\W+" "_" }} name="table_b" value={esaggs ...}
| lens_xy_chart

for this reasons we are considering keeping the multitable and making it one of the base types expressions support.
we have two options if we decide to go that way:

  1. every table manipulation function should also work with multitable. this would mean we need to add table argument to all table manipulation functions to give users an option to choose which table inside the multitable they want to manipulate.

add_column table=1 name='test'

cons: all existing table functions will need to change

  1. add a transform_multitable function which takes table argument to select which table inside the multitable you want to manipulate and expression argument which specifies what should be done to selected table. the output of a function is a new multitable with specified tables transormed.

transform_mutlitable table=1 expression={add_column name='test'}

  • multiple tables can be transformed with the same function (table and exression arguments are multi)

pros: multitable is added on top of datatable, and the existing functions don't need to change at all
cons: at the moment performance of this will be worse than option1 due to the expression we need to execute inside the function

no matter which way we take we would also need:

  • create_multitable, add_table, remove_table, select_table functions
  • type conversions between datatable and multitable.

@flash1293
Copy link
Contributor Author

I think we can go both ways and I don't feel strongly about either way.

at the moment performance of this will be worse than option1 due to the expression we need to execute inside the function

As we won't do it in a loop (like with mapColumn), I'm not too concerned about this performance hit.

@ppisljar
Copy link
Member

ppisljar commented Jul 7, 2021

we had a meeting and decided to keep removing lens multitable. we might revisit the multitable later if usecase for it comes up.

key points:

  • using multitable vs varables in lens doesn't have much tech differences but the later seems to bring in a different mental model which might be better for our users

@flash1293
Copy link
Contributor Author

Multi table got removed in #131699

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Lens Team:Visualizations Visualization editors, elastic-charts and infrastructure technical debt Improvement of the software architecture and operational architecture
Projects
None yet
Development

No branches or pull requests

3 participants