On Tue, Jan 20, 2026 at 12:08 PM shveta malik <[email protected]> wrote: > > On Wed, Jan 14, 2026 at 11:24 PM Masahiko Sawada <[email protected]> > wrote: > > > > I've attached the updated patch. > > > > Thank You for the patch. I like the idea of optimization. Few initial > comments: > > 1) > + * The query returns the slot names and their caught-up status in > + * the same order as the results collected by > + * get_old_cluster_logical_slot_infos(). If this query is changed, > > I could not find the function get_old_cluster_logical_slot_infos(), do > you mean get_old_cluster_logical_slot_infos_query()? > > 2) > " WHERE database = current_database() AND " > " slot_type = 'logical' AND " > > Is there a reason why database = current_database() is placed before > slot_type = 'logical'? I am not sure how the PostgreSQL optimizer and > executor will order these predicates, but from the first look, > slot_type = 'logical' appears cheaper and could be placed first, > consistent with the ordering used at other places. > > 3) > Shouldn’t we add a sanity check inside > get_old_cluster_logical_slot_infos_query() to ensure that when > skip_caught_up_check is true, we are on PostgreSQL 18 or lower? This > would make the function safer for future use if it's called elsewhere. > I understand the caller already performs a similar check, but I think > it's more appropriate here since we call > binary_upgrade_logical_slot_has_caught_up() from inside, which doesn’t > even exist on newer versions. >
A correction, the case I stated here is when 'skip_caught_up_check' is false. > 4) > +# Check the file content. While both test_slot1 and test_slot2 should > be reporting > +# that they have unconsumed WAL records, test_slot3 should not be reported as > +# it has caught up. > > Can you please elaborate the reason behind test_slot3 not being > reported? Also mention in the comment if possible. > > thanks > Shveta
