Re: [PATCH]: Allow errors in parameter values to be reported during the BIND phase itself..

Justin Pryzby <pryzby@telsasoft.com>

From: Justin Pryzby <pryzby@telsasoft.com>
To: Alvaro Herrera <alvherre@alvh.no-ip.org>
Cc: Alexey Bashtanov <bashtanov@imap.cc>, Franck Verrot <franck@verrot.fr>, pgsql-hackers@postgresql.org
Date: 2021-02-22T17:15:25Z
Lists: pgsql-hackers

Attachments

On Sun, Feb 21, 2021 at 11:05:26PM -0600, Justin Pryzby wrote:
> On Mon, Jan 04, 2021 at 11:09:39AM -0600, Justin Pryzby wrote:
> > For example:
> > 
> > $ python3.5 -c "import pg; db=pg.DB(); q = db.query(\"SET log_parameter_max_length_on_error=-1;\"); db.prepare('p', 'SELECT \$1::smallint'); db.query_prepared('p',66666);"
> > 2021-01-03 02:21:04.547 CST [20157] ERROR:  value "66666" is out of range for type smallint
> > 2021-01-03 02:21:04.547 CST [20157] CONTEXT:  unnamed portal with parameters: $1 = '66666'
> > 2021-01-03 02:21:04.547 CST [20157] STATEMENT:  SELECT $1::smallint
> > 
> > When there are many bind params, this can be useful to determine which is out
> > of range.  Think 900 int/smallint columns, or less-wide tables being inserted
> > multiple rows at a time with VALUES(),(),()...
> 
> This fixes a crash when there are zero bind params, and the error context was
> popped after not being pushed.

The previous patch was dysfynctional due to params_data.params = NULL.
This also fixes an issue with string termination.
And added a new test case.

I didn't understand how the existing pgbench test was working; apparently the
first of these queries generates parameter value CONTEXT messages in v13, but
(without my patch) not the 2nd.

| select column1::smallint from (values ($1)) as q;
| select $1::smallint

In the first case, it looks like:

        postgres: pryzbyj postgres [local] BIND(int2in+0x11) [0x55a57addd461]
        postgres: pryzbyj postgres [local] BIND(evaluate_expr+0x94) [0x55a57acba634]
        postgres: pryzbyj postgres [local] BIND(expression_tree_mutator+0x99) [0x55a57ac5a839]
        postgres: pryzbyj postgres [local] BIND(expression_tree_mutator+0x31e) [0x55a57ac5aabe]
        postgres: pryzbyj postgres [local] BIND(eval_const_expressions+0x53) [0x55a57acbbc73]
        postgres: pryzbyj postgres [local] BIND(subquery_planner+0x5a9) [0x55a57aca7159]
        postgres: pryzbyj postgres [local] BIND(standard_planner+0x113) [0x55a57aca8a63]
        postgres: pryzbyj postgres [local] BIND(pg_plan_query+0x4c) [0x55a57ad7cbfc]
        postgres: pryzbyj postgres [local] BIND(pg_plan_queries+0x41) [0x55a57ad7ccf1]
        postgres: pryzbyj postgres [local] BIND(GetCachedPlan+0x7d) [0x55a57ae87b9d]
        postgres: pryzbyj postgres [local] BIND(PostgresMain+0xe82) [0x55a57ad7e352]

In the 2nd case:

        postgres: pryzbyj postgres [local] BIND(int2in+0x11) [0x55a57addd461]
        postgres: pryzbyj postgres [local] BIND(InputFunctionCall+0x7f) [0x55a57aea19df]
        postgres: pryzbyj postgres [local] BIND(OidInputFunctionCall+0x4d) [0x55a57aea1bad]
        postgres: pryzbyj postgres [local] BIND(PostgresMain+0x1754) [0x55a57ad7ec24]

This could be considered a bugfix to v13, since ba79cb5dc says:
"Emit parameter values during query bind/execute errors"

-- 
Justin

Commits

  1. Improve logging of bad parameter values in BIND messages.