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 1wPaTk-000rfy-0k for pgsql-hackers@arkaria.postgresql.org; Wed, 20 May 2026 06:32:28 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.96) (envelope-from ) id 1wPaTh-006NnL-3D for pgsql-hackers@arkaria.postgresql.org; Wed, 20 May 2026 06:32:26 +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 1wPaTh-006Nn4-1t for pgsql-hackers@lists.postgresql.org; Wed, 20 May 2026 06:32:26 +0000 Received: from mail-pg1-x52f.google.com ([2607:f8b0:4864:20::52f]) by makus.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256 (Exim 4.98.2) (envelope-from ) id 1wPaTg-00000000Rk6-03ak for pgsql-hackers@lists.postgresql.org; Wed, 20 May 2026 06:32:25 +0000 Received: by mail-pg1-x52f.google.com with SMTP id 41be03b00d2f7-c802803ac17so1932100a12.1 for ; Tue, 19 May 2026 23:32:24 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1779258744; cv=none; d=google.com; s=arc-20240605; b=lwOXbqouoxRmyK+y2baH3KzdYUGWG6UxopkUGAZC6kF1DMb8tJVhsV4L1vANZlIoxW j2HcaytwSrYsx4gX2OCV3Gaz+iUTWghoZ7MHka4mDSLZR6aVonsK7VzavGP8d6Er/G/M gZJEVgHzGJyE4uvr+PPsiJHbFdO1V0MCYhtQnmOrjktLJy+HK7nCZaj+7ibuT8NU5NKn gMru/sJP9JkhlaI+OO44sWIPSOcCgdSRkbb0UwdBoP1GzxT7vfP7GL0tUwtPxu/Rz+CU 8H5kIgleSlWA4cbdLbLvnH+kIN3MlSVK7XDMl+LAONwwfrKCeQQ+GPPJNCHhJEHNSeJ/ jHUA== 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=whgJJfbRXkvpGeIdENyLm3s+5Hgza/qxK9RnKwQ4lTw=; fh=IaW0GGaaEV2aaeiQLRoXXSnPuLGeBBUd+BQb35/DmkM=; b=BLQLNNVMpvcuTUQRGHxMuYBOqKxokiWXWOapngPb7+V3sTNy9VAv5g1VGs43OyQsZZ DiZKASkXG4YCDKHUOsUfzamEJP6TiUh9CItqkVJbzf1QyLHA//mbtGDRQ0RWelhl2UB9 sWLLX0C7y3m2tTojBF/+xlkAN8MBf5pYkimhtobdMnjd10GSJGRG9+3hSdZ/i87i9GS4 jyDTuyEFN0hYq6etKV4PCsN+DslzE1B2cn+2jE39QXDLh+jSx2sHMAmKdeHqkAuK+Rmq HZ4ARZJvViNxYKZcPLmcN02XWnCxuO0pVLWMPa/NoyhL0Yb1hMK7t/K3K1u0JuaEl3fy CI+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=1779258744; x=1779863544; 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=whgJJfbRXkvpGeIdENyLm3s+5Hgza/qxK9RnKwQ4lTw=; b=sqpRVJlGzwTP+CRK2WesjDHgcPY6lKuzdbbQPcRbUwpMxuvoFx5JN3Fva/sYkoslW+ Kke32HxggwhcvRXamq23P+eciTshcXuqAwXF1FK02I76AP82Kfl0gNkCCUZkJrWLcsCp MhzlTLtu4rNgwJmSijotufQ/HyKl2LJqBpLV85KX8/d2gyuyyb3FDiyC5OsAUtnwZyYN 7CMe00LxCf+fFP0SozAbzsN7c9UOL3tzMWUtcAkdbvLW1TjKLHS9MD/t6YJVpDb5cMNo 6wo8fDNmJG86nQMU3glAO5oD6LNhmUGjKRHyVBKnQnIxcs5N/lLugDdKCCO1OSiMkkH0 7QOQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1779258744; x=1779863544; 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=whgJJfbRXkvpGeIdENyLm3s+5Hgza/qxK9RnKwQ4lTw=; b=lSPeS+uKjcEEHqMjw1m8hcvR1KjzaeYtURYqy7VF0AnefjF6RSoEvSfj01/HG7QN+T bxD7FurBEyY7lEelc9FGxIsUPjJsvMIkJEAI5yl6eDL9z9ulSr9XtP9ETWjhKDlapyJs sAoaJlJzNxDYFSZA/PsnHmLGFSsueu4t31CRLKgxdFLbJvADZYJBScKstfq9B1qwlumL XvF1rYpTeiJ8mQ/vqWan7L6y00Qt6C9lZdJgNlRifUZgPmUDkNdSJq0wCOETgiZz1hca vfMMVQMxvXU63K1bME2BWYULrDMBbW+f+5U4Oyk9O8C24UkRyDucTn9szkwLJ6MHE2NR 6OLg== X-Forwarded-Encrypted: i=1; AFNElJ/ZZFp0wEl5nNNBGAkJi42AnDKrpI+i7WBkhqmp7gzxBGbkZghehHnB2zY1YFSR53bOSvLklLV6bl3QzcGK@lists.postgresql.org X-Gm-Message-State: AOJu0YzK0qnNrGShRRSsNzBZujnStEfhtnwRYPAluXTU5eF/Al8kFk4i Aarm+Qm3ETLP2R7dhk0xjO2hW+96nPdRnhokQEeQgUpj1RQNv4ZdDd7obilrmnZ30ChurWf0ywn ib2k4zsZ6XwZDvtTiHWQSNnKTNG3Uf7U= X-Gm-Gg: Acq92OGyf4g32LK2A3mWTnCvAvOL5Ki2lk25EC1X+7dBp6kzy57qDFthKnAJ4hOCxbj ZVE5UOYadxvqqMatCRRJreIkk4TlsWRC/C7ESyLTFuomvEwTeq3olt+HItR+IPIA6hOSaexUXF1 StGbwXL/ONVK021s7p294/esRjx4iwQfzWzDjROjdjGrI2Q8bU+YA07/fJ0BMS+Q5SJDrAHD1L2 fcJNGLnU7WXVsVLyVONcu81xOkVwQqnx/Tgoff9QG90TS/wxTFss0aM21YAhbo80KBR5k9Y9Bbp k/Wdad+VdZQVsRbBSKoUc3/vB0uk69GQhXSlXow0nwtVBtiQRzk= X-Received: by 2002:a17:902:ef11:b0:2be:1eb1:eaf7 with SMTP id d9443c01a7336-2be1eb1ec68mr89577775ad.24.1779258743717; Tue, 19 May 2026 23:32:23 -0700 (PDT) MIME-Version: 1.0 References: In-Reply-To: From: shveta malik Date: Wed, 20 May 2026 12:02:11 +0530 X-Gm-Features: AVHnY4KQmplciMy9SNy3Mmo9mt3AIOgJ4dsBiGRTL-1kNXifwSLLT2kT3tmOWjA Message-ID: Subject: Re: Proposal: Conflict log history table for Logical Replication To: vignesh C Cc: Nisha Moond , Dilip Kumar , Amit Kapila , 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 Tue, May 19, 2026 at 7:30=E2=80=AFPM vignesh C wro= te: > > Rest of the comments are handled, the attached v36 version patches > have the changes for the same. > Also the comment from [1] has been fixed in this version. > Thanks Vignesh. A few comments for 0001 and 002 combined (I merged them and reviewed for ease of review) 1) + * IsConflictLogTableClass + * True iff namespace is pg_conflict. + * + * Does not perform any catalog accesses. */ bool -IsConflictClass(Form_pg_class reltuple) +IsConflictLogTableClass(Form_pg_class reltuple) I think this function is trying to find if the reltuple is a CLT rather than namepspace is pg_conflict. We should change this comment. See IsToastRelation, IsToastClass. Suggestion: True iff Form_pg_class tuple represents a subscription-specific Conflict Log Table. 2) Both DropSubscription and AlterSubscription has below code to drop CLT: + if (OidIsValid(subconflictlogrelid)) + { + char *conflictrelname =3D get_rel_name(subconflictlogrelid); + + /* + * Conflict log tables are recorded as internal dependencies of the + * subscription. We must drop the dependent objects before the + * subscription itself is removed. By using + * PERFORM_DELETION_SKIP_ORIGINAL, we ensure that only the conflict log + * table is reaped while the subscription remains for the final + * deletion step. + */ + ObjectAddressSet(object, SubscriptionRelationId, subid); + performDeletion(&object, DROP_CASCADE, + PERFORM_DELETION_INTERNAL | + PERFORM_DELETION_SKIP_ORIGINAL); + + ereport(NOTICE, + errmsg("dropped conflict log table \"%s\" for subscription \"%s\"", + get_qualified_objname(PG_CONFLICT_NAMESPACE, conflictrelname), + subname)); + } Why don't we create a function drop_conflict_log_table(subconflictlogrelid) and use it both places. 3) +++ b/src/backend/commands/subscriptioncmds.c +#include "catalog/heap.h" +#include "catalog/pg_am_d.h" It compiles now without these inclusion. 002 should remove these as well. 4) AlterSubscription: + bool want_table =3D (opts.conflictlogdest =3D=3D CONFLICT_LOG_DEST_TABLE = || + opts.conflictlogdest =3D=3D CONFLICT_LOG_DEST_ALL); + bool has_oldtable =3D (old_dest =3D=3D CONFLICT_LOG_DEST_TABLE || + old_dest =3D=3D CONFLICT_LOG_DEST_ALL); Shall we replace checks at both places with CONFLICTS_LOGGED_TO_TABLE ~~ 003,004: No comments ~~ Reviewing further. thanks Shveta