From: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
---|---|
To: | pgsql-hackers(at)lists(dot)postgresql(dot)org |
Subject: | Bogus duplicate command issued in pg_dump |
Date: | 2022-01-23 18:31:03 |
Message-ID: | 1714711.1642962663@sss.pgh.pa.us |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
While investigating for 353708e1f, I happened to notice that pg_dump's
binary_upgrade_set_type_oids_by_type_oid() contains
PQExpBuffer upgrade_query = createPQExpBuffer();
...
appendPQExpBuffer(upgrade_query,
"SELECT typarray "
...
res = ExecuteSqlQueryForSingleRow(fout, upgrade_query->data);
...
appendPQExpBuffer(upgrade_query,
"SELECT t.oid, t.typarray "
...
res = ExecuteSqlQueryForSingleRow(fout, upgrade_query->data);
How's that work? It turns out that the second ExecuteSqlQueryForSingleRow
is sending a string like "SELECT typarray ...;SELECT t.oid, t.typarray ..."
which the backend happily interprets as multiple commands. It sends
back multiple query results, and then PQexec discards all but the last
one. So, very accidentally, there's no observable bug, just some wasted
cycles.
I will go fix this, but I wondered if there are any other similar
errors, or what we might do to prevent the same type of mistake
in future. I experimented with replacing most of pg_dump's PQexec
calls with PQexecParams, as in the attached quick hack (NOT meant
for commit). That did not turn up any additional cases, but of
course I have limited faith in the code coverage of check-world.
We could consider a more global change to get rid of using
appendPQExpBuffer where it's not absolutely necessary, so that
there are fewer bad examples to copy. Another idea is to deem
it an anti-pattern to end a query with a semicolon. But I don't
have a lot of faith in people following those coding rules in
future, either. It'd also be a lot of code churn for what is
in the end a relatively harmless bug.
Thoughts?
regards, tom lane
Attachment | Content-Type | Size |
---|---|---|
hacky-check-for-bogus-pg_dump-queries.patch | text/x-diff | 4.3 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Tom Lane | 2022-01-23 18:35:22 | Re: PSA: Autoconf has risen from the dead |
Previous Message | Joel Jacobson | 2022-01-23 18:13:41 | Re: PSA: Autoconf has risen from the dead |