Comments:
* In standard_ProcessUtility(), case NotifyStmt, you add a comment:
/* XXX the new listen/notify version can be enabled
* for Hot Standby */
but I don't think that's correct. We may be able to support LISTEN
on the standby, but not NOTIFY (right?). I don't think we should
be adding speculative comments anyway, because there is some work
still needed before HS can support LISTEN (notably, WAL-logging
NOTIFY).
* You have a TODO list as a comment. Can you remove it and explain
those items on list if they aren't already?
* You have the comment:
/*
* How long can a payload string possibly be? Actually it needs
to be one
* byte less to provide space for the trailing terminating '\0'.
*/
That should be written more simply, like "Maximum size of the
payload, including terminating NULL."
* You have the Assert:
Assert(strlen(n->payload) <= NOTIFY_PAYLOAD_MAX_LENGTH);
which is inconsistent with the earlier test:
if (stmt->payload
&& strlen(stmt->payload) > NOTIFY_PAYLOAD_MAX_LENGTH - 1)
ereport(ERROR, ...
* ASCII-only is still an open issue.
* 2PC is still an open issue (notifications are lost on restart,
and there may be other problems, as well). I think it's easy enough
to throw an error on PREPARE TRANSACTION if there are any
notifications, right?
* There's a bug where an UNLISTEN can abort, and yet you still miss
the notification. This is because you unlisten before you actually
commit the transaction, and an error between those times will cause
the UNLISTEN to take effect even though the rest of the transaction
fails. For example:
-- session 1
LISTEN foo;
BEGIN;
UNLISTEN foo;
-- session 2
NOTIFY foo;
-- gdb in session 1
(gdb) break AtCommit_NotifyBeforeCommit
(gdb) c
-- session 1
COMMIT;
-- gdb in session 1
(gdb) finish
(gdb) p op_strict(7654322)
(gdb) quit
The notification is missed. It's fixed easily enough by doing the
UNLISTEN step in AtCommit_NotifyAfterCommit.
I'm still looking through some of the queue stuff, and I'll send an
update soon. I wanted to give you some feedback now though.
Regards,
Jeff Davis
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
