Proposed change to TransformResults in v1.2

[Itamar Syn-Hershko]

Hi guys,

Here's a proposed change for the TransformResults portion of RavenDB indexes, which we consider doing for v1.2. We have it on our issue tracker too (http://issues.hibernatingrhinos.com/issue/RavenDB-292), but thought I'd ping the list too to get your comments.

Here it goes:

The index definition is Map and Reduce functions. A TransformResults function is a way to fiddle with that data when the user hits the index for querying - not really tied to indexing.

Currently when you write an index and need one version of it without a TR, and then 2 different transformations - usually useful for getting ViewModels directly from the server, you have to write different indexes. Also, a change to a TR function will trigger reindexing.

Therefore the proposed change:

1. Renaming to TransformResults and dropping the Live Projections alias completely. It is all about results transformation, nothing more, and the double naming is confusing.

2. Supporting more than one such function per index. Something like:

TransformResults = { { "default", (db, results) => ... }, 

{ "view2", (db, results) => ... } }

In the index definition.

3. Allowing to hit the index and get results with or without the transformation applied, and to choose which to apply (with a default setting to hold true so behavior is well defined). This one is already supported through a hack on REST, I think it should become a client API feature.

4. Changes to TransformResults would not trigger reindexing

5. Client API would have something like this to support this:

session.Query<Type, Index>()

             .Where(...)

             .TransformResults<ProjectionType>()

             .ToList()

which is equivalent to:

session.Query<Type, Index>()

             .Where(...)

             .TransformResults<ProjectionType>("default")

             .ToList()

Querying an index and applying a non-default TransformResults transformation:

session.Query<Type, Index>()

             .Where(...)

             .TransformResults<ProjectionType>("view")

             .ToList()

Dropping the TransformResults<>() operator will skip the TransformResults operation completely (a BREAKING CHANGE)

[Chris Marisic]

I think these changes make alot of sense.

One suggestion I'd like to make, I think it'd be rare you would ever TRULY use

 .TransformResults<ProjectionType>("default")

 .TransformResults<ProjectionType>("view")

What I think would be far more useful would be supporting:

TransformResult = (db, results) => ... ;

OR

TransformResults = { { "ProjectionType", (db, results) => ... }, 

{ "SomeOtherProjectionType", (db, results) => ... } }


This would allow the user to see easier configuration for when there's only ever 1 transform, which can just behind the scenes load the TransformResults without adding obtuse syntax to configure 1 item. For your notions of default, I'd just pick the first one in the list if you don't have a typename match. (coincidentally also solves the untyped based usage). Theoretically typeof(ProjectionType) might be better, but i think the looser coupling the strings provide probably will pay out more in the long run. Users that are fine tight coupling to classes can just do typeof(ProjectionType).Name

When would you ever really do

 .TransformResults<ProjectionType>("default")

 .TransformResults<ProjectionType>("view")

As opposed to


 .TransformResults<ProjectionType>()
 .TransformResults<SomeOtherProjectionType>()

If for some crazy reason, you really needed 2 transforms into the same type, you could still work entirely within the typed api

class SomeOtherProjectionType : ProjectionType { }

[Oren Eini (Ayende Rahien)]

Chris,

Is it even valid to put the TransformResult on the index any more? Maybe it should be external and independent from the indexes?

As separate entity all together?

[Chris Marisic]

You could still support  .TransformResults<ProjectionType>("DifferentProjectionType") with "DifferentProjectionType" as an optional parameter to force the picking.

[Matt Warren]

I think it's valid just because having an index and querying it are logically associated events, you don't write an index with no intention of querying it for instance! 

Whilst (internally) the Map/Reduce and TransformResults sections happen at different times and are independent (to some extent), having them defined within a single "index" does make sense for the API point of view (internally they could always be split out though)

However I agree that options to choose the transform results to apply (or apply none) are nice changes to have and should be defined at query time. Plus preventing a re-indexing from happening after changes to a TransformResult should be fixed.

[Chris Marisic]

I agree with Matt, an Index and it's Transformation results are logically bound. I see no benefit to seperating them to the user at definition time. If you want on the server to separate them entirely, by all means, that's transparent to the user. But to define them in separate classes or files, doesn't seem to add any value because you always need to understand inherently what exactly is in the index in the first place, so crafting a transform you always need to be looking at the map/reduce additionally.

[Itamar Syn-Hershko]

inline

On Fri, May 18, 2012 at 3:39 PM, Chris Marisic <ch...@marisic.com> wrote:

I think these changes make alot of sense.

One suggestion I'd like to make, I think it'd be rare you would ever TRULY use

 .TransformResults<ProjectionType>("default")

 .TransformResults<ProjectionType>("view")

What I think would be far more useful would be supporting:

TransformResult = (db, results) => ... ;

OR

TransformResults = { { "ProjectionType", (db, results) => ... }, 

{ "SomeOtherProjectionType", (db, results) => ... } }


This would allow the user to see easier configuration for when there's only ever 1 transform, which can just behind the scenes load the TransformResults without adding obtuse syntax to configure 1 item.

If we will do that, it will only be by translating the first one to the second (creating a dictionary and adding it as "default"). See more below.

 

For your notions of default, I'd just pick the first one in the list if you don't have a typename match. (coincidentally also solves the untyped based usage). Theoretically typeof(ProjectionType) might be better, but i think the looser coupling the strings provide probably will pay out more in the long run. Users that are fine tight coupling to classes can just do typeof(ProjectionType).Name

This would introduce friction. There should really be only one way of doing this, even if it breaks old habits from v1.0...

 

When would you ever really do

 .TransformResults<ProjectionType>("default")

 .TransformResults<ProjectionType>("view")

As opposed to


 .TransformResults<ProjectionType>()
 .TransformResults<SomeOtherProjectionType>()

If for some crazy reason, you really needed 2 transforms into the same type, you could still work entirely within the typed api

I'm not sure where you got the idea of 2 chained transformations. The idea is to support one at a time, letting you choose which will be executed (if at all)

As you said, I cannot see any use for chained transformations, and if you really need that just write a new TR function doing what you need

Also, it can be quite heavy on the server, chaining transformations

[Itamar Syn-Hershko]

I absolutely agree with Matt here

Think of an Index as your aggregate root :)

[Matt Warren]

BTW is this change in response to this SO qu http://stackoverflow.com/questions/9029400/ravendb-why-is-transformresults-placed-in-index?

[Chris Marisic]

On Friday, May 18, 2012 9:42:49 AM UTC-4, Itamar Syn-Hershko wrote:

inline

On Fri, May 18, 2012 at 3:39 PM, Chris Marisic <ch...@marisic.com> wrote:

I think these changes make alot of sense.

One suggestion I'd like to make, I think it'd be rare you would ever TRULY use

 .TransformResults<ProjectionType>("default")

 .TransformResults<ProjectionType>("view")

What I think would be far more useful would be supporting:

TransformResult = (db, results) => ... ;

OR

TransformResults = { { "ProjectionType", (db, results) => ... }, 

{ "SomeOtherProjectionType", (db, results) => ... } }


This would allow the user to see easier configuration for when there's only ever 1 transform, which can just behind the scenes load the TransformResults without adding obtuse syntax to configure 1 item.

If we will do that, it will only be by translating the first one to the second (creating a dictionary and adding it as "default"). See more below.

 

For your notions of default, I'd just pick the first one in the list if you don't have a typename match. (coincidentally also solves the untyped based usage). Theoretically typeof(ProjectionType) might be better, but i think the looser coupling the strings provide probably will pay out more in the long run. Users that are fine tight coupling to classes can just do typeof(ProjectionType).Name

This would introduce friction. There should really be only one way of doing this, even if it breaks old habits from v1.0...

I'm not sure what friction you're talking about. Supporting TransformResult that implicitly uses TransformResults is absolutely zero friction. 90% of your users will never have more 1 than transform (maybe even 98%+). Forcing every user to define TransformResults when in almost all scenarios ever will they only need 1 transform is friction.
 

 

When would you ever really do

 .TransformResults<ProjectionType>("default")

 .TransformResults<ProjectionType>("view")

As opposed to


 .TransformResults<ProjectionType>()
 .TransformResults<SomeOtherProjectionType>()

If for some crazy reason, you really needed 2 transforms into the same type, you could still work entirely within the typed api

I'm not sure where you got the idea of 2 chained transformations. The idea is to support one at a time, letting you choose which will be executed (if at all)

As you said, I cannot see any use for chained transformations, and if you really need that just write a new TR function doing what you need

Also, it can be quite heavy on the server, chaining transformations

 

I'm not talking about chained transform I'm talking about why in the world would you ever have

session.Query<Type, Index>()

             .Where(...)

             .TransformResults<ProjectionType>("default")

             .ToList()

session.Query<Type, Index>()

             .Where(...)

             .TransformResults<ProjectionType>("view")

             .ToList()

 That makes no sense to be mapping multiple shapes of transform into the same type.

[Itamar Syn-Hershko]

No

[Itamar Syn-Hershko]

For your notions of default, I'd just pick the first one in the list if you don't have a typename match. (coincidentally also solves the untyped based usage). Theoretically typeof(ProjectionType) might be better, but i think the looser coupling the strings provide probably will pay out more in the long run. Users that are fine tight coupling to classes can just do typeof(ProjectionType).Name

This would introduce friction. There should really be only one way of doing this, even if it breaks old habits from v1.0...

I'm not sure what friction you're talking about. Supporting TransformResult that implicitly uses TransformResults is absolutely zero friction. 90% of your users will never have more 1 than transform (maybe even 98%+). Forcing every user to define TransformResults when in almost all scenarios ever will they only need 1 transform is friction.

Picking the first TR func as a default operation

As I said, it is valid to have TransformResults = (db, results) => ... work, but it will just translate it to a dictionary of funcs with this TR having a key of "default".

The default operation, as I see it, will not call any TR unless explicitly asked by the user in the Query, and then the one under the key "default" will be used unless a named TR was specified

I also don't think this should be tied to a typename - you can have one TR func but load it to different ViewModels (one with a property and one without). Unlike documents, this should really be type agnostic. Types for TR should only be handled by the client.

 

 

 

When would you ever really do

 .TransformResults<ProjectionType>("default")

 .TransformResults<ProjectionType>("view")

As opposed to


 .TransformResults<ProjectionType>()
 .TransformResults<SomeOtherProjectionType>()

If for some crazy reason, you really needed 2 transforms into the same type, you could still work entirely within the typed api

I'm not sure where you got the idea of 2 chained transformations. The idea is to support one at a time, letting you choose which will be executed (if at all)

As you said, I cannot see any use for chained transformations, and if you really need that just write a new TR function doing what you need

Also, it can be quite heavy on the server, chaining transformations

 

I'm not talking about chained transform I'm talking about why in the world would you ever have

session.Query<Type, Index>()

             .Where(...)

             .TransformResults<ProjectionType>("default")

             .ToList()

session.Query<Type, Index>()

             .Where(...)

             .TransformResults<ProjectionType>("view")

             .ToList()

 That makes no sense to be mapping multiple shapes of transform into the same type.

See above

[Chris Marisic]

The OP of that question including the suggestion to use Select to define transform results is rather brilliant. That actually be might be the best solution of all. That makes the transforms exist purely on the client caller. This would still allow for easy reuse of transforms by a user just sticking a func into Select().

A way that Raven could support this with a cleaner reusable way might be something similar to the way indexes are defined

SomeProjection : TransformResults<Type, IndexType>
{ ... }

Session.Query.Where.Select<SomeProjection>() with that select<> coming from the Raven.Client.Linq namespace. This does get into the seperate files, but having them be a facet of select does very appropriately model the transaction boundary.

[Itamar Syn-Hershko]

The whole point is to have TR on the server, this way you can load more docs, filter properties, and let the server do the heavy lifting

[Matt Warren]

Well you could combine the 2 ideas by using Expression<Func<..>>. So you parse it and send the text representation over to the server.

So you define the TR in the Select part of the query, but that gets sent to the server and executed there?

[Chris Marisic]

TransformResults gets executed at query runtime, why does it matter whether it's defined client side purely?

obviously it has to be ran session.Query.Where.Select.ToList, not session.Query.Where.ToList.Select

[Itamar Syn-Hershko]

Too much chatter, and would probably also force us to start using POST only

[Chris Marisic]

I think you want feedback on this part from a user who would actually use this feature. Honestly I've never even had a scenario that it even crossed my mind that multiple transforms would have any benefit. So I certainly would never be using more than 1.

[Itamar Syn-Hershko]

I agree

Another factor to consider - API confusion. I'm pretty sure we will get loads of questions if TR functions were to be defined in the 

There should be a strict line separating the query stuff and the post-query processing (which by design should be kept to the bare minimum), and between client-side post-query processing to server-side p.q.p.

As you said yourself, it is going to be too confusing Select.ToList vs ToList.Select

[Chris Marisic]

On Friday, May 18, 2012 10:30:29 AM UTC-4, Itamar Syn-Hershko wrote:

As you said yourself, it is going to be too confusing Select.ToList vs ToList.Select

I don't agree that's too confusing. This is exactly the pattern EntityFramework and other ORMs follow for doing projection type mapping.