Message-ID: <1df84daa-7d0d-e8cc-4762-85523e45e5e7@mailbox.org>
Date: Sun, 14 Jul 2024 17:39:22 +0200
MIME-Version: 1.0
Content-Language: en-US
To: pgsql-bugs@lists.postgresql.org
From: Vasilii Smirnov <vasilii.smirnov@mailbox.org>
Subject: libpq: unexpected return code from PQexecParams with a DO INSTEAD
 rule present
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Archived-At: 
 <https://www.postgresql.org/message-id/1df84daa-7d0d-e8cc-4762-85523e45e5e7%40mailbox.org>
Precedence: bulk

Let's say I have a table "_users", and also a view "users" that excludes
all soft-deleted records from that table:

> CREATE SCHEMA test_libpq;
> CREATE TABLE test_libpq._users (
>   id SERIAL PRIMARY KEY,
>   name VARCHAR(255) UNIQUE,
>   deleted_at TIMESTAMP
> );
> 
> CREATE VIEW test_libpq.users AS
> SELECT * FROM test_libpq._users
> WHERE deleted_at IS NULL;
Let's also add a DO INSTEAD rule that translates DELETE statements on
the view into soft-delete UPDATE statements:

> CREATE RULE soft_delete AS
> ON DELETE TO test_libpq.users DO INSTEAD (
>   UPDATE test_libpq._users SET deleted_at = NOW()
>   WHERE id = OLD.id AND deleted_at IS NULL
>   RETURNING *
> );

Cool, now if I go into psql command line and do a few inserts and
deletes, everything works as intended:

> postgres=# INSERT INTO test_libpq.users (name) VALUES ('Joe');
> INSERT 0 1
> postgres=# DELETE FROM test_libpq.users WHERE name = 'Joe' RETURNING id;
>  id 
> ----
>   1
> (1 row)
> 
> DELETE 0
> postgres=# SELECT * FROM test_libpq.users;
>  id | name | deleted_at 
> ----+------+------------
> (0 rows)
> 
> postgres=# SELECT * FROM test_libpq._users;
>  id | name |         deleted_at         
> ----+------+----------------------------
>   1 | Joe  | 2024-07-14 14:55:05.585433
> (1 row)

The problems begin when I translate that into C code using libpq. That
DELETE statement should return with a status code PGRES_TUPLES_OK and I
should be able to read a list of deleted ids. And indeed it works like
that with PQexec(). But not PQexecParams()!

With PQexecParams() it instead returns PGRES_COMMAND_OK and doesn't
return the list of deleted ids. This only happens with that DO INSTEAD
rule present, without it the function works as expected. But obviously
does a hard delete instead of a soft one.

This behavior isn't documented. 34.3.1. Main Functions [1] says this
about PQexecParams:

> PQexecParams is like PQexec, but offers additional
> functionality: parameter values can be specified separately from
> the command string proper, and query results can be requested in
> either text or binary format.
> [...]
> Unlike PQexec, PQexecParams allows at most one SQL command in
> the given string. (There can be semicolons in it, but not more
> than one nonempty command.) This is a limitation of the
> underlying protocol, but has some usefulness as an extra defense
> against SQL-injection attacks.
[1]: https://www.postgresql.org/docs/16/libpq-exec.html#LIBPQ-PQEXECPARAMS

The manual mentions some limitations in the protocol regarding
semicolons, but nothing about rules.

Here is a complete C++ program to reproduce the issue:
https://paste.sr.ht/~uh/80d023b772bcfe8c1eda8f6b69b5b4e2c0352dc1
(also pasted at the end of the email). I tested it with a postgresql
instance launched like this:

> docker run -it --rm -p 5432:5432 -e POSTGRES_PASSWORD=postgres postgres:16.3
I also noticed that this same problem affects another unofficial
postgresql library, but for Go: https://github.com/lib/pq. So this
problem is not local to libpq, but affects other clients as well.

The C++ program that I linked to above:
> #include <cstdio>
> #include <cstdlib>
> #include <cstring>
> #include <functional>
> #include <iostream>
> #include <libpq-fe.h>
> #include <sstream>
> #include <stdexcept>
> 
> #define TEST(EXPR)                                  \
>   try {                                             \
>     EXPR;                                           \
>     std::cout << "PASSED: " #EXPR "\n";             \
>   } catch (const std::runtime_error& e) {           \
>     std::cout << "FAILED: " #EXPR "\n" << e.what(); \
>     exit(EXIT_FAILURE);                             \
>   }
> 
> void assertRes(PGconn* conn, PGresult* res, ExecStatusType expected) {
>   auto status = PQresultStatus(res);
>   if (status != expected) {
>     auto err = std::stringstream()
>       << "query failed with status " << PQresStatus(status)
>       << "\nbut expected: " << PQresStatus(expected)
>       << "\nmessage: " << PQerrorMessage(conn);
>     throw std::runtime_error(err.str());
>   }
> }
> 
> PGconn* setup() {
>   PGconn* conn = PQconnectdb("postgresql://postgres:postgres@localhost:5432/postgres");
>   if (PQstatus(conn) != CONNECTION_OK) {
>     fprintf(stderr, "connection error: %s\n", PQerrorMessage(conn));
>     exit(EXIT_FAILURE);
>   }
> 
>   const char* query = R"(
>     DROP SCHEMA IF EXISTS test_libpq CASCADE;
>     CREATE SCHEMA test_libpq;
> 
>     CREATE TABLE test_libpq._users (
>       id SERIAL PRIMARY KEY,
>       name VARCHAR(255) UNIQUE,
>       deleted_at TIMESTAMP
>     );
> 
>     INSERT INTO test_libpq._users
>       (name)
>     VALUES
>       ('a user');
> 
>     -- A view that only contains non-deleted users:
>     CREATE VIEW test_libpq.users AS
>     SELECT * FROM test_libpq._users
>     WHERE deleted_at IS NULL;
> 
>     -- DELETE on this view does a soft delete instead:
>     CREATE RULE soft_delete AS
>     ON DELETE TO test_libpq.users DO INSTEAD (
>       UPDATE test_libpq._users SET deleted_at = NOW()
>       WHERE id = OLD.id AND deleted_at IS NULL
>       RETURNING *
>     );
>   )";
> 
>   PGresult* res = PQexec(conn, query);
>   assertRes(conn, res, PGRES_COMMAND_OK);
>   PQclear(res);
> 
>   return conn;
> }
> 
> static const char* softDelete = R"(
>   DELETE FROM test_libpq.users
>   RETURNING id
> )";
> 
> static const char* hardDelete = R"(
>   DELETE FROM test_libpq._users
>   RETURNING id
> )";
> 
> using execFn = std::function<PGresult*(PGconn* conn, const char* query)>;
> 
> void testDelete(execFn execQuery, const char* query) {
>   PGconn* conn = setup();
>   PGresult* res = execQuery(conn, query);
>   assertRes(conn, res, PGRES_TUPLES_OK);
>   PQclear(res);
>   PQfinish(conn);
> }
> 
> PGresult* usingPQExec(PGconn* conn, const char* query) {
>   return PQexec(conn, query); 
> }
> 
> PGresult* usingPQExecParams(PGconn* conn, const char* query) {
>   return PQexecParams(conn, query,
>                       0, nullptr, nullptr,
>                       nullptr, nullptr, 0);
> }
> 
> int main() {
>   TEST(testDelete(usingPQExec,       hardDelete));
>   TEST(testDelete(usingPQExecParams, hardDelete));
>   TEST(testDelete(usingPQExec,       softDelete));
>   TEST(testDelete(usingPQExecParams, softDelete)); // this fails
> }
Cheers!