From df9d8dc3d66617df75091e96b24187b1bfc405db Mon Sep 17 00:00:00 2001 From: sivaalampally Date: Tue, 10 Mar 2026 02:35:47 -0400 Subject: [PATCH 1/3] Fix for panic in rows.Next() when stored procedures emit NOTICE messages --- driver_test.go | 60 ++++++++++++++++++++++++++++++++++++++++++++++++++ rows.go | 17 +++++++++++++- 2 files changed, 76 insertions(+), 1 deletion(-) diff --git a/driver_test.go b/driver_test.go index eb89b51..65bac1f 100644 --- a/driver_test.go +++ b/driver_test.go @@ -1660,6 +1660,66 @@ func TestInvalidEmailParseStatement(t *testing.T) { assertEqual(t, all_roles, role) } } +// TestStoredProcedureWithNotice verifies that rows.Next() does not panic when a +// stored procedure raises one or more NOTICE messages before returning its +// result set. Prior to the fix, the driver built a destination slice sized +// from the first RowDescription it saw (which could be 1 column) while the +// DataRow for the actual CALL result contained all INOUT columns (2 here), +// causing an index-out-of-range panic inside rows.Next(). +func TestStoredProcedureWithNotice(t *testing.T) { + connDB := openConnection(t) + defer closeConnection(t, connDB) + + // Create the procedure; drop it afterward. + _, err := connDB.ExecContext(ctx, ` + CREATE OR REPLACE PROCEDURE test_notice_proc(INOUT a INT, INOUT b VARCHAR(64)) + LANGUAGE PLvSQL AS $$ + BEGIN + RAISE NOTICE 'Value of a: %', a; + RAISE NOTICE 'Value of b: %', b; + END; + $$`) + assertNoErr(t, err) + + defer connDB.ExecContext(ctx, "DROP PROCEDURE IF EXISTS test_notice_proc(INT, VARCHAR)") + + // CALL must not panic. We wrap in a recover so that a panic becomes a + // test failure with a meaningful message rather than crashing the whole + // test binary. + func() { + defer func() { + if r := recover(); r != nil { + t.Fatalf("rows.Next() panicked when stored procedure emitted NOTICE messages: %v", r) + } + }() + + rows, err := connDB.QueryContext(ctx, "CALL test_notice_proc(10, 'hello')") + assertNoErr(t, err) + defer rows.Close() + + // Iterate over all result sets (NOTICE messages may produce extra sets). + for { + cols, err := rows.Columns() + assertNoErr(t, err) + + valuePtrs := make([]interface{}, len(cols)) + values := make([]interface{}, len(cols)) + for i := range values { + valuePtrs[i] = &values[i] + } + + for rows.Next() { + assertNoErr(t, rows.Scan(valuePtrs...)) + testLogger.Debug("TestStoredProcedureWithNotice row: %v", values) + } + + if !rows.NextResultSet() { + break + } + } + }() +} + func init() { // One or both lines below are necessary depending on your go version testing.Init() diff --git a/rows.go b/rows.go index a5eccb8..75d2573 100644 --- a/rows.go +++ b/rows.go @@ -98,7 +98,22 @@ func (r *rows) Next(dest []driver.Value) error { rowCols := nextRow.Columns() - for idx := uint16(0); idx < rowCols.NumCols; idx++ { + // Guard against a mismatch between the number of columns described in the + // RowDescription message (len(dest) / len(r.columnDefs.Columns)) and the + // number of fields actually present in the DataRow message (rowCols.NumCols). + // This can happen when a stored procedure emits NOTICE messages before + // returning its result set, causing the driver to build a destination slice + // that is shorter than the arriving row. Iterating only up to the minimum + // of the two lengths prevents an index-out-of-range panic. + numColsToCopy := rowCols.NumCols + if destLen := uint16(len(dest)); destLen < numColsToCopy { + numColsToCopy = destLen + } + if colDefsLen := uint16(len(r.columnDefs.Columns)); colDefsLen < numColsToCopy { + numColsToCopy = colDefsLen + } + + for idx := uint16(0); idx < numColsToCopy; idx++ { colVal := rowCols.Chunk() if colVal == nil { dest[idx] = nil From 9efe90fa70df329c217f54ef9f53d6ca495d6821 Mon Sep 17 00:00:00 2001 From: sivaalampally Date: Tue, 10 Mar 2026 05:59:58 -0400 Subject: [PATCH 2/3] Fix for panic --- driver_test.go | 72 ++++++++++++++++++++++++++++++++++++++++++++++---- rows.go | 32 +++++++++++----------- stmt.go | 18 +++++++++++-- 3 files changed, 99 insertions(+), 23 deletions(-) diff --git a/driver_test.go b/driver_test.go index 65bac1f..4c4e723 100644 --- a/driver_test.go +++ b/driver_test.go @@ -1660,12 +1660,20 @@ func TestInvalidEmailParseStatement(t *testing.T) { assertEqual(t, all_roles, role) } } + // TestStoredProcedureWithNotice verifies that rows.Next() does not panic when a -// stored procedure raises one or more NOTICE messages before returning its -// result set. Prior to the fix, the driver built a destination slice sized -// from the first RowDescription it saw (which could be 1 column) while the -// DataRow for the actual CALL result contained all INOUT columns (2 here), -// causing an index-out-of-range panic inside rows.Next(). +// stored procedure raises one or more RAISE NOTICE messages before returning its +// result set. +// +// Root cause: Vertica sends a correct RowDescription (2 cols: a, b) for the CALL +// result, then emits NoticeResponse messages, then sends a spurious second +// RowDescription with fewer columns (a PLvSQL protocol artefact). The driver was +// blindly adopting the second RowDescription, so r.columnDefs ended up with 1 +// column while the DataRow still contained 2 fields — causing an index-out-of-range +// panic in rows.Next(). +// +// Fix: runSimpleStatement and collectResults now ignore any RowDescription that +// arrives after DataRows have already been buffered for the current result set. func TestStoredProcedureWithNotice(t *testing.T) { connDB := openConnection(t) defer closeConnection(t, connDB) @@ -1720,6 +1728,60 @@ func TestStoredProcedureWithNotice(t *testing.T) { }() } +// TestStoredProcedureWithNoticeSimpleQuery is the same scenario as +// TestStoredProcedureWithNotice but forces the simple query protocol path +// (use_prepared_statements=0) to verify correct behaviour on both code paths. +func TestStoredProcedureWithNoticeSimpleQuery(t *testing.T) { + simpleConnStr := strings.Replace(myDBConnectString, "use_prepared_statements=1", "use_prepared_statements=0", 1) + connDB, err := sql.Open("vertica", simpleConnStr) + assertNoErr(t, err) + assertNoErr(t, connDB.PingContext(ctx)) + defer closeConnection(t, connDB) + + _, err = connDB.ExecContext(ctx, ` + CREATE OR REPLACE PROCEDURE test_notice_proc_simple(INOUT a INT, INOUT b VARCHAR(64)) + LANGUAGE PLvSQL AS $$ + BEGIN + RAISE NOTICE 'Value of a: %', a; + RAISE NOTICE 'Value of b: %', b; + END; + $$`) + assertNoErr(t, err) + defer connDB.ExecContext(ctx, "DROP PROCEDURE IF EXISTS test_notice_proc_simple(INT, VARCHAR)") + + func() { + defer func() { + if r := recover(); r != nil { + t.Fatalf("rows.Next() panicked on simple query path: %v", r) + } + }() + + rows, err := connDB.QueryContext(ctx, "CALL test_notice_proc_simple(10, 'hello')") + assertNoErr(t, err) + defer rows.Close() + + for { + cols, err := rows.Columns() + assertNoErr(t, err) + + valuePtrs := make([]interface{}, len(cols)) + values := make([]interface{}, len(cols)) + for i := range values { + valuePtrs[i] = &values[i] + } + + for rows.Next() { + assertNoErr(t, rows.Scan(valuePtrs...)) + testLogger.Debug("TestStoredProcedureWithNoticeSimpleQuery row: %v", values) + } + + if !rows.NextResultSet() { + break + } + } + }() +} + func init() { // One or both lines below are necessary depending on your go version testing.Init() diff --git a/rows.go b/rows.go index 75d2573..7dcad1c 100644 --- a/rows.go +++ b/rows.go @@ -98,22 +98,7 @@ func (r *rows) Next(dest []driver.Value) error { rowCols := nextRow.Columns() - // Guard against a mismatch between the number of columns described in the - // RowDescription message (len(dest) / len(r.columnDefs.Columns)) and the - // number of fields actually present in the DataRow message (rowCols.NumCols). - // This can happen when a stored procedure emits NOTICE messages before - // returning its result set, causing the driver to build a destination slice - // that is shorter than the arriving row. Iterating only up to the minimum - // of the two lengths prevents an index-out-of-range panic. - numColsToCopy := rowCols.NumCols - if destLen := uint16(len(dest)); destLen < numColsToCopy { - numColsToCopy = destLen - } - if colDefsLen := uint16(len(r.columnDefs.Columns)); colDefsLen < numColsToCopy { - numColsToCopy = colDefsLen - } - - for idx := uint16(0); idx < numColsToCopy; idx++ { + for idx := uint16(0); idx < rowCols.NumCols; idx++ { colVal := rowCols.Chunk() if colVal == nil { dest[idx] = nil @@ -367,3 +352,18 @@ func newEmptyRows() *rows { be := &msgs.BERowDescMsg{Columns: cdf} return newRows(context.Background(), be, "") } + +// expandColumnDefs grows r.columnDefs to cover at least numCols columns. +// It is a last-resort fallback for CALL statements where Vertica's Describe +// phase returns an incomplete RowDescription and no execution-time +// RowDescription is sent to correct it. Synthetic entries use OID 0 so that +// rows.Next() returns the raw wire bytes as a string (the default case), +// making the behaviour explicit rather than falsely claiming a specific type. +func (r *rows) expandColumnDefs(numCols uint16) { + for uint16(len(r.columnDefs.Columns)) < numCols { + r.columnDefs.Columns = append(r.columnDefs.Columns, &msgs.BERowDescColumnDef{ + FieldName: fmt.Sprintf("unknown_col%d", len(r.columnDefs.Columns)), + DataTypeOID: 0, // unknown — falls to the default string case in rows.Next() + }) + } +} diff --git a/stmt.go b/stmt.go index ca7f4de..9f79c34 100644 --- a/stmt.go +++ b/stmt.go @@ -595,13 +595,27 @@ func (s *stmt) collectResults(ctx context.Context) (*rows, error) { switch msg := bMsg.(type) { case *msgs.BEDataRowMsg: + // Vertica's Describe phase under-reports the column count for CALL + // statements that emit RAISE NOTICE: the DataRow at execution time + // may contain more fields than columnDefs describes. Expand columnDefs + // on the first DataRow so that Columns() returns the correct width + // before database/sql sizes the destination slice. + if uint16(len(rows.columnDefs.Columns)) < msg.Columns().NumCols { + rows.expandColumnDefs(msg.Columns().NumCols) + } err = rows.addRow(msg) if err != nil { return rows, err } case *msgs.BERowDescMsg: - s.lastRowDesc = msg - rows = newRows(ctx, s.lastRowDesc, s.conn.serverTZOffset) + // An execution-time RowDescription may arrive before any DataRows + // and can carry a more complete schema than the Describe-phase one. + // Only adopt it if it has at least as many columns, to prevent a + // truncated description from silently replacing a wider one. + if rows.resultData.Peek() == nil && len(msg.Columns) >= len(rows.columnDefs.Columns) { + s.lastRowDesc = msg + rows = newRows(ctx, s.lastRowDesc, s.conn.serverTZOffset) + } case *msgs.BEErrorMsg: s.conn.sync() return newEmptyRows(), s.evaluateErrorMsg(msg) From e357666dea9baec84b1ce3801437f2f83d4132d4 Mon Sep 17 00:00:00 2001 From: sivaalampally Date: Thu, 12 Mar 2026 01:57:58 -0400 Subject: [PATCH 3/3] Addressing the review comments --- rows.go | 9 +++++++++ stmt.go | 13 +++++++++---- 2 files changed, 18 insertions(+), 4 deletions(-) diff --git a/rows.go b/rows.go index 7dcad1c..a84c814 100644 --- a/rows.go +++ b/rows.go @@ -359,6 +359,15 @@ func newEmptyRows() *rows { // RowDescription is sent to correct it. Synthetic entries use OID 0 so that // rows.Next() returns the raw wire bytes as a string (the default case), // making the behaviour explicit rather than falsely claiming a specific type. +// The caller invokes this whenever a DataRow wider than columnDefs is seen, +// not only on the first row. +// +// Ordering guarantee: this function is called inside collectResults(), which +// fully drains the wire and buffers all rows before returning the *rows object +// to database/sql. Because database/sql only calls Columns() after receiving +// that object, all expansions are complete before Columns() executes. +// This invariant would break in a streaming/lazy-row model where *rows is +// handed to the caller before all DataRows have been received. func (r *rows) expandColumnDefs(numCols uint16) { for uint16(len(r.columnDefs.Columns)) < numCols { r.columnDefs.Columns = append(r.columnDefs.Columns, &msgs.BERowDescColumnDef{ diff --git a/stmt.go b/stmt.go index 9f79c34..60ba1d1 100644 --- a/stmt.go +++ b/stmt.go @@ -596,10 +596,15 @@ func (s *stmt) collectResults(ctx context.Context) (*rows, error) { switch msg := bMsg.(type) { case *msgs.BEDataRowMsg: // Vertica's Describe phase under-reports the column count for CALL - // statements that emit RAISE NOTICE: the DataRow at execution time - // may contain more fields than columnDefs describes. Expand columnDefs - // on the first DataRow so that Columns() returns the correct width - // before database/sql sizes the destination slice. + // statements that emit RAISE NOTICE: a DataRow at execution time may + // contain more fields than columnDefs describes. Expand columnDefs + // whenever a wider DataRow is seen so that Columns() always returns + // the correct width. + // + // This is safe because collectResults() is synchronous and fully + // buffers all rows before returning *rows to the caller. database/sql + // calls Columns() only after receiving that object, so all expansions + // are complete by the time the column list is observed. if uint16(len(rows.columnDefs.Columns)) < msg.Columns().NumCols { rows.expandColumnDefs(msg.Columns().NumCols) }