On Tue, Oct 3, 2017 at 3:55 AM, Andres Freund <andres@anarazel.de> wrote:
> Attached is a revised version of this patchset.
I don't much like the functions with "_pre" affixed to their names.
It's not at all clear that "pre" means "preallocated"; it sounds more
like you're doing something ahead of time. I wonder about maybe
calling these e.g. pq_writeint16, with "write" meaning "assume
preallocation" and "send" meaning "don't assume preallocation". There
could be other ideas, too.
> I'd like to get some
> input on two points:
>
> 1) Does anybody have a better idea than the static buffer in
> SendRowDescriptionMessage()? That's not particularly pretty, but
> there's not really a convenient stringbuffer to use when called from
> exec_describe_portal_message(). We could instead create a local
> buffer for exec_describe_portal_message().
>
> An alternative idea would be to have one reeusable buffer created for
> each transaction command, but I'm not sure that's really better.
I don't have a better idea.
> 2) There's a lot of remaining pq_sendint() callers in other parts of the
> tree. If others are ok with that, I'd do a separate pass over them.
> I'd say that even after doing that, we should keep pq_sendint(),
> because a lot of extension code is using that.
I think we should change everything to the new style and I wouldn't
object to removing pq_sendint() either. However, if we want to keep
it with a note that only extension code should use it, that's OK with
me, too.
> 3) The use of restrict, with a configure based fallback, is something
> we've not done before, but it's C99 and delivers significantly more
> efficient code. Any arguments against?
It's pretty unobvious why it helps here. I think you should add
comments. Also, unless I'm missing something, there's nothing to keep
pq_sendintXX_pre from causing an alignment fault except unbridled
optimism...
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers