binary array and record recv

Started by Andrew Chernowabout 18 years ago11 messages
#1Andrew Chernow
ac@esilo.com

Both array and record binary recv functions require that the recv'd Oid
match the Oid of what the backend has deduced. We don't think this
behavior gets you very much. The backend apparently knows the Oid of a
composite or array elemtype, so if the sender chooses to send InvalidOid
why shouldn't that translate into the backend's idea of the Oid.

array_recv line 1204

element_type = pq_getmsgint(buf, sizeof(Oid));
if (element_type != spec_element_type)
{
/* XXX Can we allow taking the input element type in any cases? */
ereport(ERROR,
(errcode(ERRCODE_DATATYPE_MISMATCH),
errmsg("wrong element type")));
}

Why not?

element_type = pq_getmsgint(buf, sizeof(Oid));
if(element_type == InvalidOid) // use backend's oid
element_type = spec_element_type;
if (element_type != spec_element_type)
[snip...]

record_recv line 523 -- Same with composites

coltypoid = pq_getmsgint(buf, sizeof(Oid));
if(coltypoid == InvalidOid) // use backend's oid
coltypoid = column_type;
if (coltypoid != column_type)
ereport(ERROR,
(errcode(ERRCODE_DATATYPE_MISMATCH),
errmsg("wrong data type: %u, expected %u",
coltypoid, column_type)));

If it is desired to have strict type checking, then the sender can still
send the Oid. In this case, if the Oid doesn't match the backend's an
error is raised (current behavior).

Without this change, a libpq client is forced to know the Oids of all
types within a composite and all types used with arrays. Currently,
there is no way for a libpq client to know the Oid of a composite. The
client would have to query them from the server or hard-code them.
Hard-coding is a bad idea unless you are using a built-in type. libpq
could have an API call:

PQlookupOid(PGconn *conn, char **type_names, Oid *oids, int count);

Oid oid[2];
char *types[] = {"public.type_a", "myschema.type_a"};
PQlookupOid(conn, types, 2, oid);

andrew & merlin
eSilo

#2Andrew Chernow
ac@esilo.com
In reply to: Andrew Chernow (#1)
Re: binary array and record recv

PQlookupOid(PGconn *conn, char **type_names, Oid *oids, int count);

We are backing away from this (not such a great idea). We are actually
working hard at removing Oid dependencies from our PGparam idea. We
think it is more generic to make the server allow InvalidOid for
composites and array elmtypes, as Oids can change from server to server.

andrew & merlin
eSilo

#3Tom Lane
tgl@sss.pgh.pa.us
In reply to: Andrew Chernow (#1)
Re: binary array and record recv

Andrew Chernow <ac@esilo.com> writes:

Both array and record binary recv functions require that the recv'd Oid
match the Oid of what the backend has deduced. We don't think this
behavior gets you very much.

Other than the ability to detect errors before the code goes off into
the weeds, you mean?

regards, tom lane

#4Andrew Chernow
ac@esilo.com
In reply to: Tom Lane (#3)
Re: binary array and record recv

Tom Lane wrote:

Andrew Chernow <ac@esilo.com> writes:

Both array and record binary recv functions require that the recv'd Oid
match the Oid of what the backend has deduced. We don't think this
behavior gets you very much.

Other than the ability to detect errors before the code goes off into
the weeds, you mean?

regards, tom lane

When dealing with binary, the Oid the client sends may match what the
server thinks but the data is wrong (client sent binary formatted data
of the wrong type). Thus, the only real check we saw was on the data
length (which is rolling the dice).

Plus for non-array and non-composite types, specifying Oid is optional.
So why is it not optional for arrays/composites?

How is the client supposed to send back composite types without having a
meaningful way to get the Oids from the server? Either you have to be
flexible and trust what the client sends or you have to make it possbile
for the client to get the proper information.

Third option is to not allow client to send binary composite/array types
at all.

andrew & merlin
eSilo

#5Tom Lane
tgl@sss.pgh.pa.us
In reply to: Andrew Chernow (#4)
Re: binary array and record recv

Andrew Chernow <ac@esilo.com> writes:

When dealing with binary, the Oid the client sends may match what the
server thinks but the data is wrong (client sent binary formatted data
of the wrong type). Thus, the only real check we saw was on the data
length (which is rolling the dice).

Yes, the available checks to detect client-vs-server datatype mismatch
are very weak, but that hardly comes across as a good argument for
removing one of the ones we have.

How is the client supposed to send back composite types without having a
meaningful way to get the Oids from the server?

On what grounds do you claim that it can't get the type Oids from the
server?

regards, tom lane

#6Merlin Moncure
mmoncure@gmail.com
In reply to: Tom Lane (#5)
Re: binary array and record recv

On Dec 18, 2007 11:30 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Andrew Chernow <ac@esilo.com> writes:

When dealing with binary, the Oid the client sends may match what the
server thinks but the data is wrong (client sent binary formatted data
of the wrong type). Thus, the only real check we saw was on the data
length (which is rolling the dice).

Yes, the available checks to detect client-vs-server datatype mismatch
are very weak, but that hardly comes across as a good argument for
removing one of the ones we have.

We are not suggesting to remove the check...but only to disable it
when the client explicitly asks the server. Right now, the checking
on composites is actually much stricter than on regular query
parameters. This strictness forces the client to 'know' the oid of
every field inside the composite.

How is the client supposed to send back composite types without having a
meaningful way to get the Oids from the server?

On what grounds do you claim that it can't get the type Oids from the
server?

Well, it's certainly possible, but awkward. The client has to
directly query the system catalogs. This has problems....there are
invalidation issues (which our idea does not address) and the user is
not guaranteed to have permission to access the proper catalogs. If
we go this route, IMO there should be a API function to expose this
behavior.

With the above issues, it seems that it would be nicer not to expose
Oids to the client at all but instead do parameter binding directly on
typename. ISTM this is not possible though.

merlin & andrew
eSilo

#7Andrew Chernow
ac@esilo.com
In reply to: Tom Lane (#5)
Re: binary array and record recv

Tom Lane wrote:

Andrew Chernow <ac@esilo.com> writes:

When dealing with binary, the Oid the client sends may match what the
server thinks but the data is wrong (client sent binary formatted data
of the wrong type). Thus, the only real check we saw was on the data
length (which is rolling the dice).

Yes, the available checks to detect client-vs-server datatype mismatch
are very weak, but that hardly comes across as a good argument for
removing one of the ones we have.

How is the client supposed to send back composite types without having a
meaningful way to get the Oids from the server?

On what grounds do you claim that it can't get the type Oids from the
server?

regards, tom lane

I was wondering if anyone would be opposed to a small allowance here.
When trying to send a NULL value for a record/composite column, is it
possible to simply ignore the recv'd oid? Currently, the oid of each
column is read and checked against the server. If the oids don't match,
an error is raised. I see Tom's take on this, a weak check is better
than no check at all. But when passing in a NULL value, setting length
to -1, I'm not sure this check even registers as a weak one.

Maybe:

for each record column
1. read the oid
2. read the len
3. if oid != server_oid && len!=-1
error out, wrong data type

andrew

#8Peter Eisentraut
peter_e@gmx.net
In reply to: Tom Lane (#5)
Re: binary array and record recv

On Tuesday 18 December 2007 18:30:22 Tom Lane wrote:

Arguably, pg_dump from an older version should make sure that the auto
rules should NOT get created, else it is failing to preserve an older
view's behavior.

We extend properties of objects all the time. That is why we make new
releases. No one is required to use the new properties.

Should pg_dump also make sure that tables imported from an older version are
not usable for recursive unions or window functions, thus preserving the
older table's behavior?

#9Peter Eisentraut
peter_e@gmx.net
In reply to: Peter Eisentraut (#8)
Re: binary array and record recv

On Tuesday 27 January 2009 14:07:26 Peter Eisentraut wrote:

On Tuesday 18 December 2007 18:30:22 Tom Lane wrote:

Arguably, pg_dump from an older version should make sure that the auto
rules should NOT get created, else it is failing to preserve an older
view's behavior.

We extend properties of objects all the time. That is why we make new
releases. No one is required to use the new properties.

Should pg_dump also make sure that tables imported from an older version
are not usable for recursive unions or window functions, thus preserving
the older table's behavior?

This curiously came through with a wrong subject. The point had already been
made, though.

#10Tom Lane
tgl@sss.pgh.pa.us
In reply to: Peter Eisentraut (#8)
Re: binary array and record recv

Peter Eisentraut <peter_e@gmx.net> writes:

On Tuesday 18 December 2007 18:30:22 Tom Lane wrote:

Arguably, pg_dump from an older version should make sure that the auto
rules should NOT get created, else it is failing to preserve an older
view's behavior.

We extend properties of objects all the time. That is why we make new
releases. No one is required to use the new properties.

Should pg_dump also make sure that tables imported from an older version are
not usable for recursive unions or window functions, thus preserving the
older table's behavior?

That argument seems fairly bogus. The addition of those features won't
change the behavior of existing applications.

regards, tom lane

#11Alvaro Herrera
alvherre@commandprompt.com
In reply to: Tom Lane (#10)
Re: binary array and record recv

Tom Lane wrote:

Peter Eisentraut <peter_e@gmx.net> writes:

On Tuesday 18 December 2007 18:30:22 Tom Lane wrote:

Arguably, pg_dump from an older version should make sure that the auto
rules should NOT get created, else it is failing to preserve an older
view's behavior.

We extend properties of objects all the time. That is why we make new
releases. No one is required to use the new properties.

Should pg_dump also make sure that tables imported from an older version are
not usable for recursive unions or window functions, thus preserving the
older table's behavior?

That argument seems fairly bogus. The addition of those features won't
change the behavior of existing applications.

How will adding updatable views change them? The only change is that
when you try to insert/update/delete on a view, it used to give an
error, but the new version will accept it. How can this be a problem?
Surely no application is depending on the fact that this will raise an
error.

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