Skip to content
This repository has been archived by the owner on Jan 28, 2021. It is now read-only.

Make sql.Expression interface de/serializable... #194

Closed
kuba-- opened this issue May 17, 2018 · 4 comments · Fixed by #200
Closed

Make sql.Expression interface de/serializable... #194

kuba-- opened this issue May 17, 2018 · 4 comments · Fixed by #200
Assignees
Labels
proposal proposal for new additions or changes
Milestone

Comments

@kuba--
Copy link
Contributor

kuba-- commented May 17, 2018

In IndexDriver implementation, we have to deal with sql.Expression interface (serialize it in Create and deserialize in Load). Deserialize an interface in go is not really possible, so here are couple of proposals how to make sql.Expression type more flexible to work with:

  • Add (Un)Marshal functions, so everyone who implements the interface will have to know how to serialize it to []byte and how to deserialize it from []byte.
  • Create ExpressionRegistry and all concrete implementations will have to register Expression type with unique name in global registry. And downstream this information to components like IndexDriver where information about concrete implementation is needed.
  • Attach some kind of hash reference to every sql.Expression implementation and in components like IndexDriver, sql.Index, ... use only hash reference strings instead of expression objects.

Right now sql.Expression interface is:

// Expression is a combination of one or more SQL expressions.
type Expression interface {
	Resolvable
	fmt.Stringer
	// Type returns the expression type.
	Type() Type
	// IsNullable returns whether the expression can be null.
	IsNullable() bool
	// Eval evaluates the given row and returns a result.
	Eval(*Context, Row) (interface{}, error)
	// TransformUp transforms the expression and all its children with the
	// given transform function.
	TransformUp(TransformExprFunc) (Expression, error)
	// Children returns the children expressions of this expression.
	Children() []Expression
}

For instance in Pilosa driver apart from (de)serialize it in Load, Create we also iterate through all expression in Save (because we treat them as column names) and map them to Pilosa frame names. To make it possible we just convert them to string just calling String() function:

for i, e := range idx.Expressions() {
		encodedFrameName := encode(e.String())
		frames[i], err = index.Frame(encodedFrameName)
	...

So for index driver it would be better if expressions were just a slice of strings.
This task should be accomplished before we start continue working on: #174

@kuba-- kuba-- added the proposal proposal for new additions or changes label May 17, 2018
@ajnavarro ajnavarro added this to the Index-1 milestone May 17, 2018
@ajnavarro
Copy link
Contributor

Should we abstract all this logic from Index implementations, and just send ExpressionsIDs?

Per example:

type Index interface {
...
Expressions() []Expression
...
}

Should be:

type ExpressionID string

type Index interface {
...
ExpressionIDs() []ExpressionID
...
}

And in the case of IndexDriver interface, we can do the same.

All the encode logic can be on top of the Index implementation, and generic. WDYT?

@kuba--
Copy link
Contributor Author

kuba-- commented May 17, 2018

I'm OK with any type which can be easily (de)serialized and mapped to string and/or []byte.

@kuba--
Copy link
Contributor Author

kuba-- commented May 17, 2018

Maybe encoding/gob can help in some way, but it requires to register a type. Here I've played a little bit with deserialising interface:
https://play.golang.org/p/bVT60uj3aXw

@kuba--
Copy link
Contributor Author

kuba-- commented May 21, 2018

PR: #200

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
proposal proposal for new additions or changes
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants