Thread: BUG #8710: dblink dblink_get_pkey output bug, and dblink_build_sql_update bug

The following bug has been logged on the website:

Bug reference:      8710
Logged by:          digoal.zhou
Email address:      digoal@126.com
PostgreSQL version: 9.3.2
Operating system:   CentOS 5.x
Description:

pg93@db-172-16-3-150-> psql
psql (9.3.1)
Type "help" for help.


digoal=# create extension dblink;
CREATE EXTENSION
digoal=# create table tbl_dblink(c1 int, c2 int, c3 text, pk1 int, c4 text,
c5 int, pk2 text, c6 text, c7 int, pk3 int8, c8 int, c9 text, c10 timestamp,
primary key(pk1,pk2,pk3));
CREATE TABLE
digoal=# insert into tbl_dblink values
(1,1,'test',1,'test',2,'pk2','test',1,1,1,'test', now());
INSERT 0 1
digoal=# select * from tbl_dblink ;
 c1 | c2 |  c3  | pk1 |  c4  | c5 | pk2 |  c6  | c7 | pk3 | c8 |  c9  |
      c10
----+----+------+-----+------+----+-----+------+----+-----+----+------+----------------------------
  1 |  1 | test |   1 | test |  2 | pk2 | test |  1 |   1 |  1 | test |
2013-12-31 08:39:01.400074
(1 row)


digoal=# select * from dblink_get_pkey('tbl_dblink');
 position | colname
----------+---------
        1 | pk1
        2 | pk2
        3 | pk3
(3 rows)
NOTE, postgresql 9.0+ we use dblink_build* to build SQL , the primary key
use the logical order. but we cann't use dblink_get_pkey get the order
int2vector, we must use the pg_attribute catalog get the logical number.
So, the dblink_get_pkey  function need change .
and the second bug:
digoal=# select * from dblink_build_sql_update('tbl_dblink', '4 7 10', 3,
$${1, pk2, 1}$$, $${2,pk2,2}$$);

                                dblink_build_sql_update



------------------------------------------------------------------------------------------------------------------------------------
-------------------------------------------------------------------------------------------------------------
 UPDATE tbl_dblink SET c1 = '1', c2 = '1', c3 = 'test', pk1 = '2', c4 =
'test', c5 = '2', pk2 = 'pk2', c6 = 'test', c7 = '1', pk3 =
'2', c8 = '1', c9 = 'test', c10 = '2013-12-31 08:39:01.400074' WHERE pk1 =
'2' AND pk2 = 'pk2' AND pk3 = '2'
(1 row)
We see, the WHERE clause not src pk arrays, but target pk arrays. so this is
a bug.
and we must use dblink_build_sql_delete and dblink_build_sql_insert function
to get the correct SQL.
dblink_build_sql_update function need to change, src array contain OLD.* and
target array contain NEW.*, not only OLD.pkey and NEW.pkey now.

digoal@126.com writes:
> NOTE, postgresql 9.0+ we use dblink_build* to build SQL , the primary key
> use the logical order. but we cann't use dblink_get_pkey get the order
> int2vector, we must use the pg_attribute catalog get the logical number.
> So, the dblink_get_pkey  function need change .

That doesn't sound to me like a bug, but a feature request.  The
documentation clearly states that dblink_get_pkey's position column
runs from 1 to N.  Your real complaint seems to be "why isn't it
easier to get the column numbers to give to dblink_build_sql_update
and friends"?

I think we actually made this worse in PG 9.0: before that, someone
could get the pg_index.indkey array for the relevant index and use
that, but now that's the wrong thing if any dropped columns are involved.

I'm tempted to propose overloading dblink_build_sql_update and friends
with new functions defined like

     dblink_build_sql_update(relname regclass,
                      indexname regclass,
                 src_pk_att_vals_array text[],
                 tgt_pk_att_vals_array text[])

and letting all the column lookup machinations happen internally
to that.

> and the second bug:
> digoal=# select * from dblink_build_sql_update('tbl_dblink', '4 7 10', 3,
> $${1, pk2, 1}$$, $${2,pk2,2}$$);

>                                 dblink_build_sql_update


>
------------------------------------------------------------------------------------------------------------------------------------
> -------------------------------------------------------------------------------------------------------------
>  UPDATE tbl_dblink SET c1 = '1', c2 = '1', c3 = 'test', pk1 = '2', c4 =
> 'test', c5 = '2', pk2 = 'pk2', c6 = 'test', c7 = '1', pk3 =
> '2', c8 = '1', c9 = 'test', c10 = '2013-12-31 08:39:01.400074' WHERE pk1 =
> '2' AND pk2 = 'pk2' AND pk3 = '2'
> (1 row)
> We see, the WHERE clause not src pk arrays, but target pk arrays. so this is
> a bug.

That appears to me to be working as documented.  It might be better if the
arguments were referred to as "local_pk_att_vals_array" and
"remote_pk_att_vals_array", since the function isn't meant to change the
PK values as you seem to think.

            regards, tom lane

CkhJLAogICBUaGFua3MuIGJ1dCBub3cgZGJsaW5rIGV4dGVuc2lvbiBsYWNrIHVwZGF0ZSBzcWwg
YnVpbGRpbmcgZnVuY3Rpb24gbGlrZSBpbnNlcnQgYW5kIGRlbGV0ZSwgbm90IHRoZSBjdXJyZW50
IGRibGlua19idWlsZF9zcWxfdXBkYXRlKCkuCmZvciBleHAgdGJsLgpwazoxLCBpbmZvICd0ZXN0
JyB1cGRhdGUgdG8gcGs6MiwgaW5mbzogJ25ldycKYnVpbGQgdGhlIHVwZGF0ZSBzcWw6CnVwZGF0
ZSB0Ymwgc2V0IHBrPTIsaW5mbz0nbmV3JyB3aGVyZSBwaz0xOwoKCgoKCi0tCrmr0ubKx9K7sbLX
07XEysIsSSdtIERpZ29hbCxKdXN0IERvIEl0LgoKCgoKQXQgMjAxNC0wMS0wMSAwMDowMzo1MSwi
VG9tIExhbmUiIDx0Z2xAc3NzLnBnaC5wYS51cz4gd3JvdGU6Cj5kaWdvYWxAMTI2LmNvbSB3cml0
ZXM6Cj4+IE5PVEUsIHBvc3RncmVzcWwgOS4wKyB3ZSB1c2UgZGJsaW5rX2J1aWxkKiB0byBidWls
ZCBTUUwgLCB0aGUgcHJpbWFyeSBrZXkKPj4gdXNlIHRoZSBsb2dpY2FsIG9yZGVyLiBidXQgd2Ug
Y2Fubid0IHVzZSBkYmxpbmtfZ2V0X3BrZXkgZ2V0IHRoZSBvcmRlcgo+PiBpbnQydmVjdG9yLCB3
ZSBtdXN0IHVzZSB0aGUgcGdfYXR0cmlidXRlIGNhdGFsb2cgZ2V0IHRoZSBsb2dpY2FsIG51bWJl
ci4KPj4gU28sIHRoZSBkYmxpbmtfZ2V0X3BrZXkgIGZ1bmN0aW9uIG5lZWQgY2hhbmdlIC4KPgo+
VGhhdCBkb2Vzbid0IHNvdW5kIHRvIG1lIGxpa2UgYSBidWcsIGJ1dCBhIGZlYXR1cmUgcmVxdWVz
dC4gIFRoZQo+ZG9jdW1lbnRhdGlvbiBjbGVhcmx5IHN0YXRlcyB0aGF0IGRibGlua19nZXRfcGtl
eSdzIHBvc2l0aW9uIGNvbHVtbgo+cnVucyBmcm9tIDEgdG8gTi4gIFlvdXIgcmVhbCBjb21wbGFp
bnQgc2VlbXMgdG8gYmUgIndoeSBpc24ndCBpdAo+ZWFzaWVyIHRvIGdldCB0aGUgY29sdW1uIG51
bWJlcnMgdG8gZ2l2ZSB0byBkYmxpbmtfYnVpbGRfc3FsX3VwZGF0ZQo+YW5kIGZyaWVuZHMiPwo+
Cj5JIHRoaW5rIHdlIGFjdHVhbGx5IG1hZGUgdGhpcyB3b3JzZSBpbiBQRyA5LjA6IGJlZm9yZSB0
aGF0LCBzb21lb25lCj5jb3VsZCBnZXQgdGhlIHBnX2luZGV4LmluZGtleSBhcnJheSBmb3IgdGhl
IHJlbGV2YW50IGluZGV4IGFuZCB1c2UKPnRoYXQsIGJ1dCBub3cgdGhhdCdzIHRoZSB3cm9uZyB0
aGluZyBpZiBhbnkgZHJvcHBlZCBjb2x1bW5zIGFyZSBpbnZvbHZlZC4KPgo+SSdtIHRlbXB0ZWQg
dG8gcHJvcG9zZSBvdmVybG9hZGluZyBkYmxpbmtfYnVpbGRfc3FsX3VwZGF0ZSBhbmQgZnJpZW5k
cwo+d2l0aCBuZXcgZnVuY3Rpb25zIGRlZmluZWQgbGlrZQo+Cj4gICAgIGRibGlua19idWlsZF9z
cWxfdXBkYXRlKHJlbG5hbWUgcmVnY2xhc3MsCj4gICAgIAkJCSAgICAgaW5kZXhuYW1lIHJlZ2Ns
YXNzLAo+CQkJICAgICBzcmNfcGtfYXR0X3ZhbHNfYXJyYXkgdGV4dFtdLAo+CQkJICAgICB0Z3Rf
cGtfYXR0X3ZhbHNfYXJyYXkgdGV4dFtdKQo+Cj5hbmQgbGV0dGluZyBhbGwgdGhlIGNvbHVtbiBs
b29rdXAgbWFjaGluYXRpb25zIGhhcHBlbiBpbnRlcm5hbGx5Cj50byB0aGF0Lgo+Cj4+IGFuZCB0
aGUgc2Vjb25kIGJ1ZzoKPj4gZGlnb2FsPSMgc2VsZWN0ICogZnJvbSBkYmxpbmtfYnVpbGRfc3Fs
X3VwZGF0ZSgndGJsX2RibGluaycsICc0IDcgMTAnLCAzLAo+PiAkJHsxLCBwazIsIDF9JCQsICQk
ezIscGsyLDJ9JCQpOwo+ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAg
ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgIAo+PiAgICAgICAgICAgICAgICAgICAg
ICAgICAgICAgICAgIGRibGlua19idWlsZF9zcWxfdXBkYXRlCj4gICAgICAgICAgICAgICAgICAg
ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAg
Cj4gICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgIAo+PiAtLS0tLS0tLS0tLS0tLS0tLS0t
LS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0t
LS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0K
Pj4gLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0t
LS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLQo+
PiAgVVBEQVRFIHRibF9kYmxpbmsgU0VUIGMxID0gJzEnLCBjMiA9ICcxJywgYzMgPSAndGVzdCcs
IHBrMSA9ICcyJywgYzQgPQo+PiAndGVzdCcsIGM1ID0gJzInLCBwazIgPSAncGsyJywgYzYgPSAn
dGVzdCcsIGM3ID0gJzEnLCBwazMgPSAKPj4gJzInLCBjOCA9ICcxJywgYzkgPSAndGVzdCcsIGMx
MCA9ICcyMDEzLTEyLTMxIDA4OjM5OjAxLjQwMDA3NCcgV0hFUkUgcGsxID0KPj4gJzInIEFORCBw
azIgPSAncGsyJyBBTkQgcGszID0gJzInCj4+ICgxIHJvdykKPj4gV2Ugc2VlLCB0aGUgV0hFUkUg
Y2xhdXNlIG5vdCBzcmMgcGsgYXJyYXlzLCBidXQgdGFyZ2V0IHBrIGFycmF5cy4gc28gdGhpcyBp
cwo+PiBhIGJ1Zy4KPgo+VGhhdCBhcHBlYXJzIHRvIG1lIHRvIGJlIHdvcmtpbmcgYXMgZG9jdW1l
bnRlZC4gIEl0IG1pZ2h0IGJlIGJldHRlciBpZiB0aGUKPmFyZ3VtZW50cyB3ZXJlIHJlZmVycmVk
IHRvIGFzICJsb2NhbF9wa19hdHRfdmFsc19hcnJheSIgYW5kCj4icmVtb3RlX3BrX2F0dF92YWxz
X2FycmF5Iiwgc2luY2UgdGhlIGZ1bmN0aW9uIGlzbid0IG1lYW50IHRvIGNoYW5nZSB0aGUKPlBL
IHZhbHVlcyBhcyB5b3Ugc2VlbSB0byB0aGluay4KPgo+CQkJcmVnYXJkcywgdG9tIGxhbmUK

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 12/31/2013 08:03 AM, Tom Lane wrote:
> That doesn't sound to me like a bug, but a feature request.

I agree. It may not work they way he desires, but it works as designed
and documented as far a I can see.

To be honest I didn't think this part of dblink was getting much use
as its focus was pretty narrow from the beginning. Probably too late
to rip it out but I'm not sure we want to expand/enhance it either.

Joe

- --
Joe Conway
credativ LLC: http://www.credativ.us
Linux, PostgreSQL, and general Open Source
Training, Service, Consulting, & 24x7 Support
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.14 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iQIcBAEBAgAGBQJSxwVCAAoJEDfy90M199hlWHUP+wREwwEG8L7MuW3iFv8BqMA/
NAL3lyJCBjeUyVoZw7mkNFNEwmley96kYNfuJZU/vKcHYiGoS/STn/4TarkSYRAl
JJCKyfYz3yuE783Ba1U1RDOHJbsoqZ+WrMW3y7txjqFqKgTcTFTzT9UTxhnKjolE
6b2i+uR0f/Sipk40ptvNnUAi+ZDEHsGQFrqf4ehLWNf3vRNTq0A/Cn7yggNieZwS
sd/oBZsWjJSWxWyPi4kyEboy8Db77bsUXsNxxxk8+hOlxoANwLbJKh+jJbuczebZ
x+EifgL2BxeZbMDOvvunhuvGj+1+3aJHVZdfxRjua8ZMHzuPe48ViJAFzjg7oTgz
d/Nlb/8wNtp7Wu96ZoeUnFabj3tmO76ixmcj/bqHYiDQc6sg3Ax/M4oL+uZ087x9
AkATbF0OUezdah4RlyhHth3K6VuBJxWn9H/kMP8uAhC5ijqEeZRl3X/lzgHDcliy
3JqXBr920My72YV8Y87HvWK0Wt3M3DOZUPaS3f8MX/9kXIvB3Z/zlb121XUhL+DX
P6lIE2Y/EWEST8DAoIUmBiT5RB0w1q813+NzreN/ZVUIL6LX/dKnFGmlOeHYl/pM
x4LgE+ZxGtZKe6DoshE8fUWB2jiA6GvqDRm27gKxVyUNFQ0jyp8+tGGUO7lphBDU
kcX4EPspNEJzkz1KtgQ2
=LaZW
-----END PGP SIGNATURE-----