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

Simplifying Essence and construction of Druid query. #169

Closed
adrianmroz opened this Issue Aug 16, 2018 · 5 comments

Comments

Projects
None yet
2 participants
@adrianmroz
Copy link

adrianmroz commented Aug 16, 2018

Right now, Essence is very complex. It holds current state of visualisation and provides methods to manipulate it. Because this state is represented as partial Plywood expressions, these methods are complex. Plywood expressions are not meant to be queried early and manipulated. This complexity trickles down to components that manipulate state of visualisation, like filter menus.

General idea is to keep state as POJsO with higher level semantics that Plywood expression. Semantics should correspond with semantics of application. This object also should be easy to manipulate and should be minimal. For example limit for split should be simple value, sort for split is just product of column-id and enumeration fo directions. If filters properties are derived from split definition in some way, this should be resolved lazily during query generation and not in state.

Second component should be collection of domain specific methods to manipulate state. They should purely operate on data in state. For example changing split limit would be changing one number value. Because state is minimal, these functions also should touch smallest possible part of state.

And third, function that transforms such state to Plywood expression and in consequence Druid query. All derivations should be done here and should be done in efficient way. All declarations of columns (apply), constructions of measure expressions, relations between splits and filters should be done there based on simple state.

Additional component for this is mechanism for smart corrections to visualisation definitions. Rules could be simplified.

Main benefits are:

  • Simpler Plywood expressions and maybe later simpler queries to druid.
  • Smaller and simpler state object, more in line with current patterns in React ecosystem.
  • Simpler functions for state manipulation and inside rules and thus possibility for code reuse.
  • Simpler components which manipulate state like filter/split menus. We will dodge constant coding/decoding data as Plywood expression and need for traversing/modifying them.

General road map should be like this:

  • Express current Essence as POJsO
  • Rewrite Essence/Clicker methods as functions
  • Rewrite makeQuery function with new state

After this we should have default value of visualisation state working and it should query Druid. Next step is to rewrite UI components that manipulate state to accept simpler data and functions to manipulate it.

Then we should look into possibility to write new url converter, but writing Plywood expression -> new state converters could be hard and not worth it.

Lastly we can look into manifests and rules and try to simplyfy.

@mkuthan

This comment has been minimized.

Copy link
Member

mkuthan commented Aug 17, 2018

Great plan!

ViewDefinition should be an important part of new essence POJsOs. ViewDefinition has been designed to be fully independent from Plywood expression, it defines all view properties important for external Turnilo state (URL). It should also simplified ViewDefinitionConverters (or even get rid of the converters). We should discuss all properties of the new essence not expressed in the ViewDefinition. In general most of the UI state should be bookmarkable.

+1 for rewrite clicker methods as functions, it would be nice to have an interfaces for clickers method instead of stringly typed programming using anonymous objects with functions.

+1 for makeQuery function, new essence must not be aware of Druid queries. This function should be easy to test, where input is a new essence and the output plywood expression.

@adrianmroz

This comment has been minimized.

Copy link

adrianmroz commented Aug 23, 2018

Possible approach

Currently, we have ViewDefinition object that's created from url hash. It is POJsO and is used as seed for Essence value alongside DataCube. We could use ViewDefinition directly augmenting with some functions for actions like changing filter and updating splits accordingly. Components like SplitMenu should act on this ViewDefinition. For better API we could wrap ViewDefinition in Immutable.Record.
DataCube definition should be held separated.

Action points

  1. Define Immutable.Record for ViewDefinition.
  2. Use it as Essence object and whenever possible, pass only needed part of ViewDefinition. CubeView should act as holder of Essence reference.
  3. Rewrite Essence methods as inline code in components or pure functions.

@mkuthan mkuthan added this to the 1.9.0 milestone Aug 27, 2018

@adrianmroz

This comment has been minimized.

Copy link

adrianmroz commented Sep 3, 2018

Using ViewDefinition as Essence

First step should be to transfer most of interfaces from ViewDefinition to Essence. Native structures should be replaced with appropriate structures from immutbale-js. Instance and Class should be replaced with Immutable.Record. That would simplify modification methods. Whenever possible we should leave out fromJS/toJS code and use ViewConverters to convert to/from JSON. If possible, we should appropriately mark code that changes Essence state in response to it's internal changes (e.g. running resolveVisualization function)

@adrianmroz

This comment has been minimized.

Copy link

adrianmroz commented Oct 4, 2018

Current issues:

Splits and Filter are Records with one field because it's impossible to extend Immutable.List. There's package for that but we need to asses if we want to add next package to project. Maybe we could use less generic solution.

Essence still complects ViewDefinition state and state of application e.g. viz manifests, DataCube and visResolve object. It should be separated so Essence could become simple Record.

NumberFilterClause and FixedTimeClause accepts lists of ranges - they should handle only one range - more doesn't make sense.

StringFilterClause should capture action and value types better. For example includes filter should have Set of values but regex and contains should have only one string as value.

Regressions:

Only current version of hash/view definitions should handle toJS and fromJS. Previous version should handle only fromJS/fromHash because right away we will convert such hash to Essence and convert it to current version hash.

@adrianmroz

This comment has been minimized.

Copy link

adrianmroz commented Nov 5, 2018

Done in 1.9.0

@adrianmroz adrianmroz closed this Nov 5, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment