Merge #153940 #154184

153940: parser: remove some unused code r=yuzefovich a=yuzefovich

This commit removes some of the unused code from sql/parser and plpgsql/parser.

Epic: None
Release note: None

154184: catalog/lease: fix nil pointer dereference in purgeOldVersions r=rafiss a=fqazi

Previously, we added logic that to handle errRenewLease due to session expiration with a debug assertion making sure the latest version of a descriptor was not historical. Sadly, the assertion did not take into account concurrent clean up of the version being retained. To address this, this patch adds an explicit nil check to handle this scenario.

Note: This scenario is only likely in tests  with the knob`removeOnceDereferenced`

Fixes: #154155

Release note: None

Co-authored-by: Yahor Yuzefovich <[email protected]>
Co-authored-by: Faizan Qazi <[email protected]>

Author: 3 people

pkg/sql/catalog/lease/lease.go

@@ -1277,6 +1277,11 @@ func (m *Manager) purgeOldVersions(
 		}
 		// We encountered an error telling us to renew the lease.
 		newest := m.findNewest(id)
+		// It is possible that a concurrent drop / removal of this descriptor is
+		// occurring. If the newest version just doesn't exist, bail out.
+		if newest == nil {
+			break
+		}
 		// Assert this should never happen due to a fixed expiration, since the range
 		// feed is responsible for purging old versions and acquiring new versions.
 		if newest.hasFixedExpiration() {

pkg/sql/parser/parse.go

@@ -86,18 +86,18 @@ var defaultNakedIntType = types.Int
 
 // Parse parses the sql and returns a list of statements.
 func (p *Parser) Parse(sql string) (statements.Statements, error) {
-	return p.parseWithDepth(1, sql, DefaultParseOptions)
+	return p.parseWithOptions(sql, DefaultParseOptions)
 }
 
 // ParseWithOptions parses the sql with the provided options and returns a list of statements.
 func (p *Parser) ParseWithOptions(sql string, opts ParseOptions) (statements.Statements, error) {
-	return p.parseWithDepth(1, sql, opts)
+	return p.parseWithOptions(sql, opts)
 }
 
 func (p *Parser) parseOne(
 	sql string, opts ParseOptions,
 ) (statements.Statement[tree.Statement], error) {
-	stmts, err := p.parseWithDepth(1, sql, opts)
+	stmts, err := p.parseWithOptions(sql, opts)
 	if err != nil {
 		return statements.Statement[tree.Statement]{}, err
 	}
@@ -161,9 +161,7 @@ func (p *Parser) scanOneStmt() (sql string, tokens []sqlSymType, done bool) {
 	}
 }
 
-func (p *Parser) parseWithDepth(
-	depth int, sql string, options ParseOptions,
-) (statements.Statements, error) {
+func (p *Parser) parseWithOptions(sql string, options ParseOptions) (statements.Statements, error) {
 	stmts := statements.Statements(p.stmtBuf[:0])
 	p.scanner.Init(sql)
 	if options.retainComments {
@@ -176,7 +174,7 @@ func (p *Parser) parseWithDepth(
 	}
 	for {
 		sql, tokens, done := p.scanOneStmt()
-		stmt, err := p.parse(depth+1, sql, tokens, options.intType, numAnnotations)
+		stmt, err := p.parse(sql, tokens, options.intType, numAnnotations)
 		if err != nil {
 			return nil, err
 		}
@@ -203,11 +201,7 @@ func (p *Parser) parseWithDepth(
 
 // parse parses a statement from the given scanned tokens.
 func (p *Parser) parse(
-	depth int,
-	sql string,
-	tokens []sqlSymType,
-	nakedIntType *types.T,
-	numAnnotations tree.AnnotationIdx,
+	sql string, tokens []sqlSymType, nakedIntType *types.T, numAnnotations tree.AnnotationIdx,
 ) (statements.Statement[tree.Statement], error) {
 	p.lexer.init(sql, tokens, nakedIntType, numAnnotations)
 	defer p.lexer.cleanup()

pkg/sql/plpgsql/parser/BUILD.bazel

@@ -44,7 +44,7 @@ go_library(
         "//pkg/sql/scanner",
         "//pkg/sql/sem/plpgsqltree",
         "//pkg/sql/sem/tree",
-        "//pkg/sql/types",
+        "//pkg/sql/types",  # keep
         "//pkg/util/errorutil/unimplemented",
         "@com_github_cockroachdb_errors//:errors",
         "@com_github_cockroachdb_redact//:redact",  # keep

pkg/sql/plpgsql/parser/lexer.go

@@ -14,7 +14,6 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgerror"
 	"github.com/cockroachdb/cockroach/pkg/sql/sem/plpgsqltree"
 	"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
-	"github.com/cockroachdb/cockroach/pkg/sql/types"
 	unimp "github.com/cockroachdb/cockroach/pkg/util/errorutil/unimplemented"
 	"github.com/cockroachdb/errors"
 )
@@ -24,33 +23,26 @@ type lexer struct {
 	// tokens contains tokens generated by the scanner.
 	tokens []plpgsqlSymType
 
-	// The type that should be used when an INT or SERIAL is encountered.
-	nakedIntType *types.T
-
 	// lastPos is the position into the tokens slice of the last
 	// token returned by Lex().
 	lastPos int
 
 	stmt *plpgsqltree.Block
 
-	// numPlaceholders is 1 + the highest placeholder index encountered.
-	numPlaceholders int
-	numAnnotations  tree.AnnotationIdx
+	numAnnotations tree.AnnotationIdx
 
 	lastError error
 
 	parser plpgsqlParser
 }
 
-func (l *lexer) init(sql string, tokens []plpgsqlSymType, nakedIntType *types.T, p plpgsqlParser) {
+func (l *lexer) init(sql string, tokens []plpgsqlSymType, p plpgsqlParser) {
 	l.in = sql
 	l.tokens = tokens
 	l.lastPos = -1
 	l.stmt = nil
-	l.numPlaceholders = 0
 	l.numAnnotations = 0
 	l.lastError = nil
-	l.nakedIntType = nakedIntType
 	l.parser = p
 }
 

pkg/sql/plpgsql/parser/parse.go

@@ -13,7 +13,6 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/sql/parser/statements"
 	"github.com/cockroachdb/cockroach/pkg/sql/scanner"
 	"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
-	"github.com/cockroachdb/cockroach/pkg/sql/types"
 	"github.com/cockroachdb/errors"
 )
 
@@ -38,17 +37,6 @@ type Parser struct {
 	tokBuf     [8]plpgsqlSymType
 }
 
-// INT8 is the historical interpretation of INT. This should be left
-// alone in the future, since there are many sql fragments stored
-// in various descriptors. Any user input that was created after
-// INT := INT4 will simply use INT4 in any resulting code.
-var defaultNakedIntType = types.Int
-
-// Parse parses the sql and returns a list of statements.
-func (p *Parser) Parse(sql string) (statements.PLpgStatement, error) {
-	return p.parseWithDepth(1, sql, defaultNakedIntType)
-}
-
 func (p *Parser) scanFnBlock() (sql string, tokens []plpgsqlSymType, done bool) {
 	var lval plpgsqlSymType
 	tokens = p.tokBuf[:0]
@@ -79,27 +67,9 @@ func (p *Parser) scanFnBlock() (sql string, tokens []plpgsqlSymType, done bool)
 	}
 }
 
-func (p *Parser) parseWithDepth(
-	depth int, plpgsql string, nakedIntType *types.T,
-) (statements.PLpgStatement, error) {
-	p.scanner.Init(plpgsql)
-	defer p.scanner.Cleanup()
-	sql, tokens, done := p.scanFnBlock()
-	stmt, err := p.parse(depth+1, sql, tokens, nakedIntType)
-	if err != nil {
-		return statements.PLpgStatement{}, err
-	}
-	if !done {
-		return statements.PLpgStatement{}, errors.AssertionFailedf("invalid plpgsql function: %s", plpgsql)
-	}
-	return stmt, nil
-}
-
 // parse parses a statement from the given scanned tokens.
-func (p *Parser) parse(
-	depth int, sql string, tokens []plpgsqlSymType, nakedIntType *types.T,
-) (statements.PLpgStatement, error) {
-	p.lexer.init(sql, tokens, nakedIntType, &p.parserImpl)
+func (p *Parser) parse(sql string, tokens []plpgsqlSymType) (statements.PLpgStatement, error) {
+	p.lexer.init(sql, tokens, &p.parserImpl)
 	defer p.lexer.cleanup()
 	if p.parserImpl.Parse(&p.lexer) != 0 {
 		if p.lexer.lastError == nil {
@@ -125,10 +95,9 @@ func (p *Parser) parse(
 		return statements.PLpgStatement{}, err
 	}
 	return statements.PLpgStatement{
-		AST:             p.lexer.stmt,
-		SQL:             sql,
-		NumPlaceholders: p.lexer.numPlaceholders,
-		NumAnnotations:  p.lexer.numAnnotations,
+		AST:            p.lexer.stmt,
+		SQL:            sql,
+		NumAnnotations: p.lexer.numAnnotations,
 	}, nil
 }
 
@@ -144,5 +113,15 @@ func (p *Parser) parse(
 // operate with table OIDTypeReferences.
 func Parse(sql string) (statements.PLpgStatement, error) {
 	var p Parser
-	return p.parseWithDepth(1, sql, defaultNakedIntType)
+	p.scanner.Init(sql)
+	defer p.scanner.Cleanup()
+	sql, tokens, done := p.scanFnBlock()
+	stmt, err := p.parse(sql, tokens)
+	if err != nil {
+		return statements.PLpgStatement{}, err
+	}
+	if !done {
+		return statements.PLpgStatement{}, errors.AssertionFailedf("invalid plpgsql function: %s", sql)
+	}
+	return stmt, nil
 }