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 1wTexu-000ffK-19 for pgsql-hackers@arkaria.postgresql.org; Sun, 31 May 2026 12:08:26 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.96) (envelope-from ) id 1wTexr-007WkG-29 for pgsql-hackers@arkaria.postgresql.org; Sun, 31 May 2026 12:08:24 +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 1wTexr-007Wk7-0z for pgsql-hackers@lists.postgresql.org; Sun, 31 May 2026 12:08:23 +0000 Received: from mail-yx1-xb134.google.com ([2607:f8b0:4864:20::b134]) by magus.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256 (Exim 4.98.2) (envelope-from ) id 1wTexo-00000000SlR-3yCd for pgsql-hackers@lists.postgresql.org; Sun, 31 May 2026 12:08:23 +0000 Received: by mail-yx1-xb134.google.com with SMTP id 956f58d0204a3-660512d80b4so2036990d50.1 for ; Sun, 31 May 2026 05:08:20 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1780229299; cv=none; d=google.com; s=arc-20240605; b=AuWfslP6N/bITzUfT7jC34RwSLfnqzqc4Aa1gB7y0qyYCBAlIJ2yRH2z5qN9Gvfre/ p6PEYMFjK1RHOGIZWLnsijDRuqWQT6Ofg2IIiiOQkvY67MCPo9YShWqdGUBv0hOfGOIy Jzqlgy7MRg9lyJnxpSL0EqpOAKQd29DGo4MNJ6SZzsEgW6r67+6W7589NIuZKlo/4a4Y c0f+mKRCmU8FFsPjR1aoCeJCp/C62sncrhzz+BtQCl0pj3UqL9b2KT7zeudMC3PsPEJz taNuRueLxw2aNJEbbyVWxuNCPUOPnAKDOhXHhHqG0DpVxlqBgxXetAK9H+SGSGAae52X 2ndA== 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=wG8K8d/OPIMht+1ChHb9fQBJdKoP8QrMvonYqYD3AEs=; fh=m8ExQfDYW5lbMd8v52wBF17byr1H3ZAU0zcj4p+oj84=; b=la0puaq2ay6vp0zC6H7PuXdt36oEl0QrMiSICcgF1ISsFdznuPL5Zkt7ztcBc357CW TjzdXA6gMy/zP896cqTNpCXyzCQAxdqjhhsEDA5Ck6GbtFVLTfGeW2r8EZdEPBi9IXNq PH3eIilM+s13FTGYdOEXZqYr95BgSqzsTiF0mF9b8TaJ1eZ0UcV9s4n0nVcvplq/y55c CSYD6l3rDY2q7hjZB370YeF4Aq44mFp+hyMfsWZ1QiCEZ+e50rEyI2u162FklpeMfc9t JQnzusiTYPQyjSty3UjNu0n6hl32BkfdwyP5h+HSsWKXh38KhzptwbSqs5RoWfYkNpZw sH3g==; 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=1780229299; x=1780834099; 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=wG8K8d/OPIMht+1ChHb9fQBJdKoP8QrMvonYqYD3AEs=; b=P7ovBYxyZADfLLn/DIJMNBIgm1YV+YFbyPyxDzU9FGvXlZAgfXeChB2qeJXL1kw+/Q mvg7JrtKqp40zudHHzmwp/Jbt2Di7zfrT55TTqJFIJZCI45dfysXT83C3V/L+0o82aRN r3xcep/XyAWCUfHBGYkfC+MmvgnzuaMRnogO+X2fP5LVOihDMcqy1CyyLWVnKDqzpGct 3+nYJLLHnR6xwz+fYJT/CX0Z36XxDRmYC+Rt8/Z6m+JzfM9zN4Uap4RHn30gfrn9PDrw h0yLswX8kwbazvqBf9SFuBiMmYVwRZ+TMSDRkkyx8qmGRD3LllnY95ERHIk+TD2KP8sU zu4w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1780229299; x=1780834099; 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=wG8K8d/OPIMht+1ChHb9fQBJdKoP8QrMvonYqYD3AEs=; b=Xb39FB3IavfwMOpsTCjaFi6XpTAWqqraIR4+A345aa08TdOO69XoEZyWwcl8VJux0E IvZ/TN8p0OpyaXQFKhIbIp/KZl7AX0ADcLBPGDMtECDUy6iXUkfvWu2aZMt9H6dONzp3 xw1vCu26jXoOh74hFOjmr3zj1iHsvWmxgkrAgkELxT8w2lcyTIeECGLaPvvn1Git5yRs zQ/RgysGiPWIafIIsCvcZIoTdsXDmdU2lC4QXLk7TATN92nkx02yAF7/Pr3kNEYQ3Arz p4YHDRVHQuJT4uw+q3bmlCA8DwYmIDiBpdNRi6ViWK9LL6B/YZJHKMDVK4wG5vCUm2A+ 97Ag== X-Forwarded-Encrypted: i=1; AFNElJ/1K6hHp9Fw9UX+za7HigsINNWgplBNFlyiBBLgX0BrLHZCFCVi4WzOdT4Nbd0P+l20oTpPlDFCQ6hBLkZ0@lists.postgresql.org X-Gm-Message-State: AOJu0Yw3eheVMxnGyHleeL8viL1+EGEeoQKRTsa5byu7jtr3eZEP+fI+ /KnNg72/58WcwN6SxmTZd9Vmy/tq1B+5uHLO2gMcPOw+nT00H/jaJuVZQUtheRPLohckRFmVcYb A+iXO4X0O74p70UaVYMwsILTaeOXLyBs= X-Gm-Gg: Acq92OHGFOMvNfPQK/i7Gy1/anfe9ea7WLeCovc2CK6guTfhhbQdFqMAas+VkceQQBe lVOwZJzRS4SKOCqmpKbmrVG46dr84ZgRBcA6aaSsTwlS0YrfhU2odMdzu0qGwL5xfXkyw6Fvh9o wEPuzsNiNTbC2V/DPWOQQX44sidNCCRaHrcPzdV54vCMtIDCiqeOsxyvZJZbTToxy1XSJ5vPjwR HKWH+izWQ8SxWHRcRDH1MDVECXXyQfe31aesoSXkHklnjQp9HZqP0GDFkOXXkOBtI4oRhJI49GR PNmC0f26dGpUFFS3V08O X-Received: by 2002:a05:690e:480b:b0:65e:15fb:240e with SMTP id 956f58d0204a3-6605edd0065mr2732700d50.12.1780229299233; Sun, 31 May 2026 05:08:19 -0700 (PDT) MIME-Version: 1.0 References: In-Reply-To: From: vignesh C Date: Sun, 31 May 2026 17:38:05 +0530 X-Gm-Features: AVHnY4LV56B4fPvFWOdVZdlqg8yuplSJETJwGcMhROwfEXTADR7-r5DdWWNb6tE Message-ID: Subject: Re: Proposal: Conflict log history table for Logical Replication To: Dilip Kumar Cc: Amit Kapila , shveta malik , Nisha Moond , Peter Smith , 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 Sun, 31 May 2026 at 11:43, Dilip Kumar wrote: > > On Thu, May 28, 2026 at 2:57=E2=80=AFAM Amit Kapila wrote: > > > > On Wed, May 27, 2026 at 1:34=E2=80=AFAM vignesh C = wrote: > > > > > > On Tue, 26 May 2026 at 15:08, shveta malik w= rote: > > > > > > > > On Mon, May 25, 2026 at 10:13=E2=80=AFAM vignesh C wrote: > > > > > > > > > > > > > > > Thanks for the comments, the attached v39 version patch has the > > > > > changes for the same. > > > > > > > > > > > > > I have not yet looked at v40, but please find a few ocmments on > > > > v39-0001 and 0002 merged together. > > > > 4) > > > > Do we need to have CommandCounterIncrement() after > > > > heap_create_with_catalog() in create_conflict_log_table()? I think > > > > even if we are not doing any table_open etc for CLT in same > > > > transaction, we should call CommandCounterIncrement() (to be > > > > consistent with other such calls of heap_create_with_catalog and to > > > > make it future proof). Thoughts? > > > > > > I felt this is not required as we are not doing a table open on the > > > newly created table. > > > > > > > Okay, command counter increment would be required here if we further > > access that relation in the same command. I think I am facing a > > related problem w.r.t newly created subscription. After applying first > > six patches, the create subscription fails as follows: > > postgres=3D# create subscription sub1 connection 'dbname=3Dpostgres' > > publication pub1 with (conflict_log_destination=3D'all'); > > ERROR: dependent subscription was concurrently dropped > > > > I debugged and found that we get the above ERROR when we are trying to > > find the subscription which is not yet created. In this case, it seems > > to be happening because we are using a subscription that is yet not > > created for dependency recording. This raises a question as to why are > > we creating the conflict_log_table before subscription, at least this > > needs some comments. > > > > * > > + if (mask & (ACL_INSERT | ACL_UPDATE | ACL_DELETE | ACL_TRUNCATE | ACL= _USAGE)) > > + { > > + if (IsConflictLogTableClass(classForm)) > > + { > > + /* > > + * For conflict log tables, allow non-superusers to perform > > + * DELETE and TRUNCATE for cleanup and maintenance. Also allow > > + * INSERT and UPDATE to pass ACL checks so that later checks > > + * can raise the dedicated "cannot modify or insert data into > > + * conflict log table" error instead of a generic permission > > + * denied error. Still restrict USAGE for non-superusers. > > + */ > > + mask &=3D ~(ACL_USAGE); > > > > I see the point of giving a specific error instead of a generic error > > but this functionality is used by pg_class_aclmask() which is an > > exposed function. If we go with your proposed change, isn't there a > > risk that some extension or outside core-code using pg_class_aclmask() > > won't invoke that later functionality (CheckValidResultRel())? If we > > decide to go this way then we can change this comment as proposed in > > the attached? > > I do not understand this change; my original patch 0001 has like this, > that mean we are only allowing ACL_TRUNCATE and ACL_DELETE for > conflict log table, whats the reason for changing the same in 0002? > > if ((mask & (ACL_INSERT | ACL_UPDATE | ACL_DELETE | ACL_TRUNCATE | > ACL_USAGE)) && > - IsSystemClass(table_oid, classForm) && > - classForm->relkind !=3D RELKIND_VIEW && > + IsConflictClass(classForm) && > !superuser_arg(roleid)) > - mask &=3D ~(ACL_INSERT | ACL_UPDATE | ACL_DELETE | ACL_TRUNCATE | ACL_U= SAGE); > + mask &=3D ~(ACL_INSERT | ACL_UPDATE | ACL_USAGE); > + else if ((mask & (ACL_INSERT | ACL_UPDATE | ACL_DELETE | > ACL_TRUNCATE | ACL_USAGE)) && > + IsSystemClass(table_oid, classForm) && > + classForm->relkind !=3D RELKIND_VIEW && > + !superuser_arg(roleid)) > + mask &=3D ~(ACL_INSERT | ACL_UPDATE | ACL_DELETE | ACL_TRUNCATE | ACL_U= SAGE); This was done to fix Shveta's comments from [1] to throw "cannot modify or insert data into conflict log table" instead of a generic permission denied error for the owner of the conflict log table. [1] - https://www.postgresql.org/message-id/CAJpy0uANkzTyUjO2W0=3DRtaJCGg= =3DVYcwLGGCpqax=3DzKJgNbB0Hw@mail.gmail.com Regards, Vignesh