Hi, On Fri, Jan 19, 2024 at 11:46:51AM +0530, shveta malik wrote: > On Fri, Jan 19, 2024 at 11:23 AM Amit Kapila <amit.kapil...@gmail.com> wrote: > > > > > > 5 === (coming from v62-0002) > > > > + Assert(tuplestore_tuple_count(res->tuplestore) == 1); > > > > > > > > Is it even possible for the related query to not return only one row? > > > > (I think the > > > > "count" ensures it). > > > > > > I think you are right. This assertion was added sometime back on the > > > basis of feedback on hackers. Let me review that again. I can consider > > > this comment in the next version. > > > > > > > OTOH, can't we keep the assert as it is but remove "= 1" from > > "count(*) = 1" in the query. There shouldn't be more than one slot > > with same name on the primary. Or, am I missing something? > > There will be 1 record max and 0 record if the primary_slot_name is > invalid.
I think we'd have exactly one record in all the cases (due to the count): postgres=# SELECT pg_is_in_recovery(), count(*) FROM pg_replication_slots WHERE 1 = 2; pg_is_in_recovery | count -------------------+------- f | 0 (1 row) postgres=# SELECT pg_is_in_recovery(), count(*) FROM pg_replication_slots WHERE 1 = 1; pg_is_in_recovery | count -------------------+------- f | 1 (1 row) > Keeping 'count(*)=1' gives the benefit that it will straight > away give us true/false indicating if we are good or not wrt > primary_slot_name. I feel Assert can be removed and we can simply > have: > > if (!tuplestore_gettupleslot(res->tuplestore, true, false, tupslot)) > elog(ERROR, "failed to fetch primary_slot_name tuple"); > I'd also vote for keeping it as it is and remove the Assert. Regards, -- Bertrand Drouvot PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
