Pass session around

[erizocosmico]

We should have a Session to pass around to all nodes that need to be evaluated. With this session, we can pass dependencies around (such as the repository pool for gitquery) and any other dependencies that are needed elsewhere in the code along with user data, if any.

Possible use cases for this:

• request-scoped metrics
• dependency injection
• passing of session data

Any thoughts? /cc @ajnavarro @mcarmonaa @jfontan

Proposal:

A way of implementing this in a way that's not very disruptive to the current API would be the following:

• Implement a Session type. It could either be a concrete type or an interface with a base implementation that can be extended to provide extra features (this option could be really powerful for gitquery).

type SessionData struct { /* yada yada */ }

type Session interface {
	Data() SessionData
        // yada yada
}

type BaseSession struct {
	data SessionData
}

func (s BaseSession) Data() SessionData {
	return s.data
}

// another file or even another package

type CustomSession struct {
	BaseSession
	mything MyType
}

// inside an UDF, for example
customSess, ok := session.(*CustomSession)
if ok {
	customSess.mything.foo()
}

• Change the signature of Expression.Eval to Eval(Session, Row) (interface{}, error)
• Change the signature of Node.RowIter to RowIter(Session) (RowIter, error)
• Change the signature of AggregationExpression.Update to Update(session Session, buffer, row Row) error
• Change the signature of AggregationExpression.Merge to Merge(session Session, buffer, partial Row) error
• Change the signature of Aggregation.Update to Update(Session, Row) (Row, error)
• Change the signature of Aggregation.Merge to Merge(Session, Row)
• Change the signature of Aggregation.Eval to Eval(Session) (interface{}, error) (and make it return an error, while we are at it

That's about all the places that would need the session passed around. Then it's the job of whoever calls these methods to provide them the session and so on.

[ajnavarro]

Hmm, in my opinion, I see the session more like a global entity that can be accessed from any place, more than a parameter that is passed throught the tree. What do you think, can yo add more explained use cases for this?

[erizocosmico]

But how can it be accessed from elsewhere? Like a singleton? The UDFs, for example, would need it for dependency injection and so on.

[ajnavarro]

We were thinking about having a Context instead of a Session at that level. Then we can have a Session into the Context, and also we can cancel queries executions.

[erizocosmico]

Makes sense. On Tue, 27 Feb 2018 at 16:44, Antonio Navarro Perez <notifications@github.com<mailto:notifications@github.com>> wrote: We were thinking about having a Context instead of a Session at that level. Then we can have a Session into the Context, and also we can cancel queries executions. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub<#36 (comment)>, or mute the thread<https://github.com/notifications/unsubscribe-auth/ABQFF1DVv-NCpbWDW5Va-vJ-uTuNAkFEks5tZCLggaJpZM4SN3Co>.

[ajnavarro]

Going ahead with the first proposal, but Session will implement Context too.

[erizocosmico]

Should we also make it have a Cancel method? It depends if we want to let nodes control when the session is cancelled or not. On Thu, 1 Mar 2018 at 12:09, Antonio Navarro Perez <notifications@github.com<mailto:notifications@github.com>> wrote: Going ahead with the first proposal, but Session will implement Context too. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub<#36 (comment)>, or mute the thread<https://github.com/notifications/unsubscribe-auth/ABQFFy9Pf6vK4p2lFwHlRcvz2VD8lJDSks5tZ9bQgaJpZM4SN3Co>.

[ajnavarro]

I think nodes shouldn't cancel a session... Can you put an use case for that?

[erizocosmico]

Yeah, you're right, nodes shouldn't.