pg_listen is not working properly

[rianby64]

I've the following case:

proc notification { d } {
  puts $d
}

pg_listen $db "virtual" notification

CREATE OR REPLACE FUNCTION notify_info()
  RETURNS TRIGGER AS
$BODY$
BEGIN
  PERFORM pg_notify(CAST('virtual' AS text),
    CAST(' this is the payload I want to send to TCL '));
  RETURN NULL;
END;
$BODY$ LANGUAGE plpgsql IMMUTABLE;

CREATE TRIGGER "Mytable_notify"
  AFTER INSERT OR UPDATE OR DELETE
  ON "Mytable"
  FOR EACH ROW 
  EXECUTE PROCEDURE notify_info();

And in shell, the result of doing any modification to the table, ends up with:

wrong # args: should be "notification d"
    while executing
"notification"
    ("pg_listen" script)

Looks like you removed the parameter payload from the callback.
If try the same case under CentOS 7 with

Arch        : x86_64
Version     : 2.0.0
Release     : 5.el7

then all will work correctly.

[rianby64]

Maybe this is just a problem of compatibility? Did you change the way to perform notifications?

[rianby64]

I switched back to the old version 2.0.0 and everything works as expected...
As I was trying to compile and run this, I noticed that your last version requires tcl8.6, but in CentOS I've tcl8.5. So, can't explore this issue 😞

[bovine]

Have you tried defining your proc as:

proc notification { args } {
    puts $args
}

Using args as the literal name will allow it to be invoked with zero, one, or multiple arguments.

[rianby64]

The problem with this suggestion is that nothing comes to args. I already tried that case.
And, as args contains an empty array, then I can't read the string that I'm passing from SQL via pg_notify. And that feature is critical for me.

[bovine]

What you're trying to do might be easier with a PL/Tcl Function defined on the database: https://www.postgresql.org/docs/9.1/static/pltcl-functions.html

I don't think any of the recent changes in this Pgtcl have intentionally change the "pg_listen" functionality, as that is relatively old functionality that is not commonly used.

[resuna]

I think I see a change that might have had an effect on this, where I was cleaning up error handling.

[rianby64]

Ok, @bovine . But, how to read the info from payload?

I mean, if there's another way to receive the payload that comes from SQL notify?
Looks like notify sends a payload to nowhere.

[resuna]

There's a lot of changes since 2.0.0... can you try to find the most recent version in which this works, and the oldest version where it does not work, so I can look more closely?

[rianby64]

@resuna The version that works for me is 2.0.0. Give me some time to compile/install 2.1 and 2.2... I'll try to answer you within a couple of hours. Thanks a lot.

[rianby64]

@resuna , 2.3.0 version is not working properly on CentOS 7.
I got the following log:

$ tclsh8.5
% package require Pgtcl
couldn't load file "/usr/lib64/tcl8.5/pgtcl2.3.0/libpgtcl2.3.0.so": /usr/lib64/tcl8.5/pgtcl2.3.0/libpgtcl2.3.0.so: undefined symbol: handle_substitutions

And the compilation passed with warnings:

make
gcc -DPACKAGE_NAME=\"pgtcl\" -DPACKAGE_TARNAME=\"pgtcl\" -DPACKAGE_VERSION=\"2.3.0\" -DPACKAGE_STRING=\"pgtcl\ 2.3.0\" -DPACKAGE_BUGREPORT=\"\" -DPACKAGE_URL=\"\" -DBUILD_pgtcl=/\*\*/ -DSTDC_HEADERS=1 -DHAVE_SYS_TYPES_H=1 -DHAVE_SYS_STAT_H=1 -DHAVE_STDLIB_H=1 -DHAVE_STRING_H=1 -DHAVE_MEMORY_H=1 -DHAVE_STRINGS_H=1 -DHAVE_INTTYPES_H=1 -DHAVE_STDINT_H=1 -DHAVE_UNISTD_H=1 -DHAVE_LIMITS_H=1 -DHAVE_SYS_PARAM_H=1 -DHAVE_PQSETSINGLEROWMODE=1 -DUSE_THREAD_ALLOC=1 -D_REENTRANT=1 -D_THREAD_SAFE=1 -DTCL_THREADS=1 -DMODULE_SCOPE=extern\ __attribute__\(\(__visibility__\(\"hidden\"\)\)\) -DHAVE_HIDDEN=1 -DHAVE_CAST_TO_UNION=1 -D_LARGEFILE64_SOURCE=1 -DTCL_WIDE_INT_IS_LONG=1 -DUSE_TCL_STUBS=1  -I/usr/pgsql-10/include -I"/usr/include"    -pipe -O2 -fomit-frame-pointer -DNDEBUG -Wall -fPIC  -c `echo ./generic/pgtcl.c` -o pgtcl.o
gcc -DPACKAGE_NAME=\"pgtcl\" -DPACKAGE_TARNAME=\"pgtcl\" -DPACKAGE_VERSION=\"2.3.0\" -DPACKAGE_STRING=\"pgtcl\ 2.3.0\" -DPACKAGE_BUGREPORT=\"\" -DPACKAGE_URL=\"\" -DBUILD_pgtcl=/\*\*/ -DSTDC_HEADERS=1 -DHAVE_SYS_TYPES_H=1 -DHAVE_SYS_STAT_H=1 -DHAVE_STDLIB_H=1 -DHAVE_STRING_H=1 -DHAVE_MEMORY_H=1 -DHAVE_STRINGS_H=1 -DHAVE_INTTYPES_H=1 -DHAVE_STDINT_H=1 -DHAVE_UNISTD_H=1 -DHAVE_LIMITS_H=1 -DHAVE_SYS_PARAM_H=1 -DHAVE_PQSETSINGLEROWMODE=1 -DUSE_THREAD_ALLOC=1 -D_REENTRANT=1 -D_THREAD_SAFE=1 -DTCL_THREADS=1 -DMODULE_SCOPE=extern\ __attribute__\(\(__visibility__\(\"hidden\"\)\)\) -DHAVE_HIDDEN=1 -DHAVE_CAST_TO_UNION=1 -D_LARGEFILE64_SOURCE=1 -DTCL_WIDE_INT_IS_LONG=1 -DUSE_TCL_STUBS=1  -I/usr/pgsql-10/include -I"/usr/include"    -pipe -O2 -fomit-frame-pointer -DNDEBUG -Wall -fPIC  -c `echo ./generic/pgtclCmds.c` -o pgtclCmds.o
./generic/pgtclCmds.c: In function ‘Pg_exec’:
./generic/pgtclCmds.c:776:4: warning: passing argument 1 of ‘tclStubsPtr->tcl_Free’ from incompatible pointer type [enabled by default]
    ckfree(paramValues);
    ^
./generic/pgtclCmds.c:776:4: note: expected ‘char *’ but argument is of type ‘const char **’
./generic/pgtclCmds.c: In function ‘expand_parameters’:
./generic/pgtclCmds.c:2780:2: warning: passing argument 1 of ‘tclStubsPtr->tcl_Free’ from incompatible pointer type [enabled by default]
  if(paramValues) ckfree(paramValues);
  ^
./generic/pgtclCmds.c:2780:2: note: expected ‘char *’ but argument is of type ‘const char **’
./generic/pgtclCmds.c: In function ‘Pg_select’:
./generic/pgtclCmds.c:2927:4: warning: passing argument 1 of ‘tclStubsPtr->tcl_Free’ from incompatible pointer type [enabled by default]
    ckfree(paramValues);
    ^
./generic/pgtclCmds.c:2927:4: note: expected ‘char *’ but argument is of type ‘const char **’
./generic/pgtclCmds.c:2958:3: warning: passing argument 1 of ‘tclStubsPtr->tcl_Free’ from incompatible pointer type [enabled by default]
   if(paramValues) ckfree(paramValues);
   ^
./generic/pgtclCmds.c:2958:3: note: expected ‘char *’ but argument is of type ‘const char **’
./generic/pgtclCmds.c:3008:3: warning: passing argument 1 of ‘tclStubsPtr->tcl_Free’ from incompatible pointer type [enabled by default]
   ckfree(paramValues);
   ^
./generic/pgtclCmds.c:3008:3: note: expected ‘char *’ but argument is of type ‘const char **’
./generic/pgtclCmds.c: In function ‘Pg_sendquery’:
./generic/pgtclCmds.c:3503:4: warning: passing argument 1 of ‘tclStubsPtr->tcl_Free’ from incompatible pointer type [enabled by default]
    ckfree(paramValues);
    ^
./generic/pgtclCmds.c:3503:4: note: expected ‘char *’ but argument is of type ‘const char **’
gcc -DPACKAGE_NAME=\"pgtcl\" -DPACKAGE_TARNAME=\"pgtcl\" -DPACKAGE_VERSION=\"2.3.0\" -DPACKAGE_STRING=\"pgtcl\ 2.3.0\" -DPACKAGE_BUGREPORT=\"\" -DPACKAGE_URL=\"\" -DBUILD_pgtcl=/\*\*/ -DSTDC_HEADERS=1 -DHAVE_SYS_TYPES_H=1 -DHAVE_SYS_STAT_H=1 -DHAVE_STDLIB_H=1 -DHAVE_STRING_H=1 -DHAVE_MEMORY_H=1 -DHAVE_STRINGS_H=1 -DHAVE_INTTYPES_H=1 -DHAVE_STDINT_H=1 -DHAVE_UNISTD_H=1 -DHAVE_LIMITS_H=1 -DHAVE_SYS_PARAM_H=1 -DHAVE_PQSETSINGLEROWMODE=1 -DUSE_THREAD_ALLOC=1 -D_REENTRANT=1 -D_THREAD_SAFE=1 -DTCL_THREADS=1 -DMODULE_SCOPE=extern\ __attribute__\(\(__visibility__\(\"hidden\"\)\)\) -DHAVE_HIDDEN=1 -DHAVE_CAST_TO_UNION=1 -D_LARGEFILE64_SOURCE=1 -DTCL_WIDE_INT_IS_LONG=1 -DUSE_TCL_STUBS=1  -I/usr/pgsql-10/include -I"/usr/include"    -pipe -O2 -fomit-frame-pointer -DNDEBUG -Wall -fPIC  -c `echo ./generic/pgtclId.c` -o pgtclId.o
rm -f libpgtcl2.3.0.so
gcc -shared -pipe -O2 -fomit-frame-pointer -DNDEBUG -Wall -fPIC  -Wl,--export-dynamic  -o libpgtcl2.3.0.so pgtcl.o pgtclCmds.o pgtclId.o -L/usr/pgsql-10/lib -lpq  -L/usr/lib -ltclstub8.5 
/usr/bin/ld: skipping incompatible /usr/lib/libtclstub8.5.a when searching for -ltclstub8.5
: libpgtcl2.3.0.so
If you really want to rebuild the documentation, do the following:
        cd doc ; make all

Trying older versions...

[rianby64]

Version 2.2.0 works incorrectly.

wrong # args: should be "notification d"
    while executing
"notification"
    ("pg_listen" script)

[rianby64]

The same situation with version 2.1.0

wrong # args: should be "notification d"
    while executing
"notification"
    ("pg_listen" script)

[resuna]

And a freshly built 2.0.0 works?

[resuna]

That "handle_substitutions" problem is hard to understand, did you do a new autoconf/configure after each checkout?

Edit: This was fixed in 2.3.2.

[rianby64]

Very strange...
Even a fresh compilation of 2.0.0 is not working. My working version is that one that comes from CentOS 7.

wrong # args: should be "notification d"
    while executing
"notification"
    ("pg_listen" script)

[resuna]

And here I am digging through the differences between 2.0.0 and 2.1.0 and finding nothing. :)

Sounds like there's a change someone in the include files or compiler toolchain that's breaking things. Or Centos had some special magic in their build. That's tough to figure out.

[rianby64]

And I got more confused. By the way,

That "handle_substitutions" problem is hard to understand, did you do a new autoconf/configure after each checkout?

I'm compiling in different folders, and doing ./configure && make && make install in different context. Also I checked that for each version

package require Pgtcl
> 2.1.0

was the one I wanted to test...

[rianby64]

This is what I get when I test the version that comes with CentOS 7

<notification>
data(from)  = Keynotes
data(new)   = "id":"-","parent":null,"createdAt":"2018-01-09T18:19:22.839895+03:00","updatedAt":"2018-01-10T01:01:06.13729+03:00"
data(old)   = "id":"-","parent":null,"createdAt":"2018-01-09T18:19:22.839895+03:00","updatedAt":"2018-01-10T00:42:55.283715+03:00"
data(query) = UPDATE
</notification>

I see the full message that comes from

PERFORM pg_notify('virtual', payload);

[rianby64]

V 2.0.0-alpha2 doesn't work 😮

[resuna]

check the version of libpq you’re building against?

[rianby64]

How to check that?
I think, here's the answer

[root@locahost usr]# find . | grep libpq-fe.h
./pgsql-10/include/libpq-fe.h

So, I'm using something that is located at /usr/pgsql-10/include/libpq-fe.h

[rianby64]

I'm doing something like this...

https://gist.github.com/rianby64/b91fb02efbfaab68492cf4dbf2388855

Hope this code may help you to understand my case. Thanks a lot!

[rianby64]

What you're trying to do might be easier with a PL/Tcl Function defined on the database: https://www.postgresql.org/docs/9.1/static/pltcl-functions.html
I don't think any of the recent changes in this Pgtcl have intentionally change the "pg_listen" functionality, as that is relatively old functionality that is not commonly used.

@bovine , I'd be happy to share with you some thoughts about your remark.
My intention here is to send a message to a TCL server instance. If I'm planning to send a message, then I've to open a socket, and a socket is a file. But, SQL sets up hard restrictions related to files, so I think something like this

set sockChan [socket localhost 9900]

may fail.
Thanks for your attention.

[bovine]

You can do anything (including socket operations) if you use a fully trusted PL/TclU
https://www.postgresql.org/docs/9.5/static/pltcl-overview.html

[rianby64]

Oh! Thanks a lot @bovine . I'll read twice before doing any new conclusion.

[resuna]

I suspect that the package you got from Centos was built against libpq from postgresql 9, not 10.

[rianby64]

@resuna, I test the package postgresql96-tcl.x86_64

[root@localhost /]# yum info postgresql96-tcl.x86_64
Loaded plugins: fastestmirror, remove-with-leaves, show-leaves
Loading mirror speeds from cached hostfile
 * base: mirror.awanti.com
 * epel: mirror.awanti.com
 * extras: mirror.awanti.com
 * nux-dextop: mirror.li.nux.ro
 * updates: mirror.corbina.net
Available Packages
Name        : postgresql96-tcl
Arch        : x86_64
Version     : 2.3.1
Release     : 1.rhel7
Size        : 336 k
Repo        : pgdg96/7/x86_64
Summary     : A Tcl client library for PostgreSQL
URL         : https://github.com/flightaware/Pgtcl
License     : BSD
Description : PostgreSQL is an advanced Object-Relational database management system.
            : The tcl-pgtcl package contains Pgtcl, a Tcl client library for connecting
            : to a PostgreSQL server.

against Postgres9.6 and against Postgres9.2 days ago and I found the same issue.

Please, check my case here:

I'm doing something like this...
https://gist.github.com/rianby64/b91fb02efbfaab68492cf4dbf2388855
Hope this code may help you to understand my case. Thanks a lot!

[resuna]

That doesn't actually tell us what version of libpq you were linking against. I'm not familiar with yum's packaging for Postgres components, but that looks like a Pgtcl package for postgresql96. libpq is in the postgresql package itself.

I rolled back to 1.9 and got a different error:

$ tclsh start.tcl
Attempt to query while waiting for callback
while executing
"pg_listen $db "virtual" notification"
(file "start.tcl" line 12)

Does that ring any bells @bovine ?

[bovine]

Nothing i recognize.

[rianby64]

@bovine , what's next? Should we close this issue? Looks like don't see anything unexpected in new versions of this extension.

The reason why I raised this issue is simple: payload is not being passed when notifying via NOTIFY channel, payload;.
If using an old version, Pgtcl sees the payload. But if use new versions, Pgtcl doesn't see it.

https://gist.github.com/rianby64/b91fb02efbfaab68492cf4dbf2388855
Hope this code may help you to understand my case. Thanks a lot!

[holgerjakobs]

Any new developments regarding this issue? Still, no playload gets transferred to the listening proc.
A listening psql client receives and shows the payload, though. Therefore, it's not a PostgreSQL problem, but one of this library Also missing is the PID (session id) of the notifying backend.

[holgerjakobs]

Yesterday and today I took some time to add the channel name, the PID of the notifying backend and the payload (if present) to the callback. Luckily, changing a few lines of one file only does the trick.

Apply the attached patch to the file generic/pgtclId.c and compile.
patch.txt

The callback procedure can be defined as follows:

  proc received {channel pid {payload ""}} {
    puts "channel: $channel, pid: $pid"
    if {$payload != ""} {
      puts "payload: '$payload'"
    }
  } 

Hopefully, this is being incorporated into the next version of the library.

[resuna]

Looking at the patch, you're creating a command using string rather than list operations. That's probably not an exploitable bug in this case, but it isn't best practice.

Edit: I'm kind of busy right now but I should be able to rewrite it after next week.

[holgerjakobs]

Ok, I rewrote it with a list instead of a string. Now even payloads with unbalanced braces work fine. Thanks for the idea
Pg_Notify_EventProc.txt
patch.txt
.

[resuna]

Looks good, you could eliminate char pid[10] by using Tcl_NewIntObj.

[resuna]

I've inserted your changes with a couple of tweaks to https://github.com/flightaware/Pgtcl/tree/pg_listen_payload

[resuna]

@holgerjakobs Could you review or comment on PR#42?

Also @rianby64

In particular I would like some feedback on how the callback list should be constructed in the case of a missing notify element.