Skip to content

Commit 5b31f49

Browse files
committed
sql/export: refactor processors to implement RowSource interface
We recently found an edge case where the output of EXPORT was used as an input to a mutation. Currently, in such a setup we have two goroutines since the export processors don't implement `execinfra.RowSource` interface, so we cannot fuse them with the mutation processor. Presence of concurrency forces usage of the LeafTxns, yet mutations require access to the RootTxn, thus we have conflicting requirements. Before the previous commit we'd get a nil pointer crash, with the previous commit in place we now get an assertion failure. This commit solves this issue by refactoring both processors to implement the `RowSource` interface. In the problematic query it allows all processors to be fused together to run in a single goroutine, so we can use the RootTxn as required by the mutation. Release note (bug fix): Previously, EXPORT CSV and EXPORT PARQUET stmts could result in a node crash when their result rows were used as the input to a mutation like an INSERT within the same SQL. This bug has been present since before 22.1 version and has now been fixed.
1 parent 7f174af commit 5b31f49

File tree

5 files changed

+420
-363
lines changed

5 files changed

+420
-363
lines changed

pkg/sql/execinfra/base.go

Lines changed: 0 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -332,45 +332,6 @@ func DrainAndClose(
332332
dst.ProducerDone()
333333
}
334334

335-
// NoMetadataRowSource is a wrapper on top of a RowSource that automatically
336-
// forwards metadata to a RowReceiver. Data rows are returned through an
337-
// interface similar to RowSource, except that, since metadata is taken care of,
338-
// only the data rows are returned.
339-
//
340-
// The point of this struct is that it'd be burdensome for some row consumers to
341-
// have to deal with metadata.
342-
type NoMetadataRowSource struct {
343-
src RowSource
344-
metadataSink RowReceiver
345-
}
346-
347-
// MakeNoMetadataRowSource builds a NoMetadataRowSource.
348-
func MakeNoMetadataRowSource(src RowSource, sink RowReceiver) NoMetadataRowSource {
349-
return NoMetadataRowSource{src: src, metadataSink: sink}
350-
}
351-
352-
// NextRow is analogous to RowSource.Next. If the producer sends an error, we
353-
// can't just forward it to metadataSink. We need to let the consumer know so
354-
// that it's not under the impression that everything is hunky-dory and it can
355-
// continue consuming rows. So, this interface returns the error. Just like with
356-
// a raw RowSource, the consumer should generally call ConsumerDone() and drain.
357-
func (rs *NoMetadataRowSource) NextRow() (rowenc.EncDatumRow, error) {
358-
for {
359-
row, meta := rs.src.Next()
360-
if meta == nil {
361-
return row, nil
362-
}
363-
if meta.Err != nil {
364-
return nil, meta.Err
365-
}
366-
// We forward the metadata and ignore the returned ConsumerStatus. There's
367-
// no good way to use that status here; eventually the consumer of this
368-
// NoMetadataRowSource will figure out the same status and act on it as soon
369-
// as a non-metadata row is received.
370-
_ = rs.metadataSink.Push(nil /* row */, meta)
371-
}
372-
}
373-
374335
// RowChannelMsg is the message used in the channels that implement
375336
// local physical streams (i.e. the RowChannel's).
376337
type RowChannelMsg struct {

pkg/sql/export/BUILD.bazel

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,16 +15,17 @@ go_library(
1515
"//pkg/settings",
1616
"//pkg/sql/catalog/colinfo",
1717
"//pkg/sql/execinfra",
18+
"//pkg/sql/execinfra/execopnode",
1819
"//pkg/sql/execinfrapb",
1920
"//pkg/sql/pgwire/pgcode",
2021
"//pkg/sql/pgwire/pgerror",
2122
"//pkg/sql/rowenc",
2223
"//pkg/sql/sem/tree",
2324
"//pkg/sql/types",
2425
"//pkg/util/encoding/csv",
26+
"//pkg/util/log",
2527
"//pkg/util/mon",
2628
"//pkg/util/parquet",
27-
"//pkg/util/tracing",
2829
"//pkg/util/unique",
2930
"@com_github_cockroachdb_errors//:errors",
3031
],

0 commit comments

Comments
 (0)