Re: Skipping logical replication transactions on subscriber side - Mailing list pgsql-hackers

On Tue, Apr 5, 2022 at 9:21 AM Noah Misch <noah@leadboat.com> wrote:
>
> On Mon, Apr 04, 2022 at 06:55:45PM +0900, Masahiko Sawada wrote:
> > On Mon, Apr 4, 2022 at 3:26 PM Noah Misch <noah@leadboat.com> wrote:
> > > On Mon, Apr 04, 2022 at 08:20:08AM +0530, Amit Kapila wrote:
> > > > How about a comment like: "It has to be kept at 8-byte alignment
> > > > boundary so as to be accessed directly via C struct as it uses
> > > > TYPALIGN_DOUBLE for storage which has 4-byte alignment on platforms
> > > > like AIX."? Can you please suggest a better comment if you don't like
> > > > this one?
> > >
> > > I'd write it like this, though I'm not sure it's an improvement on your words:
> > >
> > >   When ALIGNOF_DOUBLE==4 (e.g. AIX), the C ABI may impose 8-byte alignment on
> > >   some of the C types that correspond to TYPALIGN_DOUBLE SQL types.  To ensure
> > >   catalog C struct layout matches catalog tuple layout, arrange for the tuple
> > >   offset of each fixed-width, attalign='d' catalog column to be divisible by 8
> > >   unconditionally.  Keep such columns before the first NameData column of the
> > >   catalog, since packagers can override NAMEDATALEN to an odd number.
> >
> > Thanks!
> >
> > >
> > > The best place for such a comment would be in one of
> > > src/test/regress/sql/*sanity*.sql, next to a test written to detect new
> > > violations.
> >
> > Agreed.
> >
> > IIUC in the new test, we would need a new SQL function to calculate
> > the offset of catalog columns including padding, is that right? Or do
> > you have an idea to do that by using existing functionality?
>
> Something like this:
>
> select
>   attrelid::regclass,
>   attname,
>   array(select typname
>         from pg_type t join pg_attribute pa on t.oid = pa.atttypid
>         where pa.attrelid = a.attrelid and pa.attnum > 0 and pa.attnum < a.attnum order by pa.attnum) AS
types_before,
>   (select sum(attlen)
>    from pg_type t join pg_attribute pa on t.oid = pa.atttypid
>    where pa.attrelid = a.attrelid and pa.attnum > 0 and pa.attnum < a.attnum) AS len_before
> from pg_attribute a
> join pg_class c on c.oid = attrelid
> where attalign = 'd' and relkind = 'r' and attnotnull and attlen <> -1
> order by attrelid::regclass::text, attnum;
>     attrelid     │   attname    │                types_before                 │ len_before
> ─────────────────┼──────────────┼─────────────────────────────────────────────┼────────────
>  pg_sequence     │ seqstart     │ {oid,oid}                                   │          8
>  pg_sequence     │ seqincrement │ {oid,oid,int8}                              │         16
>  pg_sequence     │ seqmax       │ {oid,oid,int8,int8}                         │         24
>  pg_sequence     │ seqmin       │ {oid,oid,int8,int8,int8}                    │         32
>  pg_sequence     │ seqcache     │ {oid,oid,int8,int8,int8,int8}               │         40
>  pg_subscription │ subskiplsn   │ {oid,oid,name,oid,bool,bool,bool,char,bool} │         81
> (6 rows)
>
> That doesn't count padding, but hazardous column changes will cause a diff in
> the output.

Yes, in this case, we can detect the violated column order even
without considering padding. On the other hand, I think this
calculation could not detect some patterns of order. For instance,
suppose the column order is {oid, bool, bool, oid, bool, bool, oid,
int8}, the len_before is 16 but offset of int8 column including
padding is 20 on ALIGNOF_DOUBLE==4 environment.

Regards,

--
Masahiko Sawada
EDB:  https://www.enterprisedb.com/