sql: implement to_reg* builtins#78652
Merged
craig[bot] merged 1 commit intocockroachdb:masterfrom Apr 5, 2022
Merged
Conversation
Member
|
This change is |
5f8bed7 to
d3b71f2
Compare
otan
reviewed
Mar 31, 2022
pkg/sql/sem/builtins/pg_builtins.go
Outdated
Comment on lines
+2135
to
+2152
| Types: tree.ArgTypes{ | ||
| {"text", types.String}, | ||
| }, | ||
| ReturnType: tree.FixedReturnType(types.RegClass), | ||
| Fn: func(ctx *tree.EvalContext, args tree.Datums) (tree.Datum, error) { | ||
| typName := tree.MustBeDString(args[0]) | ||
|
|
||
| int, _ := strconv.Atoi(string(typName)) | ||
| if int > 0 { | ||
| return tree.DNull, nil | ||
| } | ||
| typOid, err := tree.ParseDOid(ctx, string(typName), types.RegClass) | ||
| if err != nil { | ||
| //nolint:returnerrcheck | ||
| return tree.DNull, nil | ||
| } | ||
|
|
||
| return typOid, nil |
Contributor
There was a problem hiding this comment.
the builtins here are all similar. can we unify this overload to be a function?
e.g.
func makeToRegOverload(typ *types.T, helpText string) builtinDefinition {
makeBuiltin(tree.FunctionProperties{Category: categorySystemInfo},
tree.Overload{
Types: tree.ArgTypes{
{"text", types.String},
},
ReturnType: tree.FixedReturnType(typ),
Fn: func(ctx *tree.EvalContext, args tree.Datums) (tree.Datum, error) {
n := tree.MustBeDString(args[0])
int, _ := strconv.Atoi(string(n))
if int > 0 {
return tree.DNull, nil
}
typOid, err := tree.ParseDOid(ctx, string(n), typ)
if err != nil {
//nolint:returnerrcheck
return tree.DNull, nil
}
return n, nil
},
Info: helpText,
Volatility: tree.VolatilityStable,
},
),
}
e-mbrown
commented
Mar 31, 2022
Contributor
Author
e-mbrown
left a comment
There was a problem hiding this comment.
Reviewable status:
complete! 0 of 0 LGTMs obtained (waiting on @otan)
pkg/sql/sem/builtins/pg_builtins.go, line 2152 at r1 (raw file):
Previously, otan (Oliver Tan) wrote…
the builtins here are all similar. can we unify this overload to be a function?
e.g.
func makeToRegOverload(typ *types.T, helpText string) builtinDefinition { makeBuiltin(tree.FunctionProperties{Category: categorySystemInfo}, tree.Overload{ Types: tree.ArgTypes{ {"text", types.String}, }, ReturnType: tree.FixedReturnType(typ), Fn: func(ctx *tree.EvalContext, args tree.Datums) (tree.Datum, error) { n := tree.MustBeDString(args[0]) int, _ := strconv.Atoi(string(n)) if int > 0 { return tree.DNull, nil } typOid, err := tree.ParseDOid(ctx, string(n), typ) if err != nil { //nolint:returnerrcheck return tree.DNull, nil } return n, nil }, Info: helpText, Volatility: tree.VolatilityStable, }, ), }
Done.
otan
reviewed
Mar 31, 2022
pkg/sql/sem/builtins/pg_builtins.go
Outdated
| } | ||
|
|
||
| // Make crdb_internal.create_regfoo and to_regfoo builtins. | ||
| for i, typ := range []*types.T{ |
Contributor
There was a problem hiding this comment.
let's do this:
for _, b := range []struct{
toRegOverloadHelpText string
typ *types.T
} {
{toRegOverloadHgelpText: "Translates a textual relation name to its OID." typ: types.RegClass},
...
} {
typName := b.typ.SQLStandardName()
builtins["crdb_internal.create_"+typName] = makeCreateRegDef(b.typ)
builtins["to_"+typName] = makeToRegOverload(b.typ)
}
so we can have everything grouped together
| SELECT to_regclass('t1') | ||
| ---- | ||
| t1 | ||
|
|
| ---- | ||
| NULL | ||
|
|
||
|
|
pkg/sql/sem/builtins/pg_builtins.go
Outdated
| Fn: func(ctx *tree.EvalContext, args tree.Datums) (tree.Datum, error) { | ||
| typName := tree.MustBeDString(args[0]) | ||
|
|
||
| int, _ := strconv.Atoi(string(typName)) |
Contributor
There was a problem hiding this comment.
maybe we need to strings.TrimSpace here
otan
reviewed
Mar 31, 2022
| NULL | ||
|
|
||
| # Should return null, but leading space leads to it returning an oid | ||
| # Not sure if leading spaces should be removed. |
Contributor
There was a problem hiding this comment.
(in pg this returns NULL so we need to remove the space)
e-mbrown
commented
Apr 4, 2022
Contributor
Author
e-mbrown
left a comment
There was a problem hiding this comment.
Reviewable status:
pkg/sql/sem/builtins/pg_builtins.go, line 160 at r2 (raw file):
Previously, otan (Oliver Tan) wrote…
let's do this:
for _, b := range []struct{ toRegOverloadHelpText string typ *types.T } { {toRegOverloadHgelpText: "Translates a textual relation name to its OID." typ: types.RegClass}, ... } { typName := b.typ.SQLStandardName() builtins["crdb_internal.create_"+typName] = makeCreateRegDef(b.typ) builtins["to_"+typName] = makeToRegOverload(b.typ) }so we can have everything grouped together
Done.
pkg/sql/sem/builtins/pg_builtins.go, line 544 at r2 (raw file):
Previously, otan (Oliver Tan) wrote…
maybe we need to
strings.TrimSpacehere
Done.
pkg/sql/logictest/testdata/logic_test/pg_builtins, line 661 at r2 (raw file):
Previously, otan (Oliver Tan) wrote…
(in pg this returns NULL so we need to remove the space)
Done.
otan
approved these changes
Apr 5, 2022
cb8fbf2 to
6c164bb
Compare
Collaborator
|
when running |
This commit implements the `to_regclass`, `to_regnamespace`, `to_regproc`, `to_regprocedure`, `to_regrole`, and `to_regtype` builtins. Release note (sql change): The `to_regclass`, `to_regnamespace`, `to_regproc`, `to_regprocedure`, `to_regrole`, and `to_regtype` builtin functions are now supported, improving compatibility with PostgreSQL.
Contributor
Author
|
bors r=otan |
Contributor
|
Build succeeded: |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Resolves #77838
This commit implements the
to_regclass,to_regnamespace,to_regproc,to_regprocedure,to_regrole, andto_regtypebuiltins.Release note (<category, see below>): The
to_regclass,to_regnamespace,to_regproc,to_regprocedure,to_regrole, andto_regtypebuiltin functions are now supported,improving compatibility with PostgreSQL.