exec: add interface skeletons

[jordanlewis]

Add the interface skeletons for column vector operators.

Release note: None

[cockroach-teamcity]

This change is 

[cockroach-teamcity]

This change is 

[asubiotto]

Nice. Are these interfaces so we can start working independently or are you planning on keeping them as interfaces?

Reviewable status: complete! 0 of 0 LGTMs obtained

---

pkg/sql/exec/colbatch.go, line 39 at r1 (raw file):

	// slice of columns in this batch.
	b      []ColVec
	useSel bool

What about getting rid of this boolean and checking if sel != nil for the cases you would want to use the selection vector.

---

pkg/sql/exec/colbatch.go, line 41 at r1 (raw file):

	useSel bool
	// if useSel is true, a selection vector from upstream. a selection vector is
	// a list of selected column indexes in this dataFlow's columns.

I think the term dataFlow is undefined here?

---

pkg/sql/exec/colvec.go, line 19 at r1 (raw file):

// ColVec is an interface that represents a column vector that's accessible by
// Go native types.
type ColVec interface {

Do we also want to have a Type() byte method?

---

pkg/sql/exec/colvec.go, line 22 at r1 (raw file):

	Nulls

	// TODO(jordan): is a bitmap or slice of bools better?

I'm wondering about this too. Why is it an interface?

---

pkg/sql/exec/colvec.go, line 130 at r1 (raw file):

type memBytes struct {
	col [][]byte

We might want to actually make this a single slice of []byte with associated offsets. I guess this is fine for now and you're doing this for simplicity.

[jordanlewis]

These will stay interfaces. It's okay to have interfaces at this level of the code - as long as they spit out the actual data vectors for the operators to operate on. It's also useful to have multiple implementations of at least ColBatch - one is this simple, in-memory slices one, which is convenient for testing, and one is the ptable block format which is more memory efficient but harder to use.

Reviewable status: complete! 0 of 0 LGTMs obtained

---

pkg/sql/exec/colbatch.go, line 39 at r1 (raw file):

Previously, asubiotto (Alfonso Subiotto Marqués) wrote…

What about getting rid of this boolean and checking if sel != nil for the cases you would want to use the selection vector.

That won't work - you need to pass along the selection vector memory so downstream operators can set and edit it. So you do need the boolean.

---

pkg/sql/exec/colbatch.go, line 41 at r1 (raw file):

Previously, asubiotto (Alfonso Subiotto Marqués) wrote…

I think the term dataFlow is undefined here?

Oops, done.

---

pkg/sql/exec/colvec.go, line 19 at r1 (raw file):

Previously, asubiotto (Alfonso Subiotto Marqués) wrote…

Do we also want to have a Type() byte method?

Good question. I don't think it's necessary to keep a type in this interface, because every operator will statically know the type of all of its columns. Maybe it'll be convenient to have it later, but for now it's not necessary.

---

pkg/sql/exec/colvec.go, line 22 at r1 (raw file):

Previously, asubiotto (Alfonso Subiotto Marqués) wrote…

I'm wondering about this too. Why is it an interface?

I'm copying this from Peter's code in pebble. It's possible that this should just be a slice of bools, but then that doesn't let us use a bitmap if that's faster. I'm not sure - but this will give us a place to start. It'll be easy enough to change later.

---

pkg/sql/exec/colvec.go, line 130 at r1 (raw file):

Previously, asubiotto (Alfonso Subiotto Marqués) wrote…

We might want to actually make this a single slice of []byte with associated offsets. I guess this is fine for now and you're doing this for simplicity.

Yeah, the memFoo structures are not intended to be the efficient ones. I'm going to add that in soon - it'll be accessors to the ptable block format in pebble.

It remains to be seen via benchmarking and experimentation whether we want to abandon any function calls for accessing the varlen datatypes. exectoy didn't have any of that stuff, so we'll have to start somewhere. This interface is copied from pebble - it's a place to start.

[asubiotto]

Reviewable status: complete! 0 of 0 LGTMs obtained

---

pkg/sql/exec/colbatch.go, line 39 at r1 (raw file):

Previously, jordanlewis (Jordan Lewis) wrote…

That won't work - you need to pass along the selection vector memory so downstream operators can set and edit it. So you do need the boolean.

As discussed in slack, this might not be necessary but you think it's clearer. I personally think that removing this boolean is clearer than not doing so and avoids the possibility of weird states (i.e. what does it mean when useSel is false but sel is a slice). Do whatever you think is right, don't want this to block the PR.

[jordanlewis]

(i.e. what does it mean when useSel is false but sel is a slice)

You're right, but it's hard to figure out what to do one way or the other until we actually have operators to look at. Going to merge as-is just as a straw man.

bors r+

[jordanlewis]

(i.e. what does it mean when useSel is false but sel is a slice)

You're right, but it's hard to figure out what to do one way or the other until we actually have operators to look at. Going to merge as-is just as a straw man.

bors r+

[craig]

Build succeeded

• GitHub CI (Cockroach)

[craig]

Build succeeded

• GitHub CI (Cockroach)