Peter Eisentraut <pe...@eisentraut.org> writes:
> The relationship between patch 0001 and 0002 is unclear to me. Are
> these incremental or alternatives? The description doesn't make this clear.
It appears to me that 0002 is actually counterproductive. I cannot
see a reason to get a StringInfo involved here: it adds overhead
and removes no complexity worth noticing. If it were hard to get
a close-enough upper bound for the output length, then yeah a
StringInfo could be a good solution. But the "strlen(inputText)"
proposed in 0001 seems plenty good enough, especially since as you
say this is a somewhat obsolescent format. The fact that it would
often overallocate somewhat doesn't bother me --- and a StringInfo
would in most cases overallocate by a lot more.
I'm inclined to accept 0001, reject 0002, and move on. This doesn't
seem like an area that's worth a huge amount of discussion.
> The main tests for the bytea type input formats are in
> src/test/regress/sql/strings.sql, so you should add any new tests there.
> Maybe there are already enough tests there that you don't need any new
> ones.
The code coverage report shows that byteain is covered except for the
path handling "\\". I'd be content to add one test query, or extend
some existing query, to make that branch get hit.
BTW, the patch needs rebasing because this code just got moved
to bytea.c.
regards, tom lane
