printTable API (was: Show INHERIT in \du)

From: "Brendan Jurd" <direvus(at)gmail(dot)com>
To: pgsql-hackers(at)postgresql(dot)org
Cc: "Tom Lane" <tgl(at)sss(dot)pgh(dot)pa(dot)us>, "Alvaro Herrera" <alvherre(at)commandprompt(dot)com>
Subject: printTable API (was: Show INHERIT in \du)
Date: 2008-03-29 23:39:35
Message-ID: 37ed240d0803291639g3bf235a2h60683765cb27ffec@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers pgsql-patches

On 25/03/2008, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> "Brendan Jurd" <direvus(at)gmail(dot)com> writes:
> > This makes me wonder whether print.c could offer something a bit more
> > helpful to callers wishing to DIY a table; we could have a
> > table-building struct with methods like addHeader and addCell.
>
> Once you have two occurrences of a pattern, it's reasonable to assume
> there will be more later. +1 for building a little bit of infrastructure.
>

Hi hackers,

Per the above discussion, I'm looking at creating a nice API in
print.c for callers wishing to build their own psql output table.

Currently, if you want to build your own table you need to implement
your own local version of the logic in printQuery.
describeOneTableDetails is the only place this happens right now, but
due to some localisation issues it looks like my \du patch will have
to build its table manually as well.

There seem to be some differences in the way printQuery and
describeOneTableDetails go about constructing the array of cells.
Here's the version from describe:

<code>
/* Generate table cells to be printed */
/* note: initialize all cells[] to NULL in case of error exit */
cells = pg_malloc_zero((numrows * cols + 1) * sizeof(*cells));

for (i = 0; i < numrows; i++)
{
/* Name */
#ifdef WIN32
cells[i * cols + 0] = mbvalidate(PQgetvalue(res, i, 0), myopt.encoding);
#else
cells[i * cols + 0] = PQgetvalue(res, i, 0); /* don't free this
* afterwards */
#endif

/* Type */
#ifdef WIN32
cells[i * cols + 1] = mbvalidate(PQgetvalue(res, i, 1), myopt.encoding);
#else
cells[i * cols + 1] = PQgetvalue(res, i, 1); /* don't free this
* either */
#endif
</code>

And the version from printQuery:

<code>
/* set cells */
ncells = ntuples * nfields;
cells = pg_local_calloc(ncells + 1, sizeof(*cells));

i = 0;
for (r = 0; r < ntuples; r++)
{
for (c = 0; c < nfields; c++)
{
if (PQgetisnull(result, r, c))
cells[i] = opt->nullPrint ? opt->nullPrint : "";
else
{
cells[i] = (char *)
mbvalidate((unsigned char *) PQgetvalue(result, r, c),
opt->topt.encoding);
#ifdef ENABLE_NLS
if (opt->trans_columns && opt->trans_columns[c])
cells[i] = _(cells[i]);
#endif
}
i++;
}
}
</code>

So, it looks like:

1. describe malloc's the cells to zero, but print just does a local
calloc without any initialisation.

2. describe only does an mbvalidate for WIN32, but print does it in all cases.

Any opinions on which behaviour is preferable/more correct in these two cases?

Regards,
BJ

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Alex Hunsaker 2008-03-30 01:40:19 Re: [badalex@gmail.com: Re: [BUGS] Problem identifying constraints which should not be inherited]
Previous Message Tom Lane 2008-03-29 21:33:29 buildfarm member jaguar doesn't like truncate-triggers code

Browse pgsql-patches by date

  From Date Subject
Next Message Alex Hunsaker 2008-03-30 01:40:19 Re: [badalex@gmail.com: Re: [BUGS] Problem identifying constraints which should not be inherited]
Previous Message Magnus Hagander 2008-03-29 21:45:11 Re: create language ... if not exists