ftsrg / ingraph

Incremental view maintenance for openCypher graph queries.
http://docs.inf.mit.bme.hu/ingraph/
Eclipse Public License 1.0
47 stars 10 forks source link

Rename relalg-search-rete models #158

Closed szarnyasg closed 6 years ago

szarnyasg commented 7 years ago

cc @szdavid92

We had a discussion with @imbur today and concluded that the current naming is quite confusing as the search model (built by the RelalgBuilder class) does not define an actual search plan. We propose the following names:

@jmarton what do you think?

wafle commented 7 years ago

I'd prefer calling them ((Non)?Incremental)?QueryPlan. If you have to explain what the name means it's not a very good name.

szarnyasg commented 7 years ago

Well, good point, but then we'd have other lousy class names like NonIncremental2IncrementalQueryPlanTransformation. I think all (current and future) ingraph developers are well aware what Rete is, so we can safely use that term in the code - let's see what others think on this.

szarnyasg commented 7 years ago

A point against incremental and nonincremental - they have the same postfix which makes it more difficult to search for them.

jmarton commented 7 years ago

I think that refacoring of metamodels is much more important than the naming. We should separate the common base, the metamodel for the non-incremental plan and for the incremental plan, i.e. create 3 xcore files.

An other point is that rete is an implementation detail (which is unlikely to change, though), so I'd avoid to call it as such.

For the naming: as also the operators in the incremental plan encapsulates graph relational algebraic operators, I'd propose the following:

szarnyasg commented 6 years ago

As we are moving to Catalyst, we should get back to this issue and get the names right on this occassion :-).

szarnyasg commented 6 years ago

Moving to Catalyst resolved this.