pg.ddx.io  pgsql-hackers@postgresql.org mailing list archive  
help / color / mirror / Atom feed
From: Tomas Vondra <tomas.vondra@2ndquadrant.com>
To: Craig Ringer <craig@2ndquadrant.com>
To: Daniel Verite <daniel@manitou-mail.org>
Cc: Alvaro Herrera <alvherre@2ndquadrant.com>
Cc: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Robert Haas <robertmhaas@gmail.com>
Cc: Jim Nasby <Jim.Nasby@bluetreble.com>
Cc: Ronan Dunklau <ronan.dunklau@dalibo.com>
Cc: pgsql-hackers <pgsql-hackers@postgresql.org>
Subject: Re: pg_dump / copy bugs with "big lines" ?
Date: Sat, 3 Sep 2016 03:18:04 +0200
Message-ID: <5aafa6a9-e49c-ab13-be67-c68f34986801@2ndquadrant.com> (raw)
In-Reply-To: <ba62f67a-b585-03f2-cfec-fefb95549efd@2ndquadrant.com>
References: <20160318154830.GA94538@alvherre.pgsql>
	<d3fe524a-1c78-4cb3-9814-849cd4f43fe6@mm>
	<CAMsr+YHpGUVURXGFYV8Fje7KsniDPzJVUV=MU0Hxz3rWbQkKMg@mail.gmail.com>
	<ba62f67a-b585-03f2-cfec-fefb95549efd@2ndquadrant.com>
List-Unsubscribe: <mailto:majordomo@postgresql.org?body=unsub%20pgsql-hackers>

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



view thread (48+ messages)  latest in thread

Message-ID: <5aafa6a9-e49c-ab13-be67-c68f34986801@2ndquadrant.com>
Permalink:  ../5aafa6a9-e49c-ab13-be67-c68f34986801@2ndquadrant.com/
Also on:    postgresql.org/message-id/5aafa6a9-e49c-ab13-be67-c68f34986801@2ndquadrant.com

 · 

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: pgsql-hackers@postgresql.org
  Cc: tomas.vondra@2ndquadrant.com, craig@2ndquadrant.com, daniel@manitou-mail.org, alvherre@2ndquadrant.com, tgl@sss.pgh.pa.us, robertmhaas@gmail.com, Jim.Nasby@bluetreble.com, ronan.dunklau@dalibo.com
  Subject: Re: pg_dump / copy bugs with "big lines" ?
  In-Reply-To: <5aafa6a9-e49c-ab13-be67-c68f34986801@2ndquadrant.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox