From: | Ajin Cherian <itsajin(at)gmail(dot)com> |
---|---|
To: | vignesh C <vignesh21(at)gmail(dot)com> |
Cc: | "tanghy(dot)fnst(at)fujitsu(dot)com" <tanghy(dot)fnst(at)fujitsu(dot)com>, Rahila Syed <rahilasyed90(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
Subject: | Re: Added schema level support for publication. |
Date: | 2021-06-18 11:54:48 |
Message-ID: | CAFPTHDayhZrjmN6qe_5MeYNBns7nJPxxmcw_vFmTPxN-4X8R7Q@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Thu, Jun 17, 2021 at 12:41 AM vignesh C <vignesh21(at)gmail(dot)com> wrote:
> Thanks for reporting it, the attached patch is a rebased version of
> the patch with few review comment fixes, I will reply with the comment
> fixes to the respective mails.
>
I've applied the patch, it applies cleand and ran "make check" and
tests run fine.
Some comments for patch 1:
In the commit message, some grammar mistakes:
"Changes was done in
pg_dump to handle pubtype updation in pg_publication table while the database
gets upgraded."
-------------- change to --
Changes were done in
pg_dump to handle pubtype updation in pg_publication table while the database
gets upgraded.
=============
Prototypes present in pg_publication.h was moved to publicationcmds.h so
that minimal datastructures ...
----------------- change to --
Prototypes present in pg_publication.h were moved to publicationcmds.h so
that minimal datastructures ..
========================
In patch 1:
In getObjectDescription()
+ if (!nspname)
+ {
+ pfree(pubname);
+ pfree(nspname);
+ ReleaseSysCache(tup);
Why free nspname if it is NULL?
Same comment in getObjectIdentityParts()
============================
In GetAllTablesPublicationRelations()
+ ScanKeyData key[2];
TableScanDesc scan;
HeapTuple tuple;
List *result = NIL;
+ int keycount = 0;
classRel = table_open(RelationRelationId, AccessShareLock);
- ScanKeyInit(&key[0],
+ ScanKeyInit(&key[keycount++],
Here you have init key[1], but the code below in the same function
inits key[0]. I am not sure if this will affect that code below.
if (pubviaroot)
{
ScanKeyInit(&key[0],
Anum_pg_class_relkind,
BTEqualStrategyNumber, F_CHAREQ,
CharGetDatum(RELKIND_PARTITIONED_TABLE));
=================================
in UpdatePublicationTypeTupleValue():
+ tup = heap_modify_tuple(tup, RelationGetDescr(rel), values, nulls,
+ replaces);
+
+ /* Update the catalog. */
+ CatalogTupleUpdate(rel, &tup->t_self, tup);
Not sure if this tup needs to be freed or if the memory context will
take care of it.
=====================
regards,
Ajin Cherian
Fujitsu Australia
From | Date | Subject | |
---|---|---|---|
Next Message | Andrey Borodin | 2021-06-18 12:02:15 | Supply restore_command to pg_rewind via CLI argument |
Previous Message | Amit Kapila | 2021-06-18 11:40:38 | Re: row filtering for logical replication |