tembo-io / pgmq

A lightweight message queue. Like AWS SQS and RSMQ but on Postgres.
PostgreSQL License
2.68k stars 71 forks source link

`pgmq.drop_queue` issues when queue doesn't exist #275

Open CGenie opened 4 months ago

CGenie commented 4 months ago

Hello,

We have this code (https://github.com/tembo-io/pgmq/blob/main/pgmq-extension/sql/pgmq.sql#L429) for dropping a queue:

ALTER EXTENSION pgmq DROP TABLE pgmq.q_%s;

ALTER EXTENSION pgmq DROP TABLE pgmq.a_%s;

DROP TABLE IF EXISTS pgmq.q_%s;

DROP TABLE IF EXISTS pgmq.a_%s;

Currently, when I drop a non-existing queue, the first statement fails with an error:

ERROR:  relation "pgmq.q_test" does not exist
CONTEXT:  SQL statement "
        ALTER EXTENSION pgmq DROP TABLE pgmq.q_test
        "

From PostgreSQL docs on DROP member_object:

This form removes a member object from the extension. This is mainly useful in extension update scripts. The object is not dropped, only disassociated from the extension.

We're in a user-called function here, not in an update script. Is there a point of altering the extension itself with queue/archive tables? Please look at 2 other SQL statements: we are careful here not to throw an error using the IF EXISTS. There is no IF EXISTS counterpart for ALTER EXTENSION ... DROP TABLE so maybe we just get rid of that ALTER EXTENSION altogether?

In fact, this causes even more problems. For example:

postgres=# select pgmq.create('test');
 create
--------

(1 row)

postgres=# select pgmq.detach_archive('test');
 detach_archive
----------------

(1 row)

postgres=# select pgmq.drop_queue('test');
ERROR:  table pgmq.a_test is not a member of extension "pgmq"
CONTEXT:  SQL statement "
        ALTER EXTENSION pgmq DROP TABLE pgmq.a_test
        "
PL/pgSQL function pgmq.drop_queue(text,boolean) line 10 at EXECUTE

pgmq is supposed to be used in highly concurrent environments - I think the less exceptions like that are thrown, the easier it is to maintain the software that uses it.

ChuckHend commented 4 months ago

Hi @CGenie, thanks for raising the issue. The API around pgmq.detach_archive() is pretty rough at the moment. The reason it exists is to make it slightly more convenient to implement partitioning on an existing queue archive (creating new partitioned table, renaming old, etc.). The issue you showed above feels like a bug and I will need to think through how we can handle this more gracefully.

There is some ongoing work related to partitioning the archive tables that @shhnwz is handling. I think we can address this during that work.

CGenie commented 4 months ago

Well, my overall idea was to get rid of critical errors. I don't use detach_archive, just gave it as an example.

But I think one shouldn't throw errors in drop_queue, at least the IF EXISTS statements give me the clue that this was the initial intention.

ChuckHend commented 3 months ago

I agree. I think the same problem exists on pgmq.create() right now too.