On Thu, Jun 23, 2022 at 01:26:29PM +0900, Michael Paquier wrote:
> On Wed, Jun 22, 2022 at 08:00:15PM +0900, Michael Paquier wrote:
> > OK. As this is originally a feature you have committed, I originally
> > thought that you would take care of it, even if I sent a patch. I'll
> > handle that
On Wed, Jun 22, 2022 at 08:00:15PM +0900, Michael Paquier wrote:
> OK. As this is originally a feature you have committed, I originally
> thought that you would take care of it, even if I sent a patch. I'll
> handle that tomorrow then, if that's fine for you, of course. Happy
> to help.
And
On Wed, Jun 22, 2022 at 12:22:01PM +0200, Peter Eisentraut wrote:
> The latest patch was posted by you, so I was deferring to you to commit it.
> Would you like me to do it?
OK. As this is originally a feature you have committed, I originally
thought that you would take care of it, even if I
On 22.06.22 01:34, Michael Paquier wrote:
On Mon, Jun 20, 2022 at 09:03:23AM +0900, Michael Paquier wrote:
On Thu, Jun 16, 2022 at 09:24:56AM +0200, Peter Eisentraut wrote:
I don't feel very strongly about this. It makes sense to stay consistent
with the existing COPY code.
Yes, my
On Mon, Jun 20, 2022 at 09:03:23AM +0900, Michael Paquier wrote:
> On Thu, Jun 16, 2022 at 09:24:56AM +0200, Peter Eisentraut wrote:
>> I don't feel very strongly about this. It makes sense to stay consistent
>> with the existing COPY code.
>
> Yes, my previous argument is based on consistency
On Thu, Jun 16, 2022 at 09:24:56AM +0200, Peter Eisentraut wrote:
> I don't feel very strongly about this. It makes sense to stay consistent
> with the existing COPY code.
Yes, my previous argument is based on consistency with the
surroundings. I am not saying that this could not be made
On 15.06.22 13:50, Daniel Verite wrote:
The other errors in the list above are more about the format itself,
with options that make sense only for one format. So the way we're
supposed to understand ERRCODE_FEATURE_NOT_SUPPORTED in these
other cases is that such format does not support such
Julien Rouhaud wrote:
> Maybe that's just me but I understand "not supported" as "this makes
> sense, but this is currently a limitation that might be lifted
> later".
Looking at ProcessCopyOptions(), there are quite a few invalid
combinations of options that produce
On 14.06.22 11:13, Julien Rouhaud wrote:
There is no need for the extra
comment, and the error code should be ERRCODE_FEATURE_NOT_SUPPORTED.
Is there any rule for what error code should be used?
Maybe that's just me but I understand "not supported" as "this makes sense, but
this is currently
On Mon, Jun 13, 2022 at 04:46:46PM +0900, Michael Paquier wrote:
>
> Some nits. I would suggest to reword this error message, like "cannot
> use \"match\" with HEADER in COPY TO".
Agreed.
> There is no need for the extra
> comment, and the error code should be ERRCODE_FEATURE_NOT_SUPPORTED.
On Mon, Jun 13, 2022 at 10:32:13AM +0800, Julien Rouhaud wrote:
> On Sun, Jun 12, 2022 at 09:36:13AM -0400, Andrew Dunstan wrote:
> I'm fine with it. I added such a check and mentioned it in the documentation.
An error looks like the right call at this stage of the game. I am
not sure what the
On 13.06.22 04:32, Julien Rouhaud wrote:
I think it makes more sense to have a sanity check to prevent HEADER
MATCH with COPY TO.
I'm fine with it. I added such a check and mentioned it in the documentation.
I think it would still be problematic if the target table has dropped columns.
Hi,
On Sun, Jun 12, 2022 at 09:36:13AM -0400, Andrew Dunstan wrote:
>
> On 2022-06-07 Tu 11:47, Julien Rouhaud wrote:
> >
> > First, probably nitpicking, the HEADER MATCH is allowed for COPY TO, is that
> > expected? The documentation isn't really explicit about it, but there's
> > nothing to
On 2022-06-07 Tu 11:47, Julien Rouhaud wrote:
> Hi,
>
> On Wed, Mar 30, 2022 at 09:11:09AM +0200, Peter Eisentraut wrote:
>> Committed, after some further refinements as discussed.
> While working on nearby code, I found some problems with this feature.
>
> First, probably nitpicking, the HEADER
Hi,
On Wed, Mar 30, 2022 at 09:11:09AM +0200, Peter Eisentraut wrote:
>
> Committed, after some further refinements as discussed.
While working on nearby code, I found some problems with this feature.
First, probably nitpicking, the HEADER MATCH is allowed for COPY TO, is that
expected? The
On 29.03.22 17:02, Peter Eisentraut wrote:
On 25.03.22 05:21, Greg Stark wrote:
Great to see the first of the two patches committed.
It looks like the second patch got some feedback from Peter in
February and has been marked "Waiting on author" ever since.
Remi, will you have a chance to look
On 25.03.22 05:21, Greg Stark wrote:
Great to see the first of the two patches committed.
It looks like the second patch got some feedback from Peter in
February and has been marked "Waiting on author" ever since.
Remi, will you have a chance to look at this this month?
Peter, are these
Peter Eisentraut wrote:
> - The DefGetCopyHeader() function seems very bulky and might not be
> necessary. I think you can just check for the string "match" first and
> then use defGetBoolean() as before if it didn't match.
The problem is that defGetBoolean() ends like this in the
Great to see the first of the two patches committed.
It looks like the second patch got some feedback from Peter in
February and has been marked "Waiting on author" ever since.
Remi, will you have a chance to look at this this month?
Peter, are these comments blocking if Remi doesn't have a
On 31.01.22 07:54, Peter Eisentraut wrote:
On 30.01.22 23:56, Rémi Lapeyre wrote:
I notice in the 0002 patch that there is no test case for the error
"wrong header for column \"%s\": got \"%s\"", which I think is really
the core functionality of this patch. So please add that.
I added a
On 30.01.22 23:56, Rémi Lapeyre wrote:
I notice in the 0002 patch that there is no test case for the error "wrong header for column
\"%s\": got \"%s\"", which I think is really the core functionality of this patch.
So please add that.
I added a test for it in this new version of the patch.
> On 28 Jan 2022, at 09:57, Peter Eisentraut
> wrote:
>
> On 31.12.21 18:36, Rémi Lapeyre wrote:
>> Here’s an updated version of the patch that takes into account the changes
>> in d1029bb5a2. The actual code is the same as v10 which was already marked
>> as ready for committer.
>
> I have
On 31.12.21 18:36, Rémi Lapeyre wrote:
Here’s an updated version of the patch that takes into account the changes in
d1029bb5a2. The actual code is the same as v10 which was already marked as
ready for committer.
I have committed the 0001 patch. I will work on the 0002 patch next.
I notice
Here’s an updated version of the patch that takes into account the changes in
d1029bb5a2. The actual code is the same as v10 which was already marked as
ready for committer.
v11-0001-Add-header-support-to-COPY-TO-text-format.patch
Description: Binary data
>
> >> This now reads "Represents whether the header must be absent, present or
> >> match.”.
>
> Since match shouldn't be preceded with be, I think we can say:
>
> Represents whether the header must match, be absent or be present.
Thanks, here’s a v10 version of the patch that fixes this.
On Sun, Apr 11, 2021 at 4:01 AM Rémi Lapeyre
wrote:
> >
> > Hi,
> > >> sure it matches what is expected and exit immediatly if it does not.
> >
> > Typo: immediately
> >
> > +CREATE FOREIGN TABLE header_dont_match (a int, foo text) SERVER
> file_server
> >
> > nit: since header is singular, you
>
> Hi,
> >> sure it matches what is expected and exit immediatly if it does not.
>
> Typo: immediately
>
> +CREATE FOREIGN TABLE header_dont_match (a int, foo text) SERVER file_server
>
> nit: since header is singular, you can name the table header_doesnt_match
>
> + from the one
On Sat, Apr 10, 2021 at 4:17 PM Rémi Lapeyre
wrote:
>
> >
> > Michael, since the issue of duplicated options has been fixed do either
> of these patches look like they are ready for commit?
> >
>
> Here’s a rebased version of the patch.
>
>
> Cheers,
> Rémi
>
>
> > Regards,
> > --
> > -David
> >
>
> Michael, since the issue of duplicated options has been fixed do either of
> these patches look like they are ready for commit?
>
Here’s a rebased version of the patch.
Cheers,
Rémi
> Regards,
> --
> -David
> da...@pgmasters.net
On 12/7/20 6:40 PM, Rémi Lapeyre wrote:
Hi, here’s a rebased version of the patch.
Michael, since the issue of duplicated options has been fixed do either
of these patches look like they are ready for commit?
Regards,
--
-David
da...@pgmasters.net
Hi, here’s a rebased version of the patch.
Best regards,
Rémi
v7-0001-Add-header-support-to-COPY-TO-text-format.patch
Description: Binary data
v7-0002-Add-header-matching-mode-to-COPY-FROM.patch
Description: Binary data
> On 21 Oct 2020, at 19:49, Daniel Verite wrote:
>
> Rémi
Rémi Lapeyre wrote:
> It looks like this is not in the current commitfest and that Cabot does not
> find it. I’m not yet accustomed to the PostgreSQL workflow, should I just
> create a new entry in the current commitfest?
Yes. Because in the last CommitFest it was marked
as "Returned
It looks like this is not in the current commitfest and that Cabot does not
find it. I’m not yet accustomed to the PostgreSQL workflow, should I just
create a new entry in the current commitfest?
Regards,
Rémi
> Le 13 oct. 2020 à 14:49, Rémi Lapeyre a écrit :
>
> Thanks Michael for taking
Thanks Michael for taking care of that!
Here’s the rebased patches with the last one dropped.
Regards,
Rémi
v6-0001-Add-header-support-to-COPY-TO-text-format.patch
Description: Binary data
v6-0002-Add-header-matching-mode-to-COPY-FROM.patch
Description: Binary data
> Le 5 oct. 2020 à
On Sat, Oct 03, 2020 at 11:42:52PM +0200, Rémi Lapeyre wrote:
> Here’s a new version of the patches that report an error when the options are
> set multiple time.
Please note that I have applied a fix for the redundant option
handling as of 10c5291, though I have missed that you sent a patch.
> I would agree that this is a bug because we are failing to detect
> what's actually a redundant option here as the first option still
> causes the flag to be set to false, but that's not something worth a
> back-patch IMO. What we are looking here is something similar
> to what is done with
On Thu, Aug 27, 2020 at 04:53:11PM +0200, Rémi Lapeyre wrote:
> I have two remarks with the state of the current patches:
> - DefGetCopyHeader() duplicates a lot of code from defGetBoolean(),
> should we refactor this so that they can share more of their
> internals? In the current implementation
Thanks Daniel for the review and Vignesh for addressing the comments.
I have two remarks with the state of the current patches:
- DefGetCopyHeader() duplicates a lot of code from defGetBoolean(), should we
refactor this so that they can share more of their internals? In the current
Thanks for your comments, Please find my thoughts inline.
> In my tests it works fine except for one crash that I can reproduce
> on a fresh build and default configuration with:
>
> $ cat >file.txt
> i
> 1
>
> $ psql
> postgres=# create table x(i int);
> CREATE TABLE
> postgres=# \copy x(i) from
Rémi Lapeyre wrote:
> Here’s a new version that fix all the issues.
Here's a review of v4.
The patch improves COPY in two ways:
- COPY TO and COPY FROM now accept "HEADER ON" for the TEXT format
(previously it was only for CSV)
- COPY FROM also accepts "HEADER match" to tell that
Thanks for the feedback,
> There is one warning present in the changes:
> copy.c: In function ‘ProcessCopyOptions’:
> copy.c:1208:5: warning: ISO C90 forbids mixed declarations and code
> [-Wdeclaration-after-statement]
> char*sval = defGetString(defel);
>
Weirdly this is not caught by
On Fri, Jul 17, 2020 at 10:18 PM Rémi Lapeyre wrote:
>
> >
> > I don't know how to do that with git-send-email, but you can certainly do
> > it easy with git-format-patch and just attach them using your regular MUA.
> >
> > (and while the cfbot and the archives have no problems dealing with the
>
> I don't know how to do that with git-send-email, but you can certainly do it
> easy with git-format-patch and just attach them using your regular MUA.
>
> (and while the cfbot and the archives have no problems dealing with the
> change in subject, it does break threading in some other
On Fri, Jul 17, 2020 at 5:11 PM Rémi Lapeyre
wrote:
>
>
> > It's hard to find an explanation what this patch actually does. I don't
> > want to have to go through threads dating back 4 months to determine
> > what was discussed and what was actually implemented. Since you're
> > already using
> It's hard to find an explanation what this patch actually does. I don't
> want to have to go through threads dating back 4 months to determine
> what was discussed and what was actually implemented. Since you're
> already using git format-patch, just add something to the commit message.
>
>
45 matches
Mail list logo
