Add Unwrap methods to types which wrap an underlying error:
"encodinc/csv".ParseError
"encoding/json".MarshalerError
"net/http".transportReadFromServerError
"net".OpError
"net".DNSConfigError
"net/url".Error
"os/exec".Error
"signal/internal/pty".PtyError
"text/template".ExecError
Add os.ErrTemporary. A case could be made for putting this error
value in package net, since no exported error types in package os
include a Temporary method. However, syscall errors returned from
the os package do include this method.
Add Is methods to error types with a Timeout or Temporary method,
making errors.Is(err, os.Err{Timeout,Temporary}) equivalent to
testing the corresponding method:
"context".DeadlineExceeded
"internal/poll".TimeoutError
"net".adrinfoErrno
"net".OpError
"net".DNSError
"net/http".httpError
"net/http".tlsHandshakeTimeoutError
"net/pipe".timeoutError
"net/url".Error
Updates #30322
Updates #29934
Change-Id: I409fb20c072ea39116ebfb8c7534d493483870dc
Reviewed-on: https://go-review.googlesource.com/c/go/+/170037
Run-TryBot: Damien Neil <dneil@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Marcel van Lohuizen <mpvl@golang.org>
Add documentation that individual Write calls are buffered and
copy documentation from bufio.Writer notifying the user to call
Flush and Error when all writes are complete. Remove reference
to "file" since the implementation is general and allows any
io.Writer.
Fixes#30045
Change-Id: I50165470e548f296494e764707fbabe36c665015
Reviewed-on: https://go-review.googlesource.com/c/160680
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
'"' has special semantic meaning that conflicts with using it as Comma.
Change-Id: Ife25ba43ca25dba2ea184c1bb7579a230d376059
Reviewed-on: https://go-review.googlesource.com/99696
Run-TryBot: Joe Tsai <thebrokentoaster@gmail.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
In the situation where a quoted field is necessary, avoid processing
each UTF-8 rune one-by-one, which causes mangling of invalid sequences
into utf8.RuneError, causing a loss of information.
Instead, search only for the escaped characters, handle those specially
and copy everything else in between verbatim.
This symmetrically matches the behavior of Reader.
Fixes#24298
Change-Id: I9276f64891084ce8487678f663fad711b4095dbb
Reviewed-on: https://go-review.googlesource.com/99297
Run-TryBot: Joe Tsai <thebrokentoaster@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
This fixes a regression where only CRLF was folded into LF at EOF.
Now, we also truncate trailing CR at EOF to preserve the old behavior.
Every one of the test cases added exactly matches the behavior
of Go1.9, even if the results are somewhat unexpected.
Fixes#22937
Change-Id: I1bc6550533163ae489ea77ec1e598163267b7eec
Reviewed-on: https://go-review.googlesource.com/81577
Run-TryBot: Joe Tsai <thebrokentoaster@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
CL 52810 changed Reader to interpret a quoted \r\n as a raw \r\n
when reading fields. This seems likely to break existing users, and
discussion on both #21201 (the original issue that triggered the change)
and #22746 (discussing whether to revert the change) failed to identify
a single motivating example for this change. To avoid breaking existing
users for no clear reason, revert the change.
The Reader has been rewritten in the interim so this is not a git revert
but instead and adjustment (and slight simplification) of the new Reader.
Fixes#22746.
Change-Id: Ie857b2f4b1359a207d085b6d3c3a6d440a997d12
Reviewed-on: https://go-review.googlesource.com/78295
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
A record can span multiple lines (the whole reason for the extra field),
so the important fact is that it's the _start_ of the record.
Make that clear in the name.
(This API was added during the Go 1.10 cycle so it can still be cleaned up.)
Change-Id: Id95b3ceb7cdfc4aa0ed5a053cb84da8945fa5496
Reviewed-on: https://go-review.googlesource.com/78119
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
The record delimiter (not configurable by user) is "\r\n" or "\n".
It is insensible for the user to set Comma or Comment delimiters
to be some character that conflicts with the record delimiter.
Furthermore, it is insensible for Comma or Comment to be the same rune.
Allowing this leaks implementation details to the user in regards to
the evaluation order of which rune is checked for first.
Fixes#22404
Change-Id: I31e86abc9b3a8fb4584e090477795587740970ae
Reviewed-on: https://go-review.googlesource.com/72793
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Joe Tsai <thebrokentoaster@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The ErrQuote variable is only returned when a parsing error
occurs within a quoted string. Make that clear in the message.
Change-Id: I06ad5a9edb41afedde193c4f8b93551bb8342bbb
Reviewed-on: https://go-review.googlesource.com/72794
Reviewed-by: Avelino <t@avelino.xxx>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
We should be referring to ParseError.Err, which is the underlying error,
not ParseError.Error, which is the error method.
Change-Id: Ic3cef5ecbe1ada5fa14b9573222f29da8fc9a8d5
Reviewed-on: https://go-review.googlesource.com/72450
Reviewed-by: Tim Cooper <tim.cooper@layeh.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
CL 72150 fixes#22352 by reverting the problematic parts of that CL
where the line number and column number were inconsistent with each other.
This CL adds back functionality to address the issue that CL 72150
was trying to solve in the first place. That is, it reports the starting
line of the record, so that users have a frame of reference to start with
when debugging what went wrong.
In the event of gnarly CSV files with multiline quoted strings, a parse
failure likely occurs somewhere between the start of the record and
the point where the parser finally detected an error.
Since ParserError.{Line,Column} reports where the *error* occurs, we
add a RecordLine field to report where the record starts.
Also take this time to cleanup and modernize TestRead.
Fixes#19019Fixes#22352
Change-Id: I16cebf0b81922c35f75804c7073e9cddbfd11a04
Reviewed-on: https://go-review.googlesource.com/72310
Reviewed-by: Ian Lance Taylor <iant@golang.org>
The Reader implementation is slow because it operates on a rune-by-rune
basis via bufio.Reader.ReadRune. We speed this up by operating on entire
lines that we read from bufio.Reader.ReadSlice.
In order to ensure that we read the full line, we augment ReadSlice
in our Reader.readLine method to automatically expand the slice if
bufio.ErrBufferFull is every hit.
This change happens to fix#19410 because it no longer relies on
rune-by-rune parsing and only searches for the relevant delimiter rune.
In order to keep column accounting simple and consistent, this change
reverts parts of CL 52830.
This CL is an alternative to CL 36270 and builds on some of the ideas
from that change by Diogo Pinela.
name old time/op new time/op delta
Read-8 3.12µs ± 1% 2.54µs ± 2% -18.76% (p=0.000 n=10+9)
ReadWithFieldsPerRecord-8 3.12µs ± 1% 2.53µs ± 1% -18.91% (p=0.000 n=9+9)
ReadWithoutFieldsPerRecord-8 3.13µs ± 0% 2.57µs ± 3% -18.07% (p=0.000 n=10+10)
ReadLargeFields-8 52.3µs ± 1% 5.3µs ± 2% -89.93% (p=0.000 n=10+9)
ReadReuseRecord-8 2.05µs ± 1% 1.40µs ± 1% -31.48% (p=0.000 n=10+9)
ReadReuseRecordWithFieldsPerRecord-8 2.05µs ± 1% 1.41µs ± 0% -31.03% (p=0.000 n=10+9)
ReadReuseRecordWithoutFieldsPerRecord-8 2.06µs ± 1% 1.40µs ± 1% -31.70% (p=0.000 n=9+10)
ReadReuseRecordLargeFields-8 50.9µs ± 0% 4.1µs ± 3% -92.01% (p=0.000 n=10+10)
name old alloc/op new alloc/op
Read-8 664B ± 0% 664B ± 0%
ReadWithFieldsPerRecord-8 664B ± 0% 664B ± 0%
ReadWithoutFieldsPerRecord-8 664B ± 0% 664B ± 0%
ReadLargeFields-8 3.94kB ± 0% 3.94kB ± 0%
ReadReuseRecord-8 24.0B ± 0% 24.0B ± 0%
ReadReuseRecordWithFieldsPerRecord-8 24.0B ± 0% 24.0B ± 0%
ReadReuseRecordWithoutFieldsPerRecord-8 24.0B ± 0% 24.0B ± 0%
ReadReuseRecordLargeFields-8 2.98kB ± 0% 2.98kB ± 0%
name old allocs/op new allocs/op
Read-8 18.0 ± 0% 18.0 ± 0%
ReadWithFieldsPerRecord-8 18.0 ± 0% 18.0 ± 0%
ReadWithoutFieldsPerRecord-8 18.0 ± 0% 18.0 ± 0%
ReadLargeFields-8 24.0 ± 0% 24.0 ± 0%
ReadReuseRecord-8 8.00 ± 0% 8.00 ± 0%
ReadReuseRecordWithFieldsPerRecord-8 8.00 ± 0% 8.00 ± 0%
ReadReuseRecordWithoutFieldsPerRecord-8 8.00 ± 0% 8.00 ± 0%
ReadReuseRecordLargeFields-8 12.0 ± 0% 12.0 ± 0%
Updates #22352
Updates #19019Fixes#16791Fixes#19410
Change-Id: I31c27cfcc56880e6abac262f36c947179b550bbf
Reviewed-on: https://go-review.googlesource.com/72150
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Joe Tsai <thebrokentoaster@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The parser mistakenly assumed it could always fold \r\n into \n, which
is not true since a \r\n inside a quoted fields has no special meaning
and should be kept as is.
Fix this by not folding \r\n to \n inside quotes fields.
Fixes#21201
Change-Id: Ifebc302e49cf63e0a027ee90f088dbc050a2b7a6
Reviewed-on: https://go-review.googlesource.com/52810
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Errors returned by Reader contain the line where the Reader originally
encountered the error. This can be suboptimal since that line does not
always correspond with the line the current record/field started at.
This can easily happen with LazyQuotes as seen in #19019, but also
happens for example when a quoted fields has no closing quote and
the parser hits EOF before it finds another quote.
When this happens finding the erroneous field can be somewhat
complicated and time consuming, and in most cases it would be better to
report the line where the record started.
This change updates Reader to keep track of the line on which a record
begins and uses it for errors instead of the current line, making it
easier to find errors.
Although a user-visible change, this should have no impact on existing
code, since most users don't explicitly work with the line in the error
and probably already expect the new behaviour.
Updates #19019
Change-Id: Ic9bc70fad2651c69435d614d537e7a9266819b05
Reviewed-on: https://go-review.googlesource.com/52830
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
In many cases the records returned by Reader.Read will only be used between calls
to Read and become garbage once a new record is read. In this case, instead of
allocating a new slice on each call to Read, we can reuse the last allocated slice
for successive calls to avoid unnecessary allocations.
This change adds a new field ReuseRecord to the Reader struct to enable this reuse.
ReuseRecord is false by default to avoid breaking existing code which dependss on
the current behaviour.
I also added 4 new benchmarks, corresponding to the existing Read benchmarks, which
set ReuseRecord to true.
Benchstat on my local machine (old is ReuseRecord = false, new is ReuseRecord = true)
name old time/op new time/op delta
Read-8 2.75µs ± 2% 1.88µs ± 1% -31.52% (p=0.000 n=14+15)
ReadWithFieldsPerRecord-8 2.75µs ± 0% 1.89µs ± 1% -31.43% (p=0.000 n=13+13)
ReadWithoutFieldsPerRecord-8 2.77µs ± 1% 1.88µs ± 1% -32.06% (p=0.000 n=15+15)
ReadLargeFields-8 55.4µs ± 1% 54.2µs ± 0% -2.07% (p=0.000 n=15+14)
name old alloc/op new alloc/op delta
Read-8 664B ± 0% 24B ± 0% -96.39% (p=0.000 n=15+15)
ReadWithFieldsPerRecord-8 664B ± 0% 24B ± 0% -96.39% (p=0.000 n=15+15)
ReadWithoutFieldsPerRecord-8 664B ± 0% 24B ± 0% -96.39% (p=0.000 n=15+15)
ReadLargeFields-8 3.94kB ± 0% 2.98kB ± 0% -24.39% (p=0.000 n=15+15)
name old allocs/op new allocs/op delta
Read-8 18.0 ± 0% 8.0 ± 0% -55.56% (p=0.000 n=15+15)
ReadWithFieldsPerRecord-8 18.0 ± 0% 8.0 ± 0% -55.56% (p=0.000 n=15+15)
ReadWithoutFieldsPerRecord-8 18.0 ± 0% 8.0 ± 0% -55.56% (p=0.000 n=15+15)
ReadLargeFields-8 24.0 ± 0% 12.0 ± 0% -50.00% (p=0.000 n=15+15)
Fixes#19721
Change-Id: I79b14128bb9bb3465f53f40f93b1b528a9da6f58
Reviewed-on: https://go-review.googlesource.com/41730
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This commit changes parseRecord to allocate a single string per record,
instead of per field, by using indexes into the raw record.
Benchstat (done with f69991c17)
name old time/op new time/op delta
Read-8 3.17µs ± 0% 2.78µs ± 1% -12.35% (p=0.016 n=4+5)
ReadWithFieldsPerRecord-8 3.18µs ± 1% 2.79µs ± 1% -12.23% (p=0.008 n=5+5)
ReadWithoutFieldsPerRecord-8 4.59µs ± 0% 2.77µs ± 0% -39.58% (p=0.016 n=4+5)
ReadLargeFields-8 57.0µs ± 0% 55.7µs ± 0% -2.18% (p=0.008 n=5+5)
name old alloc/op new alloc/op delta
Read-8 660B ± 0% 664B ± 0% +0.61% (p=0.008 n=5+5)
ReadWithFieldsPerRecord-8 660B ± 0% 664B ± 0% +0.61% (p=0.008 n=5+5)
ReadWithoutFieldsPerRecord-8 1.14kB ± 0% 0.66kB ± 0% -41.75% (p=0.008 n=5+5)
ReadLargeFields-8 3.86kB ± 0% 3.94kB ± 0% +1.86% (p=0.008 n=5+5)
name old allocs/op new allocs/op delta
Read-8 30.0 ± 0% 18.0 ± 0% -40.00% (p=0.008 n=5+5)
ReadWithFieldsPerRecord-8 30.0 ± 0% 18.0 ± 0% -40.00% (p=0.008 n=5+5)
ReadWithoutFieldsPerRecord-8 50.0 ± 0% 18.0 ± 0% -64.00% (p=0.008 n=5+5)
ReadLargeFields-8 66.0 ± 0% 24.0 ± 0% -63.64% (p=0.008 n=5+5)
For a simple application that I wrote, which reads in a CSV file (via
ReadAll) and outputs the number of rows read (15857625 rows), this change
reduces the total time on my notebook from ~58 seconds to ~48 seconds.
This reduces time and allocations (bytes) each by ~6% for a real world
CSV file at work (~230000 rows, 13 colums).
Updates #16791
Change-Id: Ia07177c94624e55cdd3064a7d2751fb69322d3e4
Reviewed-on: https://go-review.googlesource.com/24723
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Benchmarks broken off from https://golang.org/cl/24723 and modified to
allocate less in the places we're not trying to measure.
Updates #16791
Change-Id: I508e4cfeac60322d56f1d71ff1912f6a6f183a63
Reviewed-on: https://go-review.googlesource.com/30357
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This patch updates the doc about comments whitespace for the
encoding/csv package to reflect that leading whitespace before
the hash will treat the line as not a comment.
Fixes#13775.
Change-Id: Ia468c75b242a487b4b2b4cd3d342bfb8e07720ba
Reviewed-on: https://go-review.googlesource.com/23302
Reviewed-by: Ian Lance Taylor <iant@golang.org>
The intent of this comment is to reduce the number of issues opened
against the package to add support for new kinds of CSV formats, such as
issues #3150, #8458, #12372, #12755.
Change-Id: I452c0b748e4ca9ebde3e6cea188bf7774372148e
Reviewed-on: https://go-review.googlesource.com/23401
Reviewed-by: Andrew Gerrand <adg@golang.org>
The tree's pretty inconsistent about single space vs double space
after a period in documentation. Make it consistently a single space,
per earlier decisions. This means contributors won't be confused by
misleading precedence.
This CL doesn't use go/doc to parse. It only addresses // comments.
It was generated with:
$ perl -i -npe 's,^(\s*// .+[a-z]\.) +([A-Z]),$1 $2,' $(git grep -l -E '^\s*//(.+\.) +([A-Z])')
$ go test go/doc -update
Change-Id: Iccdb99c37c797ef1f804a94b22ba5ee4b500c4f7
Reviewed-on: https://go-review.googlesource.com/20022
Reviewed-by: Rob Pike <r@golang.org>
Reviewed-by: Dave Day <djd@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Named returned values should only be used on public funcs and methods
when it contributes to the documentation.
Named return values should not be used if they're only saving the
programmer a few lines of code inside the body of the function,
especially if that means there's stutter in the documentation or it
was only there so the programmer could use a naked return
statement. (Naked returns should not be used except in very small
functions)
This change is a manual audit & cleanup of public func signatures.
Signatures were not changed if:
* the func was private (wouldn't be in public godoc)
* the documentation referenced it
* the named return value was an interesting name. (i.e. it wasn't
simply stutter, repeating the name of the type)
There should be no changes in behavior. (At least: none intended)
Change-Id: I3472ef49619678fe786e5e0994bdf2d9de76d109
Reviewed-on: https://go-review.googlesource.com/20024
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Andrew Gerrand <adg@golang.org>
The documentation listing err == EOF can be confusing to newcomers
to the language who are looking for the relevant documentation for
that error.
Change-Id: I301885950d0e1d0fbdf3a1892fca86eac7a0c616
Reviewed-on: https://go-review.googlesource.com/15806
Reviewed-by: Andrew Gerrand <adg@golang.org>
These were found by grepping the comments from the go code and feeding
the output to aspell.
Change-Id: Id734d6c8d1938ec3c36bd94a4dbbad577e3ad395
Reviewed-on: https://go-review.googlesource.com/10941
Reviewed-by: Aamir Khan <syst3m.w0rm@gmail.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Currently parseRecord will always start with a nil
slice and then resize the slice on append. For input
with a fixed number of fields per record we can preallocate
the slice to avoid having to resize the slice.
This change implements this optimization by using
FieldsPerRecord as capacity if it's > 0 and also adds a
benchmark to better show the differences.
benchmark old ns/op new ns/op delta
BenchmarkRead 19741 17909 -9.28%
benchmark old allocs new allocs delta
BenchmarkRead 59 41 -30.51%
benchmark old bytes new bytes delta
BenchmarkRead 6276 5844 -6.88%
Change-Id: I7c2abc9c80a23571369bcfcc99a8ffc474eae7ab
Reviewed-on: https://go-review.googlesource.com/8880
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
In theory both of these lines encode the same three fields:
a,,c
a,"",c
However, Postgres defines that when importing CSV, the unquoted
version is treated as NULL (missing), while the quoted version is
treated as a string value (empty string). If the middle field is supposed to
be an integer value, the first line can be imported (NULL is okay), but
the second line cannot (empty string is not).
Postgres's import command (COPY FROM) has an option to force
the unquoted empty to be interpreted as a string but it does not
have an option to force the quoted empty to be interpreted as a NULL.
From http://www.postgresql.org/docs/9.0/static/sql-copy.html:
The CSV format has no standard way to distinguish a NULL
value from an empty string. PostgreSQL's COPY handles this
by quoting. A NULL is output as the NULL parameter string
and is not quoted, while a non-NULL value matching the NULL
parameter string is quoted. For example, with the default
settings, a NULL is written as an unquoted empty string,
while an empty string data value is written with double
quotes (""). Reading values follows similar rules. You can
use FORCE_NOT_NULL to prevent NULL input comparisons for
specific columns.
Therefore printing the unquoted empty is more flexible for
imports into Postgres than printing the quoted empty.
In addition to making the output more useful with Postgres, not
quoting empty strings makes the output smaller and easier to read.
It also matches the behavior of Microsoft Excel and Google Drive.
Since we are here and making concessions for Postgres, handle this
case too (again quoting the Postgres docs):
Because backslash is not a special character in the CSV
format, \., the end-of-data marker, could also appear as a
data value. To avoid any misinterpretation, a \. data value
appearing as a lone entry on a line is automatically quoted
on output, and on input, if quoted, is not interpreted as
the end-of-data marker. If you are loading a file created by
another application that has a single unquoted column and
might have a value of \., you might need to quote that value
in the input file.
Fixes#7586.
LGTM=bradfitz
R=bradfitz
CC=golang-codereviews
https://golang.org/cl/164760043
Alex Gaynor
Ainar Garipov
Dominik Honnef
Damien Neil
Romain Baugue
Brian Kessler
Joe Tsai
Russ Cox
Justin Nuß
Brad Fitzpatrick
Jess Frazelle
Ian Lance Taylor
Robert Griesemer
Eric Lagergren
Nathan VanBenschoten
Katrina Owen
Carlos C
Josh Bleecher Snyder