Re: patch for 9.2: enhanced errors

From: Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>
To: Steve Singer <ssinger_pg(at)sympatico(dot)ca>
Cc: PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: patch for 9.2: enhanced errors
Date: 2011-06-20 19:44:20
Message-ID: BANLkTik0S1kzyLFG1Pv6KhTNMqqUHh-zkA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hello

I am sending a updated patch

>
> Coding Review
> -----------------
>
>
> In tupdesc.c
>
> line 202 the existing code is performing a deep copy of ConstrCheck.  Do you
> need to copy nkeys and conkey here as well?
>
> Then at line 250 ccname is freed but not conkey
>

fixed

>
> postgres_ext.h line 55
> + #define PG_DIAG_SCHEMA_NAME    's'
> + #define PG_DIAG_TABLE_NAME    't'
> + #define PG_DIAG_COLUMN_NAMES    'c'
> + #define PG_DIAG_CONSTRAINT_NAME 'n'
>
> The assignment of letters to parameters seems arbitrary to me, I don't have
> a different non-arbitrary mapping in mind but if anyone else does they
> should speak up.  I think it will be difficult to change this after 9.2 goes
> out.
>
>
> elog.c:
> ***************
> *** 2197,2202 ****
> --- 2299,2319 ----
>      if (application_name)
>          appendCSVLiteral(&buf, application_name);
>
> +     /* constraint_name */
> +     appendCSVLiteral(&buf, edata->constraint_name);
> +     appendStringInfoChar(&buf, ',');
> +
> +     /* schema name */
> +     appendCSVLiteral(&buf, edata->schema_name);
> +     appendStringInfoChar(&buf, ',');
>
> You need to update config.sgml at the same time you update this format.
> You need to append a "," after application name but before constraintName.
> As it stands the CSV log has something like:
> .....nbtinsert.c:433","psql""a_pkey","public","a","a"

fixed

>
>
> nbtinsert.c
>
> pg_get_indrelation is named differently than everything else in this file
> (ie _bt...).  My guess is that this function belongs somewhere else but I
> don't know the code well enough to say where you should move it too.
>

I renamed this function to IndexRelationGetParentRelation and muved to
relcache.c

I don't call a quote_identifier on only data error properties like
table_name or schema_name (but I am open to arguments for it or
against it). The quote_identifier is used for column names, because
there should be a more names and comma should be used inside name -
and this is consistent with pg_get_indexdef_columns.

Regards

Pavel Stehule

Attachment Content-Type Size
enhanced-errors-2.diff text/x-patch 25.4 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Darren Duncan 2011-06-20 19:54:52 Re: Range Types and extensions
Previous Message Florian Pflug 2011-06-20 19:19:04 Re: Range Types and extensions