sql/parser: only retain scanned SQL comments when necessary

In cockroachdb#86968 the scanner gained the ability to retain comments in scanned
SQL strings, and this was an always-on feature. However, the comments
are only used when populating the crdb_internal.cluster_queries table,
see `sql.formatActiveQuery`. Now, comments are only retained when the
parser is used from this function, reducing allocations for all other
cases.

Fixes cockroachdb#127713

Release note: None

pkg/sql/crdb_internal.go

@@ -2832,7 +2832,7 @@ func populateQueriesTable(
 // formatActiveQuery formats a serverpb.ActiveQuery by interpolating its
 // placeholders within the string.
 func formatActiveQuery(query serverpb.ActiveQuery) string {
-	parsed, parseErr := parser.ParseOne(query.Sql)
+	parsed, parseErr := parser.ParseOneRetainComments(query.Sql)
 	if parseErr != nil {
 		// If we failed to interpolate, rather than give up just send out the
 		// SQL without interpolated placeholders. Hallelujah!

pkg/sql/parser/parse.go

@@ -68,19 +68,19 @@ func NakedIntTypeFromDefaultIntSize(defaultIntSize int32) *types.T {
 
 // Parse parses the sql and returns a list of statements.
 func (p *Parser) Parse(sql string) (statements.Statements, error) {
-	return p.parseWithDepth(1, sql, defaultNakedIntType)
+	return p.parseWithDepth(1, sql, defaultNakedIntType, discardComments)
 }
 
 // ParseWithInt parses a sql statement string and returns a list of
 // Statements. The INT token will result in the specified TInt type.
 func (p *Parser) ParseWithInt(sql string, nakedIntType *types.T) (statements.Statements, error) {
-	return p.parseWithDepth(1, sql, nakedIntType)
+	return p.parseWithDepth(1, sql, nakedIntType, discardComments)
 }
 
 func (p *Parser) parseOneWithInt(
-	sql string, nakedIntType *types.T,
+	sql string, nakedIntType *types.T, comments commentsMode,
 ) (statements.Statement[tree.Statement], error) {
-	stmts, err := p.parseWithDepth(1, sql, nakedIntType)
+	stmts, err := p.parseWithDepth(1, sql, nakedIntType, comments)
 	if err != nil {
 		return statements.Statement[tree.Statement]{}, err
 	}
@@ -144,11 +144,21 @@ func (p *Parser) scanOneStmt() (sql string, tokens []sqlSymType, done bool) {
 	}
 }
 
+type commentsMode bool
+
+const (
+	retainComments  commentsMode = true
+	discardComments commentsMode = false
+)
+
 func (p *Parser) parseWithDepth(
-	depth int, sql string, nakedIntType *types.T,
+	depth int, sql string, nakedIntType *types.T, cm commentsMode,
 ) (statements.Statements, error) {
 	stmts := statements.Statements(p.stmtBuf[:0])
 	p.scanner.Init(sql)
+	if cm == retainComments {
+		p.scanner.RetainComments()
+	}
 	defer p.scanner.Cleanup()
 	for {
 		sql, tokens, done := p.scanOneStmt()
@@ -232,26 +242,33 @@ func Parse(sql string) (statements.Statements, error) {
 // Statements. The INT token will result in the specified TInt type.
 func ParseWithInt(sql string, nakedIntType *types.T) (statements.Statements, error) {
 	var p Parser
-	return p.parseWithDepth(1, sql, nakedIntType)
+	return p.parseWithDepth(1, sql, nakedIntType, discardComments)
 }
 
 // ParseOne parses a sql statement string, ensuring that it contains only a
 // single statement, and returns that Statement. ParseOne will always
 // interpret the INT and SERIAL types as 64-bit types, since this is
 // used in various internal-execution paths where we might receive
-// bits of SQL from other nodes. In general,earwe expect that all
+// bits of SQL from other nodes. In general, we expect that all
 // user-generated SQL has been run through the ParseWithInt() function.
 func ParseOne(sql string) (statements.Statement[tree.Statement], error) {
 	return ParseOneWithInt(sql, defaultNakedIntType)
 }
 
+// ParseOneRetainComments is similar to ParseOne, but it retains scanned
+// comments in the returned statement's Comment field.
+func ParseOneRetainComments(sql string) (statements.Statement[tree.Statement], error) {
+	var p Parser
+	return p.parseOneWithInt(sql, defaultNakedIntType, retainComments)
+}
+
 // ParseOneWithInt is similar to ParseOn but interprets the INT and SERIAL
 // types as the provided integer type.
 func ParseOneWithInt(
 	sql string, nakedIntType *types.T,
 ) (statements.Statement[tree.Statement], error) {
 	var p Parser
-	return p.parseOneWithInt(sql, nakedIntType)
+	return p.parseOneWithInt(sql, nakedIntType, discardComments)
 }
 
 // ParseQualifiedTableName parses a possibly qualified table name. The

pkg/sql/scanner/scan.go

@@ -69,6 +69,9 @@ type Scanner struct {
 	// quoted. Used to distinguish between quoted and non-quoted in
 	// Inspect.
 	quoted bool
+	// retainComments indicates that comments should be collected in the
+	// Comments field. If it is false, they are discarded.
+	retainComments bool
 }
 
 // SQLScanner is a scanner with a SQL specific scan function
@@ -88,17 +91,26 @@ func (s *Scanner) Pos() int {
 
 // Init initializes a new Scanner that will process str.
 func (s *Scanner) Init(str string) {
-	s.in = str
-	s.pos = 0
-	// Preallocate some buffer space for identifiers etc.
-	s.bytesPrealloc = make([]byte, len(str))
+	*s = Scanner{
+		in:  str,
+		pos: 0,
+		// Preallocate some buffer space for identifiers etc.
+		bytesPrealloc: make([]byte, len(str)),
+	}
+}
+
+// RetainComments instructs the scanner to collect SQL comments in the Comments
+// field.
+func (s *Scanner) RetainComments() {
+	s.retainComments = true
 }
 
 // Cleanup is used to avoid holding on to memory unnecessarily (for the cases
 // where we reuse a Scanner).
 func (s *Scanner) Cleanup() {
 	s.bytesPrealloc = nil
 	s.Comments = nil
+	s.retainComments = false
 }
 
 func (s *Scanner) allocBytes(length int) []byte {
@@ -529,8 +541,10 @@ func (s *Scanner) skipWhitespace(lval ScanSymType, allowComments bool) (newline,
 			if present, cok := s.ScanComment(lval); !cok {
 				return false, false
 			} else if present {
-				// Mark down the comments that we found.
-				s.Comments = append(s.Comments, s.in[startPos:s.pos])
+				if s.retainComments {
+					// Mark down the comments that we found.
+					s.Comments = append(s.Comments, s.in[startPos:s.pos])
+				}
 				continue
 			}
 		}