Hi,

On 4/7/23 4:18 AM, Andres Freund wrote:
Hi,

TBH, I don't like the state of 0001 much. I'm working on polishing it now.


Thanks Andres!

A lot of the new functions in slot.h don't seem right to me:
- ObsoleteSlotIsInvalid() - isn't an obsolete slot by definition invalid?

bad naming, agree.

- Why does ObsoleteSlotIsInvalid() sometime check invalidated_at and sometimes
   not?

because part of the existing code was doing so (checking if s->data.restart_lsn 
is valid
with/without checking if data.invalidated_at is valid) and I thought it was 
better not
to change it.

- TransactionIdIsValidPrecedesOrEquals() shouldn't be defined in slot.h -
   also, it's not actually clear what semantics it's trying to have.

Oh right, my bad for the location.

- there's no commonality in naming between the functions used to test if a
   slot needs to be invalidated (SlotIsFreshEnough() vs
   LogicalSlotIsNotConflicting()).

Agree, my bad.

Regards,

--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com


Reply via email to