[prev in list] [next in list] [prev in thread] [next in thread] 

List:       pgsql-hackers
Subject:    Re: Added schema level support for publication.
From:       Tomas Vondra <tomas.vondra () enterprisedb ! com>
Date:       2021-10-31 21:18:44
Message-ID: 17355aed-98ce-c85a-47f2-13c550a60266 () enterprisedb ! com
[Download RAW message or body]

Hi,

On 10/28/21 04:41, Amit Kapila wrote:
> On Mon, Oct 25, 2021 at 3:09 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
>>
>> On Mon, Oct 25, 2021 at 1:11 PM vignesh C <vignesh21@gmail.com> wrote:
>>>
>>> I have fixed this in the v47 version attached.
>>>
>>
>> Thanks, the first patch in the series "Allow publishing the tables of
>> schema." looks good to me. Unless there are more
>> comments/bugs/objections, I am planning to commit it in a day or so.
>>
> 
> Yesterday, I have pushed the first patch. Feel free to submit the
> remaining patches.
> 

I haven't been following this thread recently, but while rebasing the 
sequence decoding patch I noticed this adds

     PUBLICATIONOBJ_TABLE,            /* Table type */
     PUBLICATIONOBJ_REL_IN_SCHEMA,    /* Relations in schema type */

Shouldn't it be PUBLICATIONOBJ_TABLE_IN_SCHEMA, or why does it use rel 
instead of table?

I'm asking because the sequence decoding patch mimics ALTER PUBLICATION 
options for sequences, including ALL SEQUENCES IN SCHEMA etc. and this 
seems ambiguous. The same issue applies to PUBLICATIONOBJ_CURRSCHEMA, 
which does not specify the object type.

I wonder if it'd be better to just separate the schema and object type 
specification, instead of mashing it into a single constant. Otherwise 
we'll end up with (M x N) combinations, which seems silly.


regards

-- 
Tomas Vondra
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic