Re: COPY FROM performance improvements

Alon Goldshuv said:
> Andrew,
>

>> 4. We should indeed do this for CSV, especially since a lot of the
>> relevant logic for detecting attribute starts is already there for CSV
>> in CopyReadLine. I'm prepared to help you do that if necessary, since
>> I'm guilty of perpetrating that code.
>
> That will be great. My idea is to eventually keep processing for BINARY
> in one routine while text can be in another (CSV+delimited). The reason
> being is that there is a lot in common for the processing of the 2 text
> types. The CopyReadLineBuffered will have to be changed just a little
> to accommodate embedded newlines inside CSV quotes, and the
> CopyReadAttributeCSV would need to change as well.
>

That sounds right.

> Question for you -- you'll probably notice that I made the escape of
> the delimited text format (previously '\\') a constant variable
> "escapec". Reason being - maybe some people would like to choose an
> escape for delimited COPY too, or even disable it. So it's a good base
> to start with. However, I just noticed that CSV uses that exact
> variable name... Sorry about that. Any suggestion on how to rename it
> to something different?
>

I haven't seen any demand for users to be allowed to specify an escape char
in text mode (BTW, that's what the docs call what you have called delimited
mode). Maybe there's a case for it, but I somewhat doubt it.

As for variable names, choose some variant that seems reasonable. If we need
to distinguish we should use csv_escapec and text_escapec. But I'm not sure
we can't use the same variable name for both cases, unless they would have
conflicting scopes.

What I would like to have is a high level description of
. how the new text mode code differs from the old text mode code, and
. which part of the change is responsible for how much performance gain.

Maybe I have missed that in previous discussion, but this change is
sufficiently invasive that I think you owe that to the reviewers.

cheers

andrew

cheers

andrew