Re: [GENERAL] dblink - custom datatypes don't work

Hello,
I've been experimenting with dblink recently, and have encountered some
limitations I'd like to discuss.

I've been trying to create views of remote tables, like so:

CREATE VIEW stuff AS
SELECT *
FROM dblink(' ... remote connection info ... ',
'SELECT id, title, title_idx FROM stuff')
AS t(
id integer,
title text,
title_idx txtidx
);

But, dblink seems to have a problem with the custom datatype 'txtidx'
(from contrib/tsearch).

I get an error like this (from PostgreSQL 7.4.1):

ERROR: cache lookup failed for type 123456

Where 123456 is the pg_type.oid of the 'txtidx' type in the remote database,
which differs from the oid of the same datatype within the local database.

Are there anyways around this (other than trying to initialise the
datatypes remotely and
locally with the same oid - which would be highly impractical).

Is this a limitation of PostgreSQL or dblink?
Could dblink use type names instead of oid's?
If not, could dblink be adapted to use some kind of
remote oid -> local oid mapping table for datatypes?

I would be willing to have a poke around in dblink.c,
if someone could confirm my findings and point me in the right direction.

Cheers

--
Mark Gibson <gibsonm(at)cromwell(dot)co(dot)uk>
Web Developer & Database Admin
Cromwell Tools Ltd.
Leicester, England.

Mark Gibson wrote:
> [custom datatype oid mismatch between local and remote side of dblink ]

> Is this a limitation of PostgreSQL or dblink?
> Could dblink use type names instead of oid's?
> If not, could dblink be adapted to use some kind of
> remote oid -> local oid mapping table for datatypes?
>
> I would be willing to have a poke around in dblink.c,
> if someone could confirm my findings and point me in the right direction.

Without actually having tried it, I think you could hack
pgresultGetTupleDesc() in dblink.c. Replace the line:
atttypid = PQftype(res, i);
with an SPI based local lookup using attname.

But I'm not sure if it is really a good idea in general to assume that
the same name for a datatype in two databases means that they are
actually the same datatype. It would be nice if there was a way to
register a "signature" of some kind for custom datatypes that would be
unique enough to be sure same named types were actually the same.

Joe

Joe Conway wrote:

> Mark Gibson wrote:
>
>> [custom datatype oid mismatch between local and remote side of dblink ]
>
>
> Without actually having tried it, I think you could hack
> pgresultGetTupleDesc() in dblink.c. Replace the line:
> atttypid = PQftype(res, i);
> with an SPI based local lookup using attname.
>
> But I'm not sure if it is really a good idea in general to assume that
> the same name for a datatype in two databases means that they are
> actually the same datatype. It would be nice if there was a way to
> register a "signature" of some kind for custom datatypes that would be
> unique enough to be sure same named types were actually the same.
>
But matching datatypes by name would surely be safer than matching by oid -
it's not impossible that different datatypes on each side of the dblink
share the same oid.
Infact, would it not be more likely that two datatypes with the same
name are actually
the same datatype, than two types with the same oid, not counting
internal types.
But then again, are internal types guaranteed to share the same oid between
PostgreSQL versions?

I think a signature would be very difficult to maintain, it would have
to be consistent for
a datatype (across platforms, and versions of PostgreSQL) while it's
semantics remained
the same, and also be universally unique - central registry etc., urgghh :p

Could a config option be added somewhere to switch between oid and name
matching.
With name matching it could be a policy of dblink usage that datatype
names must remain
consistent between db's - but I think this would generally be a good
thing away :)

Anyway, I'm off to hack around in dblink.c, it's the first time I've
tried SPI,
should be fun! Thanks for your help.

Cheers

--
Mark Gibson <gibsonm |AT| cromwell |DOT| co |DOT| uk>
Web Developer & Database Admin
Cromwell Tools Ltd.
Leicester, England.

[ moving to HACKERS ]

Mark Gibson wrote:
> Joe Conway wrote:
>> Mark Gibson wrote:
>>> [custom datatype oid mismatch between local and remote side of
>>> dblink ]
>>
>> Without actually having tried it, I think you could hack
>> pgresultGetTupleDesc() in dblink.c. Replace the line: atttypid =
>> PQftype(res, i); with an SPI based local lookup using attname.
>>
>> But I'm not sure if it is really a good idea in general to assume
>> that the same name for a datatype in two databases means that they
>> are actually the same datatype. It would be nice if there was a way
>> to register a "signature" of some kind for custom datatypes that
>> would be unique enough to be sure same named types were actually
>> the same.
>>
> But matching datatypes by name would surely be safer than matching by
> oid - it's not impossible that different datatypes on each side of
> the dblink share the same oid. Infact, would it not be more likely
> that two datatypes with the same name are actually the same datatype,
> than two types with the same oid, not counting internal types. But
> then again, are internal types guaranteed to share the same oid
> between PostgreSQL versions?

Sounds reasonable -- any other thoughts out there?

The extra SPI lookup does add cost to every use of the function though.
We'd want to figure out how to cache the results of the lookup.

> Could a config option be added somewhere to switch between oid and
> name matching. With name matching it could be a policy of dblink
> usage that datatype names must remain consistent between db's - but I
> think this would generally be a good thing away :)

I'd be inclined to say that if matching on type name is thought to be
better, we ought to just go that way wholesale.

Joe

Joe Conway wrote:

>>> Without actually having tried it, I think you could hack
>>> pgresultGetTupleDesc() in dblink.c. Replace the line: atttypid =
>>> PQftype(res, i); with an SPI based local lookup using attname.
>>

Hi,
Well I've been adventuring into the realms of SPI and dblink over
the last couple of days.
I've taken the approach of having a table that maps remote oids to local
oids:

CREATE TABLE public.dblink_oid_map (
db_id name, -- unique identifier for a remote database (see notes below)
remote_oid oid,
local_oid oid,
typname name, -- name of the datatype on remote database (aids error
reporting)
PRIMARY KEY (db_id,remote_oid)
)
WITHOUT OIDS;

I've modified dblink.c to look up the local oid from this table
(see attached patch, against 7.4.1).
By default all dblink functions will use this table.
The table can be built using dblink itself, but to avoid a chicken-egg
situation
I've created a new variation on the dblink function:

CREATE OR REPLACE FUNCTION public.dblink (text,text,bool)
RETURNS setof record
AS '$libdir/dblink','dblink_record'
LANGUAGE 'C' WITH (isstrict);

The last arg can be set to FALSE to disable the oid map.
This allows the following function to build the oid map for a remote db:
(The arg is a connection string passed to dblink)

CREATE OR REPLACE FUNCTION public.dblink_update_oid_map(text)
RETURNS void
AS '
DECLARE
v_connstr ALIAS FOR $1;
v_db_id name := ''test'';
BEGIN
DELETE FROM dblink_oid_map WHERE db_id = v_db_id;
INSERT INTO dblink_oid_map (db_id, remote_oid, local_oid, typname)
SELECT
v_db_id, r.oid, l.oid, r.typname
FROM
dblink(v_connstr,
''SELECT oid, typname FROM pg_type WHERE typtype IN
(''''b'''',''''d'''')'',
false
) AS r (oid oid, typname name)
LEFT JOIN pg_type l ON (l.typname = r.typname);
RETURN;
END;
' LANGUAGE 'plpgsql';

I wasn't sure what to do about datatypes in different schemas,
this approach allows an admin to customise the map.
I usually have all custom types in 'public' so it isn't a problem for me.

All datatypes from the remote database are included in the table,
so in some cases 'local_oid' may be NULL -
the patch handles this by reporting an error informing the user that the
datatype
doesn't exist locally, hence the 'typname' column.

We need to find a way to uniquely identify a remote database, and create a
consistent id algorythm for the 'db_id' column.
A hash of the connection string attributes: 'host'/'hostaddr' + 'dbname'
+ 'port' ???
For testing i've used an db_id of type 'name' with the value: 'test'.

Unfortunately, it doesn't work using the oid the map, whether custom types
are involved or not. All I get is the following message:

ERROR: unsupported byval length: nnnn

SPI is very new to me (like 2 days old ;).
Any suggestions where I've gone wrong?

Cheers

--
Mark Gibson <gibsonm |AT| cromwell |DOT| co |DOT| uk>
Web Developer & Database Admin
Cromwell Tools Ltd.
Leicester, England.

Mark Gibson <gibsonm(at)cromwell(dot)co(dot)uk> writes:
> Unfortunately, it doesn't work using the oid the map, whether custom types
> are involved or not. All I get is the following message:
> ERROR: unsupported byval length: nnnn
> SPI is very new to me (like 2 days old ;).
> Any suggestions where I've gone wrong?

Not offhand. Try setting a breakpoint at errfinish() so you can get a
stack trace back from the point of the error; that should help.

regards, tom lane

Tom Lane wrote:

>Mark Gibson <gibsonm(at)cromwell(dot)co(dot)uk> writes:
>
>
>>Unfortunately, it doesn't work using the oid the map, whether custom types
>>are involved or not. All I get is the following message:
>>ERROR: unsupported byval length: nnnn
>>SPI is very new to me (like 2 days old ;).
>>Any suggestions where I've gone wrong?
>>
>>
>Not offhand. Try setting a breakpoint at errfinish() so you can get a
>stack trace back from the point of the error; that should help.
>
>

Well, I've re-compiled PostgreSQL 7.4.1 with --enable-debug
(was previously working with a Gentoo ebuild of it).
And strangely I get a different error now:

ERROR: query-specified return row and actual function return row do not
match

After placing a few elog statements around pgresultGetTupleDesc, i've
found the
error happens after this function.

I've been trying to use gdb to trace the problem, but I can't work out
how to attach
gdb to a backend process. (I've never tried to debug a multi process before)
Are there any docs on debugging the backend processes?
I've searched www.postgresql.org without luck :(

--
Mark Gibson <gibsonm |AT| cromwell |DOT| co |DOT| uk>
Web Developer & Database Admin
Cromwell Tools Ltd.
Leicester, England.

Mark Gibson <gibsonm(at)cromwell(dot)co(dot)uk> writes:
> I've been trying to use gdb to trace the problem, but I can't work out
> how to attach gdb to a backend process.

It's not hard. Start psql, then in another window use ps to determine
the PID of the connected backend. (The pg_stat_activity view might help
too.) Then

gdb /path/to/postgres-executable PID
gdb> b whereever
gdb> cont

and away you go.

regards, tom lane

Tom Lane wrote:
> It's not hard. Start psql, then in another window use ps to determine
> the PID of the connected backend. (The pg_stat_activity view might help
> too.) Then
>
> gdb /path/to/postgres-executable PID
> gdb> b whereever
> gdb> cont
>
> and away you go.

I'd add that when working with shared libraries, you need to make sure
they are loaded before you attach to the process (I think there is a way
to deal with it after the fact, but I'm not sure of the details). To do
that, either execute a function in the library (e.g. select
dblink_connect...) or use the LOAD command like this:

regression=# load '$libdir/dblink';
LOAD

HTH,

Joe

Joe Conway <mail(at)joeconway(dot)com> writes:
> I'd add that when working with shared libraries, you need to make sure
> they are loaded before you attach to the process (I think there is a way
> to deal with it after the fact, but I'm not sure of the details).

On HPUX, I find that "sharedlibrary /path/to/library" works to teach gdb
about symbols in a freshly loaded library. (Occasionally it seems to be
needed for libraries that were already present at attach time, too.)
Not sure if this applies to other OSes, but give it a try.

regards, tom lane

Mark Gibson wrote:

> Well, I've re-compiled PostgreSQL 7.4.1 with --enable-debug
> (was previously working with a Gentoo ebuild of it).
> And strangely I get a different error now:
>
> ERROR: query-specified return row and actual function return row do
> not match
>
> After placing a few elog statements around pgresultGetTupleDesc, i've
> found the
> error happens after this function.
>
Thanks for the tips of using gdb.

I've found that dblink works without the oid map, ie: dblink(... , ... ,
false)
but returns the error when enabled, ie: dblink(... , ... , true)

The only part of dblink affected by this flag (use_oid_map) is within
'pgresultGetTupleDesc'

and the only variable affected is 'atttypid'
- which is left alone if use_oid_map is false.
When use_oid_map is true, SPI is used to prepare a query,
and execute it for each attribute - atttypid is then assigned the
value retreived from the query.

So when no custom types are used, the same values are passed to
'TupleDescInitEntry' whether use_oid_map is true or false.

I just can't work it out!

--
Mark Gibson <gibsonm |AT| cromwell |DOT| co |DOT| uk>
Web Developer & Database Admin
Cromwell Tools Ltd.
Leicester, England.

> Mark Gibson wrote:
>
> I've found that dblink works without the oid map, ie: dblink(... , ...
> , false)
> but returns the error when enabled, ie: dblink(... , ... , true)
>

Hello,
I've found the problem, although I'm still a bit confused by it.

When the call to SPI_finish() at the end of pgresultGetTupleDesc
in my patch is removed, it works fine, custom datatypes and all :)

But I still don't understand why this is. Is this safe?

Also, I was thinking about how we could uniquely identify a database.
Are there any variables we could retreive from the remote db after
connecting to it that could form a consistent unique id for that db?

Is there a way we can determine whether 'pg_type'
in the remote database has been modified?
We could automatically update the oid map in that case.

--
Mark Gibson <gibsonm |AT| cromwell |DOT| co |DOT| uk>
Web Developer & Database Admin
Cromwell Tools Ltd.
Leicester, England.

Mark Gibson wrote:
> I've found the problem, although I'm still a bit confused by it.

I hate to do this to you now, but after some thought I think I have a
better approach -- I'd be interested in opinions on that assessment.

The attached eliminates pgresultGetTupleDesc() entirely and instead
depends on the TupleDesc passed as rsinfo->expectedDesc from the
executor. What this means is that the string representation of the
remote value (from the "out" function on the remote side, as provided by
libpq) will get fed into the "in" function corresponding to the local
type you assign in your SQL statement. Assuming the types on the two
sides are the same (or at least compatible), it should work well.

Please give this a try and let me know what you think.

Joe

Joe Conway wrote:

> Mark Gibson wrote:
>
>> I've found the problem, although I'm still a bit confused by it.
>
>
> I hate to do this to you now, but after some thought I think I have a
> better approach -- I'd be interested in opinions on that assessment.
>
> The attached eliminates pgresultGetTupleDesc() entirely and instead
> depends on the TupleDesc passed as rsinfo->expectedDesc from the
> executor. What this means is that the string representation of the
> remote value (from the "out" function on the remote side, as provided
> by libpq) will get fed into the "in" function corresponding to the
> local type you assign in your SQL statement. Assuming the types on the
> two sides are the same (or at least compatible), it should work well.
>
> Please give this a try and let me know what you think.

Fantastic, works perfectly (well I've only actually tested it with the
'txtidx' datatype).
That's so much better than my idea, i didn't like having the oid map
much anyway.
Oh well, I least I've learnt I little about PostgreSQL internals in the
process.
I can get back to what I was supposed to be doing now ;)

I'm going to give this a much through testing now.

Cheers

--
Mark Gibson <gibsonm |AT| cromwell |DOT| co |DOT| uk>
Web Developer & Database Admin
Cromwell Tools Ltd.
Leicester, England.

Mark Gibson wrote:
> Joe Conway wrote:
>> Please give this a try and let me know what you think.
>
> Fantastic, works perfectly (well I've only actually tested it with
> the 'txtidx' datatype). That's so much better than my idea, i didn't
> like having the oid map much anyway. Oh well, I least I've learnt I
> little about PostgreSQL internals in the process. I can get back to
> what I was supposed to be doing now ;)
>
> I'm going to give this a much through testing now.

I'd like to consider the attached a bugfix and apply for the upcoming
7.3.6 and 7.4.2 releases, as well as cvs tip. Any comments/objections?
If not I'll apply in about 24 hours.

Thanks,

Joe

Joe Conway <mail(at)joeconway(dot)com> writes:
> I'd like to consider the attached a bugfix and apply for the upcoming
> 7.3.6 and 7.4.2 releases, as well as cvs tip. Any comments/objections?

Two nitpicks (each applying in 2 places):

> ! if (!rsinfo)
> ! ereport(ERROR,
> ! (errcode(ERRCODE_SYNTAX_ERROR),
> ! errmsg("returning setof record is not " \
> ! "allowed in this context")));
> !

First, testing for null rsinfo isn't sufficient, since the resultinfo
mechanism could be used for other things; you need an IsA test too.
Second, is "syntax error" really the most appropriate classification for
this? Compare the code in functions.c:

ReturnSetInfo *rsi = (ReturnSetInfo *) fcinfo->resultinfo;

if (rsi && IsA(rsi, ReturnSetInfo))
rsi->isDone = ExprEndResult;
else
ereport(ERROR,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
errmsg("set-valued function called in context that cannot accept a set")));

(Also, the errmsg text seems a bit out of line with the wording of
comparable errors, but I can't offer better text offhand.)

regards, tom lane

Tom Lane wrote:
> Two nitpicks (each applying in 2 places):

> First, testing for null rsinfo isn't sufficient, since the resultinfo
> mechanism could be used for other things; you need an IsA test too.
> Second, is "syntax error" really the most appropriate classification for
> this?

> (Also, the errmsg text seems a bit out of line with the wording of
> comparable errors, but I can't offer better text offhand.)

Thanks for the feedback, Tom. Here's what I ended up with:

if (!rsinfo || !IsA(rsinfo, ReturnSetInfo))
ereport(ERROR,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
errmsg("function returning record called in context "
"that cannot accept type record")));

Joe