Skip to content

Commit

Permalink
sql: support int2vector::string casts properly
Browse files Browse the repository at this point in the history
Previously, casting an int2vector to a string would fail to use the
special int2vector formatting (space-separated elements instead of the
normal {}-wrapped and comma-separated elements format of arrays). This
was causing issues with SQLAlchemy.

This commit solves this problem and simplifies the code by removing the
DOidWrapper usages for int2vector and oidvector, in favor of a field on
DArray that contains a custom oid. This opens the door to teaching
DArray's format method about the special formatting, rather than having
it live only in pgwire, where it fails to be useful in the cast of e.g.
casts.

Release note (sql change): casting an int2vector to a string now
produces a Postgres-compatible result.
  • Loading branch information
jordanlewis committed Jun 21, 2019
1 parent d287001 commit 3f4d1a0
Show file tree
Hide file tree
Showing 4 changed files with 38 additions and 22 deletions.
17 changes: 2 additions & 15 deletions pkg/sql/pgwire/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -176,21 +176,8 @@ func (b *writeBuffer) writeTextDatum(
b.writeFromFmtCtx(b.textFormatter)

case *tree.DArray:
switch d.ResolvedType().Oid() {
case oid.T_int2vector, oid.T_oidvector:
// vectors are serialized as a string of space-separated values.
sep := ""
// TODO(justin): add a test for nested arrays when #32552 is
// addressed.
for _, d := range v.Array {
b.textFormatter.WriteString(sep)
b.textFormatter.FormatNode(d)
sep = " "
}
default:
// Uses the default pgwire text format for arrays.
b.textFormatter.FormatNode(v)
}
// Arrays have custom formatting depending on their OID.
b.textFormatter.FormatNode(d)
b.writeFromFmtCtx(b.textFormatter)

case *tree.DOid:
Expand Down
19 changes: 17 additions & 2 deletions pkg/sql/sem/tree/datum.go
Original file line number Diff line number Diff line change
Expand Up @@ -2993,6 +2993,9 @@ type DArray struct {
// HasNonNulls is set to true if any of the datums within the are non-null.
// This is used in expression serialization (FmtParsable).
HasNonNulls bool

// customOid, if non-0, is the oid of this array datum.
customOid oid.Oid
}

// NewDArray returns a DArray containing elements of the specified type.
Expand Down Expand Up @@ -3026,6 +3029,12 @@ func MustBeDArray(e Expr) *DArray {

// ResolvedType implements the TypedExpr interface.
func (d *DArray) ResolvedType() *types.T {
switch d.customOid {
case oid.T_int2vector:
return types.Int2Vector
case oid.T_oidvector:
return types.OidVector
}
return types.MakeArray(d.ParamTyp)
}

Expand Down Expand Up @@ -3514,13 +3523,19 @@ func NewDName(d string) Datum {
// NewDIntVectorFromDArray is a helper routine to create a *DIntVector
// (implemented as a *DOidWrapper) initialized from an existing *DArray.
func NewDIntVectorFromDArray(d *DArray) Datum {
return wrapWithOid(d, oid.T_int2vector)
ret := new(DArray)
*ret = *d
ret.customOid = oid.T_int2vector
return ret
}

// NewDOidVectorFromDArray is a helper routine to create a *DOidVector
// (implemented as a *DOidWrapper) initialized from an existing *DArray.
func NewDOidVectorFromDArray(d *DArray) Datum {
return wrapWithOid(d, oid.T_oidvector)
ret := new(DArray)
*ret = *d
ret.customOid = oid.T_oidvector
return ret
}

// DatumTypeSize returns a lower bound on the total size of a Datum
Expand Down
8 changes: 3 additions & 5 deletions pkg/sql/sem/tree/eval.go
Original file line number Diff line number Diff line change
Expand Up @@ -3610,11 +3610,9 @@ func (expr *IndirectionExpr) Eval(ctx *EvalContext) (Datum, error) {
arr := MustBeDArray(d)

// VECTOR types use 0-indexing.
if w, ok := d.(*DOidWrapper); ok {
switch w.Oid {
case oid.T_oidvector, oid.T_int2vector:
subscriptIdx++
}
switch arr.customOid {
case oid.T_oidvector, oid.T_int2vector:
subscriptIdx++
}
if subscriptIdx < 1 || subscriptIdx > arr.Len() {
return DNull, nil
Expand Down
16 changes: 16 additions & 0 deletions pkg/sql/sem/tree/pgwire_encode.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ package tree
import (
"bytes"
"unicode/utf8"

"github.com/lib/pq/oid"
)

func (d *DTuple) pgwireFormat(ctx *FmtCtx) {
Expand Down Expand Up @@ -88,6 +90,20 @@ func (d *DArray) pgwireFormat(ctx *FmtCtx) {
// instead printed as-is. Only non-valid characters get escaped to
// hex. So we delegate this formatting to a tuple-specific
// string printer called pgwireFormatStringInArray().
switch d.ResolvedType().Oid() {
case oid.T_int2vector, oid.T_oidvector:
// vectors are serialized as a string of space-separated values.
sep := ""
// TODO(justin): add a test for nested arrays when #32552 is
// addressed.
for _, d := range d.Array {
ctx.WriteString(sep)
ctx.FormatNode(d)
sep = " "
}
return
}

ctx.WriteByte('{')
comma := ""
for _, v := range d.Array {
Expand Down

0 comments on commit 3f4d1a0

Please sign in to comment.