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 1wPeNX-000vP0-2l for pgsql-hackers@arkaria.postgresql.org; Wed, 20 May 2026 10:42:19 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.96) (envelope-from ) id 1wPeNV-0079Mj-2d for pgsql-hackers@arkaria.postgresql.org; Wed, 20 May 2026 10:42:18 +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 1wPeNV-0079Mb-1I for pgsql-hackers@lists.postgresql.org; Wed, 20 May 2026 10:42:18 +0000 Received: from mail-pl1-x631.google.com ([2607:f8b0:4864:20::631]) by makus.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256 (Exim 4.98.2) (envelope-from ) id 1wPeNT-00000000TOn-197l for pgsql-hackers@lists.postgresql.org; Wed, 20 May 2026 10:42:17 +0000 Received: by mail-pl1-x631.google.com with SMTP id d9443c01a7336-2bab82d75fdso22398955ad.2 for ; Wed, 20 May 2026 03:42:16 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1779273735; cv=none; d=google.com; s=arc-20240605; b=FPYFdvS+YS5VeI1WNtXODHU7lNII7Xm1ZPxqCQWSJxba8dxq904JdwSif1z/AO+GlI 0ZLeMRJF+5A5DoSJyOZwEPdEAxOLuu+jErVXH7li5yCprDcEmTyFq+K2Bm2MCFbN2OKK ReQw95maFrwtC7yLq1gicU8//ctzVrSbmzB0BHX073LzzjUwgryUnLtZME1DhWg2U0vH 4exE0NdIfEbToSnvN/0ENc7CkLyMrJBMCW+O7bfAI4DuFTGpQB+fQ5hiWceF6vvkmvIn AmXlxvYU3WCHMS3/X/qIfsI777YCUmVQAWP7ctUuUjsfsdYwH3N4/C+lEpfrbOritVrN LKEw== 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=ZNdguDSLvxwhDIqJIuu6QEqCWSFiW1zEtFIjFV7PRhI=; fh=QPcknRCkeNq/4OPFnEc61Jv27lNprMWh7CXX9Y/5QCU=; b=bFmOPg7DTuvhj36iJGMccy3ozrW/JedmwJyEjCP7XjCNJnxWp0Ud45ZPGo7Y8YfuF0 E7C1ZhzxQv6/OgvAXnHZM1PbGf0jQOVcGSYi65ZemtPkf03BbGEtcB5PdIn1X9Gh5vpe ttBD69bW4FOW6/JyecyOY1qt7EGkt3OsC+Ov4TzM3DRdo9PAM0SJC7DVSUIxCxprQA0G dseb/eLMXOCOZolOvkHEK6a/nTHwPWHs3HID0MWxXJinCq0Jm4PBLYenRbeuqrIljfnQ beq62+GbPCYmVHTs8B+E7xfv8oSrBWLhchfFZl04eAy1xkpZvlZLd8VR6e3f35EKbg5Q jNgw==; 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=1779273735; x=1779878535; 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=ZNdguDSLvxwhDIqJIuu6QEqCWSFiW1zEtFIjFV7PRhI=; b=QWvxxi3IdFLCiBu+KS5GXT5wO6NdchSBDs3GL5A5usirt5k6y83cCgSCJtLgdoSeQA PIUgPWQef5ZG5qhyB8dUR/0WMZoNx+13Zqm8nahA4eMPOQqGi6mpbuJJNs5CpIdnfEJy 8uqyfN+QD20C3cHmg+2OS8+AsQ6y8lJn6sAAo3F7D/wezAcMRqdHhhQ/Te2zb2jcU0Mf aYNTaY/jxQuZoA29HxKhFtpgeK8Wv6nAcRlXNHv+8dtyld1p29lZzmMAVMSWEEY0+vae akxXySZAx0UHpz+ww8VYMcqlh51qNjJHk4EaBUlLB1Jk9RO6XsAxBw2Sc8Yx93zEyMz1 Pq7g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1779273735; x=1779878535; 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=ZNdguDSLvxwhDIqJIuu6QEqCWSFiW1zEtFIjFV7PRhI=; b=EHQRlVlynfuQiPT9PeCBJG1mhtPnNGJQMgo/LRW2IwW/GV7EZlRTYoqdSkePs0ts8R 2t16S7mEf1LALv0QtgQcBhJFx+N1SdkP2m1n3yaWk4rSHcXzVn9LmfpRFWL2QsilEQVj IIVw7B3Xb/A9BNreSoRs3Nj4QswWTkdJh0X+XgqK9TdMNZzMofFAue3Y5joPdrrAWw54 cWG4XUWDR6s7Qm24HTKLwmVUbU6ayYf8+D7FdkVYI/i8nmlJE7PKmP8+qbmnVWJ/4IgU IZw53eVL//9Pw302z+4VyLKzfZ04CfHJEs0JgB+cYvjVQc4Tsf9nhPSLHs80dpqsEItC qMBA== X-Forwarded-Encrypted: i=1; AFNElJ/PKqqAs6krPKdK3qeewyMBcufmoajLwMdt7R4dYBgVIN7xZNbkTf1Os3OTIQ067meHE8L4IIWyrtNxmAoB@lists.postgresql.org X-Gm-Message-State: AOJu0YzEHyTcAl5Q9Dc790C27tK2Z5j0xQI3bVf93XzyAJM+xrDvqLcD kEtq0egfDjaRp8DRHBMY+d57p6t6s1BNwPyYKPJhXTZ7PqIJGncn6jma7AEE8DNYYT8RwWtPj/t BSl6h7EhV2Gy/jrR7q0Tlp3TDWOeusf0= X-Gm-Gg: Acq92OGINGpiytFo4uXIT/IIVoBozI5JwtzNSGX+ZBhjWvHTq4q45BsMXD5gaYDV8Ac QI7RTwkSFIhiYg0TC01G/XvpB+dxPGvPpJS17kzsmDlJ2/EMGsOlIdr6nffprvxpOuJbrZxGqFA H87DNJX24JYlZ6RObiKELEBerHuKEZFc65SDZLF+erOCjtTd/bkRHGkIYsLNaoD2fzQ5+YRUTSc phQHAIGaDJFICwx0pweIkwjgfydNrMM5EuQ7OWNWFFfk17m2Z8zX4NG1jV58DRUDV+UKwQ/NS31 4lyN6tuTYc7s6AuYKh0uimDO1sPBlecirAEr/rpzNYUgihDwPtL7 X-Received: by 2002:a17:903:240c:b0:2ba:de0:1eea with SMTP id d9443c01a7336-2bd7e8196dcmr253524815ad.18.1779273734799; Wed, 20 May 2026 03:42:14 -0700 (PDT) MIME-Version: 1.0 References: In-Reply-To: From: shveta malik Date: Wed, 20 May 2026 16:12:02 +0530 X-Gm-Features: AVHnY4Ik3u9hghiH9Z84_Jpqrn-TwelvyyCTIP5nhTMgFRzOZK9M2muwWEQvHLE Message-ID: Subject: Re: Proposal: Conflict log history table for Logical Replication To: vignesh C Cc: Peter Smith , Dilip Kumar , Nisha Moond , Amit Kapila , 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 Wed, May 20, 2026 at 3:05=E2=80=AFPM vignesh C wro= te: > > > 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. > > [1] - https://www.postgresql.org/message-id/CAHut%2BPsrnU2BB1%2BM3c%2BDr5= h62BLYfwBzhTg%3DBM7QtBoPwHYrKw%40mail.gmail.com > [2] - https://www.postgresql.org/message-id/CAJpy0uCX53c40xopqmHtWSWBmh78= BqhLVGXa88fU42eOi6w%2BLQ%40mail.gmail.com > I have not yet looked at v37. But here are a few comments on v36-005, 006. I have merged them and reviewed together. 1) +#include "utils/fmgroids.h" +#include "utils/json.h" conflict.c compiles without above inclusions. 2) + bool log_dest_clt =3D false; + bool log_dest_logfile; A better and more clear name would be log_dest_table instead of log_dest_clt here. 3) @@ -6069,6 +6049,8 @@ DisableSubscriptionAndExit(void) */ pgstat_report_subscription_error(MyLogicalRepWorker->subid); + ProcessPendingConflictLogTuple(); It does not look obvious as in why we are trying to process conflict-tuple during disable-subscription? A comment will help here. 4) tuple_table_slot_to_indextup_json(): + indexDesc =3D index_open(indexid, NoLock); + + build_index_datums_from_slot(estate, localrel, slot, indexDesc, values, + isnull); + tupdesc =3D RelationGetDescr(indexDesc); + + /* Bless the tupdesc so it can be looked up by row_to_json. */ + BlessTupleDesc(tupdesc); We get the index's relcache pointer and pass it directly to BlessTupleDesc which internally changes it by assigning tdtypmod. Is this intentional i.e. do we want to change the relcache entry of index directly? Shouldn't we copy it (CreateTupleDescCopy) and then Bless it? 5) build_conflict_tupledesc() does 'CreateTemplateTupleDesc' and Bless it each time the conflict is raised. Since the tuple-descriptor here is not going to change, IMO, it will be better to create and bless it once and reuse it everytime. We can have a 'static' TupleDesc here. Thoughts? thanks Shveta