Query by reference?

[elidupree]

An event might want to look around in a medium-sized DataTimeline, in a way that would be more efficient using references than by first copying all the data it might be going to use.

This is tricky because it involves making the query API much more complex, and probably returning guards rather than plain references.

There might be other approaches that could accomplish the same thing.

[elidupree]

Tentative conclusion: return guards, and only support single references.

We would like to support returning for complex reference structures, like pairs of references. branch query_by_reference_attempt_1 tried to do this, by making trait PossiblyBorrowedStewardData which was implemented for all StewardData, &StewardData, and other things, which supported getting a clone, "borrowing" from owned data, and using as a reference. However, it wasn't able to implement for tuples even using specialization, because of conflicting impls. Furthermore, it was impossible to return a std::cell::Ref to any object except for a single reference.

Aside from returning guards, the main other approach is to pass in a closure that takes the &QueryResult as its argument.

In practice, that will be functionally equivalent – both of them make a guard exist within a particular scope, the closure just looks weirder. The closure approach doesn't have the problem with std::cell::Ref, but we COULD solve that in the guard approach by just returning a pair (Ref<'a, _>, Whatever<'a>) (probably implementing our own guard type that is like that internally).

I don't currently have a use case for returning complex structures.

Detailed design: we currently have

pub trait DataTimelineQueriableWith<Query: StewardData>: DataTimeline {
  type QueryResult: StewardData;
  
  fn query (&self, query: &Query, time: &ExtendedTime <Self::Basics>, offset: QueryOffset)->Self::QueryResult;
}

(That's going to change due to some other API issues, but I'm ignoring that for now.)

I propose to add:

pub trait DataTimelineQueryRefableWith<Query: StewardData>: DataTimelineQueriableWith<Query> {
  fn query_ref (&self, query: &Query, time: &ExtendedTime <Self::Basics>, offset: QueryOffset)->&<Self as DataTimelineQueriableWith<Query>>::QueryResult;
}

And Accessor will have corresponding additional query_ref() function that returns a guard.

This allows auditing code to clone the result and test events by feeding them references to the clone. Auditing code would also audit that query() and query_ref() always return equal results.

[elidupree]

A similar approach for more complex structures would look something like this:

pub trait DataTimelineQueryGeneralizedRefableWith<'a, Query: StewardData>: DataTimelineQueriableWith<Query> {
  type QueryResultReference : GeneralizedReference <'a, <Self as DataTimelineQueriableWith<Query>>::QueryResult>;
  fn query_ref (&self, query: &Query, time: &ExtendedTime <Self::Basics>, offset: QueryOffset)->Self::QueryResultReference;
}

trait GeneralizedReference <'a, T> {
  fn clone_to_owned (&self)->T;
  fn from_ref (source: &'a T)->Self;
}

Those methods are specifically the ones needed to store copies of the queried value, and feed those copies back into later queries.

I believe GeneralizedReference COULD be blanket-implemented for all &T, Option<GeneralizedReference<'a,T>>, (GeneralizedReference<'a,T0>, ...), etc.. It was only trying to implement PossiblyBorrowedStewardData for T itself that caused conflicting impl issues.

This would require us manually implementing a more complex guard type for Accessor::query_ref(), and it wouldn't be the same type as the current DataTimelineReadGuard, and it seems wrong to make peek() more inefficient just to simplify the code a little in a manner dependent on a quirk of the standard library, so it would be yet another return value that should be a generic associated type or impl Trait but can't be because those features aren't available yet.

Also, I believe this feature can be grafted on later without breaking existing uses. In the worst case, it could be added as a third query trait/function and definitely not break existing uses.

Conclusion: Save labor by not implementing this generalized version of this now, because the features and/or requirements could change by the time we get around to it. However, do implement basic query_ref() because it's simpler and its absence is much more glaring.

[elidupree]

Implemented the basic version. Closing this issue for now, although we might make a new issue for the more generalized version at some point in the future.