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 1wTpGH-000mFe-20 for pgsql-hackers@arkaria.postgresql.org; Sun, 31 May 2026 23:08:05 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.96) (envelope-from ) id 1wTpFG-0086t9-0u for pgsql-hackers@arkaria.postgresql.org; Sun, 31 May 2026 23:07:02 +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 1wTpFF-0086su-2t for pgsql-hackers@lists.postgresql.org; Sun, 31 May 2026 23:07:02 +0000 Received: from mail-lj1-x234.google.com ([2a00:1450:4864:20::234]) by magus.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256 (Exim 4.98.2) (envelope-from ) id 1wTpFB-00000000YJL-2xWV for pgsql-hackers@lists.postgresql.org; Sun, 31 May 2026 23:07:01 +0000 Received: by mail-lj1-x234.google.com with SMTP id 38308e7fff4ca-39664fe2dd8so11839931fa.3 for ; Sun, 31 May 2026 16:06:56 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1780268815; cv=none; d=google.com; s=arc-20240605; b=GffaDHR9rSb3egYKgWAUn0DIk51fbQ5rMWHK1LP1MzUUa0q+jjyIwR8Ri0J7Of2+ld 1SchqkX8sgFaQap1z+pBF1l7bGix5mLDX7SufWHlCa/IjE8Qa6jN5GJKBjAnE8iOcSfh o6/MdmhmQ2uucIyYtEkBGBo03DucxTLyV6ZPqizHNTt1j2YBTNKjP+XGz1bQj384hVVp SFBVL1qAwxil1Hlq2yxOnJa3XHU9h+ow7y1Ot2DT/3GeWACkRq1HnnRz6UhsIdBGOiPX qIadnQ+KrdavYl1kOSBvxiqfZoyc9e8rItq4FzWmtsGztLWGiI2Qni3xtr1AG0brUiDC 69lA== 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=dWhlWEmLJzMcuhlGsd5y/zs6cgYB+4vu0WETHjRH2wg=; fh=wg7iRtESJi/yEzmThmHeUjuGHBtElqNN7eoVlxj/jHQ=; b=VfhWdo0D+kF00o1JkRuHHWBKTkGZPWmp5tU9mG90q1zqRzYTx5BxN2TuoukpNB+CSI ULA/9wB7Da2EEsx6PNUr8Te49tPk1nJVa1nwjnyuSin+ewdikklf+ETYflk0L/2Mi9dq ovA8qRJLj0P+RCkiHU2Vlve4Q1Oej8cdNR+nfSGDsaXoEj7Lw2oYjoflOwhHNRuIWAEP eoUL8BmkBLOBY5y6bTymZDoSFvSveqCUiUA+pr2xeTodd2K+f7WtHaSYvlhlV5UKixud 1Jg/v3cNw5YGQ0cZdKb9TforhikUL1miY8lr4gUKOamp5eIEn777SLSQSgaedJA9TKaT W0+w==; 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=1780268815; x=1780873615; 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=dWhlWEmLJzMcuhlGsd5y/zs6cgYB+4vu0WETHjRH2wg=; b=lVCouyVp01XwXT3lV89ug+Rx8j4c5OfRWk8dwjktv3GCpr5yORmtac191+9GMDvU+q C2h7BPAmAhlahBNnpnJKlN6shm0qfNDI8t8W+WRthqC+xrM0qaCihvHvBjsX0/31eFtn IolMpNkQ/I4w1Xed/Y/S6nzpAmaIeZ0fTLdmISKKmVUhkiak8qeP+zcQNxx1gGp/AoOJ 14ukJgF9yuwoaS7EyPe04xoc7q2oH9cXOAyhbN+Jtf9vyLqLYhHBbrzDobUQnK7DQfP+ PuJ1Zd2G2T9Jxg4UtSDkL4uR62lF9aNpRxzCxAsCcZCCKPfmODhWMc/j/31pjxW3wK/e xcjg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1780268815; x=1780873615; 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=dWhlWEmLJzMcuhlGsd5y/zs6cgYB+4vu0WETHjRH2wg=; b=nscrZ6b53yJ0M2lpVTKWFxT8IZvj+w6b1/pN5MMqz2LaJSJwkQ0YCgBtZCN4PWxN+h 1HAplvt7fIhxk+bHt5tvrF0WC4CtADnSVNYvQOdkOGZqg1C4LYqL21dv5K29uDzv2jRH oNLrovr2n+Rh6hQqwBojMi4hZ6TS2nfFPjQ4aHKRX6htAn6xIMcHislYhR1uups3LSg6 MmMcES8rqFxF27Il6K62sJNd4vVSxUO09OqjtuEOVcy5N+8LSM01T9SFBs6sIVflqjSB KNGaoynRKRHObCF4RI/12j5xi9e9soJ08UgXwQQUI4SSW9YXASUceGS1TA/RygqKKCc6 X3LA== X-Forwarded-Encrypted: i=1; AFNElJ/TjmieKbg1pChSGqyA7E95CnnpVxGa0Mc+fcXAQ/gKZPRBPJSidF+aDl2S2Gm5+nutPAsqCwpNdKaNEPkj@lists.postgresql.org X-Gm-Message-State: AOJu0YxrpKu9JLfEiSu9xvA8JAWIiLbRaDhfwVKkYCryZF/fUotbllXt DUbYw+Wx3rENncudcLCXjj6jqLp+vUAMwF6vVEHR7KNZH1XPe0a31wSoyRC9IedrsmByRumTdtS PzjhworoM+QHyRSjwPgmf1vqoyLdbYQQ= X-Gm-Gg: Acq92OEQNKz45uDMJx4RZiaWTNPTz6osKCVx2N7c1lAGY2cq85LXmxNoBtiQjNWseIQ CVNtcMjZ69WFSWE+5KGAKYjg67ggtUtv/pI+ZyD3Zp8+19CmOU+Tviczil3U4dyUkyI/tKCErPJ 9tKC+dqsrJXXRP29BFKyYzRLmsVXOTwslDqq3LvigY7UhlOoMTFEesxkKWHw8utxK6wpJD3qnGb nqGfBAfqnL7QhZRogPMH90/9GEn4XLtKl6mzWXLly2/pcUr7HFymuGpd7XdWFbrvtSnRsvkWk4G KXEwN7em323t2nH9uBoKZdzc2R0xnEhh+MedqKgwSVhidfSxZA== X-Received: by 2002:a05:6512:3c9e:b0:5aa:6b09:2ac3 with SMTP id 2adb3069b0e04-5aa6b092d34mr780606e87.24.1780268815169; Sun, 31 May 2026 16:06:55 -0700 (PDT) MIME-Version: 1.0 References: In-Reply-To: From: Dilip Kumar Date: Mon, 1 Jun 2026 04:36:36 +0530 X-Gm-Features: AVHnY4JPf_g1HUg6uIEyxlv-g__TIHNFplnh8RyW64bzHaMxjDM_oXqflw9exHY Message-ID: Subject: Re: Proposal: Conflict log history table for Logical Replication To: vignesh C 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, May 31, 2026 at 5:38=E2=80=AFPM vignesh C wro= te: > > 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 = wrote: > > > > > > > > > > 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 thin= k > > > > > 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 firs= t > > > 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 t= o > > > find the subscription which is not yet created. In this case, it seem= s > > > to be happening because we are using a subscription that is yet not > > > created for dependency recording. This raises a question as to why ar= e > > > we creating the conflict_log_table before subscription, at least this > > > needs some comments. > > > > > > * > > > + if (mask & (ACL_INSERT | ACL_UPDATE | ACL_DELETE | ACL_TRUNCATE | A= CL_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= _USAGE); > > + 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= _USAGE); > > 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 Thanks for pointing it, I will analyze this behavior and give my opinion. --=20 Regards, Dilip Kumar Google