Re: async notification patch for dblink

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

In reference to:
http://archives.postgresql.org/message-id/6534f7ae0811181547v1dc1f096g6222e8273b461606@mail.gmail.com

Had to fix a lot of bit rot, but otherwise looks good. My updated patch
attached. Will commit in a day or so if no objections.

BTW, some commitfest procedural comments/questions:

1) I couldn't figure out how to attach my patch to the commitfest page
short of cut-n-paste into the small comment text box -- is that my only
choice?

2) It would be nice if the mail archive web page of the original message
had a "Reply To" button. Otherwise I guess I can do what I've done above.

3) I couldn't see any way to assign myself as the committer.

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

iQIcBAEBCAAGBQJKd5K8AAoJEDfy90M199hlDbIQAIYeLR1B3JKkXvoBCZUkzVNj
hkj67d38ygU/Q4RDsnPT0N96PbPvzBJGCueaCLfwzbdpEQb4Si37R8xBMXPzT/df
SEoWrx/2r1/cERyImxHVGlj0kBWBa7K42hcEotD0mNqWqX6rqByLDpVTSlpZZdfM
a/b04iXfPcIvOLSpX6PJOb047SeHQrzOmcnurBqc1HE81XiiBNQlJjOd0Zi+y+pT
zGJChGSRO1lXriOI1Pu2K18daqY8vLLZA0LlaZ3SD6UeS7Uayeo+8cOpAk6N1b7H
EKqYAwWfmRrSO6bUCmgkqlHa/TiaIL0uJb0me68YcUy08DAt1O8iRmUnAE+cxXU5
HeZgGqJI2G19Ts5i9R2YaHVe8kTmPD88zztv8giiAw01m9h6azkiM342CzNuyQTw
8TbZnjWdCkb4KuH9en4C/puIWUpCOd2OkVju3ZUJCvzaO/jS/HVf2fc08K9TddPK
OWpsytXCNS6ojM/Em5lZzfsZZ9sDcrP29dSHZEJqOIkMhFoEGqq4DVqtJnLMHsLw
3PtSfgz0SbXcuAA0vv/VJSDm+cAl+aJN93hbwNybwrT14XEotllFdhyJrSPqFgYm
4b7P29dFKP+aF90C6klxr5Mq/nRYiJVdTepqjfyikVBsdZoMwQXvG+ROiwbzjl9g
eEmht8ysuxn2Ju8FcDYc
=AJfk
-----END PGP SIGNATURE-----

Joe Conway <mail(at)joeconway(dot)com> writes:
> BTW, some commitfest procedural comments/questions:

> 1) I couldn't figure out how to attach my patch to the commitfest page
> short of cut-n-paste into the small comment text box -- is that my only
> choice?

No, what you should do is first send the patch to -hackers, then put the
message-ID of that email into the place provided. The comment box isn't
really meant for much more than a one-line summary of the message being
linked to.

> 3) I couldn't see any way to assign myself as the committer.

Yeah, the webapp doesn't explicitly record the committer for a patch.
What I've been doing is adding a comment saying that I'm taking a patch
to commit. A separate field would probably be better though.

regards, tom lane

Joe Conway escribió:

> 2) It would be nice if the mail archive web page of the original message
> had a "Reply To" button. Otherwise I guess I can do what I've done above.

I totally agree, but this is not workable given our current software.
I've been giving some time to reworking the email archives using
something completely different to allow this kind of trick, but this is
to be considered longish-term.

--
Alvaro Herrera http://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.

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

Alvaro Herrera wrote:
> Joe Conway escribió:
>
>> 2) It would be nice if the mail archive web page of the original message
>> had a "Reply To" button. Otherwise I guess I can do what I've done above.
>
> I totally agree, but this is not workable given our current software.
> I've been giving some time to reworking the email archives using
> something completely different to allow this kind of trick, but this is
> to be considered longish-term.

OK, good to know it isn't just me ;-)

Thanks,

Joe

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

iQIcBAEBCAAGBQJKd5zzAAoJEDfy90M199hlCuoP/0ZDDBSjShDeZXf5ReMEl/cw
7W5N0J78HJoM9Jrq587i/iqc//hU35V9Jq2GxHXbwYo+ByWd86KCwmhoY7fEDMTZ
bQKd26ZPmTdxME/JXF+R/6XWa5cDhzOyaMNsaJJkSrT1CvJTk6H10R5JF8ibrz2H
gcAPFYprd7IYGzXamq8cwOnsu3pTPBXDu6Z7HxSd3d1AoZfb3YwPsN+1Y18U5ffj
XCy/MbL8EmO/Vvs3cFTG2AP04tU6w63w1zENN0FHaWc6zyDQBbgrUhg9dudnIrNz
wlFigvECwPFST0pi6pNJie+0gOkgwTZq3ePWUb1J3qIVycCezS7dWNFgLRO2t8XH
zrlnYF2V9QouMfwDU+oYdS95mUNcFIKq65nHLi6skswNPh+04iX54TX85XaR9wge
MDn0vCtWV1gqC8SFrkkerNRo1pGhxIuAzjxlRrIaRkPzjcQLh/DAtYPyHNnv7LFp
hETT7AL0MUmvXlL+eHAQ+cotV98sF/Jw6Lb94e8wUABcgxoyknbUQxUCn0k//Wrz
irBRKLPLDS28EdZayV1VzyeNoNLtDH5X6vjp2Y6yqLkn+hQQNpjEPhUIFh+aaDLv
4TXe53oTnHOkle1PbhPLjE+MbRLTBxeHODNia5t3yY4mNHbiGvwS0ASPOzs5MPNX
wyfwBeLCpxSAgZsYU7SS
=Wy56
-----END PGP SIGNATURE-----

Joe Conway <mail(at)joeconway(dot)com> writes:
> In reference to:
> http://archives.postgresql.org/message-id/6534f7ae0811181547v1dc1f096g6222e8273b461606@mail.gmail.com

> Had to fix a lot of bit rot, but otherwise looks good. My updated patch
> attached. Will commit in a day or so if no objections.

After a quick look-over, here are some comments on the patch itself:

1. Sooner or later, hopefully sooner, we will have "payload" strings in
notifications, not just the "relname" string. libpq and the protocol
already have support for that (see the "extra" field in PGnotify).
It would be a shame to have this function's API become obsolete
immediately. Can we fix the API to return the "extra" string too?

2. By the same line of reasoning, the be_pid field might be useful.
Back when I was doing a lot of LISTEN/NOTIY coding, you really needed
that field in order to distinguish your own notifies coming back from
other sessions' notifies. I don't think we've done anything to
eliminate the need for that.

3. I see the function returns NULL when there's nothing to report,
but maybe an empty array would be more sensible.

[ thinks for awhile... ] Actually, it seems to me that the present
patch's definition of the function would be very hard to work with.
You would normally want to work with the events one at a time.
There isn't much you could do with the array result except unnest() it,
and I'm a bit worried that careless usage could result in multiple
evaluation of the function and hence loss of events. I wonder whether
it would be better to have the function return setof record. Which, not
incidentally, would greatly simplify adding in those extra result
fields.

regards, tom lane

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

Tom Lane wrote:
> [ thinks for awhile... ] Actually, it seems to me that the present
> patch's definition of the function would be very hard to work with.
> You would normally want to work with the events one at a time.
> There isn't much you could do with the array result except unnest() it,
> and I'm a bit worried that careless usage could result in multiple
> evaluation of the function and hence loss of events. I wonder whether
> it would be better to have the function return setof record. Which, not
> incidentally, would greatly simplify adding in those extra result
> fields.

Sure that makes sense. I'll take a stab at it.

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

iQIcBAEBCAAGBQJKd54DAAoJEDfy90M199hleEoP/i9JAI3c+5LMrz90ntAoEthf
PzC6+QpjKIEeU5vF/NFZl8r7yWiCqlw/015clGQ4bzZfoCVwKiabGl9ziH8G1xzz
ruuM/dr3H8gZfl69LdN65t+t0P1QLSLeNeOQbtLjm0n9439lCt8r8q3joxw2sKIK
HHN6BBpyINHrlgkfw0CjRYLm0kEFW2Jj76AQyOs0V4HYtK4d+Zmr0Ut3q9aTHg7h
1MYtkPHvzq0ploKmMtx7+zpqEI3JPzyWA2hxeCZJfHHM5Y7j2eadDeN+CqWaDjs5
2T1HrtXIgtzeQWBV7J8q3rGTFc3YXTzv0mCYveHULUByl/vIx6Lind6ErbPd7gig
sTUdTo77JK7J4oV4PAZfJDRMIjUiKZGHoOPMeCIIXPyuCIYCFv/YTR3lFlEllwws
3ocY/0BZzNtUiCvH7CLD0BiSNF2sSfG6bC1I9FbHwoezlPCLKInjUoRyknGSnHAV
i2W5IIdmHiwxR5sSy+zNAUASFaK2shcvn2SX0hLbPAsDAMnPa1nYVNuqoojb1HWG
uvYXtRHoBrrtQkXl2F8NzSBmiaxfG02YY5Y2o6zkv8C/UG5+eaUIbF7SKcZMmHwJ
Ar/Zdz/eY0c/Fcy3ttfHc4C03E6qn1aDKHD+sXgDMDHfbGafDGfGhntW92ipngP1
CXbtfEYLgWcO4eGHusSB
=/Q5q
-----END PGP SIGNATURE-----

Tom Lane wrote:
>> 3) I couldn't see any way to assign myself as the committer.
>>
>
> Yeah, the webapp doesn't explicitly record the committer for a patch.
> What I've been doing is adding a comment saying that I'm taking a patch
> to commit. A separate field would probably be better though.
>
>
>

+1. I raised this before, iirc.

cheers

andrew

On Mon, Aug 3, 2009 at 10:44 PM, Andrew Dunstan<andrew(at)dunslane(dot)net> wrote:
> Tom Lane wrote:
>>>
>>> 3) I couldn't see any way to assign myself as the committer.
>>>
>>
>> Yeah, the webapp doesn't explicitly record the committer for a patch.
>> What I've been doing is adding a comment saying that I'm taking a patch
>> to commit.  A separate field would probably be better though.
>>
>
> +1. I raised this before, iirc.

Yes, I just haven't had enough round tuits yet.

...Robert

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

Tom Lane wrote:
> [ thinks for awhile... ] Actually, it seems to me that the present
> patch's definition of the function would be very hard to work with.
> You would normally want to work with the events one at a time.
> There isn't much you could do with the array result except unnest() it,
> and I'm a bit worried that careless usage could result in multiple
> evaluation of the function and hence loss of events. I wonder whether
> it would be better to have the function return setof record. Which, not
> incidentally, would greatly simplify adding in those extra result
> fields.

OK, how's this look?

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

iQIcBAEBCAAGBQJKd7diAAoJEDfy90M199hlhlUP/0nsiVPY7wCRdGGs7LsTmnQR
o4Sd9f7R4XlZdakZLKHPf61Qxe33/Af9OdosLToBjssdDW4rrER1rql8+MwddKld
+H/5VZkMRTA91BLbt8kgSZzBj3sGtGpi4zCTgYrFNfTpvCNWK/YLb5rlmbyoCbST
AlnIr/MvcFGNj/JAzQFcoA+YHjEinMnnOA/VS03hbbzBUj2F3Q2uIhsx+YxxZpEQ
jeW54YMOolpsnmQBGIY/NKbU379zWdKtscgKiDO+OLM5OkowaKPbeAZTUBx4+OGR
juOfHH7A5bLZ9APPO/N1yLNHPOLr49DrsYKdkY0Ho97NBEFZhZSqKZQgUemMB+5Z
PNjxFrC2y6HTRabeV+yKQOM/jL8ZmSiSMOwrsdmomAjLNSYi2r2o+XTTDQbNwMqQ
MqHHXRNslooJft2iNWp8iF1L/wX5URroTP+7aZbdTqUqNp/ITJu4BFZTjajZP2zQ
waAAEIVz740yVL3V8mOWyHnHgH1vQEIZ7zMqd4ss0Nn+V9Yltby2hG2Cy9/MRMxt
AkzmE2H+f794mIiyp/jydLiqqgxzbSVOv3m2cx76srSkKY7/C/wZGm2DqjbXqzb/
Dcfs3TgOYozUD02lcnrC4tncdexru/sr6iiAsprslQtzsFxWlHqYqqIh1LfwThba
SJnCszfPpDHx98RA/9b8
=7koq
-----END PGP SIGNATURE-----

Joe Conway escribió:

> OK, how's this look?

Hmm, is it possible to use OUT parameters in the function instead of
declaring a new type for the result?

--
Alvaro Herrera http://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

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

Alvaro Herrera wrote:
> Joe Conway escribió:
>
>> OK, how's this look?
>
> Hmm, is it possible to use OUT parameters in the function instead of
> declaring a new type for the result?

Sure, I guess I ought to use the latest-and-greatest. Any other comments
before I commit?

Thanks,

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

iQIcBAEBCAAGBQJKeDzfAAoJEDfy90M199hl3FsP/RYXWVVlItm3jk3hKNPCqTaT
maSwtl0QrFLl1pAc3ZTk16QdERrYFjfxIcxALz2am7OtStmhOz63Y4nlY8L0GHdz
ZJDF7K3r5o2RJ12h7Zucpo3racxp1eJqfi4S61RCeIi+aPxtcJd7dds1588FYACc
cPi6AuZyFfE6lhUjgniqOjzvTMptqWbtmuRZl1m+iyXsPE2FLlMclfugbzFKFXso
cnUI31cuejg9tF1zboatopm/qAcIiCB+U2KVG7tyiI+KdL/bNJlkELRJKODxdCvB
XNHzNBey1ZmbgE+H8gu1bz7DXbTxaffoQunCd4HnfOXOGrJpG+djsSCNW860sSVK
icCvdvq3taHfcpLtIIjpecu9LatTFGkJ12YVIV33gnv/Fgr8pj+84VsxRfqlfNaN
V743KdWtEv19gii3qIa817ZcS4tpBfIxXyt37cuZMdOv3VXR7LejeOoDr4aHGjDq
SCozcQQ+Fh8diU1XMzl3rRtz3Brz1cVYE8Ue8ELcQzzuOvWT+a0cjCBii1SmZdTz
DnpVEzmeOGrnHM8+IOG+h9IFRvNx3RQ2nxs71gcbgDp62fnQqb3m7c/Aj0xWv8xp
mQhY2DNQg0AY1vr71nfMGbYL7xTHFD5fA2Md2l/029PA+qFhJlqnZpWScHwqafcC
nkPH88kPUgy33vuNTJ3A
=U88M
-----END PGP SIGNATURE-----

Joe Conway <mail(at)joeconway(dot)com> writes:
> Sure, I guess I ought to use the latest-and-greatest. Any other comments
> before I commit?

That "be_pid/be_pid" hack in the regression test is pretty ugly, and
doesn't test anything very useful anyway seeing that it's integer
division. Could you do something like
be_pid = pg_backend_pid() AS is_self_notify
instead, to verify that it's a self-notify? (This is not quite right
because you'd need to execute pg_backend_pid() at the remote end, but
I'm not awake enough to remember the dblink syntax for that. Maybe
it's too complex to be worth it, but I think demonstrating how to
check for self-notify would be a useful bit of doco.)

Otherwise it looks ok to me.

regards, tom lane

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

Tom Lane wrote:
> Joe Conway <mail(at)joeconway(dot)com> writes:
>> Sure, I guess I ought to use the latest-and-greatest. Any other comments
>> before I commit?
>
> That "be_pid/be_pid" hack in the regression test is pretty ugly, and
> doesn't test anything very useful anyway seeing that it's integer
> division. Could you do something like
> be_pid = pg_backend_pid() AS is_self_notify
> instead, to verify that it's a self-notify? (This is not quite right
> because you'd need to execute pg_backend_pid() at the remote end, but
> I'm not awake enough to remember the dblink syntax for that. Maybe
> it's too complex to be worth it, but I think demonstrating how to
> check for self-notify would be a useful bit of doco.)
>
> Otherwise it looks ok to me.

OK, still not pretty, but at least more useful. Last call for
comments...I hope ;-)

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

iQIcBAEBCAAGBQJKePNeAAoJEDfy90M199hlduYP/2UaEuEXzWnGS1RX4Be+UajS
LBrS5uanAEUJcohnpMXCHjZVuLJBhXl8r/2RPXsVt5Fem3oYkxFrK3eCysMZs0jJ
F6iWb/Hiyi3bVf8coHjeabm0j/TF5yYsKuE3D0kcxktwKjq9vyGOxkadcAXV/lq1
+awVp5tip1ko3R20zSv+N4dcZp6ewHZ9Fa9AT9BS/3D5vQn6Jq3aUajqZ1DdSw0W
Fm5DlAl632xB8dQ9G/nNUYXMf5040OyD7foZEncS30i+diyHEvx5++5peCmlAvBj
KmeULbsmGn+brlQjvMftJU5LnOzEkKAHYs90BjR/jHoivJHSBjDNbREm1lJqGU9q
DU7E3NXz2/hva+/a+PPoyZjJDj+wvxzBivySB/fPCLpQFATDYHQ+1iMOKWqiGO1q
N1I6bEEZTAH1/8drANR224/BVnOs10s+4uUL5L8Me+eWO0gIxyhpHyIg/MkhlJ1a
Aopz3p7XEuXWmo4XGEAMVhJ2NpBiNKc69Ihno3kkY6FMo4Hs8rNsP+bdZnpBhugO
JB0+EDhSaVf2O7a8zmz3fylP2cNBQTh3gZlbNzYlnYgkhkuTXda2JXjwdnAkd5DK
33vVaJ0swb7MzeBWzP5PDMXKbzVPtz7FvReVVhIZbAJk+oVlwlw+KMRo8z8g0ci8
CgX2i1YwCh0AGuk450rZ
=JQmH
-----END PGP SIGNATURE-----

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

Tom Lane wrote:
> Could you do something like
> be_pid = pg_backend_pid() AS is_self_notify
> instead, to verify that it's a self-notify? (This is not quite right
> because you'd need to execute pg_backend_pid() at the remote end, but
> I'm not awake enough to remember the dblink syntax for that. Maybe
> it's too complex to be worth it, but I think demonstrating how to
> check for self-notify would be a useful bit of doco.)
>
> Otherwise it looks ok to me.

Committed. Final version attached.

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

iQIcBAEBCAAGBQJKea/ZAAoJEDfy90M199hlZP8P/RcOrp9nHjAqVilQlHbRdlGO
7xTq2e9LU6tY2V0mzLcLZQ5SY6m12gNbQSIE6/8cNO3nWTjbm0TOYWpwKohBLvBt
0QQMUxi4JWapJcplaE10pt3xOT5+Kqn1mDG97Id92DwHTT7JtIBciDGWTuVWyQu3
8YJcbPzcTtGzL3lOTMFbZss10Lr7bLEMx0UmPJiMWDMqKIpmgC1cegIL2M54jol/
/fFx3mlz52O2F/maPtm4noBWsrDP6x/T7K8hspsqyWP0Xv52xPZ5qRorXRK3mDMb
U1jpAi/jqWqJ3X3riTeda39dg2wxZY1feOn42NBFTilbwHnpT+a6nSaR55/ZgnMp
7rlnSMOZTHCxgPOGZFXUNZsgf0HNME/2jFpfRhtbGzIre/iXcHUfhAbOiD72Gxdv
so+IOwbAEDXtIIwCAufAVZG/OvweEH8y9M3MytY82ozfmLiVwd6MvHuvjAixWrK/
/rVhH3d3j3oZh6dnjwOpZPvdQuqPJdl0sU5vvatHBMH8Af2gfSKFdmBlG47D9LGn
Brish7KmQb9u4hBinVNyMac9V/VVmUbY0K4fLQru8DtElWZzCyTRylkmiUqFtKeu
0OSx/vO/csR2Wa83hgyYLWGR8ShgYNX5Fws/BMtdzxnD4mjguJ2FI2FyduXGZ3GK
EyVBHi8/NBXt6DNoOhbS
=uKPZ
-----END PGP SIGNATURE-----