Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit79b716c

Browse files
author
Amit Kapila
committed
Reorder subskiplsn in pg_subscription to avoid alignment issues.
The column 'subskiplsn' uses TYPALIGN_DOUBLE (which has 4 bytes alignmenton AIX) for storage. But the C Struct (Form_pg_subscription) has 8-bytealignment for this field, so retrieving it from storage causes anunaligned read.To fix this, we rearranged the 'subskiplsn' column in the catalog so thatit naturally comes at an 8-byte boundary.We have fixed a similar problem in commitf3b421d. This patch adds atest to avoid a similar mistake in the future.Reported-by: Noah MischDiagnosed-by: Noah Misch, Masahiko Sawada, Amit KapilaAuthor: Masahiko SawadaReviewed-by: Noah Misch, Amit KapilaDiscussion:https://postgr.es/m/20220401074423.GC3682158@rfd.leadboat.comhttps://postgr.es/m/CAD21AoDeScrsHhLyEPYqN3sydg6PxAPVBboK=30xJfUVihNZDA@mail.gmail.com
1 parente41aed6 commit79b716c

File tree

11 files changed

+130
-20
lines changed

11 files changed

+130
-20
lines changed

‎doc/src/sgml/catalogs.sgml

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -7823,6 +7823,16 @@ SCRAM-SHA-256$<replaceable>&lt;iteration count&gt;</replaceable>:<replaceable>&l
78237823
</para></entry>
78247824
</row>
78257825

7826+
<row>
7827+
<entry role="catalog_table_entry"><para role="column_definition">
7828+
<structfield>subskiplsn</structfield> <type>pg_lsn</type>
7829+
</para>
7830+
<para>
7831+
Finish LSN of the transaction whose changes are to be skipped, if a valid
7832+
LSN; otherwise <literal>0/0</literal>.
7833+
</para></entry>
7834+
</row>
7835+
78267836
<row>
78277837
<entry role="catalog_table_entry"><para role="column_definition">
78287838
<structfield>subname</structfield> <type>name</type>
@@ -7893,16 +7903,6 @@ SCRAM-SHA-256$<replaceable>&lt;iteration count&gt;</replaceable>:<replaceable>&l
78937903
</para></entry>
78947904
</row>
78957905

7896-
<row>
7897-
<entry role="catalog_table_entry"><para role="column_definition">
7898-
<structfield>subskiplsn</structfield> <type>pg_lsn</type>
7899-
</para>
7900-
<para>
7901-
Finish LSN of the transaction whose changes are to be skipped, if a valid
7902-
LSN; otherwise <literal>0/0</literal>.
7903-
</para></entry>
7904-
</row>
7905-
79067906
<row>
79077907
<entry role="catalog_table_entry"><para role="column_definition">
79087908
<structfield>subconninfo</structfield> <type>text</type>

‎src/backend/catalog/pg_subscription.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,14 +63,14 @@ GetSubscription(Oid subid, bool missing_ok)
6363
sub= (Subscription*)palloc(sizeof(Subscription));
6464
sub->oid=subid;
6565
sub->dbid=subform->subdbid;
66+
sub->skiplsn=subform->subskiplsn;
6667
sub->name=pstrdup(NameStr(subform->subname));
6768
sub->owner=subform->subowner;
6869
sub->enabled=subform->subenabled;
6970
sub->binary=subform->subbinary;
7071
sub->stream=subform->substream;
7172
sub->twophasestate=subform->subtwophasestate;
7273
sub->disableonerr=subform->subdisableonerr;
73-
sub->skiplsn=subform->subskiplsn;
7474

7575
/* Get conninfo */
7676
datum=SysCacheGetAttr(SUBSCRIPTIONOID,

‎src/backend/catalog/system_views.sql

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1285,8 +1285,8 @@ REVOKE ALL ON pg_replication_origin_status FROM public;
12851285

12861286
-- All columns of pg_subscription except subconninfo are publicly readable.
12871287
REVOKE ALLON pg_subscriptionFROM public;
1288-
GRANTSELECT (oid, subdbid, subname, subowner, subenabled, subbinary,
1289-
substream, subtwophasestate, subdisableonerr, subskiplsn, subslotname,
1288+
GRANTSELECT (oid, subdbid,subskiplsn,subname, subowner, subenabled,
1289+
subbinary,substream, subtwophasestate, subdisableonerr, subslotname,
12901290
subsynccommit, subpublications)
12911291
ON pg_subscription TO public;
12921292

‎src/backend/commands/subscriptioncmds.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -596,6 +596,7 @@ CreateSubscription(ParseState *pstate, CreateSubscriptionStmt *stmt,
596596
Anum_pg_subscription_oid);
597597
values[Anum_pg_subscription_oid-1]=ObjectIdGetDatum(subid);
598598
values[Anum_pg_subscription_subdbid-1]=ObjectIdGetDatum(MyDatabaseId);
599+
values[Anum_pg_subscription_subskiplsn-1]=LSNGetDatum(InvalidXLogRecPtr);
599600
values[Anum_pg_subscription_subname-1]=
600601
DirectFunctionCall1(namein,CStringGetDatum(stmt->subname));
601602
values[Anum_pg_subscription_subowner-1]=ObjectIdGetDatum(owner);
@@ -607,7 +608,6 @@ CreateSubscription(ParseState *pstate, CreateSubscriptionStmt *stmt,
607608
LOGICALREP_TWOPHASE_STATE_PENDING :
608609
LOGICALREP_TWOPHASE_STATE_DISABLED);
609610
values[Anum_pg_subscription_subdisableonerr-1]=BoolGetDatum(opts.disableonerr);
610-
values[Anum_pg_subscription_subskiplsn-1]=LSNGetDatum(InvalidXLogRecPtr);
611611
values[Anum_pg_subscription_subconninfo-1]=
612612
CStringGetTextDatum(conninfo);
613613
if (opts.slot_name)

‎src/include/catalog/catversion.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,6 @@
5353
*/
5454

5555
/*yyyymmddN */
56-
#defineCATALOG_VERSION_NO202204062
56+
#defineCATALOG_VERSION_NO202204071
5757

5858
#endif

‎src/include/catalog/pg_subscription.h

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,10 @@ CATALOG(pg_subscription,6100,SubscriptionRelationId) BKI_SHARED_RELATION BKI_ROW
5454

5555
OidsubdbidBKI_LOOKUP(pg_database);/* Database the
5656
* subscription is in. */
57+
58+
XLogRecPtrsubskiplsn;/* All changes finished at this LSN are
59+
* skipped */
60+
5761
NameDatasubname;/* Name of the subscription */
5862

5963
OidsubownerBKI_LOOKUP(pg_authid);/* Owner of the subscription */
@@ -71,9 +75,6 @@ CATALOG(pg_subscription,6100,SubscriptionRelationId) BKI_SHARED_RELATION BKI_ROW
7175
boolsubdisableonerr;/* True if a worker error should cause the
7276
* subscription to be disabled */
7377

74-
XLogRecPtrsubskiplsn;/* All changes finished at this LSN are
75-
* skipped */
76-
7778
#ifdefCATALOG_VARLEN/* variable-length fields start here */
7879
/* Connection string to the publisher */
7980
textsubconninfoBKI_FORCE_NOT_NULL;
@@ -103,6 +104,8 @@ typedef struct Subscription
103104
Oidoid;/* Oid of the subscription */
104105
Oiddbid;/* Oid of the database which subscription is
105106
* in */
107+
XLogRecPtrskiplsn;/* All changes finished at this LSN are
108+
* skipped */
106109
char*name;/* Name of the subscription */
107110
Oidowner;/* Oid of the subscription owner */
108111
boolenabled;/* Indicates if the subscription is enabled */
@@ -113,8 +116,6 @@ typedef struct Subscription
113116
booldisableonerr;/* Indicates if the subscription should be
114117
* automatically disabled if a worker error
115118
* occurs */
116-
XLogRecPtrskiplsn;/* All changes finished at this LSN are
117-
* skipped */
118119
char*conninfo;/* Connection string to the publisher */
119120
char*slotname;/* Name of the replication slot */
120121
char*synccommit;/* Synchronous commit setting for worker */

‎src/test/regress/expected/sanity_check.out

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,3 +25,32 @@ SELECT relname, relkind
2525
---------+---------
2626
(0 rows)
2727

28+
--
29+
-- When ALIGNOF_DOUBLE==4 (e.g. AIX), the C ABI may impose 8-byte alignment on
30+
-- some of the C types that correspond to TYPALIGN_DOUBLE SQL types. To ensure
31+
-- catalog C struct layout matches catalog tuple layout, arrange for the tuple
32+
-- offset of each fixed-width, attalign='d' catalog column to be divisible by 8
33+
-- unconditionally. Keep such columns before the first NameData column of the
34+
-- catalog, since packagers can override NAMEDATALEN to an odd number.
35+
--
36+
WITH check_columns AS (
37+
SELECT relname, attname,
38+
array(
39+
SELECT t.oid
40+
FROM pg_type t JOIN pg_attribute pa ON t.oid = pa.atttypid
41+
WHERE pa.attrelid = a.attrelid AND
42+
pa.attnum > 0 AND pa.attnum <= a.attnum
43+
ORDER BY pa.attnum) AS coltypes
44+
FROM pg_attribute a JOIN pg_class c ON c.oid = attrelid
45+
JOIN pg_namespace n ON c.relnamespace = n.oid
46+
WHERE attalign = 'd' AND relkind = 'r' AND
47+
attnotnull AND attlen <> -1 AND n.nspname = 'pg_catalog'
48+
)
49+
SELECT relname, attname, coltypes, get_column_offset(coltypes)
50+
FROM check_columns
51+
WHERE get_column_offset(coltypes) % 8 != 0 OR
52+
'name'::regtype::oid = ANY(coltypes);
53+
relname | attname | coltypes | get_column_offset
54+
---------+---------+----------+-------------------
55+
(0 rows)
56+

‎src/test/regress/expected/test_setup.out

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -206,6 +206,10 @@ CREATE FUNCTION ttdummy ()
206206
RETURNS trigger
207207
AS :'regresslib'
208208
LANGUAGE C;
209+
CREATE FUNCTION get_column_offset (oid[])
210+
RETURNS int
211+
AS :'regresslib'
212+
LANGUAGE C STRICT STABLE PARALLEL SAFE;
209213
-- Use hand-rolled hash functions and operator classes to get predictable
210214
-- result on different machines. The hash function for int4 simply returns
211215
-- the sum of the values passed to it and the one for text returns the length

‎src/test/regress/regress.c

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@
4141
#include"storage/spin.h"
4242
#include"utils/builtins.h"
4343
#include"utils/geo_decls.h"
44+
#include"utils/lsyscache.h"
4445
#include"utils/memutils.h"
4546
#include"utils/rel.h"
4647
#include"utils/typcache.h"
@@ -1216,3 +1217,47 @@ binary_coercible(PG_FUNCTION_ARGS)
12161217

12171218
PG_RETURN_BOOL(IsBinaryCoercible(srctype,targettype));
12181219
}
1220+
1221+
/*
1222+
* Return the column offset of the last data in the given array of
1223+
* data types. The input data types must be fixed-length data types.
1224+
*/
1225+
PG_FUNCTION_INFO_V1(get_column_offset);
1226+
Datum
1227+
get_column_offset(PG_FUNCTION_ARGS)
1228+
{
1229+
ArrayType*ta=PG_GETARG_ARRAYTYPE_P(0);
1230+
Oid*type_oids;
1231+
intntypes;
1232+
intcolumn_offset=0;
1233+
1234+
if (ARR_HASNULL(ta)&&array_contains_nulls(ta))
1235+
elog(ERROR,"argument must not contain nulls");
1236+
1237+
if (ARR_NDIM(ta)>1)
1238+
elog(ERROR,"argument must be empty or one-dimensional array");
1239+
1240+
type_oids= (Oid*)ARR_DATA_PTR(ta);
1241+
ntypes=ArrayGetNItems(ARR_NDIM(ta),ARR_DIMS(ta));
1242+
for (inti=0;i<ntypes;i++)
1243+
{
1244+
Oidtypeoid=type_oids[i];
1245+
int16typlen;
1246+
booltypbyval;
1247+
chartypalign;
1248+
1249+
get_typlenbyvalalign(typeoid,&typlen,&typbyval,&typalign);
1250+
1251+
/* the data type must be fixed-length */
1252+
if (!(typbyval|| (typlen>0)))
1253+
elog(ERROR,"type %u is not fixed-length data type",typeoid);
1254+
1255+
column_offset=att_align_nominal(column_offset,typalign);
1256+
1257+
/* not include the last type size */
1258+
if (i!= (ntypes-1))
1259+
column_offset+=typlen;
1260+
}
1261+
1262+
PG_RETURN_INT32(column_offset);
1263+
}

‎src/test/regress/sql/sanity_check.sql

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,3 +19,29 @@ SELECT relname, relkind
1919
FROM pg_class
2020
WHERE relkindIN ('v','c','f','p','I')
2121
AND relfilenode<>0;
22+
23+
--
24+
-- When ALIGNOF_DOUBLE==4 (e.g. AIX), the C ABI may impose 8-byte alignment on
25+
-- some of the C types that correspond to TYPALIGN_DOUBLE SQL types. To ensure
26+
-- catalog C struct layout matches catalog tuple layout, arrange for the tuple
27+
-- offset of each fixed-width, attalign='d' catalog column to be divisible by 8
28+
-- unconditionally. Keep such columns before the first NameData column of the
29+
-- catalog, since packagers can override NAMEDATALEN to an odd number.
30+
--
31+
WITH check_columnsAS (
32+
SELECT relname, attname,
33+
array(
34+
SELECTt.oid
35+
FROM pg_type tJOIN pg_attribute paONt.oid=pa.atttypid
36+
WHEREpa.attrelid=a.attrelidAND
37+
pa.attnum>0ANDpa.attnum<=a.attnum
38+
ORDER BYpa.attnum)AS coltypes
39+
FROM pg_attribute aJOIN pg_class cONc.oid= attrelid
40+
JOIN pg_namespace nONc.relnamespace=n.oid
41+
WHERE attalign='d'AND relkind='r'AND
42+
attnotnullAND attlen<>-1ANDn.nspname='pg_catalog'
43+
)
44+
SELECT relname, attname, coltypes, get_column_offset(coltypes)
45+
FROM check_columns
46+
WHERE get_column_offset(coltypes) %8!=0OR
47+
'name'::regtype::oid= ANY(coltypes);

‎src/test/regress/sql/test_setup.sql

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -253,6 +253,11 @@ CREATE FUNCTION ttdummy ()
253253
AS :'regresslib'
254254
LANGUAGE C;
255255

256+
CREATEFUNCTIONget_column_offset (oid[])
257+
RETURNSint
258+
AS :'regresslib'
259+
LANGUAGE C STRICT STABLE PARALLEL SAFE;
260+
256261
-- Use hand-rolled hash functions and operator classes to get predictable
257262
-- result on different machines. The hash function for int4 simply returns
258263
-- the sum of the values passed to it and the one for text returns the length

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp