On Tue, Oct 3, 2017 at 3:55 AM, Andres Freund <and...@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