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 1vlKej-00A5Ts-1l for pgsql-hackers@arkaria.postgresql.org; Thu, 29 Jan 2026 05:33:26 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.96) (envelope-from ) id 1vlKeg-006XEN-39 for pgsql-hackers@arkaria.postgresql.org; Thu, 29 Jan 2026 05:33:23 +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 1vlKeg-006XDE-22 for pgsql-hackers@lists.postgresql.org; Thu, 29 Jan 2026 05:33:23 +0000 Received: from mail-pj1-x1032.google.com ([2607:f8b0:4864:20::1032]) by magus.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256 (Exim 4.98.2) (envelope-from ) id 1vlKee-000000010gY-1vvP for pgsql-hackers@lists.postgresql.org; Thu, 29 Jan 2026 05:33:22 +0000 Received: by mail-pj1-x1032.google.com with SMTP id 98e67ed59e1d1-35305538592so466419a91.0 for ; Wed, 28 Jan 2026 21:33:20 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1769664797; cv=none; d=google.com; s=arc-20240605; b=I8rKNhdluGYZ4kqRjweo4IhypIhU/i4a/WZfqmzrISqjNVzpjXu+kwLPPdsg9F+TmR 6TSniBjOZkehZoHVBotfsP7L0O76jMzefpc7GJY0F1ecXMtl7ZRHk/2AsMLO3QFxY2+P H5EssvkdS5xwnWTmZK35p9Y4MXv2gnRrfIUuerYZru6ta1ZUrLuROTmhqfxNbZ35Roef oh2IkGHvU1n3NKQRf+qTBQEQnGeZMYWLL5rQi2RmoiXoelRYufSfWcZuhWgATAEWrA25 QhwIKmyvhtGCa5bXg5FwcBoS+LegrkaWXgYbRMk7KLJFFFpTestT4INtnK01Ff6mZlG4 KsEQ== 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=43hQiJEo3PufkR8J26Y4mgMtxjBV2v5XJYb2IP1HiVs=; fh=aeOdqUKhLt+YwGfBWlkevEBMUZhahoDSjRViplxt24Y=; b=dGqjVaXa+gW/1yf8E7pZJPEEPWUtpH/dLxfRVITiLAr0VkEfo8rYAcxBPsBx+ub+XW g2u6KS/6GX7Yylb8XbGn57S5vbC8uytUj3DWEijMxsbftyTjS3jgzVLz/zuUkSgpjlyB TEQ02MoXKVsyaWbmNlsVYSlcjFQw+EFjyhaU/U+EWqGgIwEvKsPrmAqlcFGsy0HE9WuU wqME2mD0IYgBm5BZT3QQHlcQmh3BDCFwuQYVZXg0QFcAsvzEbZmB+Xg2WsBsiMQiKsnD 2jf1u7NDLt+YPxkmSDqrvFoZxgb1dZty49NFcDK3bAAzAWEOJo8ywGMCntBDKKBL9ri0 aghQ==; 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=20230601; t=1769664797; x=1770269597; 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=43hQiJEo3PufkR8J26Y4mgMtxjBV2v5XJYb2IP1HiVs=; b=ZGKGJuWWqq+2ZEWH1aBUN1u7i+ty28unUH/o/yQQRgtqxP6R6ZV5NwXwILf9NIGH06 oouxTBaFRyD0ee38MqeNX2s7xHpJtmaGFQw9uuVQshBTYrHH07VFMBnr2SVqxZAdMayA lFtCZ2f98Y4e5N+mA6FrQ1+Pl9FMrRJIyteREjSKT02Zds1Nl1gI50NYsRDKZxUGX+PT ylj4L3+8+Bnq3jZjYvsvSqoyQydSnvb//Bu2zBHvUa8xKnwRmOPqJHZkzaz7I9k9W64w /tJtZSRpq/oTzubEgc//7cR7ugCBflgOJJwSp0LpCxiNmCyHVoBATHeKLUh/wsLOoOQR 7jEw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1769664797; x=1770269597; 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=43hQiJEo3PufkR8J26Y4mgMtxjBV2v5XJYb2IP1HiVs=; b=GhLJrw91/nGHw9HfOKvkosGuWjzGNpl5w9fFFz+UED6Xr7elKBblJyTKdU1g7n1rev GK2AHu4xYQyo4NNU+1W8NMxy/Mp1KCC68gil6+wBQZRBx2Dg2PX/bf3d8X5YYYtYCUMI klmD9o+SSiZ9CtZN3g5iZ2mstfcHsJbmvifygnjj2h1+1faJl8nZaj8xdVhs4241kdS/ qBvRhFM5sWxRPGa4FHHWtGTb037FXhKrO6uvD/6/7xIQd3A+TeFez25gGyc+RtJlWVMD nY2cP43BsjdZ8MPuTRgfopI8xIETuA6S1vY6iJzZQJtX2GAnP5XVkhhDZQzcW9ymj1oL pG/w== X-Forwarded-Encrypted: i=1; AJvYcCUb37NJRMSV0GV4OJUG2LcHdkg1yHWc4YgeG8XxN8J1gLZN6Ujo+9TqxP/YIO+kvUtKThdp38NCudLgDz8P@lists.postgresql.org X-Gm-Message-State: AOJu0Yz2lCxLxFRzjzZlOTRQoRwLojV7cXqGFJEPHx/msDTRLy/glljY KDq7i7ZuwPx2E4UpCRzMswg9766B6VC7+EBH8S+Z39SNh3CSm5RA9TnCs3Idzae2+v2MBjVz4vG c6MdElTmklAduTld9xxOkEyhF/mDviQE= X-Gm-Gg: AZuq6aLwOIy6d3yaszR+SmYX//Q076AbOw5TMg5WNag7+WZcG9hBblhsu1y8MWUYuy+ I8/uXl6+eLdzrRHIk1vXHf8rISBTdh5gBu5TjIS7PYzOac+yp6OBu0eDPHX2CgYDI1RE6sVb9Mw Bth9+gG3slH5JJcef1YjKKtrjF12SvzNnLKsrZEwFp7P5cya7fXoOfoq8Cg9cTHiNLPkbftPhyD VueA4g/OViuyLcJIKc3/u8pCQ921tC/9G/6nlJzsE3X23Q4+Xh6F4qKV38Ga3VYaZOROORbDa5Y RmBqLCGExNGmgQPfZAlM9nc1fDfVN68VZJq+kVNwrULV60q+VCb/FwLT7UDFP2jczqF8W1EY X-Received: by 2002:a17:90b:3d02:b0:340:d569:d295 with SMTP id 98e67ed59e1d1-353fed5cad0mr7025116a91.24.1769664797421; Wed, 28 Jan 2026 21:33:17 -0800 (PST) MIME-Version: 1.0 References: In-Reply-To: From: shveta malik Date: Thu, 29 Jan 2026 11:03:05 +0530 X-Gm-Features: AZwV_QgSOG3_lO5PW2j9Mrbe_FcFHKYoVvIq8cbSLbicPhr0zXjPRUO6cfMwirs Message-ID: Subject: Re: Proposal: Conflict log history table for Logical Replication To: Dilip Kumar Cc: Peter Smith , vignesh C , Amit Kapila , 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 Wed, Jan 28, 2026 at 5:36=E2=80=AFPM shveta malik wrote: > > On Wed, Jan 28, 2026 at 5:33=E2=80=AFPM shveta malik wrote: > > > > > > 4) > > +# Verify that '2' is present inside the JSON structure using a regex > > +# This matches the key/value pattern for "a": 2 > > +like($raw_json, qr/\\"a\\":2/, 'Verified that key 2 exists in the > > local_conflicts'); > > + > > Oops, I missed adding my feedback for pt4 earlier, here it is: > > To properly validate correctness, given that local_conflicts is an > array of multiple local tuple details here, we should also check that > it contains the keys b:3 and c:4. > Few more comments on 002: 5) + bool log_dest_clt; + bool log_dest_logfile; Do we need 'clt' in the variable name? It would be clearer to name it log_dest_table. Having said that, neither of these names clearly indicate a boolean value. Will it be better to name these as logToTable and logToLogFile? 6) +/* + * Helper function to extract the "raw" index key Datums and their null fl= ags + * from a TupleTableSlot, given an already open index descriptor. + * This is the reusable core logic. + */ +static void +build_index_datums_from_slot(EState *estate, Relation localrel, + TupleTableSlot *slot, + Relation indexDesc, Datum *values, + bool *isnull) We should mention which ones are output variables here. Something like: Helper function to extract...to values[] and isnull[] arrays. 7) In build_index_datums_from_slot(), shall we check/assert that 'values' and 'isnull' are non-null before passing it to FormIndexDatum. 8) +static TupleDesc +build_conflict_tupledesc(void) The name gives an impression that we are building a full tuple-descriptor for CLT, while it is only for local_conflicts. Shall we name it to build_tupledesc_for_local_conflicts or build_local_conflicts_tupledesc. It aligns with its caller as well 'build_local_conflicts_json_array'. 9) build_local_conflicts_json_array(): + int i; + i =3D 0; + foreach(lc, json_datums) + { + json_datum_array[i] =3D (Datum) lfirst(lc); + i++; + } I think variable 'i' and its usage is leftover from some debugging. 10) There are few new functions such as tuple_table_slot_to_json_datum, tuple_table_slot_to_indextup_json, build_conflict_tupledesc, prepare_conflict_log_tuple which mention function name in header-comment section as well, while other functions do not. We can keep header-comment section of all functions in the same format. I think we generally do not mention function names explicitly in the header. thanks Shveta