On 2019-Jun-14, Fabien COELHO wrote:
>
> Hello Alvaro,
>
> > > Shouldn't the --rows-per-insert option also be available via pg_dumpall?
> > > All the other options for switching between COPY and INSERT are
> > > settable in pg_dumpall.
> >
> > Uh, yeah, absolutely.
> >
> > Surafel, are you in
Hello Alvaro,
Shouldn't the --rows-per-insert option also be available via pg_dumpall?
All the other options for switching between COPY and INSERT are
settable in pg_dumpall.
Uh, yeah, absolutely.
Surafel, are you in a position to provide a patch for that quickly?
End of the week, more ti
On 2019-Jun-14, Peter Eisentraut wrote:
> Shouldn't the --rows-per-insert option also be available via pg_dumpall?
> All the other options for switching between COPY and INSERT are
> settable in pg_dumpall.
Uh, yeah, absolutely.
Surafel, are you in a position to provide a patch for that quickly
Shouldn't the --rows-per-insert option also be available via pg_dumpall?
All the other options for switching between COPY and INSERT are
settable in pg_dumpall.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On Fri, 8 Mar 2019 at 01:46, Alvaro Herrera wrote:
> Pushed, thanks!
> I made a couple of edits to v17 before pushing,
Thank you for making those changes and for pushing it.
--
David Rowley http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Service
Pushed, thanks!
If anyone is feeling inspired, one additional test we could use is
--rows-per-insert together with --on-conflict-do-nothing.
I made a couple of edits to v17 before pushing,
* rename strtol endptr variable so that it can be used by other strtol
calls, if we ever have them
* use p
Thanks for looking at this again.
On Sat, 2 Mar 2019 at 20:01, Fabien COELHO wrote:
> Although I'm all in favor of checking the int associated to the option, I
> do not think that it warrants three checks and messages. I would suggest
> to factor them all as just one check and one (terse) message
On 2019-Mar-02, Fabien COELHO wrote:
> Although I'm all in favor of checking the int associated to the option, I do
> not think that it warrants three checks and messages. I would suggest to
> factor them all as just one check and one (terse) message.
I suggest ("rows-per-insert must be in range
On Sat, Mar 02, 2019 at 08:01:50AM +0100, Fabien COELHO wrote:
> About the output: I'd suggest to indent one line per row, something like:
>
> INSERT INTO foo VALUES
> (..., ..., ..., ...),
> (..., ..., ..., ...),
> (..., ..., ..., ...);
>
> so as to avoid very very very very very v
Hello David & Surafel,
I think this can be marked as ready for committer now, but I'll defer
to Fabien to see if he's any other comments.
Patch v16 applies and compiles cleanly, local and global "make check"
are ok. Doc build is ok.
I did some manual testing with limit cases which did wor
On Fri, 22 Feb 2019 at 14:40, David Rowley wrote:
>
> On Tue, 19 Feb 2019 at 02:34, Surafel Temesgen wrote:
> > I see that there are already a test for zero column table in
> > test_fourth_table_zero_col
> > and if am not wrong table_index_stats is empty table
>
> Maybe Fabien would like to see
On Tue, 19 Feb 2019 at 02:34, Surafel Temesgen wrote:
> I see that there are already a test for zero column table in
> test_fourth_table_zero_col
> and if am not wrong table_index_stats is empty table
Maybe Fabien would like to see a test that dumps that table with
--rows-per-insert= to ensure t
On Sat, Feb 2, 2019 at 11:26 AM Fabien COELHO wrote:
> There is a test, that is good! Charater "." should be backslashed in the
> regexpr. I'd consider also introducing limit cases: empty table, empty
> columns by creating corresponding tables and using -t repeatedly
I see that there are alrea
On Wed, 13 Feb 2019 at 19:36, Surafel Temesgen wrote:
> On Mon, Feb 11, 2019 at 10:20 AM David Rowley
> wrote:
>>
>> Reviewing pg_dump-rows-per-insert-option-v14.
>>
>> Also, maybe one for Fabien (because he seems keen on keeping the
>> --rows-per-insert validation code)
>>
>> strtol() returns a
On Mon, Feb 11, 2019 at 10:20 AM David Rowley
wrote:
> Reviewing pg_dump-rows-per-insert-option-v14.
>
Also, maybe one for Fabien (because he seems keen on keeping the
> --rows-per-insert validation code)
>
> strtol() returns a long. dump_inserts is an int, so on machines where
> sizeof(long) ==
Reviewing pg_dump-rows-per-insert-option-v14.
Mostly going back over things that Fabien mentioned:
On Sat, 2 Feb 2019 at 21:26, Fabien COELHO wrote:
> There is a test, that is good! Charater "." should be backslashed in the
> regexpr. I'd consider also introducing limit cases: empty table, empty
On Sat, Feb 2, 2019 at 11:26 AM Fabien COELHO wrote:
>
> Hello David,
>
> > Wondering if you have anything else here? I'm happy for the v13
> > version to be marked as ready for committer.
>
> I still have a few comments.
>
> Patch applies cleanly, compiles, global & local make check are ok.
>
>
On Sun, Feb 03, 2019 at 01:21:45PM +0300, Surafel Temesgen wrote:
> at least for processing user argument i think it is better to use strtol or
> other
> function that have better error handling. i can make a patch that change
> usage
> of atoi for user argument processing after getting feedback fr
On Sun, Feb 3, 2019 at 11:00 AM Fabien COELHO wrote:
>
> Hello David,
>
> >> I do not understand why dump_inserts declaration has left the "flags for
> >> options" section.
> >
> > I moved that because it's no longer just a flag. It now stores an int
> value.
>
> Hmmm. Indeed, all th "int"s of th
On Sun, 3 Feb 2019 at 21:00, Fabien COELHO wrote:
> >> There is a test, that is good! Charater "." should be backslashed in the
> >> regexpr.
> >
> > Yeah, you're right. I wonder if we should fix the test of them in
> > another patch.
>
> From a software engineering perspective, I'd say that a f
Hello David,
I do not understand why dump_inserts declaration has left the "flags for
options" section.
I moved that because it's no longer just a flag. It now stores an int value.
Hmmm. Indeed, all th "int"s of this section should be "bool" instead. Now,
some "flags" do not appear althou
On Sat, 2 Feb 2019 at 21:26, Fabien COELHO wrote:
> I do not understand why dump_inserts declaration has left the "flags for
> options" section.
I moved that because it's no longer just a flag. It now stores an int value.
> I'd suggest not to rely on "atoi" because it does not check the argument
Hello David,
Wondering if you have anything else here? I'm happy for the v13
version to be marked as ready for committer.
I still have a few comments.
Patch applies cleanly, compiles, global & local make check are ok.
Typos and style in the doc:
"However, since, by default this op
On Thu, 31 Jan 2019 at 11:49, David Rowley wrote:
> Here's another version, same as before but with tests this time.
Hi Fabien,
Wondering if you have anything else here? I'm happy for the v13
version to be marked as ready for committer.
--
David Rowley http://www.2ndQuadrant
On Thu, 24 Jan 2019 at 04:45, Fabien COELHO wrote:
> >> The feature is not tested anywhere. I still think that there should be a
> >> test on empty/small/larger-than-rows-per-insert tables, possibly added to
> >> existing TAP-tests.
> >
> > I was hoping to get away with not having to do that... ma
On 2019-Jan-23, David Rowley wrote:
> On Wed, 23 Jan 2019 at 04:08, Alvaro Herrera wrote:
> > Is it possible to avoid the special case for 0 columns by using the
> > UNION ALL syntax I showed?
>
> It would be possible, but my thoughts are that we're moving away from
> the SQL standard by doing s
On Thu, 24 Jan 2019 at 04:45, Fabien COELHO wrote:
> I still do not understand the need for another variable.
>
>int ninserts = 0; // default is to use copy
>while (getopt...)
>{
> switch (...) {
>case "--inserts":
> if (ninserts == 0) ninserts = 1;
> br
Hello David,
I thought about this and looked into it, but I decided it didn't look
like a smart thing to do. The reason is that if --inserts sets
dump_inserts to 1 then --rows-per-insert sets it to something else
that's likely fine, but if that happens in the opposite order then the
--rows-pe
On Wed, 23 Jan 2019 at 22:08, Fabien COELHO wrote:
> Out of abc-order rows-per-inserts option in getopt struct.
I missed that. Thanks. Fixed in attached.
> help string does not specify the expected argument.
Also fixed.
> I still think that the added rows_per_insert field is useless, ISTM That
Hello David & Surafel,
About this v10:
Patch applies and compiles cleanly, local & global "make check" ok.
A few comments, possibly redundant with some already in the thread.
Out of abc-order rows-per-inserts option in getopt struct.
help string does not specify the expected argument.
I st
On Wed, 23 Jan 2019 at 02:13, Surafel Temesgen wrote:
> okay .thank you for explaining. i attach a patch corrected as such
I did a bit of work to this to fix a bunch of things:
1. Docs for --rows-per-insert didn't mention anything about a parameter.
2. You'd not followed the alphabetical order o
On Wed, 23 Jan 2019 at 04:08, Alvaro Herrera wrote:
> Is it possible to avoid the special case for 0 columns by using the
> UNION ALL syntax I showed?
It would be possible, but my thoughts are that we're moving away from
the SQL standard by doing so.
Looking at the standard I see:
::=
[ { }
Hello Surafel,
okay .thank you for explaining. i attach a patch corrected as such
About this v9: applies cleanly, compiles, global and local "make check"
ok.
The option is not exercise in the TAP tests. I'd suggest that it should be
tested on a small table with zero, 1, more than the val
Nice stuff.
Is it possible to avoid the special case for 0 columns by using the
UNION ALL syntax I showed?
--
Álvaro Herrerahttps://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On Tue, Jan 22, 2019 at 3:35 PM David Rowley
wrote:
> On Sat, 19 Jan 2019 at 01:01, Surafel Temesgen
> wrote:
> > if you specified --inserts option you already specified the number of
> rows per statement which is 1 .
> > if more than one rows per statement needed it must be specified using
> --
On Sat, 19 Jan 2019 at 01:01, Surafel Temesgen wrote:
> if you specified --inserts option you already specified the number of rows
> per statement which is 1 .
> if more than one rows per statement needed it must be specified using
> --rows-per-insert
> and specifying one row per statement using
On Tue, Dec 25, 2018 at 4:47 AM Fabien COELHO wrote:
> ISTM that command-line switches with optional arguments should be avoided:
> This feature is seldom used (hmmm... 2 existing instances), because it
> interferes with argument processing if such switches are used as the last
> one.
Excellent p
On Fri, Jan 18, 2019 at 5:02 AM Surafel Temesgen wrote:
> On Fri, Jan 18, 2019 at 2:29 PM David Rowley
> wrote:
>>
>> On Fri, 18 Jan 2019 at 19:29, Surafel Temesgen wrote:
>> > this happen because i don't disallow the usage of --inserts and
>> > --rows-per-insert
>> > option together.it shoul
On Fri, Jan 18, 2019 at 2:29 PM David Rowley
wrote:
> On Fri, 18 Jan 2019 at 19:29, Surafel Temesgen
> wrote:
> > this happen because i don't disallow the usage of --inserts and
> --rows-per-insert
> > option together.it should be error out in those case.i correct it in
> attached patch
>
> I d
On Fri, 18 Jan 2019 at 19:29, Surafel Temesgen wrote:
> this happen because i don't disallow the usage of --inserts and
> --rows-per-insert
> option together.it should be error out in those case.i correct it in attached
> patch
I don't think it should be an error. It's not like the two options
On Fri, Jan 18, 2019 at 7:14 AM David Rowley
wrote:
> On Fri, 18 Jan 2019 at 01:15, Surafel Temesgen
> wrote:
> > The attache patch use your method mostly
>
> I disagree with the "mostly" part. As far as I can see, you took the
> idea and then made a series of changes to completely break it. F
On Thu, Jan 17, 2019 at 5:15 AM Surafel Temesgen wrote:
> On Fri, Jan 4, 2019 at 3:08 PM David Rowley
> wrote:
>>
>> On Mon, 31 Dec 2018 at 18:58, Surafel Temesgen wrote:
>>
>>
>> 2. Is --insert-multi a good name? What if they do --insert-multi=1?
>> That's not very "multi". Is --rows-per-inse
On Fri, 18 Jan 2019 at 01:15, Surafel Temesgen wrote:
> The attache patch use your method mostly
I disagree with the "mostly" part. As far as I can see, you took the
idea and then made a series of changes to completely break it. For
bonus points, you put back my comment change to make it incorr
On Fri, Jan 4, 2019 at 3:08 PM David Rowley
wrote:
> On Mon, 31 Dec 2018 at 18:58, Surafel Temesgen
> wrote:
> > On Fri, Dec 28, 2018 at 6:46 PM Fabien COELHO
> wrote:
> >> > At first i also try to do it like that but it seems the function will
> >> > became long and more complex to me
> >>
> >
FWIW you can insert multiple zero-column rows with "insert into ..
select union all select union all select".
--
Álvaro Herrerahttps://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On Mon, 31 Dec 2018 at 18:58, Surafel Temesgen wrote:
> On Fri, Dec 28, 2018 at 6:46 PM Fabien COELHO wrote:
>> > At first i also try to do it like that but it seems the function will
>> > became long and more complex to me
>>
>> Probably. But calling it with size 100 should result in the same be
On Thu, Jan 3, 2019 at 1:38 AM David Rowley
wrote:
> On Thu, 3 Jan 2019 at 01:50, Surafel Temesgen
> wrote:
> > On Mon, Dec 31, 2018 at 12:38 PM David Rowley <
> david.row...@2ndquadrant.com> wrote:
> >> Just looking at the v5 patch, it seems not to handle 0 column tables
> correctly.
>
> > The
On Thu, 3 Jan 2019 at 01:50, Surafel Temesgen wrote:
> On Mon, Dec 31, 2018 at 12:38 PM David Rowley
> wrote:
>> Just looking at the v5 patch, it seems not to handle 0 column tables
>> correctly.
> The attach patch contain a fix for it
+ /* if it is zero-column table then we're done */
+ if (
Hi,
Thank you for looking at it
On Mon, Dec 31, 2018 at 12:38 PM David Rowley
wrote:
> Just looking at the v5 patch, it seems not to handle 0 column tables
> correctly.
>
> For example:
>
> # create table t();
> # insert into t default values;
> # insert into t default values;
>
> $ pg_dump --tab
Just looking at the v5 patch, it seems not to handle 0 column tables correctly.
For example:
# create table t();
# insert into t default values;
# insert into t default values;
$ pg_dump --table t --inserts --insert-multi=100 postgres > dump.sql
# \i dump.sql
[...]
INSERT 0 1
psql:dump.sql:35:
On Fri, Dec 28, 2018 at 6:46 PM Fabien COELHO wrote:
>
> > At first i also try to do it like that but it seems the function will
> > became long and more complex to me
>
> Probably. But calling it with size 100 should result in the same behavior,
> so it is really just an extension of the preceed
At first i also try to do it like that but it seems the function will
became long and more complex to me
Probably. But calling it with size 100 should result in the same behavior,
so it is really just an extension of the preceeding one? Or am I missing
something?
--
Fabien.
On Tue, Dec 25, 2018 at 2:47 PM Fabien COELHO wrote:
Thank you for looking into it
>
> Hello Surafel,
>
> > Thank you for informing, Here is an updated patch against current master
>
> Patch applies cleanly, compiles, "make check" is okay, but given that the
> feature is not tested...
>
> Featur
Hello Surafel,
Thank you for informing, Here is an updated patch against current master
Patch applies cleanly, compiles, "make check" is okay, but given that the
feature is not tested...
Feature should be tested somewhere.
ISTM that command-line switches with optional arguments should be
On Fri, Nov 30, 2018 at 7:16 PM Dmitry Dolgov <9erthali...@gmail.com> wrote:
>
> Unfortunately, patch needs to be rebased, could you please post an updated
> version?
>
Thank you for informing, Here is an updated patch against current master
Regards
Surafel
diff --git a/doc/src/sgml/ref/pg_dump.s
> On Thu, Nov 8, 2018 at 2:03 PM Surafel Temesgen wrote:
>
> yes its not much line of code. Attach is a patch that optionally accept the
> number of row in a single insert statement and if it is not specified one row
> per statement used
Hi,
Unfortunately, patch needs to be rebased, could you
On Tue, Nov 6, 2018 at 8:18 PM Alvaro Herrera
wrote:
> On 2018-Nov-06, Surafel Temesgen wrote:
>
> > hi,
> >
> > On Sun, Nov 4, 2018 at 1:18 PM Fabien COELHO
> wrote:
> >
> > > Patch does not seem to apply anymore, could you rebase?
> > >
> > The attached patch is a rebased version and work by ‘
út 6. 11. 2018 v 18:18 odesílatel Alvaro Herrera
napsal:
> On 2018-Nov-06, Surafel Temesgen wrote:
>
> > hi,
> >
> > On Sun, Nov 4, 2018 at 1:18 PM Fabien COELHO
> wrote:
> >
> > > Patch does not seem to apply anymore, could you rebase?
> > >
> > The attached patch is a rebased version and work
On 2018-Nov-06, Surafel Temesgen wrote:
> hi,
>
> On Sun, Nov 4, 2018 at 1:18 PM Fabien COELHO wrote:
>
> > Patch does not seem to apply anymore, could you rebase?
> >
> The attached patch is a rebased version and work by ‘inserts=100’ as
> Stephen suggest
I thought the suggestion was that the
hi,
On Sun, Nov 4, 2018 at 1:18 PM Fabien COELHO wrote:
> Patch does not seem to apply anymore, could you rebase?
>
>
The attached patch is a rebased version and work by ‘inserts=100’ as
Stephen suggest
regards
Surafel
diff --git a/doc/src/sgml/ref/pg_dump.sgml b/doc/src/sgml/ref/pg_dump.sgml
Greetings,
* Michael Paquier (mich...@paquier.xyz) wrote:
> On Wed, Oct 17, 2018 at 03:05:28PM -0400, Stephen Frost wrote:
> > The point of it is that it makes loading into other RDBMS faster. Yes,
> > it has many of the same issues as our COPY does, but we support it
> > because it's much faster
On Wed, Oct 17, 2018 at 03:05:28PM -0400, Stephen Frost wrote:
> The point of it is that it makes loading into other RDBMS faster. Yes,
> it has many of the same issues as our COPY does, but we support it
> because it's much faster. The same is true here, just for other
> databases, so I'm +1 on
The patch attached add additional option for multi values insert statement
with a default values of 100 row per statement so the row lose during error
is at most 100 rather than entire table.
Patch does not seem to apply anymore, could you rebase?
--
Fabien.
Greetings,
* Tom Lane (t...@sss.pgh.pa.us) wrote:
> Surafel Temesgen writes:
> > According to the documentation –inserts option is mainly useful for making
> > dumps that can be loaded into non-PostgreSQL databases and to reduce the
> > amount of rows that might lost during error in reloading but
Surafel Temesgen writes:
> According to the documentation –inserts option is mainly useful for making
> dumps that can be loaded into non-PostgreSQL databases and to reduce the
> amount of rows that might lost during error in reloading but multi values
> insert command are equally portable and com
65 matches
Mail list logo
