From 1bf81965c6edd31cb31d5a2440c973db571b987f Mon Sep 17 00:00:00 2001 From: Yahor Yuzefovich Date: Thu, 19 Sep 2019 09:12:40 -0700 Subject: [PATCH] row: check for uninitialized KVFetcher Previously, when GetRangesInfo() and GetBytesRead() was called on row.Fetcher before the underlying KVFetcher is initialized, it would panic due to NPE. Now this is fixed. The problem was introduced by the recent refactor of moving CFetcher from sql/row into sql/colexec. Release justification: Release blocker. Release note: None --- pkg/sql/row/fetcher.go | 9 +++++++-- pkg/sql/row/fetcher_test.go | 10 ++++++++++ 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/pkg/sql/row/fetcher.go b/pkg/sql/row/fetcher.go index d49639f0d08b..349006aecf17 100644 --- a/pkg/sql/row/fetcher.go +++ b/pkg/sql/row/fetcher.go @@ -1423,7 +1423,7 @@ func (rf *Fetcher) PartialKey(nCols int) (roachpb.Key, error) { // GetRangesInfo returns information about the ranges where the rows came from. // The RangeInfo's are deduped and not ordered. func (rf *Fetcher) GetRangesInfo() []roachpb.RangeInfo { - f := rf.kvFetcher.kvBatchFetcher + f := rf.kvFetcher if f == nil { // Not yet initialized. return nil @@ -1433,7 +1433,12 @@ func (rf *Fetcher) GetRangesInfo() []roachpb.RangeInfo { // GetBytesRead returns total number of bytes read by the underlying KVFetcher. func (rf *Fetcher) GetBytesRead() int64 { - return rf.kvFetcher.bytesRead + f := rf.kvFetcher + if f == nil { + // Not yet initialized. + return 0 + } + return f.bytesRead } // Only unique secondary indexes have extra columns to decode (namely the diff --git a/pkg/sql/row/fetcher_test.go b/pkg/sql/row/fetcher_test.go index 784e557b3e48..a711d9ead8c8 100644 --- a/pkg/sql/row/fetcher_test.go +++ b/pkg/sql/row/fetcher_test.go @@ -27,6 +27,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/util" "github.com/cockroachdb/cockroach/pkg/util/encoding" "github.com/cockroachdb/cockroach/pkg/util/leaktest" + "github.com/stretchr/testify/assert" ) type initFetcherArgs struct { @@ -1052,3 +1053,12 @@ func TestRowFetcherReset(t *testing.T) { func idLookupKey(tableID TableID, indexID sqlbase.IndexID) uint64 { return (uint64(tableID) << 32) | uint64(indexID) } + +func TestFetcherUninitialized(t *testing.T) { + // Regression test for #39013: make sure it's okay to call GetRangesInfo and + // GetBytesReader even before the fetcher was fully initialized. + var fetcher Fetcher + + assert.Nil(t, fetcher.GetRangesInfo()) + assert.Zero(t, fetcher.GetBytesRead()) +}