Re: proposal psql \gdesc

From: Fabien COELHO <coelho(at)cri(dot)ensmp(dot)fr>
To: Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>
Cc: PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: proposal psql \gdesc
Date: 2017-05-07 20:55:44
Message-ID: alpine.DEB.2.20.1705072201120.3983@lancre
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers


Hello Pavel,

>> Sometimes I have to solve the result types of some query. It is invisible
>> in psql.

Indeed. My 0.02€ about this patch:

About the feature:

It looks useful to allow to show the resulting types of a query.

About the code:

Patch applies cleanly and compiles.

I'm afraid that re-using the "results" variable multiple times results in
memory leaks... ISTM that new variables should be used when going down the
nested conditions, and all cleanup should be done where and when
necessary.

Also, maybe it would be better if the statement is cleaned up server side
at the end of the execution. Not sure how to achieve that, though, libpq
seems to lack the relevant function:-(

"""although there is no libpq function for deleting a prepared
statement, the SQL DEALLOCATE statement can be used for that purpose."""

Hmmm... I have not found how to use DEALLOCATE to cleanup an unnamed
statement, it does not allow a "zero-length" name. Maybe it could be
extended somehow, or a function could be provided for the purpose, eg
by passing a NULL query to PQprepare...

Resetting "gdesc flag" could be done only when the flag was true, at the
end of the if, rather than on each query.

I understand that the second level query is to retrieve the type names in
place of the Oid returned by QPftype?

The pg_malloc length computation looks a little bit arbitrary. Would it
make sense to use PQescapeLiteral instead?

About the documentation:

I would suggest some rewording, maybe:

"Show the description of the result of the current query buffer without
actually executing it, by considering it a prepared statement."

About tests:

There should be some non-regression tests added, maybe something like:

SELECT
NULL AS zero,
1 AS one,
2.0 AS two,
'three' AS three,
$1 AS four,
CURRENT_DATE AS now
-- ...
\gdesc

And also case which trigger an error, eg:

SELECT $1 AS unknown_type \gdesc
SELECT 1 + \gdesc

Some fun:

PREPARE test AS SELECT 1;
EXECUTE test \gdesc
...

I'm unsure about some error messages, but it may be unrelated to this
patch:

calvin=# SELECT '1 km'::unit AS foo, $2 as boo \gdesc
ERROR: could not determine data type of parameter $1

--
Fabien.

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Daniele Varrazzo 2017-05-07 21:54:26 Re: Detecting schema changes during logical replication
Previous Message Mat Arye 2017-05-07 20:51:40 Re: Question about toasting code