Skip to content

Commit 6294165

Browse files
ringercringerc
committed
Amendments based on review by Tomasz Rybak
* Delete dead code HookFuncName and PGLogicalTupleData * Delete stray line in pglogical_config.c * change get_param to allow found=NULL * fix copy-pasteo in errors where a stray 'endian' appeared * Make both memory contexts children of the decoding context and don't explicitly free them
1 parent 01d5990 commit 6294165

File tree

4 files changed

+33
-44
lines changed

4 files changed

+33
-44
lines changed

‎contrib/pglogical_output/expected/params_native.out

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ SELECT data FROM pg_logical_slot_get_binary_changes('regression_slot',
5454
'min_proto_version', '1',
5555
'max_proto_version', '1',
5656
'startup_params_format', '2');
57-
ERROR: client sent startup parameters in format 2 but we only support format 1
57+
ERROR: startup_params_format 2 not supported, only version 1 supported
5858
CONTEXT: slot "regression_slot", output plugin "pglogical_output", in the startup callback
5959
-- Should be OK and result in proto version 1 selection, though we won't
6060
-- see that here.

‎contrib/pglogical_output/pglogical_config.c

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -140,11 +140,11 @@ process_parameters_v1(List *options, PGLogicalOutputData *data)
140140
* delay the ERROR until after the startup reply.
141141
*/
142142
val = get_param(options, "max_proto_version", false, false,
143-
OUTPUT_PARAM_TYPE_UINT32, &found);
143+
OUTPUT_PARAM_TYPE_UINT32, NULL);
144144
data->client_max_proto_version = DatumGetUInt32(val);
145145

146146
val = get_param(options, "min_proto_version", false, false,
147-
OUTPUT_PARAM_TYPE_UINT32, &found);
147+
OUTPUT_PARAM_TYPE_UINT32, NULL);
148148
data->client_min_proto_version = DatumGetUInt32(val);
149149

150150
/* Examine all the other params in the v1 message. */
@@ -157,8 +157,6 @@ process_parameters_v1(List *options, PGLogicalOutputData *data)
157157
/* Check each param, whether or not we recognise it */
158158
switch(get_param_key(elem->defname))
159159
{
160-
val = get_param_value(elem, false, OUTPUT_PARAM_TYPE_UINT32);
161-
162160
case PARAM_BINARY_BIGENDIAN:
163161
val = get_param_value(elem, false, OUTPUT_PARAM_TYPE_BOOL);
164162
data->client_binary_bigendian_set = true;
@@ -267,18 +265,20 @@ int
267265
process_parameters(List *options, PGLogicalOutputData *data)
268266
{
269267
Datum val;
270-
bool found;
271268
int params_format;
272269

273270
val = get_param(options, "startup_params_format", false, false,
274-
OUTPUT_PARAM_TYPE_UINT32, &found);
271+
OUTPUT_PARAM_TYPE_UINT32, NULL);
275272

276273
params_format = DatumGetUInt32(val);
277274

278275
if (params_format == 1)
279-
{
280276
process_parameters_v1(options, data);
281-
}
277+
else
278+
ereport(ERROR,
279+
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
280+
errmsg("startup_params_format %d not supported, only version 1 supported",
281+
params_format)));
282282

283283
return params_format;
284284
}
@@ -326,7 +326,10 @@ get_param(List *options, const char *name, bool missing_ok, bool null_ok,
326326
{
327327
ListCell *option;
328328

329-
*found = false;
329+
if (found != NULL)
330+
*found = false;
331+
else
332+
Assert(!missing_ok);
330333

331334
foreach(option, options)
332335
{
@@ -338,7 +341,8 @@ get_param(List *options, const char *name, bool missing_ok, bool null_ok,
338341
if (pg_strcasecmp(name, elem->defname))
339342
continue;
340343

341-
*found = true;
344+
if (found != NULL)
345+
*found = true;
342346

343347
return get_param_value(elem, null_ok, type);
344348
}

‎contrib/pglogical_output/pglogical_output.c

Lines changed: 11 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -109,42 +109,42 @@ check_binary_compatibility(PGLogicalOutputData *data)
109109
if (data->client_binary_sizeofdatum != 0
110110
&& data->client_binary_sizeofdatum != sizeof(Datum))
111111
{
112-
elog(DEBUG1, "Binary mode rejected: Server and client endian sizeof(Datum) mismatch");
112+
elog(DEBUG1, "Binary mode rejected: Server and client sizeof(Datum) mismatch");
113113
return false;
114114
}
115115

116116
if (data->client_binary_sizeofint != 0
117117
&& data->client_binary_sizeofint != sizeof(int))
118118
{
119-
elog(DEBUG1, "Binary mode rejected: Server and client endian sizeof(int) mismatch");
119+
elog(DEBUG1, "Binary mode rejected: Server and client sizeof(int) mismatch");
120120
return false;
121121
}
122122

123123
if (data->client_binary_sizeoflong != 0
124124
&& data->client_binary_sizeoflong != sizeof(long))
125125
{
126-
elog(DEBUG1, "Binary mode rejected: Server and client endian sizeof(long) mismatch");
126+
elog(DEBUG1, "Binary mode rejected: Server and client sizeof(long) mismatch");
127127
return false;
128128
}
129129

130130
if (data->client_binary_float4byval_set
131131
&& data->client_binary_float4byval != server_float4_byval())
132132
{
133-
elog(DEBUG1, "Binary mode rejected: Server and client endian float4byval mismatch");
133+
elog(DEBUG1, "Binary mode rejected: Server and client float4byval mismatch");
134134
return false;
135135
}
136136

137137
if (data->client_binary_float8byval_set
138138
&& data->client_binary_float8byval != server_float8_byval())
139139
{
140-
elog(DEBUG1, "Binary mode rejected: Server and client endian float8byval mismatch");
140+
elog(DEBUG1, "Binary mode rejected: Server and client float8byval mismatch");
141141
return false;
142142
}
143143

144144
if (data->client_binary_intdatetimes_set
145145
&& data->client_binary_intdatetimes != server_integer_datetimes())
146146
{
147-
elog(DEBUG1, "Binary mode rejected: Server and client endian integer datetimes mismatch");
147+
elog(DEBUG1, "Binary mode rejected: Server and client integer datetimes mismatch");
148148
return false;
149149
}
150150

@@ -158,7 +158,7 @@ pg_decode_startup(LogicalDecodingContext * ctx, OutputPluginOptions *opt,
158158
{
159159
PGLogicalOutputData *data = palloc0(sizeof(PGLogicalOutputData));
160160

161-
data->context = AllocSetContextCreate(TopMemoryContext,
161+
data->context = AllocSetContextCreate(ctx->context,
162162
"pglogical conversion context",
163163
ALLOCSET_DEFAULT_MINSIZE,
164164
ALLOCSET_DEFAULT_INITSIZE,
@@ -561,9 +561,8 @@ static void pg_decode_shutdown(LogicalDecodingContext * ctx)
561561

562562
call_shutdown_hook(data);
563563

564-
if (data->hooks_mctxt != NULL)
565-
{
566-
MemoryContextDelete(data->hooks_mctxt);
567-
data->hooks_mctxt = NULL;
568-
}
564+
/*
565+
* no need to delete data->context or data->hooks_mctxt as they're children
566+
* of ctx->context which will expire on return.
567+
*/
569568
}

‎contrib/pglogical_output/pglogical_output.h

Lines changed: 7 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -28,23 +28,16 @@
2828
#define PGLOGICAL_OUTPUT_VERSION_NUM 10000
2929
#define PGLOGICAL_OUTPUT_VERSION "1.0.0"
3030

31-
/* Protocol capabilities */
32-
#define PGLOGICAL_PROTO_VERSION_NUM 1
33-
#define PGLOGICAL_PROTO_MIN_VERSION_NUM 1
34-
3531
/*
36-
* The name of a hook function. This is used instead of the usual List*
37-
* because can serve as a hash key.
32+
* Protocol capabilities
3833
*
39-
* Must be zeroed on allocation if used as a hash key since padding is
40-
* *not* ignored on compare.
34+
* PGLOGICAL_PROTO_VERSION_NUM is our native protocol and the greatest version
35+
* we can support. PGLOGICAL_PROTO_MIN_VERSION_NUM is the oldest version we
36+
* have backwards compatibility for. We negotiate protocol versions during the
37+
* startup handshake. See the protocol documentation for details.
4138
*/
42-
typedef struct HookFuncName
43-
{
44-
/* funcname is more likely to be unique, so goes first */
45-
char function[NAMEDATALEN];
46-
char schema[NAMEDATALEN];
47-
} HookFuncName;
39+
#define PGLOGICAL_PROTO_VERSION_NUM 1
40+
#define PGLOGICAL_PROTO_MIN_VERSION_NUM 1
4841

4942
struct PGLogicalProtoAPI;
5043

@@ -101,11 +94,4 @@ typedef struct PGLogicalOutputData
10194
List *extra_startup_params;
10295
} PGLogicalOutputData;
10396

104-
typedef struct PGLogicalTupleData
105-
{
106-
Datum values[MaxTupleAttributeNumber];
107-
bool nulls[MaxTupleAttributeNumber];
108-
bool changed[MaxTupleAttributeNumber];
109-
} PGLogicalTupleData;
110-
11197
#endif /* PG_LOGICAL_OUTPUT_H */

0 commit comments

Comments
 (0)