These functions must be careful that they return the intended value oferrno to their callers.  There were several scenarios where this mightnot happen:1. The recent SSL renegotiation patch added a hunk of code that wouldexecute after setting errno.  In the first place, it's doubtful that weshould consider renegotiation to be successfully completed after a failure,and in the second, there's no real guarantee that the called OpenSSLroutines wouldn't clobber errno.  Fix by not executing that hunk exceptduring success exit.2. errno was left in an unknown state in case of an unrecognized returncode from SSL_get_error().  While this is a "can't happen" case, it seemslike a good idea to be sure we know what would happen, so reset errno toECONNRESET in such cases.  (The corresponding code in libpq's fe-secure.calready did this.)3. There was an (undocumented) assumption that client_read_ended() wouldn'tchange errno.  While true in the current state of the code, this seems lessthan future-proof.  Add explicit saving/restoring of errno to make surethat changes in the called functions won't break things.I see no need to back-patch, since#1 is new code and the other two issuesare mostly hypothetical.Per discussion with Amit Kapila.

These functions must be careful that they return the intended value oferrno to their callers.  There were several scenarios where this mightnot happen:1. The recent SSL renegotiation patch added a hunk of code that wouldexecute after setting errno.  In the first place, it's doubtful that weshould consider renegotiation to be successfully completed after a failure,and in the second, there's no real guarantee that the called OpenSSLroutines wouldn't clobber errno.  Fix by not executing that hunk exceptduring success exit.2. errno was left in an unknown state in case of an unrecognized returncode from SSL_get_error().  While this is a "can't happen" case, it seemslike a good idea to be sure we know what would happen, so reset errno toECONNRESET in such cases.  (The corresponding code in libpq's fe-secure.calready did this.)3. There was an (undocumented) assumption that client_read_ended() wouldn'tchange errno.  While true in the current state of the code, this seems lessthan future-proof.  Add explicit saving/restoring of errno to make surethat changes in the called functions won't break things.I see no need to back-patch, sincepostgres#1 is new code and the other two issuesare mostly hypothetical.Per discussion with Amit Kapila.

Includes documentation for executor README.  A high-level handling ofapproachpostgres#2 to value locking also appears there, since in contrast withdesignpostgres#1, that is something that lives in the head of the executor.

Includes documentation for executor README.  A high-level handling ofapproachpostgres#2 to value locking also appears there, since in contrast withdesignpostgres#1, that is something that lives in the head of the executor.

Includes documentation for executor README.  A high-level handling ofapproachpostgres#2 to value locking also appears there, since in contrast withdesignpostgres#1, that is something that lives in the head of the executor.

Includes documentation for executor README.  A high-level handling ofapproachpostgres#2 to value locking also appears there, since in contrast withdesignpostgres#1, that is something that lives in the head of the executor.

Includes documentation for executor README.  A high-level handling ofapproachpostgres#2 to value locking also appears there, since in contrast withdesignpostgres#1, that is something that lives in the head of the executor.

Includes documentation for executor README.  A high-level handling ofapproachpostgres#2 to value locking also appears there, since in contrast withdesignpostgres#1, that is something that lives in the head of the executor.

Includes documentation for executor README.  A high-level handling ofapproachpostgres#2 to value locking also appears there, since in contrast withdesignpostgres#1, that is something that lives in the head of the executor.

Includes documentation for executor README.  A high-level handling ofapproachpostgres#2 to value locking also appears there, since in contrast withdesignpostgres#1, that is something that lives in the head of the executor.

Includes documentation for executor README.  A high-level handling ofapproachpostgres#2 to value locking also appears there, since in contrast withdesignpostgres#1, that is something that lives in the head of the executor.

Includes documentation for executor README.  A high-level handling ofapproachpostgres#2 to value locking also appears there, since in contrast withdesignpostgres#1, that is something that lives in the head of the executor.

Includes documentation for executor README.  A high-level handling ofapproachpostgres#2 to value locking also appears there, since in contrast withdesignpostgres#1, that is something that lives in the head of the executor.

Includes documentation for executor README.  A high-level handling ofapproachpostgres#2 to value locking also appears there, since in contrast withdesignpostgres#1, that is something that lives in the head of the executor.

Includes documentation for executor README.  A high-level handling ofapproachpostgres#2 to value locking also appears there, since in contrast withdesignpostgres#1, that is something that lives in the head of the executor.

Includes documentation for executor README.  A high-level handling ofapproachpostgres#2 to value locking also appears there, since in contrast withdesignpostgres#1, that is something that lives in the head of the executor.

Includes documentation for executor README.  A high-level handling ofapproachpostgres#2 to value locking also appears there, since in contrast withdesignpostgres#1, that is something that lives in the head of the executor.

Includes documentation for executor README.  A high-level handling ofapproachpostgres#2 to value locking also appears there, since in contrast withdesignpostgres#1, that is something that lives in the head of the executor.

Includes documentation for executor README.  A high-level handling ofapproachpostgres#2 to value locking also appears there, since in contrast withdesignpostgres#1, that is something that lives in the head of the executor.

Includes documentation for executor README.  A high-level handling ofapproachpostgres#2 to value locking also appears there, since in contrast withdesignpostgres#1, that is something that lives in the head of the executor.

Includes documentation for executor README.  A high-level handling ofapproachpostgres#2 to value locking also appears there, since in contrast withdesignpostgres#1, that is something that lives in the head of the executor.

Includes documentation for executor README.  A high-level handling ofapproachpostgres#2 to value locking also appears there, since in contrast withdesignpostgres#1, that is something that lives in the head of the executor.

Includes documentation for executor README.  A high-level handling ofapproachpostgres#2 to value locking also appears there, since in contrast withdesignpostgres#1, that is something that lives in the head of the executor.

Includes documentation for executor README.  A high-level handling ofapproachpostgres#2 to value locking also appears there, since in contrast withdesignpostgres#1, that is something that lives in the head of the executor.

Includes documentation for executor README.  A high-level handling ofapproachpostgres#2 to value locking also appears there, since in contrast withdesignpostgres#1, that is something that lives in the head of the executor.

Includes documentation for executor README.  A high-level handling ofapproachpostgres#2 to value locking also appears there, since in contrast withdesignpostgres#1, that is something that lives in the head of the executor.

refresh_by_match_merge() has some issues in the way it builds a SQLquery to construct the "diff" table:1. It doesn't require the selected unique index(es) to be indimmediate.2. It doesn't pay attention to the particular equality semantics enforcedby a given index, but just assumes that they must be those of the columndatatype's default btree opclass.3. It doesn't check that the indexes are btrees.4. It's insufficiently careful to ensure that the parser will pick theintended operator when parsing the query.  (This would have been asecurity bug beforeCVE-2018-1058.)5. It's not careful about indexes on system columns.The way tofix#4 is to make use of the existing code in ri_triggers.cfor generating an arbitrary binary operator clause.  I chose to movethat to ruleutils.c, since that seems a more reasonable place to beexporting such functionality from than ri_triggers.c.While#1,#3, and#5 are just latent given existing feature restrictions,and#2 doesn't arise in the core system for lack of alternate opclasseswith different equality behaviors,#4 seems like an issue worthback-patching.  That's the bulk of the change anyway, so just back-patchthe whole thing to 9.4 where this code was introduced.Discussion:https://postgr.es/m/13836.1521413227@sss.pgh.pa.us

refresh_by_match_merge() has some issues in the way it builds a SQLquery to construct the "diff" table:1. It doesn't require the selected unique index(es) to be indimmediate.2. It doesn't pay attention to the particular equality semantics enforcedby a given index, but just assumes that they must be those of the columndatatype's default btree opclass.3. It doesn't check that the indexes are btrees.4. It's insufficiently careful to ensure that the parser will pick theintended operator when parsing the query.  (This would have been asecurity bug beforeCVE-2018-1058.)5. It's not careful about indexes on system columns.The way tofix#4 is to make use of the existing code in ri_triggers.cfor generating an arbitrary binary operator clause.  I chose to movethat to ruleutils.c, since that seems a more reasonable place to beexporting such functionality from than ri_triggers.c.While#1,#3, and#5 are just latent given existing feature restrictions,and#2 doesn't arise in the core system for lack of alternate opclasseswith different equality behaviors,#4 seems like an issue worthback-patching.  That's the bulk of the change anyway, so just back-patchthe whole thing to 9.4 where this code was introduced.Discussion:https://postgr.es/m/13836.1521413227@sss.pgh.pa.us

refresh_by_match_merge() has some issues in the way it builds a SQLquery to construct the "diff" table:1. It doesn't require the selected unique index(es) to be indimmediate.2. It doesn't pay attention to the particular equality semantics enforcedby a given index, but just assumes that they must be those of the columndatatype's default btree opclass.3. It doesn't check that the indexes are btrees.4. It's insufficiently careful to ensure that the parser will pick theintended operator when parsing the query.  (This would have been asecurity bug beforeCVE-2018-1058.)5. It's not careful about indexes on system columns.The way tofix#4 is to make use of the existing code in ri_triggers.cfor generating an arbitrary binary operator clause.  I chose to movethat to ruleutils.c, since that seems a more reasonable place to beexporting such functionality from than ri_triggers.c.While#1,#3, and#5 are just latent given existing feature restrictions,and#2 doesn't arise in the core system for lack of alternate opclasseswith different equality behaviors,#4 seems like an issue worthback-patching.  That's the bulk of the change anyway, so just back-patchthe whole thing to 9.4 where this code was introduced.Discussion:https://postgr.es/m/13836.1521413227@sss.pgh.pa.us

refresh_by_match_merge() has some issues in the way it builds a SQLquery to construct the "diff" table:1. It doesn't require the selected unique index(es) to be indimmediate.2. It doesn't pay attention to the particular equality semantics enforcedby a given index, but just assumes that they must be those of the columndatatype's default btree opclass.3. It doesn't check that the indexes are btrees.4. It's insufficiently careful to ensure that the parser will pick theintended operator when parsing the query.  (This would have been asecurity bug beforeCVE-2018-1058.)5. It's not careful about indexes on system columns.The way tofix#4 is to make use of the existing code in ri_triggers.cfor generating an arbitrary binary operator clause.  I chose to movethat to ruleutils.c, since that seems a more reasonable place to beexporting such functionality from than ri_triggers.c.While#1,#3, and#5 are just latent given existing feature restrictions,and#2 doesn't arise in the core system for lack of alternate opclasseswith different equality behaviors,#4 seems like an issue worthback-patching.  That's the bulk of the change anyway, so just back-patchthe whole thing to 9.4 where this code was introduced.Discussion:https://postgr.es/m/13836.1521413227@sss.pgh.pa.us

refresh_by_match_merge() has some issues in the way it builds a SQLquery to construct the "diff" table:1. It doesn't require the selected unique index(es) to be indimmediate.2. It doesn't pay attention to the particular equality semantics enforcedby a given index, but just assumes that they must be those of the columndatatype's default btree opclass.3. It doesn't check that the indexes are btrees.4. It's insufficiently careful to ensure that the parser will pick theintended operator when parsing the query.  (This would have been asecurity bug beforeCVE-2018-1058.)5. It's not careful about indexes on system columns.The way tofix#4 is to make use of the existing code in ri_triggers.cfor generating an arbitrary binary operator clause.  I chose to movethat to ruleutils.c, since that seems a more reasonable place to beexporting such functionality from than ri_triggers.c.While#1,#3, and#5 are just latent given existing feature restrictions,and#2 doesn't arise in the core system for lack of alternate opclasseswith different equality behaviors,#4 seems like an issue worthback-patching.  That's the bulk of the change anyway, so just back-patchthe whole thing to 9.4 where this code was introduced.Discussion:https://postgr.es/m/13836.1521413227@sss.pgh.pa.us

The original setup for dependencies of partitioned objects hadserious problems:1. It did not verify that a drop cascading to a partition-child objectalso cascaded to at least one of the object's partition parents.  Now,normally a child object would share all its dependencies with one oranother parent (e.g. a child index's opclass dependencies would be sharedwith the parent index), so that this oversight is usually harmless.But if some dependency failed to fit this pattern, the child could bedropped while all its parents remain, creating a logically brokensituation.  (It's easy to construct artificial cases that break it,such as attaching an unrelated extension dependency to the child objectand then dropping the extension.  I'm not sure if any less-artificialcases exist.)2. Management of partition dependencies during ATTACH/DETACH PARTITIONwas complicated and buggy; for example, after detaching a partitiontable it was possible to create cases where a formerly-child indexshould be dropped and was not, because the correct set of dependencieshad not been reconstructed.Less seriously, because multiple partition relationships wererepresented identically in pg_depend, there was an order-of-traversaldependency on which partition parent was cited in error messages.We also had some pre-existing order-of-traversal hazards for errormessages related to internal and extension dependencies.  This iscosmetic to users but causes testing problems.Tofix#1, add a check at the end of the partition tree traversalto ensure that at least one partition parent got deleted.  Tofix#2,establish a new policy that partition dependencies are in addition to,not instead of, a child object's usual dependencies; in this wayATTACH/DETACH PARTITION need not cope with adding or removing theusual dependencies.To fix the cosmetic problem, distinguish between primary and secondarypartition dependency entries in pg_depend, by giving them differentdeptypes.  (They behave identically except for having differentpriorities for being cited in error messages.)  This means that theformer 'I' dependency type is replaced with new 'P' and 'S' types.This also fixes a longstanding bug that after handling an internaldependency by recursing to the owning object, findDependentObjectsdid not verify that the current target was now scheduled for deletion,and did not apply the current recursion level's objflags to it.Perhaps that should be back-patched; but in the back branches itwould only matter if some concurrent transaction had removed theinternal-linkage pg_depend entry before the recursive call found it,or the recursive call somehow failed to find it, both of which seemunlikely.Catversion bump because the contents of pg_depend change forpartitioning relationships.Patch HEAD only.  It's annoying that we're not fixing#2 in v11,but there seems no practical way to do so given that the problemis exactly a poor choice of what entries to put in pg_depend.We can't really fix that while staying compatible with what'sin pg_depend in existing v11 installations.Discussion:https://postgr.es/m/CAH2-Wzkypv1R+teZrr71U23J578NnTBt2X8+Y=Odr4pOdW1rXg@mail.gmail.com

…tions.Commit3d956d9 added support for update row movement in postgres_fdw.This patch fixes the following issues introduced by that commit:* When a remote partition chosen to insert routed rows into was also an  UPDATE subplan target rel that would be updated later, the UPDATE that  used a direct modification plan modified those routed rows incorrectly  because those routed rows were visible to the later UPDATE command.  The right fix for this would be to have some way in postgres_fdw in  which the later UPDATE command ignores those routed rows, but it seems  hard to do so with the current infrastructure.  For now throw an error  in that case.* When a remote partition chosen to insert routed rows into was also an  UPDATE subplan target rel, fmstate created for the UPDATE that used a  non-direct modification plan was mistakenly overridden by another  fmstate created for inserting those routed rows into the partition.  This caused 1) server crash when the partition would be updated later,  and 2) resource leak when the partition had been already updated.  To  avoid that, adjust the treatment of the fmstate for the inserting.  As  for#1, since we would also have the incorrectness issue as mentioned  above, error out in that case as well.Update the docs to mention that postgres_fdw currently does not handlethe case where a remote partition chosen to insert a routed row into isalso an UPDATE subplan target rel that will be updated later.Author: Amit Langote and Etsuro FujitaReviewed-by: Amit LangoteBackpatch-through: 11 where row movement in postgres_fdw was addedDiscussion:https://postgr.es/m/21e7eaa4-0d4d-20c2-a1f7-c7e96f4ce440@lab.ntt.co.jp

…tions.Commit3d956d9 added support for update row movement in postgres_fdw.This patch fixes the following issues introduced by that commit:* When a remote partition chosen to insert routed rows into was also an  UPDATE subplan target rel that would be updated later, the UPDATE that  used a direct modification plan modified those routed rows incorrectly  because those routed rows were visible to the later UPDATE command.  The right fix for this would be to have some way in postgres_fdw in  which the later UPDATE command ignores those routed rows, but it seems  hard to do so with the current infrastructure.  For now throw an error  in that case.* When a remote partition chosen to insert routed rows into was also an  UPDATE subplan target rel, fmstate created for the UPDATE that used a  non-direct modification plan was mistakenly overridden by another  fmstate created for inserting those routed rows into the partition.  This caused 1) server crash when the partition would be updated later,  and 2) resource leak when the partition had been already updated.  To  avoid that, adjust the treatment of the fmstate for the inserting.  As  for#1, since we would also have the incorrectness issue as mentioned  above, error out in that case as well.Update the docs to mention that postgres_fdw currently does not handlethe case where a remote partition chosen to insert a routed row into isalso an UPDATE subplan target rel that will be updated later.Author: Amit Langote and Etsuro FujitaReviewed-by: Amit LangoteBackpatch-through: 11 where row movement in postgres_fdw was addedDiscussion:https://postgr.es/m/21e7eaa4-0d4d-20c2-a1f7-c7e96f4ce440@lab.ntt.co.jp

Due to how pg_size_pretty(bigint) was implemented, it's possible that whengiven a negative number of bytes that the returning value would not matchthe equivalent positive return value when given the equivalent positivenumber of bytes.  This was due to two separate issues.1. The function used bit shifting to convert the number of bytes intolarger units.  The rounding performed by bit shifting is not the same asdividing.  For example -3 >> 1 = -2, but -3 / 2 = -1.  These twooperations are only equivalent with positive numbers.2. The half_rounded() macro rounded towards positive infinity.  This meantthat negative numbers rounded towards zero and positive numbers roundedaway from zero.Here wefix#1 by dividing the values instead of bit shifting.  Wefix#2by adjusting the half_rounded macro always to round away from zero.Additionally, adjust the pg_size_pretty(numeric) function to be moreexplicit that it's using division rather than bit shifting.  A casualobserver might have believed bit shifting was used due to a staticfunction being named numeric_shift_right.  However, that function wascalculating the divisor from the number of bits and performed division.Here we make that more clear.  This change is just cosmetic and does notaffect the return value of the numeric version of the function.Here we also add a set of regression tests both versions ofpg_size_pretty() which test the values directly before and after thefunction switches to the next unit.This bug was introduced in8a1fab3. Prior to that negative values werealways displayed in bytes.Author: Dean Rasheed, David RowleyDiscussion:https://postgr.es/m/CAEZATCXnNW4HsmZnxhfezR5FuiGgp+mkY4AzcL5eRGO4fuadWg@mail.gmail.comBackpatch-through: 9.6, where the bug was introduced.

Due to how pg_size_pretty(bigint) was implemented, it's possible that whengiven a negative number of bytes that the returning value would not matchthe equivalent positive return value when given the equivalent positivenumber of bytes.  This was due to two separate issues.1. The function used bit shifting to convert the number of bytes intolarger units.  The rounding performed by bit shifting is not the same asdividing.  For example -3 >> 1 = -2, but -3 / 2 = -1.  These twooperations are only equivalent with positive numbers.2. The half_rounded() macro rounded towards positive infinity.  This meantthat negative numbers rounded towards zero and positive numbers roundedaway from zero.Here wefix#1 by dividing the values instead of bit shifting.  Wefix#2by adjusting the half_rounded macro always to round away from zero.Additionally, adjust the pg_size_pretty(numeric) function to be moreexplicit that it's using division rather than bit shifting.  A casualobserver might have believed bit shifting was used due to a staticfunction being named numeric_shift_right.  However, that function wascalculating the divisor from the number of bits and performed division.Here we make that more clear.  This change is just cosmetic and does notaffect the return value of the numeric version of the function.Here we also add a set of regression tests both versions ofpg_size_pretty() which test the values directly before and after thefunction switches to the next unit.This bug was introduced in8a1fab3. Prior to that negative values werealways displayed in bytes.Author: Dean Rasheed, David RowleyDiscussion:https://postgr.es/m/CAEZATCXnNW4HsmZnxhfezR5FuiGgp+mkY4AzcL5eRGO4fuadWg@mail.gmail.comBackpatch-through: 9.6, where the bug was introduced.

Due to how pg_size_pretty(bigint) was implemented, it's possible that whengiven a negative number of bytes that the returning value would not matchthe equivalent positive return value when given the equivalent positivenumber of bytes.  This was due to two separate issues.1. The function used bit shifting to convert the number of bytes intolarger units.  The rounding performed by bit shifting is not the same asdividing.  For example -3 >> 1 = -2, but -3 / 2 = -1.  These twooperations are only equivalent with positive numbers.2. The half_rounded() macro rounded towards positive infinity.  This meantthat negative numbers rounded towards zero and positive numbers roundedaway from zero.Here wefix#1 by dividing the values instead of bit shifting.  Wefix#2by adjusting the half_rounded macro always to round away from zero.Additionally, adjust the pg_size_pretty(numeric) function to be moreexplicit that it's using division rather than bit shifting.  A casualobserver might have believed bit shifting was used due to a staticfunction being named numeric_shift_right.  However, that function wascalculating the divisor from the number of bits and performed division.Here we make that more clear.  This change is just cosmetic and does notaffect the return value of the numeric version of the function.Here we also add a set of regression tests both versions ofpg_size_pretty() which test the values directly before and after thefunction switches to the next unit.This bug was introduced in8a1fab3. Prior to that negative values werealways displayed in bytes.Author: Dean Rasheed, David RowleyDiscussion:https://postgr.es/m/CAEZATCXnNW4HsmZnxhfezR5FuiGgp+mkY4AzcL5eRGO4fuadWg@mail.gmail.comBackpatch-through: 9.6, where the bug was introduced.

Due to how pg_size_pretty(bigint) was implemented, it's possible that whengiven a negative number of bytes that the returning value would not matchthe equivalent positive return value when given the equivalent positivenumber of bytes.  This was due to two separate issues.1. The function used bit shifting to convert the number of bytes intolarger units.  The rounding performed by bit shifting is not the same asdividing.  For example -3 >> 1 = -2, but -3 / 2 = -1.  These twooperations are only equivalent with positive numbers.2. The half_rounded() macro rounded towards positive infinity.  This meantthat negative numbers rounded towards zero and positive numbers roundedaway from zero.Here wefix#1 by dividing the values instead of bit shifting.  Wefix#2by adjusting the half_rounded macro always to round away from zero.Additionally, adjust the pg_size_pretty(numeric) function to be moreexplicit that it's using division rather than bit shifting.  A casualobserver might have believed bit shifting was used due to a staticfunction being named numeric_shift_right.  However, that function wascalculating the divisor from the number of bits and performed division.Here we make that more clear.  This change is just cosmetic and does notaffect the return value of the numeric version of the function.Here we also add a set of regression tests both versions ofpg_size_pretty() which test the values directly before and after thefunction switches to the next unit.This bug was introduced in8a1fab3. Prior to that negative values werealways displayed in bytes.Author: Dean Rasheed, David RowleyDiscussion:https://postgr.es/m/CAEZATCXnNW4HsmZnxhfezR5FuiGgp+mkY4AzcL5eRGO4fuadWg@mail.gmail.comBackpatch-through: 9.6, where the bug was introduced.

Due to how pg_size_pretty(bigint) was implemented, it's possible that whengiven a negative number of bytes that the returning value would not matchthe equivalent positive return value when given the equivalent positivenumber of bytes.  This was due to two separate issues.1. The function used bit shifting to convert the number of bytes intolarger units.  The rounding performed by bit shifting is not the same asdividing.  For example -3 >> 1 = -2, but -3 / 2 = -1.  These twooperations are only equivalent with positive numbers.2. The half_rounded() macro rounded towards positive infinity.  This meantthat negative numbers rounded towards zero and positive numbers roundedaway from zero.Here wefix#1 by dividing the values instead of bit shifting.  Wefix#2by adjusting the half_rounded macro always to round away from zero.Additionally, adjust the pg_size_pretty(numeric) function to be moreexplicit that it's using division rather than bit shifting.  A casualobserver might have believed bit shifting was used due to a staticfunction being named numeric_shift_right.  However, that function wascalculating the divisor from the number of bits and performed division.Here we make that more clear.  This change is just cosmetic and does notaffect the return value of the numeric version of the function.Here we also add a set of regression tests both versions ofpg_size_pretty() which test the values directly before and after thefunction switches to the next unit.This bug was introduced in8a1fab3. Prior to that negative values werealways displayed in bytes.Author: Dean Rasheed, David RowleyDiscussion:https://postgr.es/m/CAEZATCXnNW4HsmZnxhfezR5FuiGgp+mkY4AzcL5eRGO4fuadWg@mail.gmail.comBackpatch-through: 9.6, where the bug was introduced.

Due to how pg_size_pretty(bigint) was implemented, it's possible that whengiven a negative number of bytes that the returning value would not matchthe equivalent positive return value when given the equivalent positivenumber of bytes.  This was due to two separate issues.1. The function used bit shifting to convert the number of bytes intolarger units.  The rounding performed by bit shifting is not the same asdividing.  For example -3 >> 1 = -2, but -3 / 2 = -1.  These twooperations are only equivalent with positive numbers.2. The half_rounded() macro rounded towards positive infinity.  This meantthat negative numbers rounded towards zero and positive numbers roundedaway from zero.Here wefix#1 by dividing the values instead of bit shifting.  Wefix#2by adjusting the half_rounded macro always to round away from zero.Additionally, adjust the pg_size_pretty(numeric) function to be moreexplicit that it's using division rather than bit shifting.  A casualobserver might have believed bit shifting was used due to a staticfunction being named numeric_shift_right.  However, that function wascalculating the divisor from the number of bits and performed division.Here we make that more clear.  This change is just cosmetic and does notaffect the return value of the numeric version of the function.Here we also add a set of regression tests both versions ofpg_size_pretty() which test the values directly before and after thefunction switches to the next unit.This bug was introduced in8a1fab3. Prior to that negative values werealways displayed in bytes.Author: Dean Rasheed, David RowleyDiscussion:https://postgr.es/m/CAEZATCXnNW4HsmZnxhfezR5FuiGgp+mkY4AzcL5eRGO4fuadWg@mail.gmail.comBackpatch-through: 9.6, where the bug was introduced.

Due to how pg_size_pretty(bigint) was implemented, it's possible that whengiven a negative number of bytes that the returning value would not matchthe equivalent positive return value when given the equivalent positivenumber of bytes.  This was due to two separate issues.1. The function used bit shifting to convert the number of bytes intolarger units.  The rounding performed by bit shifting is not the same asdividing.  For example -3 >> 1 = -2, but -3 / 2 = -1.  These twooperations are only equivalent with positive numbers.2. The half_rounded() macro rounded towards positive infinity.  This meantthat negative numbers rounded towards zero and positive numbers roundedaway from zero.Here wefix#1 by dividing the values instead of bit shifting.  Wefix#2by adjusting the half_rounded macro always to round away from zero.Additionally, adjust the pg_size_pretty(numeric) function to be moreexplicit that it's using division rather than bit shifting.  A casualobserver might have believed bit shifting was used due to a staticfunction being named numeric_shift_right.  However, that function wascalculating the divisor from the number of bits and performed division.Here we make that more clear.  This change is just cosmetic and does notaffect the return value of the numeric version of the function.Here we also add a set of regression tests both versions ofpg_size_pretty() which test the values directly before and after thefunction switches to the next unit.This bug was introduced in8a1fab3. Prior to that negative values werealways displayed in bytes.Author: Dean Rasheed, David RowleyDiscussion:https://postgr.es/m/CAEZATCXnNW4HsmZnxhfezR5FuiGgp+mkY4AzcL5eRGO4fuadWg@mail.gmail.comBackpatch-through: 9.6, where the bug was introduced.