Re: Added schema level support for publication.

On 11/1/21 11:18, Amit Kapila wrote:
> On Mon, Nov 1, 2021 at 2:48 AM Tomas Vondra
> <tomas(dot)vondra(at)enterprisedb(dot)com> wrote:
>>
>> On 10/28/21 04:41, Amit Kapila wrote:
>>> On Mon, Oct 25, 2021 at 3:09 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>>>>
>>>> On Mon, Oct 25, 2021 at 1:11 PM vignesh C <vignesh21(at)gmail(dot)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