[PATCH] Add result_types column to pg_prepared_statements view

Started by Dagfinn Ilmari Mannsåkeralmost 4 years ago8 messageshackers
Jump to latest

Hi hackers,

Prompted by a question on IRC, here's a patch to add a result_types
column to the pg_prepared_statements view, so that one can see the types
of the columns returned by a prepared statement, not just the parameter
types.

I'm not quite sure about the column name, suggestions welcome.

- ilmari

Attachments:

0001-Add-result_types-column-to-pg_prepared_statements-vi.patchtext/x-diffDownload+73-46
In reply to: Dagfinn Ilmari Mannsåker (#1)
Re: [PATCH] Add result_types column to pg_prepared_statements view

Dagfinn Ilmari Mannsåker <ilmari@ilmari.org> writes:

Hi hackers,

Prompted by a question on IRC, here's a patch to add a result_types
column to the pg_prepared_statements view, so that one can see the types
of the columns returned by a prepared statement, not just the parameter
types.

Added to the 2022-07 commitfest: https://commitfest.postgresql.org/38/3644/

- ilmari

#3Peter Eisentraut
peter_e@gmx.net
In reply to: Dagfinn Ilmari Mannsåker (#1)
Re: [PATCH] Add result_types column to pg_prepared_statements view

On 19.05.22 17:34, Dagfinn Ilmari Mannsåker wrote:

Prompted by a question on IRC, here's a patch to add a result_types
column to the pg_prepared_statements view, so that one can see the types
of the columns returned by a prepared statement, not just the parameter
types.

I'm not quite sure about the column name, suggestions welcome.

I think this patch is sensible.

I see one issue: When you describe a prepared statement via the
protocol, if a result field has a domain as its type, the RowDescription
message sends the underlying base type, not the domain type directly
(see SendRowDescriptionMessage()). But it doesn't do that for the
parameters (see exec_describe_statement_message()). I don't know why
that is; the protocol documentation doesn't mention it. Might be worth
looking into, and checking whether the analogous information contained
in this view should be made consistent.

In reply to: Peter Eisentraut (#3)
Re: [PATCH] Add result_types column to pg_prepared_statements view

Peter Eisentraut <peter.eisentraut@enterprisedb.com> writes:

On 19.05.22 17:34, Dagfinn Ilmari Mannsåker wrote:

Prompted by a question on IRC, here's a patch to add a result_types
column to the pg_prepared_statements view, so that one can see the types
of the columns returned by a prepared statement, not just the parameter
types.
I'm not quite sure about the column name, suggestions welcome.

I think this patch is sensible.

I see one issue: When you describe a prepared statement via the
protocol, if a result field has a domain as its type, the RowDescription
message sends the underlying base type, not the domain type directly
(see SendRowDescriptionMessage()). But it doesn't do that for the
parameters (see exec_describe_statement_message()). I don't know why
that is; the protocol documentation doesn't mention it. Might be worth
looking into, and checking whether the analogous information contained
in this view should be made consistent.

A bit of git archaeology shows that the change was made by Tom (Cc-ed)
in 7.4:

commit d9b679c13a820eb7b464a1eeb1f177c3fea13ece
Author: Tom Lane <tgl@sss.pgh.pa.us>
Date: 2003-05-13 18:39:50 +0000

In RowDescription messages, report columns of domain datatypes as having
the type OID and typmod of the underlying base type. Per discussions
a few weeks ago with Andreas Pflug and others. Note that this behavioral
change affects both old- and new-protocol clients.

I can't find that discussion in the archive, but someone did complain
about it shortly after:

/messages/by-id/D71A1574-A772-11D7-913D-0030656EE7B2@icx.net

I think in this case returning the domain type is more useful, since
it's easy to get from that to the base type, but not vice versa.

The arguments about client-side type-specific value handling for
RowDescription don't apply here IMO, since this view is more
user-facing.

- ilmari

#5Peter Eisentraut
peter_e@gmx.net
In reply to: Dagfinn Ilmari Mannsåker (#4)
Re: [PATCH] Add result_types column to pg_prepared_statements view

On 01.07.22 14:27, Dagfinn Ilmari Mannsåker wrote:

Peter Eisentraut <peter.eisentraut@enterprisedb.com> writes:

On 19.05.22 17:34, Dagfinn Ilmari Mannsåker wrote:

Prompted by a question on IRC, here's a patch to add a result_types
column to the pg_prepared_statements view, so that one can see the types
of the columns returned by a prepared statement, not just the parameter
types.
I'm not quite sure about the column name, suggestions welcome.

I think this patch is sensible.

The arguments about client-side type-specific value handling for
RowDescription don't apply here IMO, since this view is more
user-facing.

I agree. It's also easy to change if needed. Committed as is.

In reply to: Peter Eisentraut (#5)
Re: [PATCH] Add result_types column to pg_prepared_statements view

On Tue, 5 Jul 2022, at 06:34, Peter Eisentraut wrote:

On 01.07.22 14:27, Dagfinn Ilmari Mannsåker wrote:

Peter Eisentraut <peter.eisentraut@enterprisedb.com> writes:

On 19.05.22 17:34, Dagfinn Ilmari Mannsåker wrote:

Prompted by a question on IRC, here's a patch to add a result_types
column to the pg_prepared_statements view, so that one can see the types
of the columns returned by a prepared statement, not just the parameter
types.
I'm not quite sure about the column name, suggestions welcome.

I think this patch is sensible.

The arguments about client-side type-specific value handling for
RowDescription don't apply here IMO, since this view is more
user-facing.

I agree. It's also easy to change if needed. Committed as is.

Thanks!

#7Peter Eisentraut
peter_e@gmx.net
In reply to: Dagfinn Ilmari Mannsåker (#6)
Re: [PATCH] Add result_types column to pg_prepared_statements view

On 05.07.22 09:31, Dagfinn Ilmari Mannsåker wrote:

On Tue, 5 Jul 2022, at 06:34, Peter Eisentraut wrote:

On 01.07.22 14:27, Dagfinn Ilmari Mannsåker wrote:

Peter Eisentraut <peter.eisentraut@enterprisedb.com> writes:

On 19.05.22 17:34, Dagfinn Ilmari Mannsåker wrote:

Prompted by a question on IRC, here's a patch to add a result_types
column to the pg_prepared_statements view, so that one can see the types
of the columns returned by a prepared statement, not just the parameter
types.
I'm not quite sure about the column name, suggestions welcome.

I think this patch is sensible.

The arguments about client-side type-specific value handling for
RowDescription don't apply here IMO, since this view is more
user-facing.

I agree. It's also easy to change if needed. Committed as is.

Thanks!

There was a problem that we didn't cover: Not all prepared statements
have result descriptors (e.g., DML statements), so that would crash as
written. I have changed it to return null for result_types in that
case, and added a test case.

In reply to: Peter Eisentraut (#7)
Re: [PATCH] Add result_types column to pg_prepared_statements view

Peter Eisentraut <peter.eisentraut@enterprisedb.com> writes:

There was a problem that we didn't cover: Not all prepared statements
have result descriptors (e.g., DML statements), so that would crash as
written.

D'oh!

I have changed it to return null for result_types in that case, and
added a test case.

Thanks for spotting and fixing that.

- ilmari