On 06/21/2018 04:41 PM, Tom Lane wrote:
Andrew Dunstan writes:
I left that for a separate exercise. I think this does things the way
you want. I must admit to being a bit surprised, I was expecting you to
have more to say about the upgrade function than the pg_dump changes :-)
Well, the upg
Alvaro Herrera writes:
> In terms of pgindent, I'm surprised about these lines:
> +missingval = OidFunctionCall3(
> + F_ARRAY_IN,
> Why did you put a newline there? In ancient times there was a reason
> for that in some cases, because pgindent would move the
In terms of pgindent, I'm surprised about these lines:
+missingval = OidFunctionCall3(
+ F_ARRAY_IN,
Why did you put a newline there? In ancient times there was a reason
for that in some cases, because pgindent would move the argument to the
left of the open
Andrew Dunstan writes:
> I left that for a separate exercise. I think this does things the way
> you want. I must admit to being a bit surprised, I was expecting you to
> have more to say about the upgrade function than the pg_dump changes :-)
Well, the upgrade function is certainly a bit ugly
On 06/21/2018 01:53 PM, Tom Lane wrote:
Andrew Dunstan writes:
On 06/21/2018 01:44 PM, Tom Lane wrote:
So I'm thinking that the attidentity code is just wrong, and you should
change that too while you're at it.
That should be backpatched if changed, no? I don't think we'd want this
to get o
Andrew Dunstan writes:
> On 06/21/2018 01:44 PM, Tom Lane wrote:
>> So I'm thinking that the attidentity code is just wrong, and you should
>> change that too while you're at it.
> That should be backpatched if changed, no? I don't think we'd want this
> to get out of sync between the branches.
Andres Freund writes:
> On 2018-06-21 13:44:19 -0400, Tom Lane wrote:
>> Actually, now that I think about it, there is a concrete reason for the
>> historical pattern: it provides a cross-check that you did not fat-finger
>> the query, ie misspell the column alias vs the PQfnumber parameter. This
On 06/21/2018 01:44 PM, Tom Lane wrote:
Andrew Dunstan writes:
On 06/21/2018 01:18 PM, Tom Lane wrote:
I might be OK with a patch that converts *all* of pg_dump's cross-version
difference handling code to depend on PQfnumber silently returning -1
rather than failing, but I don't want to see
On 2018-06-21 13:44:19 -0400, Tom Lane wrote:
> Andrew Dunstan writes:
> > On 06/21/2018 01:18 PM, Tom Lane wrote:
> >> I might be OK with a patch that converts *all* of pg_dump's cross-version
> >> difference handling code to depend on PQfnumber silently returning -1
> >> rather than failing, but
Andrew Dunstan writes:
> On 06/21/2018 01:18 PM, Tom Lane wrote:
>> I might be OK with a patch that converts *all* of pg_dump's cross-version
>> difference handling code to depend on PQfnumber silently returning -1
>> rather than failing, but I don't want to see it done like that in just
>> one or
Hi,
On 2018-06-21 13:28:46 -0400, Andrew Dunstan wrote:
> I don't think it's at all a bad idea, TBH. It means you only have to add one
> query per version rather than having to adjust all of them.
I'd even say it's an excellent idea.
Greetings,
Andres Freund
On 06/21/2018 12:08 PM, Tom Lane wrote:
I wrote:
We could do without the unrelated whitespace changes in dumpDefaultACL,
too (or did pgindent introduce those? Seems surprising ... oh, looks
like "type" has somehow snuck into typedefs.list. Time for another
blacklist entry.)
Hmm .. actually
On 06/21/2018 01:18 PM, Tom Lane wrote:
Andrew Dunstan writes:
On 06/21/2018 12:39 PM, Tom Lane wrote:
Andres Freund writes:
On June 21, 2018 9:04:28 AM PDT, Tom Lane wrote:
This isn't really ready to go. One clear problem is that you broke
pg_dump'ing from any pre-v11 version, because
Andrew Dunstan writes:
> On 06/21/2018 12:39 PM, Tom Lane wrote:
>> Andres Freund writes:
> On June 21, 2018 9:04:28 AM PDT, Tom Lane wrote:
> This isn't really ready to go. One clear problem is that you broke
> pg_dump'ing from any pre-v11 version, because you did not add suitable
> null outpu
On 06/21/2018 12:39 PM, Tom Lane wrote:
Andres Freund writes:
On June 21, 2018 9:04:28 AM PDT, Tom Lane wrote:
This isn't really ready to go. One clear problem is that you broke
pg_dump'ing from any pre-v11 version, because you did not add suitable
null outputs to the pre-v11 query varian
Andres Freund writes:
> On June 21, 2018 9:04:28 AM PDT, Tom Lane wrote:
>> This isn't really ready to go. One clear problem is that you broke
>> pg_dump'ing from any pre-v11 version, because you did not add suitable
>> null outputs to the pre-v11 query variants in getTableAttrs.
> Thought the
On June 21, 2018 9:04:28 AM PDT, Tom Lane wrote:
>Andrew Dunstan writes:
>> Patch after pgindent attached. This will involve a catversion bump
>since
>> we're adding a new function.
>
>This isn't really ready to go. One clear problem is that you broke
>pg_dump'ing from any pre-v11 version, b
I wrote:
> We could do without the unrelated whitespace changes in dumpDefaultACL,
> too (or did pgindent introduce those? Seems surprising ... oh, looks
> like "type" has somehow snuck into typedefs.list. Time for another
> blacklist entry.)
Hmm .. actually, "type" already is in pgindent's blac
Andrew Dunstan writes:
> Patch after pgindent attached. This will involve a catversion bump since
> we're adding a new function.
This isn't really ready to go. One clear problem is that you broke
pg_dump'ing from any pre-v11 version, because you did not add suitable
null outputs to the pre-v11
On 06/20/2018 09:04 PM, Andres Freund wrote:
Probably couldn't hurt to run the changed files through
pgindent and fix the damage...
Looks reasonable to me, but I've not tested it.
Thanks
Patch after pgindent attached. This will involve a catversion bump since
we're adding a new funct
Hi,
On 2018-06-20 20:53:34 -0400, Andrew Dunstan wrote:
> This version adds a lock on the table owning the attribute.
Cool.
>
> /*
> + * in binary upgrade mode, update the catalog with any missing
> values
> + * that might be present.
> + */
On 06/20/2018 01:46 PM, Andrew Dunstan wrote:
Attached deals with all those issues except locking.
This version adds a lock on the table owning the attribute.
cheers
andrew
--
Andrew Dunstanhttps://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, T
On 06/20/2018 12:58 PM, Andres Freund wrote:
Hi,
Just a quick scan-through review:
On 2018-06-20 12:51:07 -0400, Andrew Dunstan wrote:
diff --git a/src/backend/utils/adt/pg_upgrade_support.c
b/src/backend/utils/adt/pg_upgrade_support.c
index 0c54b02..2666ab2 100644
--- a/src/backend/utils/a
Hi,
Just a quick scan-through review:
On 2018-06-20 12:51:07 -0400, Andrew Dunstan wrote:
> diff --git a/src/backend/utils/adt/pg_upgrade_support.c
> b/src/backend/utils/adt/pg_upgrade_support.c
> index 0c54b02..2666ab2 100644
> --- a/src/backend/utils/adt/pg_upgrade_support.c
> +++ b/src/back
On 06/19/2018 10:46 PM, Andres Freund wrote:
On 2018-06-19 22:41:26 -0400, Andrew Dunstan wrote:
This unfortunately crashes and burns if we use DirectFunctionCall3 to call
array_in, because it uses fn_extra. There is the CallerFInfoFunctionCall
stuff, but it only has 1 and 2 arg variants, and
On 2018-06-19 22:41:26 -0400, Andrew Dunstan wrote:
> This unfortunately crashes and burns if we use DirectFunctionCall3 to call
> array_in, because it uses fn_extra. There is the CallerFInfoFunctionCall
> stuff, but it only has 1 and 2 arg variants, and array_in takes 3. In
> retrospect we should
On 06/19/2018 01:19 PM, Tom Lane wrote:
Andres Freund writes:
On 2018-06-19 12:37:52 -0400, Tom Lane wrote:
The problem here is that that function does not exist in 11beta1.
Since adding the "incoming" function is certainly going to require
initdb, we have to be able to dump from the server
"David G. Johnston" writes:
> On Tue, Jun 19, 2018 at 9:37 AM, Tom Lane wrote:
>> The problem here is that that function does not exist in 11beta1.
>> Since adding the "incoming" function is certainly going to require
>> initdb, we have to be able to dump from the server as it now stands,
>> or w
On Tue, Jun 19, 2018 at 9:37 AM, Tom Lane wrote:
> The problem here is that that function does not exist in 11beta1.
> Since adding the "incoming" function is certainly going to require
> initdb, we have to be able to dump from the server as it now stands,
> or we'll be cutting existing beta test
On 06/19/2018 01:19 PM, Tom Lane wrote:
Andres Freund writes:
On 2018-06-19 12:37:52 -0400, Tom Lane wrote:
The problem here is that that function does not exist in 11beta1.
Since adding the "incoming" function is certainly going to require
initdb, we have to be able to dump from the server
On Tue, Jun 19, 2018 at 10:12 AM, Robert Haas wrote:
> I have to admit that I think this feature is scary. I'm not sure that
> it was adequately reviewed and tested, and I'm worried this may not be
> the only problem it causes. But this particular problem, as Andres
> says, doesn't seem like anyth
Andres Freund writes:
> On 2018-06-19 12:37:52 -0400, Tom Lane wrote:
>> The problem here is that that function does not exist in 11beta1.
>> Since adding the "incoming" function is certainly going to require
>> initdb, we have to be able to dump from the server as it now stands,
>> or we'll be cu
On Tue, Jun 19, 2018 at 12:37 PM, Tom Lane wrote:
> The problem here is that that function does not exist in 11beta1.
> Since adding the "incoming" function is certainly going to require
> initdb, we have to be able to dump from the server as it now stands,
> or we'll be cutting existing beta test
Hi,
On 2018-06-19 12:37:52 -0400, Tom Lane wrote:
> Andres Freund writes:
> > And if the default value bugs us,
> > we can easily add a support function that dumps the value without the
> > anyarray adornment?
>
> The problem here is that that function does not exist in 11beta1.
> Since adding t
Andres Freund writes:
> On 2018-06-19 12:17:56 -0400, Tom Lane wrote:
>> The hard part here is how exactly are we going to represent the default
>> value. AFAICS, the only thing that pg_dump could readily lay its hands
>> on is the "anyarray" textual representation of attmissingval, which maybe
>
On 2018-06-19 12:17:59 -0400, Andrew Dunstan wrote:
>
>
> On 06/19/2018 12:05 PM, Andres Freund wrote:
> > Hi,
> >
> > On 2018-06-19 11:51:16 -0400, Andrew Dunstan wrote:
> > > My initial thought was that as a fallback we should disable pg_upgrade on
> > > databases containing such values, and d
On 2018-06-19 12:17:56 -0400, Tom Lane wrote:
> Andrew Dunstan writes:
> > I have no idea how large an actual fix might be. I'll at least start
> > working on it immediately. I agree it's very late in the day.
>
> On reflection, it seems like there are two moving parts needed:
>
> * Add a binar
On 06/19/2018 12:05 PM, Andres Freund wrote:
Hi,
On 2018-06-19 11:51:16 -0400, Andrew Dunstan wrote:
My initial thought was that as a fallback we should disable pg_upgrade on
databases containing such values, and document the limitation in the docs
and the release notes. The workaround would
Andrew Dunstan writes:
> I have no idea how large an actual fix might be. I'll at least start
> working on it immediately. I agree it's very late in the day.
On reflection, it seems like there are two moving parts needed:
* Add a binary-upgrade support function to the backend, which would take,
Andres Freund writes:
> On 2018-06-19 11:51:16 -0400, Andrew Dunstan wrote:
>> Have we ever recommended use of pg_upgrade for some manual catalog fix after
>> release? I don't recall doing so. Certainly it hasn't been common.
> No, but why does it matter?
We absolutely have, as recently as last
Hi,
On 2018-06-19 11:51:16 -0400, Andrew Dunstan wrote:
> My initial thought was that as a fallback we should disable pg_upgrade on
> databases containing such values, and document the limitation in the docs
> and the release notes. The workaround would be to force a table rewrite
> which would cl
On 06/19/2018 10:55 AM, Tom Lane wrote:
AFAICS, the fast-default patch neglected to consider what happens if
a database containing columns with active attmissingval entries is
pg_upgraded. I do not see any code in either pg_dump or pg_upgrade that
attempts to deal with that situation, which m
AFAICS, the fast-default patch neglected to consider what happens if
a database containing columns with active attmissingval entries is
pg_upgraded. I do not see any code in either pg_dump or pg_upgrade that
attempts to deal with that situation, which means the effect will be
that the "missing" va
43 matches
Mail list logo
