import: v23.1.13: ReturnType called on TypedExpr with empty typeAnnotation on a subquery #117724

cockroach-sentry · 2024-01-12T09:12:48Z

This issue was auto filed by Sentry. It represents a crash or reported error on a live cluster with telemetry enabled.

Sentry Link: https://cockroach-labs.sentry.io/issues/4853567853/?referrer=webhooks_plugin

Panic Message:

expr.go:152: ReturnType called on TypedExpr with empty typeAnnotation. Was the underlying Expr type-checked before asserting a type of TypedExpr?
(1) attached stack trace
  -- stack trace:
  | runtime.gopanic
  | 	GOROOT/src/runtime/panic.go:884
  | [...repeated from below...]
Wraps: (2) assertion failure
Wraps: (3) attached stack trace
  -- stack trace:
  | github.com/cockroachdb/cockroach/pkg/sql/sem/tree.typeAnnotation.assertTyped
  | 	github.com/cockroachdb/cockroach/pkg/sql/sem/tree/expr.go:152
  | github.com/cockroachdb/cockroach/pkg/sql/sem/tree.(*Subquery).TypeCheck
  | 	github.com/cockroachdb/cockroach/pkg/sql/sem/tree/type_check.go:1503
  | github.com/cockroachdb/cockroach/pkg/sql/sem/tree.(*overloadTypeChecker).typeCheckOverloadedExprs
  | 	github.com/cockroachdb/cockroach/pkg/sql/sem/tree/overload.go:842
  | github.com/cockroachdb/cockroach/pkg/sql/sem/tree.(*BinaryExpr).TypeCheck
  | 	github.com/cockroachdb/cockroach/pkg/sql/sem/tree/type_check.go:320
  | github.com/cockroachdb/cockroach/pkg/sql/sem/tree.(*overloadTypeChecker).typeCheckOverloadedExprs
  | 	github.com/cockroachdb/cockroach/pkg/sql/sem/tree/overload.go:842
  | github.com/cockroachdb/cockroach/pkg/sql/sem/tree.(*FuncExpr).TypeCheck
  | 	github.com/cockroachdb/cockroach/pkg/sql/sem/tree/type_check.go:1081
  | github.com/cockroachdb/cockroach/pkg/sql/importer.readPostgresStmt
  | 	github.com/cockroachdb/cockroach/pkg/sql/importer/read_import_pgdump.go:787
  | github.com/cockroachdb/cockroach/pkg/sql/importer.readPostgresCreateTable
  | 	github.com/cockroachdb/cockroach/pkg/sql/importer/read_import_pgdump.go:513
  | github.com/cockroachdb/cockroach/pkg/sql/importer.parseAndCreateBundleTableDescs
  | 	github.com/cockroachdb/cockroach/pkg/sql/importer/import_job.go:911
  | github.com/cockroachdb/cockroach/pkg/sql/importer.(*importResumer).parseBundleSchemaIfNeeded
  | 	github.com/cockroachdb/cockroach/pkg/sql/importer/import_job.go:781
  | github.com/cockroachdb/cockroach/pkg/sql/importer.(*importResumer).Resume
  | 	github.com/cockroachdb/cockroach/pkg/sql/importer/import_job.go:113
  | github.com/cockroachdb/cockroach/pkg/jobs.(*Registry).stepThroughStateMachine.func2
  | 	github.com/cockroachdb/cockroach/pkg/jobs/registry.go:1630
  | github.com/cockroachdb/cockroach/pkg/jobs.(*Registry).stepThroughStateMachine
  | 	github.com/cockroachdb/cockroach/pkg/jobs/registry.go:1631
  | github.com/cockroachdb/cockroach/pkg/jobs.(*Registry).runJob
  | 	github.com/cockroachdb/cockroach/pkg/jobs/adopt.go:474
  | github.com/cockroachdb/cockroach/pkg/jobs.(*StartableJob).Start.func2
  | 	github.com/cockroachdb/cockroach/pkg/jobs/jobs.go:978
  | github.com/cockroachdb/cockroach/pkg/util/stop.(*Stopper).RunAsyncTaskEx.func2
  | 	github.com/cockroachdb/cockroach/pkg/util/stop/stopper.go:470
  | runtime.goexit
  | 	GOROOT/src/runtime/asm_amd64.s:1594
Wraps: (4) ReturnType called on TypedExpr with empty typeAnnotation. Was the underlying Expr type-checked before asserting a type of TypedExpr?
Error types: (1) *withstack.withStack (2) *assert.withAssertionFailure (3) *withstack.withStack (4) *errutil.leafError
-- report composition:
*errutil.leafError: ReturnType called on TypedExpr with empty typeAnnotation. Was the underlying Expr type-checked before asserting a type of TypedExpr?
expr.go:152: *withstack.withStack (top exception)
*assert.withAssertionFailure
panic.go:884: *withstack.withStack (1)
(check the extra data payloads)

Stacktrace (expand for inline code snippets):

GOROOT/src/runtime/asm_amd64.s#L1593-L1595

cockroach/pkg/util/stop/stopper.go

Lines 469 to 471 in 8d065df

    
           	sp.UpdateGoroutineIDToCurrent() 
        
           	f(ctx) 
        
           }()

cockroach/pkg/jobs/jobs.go

Lines 977 to 979 in 8d065df

    
           defer cancel() 
        
           sj.execErr = sj.registry.runJob(resumeCtx, sj.resumer, sj.Job, StatusRunning, sj.taskName()) 
        
           close(sj.execDone)

cockroach/pkg/jobs/adopt.go

Lines 473 to 475 in 8d065df

    
           // Run the actual job. 
        
           err := r.stepThroughStateMachine(ctx, execCtx, resumer, job, status, finalResumeError) 
        
           // If the context has been canceled, disregard errors for the sake of logging

cockroach/pkg/jobs/registry.go

Lines 1630 to 1632 in 8d065df

    
           	err = resumer.Resume(resumeCtx, execCtx) 
        
           }()

cockroach/pkg/jobs/registry.go

Lines 1629 to 1631 in 8d065df

    
           	}() 
        
           	err = resumer.Resume(resumeCtx, execCtx) 
        
           }()

cockroach/pkg/sql/importer/import_job.go

Lines 112 to 114 in 8d065df

    
           p := execCtx.(sql.JobExecContext) 
        
           if err := r.parseBundleSchemaIfNeeded(ctx, p); err != nil { 
        
           	return err

cockroach/pkg/sql/importer/import_job.go

Lines 780 to 782 in 8d065df

    
           if tableDescs, schemaDescs, err = parseAndCreateBundleTableDescs( 
        
           	ctx, p, details, seqVals, skipFKs, dbDesc, files, format, walltime, owner,

cockroach/pkg/sql/importer/import_job.go

Lines 910 to 912 in 8d065df

    
           tableDescs, schemaDescs, err = readPostgresCreateTable(ctx, reader, evalCtx, p, tableName, 
        
           	parentDB, walltime, fks, int(format.PgDump.MaxRowSize), owner, unsupportedStmtLogger)

cockroach/pkg/sql/importer/read_import_pgdump.go

Lines 512 to 514 in 8d065df

    
           } 
        
           if err := readPostgresStmt(ctx, evalCtx, match, fks, &schemaObjects, stmt, p, 
        
           	parentDB.GetID(), unsupportedStmtLogger); err != nil {

cockroach/pkg/sql/importer/read_import_pgdump.go

Lines 786 to 788 in 8d065df

    
           semaCtx := tree.MakeSemaContext() 
        
           if _, err := expr.TypeCheck(ctx, &semaCtx, nil /* desired */); err != nil { 
        
           	// If the expression does not type check, it may be a case of using

cockroach/pkg/sql/sem/tree/type_check.go

Lines 1080 to 1082 in 8d065df

    
           defer s.release() 
        
           if err := s.typeCheckOverloadedExprs(ctx, semaCtx, desired, false); err != nil { 
        
           	return nil, pgerror.Wrapf(err, pgcode.InvalidParameterValue, "%s()", def.Name)

cockroach/pkg/sql/sem/tree/overload.go

Lines 841 to 843 in 8d065df

    
           } 
        
           typ, err := s.exprs[i].TypeCheck(ctx, semaCtx, paramDesired) 
        
           if err != nil {

cockroach/pkg/sql/sem/tree/type_check.go

Lines 319 to 321 in 8d065df

    
           defer s.release() 
        
           if err := s.typeCheckOverloadedExprs(ctx, semaCtx, desired, inBinOp); err != nil { 
        
           	return nil, err

cockroach/pkg/sql/sem/tree/overload.go

Lines 841 to 843 in 8d065df

    
           } 
        
           typ, err := s.exprs[i].TypeCheck(ctx, semaCtx, paramDesired) 
        
           if err != nil {

cockroach/pkg/sql/sem/tree/type_check.go

Lines 1502 to 1504 in 8d065df

    
           } 
        
           expr.assertTyped() 
        
           return expr, nil

cockroach/pkg/sql/sem/tree/expr.go

Lines 151 to 153 in 8d065df

    
           if ta.typ == nil { 
        
           	panic(errors.AssertionFailedf( 
        
           		"ReturnType called on TypedExpr with empty typeAnnotation. " +

GOROOT/src/runtime/panic.go#L883-L885
GOROOT/src/runtime/asm_amd64.s#L1593-L1595

cockroach/pkg/util/stop/stopper.go

Lines 469 to 471 in 8d065df

    
           	sp.UpdateGoroutineIDToCurrent() 
        
           	f(ctx) 
        
           }()

cockroach/pkg/jobs/jobs.go

Lines 977 to 979 in 8d065df

    
           defer cancel() 
        
           sj.execErr = sj.registry.runJob(resumeCtx, sj.resumer, sj.Job, StatusRunning, sj.taskName()) 
        
           close(sj.execDone)

cockroach/pkg/jobs/adopt.go

Lines 473 to 475 in 8d065df

    
           // Run the actual job. 
        
           err := r.stepThroughStateMachine(ctx, execCtx, resumer, job, status, finalResumeError) 
        
           // If the context has been canceled, disregard errors for the sake of logging

cockroach/pkg/jobs/registry.go

Lines 1630 to 1632 in 8d065df

    
           	err = resumer.Resume(resumeCtx, execCtx) 
        
           }()

cockroach/pkg/jobs/registry.go

Lines 1629 to 1631 in 8d065df

    
           	}() 
        
           	err = resumer.Resume(resumeCtx, execCtx) 
        
           }()

cockroach/pkg/sql/importer/import_job.go

Lines 112 to 114 in 8d065df

    
           p := execCtx.(sql.JobExecContext) 
        
           if err := r.parseBundleSchemaIfNeeded(ctx, p); err != nil { 
        
           	return err

cockroach/pkg/sql/importer/import_job.go

Lines 780 to 782 in 8d065df

    
           if tableDescs, schemaDescs, err = parseAndCreateBundleTableDescs( 
        
           	ctx, p, details, seqVals, skipFKs, dbDesc, files, format, walltime, owner,

cockroach/pkg/sql/importer/import_job.go

Lines 910 to 912 in 8d065df

    
           tableDescs, schemaDescs, err = readPostgresCreateTable(ctx, reader, evalCtx, p, tableName, 
        
           	parentDB, walltime, fks, int(format.PgDump.MaxRowSize), owner, unsupportedStmtLogger)

cockroach/pkg/sql/importer/read_import_pgdump.go

Lines 512 to 514 in 8d065df

    
           } 
        
           if err := readPostgresStmt(ctx, evalCtx, match, fks, &schemaObjects, stmt, p, 
        
           	parentDB.GetID(), unsupportedStmtLogger); err != nil {

cockroach/pkg/sql/importer/read_import_pgdump.go

Lines 786 to 788 in 8d065df

    
           semaCtx := tree.MakeSemaContext() 
        
           if _, err := expr.TypeCheck(ctx, &semaCtx, nil /* desired */); err != nil { 
        
           	// If the expression does not type check, it may be a case of using

cockroach/pkg/sql/sem/tree/type_check.go

Lines 1080 to 1082 in 8d065df

    
           defer s.release() 
        
           if err := s.typeCheckOverloadedExprs(ctx, semaCtx, desired, false); err != nil { 
        
           	return nil, pgerror.Wrapf(err, pgcode.InvalidParameterValue, "%s()", def.Name)

cockroach/pkg/sql/sem/tree/overload.go

Lines 841 to 843 in 8d065df

    
           } 
        
           typ, err := s.exprs[i].TypeCheck(ctx, semaCtx, paramDesired) 
        
           if err != nil {

cockroach/pkg/sql/sem/tree/type_check.go

Lines 319 to 321 in 8d065df

    
           defer s.release() 
        
           if err := s.typeCheckOverloadedExprs(ctx, semaCtx, desired, inBinOp); err != nil { 
        
           	return nil, err

cockroach/pkg/sql/sem/tree/overload.go

Lines 841 to 843 in 8d065df

    
           } 
        
           typ, err := s.exprs[i].TypeCheck(ctx, semaCtx, paramDesired) 
        
           if err != nil {

cockroach/pkg/sql/sem/tree/type_check.go

Lines 1502 to 1504 in 8d065df

    
           } 
        
           expr.assertTyped() 
        
           return expr, nil

cockroach/pkg/sql/sem/tree/expr.go

Lines 151 to 153 in 8d065df

    
           if ta.typ == nil { 
        
           	panic(errors.AssertionFailedf( 
        
           		"ReturnType called on TypedExpr with empty typeAnnotation. " +

GOROOT/src/runtime/asm_amd64.s in runtime.goexit at line 1594
pkg/util/stop/stopper.go in pkg/util/stop.(*Stopper).RunAsyncTaskEx.func2 at line 470
pkg/jobs/jobs.go in pkg/jobs.(*StartableJob).Start.func2 at line 978
pkg/jobs/adopt.go in pkg/jobs.(*Registry).runJob at line 474
pkg/jobs/registry.go in pkg/jobs.(*Registry).stepThroughStateMachine at line 1631
pkg/jobs/registry.go in pkg/jobs.(*Registry).stepThroughStateMachine.func2 at line 1630
pkg/sql/importer/import_job.go in pkg/sql/importer.(*importResumer).Resume at line 113
pkg/sql/importer/import_job.go in pkg/sql/importer.(*importResumer).parseBundleSchemaIfNeeded at line 781
pkg/sql/importer/import_job.go in pkg/sql/importer.parseAndCreateBundleTableDescs at line 911
pkg/sql/importer/read_import_pgdump.go in pkg/sql/importer.readPostgresCreateTable at line 513
pkg/sql/importer/read_import_pgdump.go in pkg/sql/importer.readPostgresStmt at line 787
pkg/sql/sem/tree/type_check.go in pkg/sql/sem/tree.(*FuncExpr).TypeCheck at line 1081
pkg/sql/sem/tree/overload.go in pkg/sql/sem/tree.(*overloadTypeChecker).typeCheckOverloadedExprs at line 842
pkg/sql/sem/tree/type_check.go in pkg/sql/sem/tree.(*BinaryExpr).TypeCheck at line 320
pkg/sql/sem/tree/overload.go in pkg/sql/sem/tree.(*overloadTypeChecker).typeCheckOverloadedExprs at line 842
pkg/sql/sem/tree/type_check.go in pkg/sql/sem/tree.(*Subquery).TypeCheck at line 1503
pkg/sql/sem/tree/expr.go in pkg/sql/sem/tree.typeAnnotation.assertTyped at line 152
GOROOT/src/runtime/panic.go in runtime.gopanic at line 884
GOROOT/src/runtime/asm_amd64.s in runtime.goexit at line 1594
pkg/util/stop/stopper.go in pkg/util/stop.(*Stopper).RunAsyncTaskEx.func2 at line 470
pkg/jobs/jobs.go in pkg/jobs.(*StartableJob).Start.func2 at line 978
pkg/jobs/adopt.go in pkg/jobs.(*Registry).runJob at line 474
pkg/jobs/registry.go in pkg/jobs.(*Registry).stepThroughStateMachine at line 1631
pkg/jobs/registry.go in pkg/jobs.(*Registry).stepThroughStateMachine.func2 at line 1630
pkg/sql/importer/import_job.go in pkg/sql/importer.(*importResumer).Resume at line 113
pkg/sql/importer/import_job.go in pkg/sql/importer.(*importResumer).parseBundleSchemaIfNeeded at line 781
pkg/sql/importer/import_job.go in pkg/sql/importer.parseAndCreateBundleTableDescs at line 911
pkg/sql/importer/read_import_pgdump.go in pkg/sql/importer.readPostgresCreateTable at line 513
pkg/sql/importer/read_import_pgdump.go in pkg/sql/importer.readPostgresStmt at line 787
pkg/sql/sem/tree/type_check.go in pkg/sql/sem/tree.(*FuncExpr).TypeCheck at line 1081
pkg/sql/sem/tree/overload.go in pkg/sql/sem/tree.(*overloadTypeChecker).typeCheckOverloadedExprs at line 842
pkg/sql/sem/tree/type_check.go in pkg/sql/sem/tree.(*BinaryExpr).TypeCheck at line 320
pkg/sql/sem/tree/overload.go in pkg/sql/sem/tree.(*overloadTypeChecker).typeCheckOverloadedExprs at line 842
pkg/sql/sem/tree/type_check.go in pkg/sql/sem/tree.(*Subquery).TypeCheck at line 1503
pkg/sql/sem/tree/expr.go in pkg/sql/sem/tree.typeAnnotation.assertTyped at line 152

Tags

Tag	Value
Command	start-single-node
Environment	v23.1.13
Go Version	go1.19.13
Platform	linux amd64
Distribution	CCL
Cockroach Release	v23.1.13
Cockroach SHA	`8d065df`
# of CPUs	4
# of Goroutines	334

Jira issue: CRDB-35321

The text was updated successfully, but these errors were encountered:

DrewKimball · 2024-02-01T07:40:27Z

This is another location where we call TypeCheck outside of the optbuilder:

cockroach/pkg/sql/importer/read_import_pgdump.go

Lines 784 to 796 in cc4fdff

    
           case *tree.FuncExpr: 
        
           	// Look for function calls that mutate schema (this is actually a thing). 
        
           	semaCtx := tree.MakeSemaContext() 
        
           	if _, err := expr.TypeCheck(ctx, &semaCtx, nil /* desired */); err != nil { 
        
           		// If the expression does not type check, it may be a case of using 
        
           		// a column that does not exist yet in a setval call (as is the case 
        
           		// of PGDUMP output from ogr2ogr). We're not interested in setval 
        
           		// calls during schema reading so it is safe to ignore this for now. 
        
           		if f := expr.Func.String(); pgerror.GetPGCode(err) == pgcode.UndefinedColumn && f == "setval" { 
        
           			continue 
        
           		} 
        
           		return err 
        
           	}

118569: importer: disallow subqueries in function arguments for pg_dump r=DrewKimball a=DrewKimball Some functions can modify the db schema, and so can be included in a pg_dump file. The pg_dump importer logic type-checks such functions, which can lead to a nil-pointer panic in cases like the following: `SELECT addgeometrycolumn('t', 'foo', 4326, (SELECT 'POINT'), 2)` This is because subqueries cannot be type-checked outside the optbuilder, which sets the type annotation for the subquery. This patch explicitly disallows subqueries in the type-checking that happens for processing a pg_dump file. This will ensure that users get an expected `subqueries are not allowed in pg_dump function arguments` error, instead of the panic. Fixes #117724 Release note (bug fix): Fixed a rare panic that could happen during a pg_dump import that contains a function like `SELECT addgeometrycolumn(...)`. Now, attempting to import a pg_dump with a function that has a subquery in one of its arguments results in an expected error. Co-authored-by: Drew Kimball <drewk@cockroachlabs.com>

Some functions can modify the db schema, and so can be included in a pg_dump file. The pg_dump importer logic type-checks such functions, which can lead to a nil-pointer panic in cases like the following: `SELECT addgeometrycolumn('t', 'foo', 4326, (SELECT 'POINT'), 2)` This is because subqueries cannot be type-checked outside the optbuilder, which sets the type annotation for the subquery. This patch explicitly disallows subqueries in the type-checking that happens for processing a pg_dump file. This will ensure that users get an expected `subqueries are not allowed in pg_dump function arguments` error, instead of the panic. Fixes #117724 Release note (bug fix): Fixed a rare panic that could happen during a pg_dump import that contains a function like `SELECT addgeometrycolumn(...)`. Now, attempting to import a pg_dump with a function that has a subquery in one of its arguments results in an expected error.

Some functions can modify the db schema, and so can be included in a pg_dump file. The pg_dump importer logic type-checks such functions, which can lead to a nil-pointer panic in cases like the following: `SELECT addgeometrycolumn('t', 'foo', 4326, (SELECT 'POINT'), 2)` This is because subqueries cannot be type-checked outside the optbuilder, which sets the type annotation for the subquery. This patch explicitly disallows subqueries in the type-checking that happens for processing a pg_dump file. This will ensure that users get an expected `subqueries are not allowed in pg_dump function arguments` error, instead of the panic. Fixes cockroachdb#117724 Release note (bug fix): Fixed a rare panic that could happen during a pg_dump import that contains a function like `SELECT addgeometrycolumn(...)`. Now, attempting to import a pg_dump with a function that has a subquery in one of its arguments results in an expected error.

cockroach-sentry added O-sentry Originated from an in-the-wild panic report. C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. labels Jan 12, 2024

yuzefovich mentioned this issue Jan 17, 2024

Sentry: expr.go:152: ReturnType called on TypedExpr with empty typeAnnotation. Was the underlying Expr type-checked before asserting a type of TypedExpr? (1) attached stack trace -- stack trace: |... #117725

Closed

yuzefovich added the T-sql-queries SQL Queries Team label Jan 17, 2024

DrewKimball mentioned this issue Feb 1, 2024

importer: disallow subqueries in function arguments for pg_dump #118569

Merged

DrewKimball added the P-2 Issues/test failures with a fix SLA of 3 months label Feb 1, 2024

craig bot closed this as completed in 850bd2e Feb 2, 2024

blathers-crl bot mentioned this issue Feb 2, 2024

release-23.1: importer: disallow subqueries in function arguments for pg_dump #118611

Merged

blathers-crl bot mentioned this issue Feb 2, 2024

release-23.2: importer: disallow subqueries in function arguments for pg_dump #118612

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

import: v23.1.13: ReturnType called on TypedExpr with empty typeAnnotation on a subquery #117724

import: v23.1.13: ReturnType called on TypedExpr with empty typeAnnotation on a subquery #117724

cockroach-sentry commented Jan 12, 2024 •

edited by cockroach-jira-scripts

Loading

DrewKimball commented Feb 1, 2024

import: v23.1.13: ReturnType called on TypedExpr with empty typeAnnotation on a subquery #117724

import: v23.1.13: ReturnType called on TypedExpr with empty typeAnnotation on a subquery #117724

Comments

cockroach-sentry commented Jan 12, 2024 • edited by cockroach-jira-scripts Loading

Tags

DrewKimball commented Feb 1, 2024

cockroach-sentry commented Jan 12, 2024 •

edited by cockroach-jira-scripts

Loading