Skip to content

Commit

Permalink
Update client varcache with canonical values from server
Browse files Browse the repository at this point in the history
Since PG14, Postgres doesn't send ParameterStatus packets anymore when a
setting has not actually changed. This could cause the client varcache
and server varcache to become out of sync. Which then in turn resulted
in unnecessary `SET` commands to be sent to the server.

Fixes #776
  • Loading branch information
JelteF committed Jul 27, 2023
1 parent 5dcb5d0 commit 889fb5e
Show file tree
Hide file tree
Showing 4 changed files with 46 additions and 1 deletion.
1 change: 1 addition & 0 deletions include/varcache.h
Expand Up @@ -23,3 +23,4 @@ void varcache_fill_unset(VarCache *src, PgSocket *dst);
void varcache_clean(VarCache *cache);
void varcache_add_params(PktBuf *pkt, VarCache *vars);
void varcache_deinit(void);
void varcache_set_canonical(PgSocket *server, PgSocket *client);
12 changes: 12 additions & 0 deletions src/server.c
Expand Up @@ -611,6 +611,18 @@ bool server_proto(SBuf *sbuf, SBufEvent evtype, struct MBuf *data)
PgSocket *client = server->link;
Assert(client);

/*
* It's possible that the client vars and server vars have
* different string representations, but still Postgres did not
* send a ParamaterStatus packet. This happens when the server
* variable is the canonical version of the client variable, i.e.
* they mean the same just written slightly different. To make sure
* that the canonical version is also stored in the client, we now
* copy the server variables over to the client variables.
* See issue #776 for an example of this.
*/
varcache_set_canonical(server, client);

server->setting_vars = false;
sbuf_continue(&client->sbuf);
break;
Expand Down
17 changes: 17 additions & 0 deletions src/varcache.c
Expand Up @@ -238,6 +238,23 @@ bool varcache_apply(PgSocket *server, PgSocket *client, bool *changes_p)
return pktbuf_send_immediate(pkt, server);
}

void varcache_set_canonical(PgSocket *server, PgSocket *client)
{
struct PStr *server_val, *client_val;
const struct var_lookup *lk;
for (lk = lookup; lk->name; lk++) {
server_val = server->vars.var_list[lk->idx];
client_val = client->vars.var_list[lk->idx];
if (client_val && server_val && client_val != server_val) {
slog_debug(client, "varcache_set_canonical: setting %s to its canonical version %s -> %s",
lk->name, client_val->str, server_val->str);
strpool_incref(server_val);
strpool_decref(client_val);
client->vars.var_list[lk->idx] = server_val;
}
}
}

void varcache_fill_unset(VarCache *src, PgSocket *dst)
{
struct PStr *srcval, *dstval;
Expand Down
17 changes: 16 additions & 1 deletion test/test_misc.py
Expand Up @@ -5,7 +5,7 @@
import psycopg
import pytest

from .utils import HAVE_IPV6_LOCALHOST, WINDOWS
from .utils import HAVE_IPV6_LOCALHOST, PG_MAJOR_VERSION, WINDOWS


def test_connect_query(bouncer):
Expand Down Expand Up @@ -203,3 +203,18 @@ def test_options_startup_param(bouncer):
match="unsupported startup parameter in options: enable_seqscan",
):
bouncer.test(options="-c enable_seqscan=true")


def test_equivalent_startup_param(bouncer):
bouncer.admin("set verbose=2")

canonical_expected_times = 1 if PG_MAJOR_VERSION >= 14 else 0
with bouncer.cur(options="-c DateStyle=ISO") as cur:
with bouncer.log_contains(
"varcache_apply: .*SET DateStyle='ISO'", times=1
), bouncer.log_contains(
"varcache_set_canonical: setting DateStyle to its canonical version ISO -> ISO, MDY",
times=canonical_expected_times,
):
cur.execute("SELECT 1")
cur.execute("SELECT 1")

0 comments on commit 889fb5e

Please sign in to comment.