Re: logical changeset generation v6.8

On 2013-12-03 15:19:26 -0500, Robert Haas wrote:
> Yeah, you're right.  I think the current logic will terminate when all
> flags are set to false or all attribute numbers have been checked, but
> it doesn't know that if HOT's been disproven then we needn't consider
> further HOT columns.  I think the way to fix that is to tweak this
> part:
>
> +               if (next_hot_attnum > FirstLowInvalidHeapAttributeNumber)
>                         check_now = next_hot_attnum;
> +               else if (next_key_attnum > FirstLowInvalidHeapAttributeNumber)
> +                       check_now = next_key_attnum;
> +               else if (next_id_attnum > FirstLowInvalidHeapAttributeNumber)
> +                       check_now = next_id_attnum;
>                 else
> +                       break;
>
> What I think we ought to do there is change each of those criteria to
> say if (hot_result && next_hot_attnum >
> FirstLowInvalidHeapAttributeNumber) and similarly for the other two.
> That way we consider each set a valid source of attribute numbers only
> until the result flag for that set flips false.

That seems to work well, yes.

Updated & rebased series attached.

* Rebased since the former patch 01 has been applied
* Lots of smaller changes in the wal_level=logical patch
  * Use Robert's version of wal_level=logical, with the above fixes
  * Use only macros for RelationIsAccessibleInLogicalDecoding/LogicallyLogged
  * Moved a mit more logic into ExtractReplicaIdentity
  * some comment copy-editing
  * Bug noted by Euler fixed, testcase added
* Some copy editing in later patches, nothing significant.

I've primarily sent this, because I don't know of further required
changes in 0001-0003. I am trying reviewing the other patches in detail
atm.

Greetings,

Andres Freund

--
 Andres Freund                       http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services