coldataext: remove a wrapper and use tree.Datum directly

Previously, we had separate `coldataext.Datum` wrapper around
`tree.Datum`. I don't remember what was the reasoning behind introducing
it (probably so that we could define methods with the wrapper as the
receiver in `coldataext` package), but it seems unnecessary. It also has
some performance cost because on every `DatumVec.Get` we are currently
allocating a new object on the heap. This commit removes the wrapper in
favor of working with `tree.Datum`s directly. This work was prompted by
looking at some profiles around the memory accounting for datum-backed
types in the cFetcher.

Release note: None

pkg/col/coldataext/datum_vec.go

@@ -24,14 +24,6 @@ import (
 	"github.com/cockroachdb/errors"
 )
 
-// Datum wraps a tree.Datum. This is the struct that datumVec.Get() returns.
-type Datum struct {
-	tree.Datum
-}
-
-var _ coldata.Datum = &Datum{}
-var _ tree.Datum = &Datum{}
-
 // datumVec is a vector of tree.Datums of the same type.
 type datumVec struct {
 	// t is the type of the tree.Datums that datumVec stores.
@@ -56,27 +48,19 @@ func newDatumVec(t *types.T, n int, evalCtx *tree.EvalContext) coldata.DatumVec
 	}
 }
 
-// BinFn evaluates the provided binary function between the receiver and other.
-// other can either be nil, tree.Datum, or *Datum.
-func (d *Datum) BinFn(
-	binFn tree.TwoArgFn, evalCtx *tree.EvalContext, other interface{},
-) (tree.Datum, error) {
-	return binFn(evalCtx, d.Datum, maybeUnwrapDatum(other))
-}
-
 // CompareDatum returns the comparison between d and other. The other is
 // assumed to be tree.Datum. dVec is the datumVec that stores either d or other
 // (it doesn't matter which one because it is used only to supply the eval
 // context).
 // Note that the method is named differently from "Compare" so that we do not
 // overload tree.Datum.Compare method.
-func (d *Datum) CompareDatum(dVec, other interface{}) int {
-	return d.Datum.Compare(dVec.(*datumVec).evalCtx, maybeUnwrapDatum(other))
+func CompareDatum(d, dVec, other interface{}) int {
+	return d.(tree.Datum).Compare(dVec.(*datumVec).evalCtx, convertToDatum(other))
 }
 
 // Hash returns the hash of the datum as a byte slice.
-func (d *Datum) Hash(da *rowenc.DatumAlloc) []byte {
-	ed := rowenc.EncDatum{Datum: maybeUnwrapDatum(d)}
+func Hash(d tree.Datum, da *rowenc.DatumAlloc) []byte {
+	ed := rowenc.EncDatum{Datum: convertToDatum(d)}
 	// We know that we have tree.Datum, so there will definitely be no need to
 	// decode ed for fingerprinting, so we pass in nil memory account.
 	b, err := ed.Fingerprint(context.TODO(), d.ResolvedType(), da, nil /* appendTo */, nil /* acc */)
@@ -89,24 +73,15 @@ func (d *Datum) Hash(da *rowenc.DatumAlloc) []byte {
 	return b
 }
 
-// Size returns a lower bound on the total size of the receiver in bytes,
-// including memory that is pointed at (even if shared between Datum
-// instances) but excluding allocation overhead.
-//
-// It is assumed that d was obtained via datumVec.Get().
-func (d *Datum) Size() uintptr {
-	return d.Datum.Size()
-}
-
 // Get implements coldata.DatumVec interface.
 func (dv *datumVec) Get(i int) coldata.Datum {
 	dv.maybeSetDNull(i)
-	return &Datum{Datum: dv.data[i]}
+	return dv.data[i]
 }
 
 // Set implements coldata.DatumVec interface.
 func (dv *datumVec) Set(i int, v coldata.Datum) {
-	datum := maybeUnwrapDatum(v)
+	datum := convertToDatum(v)
 	dv.assertValidDatum(datum)
 	dv.data[i] = datum
 }
@@ -136,7 +111,7 @@ func (dv *datumVec) AppendSlice(src coldata.DatumVec, destIdx, srcStartIdx, srcE
 
 // AppendVal implements coldata.DatumVec interface.
 func (dv *datumVec) AppendVal(v coldata.Datum) {
-	datum := maybeUnwrapDatum(v)
+	datum := convertToDatum(v)
 	dv.assertValidDatum(datum)
 	dv.data = append(dv.data, datum)
 }
@@ -230,15 +205,12 @@ func (dv *datumVec) maybeSetDNull(i int) {
 	}
 }
 
-// maybeUnwrapDatum possibly unwraps tree.Datum from inside of Datum. It also
-// checks whether v is nil and returns tree.DNull if it is.
-func maybeUnwrapDatum(v coldata.Datum) tree.Datum {
+// convertToDatum converts v to the corresponding tree.Datum.
+func convertToDatum(v coldata.Datum) tree.Datum {
 	if v == nil {
 		return tree.DNull
 	}
-	if datum, ok := v.(*Datum); ok {
-		return datum.Datum
-	} else if datum, ok := v.(tree.Datum); ok {
+	if datum, ok := v.(tree.Datum); ok {
 		return datum
 	}
 	colexecerror.InternalError(errors.AssertionFailedf("unexpected value: %v", v))

pkg/col/coldataext/datum_vec_test.go

@@ -21,40 +21,6 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
-func TestDatum(t *testing.T) {
-	defer leaktest.AfterTest(t)()
-
-	dv := &datumVec{evalCtx: &tree.EvalContext{}}
-	var d1 *Datum
-	var d2 tree.Datum
-	d1 = &Datum{Datum: &tree.DJSON{JSON: json.FromString("string")}}
-	d2 = &tree.DJSON{JSON: json.FromString("string")}
-
-	// Datum can be compared with regular datum.
-	require.True(t, d1.CompareDatum(dv, d2) == 0)
-
-	d2 = &Datum{Datum: d2}
-
-	// Datum can be compared with another Datum.
-	require.True(t, d1.CompareDatum(dv, d2) == 0)
-
-	// Datum implicitly views nil as tree.DNull.
-	require.True(t, d1.CompareDatum(dv, tree.DNull) == d1.CompareDatum(dv, nil /* other */))
-
-	// Datum panics if compared with incompatible type.
-	d2 = tree.NewDString("s")
-	require.Panics(t,
-		func() { d1.CompareDatum(dv, d2) },
-		"different datum type should have caused panic when compared",
-	)
-
-	d2 = &Datum{Datum: d2}
-	require.Panics(t,
-		func() { d1.CompareDatum(dv, d2) },
-		"different datum type should have caused panic when compared",
-	)
-}
-
 func TestDatumVec(t *testing.T) {
 	defer leaktest.AfterTest(t)()
 

pkg/sql/colexec/colexecagg/any_not_null_agg_tmpl.go

@@ -24,7 +24,6 @@ import (
 
 	"github.com/cockroachdb/apd/v2"
 	"github.com/cockroachdb/cockroach/pkg/col/coldata"
-	"github.com/cockroachdb/cockroach/pkg/col/coldataext"
 	"github.com/cockroachdb/cockroach/pkg/col/typeconv"
 	"github.com/cockroachdb/cockroach/pkg/sql/colexec/execgen"
 	"github.com/cockroachdb/cockroach/pkg/sql/colexecerror"
@@ -44,7 +43,6 @@ var (
 	_ duration.Duration
 	_ json.JSON
 	_ colexecerror.StorageError
-	_ coldataext.Datum
 )
 
 // {{/*

pkg/sql/colexec/colexecagg/min_max_agg_tmpl.go

@@ -43,7 +43,7 @@ var (
 	_ apd.Context
 	_ duration.Duration
 	_ json.JSON
-	_ coldataext.Datum
+	_ = coldataext.CompareDatum
 )
 
 // Remove unused warning.