Re: Fix of fake unlogged LSN initialization
On Fri, Oct 25, 2019 at 02:11:55AM +, tsunakawa.ta...@fujitsu.com wrote: > Thanks for taking a look. I'm afraid my patch includes the fix for this part. Yes. And now this is applied and back-patched. -- Michael signature.asc Description: PGP signature
Re: Proposal: Add more compile-time asserts to expose inconsistencies.
Hi, On October 26, 2019 6:06:07 AM PDT, Peter Eisentraut wrote: >On 2019-10-10 00:52, Smith, Peter wrote: >> I liked your idea of using an extern function declaration for >implementing the file-scope compile-time asserts. AFAIK it is valid >standard C. >> >> Thank you for the useful link to that compiler explorer. I tried many >scenarios of the new StaticAssertDecl and all seemed to work ok. >> https://godbolt.org/z/fDrmXi >> >> The patch has been updated accordingly. All assertions identified in >the original post are now adjacent the global variables they are >asserting. >> > >The problem with this implementation is that you get a crappy error >message when the assertion fails, namely something like: > >../../../../src/include/c.h:862:84: error: size of array >'static_assert_failure' is negative My proposal for this really was just to use this as a fallback for when static assert isn't available. Which in turn I was just suggesting because Tom wanted a fallback. >Ideally, the implementation should end up calling _Static_assert() >somehow, so that we get the compiler's native error message. > >We could do a configure check for whether _Static_assert() works at >file >scope. I don't know what the support for that is, but it seems to work >in gcc and clang. I think it should work everywhere that has static assert. So we should need a separate configure check. Andres -- Sent from my Android device with K-9 Mail. Please excuse my brevity.
Re: Proposition to use '==' as synonym for 'IS NOT DISTINCT FROM'
Hi, On October 26, 2019 4:09:29 PM PDT, Vik Fearing wrote: >On 26/10/2019 17:41, Eugen Konkov wrote: >> Hi. >> >> I have noticed that it would be cool to use '==' in place of 'IS >NOT >> DISTICT FROM' >> >> What do you think about this crazy idea? > > >I think this is a terrible idea. The only reason to do this would be >to >index it, but indexes (btree at least) expect STRICT operators, which >this would not be. It sounds like what's being suggested is just some abbreviated formulation of IS NOT DISTINCT. If implement that way, rather than manually adding non strict operators, I don't think there would be an indexing issue. Andres -- Sent from my Android device with K-9 Mail. Please excuse my brevity.
Re: Proposition to use '==' as synonym for 'IS NOT DISTINCT FROM'
On 26/10/2019 17:41, Eugen Konkov wrote: > Hi. > > I have noticed that it would be cool to use '==' in place of 'IS NOT > DISTICT FROM' > > What do you think about this crazy idea? I think this is a terrible idea. The only reason to do this would be to index it, but indexes (btree at least) expect STRICT operators, which this would not be.
Re: pg_dump compatibility level / use create view instead of create table/rule
Andres Freund writes: > On 2019-10-10 11:20:14 -0400, Tom Lane wrote: >> The reason we get "REPLICA IDENTITY NOTHING" is that a view's relreplident >> is set to 'n' not 'd', which might not have been a great choice. > Hm, yea. I wonder if we should add a REPLICA_IDENTITY_INVALID or such, > for non relation relkinds? I'm mildly inclined to think that setting it > to REPLICA_IDENTITY_DEFAULT is at least as confusing as > REPLICA_IDENTITY_DEFAULT... Yeah, I'd be for that in HEAD probably. But of course we can't change the 9.x branches like that. >> This is fixed in v10 and up thanks to d8c05aff5. I was hesitant to >> back-patch that at the time, but now that it's survived in the field >> for a couple years, I think a good case could be made for doing so. > +1 Just finishing up the back-patch now. regards, tom lane
Re: Proposition to use '==' as synonym for 'IS NOT DISTINCT FROM'
Andres Freund writes:
> On 2019-10-26 14:23:49 -0400, Tom Lane wrote:
>> ... instead of
>> x IS NOT DISTINCT FROM y
>> I'm vaguely imagining
>> x = {magic} y
>> where unlike Eugen's suggestion, "=" is the real name of the underlying
>> comparison operator. For dump/restore this could be spelled verbosely
>> as
>> x OPERATOR(someplace.=) {magic} y
>> The hard part is to figure out some {magic} annotation that is both
>> short and unambiguous. We have to cover the IS DISTINCT variant, too.
To clarify, what I have in mind here doesn't have any effect whatever
on the parse tree or the execution semantics, it's just about offering
an alternative SQL textual representation.
> Leaving the exact choice of how {magic} would look like, are you
> thinking of somehow making it work for every operator, or just for some
> subset? It's intriguing to have something generic, but I'm not quite
> clear how that'd would work? It's not clear to me how we'd
> automatically infer a sensible meaning for e.g. < etc.
Yeah, I think it could only be made to work sanely for underlying
operators that have the semantics of equality. The NOT DISTINCT
wrapper has the semantics
NULL vs NULL-> true
NULL vs not-NULL-> false
not-NULL vs NULL-> false
not-NULL vs not-NULL-> apply operator
and while theoretically the operator needn't be equality, those
NULL behaviors don't make much sense otherwise. (IS DISTINCT
just inverts all the results, of course.)
I suppose that we could also imagine generalizing DistinctExpr
into something that could work with other operator semantics,
but as you say, it's a bit hard to wrap ones head around what
that would look like.
> And even if we just restrict it to = (and presumably <> and !=), in
> which cases is this magic going to work? Would we tie it to the textual
> '=', '<>' operators? btree opclass members?
See the other thread I cited --- right now, the underlying operator is
always "=" and it's looked up by name. Whether that ought to change
seems like a separate can o' worms.
regards, tom lane
Re: pg_dump compatibility level / use create view instead of create table/rule
Hi, On 2019-10-10 11:20:14 -0400, Tom Lane wrote: > regression=# create table mytab (f1 int primary key, f2 text); > CREATE TABLE > regression=# create view myview as select * from mytab group by f1; > CREATE VIEW > > This situation is problematic for pg_dump because validity of the > view depends on the existence of mytab's primary key constraint, > and we don't create primary keys till late in the restore process. > So it has to break myview into two parts, one to emit during normal > table/view creation and one to emit after index creation. > > With 9.5's pg_dump, what comes out is: > > -- > -- Name: myview; Type: TABLE; Schema: public; Owner: postgres > -- > > CREATE TABLE public.myview ( > f1 integer, > f2 text > ); > > ALTER TABLE ONLY public.myview REPLICA IDENTITY NOTHING; Ick. > The reason we get "REPLICA IDENTITY NOTHING" is that a view's relreplident > is set to 'n' not 'd', which might not have been a great choice. Hm, yea. I wonder if we should add a REPLICA_IDENTITY_INVALID or such, for non relation relkinds? I'm mildly inclined to think that setting it to REPLICA_IDENTITY_DEFAULT is at least as confusing as REPLICA_IDENTITY_DEFAULT... > This is fixed in v10 and up thanks to d8c05aff5. I was hesitant to > back-patch that at the time, but now that it's survived in the field > for a couple years, I think a good case could be made for doing so. +1 > /* pretend view is a plain table and dump it that way */ > viewinfo->relkind = 'r';/* RELKIND_RELATION */ > viewinfo->relkind = 'r';/* RELKIND_RELATION */ > + viewinfo->relreplident = 'd'; /* REPLICA_IDENTITY_DEFAULT */ > > That's mighty ugly but it doesn't seem to carry any particular > risk. I also could live with this, given it'd only be in older back-branches. Greetings, Andres Freund
Re: Proposition to use '==' as synonym for 'IS NOT DISTINCT FROM'
Hi,
On 2019-10-26 14:23:49 -0400, Tom Lane wrote:
> I wrote:
> > We do have some unresolved issues around how to let dump/restore
> > control the interpretation of IS [NOT] DISTINCT FROM, cf
> > https://www.postgresql.org/message-id/flat/ffefc172-a487-aa87-a0e7-472bf29735c8%40gmail.com
> > but I don't think this idea is helping with that at all.
>
> BTW, taking a step back and viewing this suggestion as "it'd be nice
> to have *some* shorter notation than IS [NOT] DISTINCT FROM", maybe
> there's a way to unify that desire with the dump/restore fix. What
> we'd really need to fix the dump/restore problem, AFAICS, is to name
> the underlying equality operator --- potentially with a schema
> qualification --- but then have some notation that says "handle NULLs
> like IS [NOT] DISTINCT FROM does". So instead of
>
> x IS NOT DISTINCT FROM y
>
> I'm vaguely imagining
>
> x = {magic} y
>
> where unlike Eugen's suggestion, "=" is the real name of the underlying
> comparison operator. For dump/restore this could be spelled verbosely
> as
>
> x OPERATOR(someplace.=) {magic} y
>
> The hard part is to figure out some {magic} annotation that is both
> short and unambiguous. We have to cover the IS DISTINCT variant, too.
Leaving the exact choice of how {magic} would look like, are you
thinking of somehow making it work for every operator, or just for some
subset? It's intriguing to have something generic, but I'm not quite
clear how that'd would work? It's not clear to me how we'd
automatically infer a sensible meaning for e.g. < etc.
And even if we just restrict it to = (and presumably <> and !=), in
which cases is this magic going to work? Would we tie it to the textual
'=', '<>' operators? btree opclass members?
Greetings,
Andres Freund
Re: define bool in pgtypeslib_extern.h
Andrew Gierth writes: > "Tom" == Tom Lane writes: > Tom> I'm inclined to think that we need to make ecpglib.h's > Tom> bool-related definitions exactly match c.h, > I'm wondering whether we should actually go the opposite way and say > that c.h's "bool" definition should be backend only, and that in > frontend code we should define a PG_bool type or something of that ilk > for when we want "PG's 1-byte bool" and otherwise let the platform > define "bool" however it wants. > And we certainly shouldn't be defining "bool" in something that's going > to be included in the user's code the way that ecpglib.h is. The trouble here is the hazard of creating an ABI break, if we modify ecpglib.h in a way that causes its "bool" references to be interpreted differently than they were before. I don't think we want that (although I suspect we have inadvertently caused ABI breaks already on platforms where this matters). In practice, since v11 on every modern platform, the exported ecpglib functions have supposed that "bool" is _Bool, because they were compiled in files that included c.h before ecpglib.h. I assert furthermore that clients might well have included before ecpglib.h and thereby been fully compatible with that. If we start having ecpglib.h include itself, we'll just be eliminating a minor header inclusion order hazard. It's also rather hard to argue that including automatically is really likely to break anything that was including ecpglib.h already, since that file was already usurping those symbols. Except on platforms where sizeof(_Bool) isn't 1, but things are already pretty darn broken there. I think it's possible to construct a counterexample that will fail for *anything* we can do here. I'm not inclined to uglify things like mad to reduce the problem space from 0.1% to 0.01% of use-cases, or whatever the numbers would be in practice. regards, tom lane
Re: define bool in pgtypeslib_extern.h
> "Tom" == Tom Lane writes: Tom> On closer inspection, it seems to be just blind luck. For example, Tom> if I rearrange the inclusion order in a file using ecpglib.h: Ugh. Tom> I'm inclined to think that we need to make ecpglib.h's Tom> bool-related definitions exactly match c.h, I'm wondering whether we should actually go the opposite way and say that c.h's "bool" definition should be backend only, and that in frontend code we should define a PG_bool type or something of that ilk for when we want "PG's 1-byte bool" and otherwise let the platform define "bool" however it wants. And we certainly shouldn't be defining "bool" in something that's going to be included in the user's code the way that ecpglib.h is. -- Andrew.
Re: pg_dump compatibility level / use create view instead of create table/rule
I wrote: > Alex Williams writes: >> [ gripes about pg_dump printing REPLICA IDENTITY NOTHING for a view ] > This is fixed in v10 and up thanks to d8c05aff5. I was hesitant to > back-patch that at the time, but now that it's survived in the field > for a couple years, I think a good case could be made for doing so. > After a bit of looking around, the main argument I can find against > it is that emitting 'CREATE OR REPLACE VIEW' in a dropStmt will > break pg_restore versions preceding this commit: > Author: Tom Lane > Branch: master Release: REL_10_BR [ac888986f] 2016-11-17 14:59:13 -0500 > Branch: REL9_6_STABLE Release: REL9_6_2 [0eaa5118a] 2016-11-17 14:59:19 -0500 > Branch: REL9_5_STABLE Release: REL9_5_6 [a7864037d] 2016-11-17 14:59:23 -0500 > Branch: REL9_4_STABLE Release: REL9_4_11 [e69b532be] 2016-11-17 14:59:26 -0500 > Improve pg_dump/pg_restore --create --if-exists logic. After further digging, I remembered that we bumped the archive file version number in 3d2aed664 et al. to fix CVE-2018-1058. So current versions of pg_dump already emit archive files that will be rejected by pg_restore versions preceding the above fix, and so there should be no downside to emitting data that depends on it. I'll go see about backpatching d8c05aff5. regards, tom lane
Re: Proposition to use '==' as synonym for 'IS NOT DISTINCT FROM'
I wrote:
> We do have some unresolved issues around how to let dump/restore
> control the interpretation of IS [NOT] DISTINCT FROM, cf
> https://www.postgresql.org/message-id/flat/ffefc172-a487-aa87-a0e7-472bf29735c8%40gmail.com
> but I don't think this idea is helping with that at all.
BTW, taking a step back and viewing this suggestion as "it'd be nice
to have *some* shorter notation than IS [NOT] DISTINCT FROM", maybe
there's a way to unify that desire with the dump/restore fix. What
we'd really need to fix the dump/restore problem, AFAICS, is to name
the underlying equality operator --- potentially with a schema
qualification --- but then have some notation that says "handle NULLs
like IS [NOT] DISTINCT FROM does". So instead of
x IS NOT DISTINCT FROM y
I'm vaguely imagining
x = {magic} y
where unlike Eugen's suggestion, "=" is the real name of the underlying
comparison operator. For dump/restore this could be spelled verbosely
as
x OPERATOR(someplace.=) {magic} y
The hard part is to figure out some {magic} annotation that is both
short and unambiguous. We have to cover the IS DISTINCT variant, too.
regards, tom lane
Re: vacuum on table1 skips rows because of a query on table2
If long-running transaction is "read committed", then we are sure that any new query coming (even on same table1 as vacuum table) will need snapshot on point of time query start and not the time transaction starts (but still why read committed transaction on table2 cause vacuum on table1 to skip rows). Hence if a vacuum on table1 sees that all the transactions in the database are "read committed" and no one accessing table1, vacuum should be able to clear dead rows. For read committed transactions, different table should not interfere with each other. On Fri, Oct 25, 2019 at 10:16 PM Tom Lane wrote: > Virender Singla writes: > > Currently I see the vacuum behavior for a table is that, even if a long > > running query on a different table is executing in another read committed > > transaction. > > That vacuum in the 1st transaction skips the dead rows until the long > > running query finishes. > > Why that is the case, On same table long running query blocking vacuum we > > can understand but why query on a different table block it. > > Probably because vacuum's is-this-row-dead-to-everyone tests are based > on the global xmin minimum. This must be so, because even if the > long-running transaction hasn't touched the table being vacuumed, > we don't know that it won't do so in future. So we can't remove > rows that it should be able to see if it were to look. > > regards, tom lane >
Re: define bool in pgtypeslib_extern.h
Amit Kapila writes: > On Fri, Oct 25, 2019 at 9:55 PM Tom Lane wrote: >> However, we're not out of the woods, because lookee here in >> ecpglib.h: >> #ifndef bool >> #define bool char >> #endif /* ndef bool */ >> I'm more than slightly surprised that we haven't already seen >> problems due to this conflicting with d26a810eb. > I think it is because it never gets any imports from c.h. On closer inspection, it seems to be just blind luck. For example, if I rearrange the inclusion order in a file using ecpglib.h: diff --git a/src/interfaces/ecpg/ecpglib/data.c b/src/interfaces/ecpg/ecpglib/data.c index 7d2a78a..09944ff 100644 --- a/src/interfaces/ecpg/ecpglib/data.c +++ b/src/interfaces/ecpg/ecpglib/data.c @@ -6,8 +6,8 @@ #include #include "ecpgerrno.h" -#include "ecpglib.h" #include "ecpglib_extern.h" +#include "ecpglib.h" #include "ecpgtype.h" #include "pgtypes_date.h" #include "pgtypes_interval.h" then on a PPC Mac I get data.c:210: error: conflicting types for 'ecpg_get_data' ecpglib_extern.h:167: error: previous declaration of 'ecpg_get_data' was here It's exactly the same problem as we saw with pgtypeslib_extern.h: header ordering changes affect the meaning of uses of bool, and that's just not acceptable. In this case it's even worse because we're mucking with type definitions in a user-visible header. I'm surprised we've not gotten bug reports about that. Maybe all ECPG users include before they include ecpglib.h, but that doesn't exactly make things worry-free either, because code doing that will think that these functions return _Bool, when the compiled library possibly thinks differently. Probably the only thing saving us is that sizeof(_Bool) is 1 on just about every platform in common use nowadays. I'm inclined to think that we need to make ecpglib.h's bool-related definitions exactly match c.h, which will mean that it has to pull in on most platforms, which will mean adding a control symbol for that to ecpg_config.h. I do not think we should export HAVE_STDBOOL_H and SIZEOF_BOOL there though; probably better to have configure make the choice and export something named like PG_USE_STDBOOL. regards, tom lane
Re: Proposition to use '==' as synonym for 'IS NOT DISTINCT FROM'
On Sat, Oct 26, 2019 at 12:49 PM Tom Lane wrote: > David Fetter writes: > > On Sat, Oct 26, 2019 at 06:41:10PM +0300, Eugen Konkov wrote: > >> I have noticed that it would be cool to use '==' in place of 'IS NOT > >> DISTICT FROM' > >> What do you think about this crazy idea? > > > Turning "IS NOT DISTINCT FROM" into an operator sounds like a great > > idea. > > No it isn't. +1 -- Jonah H. Harris
Re: Proposition to use '==' as synonym for 'IS NOT DISTINCT FROM'
David Fetter writes: > On Sat, Oct 26, 2019 at 06:41:10PM +0300, Eugen Konkov wrote: >> I have noticed that it would be cool to use '==' in place of 'IS NOT >> DISTICT FROM' >> What do you think about this crazy idea? > Turning "IS NOT DISTINCT FROM" into an operator sounds like a great > idea. No it isn't. For starters, somebody very possibly has used that operator name in an extension. For another, it'd be really inconsistent to have an abbreviation for 'IS NOT DISTINCT FROM' but not 'IS DISTINCT FROM', so you'd need another reserved operator name for that, making the risk of breakage worse. There's an independent set of arguments around why we'd invent a proprietary replacement for perfectly good standard SQL. We do have some unresolved issues around how to let dump/restore control the interpretation of IS [NOT] DISTINCT FROM, cf https://www.postgresql.org/message-id/flat/ffefc172-a487-aa87-a0e7-472bf29735c8%40gmail.com but I don't think this idea is helping with that at all. regards, tom lane
Re: Proposition to use '==' as synonym for 'IS NOT DISTINCT FROM'
On Sat, Oct 26, 2019 at 06:41:10PM +0300, Eugen Konkov wrote: > Hi. > > I have noticed that it would be cool to use '==' in place of 'IS NOT > DISTICT FROM' > > What do you think about this crazy idea? Turning "IS NOT DISTINCT FROM" into an operator sounds like a great idea. Let the name bike-shedding begin! Best, David. -- David Fetter http://fetter.org/ Phone: +1 415 235 3778 Remember to vote! Consider donating to Postgres: http://www.postgresql.org/about/donate
Re: fairywren failures
On 10/25/19 3:09 PM, Peter Eisentraut wrote: > On 2019-10-16 13:34, Andrew Dunstan wrote: >>> Could you please check how this animal is labeled? AFAICT, this is not >>> an msys2 build but a mingw build (x86_64-w64-mingw32). >> It is indeed an msys2 system. However, when we set MSYSTEM=MINGW64 as >> we do in fairywren's config environment so that the compiler it is >> properly detected by configure (using Msys2's /etc/config.site) >> 'uname -a' reports MINGW64... instead of MSYS... > When you install MSYS2 from msys2.org, you get three possible build > targets, depending on what you set MSYSTEM to: > > MSYSTEM=MINGW32 > MSYSTEM=MINGW64 > MSYSTEM=MSYS > > When a buildfarm member identifiers itself as "msys ... 2", then I would > expect the third variant, but that's not what it's doing. A > MSYSTEM=MSYS build is similar to a Cygwin build (since MSYS2 is a fork > of Cygwin), which is also a valid thing to do, but it's obviously quite > different from a mingw build. If it helps you I can change the compiler name in the animal metainfo to mingw64-gcc. Msys2 is the build environment, but not the target, which is native Windows. cheers andrew -- Andrew Dunstanhttps://www.2ndQuadrant.com PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Proposition to use '==' as synonym for 'IS NOT DISTINCT FROM'
Hi. I have noticed that it would be cool to use '==' in place of 'IS NOT DISTICT FROM' What do you think about this crazy idea? -- Best regards, Eugen Konkov
Re: Proposal: Add more compile-time asserts to expose inconsistencies.
On 2019-10-10 00:52, Smith, Peter wrote: > I liked your idea of using an extern function declaration for implementing > the file-scope compile-time asserts. AFAIK it is valid standard C. > > Thank you for the useful link to that compiler explorer. I tried many > scenarios of the new StaticAssertDecl and all seemed to work ok. > https://godbolt.org/z/fDrmXi > > The patch has been updated accordingly. All assertions identified in the > original post are now adjacent the global variables they are asserting. > The problem with this implementation is that you get a crappy error message when the assertion fails, namely something like: ../../../../src/include/c.h:862:84: error: size of array 'static_assert_failure' is negative Ideally, the implementation should end up calling _Static_assert() somehow, so that we get the compiler's native error message. We could do a configure check for whether _Static_assert() works at file scope. I don't know what the support for that is, but it seems to work in gcc and clang. -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Re: errbacktrace
On 2019-09-30 20:16, Peter Eisentraut wrote: > On 2019-09-27 17:50, Alvaro Herrera wrote: >> On 2019-Sep-13, Alvaro Herrera wrote: >> >>> On 2019-Aug-20, Peter Eisentraut wrote: >>> The memory management of that seems too complicated. The "extra" mechanism of the check/assign hooks only supports one level of malloc. Using a List seems impossible. I don't know if you can safely do a malloc-ed array of malloc-ed strings either. >>> >>> Here's an idea -- have the check/assign hooks create a different >>> representation, which is a single guc_malloc'ed chunk that is made up of >>> every function name listed in the GUC, separated by \0. That can be >>> scanned at error time comparing the function name with each piece. >> >> Peter, would you like me to clean this up for commit, or do you prefer >> to keep authorship and get it done yourself? > > If you want to finish it using the idea from your previous message, > please feel free. I won't get to it this week. I hadn't realized that you had already attached a patch that implements your idea. It looks good to me. Maybe a small comment near check_backtrace_functions() why we're not using a regular list. Other than that, please go ahead with this. -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Restore replication settings when modifying a field type
When the user modifies the REPLICA IDENTIFY field type, the logical
replication settings are lost.
For example:
postgres=# \d+ t1
Table "public.t1"
Column | Type | Collation | Nullable | Default | Storage | Stats
target | Description
+-+---+--+-+-+--+-
col1 | integer | | | | plain |
|
col2 | integer | | not null | | plain |
|
Indexes:
"t1_col2_key" UNIQUE CONSTRAINT, btree (col2) REPLICA IDENTITY
postgres=# alter table t1 alter col2 type smallint;
ALTER TABLE
postgres=# \d+ t1
Table "public.t1"
Column | Type | Collation | Nullable | Default | Storage | Stats
target | Description
+--+---+--+-+-+--+-
col1 | integer | | | | plain |
|
col2 | smallint | | not null | | plain |
|
Indexes:
"t1_col2_key" UNIQUE CONSTRAINT, btree (col2)
In fact, the replication property of the table has not been modified,
and it is still 'i'(REPLICA_IDENTITY_INDEX). But the previously
specified index property 'indisreplident' is set to false because of the
rebuild.
So I developed a patch. If the user modifies the field type. The
associated index is REPLICA IDENTITY. Rebuild and restore replication
settings.
Regards,
Quan Zongliang
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index b53f6ed3ac..c21372fe51 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -175,6 +175,8 @@ typedef struct AlteredTableInfo
List *changedConstraintDefs; /* string definitions of same */
List *changedIndexOids; /* OIDs of indexes to rebuild */
List *changedIndexDefs; /* string definitions of same */
+ OidchangedReplIdentOid; /* OID of index to reset
REPLICA IDENTIFY */
+ char *changedReplIdentDef;/* string definitions of same */
} AlteredTableInfo;
/* Struct describing one new constraint to check in Phase 3 scan */
@@ -428,6 +430,7 @@ static ObjectAddress ATExecAlterColumnType(AlteredTableInfo
*tab, Relation rel,
AlterTableCmd *cmd, LOCKMODE
lockmode);
static void RememberConstraintForRebuilding(Oid conoid, AlteredTableInfo *tab);
static void RememberIndexForRebuilding(Oid indoid, AlteredTableInfo *tab);
+static void RememberReplicaIdentForRebuilding(Oid indoid, AlteredTableInfo
*tab);
static ObjectAddress ATExecAlterColumnGenericOptions(Relation rel, const char
*colName,
List *options,
LOCKMODE lockmode);
static void ATPostAlterTypeCleanup(List **wqueue, AlteredTableInfo *tab,
@@ -9991,6 +9994,9 @@ ATExecAlterColumnType(AlteredTableInfo *tab, Relation rel,
{
Assert(foundObject.objectSubId
== 0);
RememberIndexForRebuilding(foundObject.objectId, tab);
+
+ if
(RelationGetForm(rel)->relreplident==REPLICA_IDENTITY_INDEX)
+
RememberReplicaIdentForRebuilding(foundObject.objectId, tab);
}
else if (relKind == RELKIND_SEQUENCE)
{
@@ -10010,8 +10016,14 @@ ATExecAlterColumnType(AlteredTableInfo *tab, Relation
rel,
}
case OCLASS_CONSTRAINT:
- Assert(foundObject.objectSubId == 0);
-
RememberConstraintForRebuilding(foundObject.objectId, tab);
+ {
+ Oid indId;
+ Assert(foundObject.objectSubId == 0);
+
RememberConstraintForRebuilding(foundObject.objectId, tab);
+ indId =
get_constraint_index(foundObject.objectId);
+ if (OidIsValid(indId))
+
RememberReplicaIdentForRebuilding(indId, tab);
+ }
break;
case OCLASS_REWRITE:
@@ -10324,6 +10336,36 @@ RememberConstraintForRebuilding(Oid conoid,
AlteredTableInfo *tab)
}
}
+/*
+ * Subroutine for ATExecAlterColumnType: remember that a replica identify
+ * needs to be reset (which we might already know).
+ */
+static void
+RememberReplicaIdentForRebuilding(Oid indoid, AlteredTableInfo *tab)
+{
+ char *defstring;
+
+ /*
+
Re: MinGW compiler warnings in ecpg tests
> These files don't use our printf replacement or the c.h porting > layer, > so unless we want to start doing that, I propose the attached patch > to > determine the appropriate format conversion the hard way. I don't think such porting efforts are worth it for a single test case, or in other words, if you ask me go ahead with your patch. Michael -- Michael Meskes Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org) Meskes at (Debian|Postgresql) dot Org Jabber: michael at xmpp dot meskes dot org VfL Borussia! Força Barça! SF 49ers! Use Debian GNU/Linux, PostgreSQL
Cleanup - Removal of apply_typmod function in #if 0
Hi, One of the function apply_typmod in numeric.c file present within #if 0. This is like this for many years. I felt this can be removed. Attached patch contains the changes to handle removal of apply_typmod present in #if 0. Thoughts? Regards, Vignesh EnterpriseDB: http://www.enterprisedb.com 0001-Cleanup-removal-of-function-present-in-if-0.patch Description: Binary data
Re: TOAST corruption in standby database
On Fri, Oct 25, 2019 at 1:50 AM Alex Adriaanse wrote: > > Standby (corrupted): > > # dd if=data/base/18034/16103928.13 bs=8192 skip=89185 count=1 status=none | > hexdump -C | head -8 > a3 0e 00 00 48 46 88 0e 00 00 05 00 30 00 58 0f |HF..0.X.| > 0010 00 20 04 20 00 00 00 00 00 00 00 00 00 00 00 00 |. . | > 0020 10 98 e0 0f 98 97 e8 00 a8 8f e0 0f 58 8f 96 00 |X...| > 0030 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 || > * > 0f50 00 00 00 00 00 00 00 00 32 b0 0a 01 00 00 00 00 |2...| > 0f60 00 00 00 00 1b 00 61 5c 06 00 03 00 02 09 18 00 |..a\| > 0f70 9b 90 75 02 01 00 00 00 ac 00 00 00 83 9f 64 00 |..u...d.| > > Primary: > > # dd if=data/base/18034/16103928.13 bs=8192 skip=89185 count=1 status=none | > hexdump -C | head -8 > bd 0e 00 00 08 ad 32 b7 00 00 05 00 30 00 90 04 |..2.0...| > 0010 00 20 04 20 00 00 00 00 68 87 e0 0f 90 84 a8 05 |. . h...| > 0020 10 98 e0 0f 98 97 e8 00 a8 8f e0 0f 58 8f 96 00 |X...| > 0030 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 || > * > 0490 a6 07 7e 02 00 00 00 00 00 00 00 00 1b 00 61 5c |..~...a\| > 04a0 02 00 03 00 02 09 18 00 ae 9d d4 03 01 00 00 00 || > 04b0 d0 0a 00 00 23 25 10 07 88 02 13 0f 2c 04 78 01 |#%..,.x.| > > Based on the above observations it seems to me that occasionally some of the > changes aren't replicating to or persisting by the standby database. > I am not sure what is the best way to detect this, but one idea could be to enable wal_consistency_checking [1]. This will at the very least can detect if the block is replicated correctly for the very first time. Also, if there is some corruption issue on standby, you might be able to detect. But the point to note is that enabling this option has overhead, so you need to be careful. [1] - https://www.postgresql.org/docs/devel/runtime-config-developer.html -- With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com
Remove one use of IDENT_USERNAME_MAX
It seems to me that using IDENT_USERNAME_MAX for peer authentication is
some kind of historical leftover and not really appropriate or useful,
so I propose the attached cleanup.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
From 1fed5a94b801471cb380e447b5f0b924b3819be6 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut
Date: Sat, 26 Oct 2019 08:48:28 +0200
Subject: [PATCH] Remove one use of IDENT_USERNAME_MAX
IDENT_USERNAME_MAX is the maximum length of the information returned
by an ident server, per RFC 1413. Using it as the buffer size in peer
authentication is inappropriate. It was done here because of the
historical relationship between peer and ident authentication. But
since it's also completely useless code-wise, remove it.
---
src/backend/libpq/auth.c | 12 +++-
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/src/backend/libpq/auth.c b/src/backend/libpq/auth.c
index 0cf65ba5de..b939e8b205 100644
--- a/src/backend/libpq/auth.c
+++ b/src/backend/libpq/auth.c
@@ -65,7 +65,7 @@ static intCheckSCRAMAuth(Port *port, char *shadow_pass,
char **logdetail);
* Ident authentication
*
*/
-/* Max size of username ident server can return */
+/* Max size of username ident server can return (per RFC 1413) */
#define IDENT_USERNAME_MAX 512
/* Standard TCP port number for Ident service. Assigned by IANA */
@@ -73,6 +73,11 @@ static int CheckSCRAMAuth(Port *port, char *shadow_pass,
char **logdetail);
static int ident_inet(hbaPort *port);
+
+/*
+ * Peer authentication
+ *
+ */
#ifdef HAVE_UNIX_SOCKETS
static int auth_peer(hbaPort *port);
#endif
@@ -1979,7 +1984,6 @@ ident_inet(hbaPort *port)
static int
auth_peer(hbaPort *port)
{
- charident_user[IDENT_USERNAME_MAX + 1];
uid_t uid;
gid_t gid;
struct passwd *pw;
@@ -2011,9 +2015,7 @@ auth_peer(hbaPort *port)
return STATUS_ERROR;
}
- strlcpy(ident_user, pw->pw_name, IDENT_USERNAME_MAX + 1);
-
- return check_usermap(port->hba->usermap, port->user_name, ident_user,
false);
+ return check_usermap(port->hba->usermap, port->user_name, pw->pw_name,
false);
}
#endif /* HAVE_UNIX_SOCKETS */
--
2.23.0
