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 1vVkUi-0071eF-2B for pgsql-hackers@arkaria.postgresql.org; Wed, 17 Dec 2025 05:54:41 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.96) (envelope-from ) id 1vVkUh-00AUK6-23 for pgsql-hackers@arkaria.postgresql.org; Wed, 17 Dec 2025 05:54:40 +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 1vVkUh-00AUJy-12 for pgsql-hackers@lists.postgresql.org; Wed, 17 Dec 2025 05:54:40 +0000 Received: from mail-pj1-x1031.google.com ([2607:f8b0:4864:20::1031]) by magus.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256 (Exim 4.96) (envelope-from ) id 1vVkUf-001AvA-2t for pgsql-hackers@lists.postgresql.org; Wed, 17 Dec 2025 05:54:39 +0000 Received: by mail-pj1-x1031.google.com with SMTP id 98e67ed59e1d1-34c3259da34so3990343a91.2 for ; Tue, 16 Dec 2025 21:54:37 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1765950876; x=1766555676; 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=FvNxh1uRFewpVwvxzS+IW/c8P+smAttnLLU0AN82buY=; b=XVud+W/7Mv3G7oU5f8uDTOXlFnN5oHON8nBahJriLyTpvtThMV+UswYKKx+/UVhpBb FkpMOX7REs4nD7EjJtyTN2C8zBHPYYBvIvbBmn746k8EahXYOkINZR+6xr2ZVRPs5KXA E2yN0j5BuFD2Ei+Y0NTvjDv+d1+XZCc6/PgKxyQxA1N+P2P22dCv+DEjBguxEmMZbbut WJKOJZprfUiI5fakgp/9JTiQ2cqpuUv/vIPyxsjfokvggT+xFzdkESSDV7WdPi1Xjfgb uwlc0DX6+I25Pu15SI1Ut5b3bkqYcF23szPLGs19glJsGP+3gtaS9hhpJ1i+VbabsjcV zhOQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1765950876; x=1766555676; 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=FvNxh1uRFewpVwvxzS+IW/c8P+smAttnLLU0AN82buY=; b=eU7j2RNj5VUxk6azyBYP1dQl6wKGU9coAMnfNwA5TEPZJxFLE6FhZXOHilCA45qHsg 1xYDzBAFIhA/rkFsd/TNUSYqFbj4J1++IgdG7IHz8Ldo3Wxw7L1Pf6KwaPlU2BzjfNS+ cTuxvW7s5kpyVuhsP0dPYevLHddtw+f5huk8H79DRm1G2kqtSn6ze4ZmldImFZHV9ZVJ O/zZXVFvdkkgS9bsjGF8fxaGQaJ8ZRRuIvyqtPHm0ElSqEk04weDTxlSYMIFFoIZ/2YL caP98RxoNQarL4RTs+siqP7UATTU4PvFzuFNog9NOi0OErTdbSKWQWttGcocql3Mm8aa tjAw== X-Forwarded-Encrypted: i=1; AJvYcCWwyZdb1ptErjwqlsg1PAPuu8CF61rdEJJ4MoYmmO3Oon3eGT51iSivC+2g6jEqQLUXaDhssm+cf8HbXCEu@lists.postgresql.org X-Gm-Message-State: AOJu0YzGyCr9WxUiOonvYDnh+e5hXXEtakWLiK0wvAG2S+H/nSd21iYp ygWuJ3MePiEsv4CcBxpKe1lZKVU94biO9S/8JXBUpFse6i5YU3HsSzW5B1Ha4/YRUHdxSKOlzWB 2DaeYreTbm6ymyD1P+shso99+Fp/H7PQ= X-Gm-Gg: AY/fxX5U9dOhCx/ayCiCbrw1i6uK2kD700K4qP1dfXD6T4t3GC7OZv/FO9dilG7Z6uj uFpEpViokzBZ4F3sg7iokyW03hUL/23IqSDibLtNnDaqupxMfxKtQbFoT1sYiLnwlvfFz5icQYM KqTuVH1JmoF6nvLXSdvE45DLGV9Ri8SCsYFn9p0v7bWHujQjefuS24BSXp85tqq4HvMKrLRot7r J/9LWs+Ku1vdD315gyEcxT0HDsPDzvvW3gT9kOP1Rs+HxWMXGomjfaUOfVO1wHBHqwKnVpx/hM9 xPf+pZu/2KYRLmFeqbW3q+s+ajJl8JKH2U0JxhB4Ow== X-Google-Smtp-Source: AGHT+IHRx58xO+eSOTa9iBVyxWvZNedCToqrWVRGXqOzsbWHdiuo1Qqr6Xj48MQOhXO7mp9R2i9xQWGL/Yqry1RJL4o= X-Received: by 2002:a17:90b:2d83:b0:34a:aa7b:1af8 with SMTP id 98e67ed59e1d1-34abe4a2b9emr12182944a91.32.1765950875995; Tue, 16 Dec 2025 21:54:35 -0800 (PST) MIME-Version: 1.0 References: In-Reply-To: From: shveta malik Date: Wed, 17 Dec 2025 11:24:23 +0530 X-Gm-Features: AQt7F2oST-RBxWyUjKdBtZDZvhrvtLI7uRBh9Xj1cojm9NoU3KH1kVyzGGrNx7s Message-ID: Subject: Re: Skipping schema changes in publication To: Shlok Kyal Cc: Peter Smith , Amit Kapila , vignesh C , "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 Tue, Dec 16, 2025 at 2:50=E2=80=AFPM Shlok Kyal wrote: > > I have also addressed the remaining comments and attached the latest patc= h. > Thanks. A few comments: 1) + if (!set_top && puballtables) + set_top =3D !list_member_oid(aexceptpubids, puboid); In GetTopMostAncestorInPublication(), we have made the above change which will now get ancestor from all-tables publication as well, provided table is not part of 'except' List. Earlier this function was only checking pg_subscription_rel and pg_publication_namespace which does not include all-tables publication. Won't it change the result-set for callers? 2) + * Publications declared with FOR ALL TABLES or FOR ALL SEQUENCES should u= se + * GetAllPublicationRelations() to obtain the complete set of tables cover= ed by + * the publication. + */ +List * +GetPublicationIncludedRelations(Oid pubid, PublicationPartOpt pub_partopt) +{ + return GetPublicationRelationsInternal(pubid, pub_partopt, false); +} We can have an Assert here that pubid passed is not for ALL-Tables or ALL-sequences 3) GetAllPublicationRelations: * If the publication publishes partition changes via their respective root * partitioned tables, we must exclude partitions in favor of including the * root partitioned tables. This is not applicable to FOR ALL SEQUENCES * publication. + * The list does not include relations that are explicitly excluded via th= e + * EXCEPT TABLE clause of the publication specified by pubid. Suggestion: /* * If the publication publishes partition changes via their respective root * partitioned tables, we must exclude partitions in favor of including the * root partitioned tables. The list also excludes tables that are * explicitly excluded via the EXCEPT TABLE clause of the publication * identified by pubid. Neither of these rules applies to FOR ALL SEQUENCES * publications. */ 4) GetAllPublicationRelations: + if (relkind =3D=3D RELKIND_RELATION) + exceptlist =3D GetPublicationExcludedRelations(pubid, pubviaroot ? + PUBLICATION_PART_ALL : + PUBLICATION_PART_ROOT); Assert(!(relkind =3D=3D RELKIND_SEQUENCE && pubviaroot)); Generally we keep such parameters' sanity checks as the first step. We can add new code after Assert. 5) ObjectsInAllPublicationToOids() only has one caller which calls it under check: 'if (stmt->for_all_tables)' Thus IMO, we do not need a switch-case in ObjectsInAllPublicationToOids(). We can simply have a sanity check to see it is 'PUBLICATION_ALL_TABLES' and then do the needed operation for this pub-type. 6) CreatePublication(): /* * If publication is for ALL TABLES and relations is not empty, it means * that there are some relations to be excluded from the publication. * Else, relations is the list of relations to be added to the * publication. */ Shall we rephrase slightly to: /* * If the publication is for ALL TABLES and 'relations' is not empty, * it indicates that some relations should be excluded from the publication= . * Add those excluded relations to the publication with 'prexcept' set to t= rue. * Otherwise, 'relations' contains the list of relations to be explicitly * included in the publication. */ 7) + /* Associate objects with the publication. */ + if (stmt->for_all_tables) + { + /* Invalidate relcache so that publication info is rebuilt. */ + CacheInvalidateRelcacheAll(); + } I think this comment is misplaced. We shall have it at previous place, atop= : if (stmt->for_all_tables) This is because here we are just trying to invalidate cache while at previous place we are trying to associate. thanks Shveta