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 1wfWXG-005Uw0-1N for pgsql-hackers@arkaria.postgresql.org; Fri, 03 Jul 2026 05:33:58 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.96) (envelope-from ) id 1wfWXF-005XJ4-14 for pgsql-hackers@arkaria.postgresql.org; Fri, 03 Jul 2026 05:33:57 +0000 Received: from magus.postgresql.org ([2a02:c0:301:0:ffff::29]) by malur.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.96) (envelope-from ) id 1wfWXE-005XIG-3A for pgsql-hackers@lists.postgresql.org; Fri, 03 Jul 2026 05:33:57 +0000 Received: from mail-qt1-x829.google.com ([2607:f8b0:4864:20::829]) by magus.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256 (Exim 4.98.2) (envelope-from ) id 1wfWXC-00000001Qfo-3WQs for pgsql-hackers@lists.postgresql.org; Fri, 03 Jul 2026 05:33:56 +0000 Received: by mail-qt1-x829.google.com with SMTP id d75a77b69052e-51c0006ea8eso1095731cf.1 for ; Thu, 02 Jul 2026 22:33:54 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1783056833; cv=none; d=google.com; s=arc-20260327; b=KG8hIZfNVLShJFQQ1cvcCZD4s4MfvX1X6+k1gtTvkXfDLWFcroZD5/W6QcIOsRP9fM SQuKe0bg1oTrYl8HiVlQW12TajhVE/9zeibokLoWtx748Qceaeq2lm2tfI5peNHJbpBR zPTvWKjB1hXCWGZPqzceUXZbF358ieCgKaU9ILJZa1ef5AKTpo0iIZmt3XN8BpSjn5jn YDiernCC0Bc6DD4Pj1P2dpsSEi2EJ/VdNhfkLMoi9HWya0XRstWNQ5Y928ieVWVtNNdx 1e/BwiR+IBo3bEuXIVJNAilT6soESKqqTi0FGqPL/C7ocIA9U8JoC4MxXkm8Wc+xUTmY 9PRQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20260327; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:dkim-signature; bh=ZhO2UXap/gldcmAOHnTj6kglpUWoLo3CIoGFAlyPUlw=; fh=mpL5U3YDZZCr8AMGEDXz6IblQu8P+HRLjE61eoicPyw=; b=ZCnuh3UEgk8JxG0hCWzCISM2/Xwl2Qzxgq8BP2q8RIagPS3UQfAF78uLdVZhwcr3Ty 9gGZlpkCABdZzAxvWUpKRV5g4JvbKdlXFqNx30cMyo1TCublNH3R5diplRaiQNs1Qgma jMG4fZdtvAJbq+l0ztylehlobNbqp89Y2RVfbbd/FtjaTB2ImN1RdQ91nCyoolYKEDQ5 ickbmnEs8nv4JzM2F2xPvQjNcNJqI4ANlvBsrx6DKZ0UjxsV1SKvd1JUooin5hBO9S5D hOIAutNwpgWqVixcZuhTgWoFiemzW0q918gW9B0v4De/4aiziPNK6OxRBwuk/xc+8MQd HyfA==; 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=1783056833; x=1783661633; darn=lists.postgresql.org; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=ZhO2UXap/gldcmAOHnTj6kglpUWoLo3CIoGFAlyPUlw=; b=WKPywXg7BvgyvHSrH/iSriO5SQ24WBIKT//WM+vmQKgQG5kAMRbEUQY/0fXwmRLtLJ SuNa6uxJxn0xABC2RGxOJyN9i+n0dYKiBkf1TB59Z7paXL5Zxyoc/dr4Cm2hgwG7mEvg bUBmawDaxKqPOjkDdXZPXYhMjMjvGsOqkD5NIexnMftmDhkWG+30L/C4eYxRKFX6Rvfc t9kAKWOCeRY32cFUB7DD8NH5smLkZiPGPv53b2JduvP++73rzi2R9FYPHU2FNcqHU9Sp UkLHinB4dQS1JhxZBSfdHf9CmR0Kc994LC/qrnyfWrAbxkDrjnul5saOWa8ual43AtXV rQRg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1783056833; x=1783661633; h=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=ZhO2UXap/gldcmAOHnTj6kglpUWoLo3CIoGFAlyPUlw=; b=eh0tJZp0YJn416Rw4O3gs95omovEYDSdiwZQSWgxxqwWK4pil1SQ2C0XDJe3rl4KGD 7c5yhXBKoFqVpdc9jPysZGVLENnL9fw63c7KfJ1USOzF5fdXy/Eis2HbU9j+YDUbRwoO raLap0iCJKY/1E9fUB6r8Puwye3Bs9SQwwgKZ3KIfgBAZ5rwQogXsYB9AwEr9mSk56Sw TxVA7c04+20ycL98vfZp/jOYG5mhX/Nwi0kfAXNj50oKJTxVuLDA1Ystggvz7HJxi8be AGI6sWhVM1lbWL/h9fQCcYqc70yU8YMi6ERqIiWNNmleBY/E+wEV2vih5O29YwZBh3c4 3WIA== X-Forwarded-Encrypted: i=1; AFNElJ/KvKLkc8Wv/G66ap8U/lkls+aRE/6qTI+ixwCeX19A9aiBpAAbn/owFgKLC1GFBlYbkav2UMR0WNcT0jVs@lists.postgresql.org X-Gm-Message-State: AOJu0Yy3TaqZ1vaJalF3CzBMyhbXscpyQLuj3rOLzg+uiTaHgzKJMRPc mE2jFmjGItO2QM6MUa9hvHVyYbAcCwrC4L8HKUFqICQpwHVQiMaNILim/xioDgJ4+A99SOSIA7v 9pFbC1W60jJPnYl/IzJ86BqShIfcEKOw= X-Gm-Gg: AfdE7cl8DqukHZXamyBoRYKzj7C++MrEuZL1jDAXTC+nxOCQADrlM/rfiVtON7lFF8F kpWgTXe9XfJQlXXBM5swVSySIxWl6S+2fmocLgXCWE20FUkjozwd8vp+siTeIsYArmGkWxsH8YD S+BHirZFZXRjaAIF90MDQUrQlAREq0vOmRhtUieTddwzneTk4WrhXz2YzP1qLeCjoQOzR1+eZS+ uPJK5x2T1NgzLsyUw7as6mItsswt14ET4LYdixtZr3nQpCrGeBM1Wtha/tgAApIzemCHE/WzDxs QXTT13BpSoyIn2G21hpMUTcTTDwIYrGhuGD0V3w19wICOfcH3dgRbDoGBOmKKANz8Q== X-Received: by 2002:ac8:5981:0:b0:51b:f98f:ea5e with SMTP id d75a77b69052e-51c26a63870mr116142631cf.22.1783056832718; Thu, 02 Jul 2026 22:33:52 -0700 (PDT) MIME-Version: 1.0 References: From: Peter Smith Date: Fri, 3 Jul 2026 17:33:26 +1200 X-Gm-Features: AVVi8CcydwETr3BYbBxxYDLYZILahWcvclBtshgNzw6AR2gV4AcSaAu1UIsmjy8 Message-ID: Subject: Re: Proposal: Conflict log history table for Logical Replication To: Dilip Kumar Cc: shveta malik , Amit Kapila , vignesh C , Nisha Moond , Masahiko Sawada , Bharath Rupireddy , PostgreSQL Hackers Content-Type: text/plain; charset="UTF-8" List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Archived-At: Precedence: bulk Some review comments for v62-0001. I was midway through reviewing patch v62-0001 when I saw it had already been pushed [1]. I am posting my review comments here anyway. ====== doc/src/sgml/ddl.sgml 1. + + The pg_conflict schema (sometimes referred to as the + conflict schema) contains system-managed conflict + log tables used for logical replication conflict tracking. + These tables are created and maintained by the system and are not intended for + direct user manipulation. Unlike pg_catalog, the + pg_conflict schema is not implicitly included in the + search path, so objects within it must be referenced explicitly or by + adjusting the search path. + 1a. Why say "sometimes"? ~ 1b. I thought the markup should be instead of . ~ 1c. We just said this is referred to as the "conflict schema" so let's do that! /the pg_conflict schema is not/the conflict schema is not/ ====== doc/src/sgml/ref/create_subscription.sgml 2. + + The system automatically creates a structured table named + pg_conflict_log_<subid> in the + pg_conflict schema. This allows for easy + querying and analysis of conflicts. + Don't say "pg_conflict schema". Instead, use the term "conflict schema" with the appropriate . ====== doc/src/sgml/ref/drop_subscription.sgml 3. + + If a conflict log table exists for the subscription (that is, when + + conflict_log_destination is set to table + or all), DROP SUBSCRIPTION automatically drops + the associated conflict log table. + This seems a bit awkwardly worded: SUGGESTION When conflict_log_destination is set to table or all, DROP SUBSCRIPTION automatically drops the associated conflict log table. ====== GENERAL 4. policy.c: + /* + * Conflict log tables are used internally for logical replication + * conflict logging and should not be modified directly, as it could + * disrupt conflict logging. + */ tablecmds.c: + /* + * Conflict log tables are used internally for logical replication + * conflict logging and should not be modified directly, as it could + * disrupt conflict logging. + */ + /* + * Conflict log tables are used internally for logical replication + * conflict logging and should not be altered directly, as it could + * disrupt conflict logging. Direct ALTER commands are already rejected + /* + * Conflict log tables are used internally for logical replication + * conflict logging and should not be referenced by foreign keys, as it + * could disrupt conflict logging. + */ + /* + * Conflict log tables are used internally for logical replication + * conflict logging and should not be modified directly, as it could + * disrupt conflict logging. + */ + /* + * Conflict log tables are used internally for logical replication + * conflict logging and should not be altered directly, as it could + * disrupt conflict logging. + */ trigger.c: + /* + * Conflict log tables are used internally for logical replication + * conflict logging and should not have triggers, as it could disrupt + * conflict logging. + */ + /* + * Conflict log tables are used internally for logical replication + * conflict logging and should not have triggers, as it could disrupt + * conflict logging. + */ rewriteDefine.c: + /* + * Conflict log tables are used internally for logical replication + * conflict logging and should not have rules, as it could disrupt + * conflict logging. + */ + /* + * Conflict log tables are used internally for logical replication + * conflict logging and should not have rules, as it could disrupt + * conflict logging. + */ ~~~ The wording of the comments above is extremely repetitive. That may not be noteworthy if the comment was just in one place, but it's everywhere. How about like: BEFORE Conflict log tables are used internally for logical replication conflict logging and should not have , as it could disrupt conflict logging. SUGGESTION Conflict log tables are internal to logical replication and must not have , since it could interfere with their operation. ====== src/include/catalog/pg_subscription.h 5. + /* + * Strategy for logging replication conflicts: 'log' - server log only, + * 'table' - conflict log table only, 'all' - both log and table. + */ + text subconflictlogdest BKI_FORCE_NOT_NULL; IIUC, this comment was deliberately aligned for readability for ~50 versions. But then pg_indent wrapped everything. Consider putting it back how it was and using "/**" style comment so that pg_indent will leave it alone. ====== [1] https://github.com/postgres/postgres/commit/a5918fddf10d297c70f7ec9067e9177e0be6d520 Kind Regards, Peter Smith. Fujitsu Australia