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 1wZ5wI-000ljV-03 for pgsql-hackers@arkaria.postgresql.org; Mon, 15 Jun 2026 11:57:14 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.96) (envelope-from ) id 1wZ5wE-00Bl0S-2D for pgsql-hackers@arkaria.postgresql.org; Mon, 15 Jun 2026 11:57:10 +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 1wZ5wE-00Bl0K-12 for pgsql-hackers@lists.postgresql.org; Mon, 15 Jun 2026 11:57:10 +0000 Received: from mail-lj1-x235.google.com ([2a00:1450:4864:20::235]) by magus.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256 (Exim 4.98.2) (envelope-from ) id 1wZ5wC-00000000VYA-12Is for pgsql-hackers@lists.postgresql.org; Mon, 15 Jun 2026 11:57:10 +0000 Received: by mail-lj1-x235.google.com with SMTP id 38308e7fff4ca-39666ac91a2so37630321fa.1 for ; Mon, 15 Jun 2026 04:57:07 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1781524626; cv=none; d=google.com; s=arc-20240605; b=WpNW2MGeXTs7xVCPJM0Brlj7rnvSrf1EBWCupFrRDULI/vnrJbJQcemcWxtZQPkYPX BCvMpHJfnDteJ+w2TL6VRjcy5Vf1PQRi+NJfEBRN6QYrnolR8tvI8/moxMCEs61fz7bt zFlWyr0dlcYOCV8J8HGf1XL1adRWUCHyQ7ZpbsRQipusawh+E820x4O7FzG3Zb+xc6BH V0J1B4BJ0dgD+sR/gzC1F9z1KR1DCFK3qH9NrFJenABOKOoShMDo/VK5c857PIkGMqBi c6sZSc4lG9xJ6kI/J+j3Zi8z/4cV1mJeMBCy05BMZQawdj6oqzU7MeeO4LQtaAS26uXL zW/Q== 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=e43SsbRfWyro4HRj8J7uDi6gTqQs3yDJy2cLxV0Y1/E=; fh=juW1mALnHp/EZxCmzrNJA0p9t8C4YzwCHnYQ2fhtDWk=; b=IbPC+8ergF//j1PBs4Ao0ZZ/2R7e8CMiijxCOGBQ5DsuYoaautPezAwxf6v7wefP3b j0QHC82KJiy7tXbSMLFDzSfHq/YS2kUmXoak2wWMPjltAyu+foQv+99NdnKXN2k+35w1 /Olg6gnFXkIBOsPMoQOdLheABQIIVZ1mOEN7UZZvCTZ07Wg23+b5ltEkhc7VhDqYGZls yNMZm1UuZPDtt8FEvDPw7YlCxDGQTR15tWDMNpTGy3t0Fnw2dxqZG1Tg0H3DRIDjqIVV SqpkU70hEtogETfqW1/06HAuWdxIYYXJLicyqmPokGprmPbTx6kNJSR/BmVgrwP8XMDB XnCg==; 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=1781524626; x=1782129426; 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=e43SsbRfWyro4HRj8J7uDi6gTqQs3yDJy2cLxV0Y1/E=; b=htAYFEpzfZcsYzMZaeTOf0/Z1TAfrsfAdY+ksC0EM4WvvtLKKdGoMxQD5DYnQVUWGP re6v3Gog04Xdsu8fEArJ8BXF6WdtWaF8ZbwzpL+HtPG87iiO1zQdPzsWzuFQ4AtfbYPn tt1O0p2HLvtSCPgOuIqAanKIeAuXxD5MQ4uJSKr6XVmshpD/RUpNveL902XfSK1fZ7ol PzNkgaCaeTi7DOB9OwsabjYK1N3JmBC6ekMH9B/yKtLSV8w6ANrLnL8cfXHl2dMMDoCx rfSH6bNkW+/vZ61sXaKMe9qd0fWxzSygHGV+oIgIrmZJDNdLgelY4OLwOq+NJPHi1VGh RNwQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1781524626; x=1782129426; 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=e43SsbRfWyro4HRj8J7uDi6gTqQs3yDJy2cLxV0Y1/E=; b=EARc2MnDSuxvvR2NJbqMqX+EXjctVv+gPrNiE7zBmELkCLOwjBjTZ3VWUcpFplR//j mupOQWLYW+ntNJWl5dj563/7vHUgeCb0cMj50dV0FX4uqk4kY1edhPLzHLgOw5crD5S+ lFAl15AF+6BLx0+i70zmbh4f5AN2ATiO5JDC9l45OUrxhh7K/9Jed0UMDZeAzTCb1okN CkM4lnxDicL5aLEwWPfPYqvF3T1CrLzgisauGTrTNQhkQg81cUeIUMLNpmY/BjFBwK2K 1nhmrc/80PY3S0s9LhM0200CREiBz3XK4RuP4r3G98VddwVTFjWmH8sCuQUzKHmvzlF7 owyA== X-Forwarded-Encrypted: i=1; AFNElJ+CPt7SsE1pNNWwrLOu2FDVSHPrd9oIi+4n9a5+7LIi0FF+o4RZhqakol9m9swhAd/ewtsciD3A4Tfa22bZ@lists.postgresql.org X-Gm-Message-State: AOJu0Yy1HXMmVSTMXLezV40DxV8Teh35Ug6S85hOTLTeM3RESNaiVnWU alm/JX4r8enbfx8+Jdd37KpMjhgfwCM2paaV4tyRT+WdI5zJgPn2+iku75GDGPnBMF12yWdbgef iq85OTnU+3iaxkG0Fy29mrnfXwTr9154= X-Gm-Gg: Acq92OEeHrlG1IwqhYFc7d9vj9ZGqtU8RE32ZeZjJ+NEsvBAziyH0MXGJ1WX8YcpGL2 0EzUDqkLYDgLIKomCyWr6Q3YBIN3UgY+qR9/gU8UnzOIqpgCUgGV6aYaQdXeFPOUS+vJ5RQoZqW 1A9D/jOz3ZPpYtx3uSYIBOtlnly05D1rE5sIGLp1laILLoNgF8Gx4JJOEefD0KMIUznqeqzgcTE YW6h31WxE9uZBfPJnAm722m9MlFa70UQW/Bu9hx+1QqR6pALPEpRSk31iglnm6NFo7HCHUiv22G PsDI2ersqCmX5PdV1WU= X-Received: by 2002:a05:6512:2216:b0:5aa:6c89:72d2 with SMTP id 2adb3069b0e04-5ad2e13b26dmr3107145e87.21.1781524625532; Mon, 15 Jun 2026 04:57:05 -0700 (PDT) MIME-Version: 1.0 References: In-Reply-To: From: Dilip Kumar Date: Mon, 15 Jun 2026 17:26:48 +0530 X-Gm-Features: AVVi8CcGHV7Ev_7FbIdzU6m-w-DPLLQ3i4ykydcXjm75RyRJxxYOeMCZ1du3Umg Message-ID: Subject: Re: Proposal: Conflict log history table for Logical Replication To: Peter Smith Cc: vignesh C , shveta malik , Amit Kapila , Nisha Moond , Masahiko Sawada , Bharath Rupireddy , PostgreSQL Hackers 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 Mon, Jun 15, 2026 at 10:35=E2=80=AFAM Peter Smith wrote: > > Some review comments for v50-0002. > > =3D=3D=3D=3D=3D=3D > Commit message > > 1. > Missing commit message. > > =3D=3D=3D=3D=3D=3D > GENERAL > > 2. > + if (IsConflictLogTableClass(rel->rd_rel)) > + ereport(ERROR, > + (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), > + errmsg("permission denied: \"%s\" is a conflict log table", > + RelationGetRelationName(rel)), > + errdetail("Conflict log tables are system-managed tables for logical > replication conflicts."))); > > 2a. > There are many duplicate "permission denied" messages in this patch, > like above. Is it possible to do something like introduce an > OBJECT_SUBSCRIPTION_CLT so you can delegate all these to common > aclcheck_error(ACLCHECK_NO_PRIV, OBJECT_SUBSCRIPTION_CLT, clt_name) > error processing? > > Or, if that's a bit too hacky, put a common error function in catalog.c. > > e.g. > if (IsConflictLogTableClass(rel->rd_rel)) > ConflictLogTablePermissionDenied(rel); I do not really think we need a separate function or a new error code, but I am open for what other thinks so for now I am not fixing it. > ~ > > 2b. > I'm not sure that the errdetail is really needed, since none of the > nearby messages have extra errdetail. Instead, you could just modify > the errmsg like: > "permission denied: \"%s\" is a system-managed conflict log table". > > =3D=3D=3D=3D=3D=3D > src/backend/commands/lockcmds.c Yeah we can remove errdetails considering the other nearby permission check code, provided no one else objects. > > 3. > + if (IsConflictLogTableNamespace(get_rel_namespace(relid)) && > + lockmode > AccessShareLock) > + ereport(ERROR, > + (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), > + errmsg("permission denied: \"%s\" is a conflict log table", > + rv->relname), > + errdetail("Conflict log tables are system-managed and cannot be > locked explicitly, except in ACCESS SHARE mode."))); > + > > Is ERRCODE_INSUFFICIENT_PRIVILEGE/"permission denied" the correct > errcode/errmsg to use here. e.g., is lack of privilege really the > problem given it was not supposed to be locked by *anybody*? Based on our previous discussion, we are now allowing locking of the conflict log tables. Therefore, this code block will be removed in upcoming version and a note will be added to explain why this is permitted. --=20 Regards, Dilip Kumar Google