mirror of
https://github.com/johnkerl/miller.git
synced 2026-07-17 16:38:54 +00:00
The join verb's left-file ingest silently swallowed read errors: a
nonexistent or malformed left file produced empty (or partial) output
with exit code 0.
Two defects, one per ingest path:
* Unsorted (half-streaming) path, ingestLeftFile in
pkg/transformers/join.go: errors from the record-reader's error
channel were dropped ("TODO: propagate error to caller"), as were
record-reader construction errors. All left-file read failures were
silent.
* Sorted (-s) path, readRecord in
pkg/transformers/utils/join_bucket_keeper.go: the error channel was
consulted, but the record-reader goroutine sends the error and the
end-of-stream marker on separate channels, so the select could see
end-of-stream first and drop the error. A nonexistent left file with
-s exited 0 about half the time (racy).
Both paths now print "mlr: <error>" to stderr and exit 1, matching how
read errors on regular (right) input files are reported. On receipt of
the end-of-stream marker, both consumers now do a final non-blocking
drain of the error channel, which is deterministic since the reader
sends any error before the marker.
Discovered while verifying #377.
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
parent
e291280fde
commit
5f152d7669
19 changed files with 37 additions and 5 deletions
|
|
@ -491,8 +491,8 @@ func (tr *TransformerJoin) ingestLeftFile() {
|
||||||
// TODO: perhaps increase recordsPerBatch, and/or refactor
|
// TODO: perhaps increase recordsPerBatch, and/or refactor
|
||||||
recordReader, err := input.Create(readerOpts, 1)
|
recordReader, err := input.Create(readerOpts, 1)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
// TODO: propagate error to caller
|
fmt.Fprintf(os.Stderr, "mlr: %v\n", err)
|
||||||
return
|
os.Exit(1)
|
||||||
}
|
}
|
||||||
|
|
||||||
// Set the initial context for the left-file.
|
// Set the initial context for the left-file.
|
||||||
|
|
@ -521,9 +521,8 @@ func (tr *TransformerJoin) ingestLeftFile() {
|
||||||
select {
|
select {
|
||||||
|
|
||||||
case err := <-errorChannel:
|
case err := <-errorChannel:
|
||||||
// TODO: propagate error to caller
|
fmt.Fprintf(os.Stderr, "mlr: %v\n", err)
|
||||||
_ = err
|
os.Exit(1)
|
||||||
return
|
|
||||||
|
|
||||||
case leftrecsAndContexts := <-readerChannel:
|
case leftrecsAndContexts := <-readerChannel:
|
||||||
// TODO: temp for batch-reader refactor
|
// TODO: temp for batch-reader refactor
|
||||||
|
|
@ -532,6 +531,17 @@ func (tr *TransformerJoin) ingestLeftFile() {
|
||||||
leftrecAndContext.Record = utils.KeepLeftFieldNames(leftrecAndContext.Record, tr.leftKeepFieldNameSet)
|
leftrecAndContext.Record = utils.KeepLeftFieldNames(leftrecAndContext.Record, tr.leftKeepFieldNameSet)
|
||||||
|
|
||||||
if leftrecAndContext.EndOfStream {
|
if leftrecAndContext.EndOfStream {
|
||||||
|
// The record-reader may have sent an error (e.g. the left file
|
||||||
|
// is missing or unreadable) immediately before its end-of-stream
|
||||||
|
// marker. Since those are separate channels, the select can see
|
||||||
|
// the end-of-stream marker first -- so, check the error channel
|
||||||
|
// before declaring the ingest complete.
|
||||||
|
select {
|
||||||
|
case err := <-errorChannel:
|
||||||
|
fmt.Fprintf(os.Stderr, "mlr: %v\n", err)
|
||||||
|
os.Exit(1)
|
||||||
|
default:
|
||||||
|
}
|
||||||
done = true
|
done = true
|
||||||
break // breaks the switch, not the for, in Golang
|
break // breaks the switch, not the for, in Golang
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -553,6 +553,17 @@ func (keeper *JoinBucketKeeper) readRecord() *types.RecordAndContext {
|
||||||
leftrecAndContext := leftrecsAndContexts[0]
|
leftrecAndContext := leftrecsAndContexts[0]
|
||||||
leftrecAndContext.Record = KeepLeftFieldNames(leftrecAndContext.Record, keeper.leftKeepFieldNameSet)
|
leftrecAndContext.Record = KeepLeftFieldNames(leftrecAndContext.Record, keeper.leftKeepFieldNameSet)
|
||||||
if leftrecAndContext.EndOfStream { // end-of-stream marker
|
if leftrecAndContext.EndOfStream { // end-of-stream marker
|
||||||
|
// The record-reader may have sent an error (e.g. the left file is
|
||||||
|
// missing or unreadable) immediately before its end-of-stream
|
||||||
|
// marker. Since those are separate channels, the select can see
|
||||||
|
// the end-of-stream marker first -- so, check the error channel
|
||||||
|
// before declaring the left-file read complete.
|
||||||
|
select {
|
||||||
|
case err := <-keeper.errorChannel:
|
||||||
|
fmt.Fprintf(os.Stderr, "mlr: %v\n", err)
|
||||||
|
os.Exit(1)
|
||||||
|
default:
|
||||||
|
}
|
||||||
keeper.recordReaderDone = true
|
keeper.recordReaderDone = true
|
||||||
return nil
|
return nil
|
||||||
}
|
}
|
||||||
|
|
|
||||||
1
test/cases/verb-join/left-file-malformed-sorted/cmd
Normal file
1
test/cases/verb-join/left-file-malformed-sorted/cmd
Normal file
|
|
@ -0,0 +1 @@
|
||||||
|
mlr --icsv --opprint join -s -j a -f test/input/join-left-ragged.csv test/input/abixy.csv
|
||||||
1
test/cases/verb-join/left-file-malformed-sorted/experr
Normal file
1
test/cases/verb-join/left-file-malformed-sorted/experr
Normal file
|
|
@ -0,0 +1 @@
|
||||||
|
mlr: CSV header/data length mismatch 2 != 4 at filename test/input/join-left-ragged.csv row 3
|
||||||
0
test/cases/verb-join/left-file-malformed-sorted/expout
Normal file
0
test/cases/verb-join/left-file-malformed-sorted/expout
Normal file
1
test/cases/verb-join/left-file-malformed-unsorted/cmd
Normal file
1
test/cases/verb-join/left-file-malformed-unsorted/cmd
Normal file
|
|
@ -0,0 +1 @@
|
||||||
|
mlr --icsv --opprint join -j a -f test/input/join-left-ragged.csv test/input/abixy.csv
|
||||||
1
test/cases/verb-join/left-file-malformed-unsorted/experr
Normal file
1
test/cases/verb-join/left-file-malformed-unsorted/experr
Normal file
|
|
@ -0,0 +1 @@
|
||||||
|
mlr: CSV header/data length mismatch 2 != 4 at filename test/input/join-left-ragged.csv row 3
|
||||||
0
test/cases/verb-join/left-file-malformed-unsorted/expout
Normal file
0
test/cases/verb-join/left-file-malformed-unsorted/expout
Normal file
|
|
@ -0,0 +1 @@
|
||||||
|
mlr --icsv --opprint join -s -j a -f /nonesuch/nope/never test/input/abixy.csv
|
||||||
|
|
@ -0,0 +1 @@
|
||||||
|
mlr: open /nonesuch/nope/never: no such file or directory
|
||||||
|
|
@ -0,0 +1 @@
|
||||||
|
mlr --icsv --opprint join -j a -f /nonesuch/nope/never test/input/abixy.csv
|
||||||
|
|
@ -0,0 +1 @@
|
||||||
|
mlr: open /nonesuch/nope/never: no such file or directory
|
||||||
3
test/input/join-left-ragged.csv
Normal file
3
test/input/join-left-ragged.csv
Normal file
|
|
@ -0,0 +1,3 @@
|
||||||
|
a,l
|
||||||
|
pan,one
|
||||||
|
eks,two,three,four
|
||||||
|
Loading…
Add table
Add a link
Reference in a new issue