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 1weNhq-004lYr-27 for pgsql-hackers@arkaria.postgresql.org; Tue, 30 Jun 2026 01:56:10 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.96) (envelope-from ) id 1weNhp-005uBZ-0Y for pgsql-hackers@arkaria.postgresql.org; Tue, 30 Jun 2026 01:56:09 +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 1weNho-005uBQ-22 for pgsql-hackers@lists.postgresql.org; Tue, 30 Jun 2026 01:56:08 +0000 Received: from mail-qt1-x82a.google.com ([2607:f8b0:4864:20::82a]) by makus.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256 (Exim 4.98.2) (envelope-from ) id 1weNhm-00000000qXy-2Avm for pgsql-hackers@lists.postgresql.org; Tue, 30 Jun 2026 01:56:07 +0000 Received: by mail-qt1-x82a.google.com with SMTP id d75a77b69052e-51a84669e39so25555511cf.3 for ; Mon, 29 Jun 2026 18:56:06 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1782784565; cv=none; d=google.com; s=arc-20260327; b=R4DAfLPJm4LhcqsWjlpswP0TT9Snm4je7rMuJoriOv+oImo3384CAaoVwsIWOpcot8 gCZ9Si36K+177nHo9KK+d8Gp5moK923l+KnqA2T8fIXa5GLqGqxgwAnWTh4HPkHNYuqC 0qM9n0dOFkDIoUGaUL1D6HBeNhXCVYq9uTF4nq759ELskPI265JekFD53q6QKXT+W1IW FxnqXuyzfuQuPbweqtdqXRVKjQe4WiVc1ZXExQNtM8JVUxxvVgQSC01NFd+lLTfWQa9O minf4syI9kDriteDYVq3WyVJ78DJg2pUlA+B20gRGt99r2QCRzPzPmJ+3GpnJRUoAnTF Cbvw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20260327; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:dkim-signature; bh=eliDGQYyYmfDx7hIFHq2zWksbbKavRcp7VOoa7zF0wE=; fh=3uIfqys2Yd20OyhhdbCfejocx+ti5jl8vTem4OwTeWw=; b=RmaAZGuojbZbm0RuLorKSW1kFSNgjBVssrn7Aw9ymFRrGskbAjHqjbLHLq8pTK8H1C /haMZ27EPZ2SMm6L3DMyeu85x5bkXY3QgXVlow+guwdlP1d7AyUIxbIgio0Id6VCW2sj tmraEtJ9ypXJFdOOManZ1fZ2PZmke0+lH0AYz86O/abLOex+YHzL6ihL4mGgv3KVMpRe YwMdIcblqOQsnUi8nOKotx3/dmabkmyHMxfmgITciZwox10RfJdaDYmw4zug6sJVT5AV NqWH7XFqmZBWhXD8T4uhpj1gqxNt221jtvs7TbDzBSZYyZ9MhOfKzREsFp+Hkm0QFCHt iQmA==; 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=1782784565; x=1783389365; darn=lists.postgresql.org; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=eliDGQYyYmfDx7hIFHq2zWksbbKavRcp7VOoa7zF0wE=; b=M4kHNfBwpRftnt8b4LEYidSTqfJ8gnP3+yMmi3UNUCJ6mjg9Kh+XgR7FYkNrdbwxqh flcVuyT4AKEGuHXPet+yUR79sDzD0DhJOJTlPdJjJl8vqjC3s8QfIA5pU0ncOvZKrMeH N1mrOtNh0f5FbNwMUGSCigGK6QsunTxg/TOzTPg4qufabyrJklOhC8ofCSFrjYrgakBi yczwpZ6USXuY8/tie6x5zCSLsDM2UTgUTVqeOKrZ4mpMd4VyVAOqtNnPNyNqV5JywhsV HQQDnUoKLKiOH8ZshZEec9mZkI5CFVTdItZJRf98mZ7HXP8lXwf183swf1rWF8Ohyhd8 3Znw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1782784565; x=1783389365; h=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=eliDGQYyYmfDx7hIFHq2zWksbbKavRcp7VOoa7zF0wE=; b=b6KVbPV/GYx2lD8zE8dwH5pBPIpPGwWDkquu06EbWHWe7xv0xCqdFTgII1uaZzTQcu 23KcwZp1WaCTtwbB4ePSYIYThhm+rtWGNLREorQsNK1R36YjCEVDh+0RuPNCAKHEtUMn puBUxHNG/WFoJmVAwF3R5yLIGsbRZ9zrkxFhOu7HWm1Twn5AH2LJ+F4RjhE0TibAuCov C1jG9NZjGjK1Gj1tX3g2xuiQ0yC49fxOXmmf3pbC4nnbZw0SN76JjOePX1y2nr9OgFhE oMtIRzTxIwe5B1X60vjptidGcK+hN4TKRJvEHoLUhH30ChxaChCxEJCx0d3He+t2tSVg fWEQ== X-Forwarded-Encrypted: i=1; AFNElJ8pgFkRoVvcXzLiNtKyPfmfwCb0ZtvVUM06JGGYt3Jn4UkLPuzr50nPdsnn+nBlMoR1ZMFzUNsOwPYZ92CR@lists.postgresql.org X-Gm-Message-State: AOJu0YzBCB/Y20XJKAiO8oKxqCDRxkXxmMSpah9K6PIqqiMu6OERO56s SWXHDG4+1/vLw+g4ui9MeexUfr6TtrtyvDB2zfePSEU7Yjm89LX+CAe7B/LgULWJlrRGixWS7yC MthU+SHU11vy24M9N2ww51T5uKcGM2sQ= X-Gm-Gg: AfdE7cmevpEPMfzf2goq8YTXG17fdMMb+L/FkbKeWWvf22FlR6SaoEOCxcVk3M5gm5L NhdXbcp5/zWPMemmKfFBeG0hyS4rSWRestjVB1AAlwLxaimhGXkntJkWAAwTkFotU7RzINc4BpR vs27mM5zu7oS9z3ZY3onkjFMqBgDKpetXgT0zqpmP8gA921DH7w+3BFh3QoXf52CmRZK5VrAb33 Xa1o+RxirKm1oiBTOE2N4RP1sBZTthnqRzkJV7GTaaBEyARAf/60UsBYVcCHY8hgJDUV6FuKQ3A mX1wpt2dng== X-Received: by 2002:a05:622a:3cb:b0:51c:223:3c59 with SMTP id d75a77b69052e-51c10825a24mr21688581cf.39.1782784565294; Mon, 29 Jun 2026 18:56:05 -0700 (PDT) MIME-Version: 1.0 References: In-Reply-To: From: Peter Smith Date: Tue, 30 Jun 2026 13:55:37 +1200 X-Gm-Features: AVVi8CcCdchwntC2a4_F7FvBbrqpBTw_rO4GHiyU88MU3tsvuPuYQhOvFD-7Suo Message-ID: Subject: Re: Proposal: Conflict log history table for Logical Replication To: Dilip Kumar Cc: Amit Kapila , vignesh C , Nisha Moond , shveta malik , Masahiko Sawada , Bharath Rupireddy , PostgreSQL Hackers Content-Type: text/plain; charset="UTF-8" List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Archived-At: Precedence: bulk Some review comments for v59-0001/0002. ////// v59-0001 ////// ====== src/backend/replication/logical/conflict.c 1. + * + * The tuple/key columns (replica_identity, remote_tuple, local_conflicts) are + * typed json rather than jsonb on purpose: they hold an exact audit snapshot + * of the applied tuples and replica identity, and json preserves the verbatim + * 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 JSON + * columns are per-conflict payload to inspect, not search keys. + */ Use consistent case. This comment has a mixture of json and JSON. ~~~ 2. + { .attname = "remote_commit_lsn",.atttypid = LSNOID }, + { .attname = "remote_commit_ts", .atttypid = TIMESTAMPTZOID }, + { .attname = "remote_origin", .atttypid = TEXTOID }, + { .attname = "replica_identity_full", .atttypid = BOOLOID }, + { .attname = "replica_identity", .atttypid = JSONOID }, + { .attname = "remote_tuple", .atttypid = JSONOID }, The replica identity attributes are splitting the "remote" attributes. Isn't it better to keep all the remote attributes grouped together? ~~~ 3. +/* + * Convert the string representation of a conflict logging destination to its + * corresponding enum value. + */ +ConflictLogDest +GetConflictLogDest(const char *dest) +{ + /* NULL defaults to LOG. */ + if (dest == NULL || pg_strcasecmp(dest, "log") == 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 = "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. ////// v59-0002 ////// ====== 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; ====== 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? ====== Kind Regards, Peter Smith. Fujitsu Australia