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

digoal(at)126(dot)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