Horton

Re: Make COPY format extendable: Extract COPY TO format implementations

Sutou Kouhei <kou@clear-code.com>

View thread View on postgr.es
From: Sutou Kouhei <kou@clear-code.com>
To: sawada.mshk@gmail.com
Cc: michael@paquier.xyz, zhjwpku@gmail.com, andrew@dunslane.net, nathandbossart@gmail.com, pgsql-hackers@postgresql.org
Date: 2024-01-11T01:24:45Z
Lists: pgsql-hackers 
Hi,

In <CAD21AoC4HVuxOrsX1fLwj=5hdEmjvZoQw6PJGzxqxHNnYSQUVQ@mail.gmail.com>
  "Re: Make COPY format extendable: Extract COPY TO format implementations" on Wed, 10 Jan 2024 16:53:48 +0900,
  Masahiko Sawada <sawada.mshk@gmail.com> wrote:

>> Interesting. But I feel that it introduces another (a bit)
>> tricky mechanism...
> 
> Right. On the other hand, I don't think the idea 3 is good for the
> same reason Michael-san pointed out before[1][2].
>
> [1] https://www.postgresql.org/message-id/ZXEUIy6wl4jHy6Nm%40paquier.xyz
> [2] https://www.postgresql.org/message-id/ZXKm9tmnSPIVrqZz%40paquier.xyz

I think that the important part of the Michael-san's opinion
is "keep COPY TO implementation and COPY FROM implementation
separated for maintainability".

The patch focused in [1][2] uses one routine for both of
COPY TO and COPY FROM. If we use the approach, we need to
change one common routine from copyto.c and copyfrom.c (or
export callbacks from copyto.c and copyfrom.c and use them
in copyto.c to construct one common routine). It's
the problem.

The idea 3 still has separated routines for COPY TO and COPY
FROM. So I think that it still keeps COPY TO implementation
and COPY FROM implementation separated.

>> BTW, we also need to set .type:
>>
>>      .routine = COPYTO_ROUTINE (
>>          .type = T_CopyToFormatRoutine,
>>          .start_fn = testfmt_copyto_start,
>>          .onerow_fn = testfmt_copyto_onerow,
>>          .end_fn = testfmt_copyto_end
>>          )
> 
> I think it's fine as the same is true for table AM.

Ah, sorry. I should have said explicitly. I don't this that
it's not a problem. I just wanted to say that it's missing.


Defining one more static const struct instead of providing a
convenient (but a bit tricky) macro may be straightforward:

static const CopyToFormatRoutine testfmt_copyto_routine = {
    .type = T_CopyToFormatRoutine,
    .start_fn = testfmt_copyto_start,
    .onerow_fn = testfmt_copyto_onerow,
    .end_fn = testfmt_copyto_end
};

static const CopyFormatRoutine testfmt_copyto_handler = {
    .type = T_CopyFormatRoutine,
    .is_from = false,
    .routine = (Node *) &testfmt_copyto_routine
};


Thanks,
-- 
kou

Commits

  1. Refactor Copy{From|To}GetRoutine() to use pass-by-reference argument.

  2. Refactor COPY FROM to use format callback functions.

  3. Refactor COPY TO to use format callback functions.

  4. Another try to fix BF failure introduced in commit ddd5f4f54a.

  5. Revert "Refactor CopyReadAttributes{CSV,Text}() to use a callback in COPY FROM"

  6. Improve COPY TO performance when server and client encodings match

  7. Simplify signature of CopyAttributeOutCSV() in copyto.c

  8. Revert "Refactor CopyAttributeOut{CSV,Text}() to use a callback in COPY TO"

  9. Refactor CopyAttributeOut{CSV,Text}() to use a callback in COPY TO

  10. Refactor CopyReadAttributes{CSV,Text}() to use a callback in COPY FROM

  11. Add progress reporting of skipped tuples during COPY FROM.

  12. pgbench: Add \syncpipeline

  13. meson: Make gzip and tar optional

  14. Export the external file reader used in COPY FROM as APIs.