Skip to content

Commit

Permalink
Merge #97093
Browse files Browse the repository at this point in the history
97093: builtins: automatically add builtins for each type cast r=rafiss a=otan

In PG, casts from one type to another can also be use the function syntax, e.g. `date(now())` = `now()::date`. This is done at type resolution time.

Unfortunately we do not support that in type resolution, and from experience long ago it was tricky to do so (happy to be proven wrong).

This change instead defines a builtin for each castable type, which emulates the same behavior. We already kind of do this for `oid` and `inet`, so this isn't much worse right?

Release note (sql change): Each type cast is now expressable as a function, e.g. `now()::date` can be expressed as `date(now())`.

Resolves #97067

Co-authored-by: Oliver Tan <[email protected]>
  • Loading branch information
craig[bot] and otan committed Mar 2, 2023
2 parents 65752af + d10c3dd commit 20e2add
Show file tree
Hide file tree
Showing 20 changed files with 762 additions and 216 deletions.
6 changes: 0 additions & 6 deletions docs/generated/sql/functions.md
Original file line number Diff line number Diff line change
Expand Up @@ -980,8 +980,6 @@ available replica will error.</p>
</span></td><td>Immutable</td></tr>
<tr><td><a name="set_masklen"></a><code>set_masklen(val: <a href="inet.html">inet</a>, prefixlen: <a href="int.html">int</a>) &rarr; <a href="inet.html">inet</a></code></td><td><span class="funcdesc"><p>Sets the prefix length of <code>val</code> to <code>prefixlen</code>.</p>
<p>For example, <code>set_masklen('192.168.1.2', 16)</code> returns <code>'192.168.1.2/16'</code>.</p>
</span></td><td>Immutable</td></tr>
<tr><td><a name="text"></a><code>text(val: <a href="inet.html">inet</a>) &rarr; <a href="string.html">string</a></code></td><td><span class="funcdesc"><p>Converts the IP address and prefix length to text.</p>
</span></td><td>Immutable</td></tr></tbody>
</table>

Expand Down Expand Up @@ -2784,8 +2782,6 @@ The output can be used to recreate a database.’</p>
</span></td><td>Immutable</td></tr>
<tr><td><a name="get_byte"></a><code>get_byte(byte_string: <a href="bytes.html">bytes</a>, index: <a href="int.html">int</a>) &rarr; <a href="int.html">int</a></code></td><td><span class="funcdesc"><p>Extracts a byte at the given index in the byte array.</p>
</span></td><td>Immutable</td></tr>
<tr><td><a name="inet"></a><code>inet(val: <a href="string.html">string</a>) &rarr; <a href="inet.html">inet</a></code></td><td><span class="funcdesc"><p>If possible, converts input to that of type inet.</p>
</span></td><td>Immutable</td></tr>
<tr><td><a name="initcap"></a><code>initcap(val: <a href="string.html">string</a>) &rarr; <a href="string.html">string</a></code></td><td><span class="funcdesc"><p>Capitalizes the first letter of <code>val</code>.</p>
</span></td><td>Immutable</td></tr>
<tr><td><a name="left"></a><code>left(input: <a href="bytes.html">bytes</a>, return_set: <a href="int.html">int</a>) &rarr; <a href="bytes.html">bytes</a></code></td><td><span class="funcdesc"><p>Returns the first <code>return_set</code> bytes from <code>input</code>.</p>
Expand Down Expand Up @@ -3537,8 +3533,6 @@ table. Returns an error if validation fails.</p>
</span></td><td>Stable</td></tr>
<tr><td><a name="obj_description"></a><code>obj_description(object_oid: oid, catalog_name: <a href="string.html">string</a>) &rarr; <a href="string.html">string</a></code></td><td><span class="funcdesc"><p>Returns the comment for a database object specified by its OID and the name of the containing system catalog. For example, obj_description(123456, ‘pg_class’) would retrieve the comment for the table with OID 123456.</p>
</span></td><td>Stable</td></tr>
<tr><td><a name="oid"></a><code>oid(int: <a href="int.html">int</a>) &rarr; oid</code></td><td><span class="funcdesc"><p>Converts an integer to an OID.</p>
</span></td><td>Immutable</td></tr>
<tr><td><a name="pg_backend_pid"></a><code>pg_backend_pid() &rarr; <a href="int.html">int</a></code></td><td><span class="funcdesc"><p>Returns a numerical ID attached to this session. This ID is part of the query cancellation key used by the wire protocol. This function was only added for compatibility, and unlike in Postgres, the returned value does not correspond to a real process ID.</p>
</span></td><td>Stable</td></tr>
<tr><td><a name="pg_collation_for"></a><code>pg_collation_for(str: anyelement) &rarr; <a href="string.html">string</a></code></td><td><span class="funcdesc"><p>Returns the collation of the argument</p>
Expand Down
8 changes: 8 additions & 0 deletions pkg/internal/sqlsmith/schema.go
Original file line number Diff line number Diff line change
Expand Up @@ -495,6 +495,14 @@ var functions = func() map[tree.FunctionClass]map[oid.Oid][]function {
// See #69213.
continue
}

if n := tree.Name(def.Name); n.String() != def.Name {
// sqlsmith doesn't know how to quote function names, e.g. for
// the numeric cast, we need to use `"numeric"(val)`, but sqlsmith
// makes it `numeric(val)` which is incorrect.
continue
}

skip := false
for _, substr := range []string{
// crdb_internal.complete_stream_ingestion_job is a stateful
Expand Down
10 changes: 10 additions & 0 deletions pkg/sql/logictest/testdata/logic_test/inet
Original file line number Diff line number Diff line change
Expand Up @@ -840,11 +840,21 @@ SELECT text('10.1.0.0/16'::INET)
----
10.1.0.0/16

query T
SELECT '192.168.0.1'::INET::TEXT
----
192.168.0.1/32

query T
SELECT text('192.168.0.1/16'::INET)
----
192.168.0.1/16

query T
SELECT '192.168.0.1/16'::INET::TEXT
----
192.168.0.1/16

query T
SELECT text('192.168.0.1'::INET)
----
Expand Down
324 changes: 165 additions & 159 deletions pkg/sql/logictest/testdata/logic_test/pg_catalog

Large diffs are not rendered by default.

8 changes: 7 additions & 1 deletion pkg/sql/pg_catalog.go
Original file line number Diff line number Diff line change
Expand Up @@ -515,11 +515,17 @@ https://www.postgresql.org/docs/9.6/catalog-pg-cast.html`,
if ctxOrigin == cast.ContextOriginPgCast {
castCtx := cCtx.PGString()

castFunc := tree.DNull
if srcTyp, ok := types.OidToType[src]; ok {
if v, ok := builtins.CastBuiltinOIDs[tgt][srcTyp.Family()]; ok {
castFunc = tree.NewDOid(v)
}
}
_ = addRow(
h.CastOid(src, tgt), // oid
tree.NewDOid(src), // cast source
tree.NewDOid(tgt), // casttarget
tree.DNull, // castfunc
castFunc, // castfunc
tree.NewDString(castCtx), // castcontext
tree.DNull, // castmethod
)
Expand Down
3 changes: 3 additions & 0 deletions pkg/sql/sem/builtins/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ go_library(
"//pkg/sql/sem/builtins/builtinconstants",
"//pkg/sql/sem/builtins/builtinsregistry",
"//pkg/sql/sem/builtins/pgformat",
"//pkg/sql/sem/cast",
"//pkg/sql/sem/catconstants",
"//pkg/sql/sem/catid",
"//pkg/sql/sem/eval",
Expand Down Expand Up @@ -155,6 +156,7 @@ go_test(
"aggregate_builtins_test.go",
"all_builtins_test.go",
"builtins_test.go",
"cast_test.go",
"datums_to_bytes_builtin_test.go",
"fingerprint_builtin_test.go",
"generator_builtins_test.go",
Expand Down Expand Up @@ -187,6 +189,7 @@ go_test(
"//pkg/sql/randgen",
"//pkg/sql/sem/builtins/builtinconstants",
"//pkg/sql/sem/builtins/builtinsregistry",
"//pkg/sql/sem/cast",
"//pkg/sql/sem/eval",
"//pkg/sql/sem/tree",
"//pkg/sql/sem/tree/treewindow",
Expand Down
7 changes: 7 additions & 0 deletions pkg/sql/sem/builtins/all_builtins.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,13 @@ func init() {
for i, fn := range overloads {
signature := name + fn.Signature(true)
overloads[i].Oid = signatureMustHaveHardcodedOID(signature)
if _, ok := CastBuiltinNames[name]; ok {
retOid := fn.ReturnType(nil).Oid()
if _, ok := CastBuiltinOIDs[retOid]; !ok {
CastBuiltinOIDs[retOid] = make(map[types.Family]oid.Oid, len(overloads))
}
CastBuiltinOIDs[retOid][fn.Types.GetAt(0).Family()] = overloads[i].Oid
}
}
fDef := tree.NewFunctionDefinition(name, props, overloads)
addResolvedFuncDef(tree.ResolvedBuiltinFuncDefs, tree.OidToQualifiedBuiltinOverload, fDef)
Expand Down
1 change: 1 addition & 0 deletions pkg/sql/sem/builtins/builtinconstants/constants.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ const (
CategoryArray = "Array"
CategoryComparison = "Comparison"
CategoryCompatibility = "Compatibility"
CategoryCast = "Cast"
CategoryCrypto = "Cryptographic"
CategoryDateAndTime = "Date and time"
CategoryEnum = "Enum"
Expand Down
34 changes: 0 additions & 34 deletions pkg/sql/sem/builtins/builtins.go
Original file line number Diff line number Diff line change
Expand Up @@ -983,24 +983,6 @@ var regularBuiltins = map[string]builtinDefinition{
},
),

"text": makeBuiltin(defProps(),
tree.Overload{
Types: tree.ParamTypes{{Name: "val", Typ: types.INet}},
ReturnType: tree.FixedReturnType(types.String),
Fn: func(_ context.Context, _ *eval.Context, args tree.Datums) (tree.Datum, error) {
dIPAddr := tree.MustBeDIPAddr(args[0])
s := dIPAddr.IPAddr.String()
// Ensure the string has a "/mask" suffix.
if strings.IndexByte(s, '/') == -1 {
s += "/" + strconv.Itoa(int(dIPAddr.Mask))
}
return tree.NewDString(s), nil
},
Info: "Converts the IP address and prefix length to text.",
Volatility: volatility.Immutable,
},
),

"inet_same_family": makeBuiltin(defProps(),
tree.Overload{
Types: tree.ParamTypes{
Expand Down Expand Up @@ -1054,22 +1036,6 @@ var regularBuiltins = map[string]builtinDefinition{
},
),

"inet": makeBuiltin(defProps(),
tree.Overload{
Types: tree.ParamTypes{{Name: "val", Typ: types.String}},
ReturnType: tree.FixedReturnType(types.INet),
Fn: func(ctx context.Context, evalCtx *eval.Context, args tree.Datums) (tree.Datum, error) {
inet, err := eval.PerformCast(ctx, evalCtx, args[0], types.INet)
if err != nil {
return nil, pgerror.WithCandidateCode(err, pgcode.InvalidTextRepresentation)
}
return inet, nil
},
Info: "If possible, converts input to that of type inet.",
Volatility: volatility.Immutable,
},
),

"from_ip": makeBuiltin(defProps(),
tree.Overload{
Types: tree.ParamTypes{{Name: "val", Typ: types.Bytes}},
Expand Down
63 changes: 63 additions & 0 deletions pkg/sql/sem/builtins/cast_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
// Copyright 2023 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 builtins_test

import (
"context"
"fmt"
"testing"

"github.com/cockroachdb/cockroach/pkg/base"
"github.com/cockroachdb/cockroach/pkg/sql/sem/cast"
"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
"github.com/cockroachdb/cockroach/pkg/sql/sem/volatility"
"github.com/cockroachdb/cockroach/pkg/sql/types"
"github.com/cockroachdb/cockroach/pkg/testutils/serverutils"
"github.com/cockroachdb/cockroach/pkg/util/leaktest"
"github.com/cockroachdb/cockroach/pkg/util/log"
"github.com/lib/pq/oid"
"github.com/stretchr/testify/require"
)

// TestCastBuiltins sanity checks all casts for the cast map work.
// Note we don't have to check for families or anything crazy like we do
// for shouldMakeFromCastBuiltin.
func TestCastBuiltins(t *testing.T) {
defer leaktest.AfterTest(t)()
defer log.Scope(t).Close(t)

ctx := context.Background()
serv, sqlDB, _ := serverutils.StartServer(t, base.TestServerArgs{})
defer serv.Stopper().Stop(ctx)

cast.ForEachCast(func(
fromOID oid.Oid, toOID oid.Oid, castCtx cast.Context, ctxOrigin cast.ContextOrigin, v volatility.V,
) {
fromTyp, ok := types.OidToType[fromOID]
if !ok {
return
}
// Cannot cast as tuple.
if fromTyp.Family() == types.TupleFamily {
return
}
toTyp, ok := types.OidToType[toOID]
if !ok {
return
}
toName := tree.Name(cast.CastTypeName(toTyp))
q := fmt.Sprintf("SELECT %s(NULL::%s)", toName.String(), fromTyp.String())
t.Run(q, func(t *testing.T) {
_, err := sqlDB.Exec(q)
require.NoError(t, err)
})
})
}
Loading

0 comments on commit 20e2add

Please sign in to comment.