From: | Shigeru Hanada <shigeru(dot)hanada(at)gmail(dot)com> |
---|---|
To: | Kohei Kaigai <Kohei(dot)Kaigai(at)EMEA(dot)NEC(dot)COM> |
Cc: | Kohei KaiGai <kaigai(at)kaigai(dot)gr(dot)jp>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: force_not_null option support for file_fdw |
Date: | 2011-09-09 05:02:30 |
Message-ID: | 4E699DE6.8010606@gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Thanks for the review, Kaigai-san.
(2011/09/09 0:47), Kohei Kaigai wrote:
> I found one other point to be fixed:
> On get_force_not_null(), it makes a list of attribute names with force_not_null option.
>
> + foreach (cell, options)
> + {
> + DefElem *def = (DefElem *) lfirst(cell);
> + const char *value = defGetString(def);
> +
> + if (strcmp(def->defname, "force_not_null") == 0&&
> + strcmp(value, "true") == 0)
> + {
> + columns = lappend(columns, makeString(NameStr(attr->attname)));
> + elog(DEBUG1, "%s: force_not_null", NameStr(attr->attname));
> + }
> +
> + }
>
> makeString() does not copy the supplied string itself, so it is not preferable to reference
> NameStr(attr->attname) across ReleaseSysCache().
> I'd like to suggest to supply a copied attname using pstrdup for makeString
Oops, fixed.
[ I should check some of my projects for this issue... ]
Attached patch also includes some cosmetic changes such as removing
useless blank lines.
Regards,
--
Shigeru Hanada
Attachment | Content-Type | Size |
---|---|---|
force_not_null_v5.patch | text/plain | 12.3 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Pavel Golub | 2011-09-09 05:25:34 | Re: pg_dump.c |
Previous Message | Fujii Masao | 2011-09-09 00:42:03 | Re: pg_last_xact_insert_timestamp |