On 09/03/2016 02:21 AM, Tomas Vondra wrote:
A few minor comments regarding the patch: 1) CopyStartSend seems pretty pointless - It only has one function call in it, and is called on exactly one place (and all other places simply call allowLongStringInfo directly). I'd get rid of this function and replace the call in CopyOneRowTo(() with allowLongStringInfo(). 2) allowlong seems awkward, allowLong or allow_long would be better 3) Why does resetStringInfo reset the allowLong flag? Are there any cases when we want/need to forget the flag value? I don't think so, so let's simply not reset it and get rid of the allowLongStringInfo() calls. Maybe it'd be better to invent a new makeLongStringInfo() method instead, which would make it clear that the flag value is permanent. 4) HandleParallelMessage needs a tweak, as it uses msg->len in a log message, but with '%d' and not '%ld' (as needed after changing the type to Size).
5) The comment at allowLongStringInfo talks about allocLongStringInfo (i.e. wrong function name).
regards -- Tomas Vondra http://www.2ndQuadrant.com PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
