Skip to content

Commit

Permalink
Merge #49578
Browse files Browse the repository at this point in the history
49578: sql: refactor casts and add volatility information r=RaduBerinde a=RaduBerinde

#### sql: move code related to casts into separate file

Release note: None

#### sql: refactor casts and add volatility information

This change refactors the lists of valid casts into a single list and associated
lookup-up map. The new list defines casts between *families* of types, which was
effectively what was happening before but in a more roundabout way.

The casts now have volatility information, which will be used by the optimizer;
the list was obtained by auditing the usage of `ctx` in `PerformCast`.

Informs #48717.

Release note: None

Co-authored-by: Radu Berinde <[email protected]>
  • Loading branch information
craig[bot] and RaduBerinde committed May 28, 2020
2 parents f4a5cac + 123ae87 commit cc48113
Show file tree
Hide file tree
Showing 8 changed files with 1,355 additions and 862 deletions.
1,021 changes: 1,021 additions & 0 deletions pkg/sql/sem/tree/casts.go

Large diffs are not rendered by default.

121 changes: 121 additions & 0 deletions pkg/sql/sem/tree/casts_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,121 @@
// Copyright 2020 The Cockroach Authors.
//
// Use of this software is governed by the Business Source License
// included in the file licenses/BSL.txt.
//
// As of the Change Date specified in that file, in accordance with
// the Business Source License, use of this software will be governed
// by the Apache License, Version 2.0, included in the file
// licenses/APL.txt.

package tree

import (
"encoding/csv"
"fmt"
"io"
"os"
"path/filepath"
"strconv"
"testing"

"github.com/cockroachdb/cockroach/pkg/sql/oidext"
"github.com/cockroachdb/cockroach/pkg/sql/types"
"github.com/cockroachdb/cockroach/pkg/util/leaktest"
"github.com/lib/pq/oid"
"github.com/stretchr/testify/require"
)

// TestCastsVolatilityMatchesPostgres checks that our defined casts match
// Postgres' casts for Volatility.
//
// Dump command below:
// COPY (
// SELECT c.castsource, c.casttarget, p.provolatile FROM pg_cast c JOIN pg_proc p ON (c.castfunc = p.oid)
// ) TO STDOUT WITH CSV DELIMITER '|' HEADER;
func TestCastsVolatilityMatchesPostgres(t *testing.T) {
defer leaktest.AfterTest(t)()
csvPath := filepath.Join("testdata", "pg_cast_provolatile_dump.csv")
f, err := os.Open(csvPath)
require.NoError(t, err)

defer f.Close()

reader := csv.NewReader(f)
reader.Comma = '|'

// Read header row
_, err = reader.Read()
require.NoError(t, err)

type pgCast struct {
from, to oid.Oid
provolatile byte
}
var pgCasts []pgCast

for {
line, err := reader.Read()
if err == io.EOF {
break
}
require.Len(t, line, 3)
require.NoError(t, err)

fromOid, err := strconv.Atoi(line[0])
require.NoError(t, err)

toOid, err := strconv.Atoi(line[1])
require.NoError(t, err)

provolatile := line[2]
require.Len(t, provolatile, 1)
pgCasts = append(pgCasts, pgCast{
from: oid.Oid(fromOid),
to: oid.Oid(toOid),
provolatile: provolatile[0],
})
}

oidToFamily := func(o oid.Oid) (_ types.Family, ok bool) {
t, ok := types.OidToType[o]
if !ok {
return 0, false
}
return t.Family(), true
}

oidStr := func(o oid.Oid) string {
res, ok := oidext.TypeName(o)
if !ok {
res = fmt.Sprintf("%d", o)
}
return res
}

for _, c := range validCasts {
if c.ignoreVolatilityCheck {
continue
}

// Look through all pg casts and find any where the Oids map to these
// families.
found := false
for i := range pgCasts {
fromFamily, fromOk := oidToFamily(pgCasts[i].from)
toFamily, toOk := oidToFamily(pgCasts[i].to)
if fromOk && toOk && fromFamily == c.from && toFamily == c.to {
found = true
if c.volatility != Volatility(pgCasts[i].provolatile) {
t.Errorf("cast %s::%s has volatility '%c'; corresponding pg cast %s::%s has volatility '%c'",
c.from.Name(), c.to.Name(), c.volatility,
oidStr(pgCasts[i].from), oidStr(pgCasts[i].to), pgCasts[i].provolatile,
)
}
}
}
if !found && testing.Verbose() {
t.Logf("cast %s::%s has no corresponding pg cast", c.from.Name(), c.to.Name())
}
}
}
Loading

0 comments on commit cc48113

Please sign in to comment.