New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
pgwire text encoding fixes #32144
pgwire text encoding fixes #32144
Commits on Nov 27, 2018
-
pgwire: enable text encoding tests in generate-binary
These don't pass as of this commit and are thus skipped. Release note: None
-
pgwire: remove time output from dates
Release note (bug fix): dates no longer have a time component in their text encoding over the wire.
-
tree: change TestEval to use a data directory
Using the giant list is cumbersome and makes fixing output difficult. Using datadriven gives us some additional organization and allows us to use the -rewrite flag when the format of a datum changes.
-
pgwire: use postgres instead of Go interval encoding for pgwire
These changes match postgres for all tested inputs. Benchmarks are slower across the board which makes sense due to the div/mod nanosecond section that is now not conditional. name old time/op new time/op delta Encodings/'10y10mon'::interval/text-8 836ns ± 4% 1055ns ± 4% +26.19% (p=0.008 n=5+5) Encodings/'1y1mon'::interval/text-8 830ns ± 6% 1019ns ±15% +22.78% (p=0.008 n=5+5) Encodings/'23:12:34'::interval/text-8 1.00µs ± 5% 1.10µs ± 9% +9.18% (p=0.032 n=5+5) Encodings/'-23:00:00'::interval/text-8 791ns ± 7% 1016ns ± 5% +28.56% (p=0.008 n=5+5) Encodings/'-10d'::interval/text-8 798ns ±12% 958ns ± 3% +20.03% (p=0.008 n=5+5) Encodings/'1ms'::interval/text-8 765ns ± 3% 1418ns ± 6% +85.36% (p=0.008 n=5+5) Encodings/'.2ms'::interval/text-8 777ns ± 3% 1387ns ± 4% +78.39% (p=0.008 n=5+5) Encodings/'-1mon1m'::interval/text-8 898ns ± 5% 1386ns ± 9% +54.29% (p=0.008 n=5+5) Encodings/'-1y1m'::interval/text-8 900ns ± 7% 1380ns ± 1% +53.34% (p=0.008 n=5+5) Encodings/'3y4mon5d6ms'::interval/text-8 1.13µs ± 2% 2.23µs ±13% +97.24% (p=0.008 n=5+5) Encodings/'296537y20d15h30m7s'::interval/text-8 1.35µs ± 8% 1.77µs ± 8% +31.03% (p=0.008 n=5+5) Encodings/'-2965y_-20d_-15h_-30m_-7s'::interval/text-8 1.39µs ± 6% 1.67µs ± 4% +19.92% (p=0.008 n=5+5) Encodings/'00:00:00'::interval/text-8 566ns ± 3% 583ns ± 3% ~ (p=0.175 n=5+5) Release note (bug fix): intervals now match postgres in their text encoding over the wire. Release note (bug fix): intervals no longer sometimes lose 1ns of precision. This only happened rarely due to floating point inaccuracy.
-
pgwire: test timezones and BC for timestamp
timestamptz differs slightly but isn't broken, which is why it's commented out.
-
-
pgwire: verify encoding length instead of ignoring it in TestEncodings
Improved output and smarter type handling as well.
-
pgwire: fix array and tuple encoding
Match postgres encoding much more closely for tuples and arrays. Implement binary tuple encoding. It's pretty simple: number of elements then each element's OID and binary encoding. Text encoding for arrays and tuples has some fairly simple quoting rules: whitespace, quotes, or commas. We were also encoding unicode runes more than needed. Unwray datums to remove any OID oddities. This is already done in non-array/tuple encoding. Change the encoding tests to specify exactly the bytes the Text format wants. We still keep the text format around in the encodings.json file just for human consumption. Since text editors have problems displaying weirdo things like the byte order mark it's safer to just print out the expected bytes as numbers instead of hoping there's a value there that our editors can't show. But in any case switch to %q for the Text display to try to improve the debugability anyway. The benchmark below is faster in cases where the type in the array or tuple is a string. I think this is because of the removed call to unicode.IsGraphic on each rune. The slower benchmark is mostly ints, which now go through the same escape and quote check logic as strings, which would explain the slowdown. We could special case those to improve performance further. name old time/op new time/op delta Encodings/array['']/text-8 511ns ± 8% 475ns ± 4% ~ (p=0.095 n=5+5) Encodings/array['\x0d53e338548082'::BYTEA]/text-8 846ns ±10% 604ns ± 9% -28.57% (p=0.008 n=5+5) Encodings/array['test_with_spaces'::BYTEA]/text-8 883ns ± 3% 719ns ±10% -18.66% (p=0.008 n=5+5) Encodings/array['name'::NAME]/text-8 893ns ± 6% 599ns ±11% -32.95% (p=0.008 n=5+5) Encodings/array[NULL]::text[]/text-8 615ns ± 9% 523ns ± 2% -14.99% (p=0.008 n=5+5) Encodings/array['']::text[]/text-8 587ns ± 1% 553ns ± 9% ~ (p=0.151 n=5+5) Encodings/array['test']::text[]/text-8 651ns ± 3% 579ns ± 8% -10.97% (p=0.008 n=5+5) Encodings/array['test_with_spaces']::text[]/text-8 837ns ± 9% 688ns ± 7% -17.79% (p=0.008 n=5+5) Encodings/array[e'\f']::text[]/text-8 634ns ± 2% 550ns ± 8% -13.27% (p=0.008 n=5+5) Encodings/array[e'\uFEFF']::text[]/text-8 1.18µs ± 4% 0.57µs ± 3% -51.94% (p=0.008 n=5+5) Encodings/array[e'\u2603']::text[]/text-8 799ns ± 3% 573ns ± 7% -28.25% (p=0.008 n=5+5) Encodings/(1::int8,2::int8,3::int8,4::int8,null)/text-8 488ns ± 7% 619ns ± 3% +26.99% (p=0.008 n=5+5) Encodings/('test_with_spaces'::BYTEA,null)/text-8 720ns ±27% 460ns ± 8% -36.12% (p=0.008 n=5+5) Encodings/('test_with_spaces'::TEXT,null)/text-8 517ns ± 9% 437ns ±10% -15.51% (p=0.008 n=5+5) Encodings/('name'::NAME,null)/text-8 525ns ± 6% 325ns ± 7% -38.02% (p=0.008 n=5+5) Encodings/('false'::JSONB,null)/text-8 1.04µs ± 7% 0.63µs ± 6% -39.00% (p=0.008 n=5+5) Encodings/('{"a":_[]}'::JSONB,null)/text-8 1.25µs ± 6% 0.75µs ±13% -40.44% (p=0.008 n=5+5) Fixes #31847 Release note (bug fix): Correct pgwire encoding for arrays and tuples.
-
-
pgwire: encoding tests/fixes for decimal and float nan/inf
Release note (bug fix): Correct binary decimal encoding for NaN.
-
pgwire: add encoding benchmarks
These use the same machinery as the encoding tests, so factor out some common code. Release note: None