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 1wePWl-004mhv-0p for pgsql-hackers@arkaria.postgresql.org; Tue, 30 Jun 2026 03:52:51 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.96) (envelope-from ) id 1wePWk-006I1Q-0V for pgsql-hackers@arkaria.postgresql.org; Tue, 30 Jun 2026 03:52:50 +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 1wePWj-006I1H-22 for pgsql-hackers@lists.postgresql.org; Tue, 30 Jun 2026 03:52:49 +0000 Received: from mail-lf1-x12e.google.com ([2a00:1450:4864:20::12e]) by makus.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256 (Exim 4.98.2) (envelope-from ) id 1wePWh-00000000rLF-2tjw for pgsql-hackers@lists.postgresql.org; Tue, 30 Jun 2026 03:52:48 +0000 Received: by mail-lf1-x12e.google.com with SMTP id 2adb3069b0e04-5aeb5e85378so2344788e87.1 for ; Mon, 29 Jun 2026 20:52:47 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1782791565; cv=none; d=google.com; s=arc-20260327; b=UBS4Oxp27mKO6S9bJA383WfsFNGUJh2NCvWlHYbpl6o0Yu8c2UuMIL6H1AvEBG4S2q V6bGdL1wEIzkKYH1Gfah/0hef51MsdVkRQjWdfMarP/5ME6+m10XUi7oKHPIWD+wIh+I GLhYEm2sB0BHuovNcbto8ZB4t50H5LD4fxaHxJ2SCwhm3AvQl6+NU3ecRWBrWForOkYp RWsotiM7A5aH3ah/aNYwMIsd+eiNCTwdCGq7xAgGEz1ar7nhTkXOLEMtw5CI5YKY2cp0 Lj7vq30VVYtBr2DBa3relLYIYUZzk9bMm9OkMvc9w0MMaIsCC5BxMHhfDSjTaw6bJ6eH AweQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20260327; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:dkim-signature; bh=OAFvoOg2E4EQQTHH32/hCaMh+ZoPR7i0LT0vY9E36g0=; fh=pdi84prH0yXRnB1QdfvHwTaHzfBK4evX2WKDqbdAiYA=; b=i00tTrjpvjyEJb9Cn8UsIWSa3/UpLTnqDQY1GxY1RrJJNq4zeAM2EOyT3AiUN9FXH5 /O87qCTNsl43qqh0cXiF6szis0g+nc62+PtcjPqlzC2MJhB7BzQLwO1606RKP6yGtGM9 DQJxgXT1qQTqH+q/tK+bdiLAkfRf7W5RLo6YnvXLEIXseDHgZOwysDNOq0Ku2UfxTEH1 j10c+bh82icKVbnFnLNaJZD+S3elyj8Rrw1/sIJm470XPH7xptzr0JnPjWjJvgP+GiRn HDRirQi+dnDXMUurCrDVdKDozKCy6u1J2Id/84t5pDo1LHdOdONBim1sBTGOpwKHRcDt IXIg==; 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=1782791565; x=1783396365; 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=OAFvoOg2E4EQQTHH32/hCaMh+ZoPR7i0LT0vY9E36g0=; b=GA3Aq5Z8F64xVEvFWfrwO9G0G95Ce+iyor2LAKKRxiUqCU0dDCv4Jo5dXvwW7PYw3c zSTUFnRtZ86hoTTK/MnOcXL4RviUJ55rRfyxcJqMfPaPyx3MLFZuJs97weUMSo1xi/+L GbR8TjaX9Tox4VT5TRbn0hakDIKoUUimnyhlw6PyzOFShcPoK5ri89+NIR+7EH3vz77B ZFTVWHG+Hc2quU8d/7N3+kp9b2M5vKHK8gep42C8IinSuIIGgOsFdWKxJqtLjL2UEvKC RpkMptWAwtYKZf/00RWz7lp1mcioWykvNcp2GGf/bv3OhJ2HbRWTw7eZ/Mh/6OjdRdt+ 3OaQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1782791565; x=1783396365; 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=OAFvoOg2E4EQQTHH32/hCaMh+ZoPR7i0LT0vY9E36g0=; b=nzeluRGTERF7HpbJBCX5wL8SUrXW5vlHGkP5JBcxDzpWQZtNoh3i4aB8T8pArdo8Jf ocIyR2UV3NVuZIyFAbZpwPipgEUj6rC/Mes/sNaZg7AWoJIfJ0ipyweMY7ozz3TufoRg yAIeiSMUZuZVmgSVpgmrrOlD1KJEih0waBjB9J8H+1BUPqQFjJuk+hQa+p2SKGlgNM9Z 4nLfCeJXjAqMy+6/N/2ULN+fvKqcE8DZfOBF+BIfCDmvDnARO9PxcQY8QS2zQ2QGNOMu S0TKqrYUptaOw86DuCe2+bSeGlG6JUIl3F5CzdbdSf0yJ0GbIoJ0twgOm8R2A1EE8+3d SCDg== X-Forwarded-Encrypted: i=1; AHgh+RozCoT5hR7+zdN7Cetl2RbzrOrq7RGe2VWNKmt6lCK8EJjfru0ixuN91uIk0aC4gAn/+Fl2YT36I9xZKpYC@lists.postgresql.org X-Gm-Message-State: AOJu0YxWr40AZRTF9Pyb3TxCsd4vHdPAJIqIixIStxhQn9inVOL+fLiA KTW+gO1KQKR/u7cug071owQJeKRd2dRuvj1vcLgv+ByvNbuqnvRzKE9BNfxyN8W6vGr52mKfr0D vYqlcUQgjpNVD+TBVhSi3E6V8miAs/Kg= X-Gm-Gg: AfdE7cnJ0JRjdQtYzwLU1/Dy5LWOQkrghxMVx8gRWa9G8I9Q7u4L/nMWC6WQ4fTENVc SNaupm4tlhvevJgwAKyAc1o86lHujuSm1om9O3UjfMkyX1sABr4QVr+JkQjGjpOnfmdqrCTz12f dbEWgfn55BBBQy+JpyIqNWb+QfWaBzttnOvt8SNCy7L8rsTPfTnsHCpBscMg18IgJAnnQUNf454 Y+8t1akre4s+zC73ef+n7WfnrYhSvPOOGemFc0sf6Kw0HaTEKU0O2wifNpeYK6Ls6cI34AyYpH0 WsXPFRXkL+Qce4ryriAW2z5v3F2uKw== X-Received: by 2002:a05:6512:340a:b0:5ae:ba95:589c with SMTP id 2adb3069b0e04-5aebdbd1074mr346294e87.45.1782791564369; Mon, 29 Jun 2026 20:52:44 -0700 (PDT) MIME-Version: 1.0 References: In-Reply-To: From: Dilip Kumar Date: Tue, 30 Jun 2026 09:22:26 +0530 X-Gm-Features: AVVi8CdMKbLRW8YZ9r9FzAdroOewFApF-JC0BrV3PzxRjJXktabQTQQBpBy1gVQ Message-ID: Subject: Re: Proposal: Conflict log history table for Logical Replication To: Peter Smith Cc: Amit Kapila , vignesh C , Nisha Moond , shveta malik , 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 Tue, Jun 30, 2026 at 7:26=E2=80=AFAM Peter Smith = wrote: > > Some review comments for v59-0001/0002. > > ////// > v59-0001 > ////// > > =3D=3D=3D=3D=3D=3D > src/backend/replication/logical/conflict.c > > 1. > + * > + * The tuple/key columns (replica_identity, remote_tuple, local_conflict= s) are > + * typed json rather than jsonb on purpose: they hold an exact audit sna= pshot > + * of the applied tuples and replica identity, and json preserves the ve= rbatim > + * representation whereas jsonb would normalize it. Indexing them (jsonb= 's main > + * advantage) wouldn't help anyway, as the conflict log is looked up by = its > + * scalar columns(relid, conflict_type, commit timestamp) while these JS= ON > + * columns are per-conflict payload to inspect, not search keys. > + */ > > Use consistent case. This comment has a mixture of json and JSON. Thanks, I will fix. > ~~~ > > 2. > + { .attname =3D "remote_commit_lsn",.atttypid =3D LSNOID }, > + { .attname =3D "remote_commit_ts", .atttypid =3D TIMESTAMPTZOID }, > + { .attname =3D "remote_origin", .atttypid =3D TEXTOID }, > + { .attname =3D "replica_identity_full", .atttypid =3D BOOLOID }, > + { .attname =3D "replica_identity", .atttypid =3D JSONOID }, > + { .attname =3D "remote_tuple", .atttypid =3D JSONOID }, > > The replica identity attributes are splitting the "remote" attributes. > Isn't it better to keep all the remote attributes grouped together? I prefer to keep all JSONOID together > ~~~ > > 3. > +/* > + * Convert the string representation of a conflict logging destination t= o its > + * corresponding enum value. > + */ > +ConflictLogDest > +GetConflictLogDest(const char *dest) > +{ > + /* NULL defaults to LOG. */ > + if (dest =3D=3D NULL || pg_strcasecmp(dest, "log") =3D=3D 0) > + return CONFLICT_LOG_DEST_LOG; > > Instead of default/NULL always being equivalent of 'log', what are the > thoughts about introducing a new GUC like > `default_conflict_log_destination` to override that default? > > e.g > * SUBCRIPTION option `conflict_log_destination` valid values can be > "default" (default), "log", "table", "all" > * option conflict_log_destination =3D "default" means use value from GUC > `default_conflict_log_destination` which can be "log" (default) or > "table" or "all". > > Existing code could then be configured to make use CLTs without having > to touch the individual subscription DDLs. > It might be useful when there are many subscriptions, or when a DBA > wants to set a site-wide policy (e.g. always log to tables) without > individual subscription management. I do not have strong opinion on immediately providing GUC for this. > > ////// > v59-0002 > ////// > > =3D=3D=3D=3D=3D=3D > src/backend/catalog/aclchk.c > > 1. > + /* > + * Disallow creation in the conflict schema for everyone, including > + * superusers, unless in binary-upgrade mode. > + */ > + if (!IsBinaryUpgrade && (mask & ACL_CREATE) && > + IsConflictLogTableNamespace(nsp_oid)) > + return mask & ~ACL_CREATE; > + > > Is this code OK? It only checks ACL_CREATE but how do you know the > rest of the mask is sane? > > How about: > return mask & ACL_ALL_RIGHTS_SCHEMA & ~ACL_CREATE; I think this makes sense. > =3D=3D=3D=3D=3D=3D > src/backend/commands/tablecmds.c > > 2. > + /* > + * Conflict log tables are managed by the system for logical > + * replication and should not be used as parent tables, as > + * inheritance could interfere with the logging behavior. > + */ > + if (IsConflictLogTableNamespace(relation->rd_rel->relnamespace)) > + ereport(ERROR, > + (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), > + errmsg("permission denied: cannot inherit from conflict log table \"%s\= "", > + RelationGetRelationName(relation)), > + errdetail("Conflict log tables are system-managed tables for logical > replication conflicts."))); > > It is not really a lack of "privilege" preventing this -- it is just > disallowed for everybody. Would ERRCODE_WRONG_OBJECT_TYPE be a more > appropriate errcode? I agree that we currently restrict most operations on CLTs, but the issue isn't that the CLT is the wrong object type, they are still relations. While we might open up certain operations in the future based on business needs (such as allowing index creation), I still believe this is a matter of privileges rather than object type. Because these are internally managed system tables, no user has the privilege to inherit from, alter, insert, update, or create indexes on them. However, users *can* perform delete and truncate operations. Therefore, the object type itself is correct; it is simply that users currently lack the privileges to execute those specific operations. --=20 Regards, Dilip Kumar Google