Re: [HACKERS] [BUGS] BUG #14230: Wrong timeline returned by pg_stop_backup on a standby
On Thu, Jul 7, 2016 at 12:57 AM, Marco Nenciarini wrote: > After further analysis, the issue is that we retrieve the starttli from > the ControlFile structure, but it was using ThisTimeLineID when writing > the backup label. > > I've attached a very simple patch that fixes it. ThisTimeLineID is always set at 0 on purpose on a standby, so we cannot rely on it (well it is set temporarily when recycling old segments). At recovery when parsing the backup_label file there is no actual use of the start segment name, so that's only a cosmetic change. But surely it would be better to get that fixed, because that's useful for debugging. While looking at your patch, I thought that it would have been tempting to use GetXLogReplayRecPtr() to get the timeline ID when in recovery, but what we really want to know here is the timeline of the last REDO pointer, which is starttli, and that's more consistent with the fact that we use startpoint when writing the backup_label file. In short, +1 for this fix. I am adding that in the list of open items, adding Magnus in CC whose commit for non-exclusive backups is at the origin of this defect. -- Michael -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Documentation fixes for pg_visibility
On Tue, Jul 5, 2016 at 8:51 PM, Robert Haas wrote: > > So I'm a bit confused about what you are saying here. If the page is > marked all-frozen but actually isn't all-frozen, then we should clear > the all-frozen bit in the VM. > Agreed. > The easiest way to do that is to clear > both bits in the VM plus the page-level bit, as done here, because we > don't presently have a way of clearing just one of the visibility map > bits. > Okay, but due to that we are clearing the visibility information (all-visible) even though we should not clear it based on all-frozen. I don't know if there is much harm even if we do the way it is proposed in patch, but why not improve it if we can. > Now, since the heap_lock_tuple issue requires us to introduce a new > method to clear all-visible without clearing all-frozen, we could > possibly use that here too, once we have it. > makes sense. -- With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Reviewing freeze map code
On Thu, Jul 7, 2016 at 3:36 AM, Andres Freund wrote:
> On 2016-07-05 23:37:59 +0900, Masahiko Sawada wrote:
>
>> @@ -8694,6 +8761,23 @@ heap_xlog_lock(XLogReaderState *record)
>> }
>> HeapTupleHeaderSetXmax(htup, xlrec->locking_xid);
>> HeapTupleHeaderSetCmax(htup, FirstCommandId, false);
>> +
>> + /* The visibility map need to be cleared */
>> + if ((xlrec->infobits_set & XLHL_ALL_VISIBLE_CLEARED) != 0)
>> + {
>> + RelFileNode rnode;
>> + Buffer vmbuffer = InvalidBuffer;
>> + BlockNumber blkno;
>> + Relationreln;
>> +
>> + XLogRecGetBlockTag(record, 0, &rnode, NULL, &blkno);
>> + reln = CreateFakeRelcacheEntry(rnode);
>> +
>> + visibilitymap_pin(reln, blkno, &vmbuffer);
>> + visibilitymap_clear(reln, blkno, vmbuffer);
>> + PageClearAllVisible(page);
>> + }
>> +
>
>
>> PageSetLSN(page, lsn);
>> MarkBufferDirty(buffer);
>> }
>> diff --git a/src/include/access/heapam_xlog.h
>> b/src/include/access/heapam_xlog.h
>> index a822d0b..41b3c54 100644
>> --- a/src/include/access/heapam_xlog.h
>> +++ b/src/include/access/heapam_xlog.h
>> @@ -242,6 +242,7 @@ typedef struct xl_heap_cleanup_info
>> #define XLHL_XMAX_EXCL_LOCK 0x04
>> #define XLHL_XMAX_KEYSHR_LOCK0x08
>> #define XLHL_KEYS_UPDATED0x10
>> +#define XLHL_ALL_VISIBLE_CLEARED 0x20
>
> Hm. We can't easily do that in the back-patched version; because a
> standby won't know to check for the flag . That's kinda ok, since we
> don't yet need to clear all-visible yet at that point of
> heap_update. But that better means we don't do so on the master either.
>
To clarify, do you mean to say that lets have XLHL_ALL_FROZEN_CLEARED
and do that just for master. For back-branches no need to clear
visibility any flags?
--
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Hash Indexes
On Wed, Jun 22, 2016 at 8:44 PM, Robert Haas wrote: > On Wed, Jun 22, 2016 at 5:10 AM, Amit Kapila wrote: >>> > Insertion will happen by scanning the appropriate bucket and needs to >>> > retain pin on primary bucket to ensure that concurrent split doesn't >>> > happen, >>> > otherwise split might leave this tuple unaccounted. >>> >>> What do you mean by "unaccounted"? >> >> It means that split might leave this tuple in old bucket even if it can be >> moved to new bucket. Consider a case where insertion has to add a tuple on >> some intermediate overflow bucket in the bucket chain, if we allow split >> when insertion is in progress, split might not move this newly inserted >> tuple. > >>> I think this is basically correct, although I don't find it to be as >>> clear as I think it could be. It seems very clear that any operation >>> which potentially changes the order of tuples in the bucket chain, >>> such as the squeeze phase as currently implemented, also needs to >>> exclude all concurrent scans. However, I think that it's OK for >>> vacuum to remove tuples from a given page with only an exclusive lock >>> on that particular page. >> >> How can we guarantee that it doesn't remove a tuple that is required by scan >> which is started after split-in-progress flag is set? > > If the tuple is being removed by VACUUM, it is dead. We can remove > dead tuples right away, because no MVCC scan will see them. In fact, > the only snapshot that will see them is SnapshotAny, and there's no > problem with removing dead tuples while a SnapshotAny scan is in > progress. It's no different than heap_page_prune() removing tuples > that a SnapshotAny sequential scan was about to see. > While again thinking about this case, it seems to me that we need a cleanup lock even for dead tuple removal. The reason for the same is that scans that return multiple tuples always restart the scan from the previous offset number from which they have returned last tuple. Now, consider the case where the first tuple is returned from offset number-3 in page and after that another backend removes the corresponding tuple from heap and vacuum also removes the dead tuple corresponding to offset-3. When the scan will try to get the next tuple, it will start from offset-3 which can lead to incorrect results. Now, one way to solve above problem could be if we change scan for hash indexes such that it works page at a time like we do for btree scans (refer BTScanPosData and comments on top of it). This has an additional advantage that it will reduce lock/unlock calls for retrieving tuples from a page. However, I think this solution can only work for MVCC scans. For non-MVCC scans, still there is a problem, because after fetching all the tuples from a page, when it tries to check the validity of tuples in heap, we won't be able to detect if the old tuple is deleted and a new tuple has been placed at that location in heap. I think what we can do to solve this for non-MVCC scans is that allow vacuum to always take a cleanup lock on a bucket and MVCC-scans will release both the lock and pin as it proceeds. Non-MVCC scans and scans that are started during split-in-progress will release the lock, but not a pin on primary bucket. This way, we can allow vacuum to proceed even if there is a MVCC-scan going on a bucket if it is not started during a bucket split operation. For btree code, we do something similar, which means that vacuum always take cleanup lock on a bucket and non-MVCC scan retains a pin on the bucket. The insertions should work as they are currently in patch, that is they always need to retain a pin on primary bucket to avoid the concurrent split problem as mentioned above (refer the first paragraph discussion of this mail). Thoughts? -- With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] primary_conninfo missing from pg_stat_wal_receiver
On Thu, Jul 7, 2016 at 4:43 AM, Stephen Frost wrote: > All, > > * Alvaro Herrera (alvhe...@2ndquadrant.com) wrote: >> Michael Paquier wrote: >> > On Wed, Jul 6, 2016 at 7:34 PM, Fujii Masao wrote: >> > > I have one question; why do we call the column "conn_info" instead of >> > > "conninfo" which is basically used in other places? "conninfo" is better >> > > to me. >> > >> > No real reason for one or the other to be honest. If you want to >> > change it you could just apply the attached. >> >> I was of two minds myself, and found no reason to change conn_info, so I >> decided to keep what was submitted. If you want to change it, I'm not >> opposed. >> >> Don't forget to bump catversion. > > 'conninfo' certainly seems to be more commonly used and I believe is > what was agreed to up-thread. +1. So since no one objects to change the column name, I applied Michael's patch. Thanks! Regards, -- Fujii Masao -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Set sgml-basic-offset to 1 in .dir-locals.el
On 7/6/16 4:52 AM, Dagfinn Ilmari Mannsåker wrote: This keeps the indentation consistent when editing the documentation using Emacs. Unfortunately, sgml-basic-offset is not a "safe" variable, so if we put it into .dir-locals.el, then users will always be bothered with a warning about that. -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] can we optimize STACK_DEPTH_SLOP
Greg Stark writes:
> Ok, I managed to get __atribute__((destructor)) working and capitured
> the attached pmap output for all the revisions. You can see the git
> revision in the binary name along with a putative date though in the
> case of branches the date can be deceptive. It looks to me like REL8_4
> is already bloated by REL8_4_0~2268 (which means 2268 commits *before*
> the REL8_4_0 tag -- i.e. soon after it branched).
I traced through this by dint of inserting a lot of system("pmap") calls,
and what I found out is that it's the act of setting one of the timezone
variables that does it. This is because tzload() allocates a local
variable "union local_storage ls", which sounds harmless enough, but
in point of fact the darn thing is 78K! And to add insult to injury,
with my setting (US/Eastern) there is a recursive call to parse the
"posixrules" timezone file. So that's 150K worth of stack right
there, although possibly it's only half that for some zone settings.
(And if you use "GMT" you escape all of this, since that's hard coded.)
So now I understand why the IANA code has provisions for malloc'ing
that storage rather than just using the stack. We should do likewise.
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] dumping database privileges broken in 9.6
All, * Noah Misch (n...@leadboat.com) wrote: > On Wed, Jun 29, 2016 at 11:50:17AM -0400, Stephen Frost wrote: > > * Peter Eisentraut (peter.eisentr...@2ndquadrant.com) wrote: > > > Do this: > > > > > > CREATE DATABASE test1; > > > REVOKE CONNECT ON DATABASE test1 FROM PUBLIC; > > > > > > Run pg_dumpall. > > > > > > In 9.5, this produces > > > > > > CREATE DATABASE test1 WITH TEMPLATE = template0 OWNER = peter; > > > REVOKE ALL ON DATABASE test1 FROM PUBLIC; > > > REVOKE ALL ON DATABASE test1 FROM peter; > > > GRANT ALL ON DATABASE test1 TO peter; > > > GRANT TEMPORARY ON DATABASE test1 TO PUBLIC; > > > > > > In 9.6, this produces only > > > > > > CREATE DATABASE test1 WITH TEMPLATE = template0 OWNER = peter; > > > GRANT TEMPORARY ON DATABASE test1 TO PUBLIC; > > > GRANT ALL ON DATABASE test1 TO peter; > > > > > > Note that the REVOKE statements are missing. This does not > > > correctly recreate the original state. > > > > I see what happened here, the query in dumpCreateDB() needs to be > > adjusted to pull the default information to then pass to > > buildACLComments(), similar to how the objects handled by pg_dump work. > > The oversight was in thinking that databases didn't have any default > > rights granted, which clearly isn't correct. > > > > I'll take care of that in the next day or so and add an appropriate > > regression test. > > This PostgreSQL 9.6 open item is past due for your status update. Kindly send > a status update within 24 hours, and include a date for your subsequent status > update. Refer to the policy on open item ownership: > http://www.postgresql.org/message-id/20160527025039.ga447...@tornado.leadboat.com I've not forgotten about this and have an initial patch, but I'm considering if I like the way template0/template1 are handled. Specifically, we don't currently record their initdb-set privileges into pg_init_privs (unlike all other objects with initial privileges). This is complicated by the idea that template1 could be dropped/recreated (ending up with a different OID in the process). More to come tomorrow. Thanks! Stephen signature.asc Description: Digital signature
Re: [HACKERS] Password identifiers, protocol aging and SCRAM protocol
* Michael Paquier (michael.paqu...@gmail.com) wrote: > On Tue, Jul 5, 2016 at 5:50 PM, Magnus Hagander wrote: > > On Tue, Jul 5, 2016 at 10:06 AM, Michael Paquier > > wrote: > > However, is there something that's fundamentally better with the OpenSSL > > implementation? Or should we just keep *just* the #else branch in the code, > > the part we've imported from OpenBSD? > > Good question. I think that we want both, giving priority to OpenSSL > if it is there. Usually their things prove to have more entropy, but I > didn't look at their code to be honest. If we only use the OpenBSD > stuff, it would be a good idea to refresh the in-core code. This is > from OpenBSD of 2002. I agree that we definitely want to use the OpenSSL functions when they are available. > > I'm not sure how common a build without openssl is in the real world though. > > RPMs, DEBs, Windows installers etc all build with OpenSSL. But we probably > > don't want to make it mandatory, no... > > I don't think that it is this much common to have an enterprise-class > build of Postgres without SSL, but each company has always its own > reasons, so things could exist. I agree that it's useful to have the support if PG isn't built with OpenSSL for some reason. Thanks! Stephen signature.asc Description: Digital signature
[HACKERS] PARALLEL SAFE/UNSAFE question
Hi all, I was trying writing my own parallel aggregation functions on 9.6beta2. During that, we found a bit confusing behavior with SAFE/UNSAFE options. Once a PARALLEL UNSAFE function f1_unsafe() is wrapped by a PARALLEL SAFE function f1_safe_unsafe(), calling f1_safe_unsafe() produces a parallel execution plan despite it implicitly calls the UNSAFE FUNCTION f1_unsafe(). Is this intentional? Please take a look at our example here: https://gist.github.com/snaga/362a965683fb2581bc693991b1fcf721 According to the manual[1], it is described as: > the presence of such a function in an SQL statement forces a serial execution > plan. so, I expected that calling UNSAFE functions should produce a non-parallel execution plan even wrapped in SAFE functions. Is this a sort of limitations? Is this working correctly as we expected? Regards, [1] https://www.postgresql.org/docs/9.6/static/sql-createfunction.html -- Satoshi Nagayasu -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Reviewing freeze map code
On 2016-07-05 23:37:59 +0900, Masahiko Sawada wrote:
> diff --git a/src/backend/access/heap/heapam.c
> b/src/backend/access/heap/heapam.c
> index 57da57a..fd66527 100644
> --- a/src/backend/access/heap/heapam.c
> +++ b/src/backend/access/heap/heapam.c
> @@ -3923,6 +3923,17 @@ l2:
>
> if (need_toast || newtupsize > pagefree)
> {
> + /*
> + * To prevent data corruption due to updating old tuple by
> + * other backends after released buffer
That's not really the reason, is it? The prime problem is crash safety /
replication. The row-lock we're faking (by setting xmax to our xid),
prevents concurrent updates until this backend died.
> , we need to emit that
> + * xmax of old tuple is set and clear visibility map bits if
> + * needed before releasing buffer. We can reuse xl_heap_lock
> + * for this purpose. It should be fine even if we crash midway
> + * from this section and the actual updating one later, since
> + * the xmax will appear to come from an aborted xid.
> + */
> + START_CRIT_SECTION();
> +
> /* Clear obsolete visibility flags ... */
> oldtup.t_data->t_infomask &= ~(HEAP_XMAX_BITS | HEAP_MOVED);
> oldtup.t_data->t_infomask2 &= ~HEAP_KEYS_UPDATED;
> @@ -3936,6 +3947,46 @@ l2:
> /* temporarily make it look not-updated */
> oldtup.t_data->t_ctid = oldtup.t_self;
> already_marked = true;
> +
> + /* Clear PD_ALL_VISIBLE flags */
> + if (PageIsAllVisible(BufferGetPage(buffer)))
> + {
> + all_visible_cleared = true;
> + PageClearAllVisible(BufferGetPage(buffer));
> + visibilitymap_clear(relation,
> BufferGetBlockNumber(buffer),
> + vmbuffer);
> + }
> +
> + MarkBufferDirty(buffer);
> +
> + if (RelationNeedsWAL(relation))
> + {
> + xl_heap_lock xlrec;
> + XLogRecPtr recptr;
> +
> + /*
> + * For logical decoding we need combocids to properly
> decode the
> + * catalog.
> + */
> + if (RelationIsAccessibleInLogicalDecoding(relation))
> + log_heap_new_cid(relation, &oldtup);
Hm, I don't see that being necessary here. Row locks aren't logically
decoded, so there's no need to emit this here.
> + /* Clear PD_ALL_VISIBLE flags */
> + if (PageIsAllVisible(page))
> + {
> + Buffer vmbuffer = InvalidBuffer;
> + BlockNumber block = BufferGetBlockNumber(*buffer);
> +
> + all_visible_cleared = true;
> + PageClearAllVisible(page);
> + visibilitymap_pin(relation, block, &vmbuffer);
> + visibilitymap_clear(relation, block, vmbuffer);
> + }
> +
That clears all-visible unnecessarily, we only need to clear all-frozen.
> @@ -8694,6 +8761,23 @@ heap_xlog_lock(XLogReaderState *record)
> }
> HeapTupleHeaderSetXmax(htup, xlrec->locking_xid);
> HeapTupleHeaderSetCmax(htup, FirstCommandId, false);
> +
> + /* The visibility map need to be cleared */
> + if ((xlrec->infobits_set & XLHL_ALL_VISIBLE_CLEARED) != 0)
> + {
> + RelFileNode rnode;
> + Buffer vmbuffer = InvalidBuffer;
> + BlockNumber blkno;
> + Relationreln;
> +
> + XLogRecGetBlockTag(record, 0, &rnode, NULL, &blkno);
> + reln = CreateFakeRelcacheEntry(rnode);
> +
> + visibilitymap_pin(reln, blkno, &vmbuffer);
> + visibilitymap_clear(reln, blkno, vmbuffer);
> + PageClearAllVisible(page);
> + }
> +
> PageSetLSN(page, lsn);
> MarkBufferDirty(buffer);
> }
> diff --git a/src/include/access/heapam_xlog.h
> b/src/include/access/heapam_xlog.h
> index a822d0b..41b3c54 100644
> --- a/src/include/access/heapam_xlog.h
> +++ b/src/include/access/heapam_xlog.h
> @@ -242,6 +242,7 @@ typedef struct xl_heap_cleanup_info
> #define XLHL_XMAX_EXCL_LOCK 0x04
> #define XLHL_XMAX_KEYSHR_LOCK0x08
> #define XLHL_KEYS_UPDATED0x10
> +#define XLHL_ALL_VISIBLE_CLEARED 0x20
Hm. We can't easily do that in the back-patched version; because a
standby won't know to check for the flag . That's kinda ok, since we
don't yet need to clear all-visible yet at that point of
heap_update. But that better means we don't do so on the master either.
Greetings,
Andres Freund
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make
Re: [HACKERS] Re: [COMMITTERS] pgsql: Avoid extra locks in GetSnapshotData if old_snapshot_threshold
On 2016-07-02 14:20:13 -0500, Kevin Grittner wrote:
> The possible fixes considered were these:
>
> (1) Always vacuum the heap before its related TOAST table.
I think that's clearly not ok from a cost perspective.
> (2) Same as (1) but only when old_snapshot_threshold >= 0.
> (3) Allow the special snapshot used for TOAST access to generate
> the "snapshot too old" error, so that the modified page from the
> pruning/vacuuming (along with other conditions) would cause that
> rather than something suggesting corrupt internal structure.
> (4) When looking to read a toasted value for a tuple past the
> early pruning horizon, if the value was not found consider it a
> "snapshot too old" error.
Doesn't solve the issue that a toast id might end up being reused.
> (5) Don't vacuum or prune a TOAST table except as part of the heap
> vacuum when early pruning is enabled.
That's pretty costly.
> (6) Don't allow early vacuuming/pruning of TOAST values except as
> part of the vacuuming of the related heap.
> It became evident pretty quickly that the HOT pruning of TOAST
> values should not do early cleanup, based on practical concerns of
> coordinating that with the heap cleanup for any of the above
> options. What's more, since we don't allow updating of tuples
> holding TOAST values, HOT pruning seems to be of dubious value on a
> TOAST table in general -- but removing that would be the matter for
> a separate patch.
I'm not following here. Sure, there'll be no HOT chains, but hot pruning
also releases space (though not item pointers) for dead tuples. And
that's fairly valuable in high-churn tables?
> Anyway, this patch includes a small hunk of code
> (two lines) to avoid early HOT pruning for TOAST tables.
I see it's only prohibiting the old_snapshot_threshold triggered
cleanup, good.
> For the vacuuming, option (6) seems a clear winner, and that is
> what this patch implements. A TOAST table can still be vacuumed on
> its own, but in that case it will not use old_snapshot_threshold to
> try to do any early cleanup.
> We were already normally vacuuming
> the TOAST table whenever we vacuumed the related heap; in such a
> case it uses the "oldestXmin" used for the heap to vacuum the TOAST
> table.
That's not the case. Autovacuum schedules main and toast tables
independently. Check the two collection loops in do_autovacuum:
/*
* On the first pass, we collect main tables to vacuum, and also the
main
* table relid to TOAST relid mapping.
*/
while ((tuple = heap_getnext(relScan, ForwardScanDirection)) != NULL)
{
...
relation_needs_vacanalyze(relid, relopts, classForm, tabentry,
effective_multixact_freeze_max_age,
&dovacuum,
&doanalyze, &wraparound);
...
/* relations that need work are added to table_oids */
if (dovacuum || doanalyze)
table_oids = lappend_oid(table_oids, relid);
}
...
/* second pass: check TOAST tables */
while ((tuple = heap_getnext(relScan, ForwardScanDirection)) != NULL)
{
...
relation_needs_vacanalyze(relid, relopts, classForm, tabentry,
effective_multixact_freeze_max_age,
&dovacuum,
&doanalyze, &wraparound);
/* ignore analyze for toast tables */
if (dovacuum)
table_oids = lappend_oid(table_oids, relid);
}
So I don't think that approach still allows old snapshot related
cleanups for toast triggered vacuums? Is that an acceptable
restriction?
Greetings,
Andres Freund
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Question about an inconsistency - 1
"pet...@gmail.com" writes: > I have a question regarding the source code in file pg_proc.h > (postgresql-9.4.4). > At line 1224 (copied below) why the 28th identifier is timestamp_eq? > DATA(insert OID = 1152 ( timestamptz_eq PGNSP PGUID 12 1 0 0 0 f f f t t f > i 2 0 16 "1184 1184" _null_ _null_ _null_ _null_ timestamp_eq _null_ _null_ > _null_ )); > I would expect it to be timestamptz_eq (the same as the 5th identifier > from the same line). If timestamptz_eq actually existed as a separate C function, then yes that would be correct. But see this bit in timestamp.h: extern int timestamp_cmp_internal(Timestamp dt1, Timestamp dt2); /* timestamp comparison works for timestamptz also */ #define timestamptz_cmp_internal(dt1,dt2) timestamp_cmp_internal(dt1, dt2) AFAICS there are not similar #defines equating timestamptz_eq to timestamp_eq and so on, probably because we don't have much need to refer to those functions in the C code. But even if we had such #defines, I think that pg_proc.h could not rely on them. It has to reference actual C functions. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] primary_conninfo missing from pg_stat_wal_receiver
All, * Alvaro Herrera (alvhe...@2ndquadrant.com) wrote: > Michael Paquier wrote: > > On Wed, Jul 6, 2016 at 7:34 PM, Fujii Masao wrote: > > > I have one question; why do we call the column "conn_info" instead of > > > "conninfo" which is basically used in other places? "conninfo" is better > > > to me. > > > > No real reason for one or the other to be honest. If you want to > > change it you could just apply the attached. > > I was of two minds myself, and found no reason to change conn_info, so I > decided to keep what was submitted. If you want to change it, I'm not > opposed. > > Don't forget to bump catversion. 'conninfo' certainly seems to be more commonly used and I believe is what was agreed to up-thread. Thanks! Stephen signature.asc Description: Digital signature
Re: [HACKERS] can we optimize STACK_DEPTH_SLOP
On Wed, Jul 6, 2016 at 2:34 PM, Tom Lane wrote: > > Conclusion: something we did in 8.4 greatly bloated the postmaster's > stack space consumption, to the point that it's significantly more than > anything a normal backend does. That's surprising and scary, because > it means the postmaster is *more* exposed to stack SIGSEGV than most > backends. We need to find the cause, IMO. Hm. I do something based on your test where I build a .so and started the postmaster with -c shared_preload_libraries to load it. I tried to run it on every revision I have built for the historic benchmarks. That only worked as far back as 8.4.0 -- which makes me suspect it's possibly because of precisely shared_preload_libraries and the dynamic linker that the stack size grew The only thing it actually revealed was a *drop* of 50kB between REL9_2_0~1610 and REL9_2_0~1396. REL8_4_0~1702 188K REL8_4_0~1603 192K REL8_4_0~1498 188K REL8_4_0~1358 192K REL8_4_0~1218 184K REL8_4_0~1013 188K REL8_4_0~996 192K REL8_4_0~856 192K REL8_4_0~775 192K REL8_4_0~567 192K REL8_4_0~480 188K REL8_4_0~360 188K REL8_4_0~151 188K REL9_0_0~1855 188K REL9_0_0~1654 188K REL9_0_0~1538 192K REL9_0_0~1454 184K REL9_0_0~1351 184K REL9_0_0~1249 188K REL9_0_0~1107 184K REL9_0_0~938 184K REL9_0_0~627 184K REL9_0_0~414 184K REL9_0_0~202 184K REL9_1_0~1867 188K REL9_1_0~1695 184K REL9_1_0~1511 188K REL9_1_0~1328 192K REL9_1_0~978 192K REL9_1_0~948 188K REL9_1_0~628 188K REL9_1_0~382 192K REL9_2_0~1825 184K REL9_2_0~1610 192K <--- here REL9_2_0~1396 148K REL9_2_0~1226 148K REL9_2_0~1190 148K REL9_2_0~1072 140K REL9_2_0~1071 144K REL9_2_0~984 144K REL9_2_0~777 144K REL9_2_0~767 148K REL9_2_0~551 148K REL9_2_0~309 144K REL9_3_0~1509 148K REL9_3_0~1304 148K REL9_3_0~1099 144K REL9_3_0~1030 144K REL9_3_0~944 140K REL9_3_0~789 144K REL9_3_0~735 148K REL9_3_0~589 144K REL9_3_0~390 148K REL9_3_0~223 144K REL9_4_0~1923 148K REL9_4_0~1894 148K REL9_4_0~1755 144K REL9_4_0~1688 144K REL9_4_0~1617 144K REL9_4_0~1431 144K REL9_4_0~1246 144K REL9_4_0~1142 148K REL9_4_0~995 148K REL9_4_0~744 140K REL9_4_0~462 148K REL9_5_0~2370 148K REL8_4_22 192K REL9_5_0~2183 148K REL9_5_0~1996 148K REL9_5_0~1782 144K REL9_5_0~1569 148K REL9_5_0~1557 144K REL9_5_ALPHA1-20-g7b156c1 144K REL9_5_ALPHA1-299-g47ebbdc 144K REL9_5_ALPHA1-489-ge06b2e1 144K REL9_0_23 188K REL9_1_19 192K REL9_2_14 144K REL9_3_10 148K REL9_4_5 148K REL9_5_ALPHA1-683-ge073490 144K REL9_5_ALPHA1-844-gdfcd9cb 148K REL9_5_0 148K REL9_5_ALPHA1-972-g7dc09c1 144K REL9_5_ALPHA1-1114-g57a6a72 148K -- greg -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] primary_conninfo missing from pg_stat_wal_receiver
Michael Paquier wrote: > On Wed, Jul 6, 2016 at 7:34 PM, Fujii Masao wrote: > > I have one question; why do we call the column "conn_info" instead of > > "conninfo" which is basically used in other places? "conninfo" is better to > > me. > > No real reason for one or the other to be honest. If you want to > change it you could just apply the attached. I was of two minds myself, and found no reason to change conn_info, so I decided to keep what was submitted. If you want to change it, I'm not opposed. Don't forget to bump catversion. -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Re: GiST optimizing memmoves in gistplacetopage for fixed-size updates [PoC]
Here is a new patch. Updates: 1. Uses MAXALIGN to allocate space on page 2. Uses ItemIdSetNormal in case when ItemId was not normal for some reasons before call 3. Improved comments and var names Best regards, Andrey Borodin, Octonica & Ural Federal University. 2016-07-05 17:54 GMT+05:00 Andrew Borodin : > Here is a patch with updated WAL behavior. > > I'm not certain about MAXALIGN for size of appended tuple. Currently > if anyone calls PageIndexTupleOverwrite() with unalligned tuple it > will ereport(ERROR). > I suspect that it should allign size instead. > > Also I suspect that PageIndexTupleOverwrite() should make a call to > ItemIdSetNormal() to pretend it is generic function and not just a > private part of GiST. > > Best regards, Andrey Borodin, Octonica & Ural Federal University. > > 2016-07-04 22:59 GMT+05:00 Andrew Borodin : >> Thank you, Alvaro. >> >> Yes, now I see. I'll update gistRedoPageUpdateRecord() to work >> accordingly with patched gistplacetopage(). >> Also, i've noticed that PageAddItem uses MAXALIGN() macro to calculate >> tuple size. So, PageIndexTupleOverwrite should behave the same. >> >> About PageIndexDeleteNoCompact() in BRIN. I do not see why >> PageIndexDeleteNoCompact() is not a good solution instead of >> PageIndexTupleOverwrite? Will it mark tuple as unused until next >> PageAddItem will use it's place? >> >> Best regards, Andrey Borodin, Octonica & Ural Federal University. >> >> 2016-07-04 22:16 GMT+05:00 Alvaro Herrera : >>> Andrew Borodin wrote: Thank you, Amit. Currently, if WAL reconstruct page in another order it won't be a problem. >>> >>> We require that replay of WAL leads to identical bytes than the >>> original. Among other things, this enables use of a tool that verifies >>> that WAL is working correctly simply by comparing page images. So >>> please fix it instead of just verifying that this works for GiST. >>> >>> By the way, BRIN indexes have a need of this operation too. The current >>> approach is to call PageIndexDeleteNoCompact followed by PageAddItem. >>> I suppose it would be beneficial to use your new routine there too. >>> >>> -- >>> Álvaro Herrerahttp://www.2ndQuadrant.com/ >>> PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services PageIndexTupleOverwrite v4.patch Description: Binary data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Question about an inconsistency - 1
Hi, I have a question regarding the source code in file pg_proc.h (postgresql-9.4.4). At line 1224 (copied below) why the 28th identifier is timestamp_eq? DATA(insert OID = 1152 ( timestamptz_eq PGNSP PGUID 12 1 0 0 0 f f f t t f i 2 0 16 "1184 1184" _null_ _null_ _null_ _null_ timestamp_eq _null_ _null_ _null_ )); I would expect it to be timestamptz_eq (the same as the 5th identifier from the same line). Shouldn't it be the same like in the line 2940 copied also below having the same identifier in the 28th and 5th positions (timestamp_eq)? DATA(insert OID = 2052 ( timestamp_eq PGNSP PGUID 12 1 0 0 0 f f f t t f i 2 0 16 "1114 1114" _null_ _null_ _null_ _null_ timestamp_eq _null_ _null_ _null_ )); A similar question can be asked for lines 1225 to 1229 (but for their specific identifiers). Regards, Pepi signature.asc Description: Message signed with OpenPGP using GPGMail
[HACKERS] Re: [BUGS] BUG #14230: Wrong timeline returned by pg_stop_backup on a standby
On 06/07/16 17:41, Marco Nenciarini wrote:
> On 06/07/16 17:37, Marco Nenciarini wrote:
>> Hi,
>>
>> On 06/07/16 17:07, francesco.cano...@2ndquadrant.it wrote:
>>> The following bug has been logged on the website:
>>>
>>> Bug reference: 14230
>>> Logged by: Francesco Canovai
>>> Email address: francesco.cano...@2ndquadrant.it
>>> PostgreSQL version: 9.6beta2
>>> Operating system: Linux
>>> Description:
>>>
>>> I'm taking a concurrent backup from a standby in PostgreSQL beta2 and I get
>>> the wrong timeline from pg_stop_backup(false).
>>>
>>> This is what I'm doing:
>>>
>>> 1) I set up an environment with a primary server and a replica in streaming
>>> replication.
>>>
>>> 2) On the replica, I run
>>>
>>> postgres=# SELECT pg_start_backup('test_backup', true, false);
>>> pg_start_backup
>>> -
>>> 0/3000A00
>>> (1 row)
>>>
>>> 3) When I run pg_stop_backup, it returns a start wal location belonging to a
>>> file with timeline 0.
>>>
>>> postgres=# SELECT pg_stop_backup(false);
>>> pg_stop_backup
>>>
>>> ---
>>> (0/3000AE0,"START WAL LOCATION: 0/3000A00 (file
>>> 0003)+
>>> CHECKPOINT LOCATION: 0/3000A38
>>> +
>>> BACKUP METHOD: streamed
>>> +
>>> BACKUP FROM: standby
>>> +
>>> START TIME: 2016-07-06 16:44:31 CEST
>>> +
>>> LABEL: test_backup
>>> +
>>> ","")
>>> (1 row)
>>>
>>> The timeline returned is fine (is 1) when running the same commands on the
>>> master.
>>>
>>> An incorrect backup label doesn't prevent PostgreSQL from starting up, but
>>> it affects the tools using that information.
>>>
>>>
>>
>> The issue here is that the do_pg_stop_backup function uses the
>> ThisTimeLineID variable that is not valid on standbys.
>>
>> I think that it should read it from
>> ControlFile->checkPointCopy.ThisTimeLineID as we do in do_pg_start_backup.
>>
>
> No, that's not the solution.
>
> The backup_label is generated during the do_pg_start_backup call, so
> also the copy in ControlFile->checkPointCopy.ThisTimeLineID is
> uninitialized.
>
After further analysis, the issue is that we retrieve the starttli from
the ControlFile structure, but it was using ThisTimeLineID when writing
the backup label.
I've attached a very simple patch that fixes it.
Regards,
Marco
--
Marco Nenciarini - 2ndQuadrant Italy
PostgreSQL Training, Services and Support
marco.nenciar...@2ndquadrant.it | www.2ndQuadrant.it
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index e4645a3..aecede1 100644
*** a/src/backend/access/transam/xlog.c
--- b/src/backend/access/transam/xlog.c
*** do_pg_start_backup(const char *backupids
*** 9974,9980
} while (!gotUniqueStartpoint);
XLByteToSeg(startpoint, _logSegNo);
! XLogFileName(xlogfilename, ThisTimeLineID, _logSegNo);
/*
* Construct tablespace_map file
--- 9974,9980
} while (!gotUniqueStartpoint);
XLByteToSeg(startpoint, _logSegNo);
! XLogFileName(xlogfilename, starttli, _logSegNo);
/*
* Construct tablespace_map file
signature.asc
Description: OpenPGP digital signature
[HACKERS] Re: [BUGS] BUG #14230: Wrong timeline returned by pg_stop_backup on a standby
On 06/07/16 17:37, Marco Nenciarini wrote:
> Hi,
>
> On 06/07/16 17:07, francesco.cano...@2ndquadrant.it wrote:
>> The following bug has been logged on the website:
>>
>> Bug reference: 14230
>> Logged by: Francesco Canovai
>> Email address: francesco.cano...@2ndquadrant.it
>> PostgreSQL version: 9.6beta2
>> Operating system: Linux
>> Description:
>>
>> I'm taking a concurrent backup from a standby in PostgreSQL beta2 and I get
>> the wrong timeline from pg_stop_backup(false).
>>
>> This is what I'm doing:
>>
>> 1) I set up an environment with a primary server and a replica in streaming
>> replication.
>>
>> 2) On the replica, I run
>>
>> postgres=# SELECT pg_start_backup('test_backup', true, false);
>> pg_start_backup
>> -
>> 0/3000A00
>> (1 row)
>>
>> 3) When I run pg_stop_backup, it returns a start wal location belonging to a
>> file with timeline 0.
>>
>> postgres=# SELECT pg_stop_backup(false);
>> pg_stop_backup
>>
>> ---
>> (0/3000AE0,"START WAL LOCATION: 0/3000A00 (file
>> 0003)+
>> CHECKPOINT LOCATION: 0/3000A38
>> +
>> BACKUP METHOD: streamed
>> +
>> BACKUP FROM: standby
>> +
>> START TIME: 2016-07-06 16:44:31 CEST
>> +
>> LABEL: test_backup
>> +
>> ","")
>> (1 row)
>>
>> The timeline returned is fine (is 1) when running the same commands on the
>> master.
>>
>> An incorrect backup label doesn't prevent PostgreSQL from starting up, but
>> it affects the tools using that information.
>>
>>
>
> The issue here is that the do_pg_stop_backup function uses the
> ThisTimeLineID variable that is not valid on standbys.
>
> I think that it should read it from
> ControlFile->checkPointCopy.ThisTimeLineID as we do in do_pg_start_backup.
>
No, that's not the solution.
The backup_label is generated during the do_pg_start_backup call, so
also the copy in ControlFile->checkPointCopy.ThisTimeLineID is
uninitialized.
Regards,
Marco
--
Marco Nenciarini - 2ndQuadrant Italy
PostgreSQL Training, Services and Support
marco.nenciar...@2ndquadrant.it | www.2ndQuadrant.it
signature.asc
Description: OpenPGP digital signature
[HACKERS] Re: [BUGS] BUG #14230: Wrong timeline returned by pg_stop_backup on a standby
Hi,
On 06/07/16 17:07, francesco.cano...@2ndquadrant.it wrote:
> The following bug has been logged on the website:
>
> Bug reference: 14230
> Logged by: Francesco Canovai
> Email address: francesco.cano...@2ndquadrant.it
> PostgreSQL version: 9.6beta2
> Operating system: Linux
> Description:
>
> I'm taking a concurrent backup from a standby in PostgreSQL beta2 and I get
> the wrong timeline from pg_stop_backup(false).
>
> This is what I'm doing:
>
> 1) I set up an environment with a primary server and a replica in streaming
> replication.
>
> 2) On the replica, I run
>
> postgres=# SELECT pg_start_backup('test_backup', true, false);
> pg_start_backup
> -
> 0/3000A00
> (1 row)
>
> 3) When I run pg_stop_backup, it returns a start wal location belonging to a
> file with timeline 0.
>
> postgres=# SELECT pg_stop_backup(false);
> pg_stop_backup
>
> ---
> (0/3000AE0,"START WAL LOCATION: 0/3000A00 (file
> 0003)+
> CHECKPOINT LOCATION: 0/3000A38
> +
> BACKUP METHOD: streamed
> +
> BACKUP FROM: standby
> +
> START TIME: 2016-07-06 16:44:31 CEST
> +
> LABEL: test_backup
> +
> ","")
> (1 row)
>
> The timeline returned is fine (is 1) when running the same commands on the
> master.
>
> An incorrect backup label doesn't prevent PostgreSQL from starting up, but
> it affects the tools using that information.
>
>
The issue here is that the do_pg_stop_backup function uses the
ThisTimeLineID variable that is not valid on standbys.
I think that it should read it from
ControlFile->checkPointCopy.ThisTimeLineID as we do in do_pg_start_backup.
Regards,
Marco
--
Marco Nenciarini - 2ndQuadrant Italy
PostgreSQL Training, Services and Support
marco.nenciar...@2ndquadrant.it | www.2ndQuadrant.it
signature.asc
Description: OpenPGP digital signature
[HACKERS] Don't include MMAP DSM's transient files in basebackup
The files are not useful when restoring a backup and would be
automatically deleted on startup anyway. Also deleted an out-of-date
comment in dsm.c.
/ Oskari
>From f26f06049b5f89ca3140462d6816259268322d78 Mon Sep 17 00:00:00 2001
From: Oskari Saarenmaa
Date: Wed, 6 Jul 2016 16:35:39 +0300
Subject: [PATCH] Don't include MMAP DSM's transient files in basebackup
Also drop an out-of-date comment about AllocateDir usage in dsm.c.
---
src/backend/replication/basebackup.c | 6 +++---
src/backend/storage/ipc/dsm.c| 1 -
2 files changed, 3 insertions(+), 4 deletions(-)
diff --git a/src/backend/replication/basebackup.c b/src/backend/replication/basebackup.c
index da9b7a6..8867ad2 100644
--- a/src/backend/replication/basebackup.c
+++ b/src/backend/replication/basebackup.c
@@ -976,10 +976,10 @@ sendDir(char *path, int basepathlen, bool sizeonly, List *tablespaces,
}
/*
- * Skip pg_replslot, not useful to copy. But include it as an empty
- * directory anyway, so we get permissions right.
+ * Skip pg_replslot and pg_dynshmem, not useful to copy. But include
+ * them as empty directories anyway, so we get permissions right.
*/
- if (strcmp(de->d_name, "pg_replslot") == 0)
+ if (strcmp(de->d_name, "pg_replslot") == 0 || strcmp(de->d_name, "pg_dynshmem") == 0)
{
if (!sizeonly)
_tarWriteHeader(pathbuf + basepathlen + 1, NULL, &statbuf);
diff --git a/src/backend/storage/ipc/dsm.c b/src/backend/storage/ipc/dsm.c
index 47f2bea..57fecdb 100644
--- a/src/backend/storage/ipc/dsm.c
+++ b/src/backend/storage/ipc/dsm.c
@@ -293,7 +293,6 @@ dsm_cleanup_for_mmap(void)
DIR *dir;
struct dirent *dent;
- /* Open the directory; can't use AllocateDir in postmaster. */
if ((dir = AllocateDir(PG_DYNSHMEM_DIR)) == NULL)
ereport(ERROR,
(errcode_for_file_access(),
--
2.5.5
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] can we optimize STACK_DEPTH_SLOP
Greg Stark writes: > I did a similar(ish) test which is admittedly not as exhaustive as > using pmap. I instrumented check_stack_depth() itself to keep track of > a high water mark (and based on Robert's thought process) to keep > track of the largest increment over the previous checked stack depth. > This doesn't cover any cases where there's no check_stack_depth() call > in the call stack at all (but then if there's no check_stack_depth > call at all it's hard to see how any setting of STACK_DEPTH_SLOP is > necessarily going to help). Well, the point of STACK_DEPTH_SLOP is that we don't want to have to put check_stack_depth calls in every function in the backend, especially not otherwise-inexpensive leaf functions. So the idea is for the slop number to cover the worst-case call graph after the last function with a check. Your numbers are pretty interesting, in that they clearly prove we need a slop value of at least 40-50K, but they don't really show that that's adequate. I'm a bit disturbed by the fact that you seem to be showing maximum measured depth for the non-outlier tests as only around 40K-ish. That doesn't match up very well with my pmap results, since in no case did I see a physical stack size below 188K. [ pokes around for a little bit... ] Oh, this is interesting: it looks like the *postmaster*'s stack size is 188K, and of course every forked child is going to inherit that as a minimum stack depth. What's more, pmap shows stack sizes near that for all my running postmasters going back to 8.4. But 8.3 and before show a stack size of 84K, which seems to be some sort of minimum on this machine; even a trivial "cat" process has that stack size according to pmap. Conclusion: something we did in 8.4 greatly bloated the postmaster's stack space consumption, to the point that it's significantly more than anything a normal backend does. That's surprising and scary, because it means the postmaster is *more* exposed to stack SIGSEGV than most backends. We need to find the cause, IMO. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] EXISTS clauses not being optimized in the face of 'one time pass' optimizable expressions
On Fri, Jul 1, 2016 at 11:45 AM, Alvaro Herrera wrote: > Merlin Moncure wrote: > >> It's pretty easy to craft a query where you're on the winning side, >> but what's the worst case of doing two pass...is constant folding a >> non trivial fraction of planning time? > > One thing that has been suggested is to re-examine the plan after > planning is done, and if execution time is estimated to be large (FSVO), > then run a second planning pass with more expensive optimizations > enabled to try and find better plans. The guiding principle would be to > continue to very quickly find good enough plans for > frequent/small/simple queries, but spend more planning effort on more > complex ones where execution is likely to take much longer than planning > time. > > So doing constant-folding twice would be enabled for the second planning > pass. I like this idea. Maybe a GUC controlling the cost based cutoff (with 0 meaning, "assume the worst and plan the hard way first"). merlin -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] primary_conninfo missing from pg_stat_wal_receiver
On Wed, Jul 6, 2016 at 7:34 PM, Fujii Masao wrote:
> I have one question; why do we call the column "conn_info" instead of
> "conninfo" which is basically used in other places? "conninfo" is better to
> me.
No real reason for one or the other to be honest. If you want to
change it you could just apply the attached.
--
Michael
diff --git a/doc/src/sgml/monitoring.sgml b/doc/src/sgml/monitoring.sgml
index a8b8bb0..b620feb 100644
--- a/doc/src/sgml/monitoring.sgml
+++ b/doc/src/sgml/monitoring.sgml
@@ -1303,7 +1303,7 @@ SELECT pid, wait_event_type, wait_event FROM pg_stat_activity WHERE wait_event i
Replication slot name used by this WAL receiver
- conn_info
+ conninfo
text
Connection string used by this WAL receiver,
diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql
index f52de3a..4fc5d5a 100644
--- a/src/backend/catalog/system_views.sql
+++ b/src/backend/catalog/system_views.sql
@@ -682,7 +682,7 @@ CREATE VIEW pg_stat_wal_receiver AS
s.latest_end_lsn,
s.latest_end_time,
s.slot_name,
-s.conn_info
+s.conninfo
FROM pg_stat_get_wal_receiver() s
WHERE s.pid IS NOT NULL;
diff --git a/src/include/catalog/pg_proc.h b/src/include/catalog/pg_proc.h
index d92c05e..5d233e3 100644
--- a/src/include/catalog/pg_proc.h
+++ b/src/include/catalog/pg_proc.h
@@ -2746,7 +2746,7 @@ DATA(insert OID = 3318 ( pg_stat_get_progress_info PGNSP PGUID 12 1 100 0 0
DESCR("statistics: information about progress of backends running maintenance command");
DATA(insert OID = 3099 ( pg_stat_get_wal_senders PGNSP PGUID 12 1 10 0 0 f f f f f t s r 0 0 2249 "" "{23,25,3220,3220,3220,3220,23,25}" "{o,o,o,o,o,o,o,o}" "{pid,state,sent_location,write_location,flush_location,replay_location,sync_priority,sync_state}" _null_ _null_ pg_stat_get_wal_senders _null_ _null_ _null_ ));
DESCR("statistics: information about currently active replication");
-DATA(insert OID = 3317 ( pg_stat_get_wal_receiver PGNSP PGUID 12 1 0 0 0 f f f f f f s r 0 0 2249 "" "{23,25,3220,23,3220,23,1184,1184,3220,1184,25,25}" "{o,o,o,o,o,o,o,o,o,o,o,o}" "{pid,status,receive_start_lsn,receive_start_tli,received_lsn,received_tli,last_msg_send_time,last_msg_receipt_time,latest_end_lsn,latest_end_time,slot_name,conn_info}" _null_ _null_ pg_stat_get_wal_receiver _null_ _null_ _null_ ));
+DATA(insert OID = 3317 ( pg_stat_get_wal_receiver PGNSP PGUID 12 1 0 0 0 f f f f f f s r 0 0 2249 "" "{23,25,3220,23,3220,23,1184,1184,3220,1184,25,25}" "{o,o,o,o,o,o,o,o,o,o,o,o}" "{pid,status,receive_start_lsn,receive_start_tli,received_lsn,received_tli,last_msg_send_time,last_msg_receipt_time,latest_end_lsn,latest_end_time,slot_name,conninfo}" _null_ _null_ pg_stat_get_wal_receiver _null_ _null_ _null_ ));
DESCR("statistics: information about WAL receiver");
DATA(insert OID = 2026 ( pg_backend_pidPGNSP PGUID 12 1 0 0 0 f f f f t f s r 0 0 23 "" _null_ _null_ _null_ _null_ _null_ pg_backend_pid _null_ _null_ _null_ ));
DESCR("statistics: current backend PID");
diff --git a/src/test/regress/expected/rules.out b/src/test/regress/expected/rules.out
index 125c31b..ad44ae2 100644
--- a/src/test/regress/expected/rules.out
+++ b/src/test/regress/expected/rules.out
@@ -1893,8 +1893,8 @@ pg_stat_wal_receiver| SELECT s.pid,
s.latest_end_lsn,
s.latest_end_time,
s.slot_name,
-s.conn_info
- FROM pg_stat_get_wal_receiver() s(pid, status, receive_start_lsn, receive_start_tli, received_lsn, received_tli, last_msg_send_time, last_msg_receipt_time, latest_end_lsn, latest_end_time, slot_name, conn_info)
+s.conninfo
+ FROM pg_stat_get_wal_receiver() s(pid, status, receive_start_lsn, receive_start_tli, received_lsn, received_tli, last_msg_send_time, last_msg_receipt_time, latest_end_lsn, latest_end_time, slot_name, conninfo)
WHERE (s.pid IS NOT NULL);
pg_stat_xact_all_tables| SELECT c.oid AS relid,
n.nspname AS schemaname,
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Forthcoming SQL standards about JSON and Multi-Dimensional Arrays (FYI)
Thomas 2016-07-04 6:44 GMT+02:00 Thomas Munro : > ... But ISO/IEC CD 9075-15 > (Multi-Dimensional Arrays) is in stage 30.92 "CD referred back to > Working Group". Is that how they say "returned with feedback"? > ISO/IEC PDTR 19075-5 (Row Pattern Recognition) has also reached stage > 30.60. Does anyone know what that one is about? Maybe something like Peter surely would know: https://www.jacobs-university.de/directory/pbaumann :Stefan 2016-07-04 6:44 GMT+02:00 Thomas Munro : > On Wed, Jun 29, 2016 at 11:51 AM, Stefan Keller wrote: >> Hi, >> >> FYI: I'd just like to point you to following two forthcoming standard >> parts from "ISO/IEC JTS 1/SC 32" comittee: one on JSON, and one on >> "Multi-Dimensional Arrays" (SQL/MDA). >> >> They define there some things different as already in PG. See also >> Peter Baumann's slides [1] and e.g. [2] >> >> :Stefan >> >> [1] >> https://www.unibw.de/inf4/professors/geoinformatics/agile-2016-workshop-gis-with-nosql >> [2] >> http://jtc1sc32.org/doc/N2501-2550/32N2528-WG3-Tutorial-Opening-Plenary.pdf > > Thanks for these pointers. On the "standards under development" > page[1], I see that ISO/IEC PDTR 19075-6 (SQL/JSON) is at stage 30.60 > "Close of voting/ comment period". But ISO/IEC CD 9075-15 > (Multi-Dimensional Arrays) is in stage 30.92 "CD referred back to > Working Group". Is that how they say "returned with feedback"? > ISO/IEC PDTR 19075-5 (Row Pattern Recognition) has also reached stage > 30.60. Does anyone know what that one is about? Maybe something like > MATCH_RECOGNIZE in Oracle? > > [1] > http://www.iso.org/iso/home/store/catalogue_tc/catalogue_tc_browse.htm?commid=45342&development=on > > -- > Thomas Munro > http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] primary_conninfo missing from pg_stat_wal_receiver
On Mon, Jul 4, 2016 at 12:40 PM, Michael Paquier wrote: > On Sat, Jul 2, 2016 at 2:56 AM, Alvaro Herrera > wrote: >> Michael Paquier wrote: >>> On Fri, Jul 1, 2016 at 8:50 AM, Michael Paquier >>> wrote: >> >>> >> Okay, that argument I buy. >>> >> >>> >> I suppose this function/view should report no row at all if there is no >>> >> wal receiver connected, rather than a view with nulls. >>> > >>> > The function returns PG_RETURN_NULL() so as we don't have to use a >>> > SRF, and the view checks for IS NOT NULL, so there would be no rows >>> > popping up. >>> >>> In short, I would just go with the attached and call it a day. >> >> Done, thanks. Thanks! I have one question; why do we call the column "conn_info" instead of "conninfo" which is basically used in other places? "conninfo" is better to me. Regards, -- Fujii Masao -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Fix typo in jsonb.c
On Wed, Jul 6, 2016 at 10:27 AM, Masahiko Sawada wrote: > Hi all, > > Attached patch fixes the typo in jsonb.c > Please find it. Pushed. Thanks! Regards, -- Fujii Masao -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] minor plpgsql doc patch
Hello pgdevs, The very minor patch attached improves the PL/pgSQL documentation about trigger functions. It moves the description common to both data change & database event triggers out of the first section and into a common header. It adds a link at the beginning of the sections to their corresponding generic chapters. -- Fabien.diff --git a/doc/src/sgml/plpgsql.sgml b/doc/src/sgml/plpgsql.sgml index 7f23c2f..5d4080f 100644 --- a/doc/src/sgml/plpgsql.sgml +++ b/doc/src/sgml/plpgsql.sgml @@ -3660,18 +3660,26 @@ ASSERT condition , in PL/pgSQL + + PL/pgSQL can be used to define trigger + procedures on data changes or database events. + A trigger procedure is created with the CREATE FUNCTION command, + declaring it as a function with no arguments and a return type of + trigger (for data change triggers) or + event_trigger (for database event triggers). + Special variables PG_something are automatically + available to test the condition which triggered the call. + + Triggers on Data Changes - -PL/pgSQL can be used to define trigger -procedures. A trigger procedure is created with the -CREATE FUNCTION command, declaring it as a function with -no arguments and a return type of trigger. Note that -the function must be declared with no arguments even if it expects -to receive arguments specified in CREATE TRIGGER — -trigger arguments are passed via TG_ARGV, as described -below. + + A data change trigger is declared as a function + with no arguments and a return type of trigger. + Note that the function must be declared with no arguments even if it expects + to receive arguments specified in CREATE TRIGGER — + trigger arguments are passed via TG_ARGV, as described below. @@ -4218,8 +4226,9 @@ SELECT * FROM sales_summary_bytime; Triggers on Events -PL/pgSQL can be used to define event -triggers. PostgreSQL requires that a procedure that +PL/pgSQL can be used to define +event triggers. +PostgreSQL requires that a procedure that is to be called as an event trigger must be declared as a function with no arguments and a return type of event_trigger. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] [PATCH] Set sgml-basic-offset to 1 in .dir-locals.el
This keeps the indentation consistent when editing the documentation using Emacs. >From c345671ae4704df500dd17719c5e9973001663c1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dagfinn=20Ilmari=20Manns=C3=A5ker?= Date: Sat, 26 Mar 2016 21:58:32 + Subject: [PATCH] Set sgml-basic-offset to 1 in .dir-locals.el --- .dir-locals.el | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.dir-locals.el b/.dir-locals.el index d8827a6..9574300 100644 --- a/.dir-locals.el +++ b/.dir-locals.el @@ -16,4 +16,5 @@ (indent-tabs-mode . t) (tab-width . 4))) (sgml-mode . ((fill-column . 78) - (indent-tabs-mode . nil + (indent-tabs-mode . nil) + (sgml-basic-offset . 1 -- 2.8.1 -- - Twitter seems more influential [than blogs] in the 'gets reported in the mainstream press' sense at least. - Matt McLeod - That'd be because the content of a tweet is easier to condense down to a mainstream media article. - Calle Dybedahl -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Password identifiers, protocol aging and SCRAM protocol
On Wed, Jul 6, 2016 at 4:18 PM, Michael Paquier
wrote:
> OK, after hacking that for a bit I have finished with option 2 and the
> set of PG-like set of routines, the use of USE_SSL in the file
> containing all the SHA functions of OpenBSD has proved to be really
> ugly, but with a split things are really clear to the eye. The stuff I
> got builds on OSX, Linux and MSVC. pgcrypto cannot link directly to
> libpgcommon.a, so I am making it compile directly with the source
> files, as it is doing on HEAD.
Btw, attached is the patch I did for this part if there is any interest in it.
Also, while working on the rest, I am not adding a new column to
pg_auth_id to identify the password verifier type. That's just to keep
the patch at a bare minimum size. Are there issues with that?
--
Michael
From 6101ffb3baf12cbb13a20812b1d8d10350683ff7 Mon Sep 17 00:00:00 2001
From: Michael Paquier
Date: Wed, 6 Jul 2016 16:09:31 +0900
Subject: [PATCH 1/3] Refactor SHA functions and move them to src/common/
This way both frontend and backends can refer to them if needed. Those
functions are taken from pgcrypto, which now fetches directly the source
files it needs from src/common/ when compiling its library.
A new interface, which is more PG-like is designed for those SHA functions,
allowing to link to either OpenSSL or the in-core stuff taken from OpenBSD
as need be, which is the most flexible solution.
---
contrib/pgcrypto/.gitignore |4 +
contrib/pgcrypto/Makefile | 11 +-
contrib/pgcrypto/fortuna.c | 12 +-
contrib/pgcrypto/internal-sha2.c| 82 +-
contrib/pgcrypto/internal.c | 32 +-
contrib/pgcrypto/sha1.c | 341 -
contrib/pgcrypto/sha1.h | 75 --
contrib/pgcrypto/sha2.h | 100 ---
src/common/Makefile |6 +
contrib/pgcrypto/sha2.c => src/common/sha.c | 1101 +--
src/common/sha_openssl.c| 120 +++
src/include/common/sha.h| 95 +++
src/tools/msvc/Mkvcbuild.pm | 11 +-
13 files changed, 1012 insertions(+), 978 deletions(-)
delete mode 100644 contrib/pgcrypto/sha1.c
delete mode 100644 contrib/pgcrypto/sha1.h
delete mode 100644 contrib/pgcrypto/sha2.h
rename contrib/pgcrypto/sha2.c => src/common/sha.c (54%)
create mode 100644 src/common/sha_openssl.c
create mode 100644 src/include/common/sha.h
diff --git a/contrib/pgcrypto/.gitignore b/contrib/pgcrypto/.gitignore
index 5dcb3ff..582110e 100644
--- a/contrib/pgcrypto/.gitignore
+++ b/contrib/pgcrypto/.gitignore
@@ -1,3 +1,7 @@
+# Source file copied from src/common
+/sha.c
+/sha_openssl.c
+
# Generated subdirectories
/log/
/results/
diff --git a/contrib/pgcrypto/Makefile b/contrib/pgcrypto/Makefile
index 805db76..492184d 100644
--- a/contrib/pgcrypto/Makefile
+++ b/contrib/pgcrypto/Makefile
@@ -1,6 +1,6 @@
# contrib/pgcrypto/Makefile
-INT_SRCS = md5.c sha1.c sha2.c internal.c internal-sha2.c blf.c rijndael.c \
+INT_SRCS = md5.c internal.c internal-sha2.c blf.c rijndael.c \
fortuna.c random.c pgp-mpi-internal.c imath.c
INT_TESTS = sha2
@@ -22,6 +22,12 @@ SRCS = pgcrypto.c px.c px-hmac.c px-crypt.c \
pgp-pubdec.c pgp-pubenc.c pgp-pubkey.c pgp-s2k.c \
pgp-pgsql.c
+ifeq ($(with_openssl),yes)
+SRCS += sha_openssl.c
+else
+SRCS += sha.c
+endif
+
MODULE_big = pgcrypto
OBJS = $(SRCS:.c=.o) $(WIN32RES)
@@ -59,6 +65,9 @@ SHLIB_LINK += $(filter -leay32, $(LIBS))
SHLIB_LINK += -lws2_32
endif
+sha.c sha_openssl.c: % : $(top_srcdir)/src/common/%
+ rm -f $@ && $(LN_S) $< .
+
rijndael.o: rijndael.tbl
rijndael.tbl:
diff --git a/contrib/pgcrypto/fortuna.c b/contrib/pgcrypto/fortuna.c
index 5028203..6bc6faf 100644
--- a/contrib/pgcrypto/fortuna.c
+++ b/contrib/pgcrypto/fortuna.c
@@ -34,9 +34,9 @@
#include
#include
+#include "common/sha.h"
#include "px.h"
#include "rijndael.h"
-#include "sha2.h"
#include "fortuna.h"
@@ -112,7 +112,7 @@
#define CIPH_BLOCK 16
/* for internal wrappers */
-#define MD_CTX SHA256_CTX
+#define MD_CTX pg_sha256_ctx
#define CIPH_CTX rijndael_ctx
struct fortuna_state
@@ -154,22 +154,22 @@ ciph_encrypt(CIPH_CTX * ctx, const uint8 *in, uint8 *out)
static void
md_init(MD_CTX * ctx)
{
- SHA256_Init(ctx);
+ pg_sha256_init(ctx);
}
static void
md_update(MD_CTX * ctx, const uint8 *data, int len)
{
- SHA256_Update(ctx, data, len);
+ pg_sha256_update(ctx, data, len);
}
static void
md_result(MD_CTX * ctx, uint8 *dst)
{
- SHA256_CTX tmp;
+ pg_sha256_ctx tmp;
memcpy(&tmp, ctx, sizeof(*ctx));
- SHA256_Final(dst, &tmp);
+ pg_sha256_final(&tmp, dst);
px_memset(&tmp, 0, sizeof(tmp));
}
diff --git a/contrib/pgcrypto/internal-sha2.c b/contrib/pgcrypto/internal-sha2.c
index 55ec7e1..3868fd2 100644
--- a/contrib/pgcrypto/internal-sha2.c
+++ b/contrib/pgcrypto/internal-sha2.c
@@ -33,8 +33,8 @@
#include
+#include "common/sha.h"
Re: [HACKERS] Password identifiers, protocol aging and SCRAM protocol
On Tue, Jul 5, 2016 at 5:50 PM, Magnus Hagander wrote: > On Tue, Jul 5, 2016 at 10:06 AM, Michael Paquier > wrote: > However, is there something that's fundamentally better with the OpenSSL > implementation? Or should we just keep *just* the #else branch in the code, > the part we've imported from OpenBSD? Good question. I think that we want both, giving priority to OpenSSL if it is there. Usually their things prove to have more entropy, but I didn't look at their code to be honest. If we only use the OpenBSD stuff, it would be a good idea to refresh the in-core code. This is from OpenBSD of 2002. > TLS is complex, we don't want to do that in that case. But just the sha > functions isn't *that* complex, is it? No, they are not. >> Another possibility is that we could say that SCRAM is designed to >> work with TLS, as mentioned a bit upthread via the RFC, so we would >> not support it in builds compiled without OpenSSL. I think that would >> be a shame, but it would simplify all this refactoring juggling. >> >> So, 3 possibilities here: >> 1) Use a single file src/common/sha.c that includes a set of functions >> using USE_SSL >> 2) Have two files in src/common, one when build is used with OpenSSL, >> and the second one when built-in methods are used >> 3) Disable the use of SCRAM when OpenSSL is not present in the build. >> >> Opinions? My heart goes for 2) because 1) is ugly, and 3) is not >> appealing in terms of flexibility. > > I really dislike #3 - we want everybody to start using this... OK, after hacking that for a bit I have finished with option 2 and the set of PG-like set of routines, the use of USE_SSL in the file containing all the SHA functions of OpenBSD has proved to be really ugly, but with a split things are really clear to the eye. The stuff I got builds on OSX, Linux and MSVC. pgcrypto cannot link directly to libpgcommon.a, so I am making it compile directly with the source files, as it is doing on HEAD. > I'm not sure how common a build without openssl is in the real world though. > RPMs, DEBs, Windows installers etc all build with OpenSSL. But we probably > don't want to make it mandatory, no... I don't think that it is this much common to have an enterprise-class build of Postgres without SSL, but each company has always its own reasons, so things could exist. And I continue to move on... Thanks for the feedback. -- Michael -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
