pgsql: Fix corner-case loss of precision in numeric ln().
Fix corner-case loss of precision in numeric ln(). When deciding on the local rscale to use for the Taylor series expansion, ln_var() neglected to account for the fact that the result is subsequently multiplied by a factor of 2^(nsqrt+1), where nsqrt is the number of square root operations performed in the range reduction step, which can be as high as 22 for very large inputs. This could result in a loss of precision, particularly when combined with large rscale values, for which a large number of Taylor series terms is required (up to around 400). Fix by computing a few extra digits in the Taylor series, based on the weight of the multiplicative factor log10(2^(nsqrt+1)). It remains to be proven whether or not the other 8 extra digits used for the Taylor series is appropriate, but this at least deals with the obvious oversight of failing to account for the effects of the final multiplication. Per report from Justin AnyhowStep. Reviewed by Tom Lane. Discussion: https://postgr.es/m/16280-279f299d9c06e...@postgresql.org Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/43a899f41f46918a0bf442edb091b08c214c68f8 Modified Files -- src/backend/utils/adt/numeric.c | 10 +- src/test/regress/expected/numeric_big.out | 19 +++ src/test/regress/sql/numeric_big.sql | 14 ++ 3 files changed, 42 insertions(+), 1 deletion(-)
Re: pgsql: Add deduplication to nbtree.
Peter Geoghegan writes:
> Add deduplication to nbtree.
Coverity isn't very happy with the coding in _bt_update_posting():
*** CID 1460433: Memory - corruptions (ARRAY_VS_SINGLETON)
/srv/coverity/git/pgsql-git/postgresql/src/backend/access/nbtree/nbtdedup.c:
723 in _bt_update_posting()
717 {
718 if (d < vacposting->ndeletedtids &&
vacposting->deletetids[d] == i)
719 {
720 d++;
721 continue;
722 }
>>> CID 1460433: Memory - corruptions (ARRAY_VS_SINGLETON)
>>> Using "htids" as an array. This might corrupt or misinterpret adjacent
>>> memory locations.
723 htids[ui++] = *BTreeTupleGetPostingN(origtuple, i);
724 }
725 Assert(ui == nhtids);
726 Assert(d == vacposting->ndeletedtids);
727 Assert(nhtids == 1 || _bt_posting_valid(itup));
I can see its point: asserting after the fact that you didn't clobber
memory isn't a terribly safe coding method, especially in a production
build where you won't even have the asserts. Not sure if there's a
better way though.
regards, tom lane
Re: pgsql: Add deduplication to nbtree.
On Sun, Mar 1, 2020 at 10:24 AM Tom Lane wrote: > I can see its point: asserting after the fact that you didn't clobber > memory isn't a terribly safe coding method, especially in a production > build where you won't even have the asserts. Not sure if there's a > better way though. I found it slightly more elegant to treat itup->t_tid as a degenerate 1 element posting list here, but I'm not particularly attached to that approach. The loop is only truly necessary when dealing with a posting list tuple. Do you think that _bt_update_posting() should avoid this loop when itup is just a plain tuple, that lacks a posting list? I can do it that way if you prefer. -- Peter Geoghegan
Re: pgsql: Add deduplication to nbtree.
Peter Geoghegan writes: > On Sun, Mar 1, 2020 at 10:24 AM Tom Lane wrote: >> I can see its point: asserting after the fact that you didn't clobber >> memory isn't a terribly safe coding method, especially in a production >> build where you won't even have the asserts. Not sure if there's a >> better way though. > I found it slightly more elegant to treat itup->t_tid as a degenerate > 1 element posting list here, but I'm not particularly attached to that > approach. The loop is only truly necessary when dealing with a posting > list tuple. > Do you think that _bt_update_posting() should avoid this loop when > itup is just a plain tuple, that lacks a posting list? I can do it > that way if you prefer. Hm. That would probably be enough to shut up Coverity, but I'm unsure whether it'd really be an improvement from the legibility and safety viewpoints. Do you want to try coding it that way and see what it comes out like? Note that we do have the ability to just dismiss the Coverity complaint, if we decide that there's no better way to write the function. But it's usually worth looking to see if it could be written more cleanly. regards, tom lane
Re: pgsql: Add deduplication to nbtree.
On Sun, Mar 1, 2020 at 11:29 AM Tom Lane wrote: > Hm. That would probably be enough to shut up Coverity, but I'm unsure > whether it'd really be an improvement from the legibility and safety > viewpoints. I noticed that _bt_update_posting() behaves as if the origtuple might not be a posting list tuple at the point that keysize is calculated, despite generally depending on it being a posting list tuple (which it asserts by way of its "_bt_posting_valid(origtuple)" assertion). The final tuple might not be a posting list, but the original one must be (if it isn't, then nbtree VACUUM should be deleting it outright in the traditional way, rather than updating it). I should fix that, either way. > Do you want to try coding it that way and see what it > comes out like? Sure. -- Peter Geoghegan
pgsql: Remove dead code from _bt_update_posting().
Remove dead code from _bt_update_posting(). Discussion: https://postgr.es/m/CAH2-WzmAufHiOku6AGiFD=81vqs5nyj1l2ykhw1t+bh4cms...@mail.gmail.com Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/84ec9b231a865348f5388dcc125c084297709332 Modified Files -- src/backend/access/nbtree/nbtdedup.c | 6 +- 1 file changed, 1 insertion(+), 5 deletions(-)
Re: pgsql: Add deduplication to nbtree.
On Sun, Mar 1, 2020 at 11:42 AM Peter Geoghegan wrote: > > Do you want to try coding it that way and see what it > > comes out like? > > Sure. Attached patch shows how this could work. I prefer my original approach, but I can see the argument for doing it this way. If we keep my original approach, we should still add a new "ItemPointerIsValid(&itup->t_tid)" assertion that covers the plain tupe case in a way that mirrors the current "_bt_posting_valid(itup)" assert. -- Peter Geoghegan 0001-Restructure-_bt_update_posting.patch Description: Binary data
Re: pgsql: Add deduplication to nbtree.
Peter Geoghegan writes: > Attached patch shows how this could work. I prefer my original > approach, but I can see the argument for doing it this way. This does seem a bit duplicative ... and shouldn't both code paths include a final "Assert(d == vacposting->ndeletedtids)"? So maybe we're better off just rejecting the Coverity complaint. > If we keep my original approach, we should still add a new > "ItemPointerIsValid(&itup->t_tid)" assertion that covers the plain > tupe case in a way that mirrors the current "_bt_posting_valid(itup)" > assert. Another thing that maybe bears closer scrutiny is the size calculation. It seems a bit confused as to whether the offset of the posting list within the tuple, or the total tuple size, or both, needs to be MAXALIGN'd. regards, tom lane
Re: pgsql: Add deduplication to nbtree.
On Sun, Mar 1, 2020 at 2:14 PM Tom Lane wrote: > > Attached patch shows how this could work. I prefer my original > > approach, but I can see the argument for doing it this way. > > This does seem a bit duplicative ... and shouldn't both code paths > include a final "Assert(d == vacposting->ndeletedtids)"? No, because the "nhtids == 1" loop has a "break" when the first and only TID that we need to keep around is hit. > Another thing that maybe bears closer scrutiny is the size calculation. > It seems a bit confused as to whether the offset of the posting list > within the tuple, or the total tuple size, or both, needs to be > MAXALIGN'd. That's not quite true in my opinion. BTreeTupleSetPosting() has an Assert() of its own about alignment. ISTM that it's reasonable for _bt_update_posting() to assume that BTreeTupleGetPostingOffset() will return a MAXALIGN()'d offset. Note also that _bt_form_posting() is explicit about what it expects around alignment. This is _bt_update_posting()'s sibling function. The _bt_update_posting() calculation is explicitly documented as being derived from the one in _bt_form_posting(). I am happy to add parallel-to-_bt_form_posting() assertions about alignment to _bt_form_posting(), to nail it down completely. Plus I'll add the assertion I suggested already. Once I commit a patch with these two new assertions, I think that we can consider the matter resolved. Does that seem reasonable to you? -- Peter Geoghegan
Re: pgsql: Add deduplication to nbtree.
On Sun, Mar 1, 2020 at 3:01 PM Peter Geoghegan wrote: > I am happy to add parallel-to-_bt_form_posting() assertions about > alignment to _bt_form_posting(), to nail it down completely. Plus I'll > add the assertion I suggested already. Once I commit a patch with > these two new assertions, I think that we can consider the matter > resolved. Does that seem reasonable to you? I was thinking of the approach taken in the attached patch. It more or less copies over the assertions from _bt_form_posting() that did not appear in _bt_update_posting(). -- Peter Geoghegan 0001-Add-assertions-to-_bt_update_posting.patch Description: Binary data
pgsql: Handle logical decoding in multi-insert for catalog tuples
Handle logical decoding in multi-insert for catalog tuples The code path for multi-insert decoding is not stressed yet for catalogs (a future patch may introduce this capability), so no back-patch is needed. Author: Daniel Gustafsson Discussion: https://postgr.es/m/9690d72f-5c4f-4016-9572-6d16684e1...@yesql.se Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/12c5cad76f9247f39b6e542ef1c6255912c2adda Modified Files -- src/backend/replication/logical/decode.c | 61 +++- 1 file changed, 28 insertions(+), 33 deletions(-)
Re: pgsql: Revert "initdb: Change authentication defaults"
On Sat, Feb 29, 2020 at 10:41 AM Tom Lane wrote: > > Magnus Hagander writes: > > On Mon, Jul 22, 2019 at 10:29 AM Peter Eisentraut > > wrote: > >> Revert "initdb: Change authentication defaults" > >> This reverts commit 09f08930f0f6fd4a7350ac02f29124b919727198. > >> The buildfarm client needs some adjustments first. > > > What ended up happening with this? Did we end up somewhere deciding we > > didn't actually want this, or has it been dropped for 13? (Tried and > > failed to find discussion around it) > > Did the buildfarm adjustments get made? (I'm assuming Andrew knows) > It's in Release 11 of the client and is mentioned in the release notes. The release is dated last September. Making this change would force a flag day update to that version for the buildfarm client, Note - the buildfarm code is completely public. In this case, see https://github.com/PGBuildFarm/client-code/commit/55b4d691552607197207e4462d7c0e6d9608d3e2 cheers andrew -- Andrew Dunstanhttps://www.2ndQuadrant.com PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Re: pgsql: Add deduplication to nbtree.
Peter Geoghegan writes: > On Sun, Mar 1, 2020 at 3:01 PM Peter Geoghegan wrote: >> I am happy to add parallel-to-_bt_form_posting() assertions about >> alignment to _bt_form_posting(), to nail it down completely. Plus I'll >> add the assertion I suggested already. Once I commit a patch with >> these two new assertions, I think that we can consider the matter >> resolved. Does that seem reasonable to you? > I was thinking of the approach taken in the attached patch. It more or > less copies over the assertions from _bt_form_posting() that did not > appear in _bt_update_posting(). OK by me. regards, tom lane
pgsql: Fix command-line colorization on Windows with VT100-compatible e
Fix command-line colorization on Windows with VT100-compatible environments When setting PG_COLOR to "always" or "auto" in a Windows terminal VT100-compatible, the colorization output was not showing up correctly because it is necessary to update the console's output handling mode. This fix allows to detect automatically if the environment is compatible with VT100. Hence, PG_COLOR=auto is able to detect and handle both compatible and non-compatible environments. The behavior of PG_COLOR=always remains unchanged, as it enforces the use of colorized output even if the environment does not allow it. This fix is based on an initial suggestion from Thomas Munro. Reported-by: Haiying Tang Author: Juan José Santamaría Flecha Reviewed-by: Michail Nikolaev, Michael Paquier, Haiying Tang Discussion: https://postgr.es/m/16108-134692e97146b...@postgresql.org Backpatch-through: 12 Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/3b77dce863d94de2de40b2a302c0f58248655e6c Modified Files -- src/common/logging.c | 50 +- 1 file changed, 49 insertions(+), 1 deletion(-)
pgsql: Fix command-line colorization on Windows with VT100-compatible e
Fix command-line colorization on Windows with VT100-compatible environments When setting PG_COLOR to "always" or "auto" in a Windows terminal VT100-compatible, the colorization output was not showing up correctly because it is necessary to update the console's output handling mode. This fix allows to detect automatically if the environment is compatible with VT100. Hence, PG_COLOR=auto is able to detect and handle both compatible and non-compatible environments. The behavior of PG_COLOR=always remains unchanged, as it enforces the use of colorized output even if the environment does not allow it. This fix is based on an initial suggestion from Thomas Munro. Reported-by: Haiying Tang Author: Juan José Santamaría Flecha Reviewed-by: Michail Nikolaev, Michael Paquier, Haiying Tang Discussion: https://postgr.es/m/16108-134692e97146b...@postgresql.org Backpatch-through: 12 Branch -- REL_12_STABLE Details --- https://git.postgresql.org/pg/commitdiff/3b5709e664c44c75c1d27a20c59fdbb0dec3ded5 Modified Files -- src/common/logging.c | 50 +- 1 file changed, 49 insertions(+), 1 deletion(-)
Re: pgsql: Revert "initdb: Change authentication defaults"
Andrew Dunstan writes: > On Sat, Feb 29, 2020 at 10:41 AM Tom Lane wrote: >> Did the buildfarm adjustments get made? (I'm assuming Andrew knows) > It's in Release 11 of the client and is mentioned in the release > notes. The release is dated last September. Making this change would > force a flag day update to that version for the buildfarm client, Hm, so scraping the buildfarm logs shows that we currently have this many animals reporting (on HEAD) for each client script_version: 42 'REL_11' 64 'REL_10' 2 'REL_9' 7 'REL_8' 1 'REL_7' 1 'REL_4.15' Looks like requiring REL_11 would still be a pretty large ask. regards, tom lane
pgsql: Remove long unused code behind a #if 0
Remove long unused code behind a #if 0 Author: Vignesh C Discussion: https://www.postgresql.org/message-id/flat/caldanm3sn4yoq-4rogb-cfe0eyw6b3mvzz8+dns9bnrwpnh...@mail.gmail.com Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/d433b79b89c7d429ab69cb00857a8aca45d0ad47 Modified Files -- src/interfaces/ecpg/pgtypeslib/numeric.c | 82 1 file changed, 82 deletions(-)
