Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.96) (envelope-from ) id 1wQMrw-001V34-1P for pgsql-hackers@arkaria.postgresql.org; Fri, 22 May 2026 10:12:40 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.96) (envelope-from ) id 1wQMru-00CtlP-0U for pgsql-hackers@arkaria.postgresql.org; Fri, 22 May 2026 10:12:39 +0000 Received: from makus.postgresql.org ([2001:4800:3e1:1::229]) by malur.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.96) (envelope-from ) id 1wQMrt-00CtlG-2P for pgsql-hackers@lists.postgresql.org; Fri, 22 May 2026 10:12:38 +0000 Received: from mail-lj1-x229.google.com ([2a00:1450:4864:20::229]) by makus.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256 (Exim 4.98.2) (envelope-from ) id 1wQMrs-00000000FmU-2cQ8 for pgsql-hackers@lists.postgresql.org; Fri, 22 May 2026 10:12:37 +0000 Received: by mail-lj1-x229.google.com with SMTP id 38308e7fff4ca-39397d63804so20046661fa.2 for ; Fri, 22 May 2026 03:12:36 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1779444754; cv=none; d=google.com; s=arc-20240605; b=NYjsxGdN/ksEjK6TLNriBvepmOsEw8jWFa6DH2NdCRzaFuRHivZd+bCIaIk41dJqIf YsUloz+/f+PPOf1RNwIUxyfDXlM3lKX6sbPgydK5Vi/i6AlwxWtdqnAsOI1UGKBmuye4 u3IUU44E00eDeZrmo/BFdaH3lcPA/Zgby4bpl6qjkeV7QHOvS/xwMrP1u2mb6buPGH6G BBCpsM1LO5WTeBOLSQflMk7325IGdyWZM/GXuEmz28/heXnv32PY2raordHzSiP951TA SiTzdbJNxgfSIEltRkffbJMRsJfIToVKC1g9PyH9g3EAwO6RoXHqZJbXTlXd040nU3Jl y/pQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20240605; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:dkim-signature; bh=WxksWEXIIgZsamwRTTcO2mdZoyXzdXd6Gwtb9uTcNfE=; fh=Thpp/qBCGzfScZwEyQ7aoj8HYY1DjU1N9hzlMnN97tA=; b=Bb+ZGWsO5Lu1eIIw2eRI4UZ24ULb8O9cJBn12So4E2VFP5K/5+sndo26oajXSHSaDi PmQKykuCdqS6MQO9a5qd+HN03G5Rspe6NROZoEi1Mp54SglY91Q698AUgFv/ov6m2fGv 1gc0gFSW2QUy8X7KLq0Uw+VDB+/PN/1RWi1MioYbJo4mgxClerM/kNTAcd5sIz+g8ulS LaCpp+wEbl4gz5BEARBhJNN4DuDV73H6oyQLvHrhJDbzPU/9VIysHXPsZrGmZsAKKBPD L1Kt7Z0SDYDVwINoRAVCWODFbYExCoP8azvCwgmCyBhUVkiVmlArG2UlTPlzD8oMTkFq 1MSg==; darn=lists.postgresql.org ARC-Authentication-Results: i=1; mx.google.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20251104; t=1779444754; x=1780049554; darn=lists.postgresql.org; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=WxksWEXIIgZsamwRTTcO2mdZoyXzdXd6Gwtb9uTcNfE=; b=hCPJoM7Wi1w+BGqA5XOSdasZFU0rne0ilMJfFwvW8wSe7TtETVhJAUV0Uh8DufOqvS yHEYkAH65payhgn857hM2DA2v2xt27OdzSwlpx3GAZadk7v1wCUkjpYhjVn7wCtgOl1M 7ebsd1BybozUIsC/Q1LzOLmQNdxiPRrfc30/n3lwg0mIwwJDzFvIaWJjqCcVrXx9aAjA s3DqAfRcMTXDizPZxjbyB2EoSxHDbQOPLPwThv487d7HCEpUw7U9+j8uIxRm/lZJP0Ru t0UsGXpYKGaHl8sdf0QxhrJyPXMMn1PWUlDrPrPScnySoHyXaGqjSuLwBSoFOBR21CDo L2ew== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1779444754; x=1780049554; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-gg:x-gm-message-state:from :to:cc:subject:date:message-id:reply-to; bh=WxksWEXIIgZsamwRTTcO2mdZoyXzdXd6Gwtb9uTcNfE=; b=gYqDHbSiBxMzG0PH6oXD8SMxS8mZkq/6uVWK3mlfd5SZVR6hYcloy9R4IkklpKUfFS IaDTuQfK3n5NM7GzLSf+cAcz/UCKdBwxa11XGBiFquNuxzWQzo394A39VY2arlM/OJis PpyfWSGTg9NCxvxxwn1RQ9EMRWzPP/Iz8hR9jzQg5aOPvLrLtt/kGDRXBB6BWq7H1yIi spyNpJnWS0vW14OwTYD2NnM2Q8qmZ/jzKQBFaDgzkjnCl3p16YiU28eiEf9HvhzEy9O7 pzf/SjKfq2/aBCIYLHE7AgFMoK9hsg9Q3Uv06q1ZIBFgND/Q9eUjhZ7uL793PHwPPKTz Mzvw== X-Forwarded-Encrypted: i=1; AFNElJ820/toYhbXBH+ZllBdY8IjrFSybqv2+CksMWIqNMTL5mtnN9Mr2ObNnvRZ1mOJFYHa3NMRHPVty8O//Nft@lists.postgresql.org X-Gm-Message-State: AOJu0YwNjE4vJOZX9ggaULA9p7VEQS79qKt858DGaaF9gwfMdxjenfd7 rsgPs64d4Nv2OxTVp88naN0rrbEU8rGFDWIfFHPT5I8l5Nrn2wxL+DWD3o45myHkTuF1k8gRz9I wSFQZCYpR8PK9aWhyamJmuvYr0c4OBA== X-Gm-Gg: Acq92OHDIUJP1Ndm4JUk9ZlI96mP0RdhXxLwvWwAvUa5Tk++FKkNlGOMQ4DmAP0/r45 G9CVLoFB3UWfIjdeAQrv4FUXvoLyI0Uyva3nMdYPuSi2FQEb8LLo02tO4c0lNAkz3afPU3bEBli C7XKxuvVcA7fHo6M7FqtNmQaXgP6ZhtKfeBP69uYDJlU5PtFj55xnHEef1/V/LbI8Bo5adYHkj2 hfsK5TvwEMsSa54OnHuKoht3rj+Sx/H3QhRM0fj0ZFHTC1nl/E3w1YiyYILc3lcOMoqIm5S+kDN KWbgR+Sp7THKC+GntpPgkMPT X-Received: by 2002:a05:651c:1989:b0:394:898:a351 with SMTP id 38308e7fff4ca-395d8cc2598mr9227601fa.13.1779444753253; Fri, 22 May 2026 03:12:33 -0700 (PDT) MIME-Version: 1.0 References: In-Reply-To: From: Nisha Moond Date: Fri, 22 May 2026 15:42:20 +0530 X-Gm-Features: AVHnY4J5S0UzTM7-ZmGTyFb1BXFmsNLoXckTHqC902kRFGwASzjGroBc2WXNTF0 Message-ID: Subject: Re: Proposal: Conflict log history table for Logical Replication To: vignesh C Cc: Peter Smith , Dilip Kumar , Amit Kapila , shveta malik , Masahiko Sawada , Bharath Rupireddy , PostgreSQL Hackers , shveta malik Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Archived-At: Precedence: bulk On Fri, May 22, 2026 at 10:21=E2=80=AFAM Nisha Moond wrote: > > On Wed, May 20, 2026 at 3:05=E2=80=AFPM vignesh C w= rote: > > > > Rest of the comments were fixed. > > The attached v37 version patch has the changes for the same. Also > > Peter's comments on the documentation patch from [1] and Shveta's > > comments from [2] are addressed in the attached patch. > > > > Here are few comments based on v37 testing: > Here are few more review comments - 1) Patch-0001 + 0002: In subscription.sql: -- Verify the table OID for reap check SELECT 'pg_conflict_log_for_subid_' || oid AS internal_tablename FROM pg_subscription WHERE subname =3D 'regress_conflict_test1' \gset SET client_min_messages =3D WARNING; DROP SUBSCRIPTION regress_conflict_test1; -- should return NULL, meaning the conflict log table was reaped via depen= dency SELECT to_regclass(:'internal_tablename'); Here, internal_tablename becomes "pg_conflict_log_*" without the pg_conflict. schema prefix. So, "SELECT to_regclass(:'internal_tablename');" will always return NULL even if the table still exists in the pg_conflict schema, which skips the actual drop verification scenario. Should we instead use: "SELECT 'pg_conflict.pg_conflict_log_' || oid AS internal_tablename..." ~~~ For Patch-0007: 2) @@ -2067,9 +2095,31 @@ selectDumpableNamespace(NamespaceInfo *nsinfo, Archive *fout) static void selectDumpableTable(TableInfo *tbinfo, Archive *fout) .... + if (strcmp(tbinfo->dobj.namespace->dobj.name, "pg_conflict") =3D=3D 0) ... + * Dump pg_conflict tables only during binary upgrade. The schema + * is assumed to already exist. + */ + tbinfo->dobj.dump =3D DUMP_COMPONENT_DEFINITION; .... + else + tbinfo->dobj.dump =3D DUMP_COMPONENT_NONE; + } + For conflict log tables during binary upgrade, we set: tbinfo->dobj.dump =3D DUMP_COMPONENT_DEFINITION; but then execution falls through to the later logic: ... else tbinfo->dobj.dump =3D tbinfo->dobj.namespace->dobj.dump_contains; which seems to overwrite the earlier 'dobj.dump' value. So it looks like DUMP_COMPONENT_DEFINITION may never actually survive here. Should we return from this block instead of continuing further? 3) @@ -5656,6 +5757,11 @@ dumpSubscription(Archive *fout, const SubscriptionInfo *subinfo) appendPQExpBufferStr(query, ");\n"); + appendPQExpBuffer(query, + "\n\nALTER SUBSCRIPTION %s SET (conflict_log_destination =3D %s);\n", + qsubname, + subinfo->subconflictlogdest); + The above ALTER SUBSCRIPTION command seems to be dumped unconditionally for every subscription. Since the default value during subscription creation is already "subconflictlogdest =3D 'log' ", should we emit this command only when subconflictlogdest is non-NULL and not 'log'? 4) + if (PQgetisnull(res, i, i_sublogdestination)) + subinfo[i].subconflictlogdest =3D NULL; + else + subinfo[i].subconflictlogdest =3D + pg_strdup(PQgetvalue(res, i, i_sublogdestination)); + + if (PQgetisnull(res, i, i_sublogdestination)) + subinfo[i].subconflictlogdest =3D NULL; + else + subinfo[i].subconflictlogdest =3D + pg_strdup(PQgetvalue(res, i, i_sublogdestination)); + /* Decide whether we want to dump it */ Looks like the same if-else block is repeated twice here. -- Thanks, Nisha