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 1vvUbj-004hh9-1c for pgsql-hackers@arkaria.postgresql.org; Thu, 26 Feb 2026 06:12:19 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.96) (envelope-from ) id 1vvUbi-00AFwM-0e for pgsql-hackers@arkaria.postgresql.org; Thu, 26 Feb 2026 06:12:18 +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 1vvUbh-00AFvo-28 for pgsql-hackers@lists.postgresql.org; Thu, 26 Feb 2026 06:12:17 +0000 Received: from mail-pg1-x52a.google.com ([2607:f8b0:4864:20::52a]) by makus.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256 (Exim 4.98.2) (envelope-from ) id 1vvUbe-00000001FE1-2l6w for pgsql-hackers@lists.postgresql.org; Thu, 26 Feb 2026 06:12:16 +0000 Received: by mail-pg1-x52a.google.com with SMTP id 41be03b00d2f7-bde0f62464cso179147a12.2 for ; Wed, 25 Feb 2026 22:12:15 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1772086335; cv=none; d=google.com; s=arc-20240605; b=dN0p3hyF7r845FpwU/hFajyVMrACZ4FgBLJyphkRb+lQmWWIOxMOYRdxUwXlezl5AD eEZeYNzAoVY/P63VoOL3LBa/2gP0oUz0ij13GuAVPnjRazDpYKuOcCy4Ch+VrFGozCQ3 aAN+zke+UhQJ97vgvRTgnsPk3lySQOBjvIdW+XgrovPR4tftcPv0SlBYW4cKd9/1S33Q SPMwcrMh4V00UacrHYmXEmTjc19jQ+N/nY4U3ygTEzr/1vkG1SvwN1rOGDVg/cum+Cim CV8/ULK6oeuUoVGSzbaoxiBySRj8KcBzodBl83MdItfMNLfEACc/HVKX5iFyNPyB+RaR W38A== 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=t4w81m8a/lOsuxiqFuXOYc3Dvouw1fGQTh2aiCaxESU=; fh=rgzMmm0GkbLMeiovwA/eE4eN00cHVBwXJtZ/Vst9ojk=; b=UoVtXDoosANs/ejatnRtGIyhQKneC2f2q/0+UC/5l7YO/GbwdY7DDKtSsfL77N0eFy m4JQlOVCEUjoZSjXt8wyHKd+5tGQi7Zkyc92TfeFcFDX+nx7+/Zr6MjvA3OnjUzErE/t t8NEgkkeJD12Up63sMEgp80Z5tJwLC/CGTJUY6oxTGvIPnf759d78OvomsrSOiuOB4vW NBQGMpnd2RTxeaTymnXK36pCLg6nEwdAPIJ50DuvrGfNQiJPly5MZ0DIhP31p5bu3XRX XKb9dkkZqPlAoItZFq7zbolsptmeGOiKnVJGJoJRhoTvyEdOpjOVub2rWHIt/fRWWloq esAw==; 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=1772086335; x=1772691135; 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=t4w81m8a/lOsuxiqFuXOYc3Dvouw1fGQTh2aiCaxESU=; b=QW+NEAkOPQwV1fP5YuoRSn74ZsTBC1ULtZi2Z3/IWo0thVbfCkWvV+2Dd8IQ6xK5Jj 4D0OMrf2P9u510igSIgWfukAxTCVgRrm7AduUL4O/p10+2QTiwrOAKyEUSejE/Lez1YI 3vyoCl7hvXVdWhe6VydvrPVnBr27//YnmJ2t95Xe1yAHy4Wv0N5M21x1bztofmPRQHgQ SJUZyc939mInpamWWzcAG6ELHsZ3ppjXQhwuV24OwaSylk2Api6Rk8etJACCL1o/xKCM Xip0g47o6Ed3kvSEACVysVBG4XXeNdkzsYrpeR8iD/NtqcbLUAm8r9wNQrXX8M+obxqm yjNw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1772086335; x=1772691135; 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=t4w81m8a/lOsuxiqFuXOYc3Dvouw1fGQTh2aiCaxESU=; b=OSzs89O/i9V+FgDhLtk5kYzlOOtUFetEVbr7Zcie8FDYlwbi48tsjNLjjpJ7Eutgdn XUG70HYSqUVDxsmLwj3b9ikX7K+j5QFfTTC6G5YFk18a76FUaDO1EUASudOEqoWikPWY MP1SmL5ZMcQVB81zAOozhtiy1DUElEpPC5OqgKK8j81KMfljT1gKP8FS89dtkvYbZckS 1kcKPk8w79AkHaknpaaAVpKEDa03Wnv+SNb42MuBpseGhKXetXls5J7zcnAqJJwJwA15 ZZBUMHVLjxXFrQiWpp6B77maHTGMID/sw++EqmrP2TvuRBOYY4hfnoyoBTFLcetVU2/7 mdrw== X-Forwarded-Encrypted: i=1; AJvYcCWCOj6UbXjhaFxMjeoTvtV2L0rJrAR/TRIRzuXw7o3aqoszQDXxQ97fvQGt7AWYM6W8KCl5yk1NpcqIJ20L@lists.postgresql.org X-Gm-Message-State: AOJu0YyFC4/w4pjze7n+CaOGFm2WayzRT/5snwXwkbeCvCjb4Map6QDL O3ZGAG2y3dWBnw9H2AX9zUXX2cdzQ7X6Zj88vzIle3vE9TVPfQiyuQGQXgrwEIeYksc8Y84Bo/H hdWoZnwQvy3vjKI9HMpt1ZmOTztstQuE= X-Gm-Gg: ATEYQzyIb+4XlHMi0WqR2pxLajJhxiFsmKAoNtgMCzy18BRbbQoqkKuEh9xHSeD8PXC TwxN93g0lZgFoiLP4jjhIi8k2ZUOCQI4Kwco3XY/cNxCbbSJnhAEetYR4QAS+VkWM+ksfo4gN6k 6tH0eDCVppSTT09E8OM6RJ33NjE5mGkoiqycPC1Nr9YK5QZcHG8g1v2vpWLZICQfgt5SonuOMC5 rq38nYOtvnlj/DdsvkNcjgfGU2hshbes3GvqBg9JmQqbc4mxw84Saqw7dfy+ubPCIOEXinnCIiZ W6wpRhF58enQXQfR8JFsqAkFVI7//p7Q5IwBFSdHOWRKugfeeB0ROA== X-Received: by 2002:a05:6a21:d8c:b0:38d:fc80:3e22 with SMTP id adf61e73a8af0-395ad05d9fcmr2915643637.4.1772086334935; Wed, 25 Feb 2026 22:12:14 -0800 (PST) MIME-Version: 1.0 References: In-Reply-To: From: shveta malik Date: Thu, 26 Feb 2026 11:42:01 +0530 X-Gm-Features: AaiRm50gvSrRUfDLEladVqK2cFwPDZWPB5CJ6_S2pBQOj8JvyRQdIqsh5v26N3I Message-ID: Subject: Re: Skipping schema changes in publication To: Shlok Kyal Cc: Amit Kapila , Ashutosh Sharma , vignesh C , "David G. Johnston" , Dilip Kumar , Peter Smith , "Zhijie Hou (Fujitsu)" , YeXiu <1518981153@qq.com>, Ian Lawrence Barwick , 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, Feb 25, 2026 at 10:43=E2=80=AFPM Shlok Kyal wrote: > > Hi Shveta, Ashutosh, Chao-san, > > Thanks for reviewing the patch. > I have addressed the above comments and the comments in [1] and [2]. > Attached is the latest v50 version. > Thank You Shlok. Please find a few comments on v50-001. 1) -check_publication_add_relation(Relation targetrel) +check_publication_add_relation(PublicationRelInfo *pri, bool puballtables) Why do we need argument puballtables? I think we can give partition related error even without checking 'puballtables', like we are giving for temp and unlogged table error in EXCEPT list without checking puballtables. Or let me know if I am missing the point here? 2) publication_has_any_except_table(): Shall we optimize it: a) if it is not all-table pub, return false there itself. b) if it is all table, proceed with fetching tuples. At first tuple we can break the loop irrespective of check 'pubrel->prexcept', as there is no other feasibility. But we shall Assert (pubrel->prexcept) . Thoughts? 3) + /* Process EXCEPT table list */ + if (relations !=3D NIL) + { + Assert(rels !=3D NIL); + PublicationAddTables(puboid, rels, true, NULL); + } We can remove Assert from here too as we don't have it in the later part of code where we use rels. I feel it is okay to not have Assert as we are fetching rels in nearby logic only. 4) CheckPublicationsForExceptClauses() frees except_publications list only in case where it has to emit ERROR. It does not free it for a case where there is only one element in it. Also one of the callers free the list while another does not. Is it by design? Shall we make the behaviour more consistent for callers? 5) postgres=3D# ALTER TABLE tab_top_root ATTACH PARTITION tab_root FOR VALUES FROM (0) TO (2000); ERROR: cannot attach table "tab_root" as partition because it is referenced in a publication EXCEPT clause DETAIL: Tables excluded from publications cannot be attached as partitions= . HINT: Remove the table from the publication EXCEPT list before attaching i= t. a) It will be good to list pubnames here for which we are referring EXCEPT = list. b) Also I feel, instead of emphasising on 'cannot attach table as partition' in DETAIL, we should emphasise on 'partition cannot be part of EXCEPT'. How about? DETAIL: The publication EXCEPT clause cannot contain tables that are partit= ions. 6) I see a few error-messages mentioning EXCEPT 'clause' while others mention EXCEPT 'list'. We can make the wording the same throughout. 7) + /* + * Check that the table is not part of any publication when changing to + * UNLOGGED, as UNLOGGED tables can't be published. + */ + GetRelationPublications(RelationGetRelid(attachrel), NULL, &exceptpuboids= ); + if (exceptpuboids !=3D NIL) + ereport(ERROR, + errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE), + errmsg("cannot attach table \"%s\" as partition because it is referenced in a publication EXCEPT clause", Comments need to be corrected. Copy/paste issue. 8) tablesync.c: +#include "replication/logical.h" This inclusion is not needed now. Reviewing further. thanks Shveta