Re: pg_upgrade: optimize replication slot caught-up check

On Wed, Jan 14, 2026 at 11:24 PM Masahiko Sawada <sawada.mshk@gmail.com> 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.

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