Re: Added schema level support for publication. - Mailing list pgsql-hackers

On 11/1/21 11:18, Amit Kapila wrote:
> On Mon, Nov 1, 2021 at 2:48 AM Tomas Vondra
> <tomas.vondra@enterprisedb.com> wrote:
>>
>> 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?
>>
> 
> Yeah, it should be PUBLICATIONOBJ_TABLE_IN_SCHEMA considering we have
> to add other objects like sequence.
> 
>> 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 think we should name it PUBLICATIONOBJ_TABLE_CURRSCHEMA. Does that make sense?
> 
>> I wonder if it'd be better to just separate the schema and object type
>> specification, instead of mashing it into a single constant.
>>
> 
> Do you mean to say the syntax on the lines of Create Publication For
> Table t1, t2 Schema s1, s2;? If so, then originally the patch had the
> syntax on those lines but Tom pointed out that the meaning of such a
> syntax can change over a period of time and that can break apps [1]. I
> think the current syntax gives a lot of flexibility to users and we
> have some precedent for it as well.
> 

No, I'm not talking about the syntax at all - I'm talking about how we 
represent it. PUBLICATIONOBJ_TABLE_CURRSCHEMA mixes the object type and 
schema in the same constant, so I am wondering if we should just split 
that into two pieces - one determining the schema, one determining the 
object type. So PublicationObjSpec would have two fields instead of just 
pubobjtype.

The advantage would be we wouldn't need a whole lot of new constants for 
each object type - adding sequences pretty much means adding

     PUBLICATIONOBJ_SEQUENCE
     PUBLICATIONOBJ_SEQUENCE_IN_SCHEMA
     PUBLICATIONOBJ_SEQUENCE_CURRSCHEMA

and after splitting we'd need just the first one. But maybe it's not 
that bad, though. We don't expect all that many object types in 
publications, I guess.


regards

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