On 11/1/21 11:18, Amit Kapila wrote:
On Mon, Nov 1, 2021 at 2:48 AM Tomas Vondra
<tomas.von...@enterprisedb.com> wrote:

On 10/28/21 04:41, Amit Kapila wrote:
On Mon, Oct 25, 2021 at 3:09 PM Amit Kapila <amit.kapil...@gmail.com> wrote:

On Mon, Oct 25, 2021 at 1:11 PM vignesh C <vignes...@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


Reply via email to