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 |
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 |