Re: [PATCH] Implement (and document, and test) has_sequence_privilege()

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

In response to:
http://archives.postgresql.org/message-id/1238394513-21474-1-git-send-email-ams@oryx.com

Review complete. Somewhat modified patch attached. I wasn't happy with
creation of verify_sequence_oid() when it more-or-less duplicates the
functionality of get_rel_relkind(). Also, the original patch was
misleading in that the following comment:

+ * The result is a boolean value: true if user has the indicated
+ * privilege, false if not. The variants that take a relation OID
+ * return NULL if the OID doesn't exist (rather than failing, as
+ * they did before Postgres 8.4).

implies that the relname variants should throw an ERROR for non-existing
names (which they in fact do), but the code for these functions reads, e.g.:

+Datum
+has_sequence_privilege_name_name(PG_FUNCTION_ARGS)
+{
+ Name rolename = PG_GETARG_NAME(0);
+ text *sequencename = PG_GETARG_TEXT_P(1);
<snip>
+ sequenceoid = convert_table_name(sequencename);
+ if (!verify_sequence_oid(sequenceoid))
+ PG_RETURN_NULL();

Here, verify_sequence_oid() is never called for a non-existing relname,
because the call to convert_table_name() will throw an ERROR first. And
verify_sequence_oid() will throw an ERROR if the relation is not a
sequence. Therefore verify_sequence_oid() will only return if TRUE, and
PG_RETURN_NULL() is never reached. But if it were able to be reached, it
would be in conflict with the comment.

If there are no objections, I'll commit in a day or two.

Joe
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org

iQIcBAEBCAAGBQJKdOwTAAoJEDfy90M199hlFpwP/0merKdY+czyozrFuEdiGsPA
Ai7twjRNyNNiJlWfr7hU+RxsXlNxST7lxCI792b/ZKJe8Z1gx4u7qYlddKVCMgFn
Qeqvo0sC2N2FUMWMuBOPpDCl2sR6N2YtCxHep1NrAxDK58nupeSZ9vZ69ywwWA8e
xFhbTabJDSIECbN+FISfaR60LIrTd6dlV18maPOkWmDgM3Ff11GagtB9ngZngmdY
3eN/D6wOMqtAQOcCZNyTIf68rLvKrMDu1U+bses3JUZE0NcMa76H1jm2BE3KNY1A
tp+5fXEuazk+NEzBND6eSksUOnemqCt+MsyC4eim2bGQX61+tmm8tKHjCt0CYucO
ERTeOhX7vJcZILzzrU287SC880spGXUz74ivApLG7SNquZ8qr1YkdYLHJdKt4SYh
pJc1T+b1ngYK1/SWWzoUzEiZa+3dBReRXXmXv2IP59RSaCzYMgQ+Ohqt6D8cTHnS
gSdV2csd0nae2vRuSSOms4yviLzPzh2t8GxVo7eDHre8/kHsOw0eMPPVYeA6nE/l
v5r42raBr++WT/VSS0/3nOGOCAZoYcPGCv7R6Cbz7QkIOTNXbZ9PA2IUaQ2znNcQ
9nF0/aRmgPOuJUtubxMN8Qs5UE5g7HqDU7EaJ3NfL5jWCrYC2f1HWnwxS17xszCs
ropug1p5344+3GZRLQtS
=1T1A
-----END PGP SIGNATURE-----

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

Joe Conway wrote:
> If there are no objections, I'll commit in a day or two.

Attached version committed. Only difference from the last is catversion.

Joe
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org

iQIcBAEBCAAGBQJKd1MXAAoJEDfy90M199hlz6IP/RvNrU3CQ0wUvipvR3sJi3wV
Bis3TqPiSzGlKHDkV67Mh+sR0oQLJ2UG5yEgmSPM8yus+zd6rRzSIpS1Z+npE1sg
CKJtYTzDjVby9szhKk9znn37dY/dyIOzW7S9AXCjwF1bM+60Pyavue3J8br3TnU7
yCCVH1wow4D7Pg0pO/kKEiOLAO+Qx9NUA5AGdUh47c0NLvz7XbmhkrG2Kt1KKrha
2qVgKUEqsi8Q6vahx5qczCM6TuYx33WFfwCLWT0HB0igtfuEp5Pp8D0C/5sRNer/
J10JqdWp/XPEv69rLkLNkdHns0pA+J0uPJLZkEcwi38a7YThwSzraDSw73gslcRX
UdAlSe06tOuxUky4IlHNFZZnzaAaXwGNjJARmjKK+PtMozatfO0lUKU5QCeQhRx1
QgtlDC4MQuFcVsdDy5jFWLviP0PiMtiWtkKhAp9e0FZa5CHQQby5AuTKNHSfS3To
XhE+abZfbwFTQFrGFGKJnNWPepbLnl1gm2sm6id175m/8FKXNQo++jr0SasqkEzA
d52C6/WI7Ll6Zlo9smnc3ogt9ggnQ866AOdy1SXpkEDjFdGDsMxgxjI8rObNxYEL
ew9VAgFI5lwnABKlRv7y2xIMxOEgdO6gduURJhQ2U20yhq1W8QlsjE+IE7UYURBL
VXj6LlvceXdDFF2uUjMz
=gAqc
-----END PGP SIGNATURE-----