On Tue, Apr 13, 2021 at 6:27 AM Justin Pryzby <pryzby@telsasoft.com> wrote:
>
> On Sun, Apr 11, 2021 at 03:45:36PM +0530, Bharath Rupireddy wrote:
> > On Sun, Apr 11, 2021 at 9:47 AM Justin Pryzby <pryzby@telsasoft.com> wrote:
> > > Also, you currently test:
> > > > + if (extra & TRUNCATE_REL_CONTEXT_ONLY)
> > >
> > > but TRUNCATE_REL_ aren't indepedent bits, so shouldn't be tested with "&".
> >
> > Yeah this is an issue. We could just change the #defines to values
> > 0x0001, 0x0002, 0x0004, 0x0008 ... 0x0020 and so on and then testing
> > with & would work. So, this way, more than option can be multiplexed
> > into the same int value. To multiplex, we need to think: will there be
> > a scenario where a single rel in the truncate can have multiple
> > options at a time and do we want to distinguish these options while
> > deparsing?
> >
> > #define TRUNCATE_REL_CONTEXT_NORMAL 0x0001 /* specified without
> > ONLY clause */
> > #define TRUNCATE_REL_CONTEXT_ONLY 0x0002 /* specified with
> > ONLY clause */
> > #define TRUNCATE_REL_CONTEXT_CASCADING 0x0004 /* not specified
> > but truncated
> >
> > And I'm not sure what's the agreement on retaining or removing #define
> > values, currently I see only TRUNCATE_REL_CONTEXT_ONLY is being used,
> > others are just being set but not used. As I said upthread, it will be
> > good to remove the unused macros/enums, retain only the ones that are
> > used, especially TRUNCATE_REL_CONTEXT_CASCADING this is not required I
> > feel, because we can add the child partitions that are foreign tables
> > to relids as just normal foreign tables with TRUNCATE_REL_CONTEXT_ONLY
> > option.
>
> Converting to "bits" would collapse TRUNCATE_REL_CONTEXT_ONLY and
> TRUNCATE_REL_CONTEXT_NORMAL into a single bit. TRUNCATE_REL_CONTEXT_CASCADING
> could optionally be removed.
>
> +1 to convert to bits instead of changing "&" to "==".
Thanks for the patches.
I agree to convert to bits and pass it as int value which is
extensible i.e. we can pass more extra parameters across if required.
Also I'm not in favour of removing relids_extra altogether, we might
need this to send some info in future.
Both 0001 and 0002(along with the new phrasings) look good to me.
With Regards,
Bharath Rupireddy.
EnterpriseDB: http://www.enterprisedb.com