Horton

Re: Parallel copy

vignesh C <vignesh21@gmail.com>

View thread
From: vignesh C <vignesh21@gmail.com>
To: Amit Kapila <amit.kapila16@gmail.com>
Cc: "Hou, Zhijie" <houzj.fnst@cn.fujitsu.com>, Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>, Rafia Sabih <rafia.pghackers@gmail.com>, Ashutosh Sharma <ashu.coek88@gmail.com>, Andres Freund <andres@anarazel.de>, Robert Haas <robertmhaas@gmail.com>, Ants Aasma <ants@cybertec.at>, Tomas Vondra <tomas.vondra@2ndquadrant.com>, Alastair Turner <minion@decodable.me>, Thomas Munro <thomas.munro@gmail.com>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Date: 2020-10-21T06:37:55Z
Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Allow WaitLatch() to be used without a latch.

  2. Add %P to log_line_prefix for parallel group leader

  3. Include replication origins in SQL functions for commit timestamp

  4. Avoid useless buffer allocations during binary COPY FROM.

Attachments

On Mon, Oct 19, 2020 at 2:40 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
>
> On Sun, Oct 18, 2020 at 7:47 AM Hou, Zhijie <houzj.fnst@cn.fujitsu.com> wrote:
> >
> > Hi Vignesh,
> >
> > After having a look over the patch,
> > I have some suggestions for
> > 0003-Allow-copy-from-command-to-process-data-from-file.patch.
> >
> > 1.
> >
> > +static uint32
> > +EstimateCstateSize(ParallelContext *pcxt, CopyState cstate, List *attnamelist,
> > +                                  char **whereClauseStr, char **rangeTableStr,
> > +                                  char **attnameListStr, char **notnullListStr,
> > +                                  char **nullListStr, char **convertListStr)
> > +{
> > +       uint32          strsize = MAXALIGN(sizeof(SerializedParallelCopyState));
> > +
> > +       strsize += EstimateStringSize(cstate->null_print);
> > +       strsize += EstimateStringSize(cstate->delim);
> > +       strsize += EstimateStringSize(cstate->quote);
> > +       strsize += EstimateStringSize(cstate->escape);
> >
> >
> > It use function EstimateStringSize to get the strlen of null_print, delim, quote and escape.
> > But the length of null_print seems has been stored in null_print_len.
> > And delim/quote/escape must be 1 byte, so I think call strlen again seems unnecessary.
> >
> > How about  " strsize += sizeof(uint32) + cstate->null_print_len + 1"
> >
>
> +1. This seems like a good suggestion but add comments for
> delim/quote/escape to indicate that we are considering one-byte for
> each. I think this will obviate the need of function
> EstimateStringSize. Another thing in this regard is that we normally
> use add_size function to compute the size but I don't see that being
> used in this and nearby computation. That helps us to detect overflow
> of addition if any.
>
> EstimateCstateSize()
> {
> ..
> +
> + strsize++;
> ..
> }
>
> Why do we need this additional one-byte increment? Does it make sense
> to add a small comment for the same?
>

Changed it to handle null_print, delim, quote & escape accordingly in
the attached patch, the one byte increment is not required, I have
removed it.

Regards,
Vignesh
EnterpriseDB: http://www.enterprisedb.com