Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

colexecbase: fix bpchar to bpchar cast #85869

Merged
merged 1 commit into from
Aug 10, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions pkg/sql/colexec/colexecbase/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -74,14 +74,12 @@ go_test(
"//pkg/sql/randgen",
"//pkg/sql/sem/cast",
"//pkg/sql/sem/eval",
"//pkg/sql/sem/tree",
"//pkg/sql/types",
"//pkg/testutils/skip",
"//pkg/util/leaktest",
"//pkg/util/log",
"//pkg/util/mon",
"//pkg/util/randutil",
"@com_github_lib_pq//oid",
"@com_github_stretchr_testify//require",
],
)
Expand Down
54 changes: 53 additions & 1 deletion pkg/sql/colexec/colexecbase/cast.eg.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

33 changes: 4 additions & 29 deletions pkg/sql/colexec/colexecbase/cast_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,10 @@ import (
"github.com/cockroachdb/cockroach/pkg/sql/randgen"
"github.com/cockroachdb/cockroach/pkg/sql/sem/cast"
"github.com/cockroachdb/cockroach/pkg/sql/sem/eval"
"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
"github.com/cockroachdb/cockroach/pkg/sql/types"
"github.com/cockroachdb/cockroach/pkg/util/leaktest"
"github.com/cockroachdb/cockroach/pkg/util/log"
"github.com/cockroachdb/cockroach/pkg/util/randutil"
"github.com/lib/pq/oid"
"github.com/stretchr/testify/require"
)

Expand All @@ -48,12 +46,6 @@ func TestRandomizedCast(t *testing.T) {
getValidSupportedCast := func() (from, to *types.T) {
for {
from, to = randgen.RandType(rng), randgen.RandType(rng)
if from.Oid() == oid.T_void && to.Oid() == oid.T_bpchar {
// Skip the cast from void to char because such setup would get
// stuck forever in the datum generation (due to the TODO
// below).
continue
}
if _, ok := cast.LookupCastVolatility(from, to); ok {
if colexecbase.IsCastSupported(from, to) {
return from, to
Expand All @@ -73,27 +65,10 @@ func TestRandomizedCast(t *testing.T) {
toConverter := colconv.GetDatumToPhysicalFn(to)
errorExpected := false
for i := 0; i < numRows; i++ {
var (
fromDatum, toDatum tree.Datum
err error
)
// Datum generation. The loop exists only because of the TODO below.
for {
// We don't allow any NULL datums to be generated, so disable
// this ability in the RandDatum function.
fromDatum = randgen.RandDatum(rng, from, false)
toDatum, err = eval.PerformCast(&evalCtx, fromDatum, to)
if to.Oid() == oid.T_bpchar && string(*toDatum.(*tree.DString)) == "" {
// There is currently a problem when converting an empty
// string datum to a physical representation, so we skip
// such a datum and retry generation.
// TODO(yuzefovich): figure it out. When removing this
// check, remove the special casing for 'void -> char' cast
// above.
continue
}
break
}
// We don't allow any NULL datums to be generated, so disable this
// ability in the RandDatum function.
fromDatum := randgen.RandDatum(rng, from, false)
toDatum, err := eval.PerformCast(&evalCtx, fromDatum, to)
var toPhys interface{}
if err != nil {
errorExpected = true
Expand Down
54 changes: 53 additions & 1 deletion pkg/sql/colexec/colexecbase/cast_tmpl.go
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,15 @@ func GetCastOperator(
return &castOpNullAny{castOpBase: base}, nil
}
if isIdentityCast(fromType, toType) {
return &castIdentityOp{castOpBase: base}, nil
// bpchars require special handling.
if toType.Oid() == oid.T_bpchar {
return &castBPCharIdentityOp{castOpBase: base}, nil
}
// If we don't have an array of bpchars, then we use the identity,
// otherwise we'll fallback to datum-datum cast below.
if toType.Oid() != oid.T__bpchar {
return &castIdentityOp{castOpBase: base}, nil
}
}
isFromDatum := typeconv.TypeFamilyToCanonicalTypeFamily(fromType.Family()) == typeconv.DatumVecCanonicalTypeFamily
isToDatum := typeconv.TypeFamilyToCanonicalTypeFamily(toType.Family()) == typeconv.DatumVecCanonicalTypeFamily
Expand Down Expand Up @@ -322,6 +330,50 @@ func (c *castIdentityOp) Next() coldata.Batch {
return batch
}

// castBPCharIdentityOp is a specialization of castIdentityOp which handles
// casts to the bpchar type (which trims trailing whitespaces).
type castBPCharIdentityOp struct {
castOpBase
}

var _ colexecop.ClosableOperator = &castBPCharIdentityOp{}

func (c *castBPCharIdentityOp) Next() coldata.Batch {
batch := c.Input.Next()
n := batch.Length()
if n == 0 {
return coldata.ZeroBatch
}
inputVec := batch.ColVec(c.colIdx)
inputCol := inputVec.Bytes()
inputNulls := inputVec.Nulls()
outputVec := batch.ColVec(c.outputIdx)
outputCol := outputVec.Bytes()
outputNulls := outputVec.Nulls()
// Note that the loops below are not as optimized as in other cast operators
// since this operator should only be planned in tests.
c.allocator.PerformOperation([]coldata.Vec{outputVec}, func() {
if sel := batch.Selection(); sel != nil {
for _, i := range sel[:n] {
if inputNulls.NullAt(i) {
outputNulls.SetNull(i)
} else {
outputCol.Set(i, bytes.TrimRight(inputCol.Get(i), " "))
}
}
} else {
for i := 0; i < n; i++ {
if inputNulls.NullAt(i) {
outputNulls.SetNull(i)
} else {
outputCol.Set(i, bytes.TrimRight(inputCol.Get(i), " "))
}
}
}
})
return batch
}

type castNativeToDatumOp struct {
castOpBase

Expand Down