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

Hi,

In <CAEG8a3+KN=uofw5ksnCwh5s3m_VcfFYd=jTzcpO5uVLBHwSQEg(at)mail(dot)gmail(dot)com>
"Re: Make COPY format extendable: Extract COPY TO format implementations" on Sun, 28 Jul 2024 22:49:47 +0800,
Junwang Zhao <zhjwpku(at)gmail(dot)com> wrote:

> Thanks for updating the patches, I applied them and test
> in my local machine, I did not use tmpfs in my test, I guess
> if I run the tests enough rounds, the OS will cache the
> pages, below is my numbers(I run each test 30 times, I
> count for the last 10 ones):
>
> HEAD PATCHED
>
> COPY to_tab_30 TO '/dev/null' WITH (FORMAT text);
>
> 5628.280 ms 5679.860 ms
> 5583.144 ms 5588.078 ms
> 5604.444 ms 5628.029 ms
> 5617.133 ms 5613.926 ms
> 5575.570 ms 5601.045 ms
> 5634.828 ms 5616.409 ms
> 5693.489 ms 5637.434 ms
> 5585.857 ms 5609.531 ms
> 5613.948 ms 5643.629 ms
> 5610.394 ms 5580.206 ms
>
> COPY from_tab_30 FROM '/tmp/to_tab_30.txt' WITH (FORMAT text);
>
> 3929.955 ms 4050.895 ms
> 3909.061 ms 3890.156 ms
> 3940.272 ms 3927.614 ms
> 3907.535 ms 3925.560 ms
> 3952.719 ms 3942.141 ms
> 3933.751 ms 3904.250 ms
> 3958.274 ms 4025.581 ms
> 3937.065 ms 3894.149 ms
> 3949.896 ms 3933.878 ms
> 3925.399 ms 3936.170 ms
>
> I did not see obvious performance degradation, maybe it's
> because I did not use tmpfs, but I think this OTH means
> that the *function call* and *if branch* added for each row
> is not the bottleneck of the whole execution path.

Thanks for sharing your numbers. I agree with there are no
obvious performance degradation.

> In 0001,
>
> +typedef struct CopyFromRoutine
> +{
> + /*
> + * Called when COPY FROM is started to set up the input functions
> + * associated to the relation's attributes writing to. `finfo` can be
> + * optionally filled to provide the catalog information of the input
> + * function. `typioparam` can be optionally filled to define the OID of
> + * the type to pass to the input function. `atttypid` is the OID of data
> + * type used by the relation's attribute.
>
> +typedef struct CopyToRoutine
> +{
> + /*
> + * Called when COPY TO is started to set up the output functions
> + * associated to the relation's attributes reading from. `finfo` can be
> + * optionally filled. `atttypid` is the OID of data type used by the
> + * relation's attribute.
>
> The second comment has a simplified description for `finfo`, I think it
> should match the first by:
>
> `finfo` can be optionally filled to provide the catalog information of the
> output function.

Good catch. I'll update it as suggested in the next patch set.

> After I post the patch diffs, the gmail grammer shows some hints that
> it should be *associated with* rather than *associated to*, but I'm
> not sure about this one.

Thanks. I'll use "associated with".

> I think the patches are in good shape, I can help to do some
> further tests if needed, thanks for working on this.

Thanks!

--
kou