Skip to content

Commit

Permalink
Merge pull request #132455 from cockroachdb/blathers/backport-release…
Browse files Browse the repository at this point in the history
…-24.2-132348

release-24.2: sql: fix panic caused by type resolution in AOST expression
  • Loading branch information
rafiss authored Oct 15, 2024
2 parents 9b0d176 + badf7cc commit 1cdc2eb
Show file tree
Hide file tree
Showing 4 changed files with 45 additions and 1 deletion.
12 changes: 12 additions & 0 deletions pkg/sql/logictest/testdata/logic_test/txn_as_of
Original file line number Diff line number Diff line change
Expand Up @@ -280,3 +280,15 @@ SET TRANSACTION AS OF SYSTEM TIME '0'

statement ok
ROLLBACK

statement error AS OF SYSTEM TIME: only constant expressions or follower_read_timestamp are allowed
BEGIN TRANSACTION AS OF SYSTEM TIME ('abc' IS OF (ident))

statement ok
BEGIN

statement error AS OF SYSTEM TIME: only constant expressions or follower_read_timestamp are allowed
SET TRANSACTION AS OF SYSTEM TIME ('abc' IS OF (ident))

statement ok
ROLLBACK
1 change: 0 additions & 1 deletion pkg/sql/schema_resolver.go
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,6 @@ func (sr *schemaResolver) byNameGetterBuilder() descs.ByNameGetterBuilder {
func (sr *schemaResolver) LookupObject(
ctx context.Context, flags tree.ObjectLookupFlags, dbName, scName, obName string,
) (found bool, prefix catalog.ResolvedObjectPrefix, desc catalog.Descriptor, err error) {

// Check if we are looking up a type which matches a built-in type in
// CockroachDB but is an extension type on the public schema in PostgreSQL.
if flags.DesiredObjectKind == tree.TypeObject && scName == catconstants.PublicSchemaName {
Expand Down
1 change: 1 addition & 0 deletions pkg/sql/sem/asof/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ go_library(
"//pkg/util/hlc",
"@com_github_cockroachdb_apd_v3//:apd",
"@com_github_cockroachdb_errors//:errors",
"@com_github_lib_pq//oid",
],
)

Expand Down
32 changes: 32 additions & 0 deletions pkg/sql/sem/asof/as_of.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"github.com/cockroachdb/cockroach/pkg/util/duration"
"github.com/cockroachdb/cockroach/pkg/util/hlc"
"github.com/cockroachdb/errors"
"github.com/lib/pq/oid"
)

// FollowerReadTimestampFunctionName is the name of the function which can be
Expand Down Expand Up @@ -132,6 +133,14 @@ func Eval(
defer scalarProps.Restore(*scalarProps)
scalarProps.Require("AS OF SYSTEM TIME", tree.RejectSpecial|tree.RejectSubqueries)

// Disable type resolution. Since type resolution requires a transaction, but
// this expression is being evaluated before a transaction begins, resolving
// any type should result in an error. There is no valid AS OF SYSTEM TIME
// expression that requires type resolution.
origTypeResolver := semaCtx.GetTypeResolver()
semaCtx.TypeResolver = &asOfTypeResolver{errFactory: newInvalidExprError}
defer func() { semaCtx.TypeResolver = origTypeResolver }()

var ret eval.AsOfSystemTime

// In order to support the follower reads feature we permit this expression
Expand Down Expand Up @@ -204,6 +213,29 @@ func Eval(
return ret, nil
}

// asOfTypeResolver is a type resolver that always returns an error. It is used
// to block type resolution while evaluating the AS OF SYSTEM TIME expression.
type asOfTypeResolver struct {
// errFactory is a function that returns the error to be returned by the
// type resolver. Using a closure lets us avoid instantiating the error
// unless something actually tries to resolve a type.
errFactory func() error
}

var _ tree.TypeReferenceResolver = (*asOfTypeResolver)(nil)

// ResolveType implements the tree.TypeReferenceResolver interface.
func (r *asOfTypeResolver) ResolveType(
ctx context.Context, name *tree.UnresolvedObjectName,
) (*types.T, error) {
return nil, r.errFactory()
}

// ResolveTypeByOID implements the tree.TypeReferenceResolver interface.
func (r *asOfTypeResolver) ResolveTypeByOID(ctx context.Context, oid oid.Oid) (*types.T, error) {
return nil, r.errFactory()
}

// DatumToHLCUsage specifies which statement DatumToHLC() is used for.
type DatumToHLCUsage int64

Expand Down

0 comments on commit 1cdc2eb

Please sign in to comment.