On Fri, Jan 30, 2026 at 2:15 AM Masahiko Sawada <[email protected]> wrote: > > On Wed, Jan 28, 2026 at 10:06 PM Amit Kapila <[email protected]> wrote: > > > > On Wed, Jan 28, 2026 at 2:06 AM Masahiko Sawada <[email protected]> > > wrote: > > > > > > I missed fixing one place. Attached the new version. > > > > > > > One question/comment on following change: > > + bool use_fast_caught_up_check; > > + > > + logical_slot_infos_query = > > get_old_cluster_logical_slot_infos_query(cluster, > > + &use_fast_caught_up_check); > > + > > upgrade_task_add_step(task, > > logical_slot_infos_query, > > process_old_cluster_logical_slot_infos, > > true, NULL); > > + > > + /* > > + * Check whether slots have consumed all WAL records efficiently by > > + * using another query, if not during a live_check. > > + */ > > + if (use_fast_caught_up_check && !user_opts.live_check) > > + { > > > > Won't this lead to two steps to set caught_up for slots in PG19 and > > following versions? If so, is it possible to use just one step even > > for PG19 and following versions? > > Yes, it seems like a good simplification. I've updated the patch accordingly. >
At first glance it looks like a simplification, but on closer look, it actually makes the code harder to follow and more prone to errors if someone modifies it in the future. The check_caught_up CTE is appended only when both conditions are met: + if (use_fast_caught_up_check && !user_opts.live_check) + appendPQExpBuffer(&query, + "WITH check_caught_up AS ( " But the reference to check_caught_up in the FROM clause is appended when only one condition is met: + appendPQExpBuffer(&query, + "invalidation_reason IS NOT NULL as invalid " + "FROM pg_catalog.pg_replication_slots %s" + "WHERE slot_type = 'logical' AND " + "database = current_database() AND " + "temporary IS FALSE " + "ORDER BY 1;", + use_fast_caught_up_check ? ", check_caught_up " : ""); IIUC, if use_fast_caught_up_check is true while user_opts.live_check is also true, the query will reference check_caught_up without defining it, resulting in an incorrectly constructed query. Or am I missing something here? thanks Shveta
