Re: pgsql: Include permissive/enforcing state in sepgsql log messages.

[Simon Riggs]

On Wed, 12 Jan 2022 at 19:23, Tom Lane  wrote:
>
> Include permissive/enforcing state in sepgsql log messages.
>
> SELinux itself does this (at least in modern releases), and it
> seems like a good idea to reduce confusion.
>
> Dave Page
>
> Discussion: 
> https://postgr.es/m/CA+OCxowsQoLEYc=jN7OtNvOdX0Jg5L7nMYt++=k0x78hgq-...@mail.gmail.com
>
> Branch
> --
> master
>
> Details
> ---
> https://git.postgresql.org/pg/commitdiff/134d9746364425e437a6d8eb1e2de0f3c59bfd2b


Is that a bug fix to be backpatched, or a new/changed feature to be documented?

-- 
Simon Riggshttp://www.EnterpriseDB.com/

Re: pgsql: Allow numeric scale to be negative or greater than precision.

[Simon Riggs]

On Tue, 27 Jul 2021 at 17:22, Tom Lane  wrote:
>
> Simon Riggs  writes:
> > This is for migration?
>
> Yeah, compatibility with some other DBMSes that allow this.
>
> > Or does it mean very small values now occupy less space since we store
> > fewer zeros?
>
> It won't affect space consumption at all, as NUMERIC has never stored
> leading or trailing zeroes.  (Modulo the fact that "zero" here means
> a base-NDIGIT digit.)

Thanks, just trying to understand what that was for.

-- 
Simon Riggshttp://www.EnterpriseDB.com/

Re: pgsql: Allow numeric scale to be negative or greater than precision.

[Simon Riggs]

This is for migration?

Or does it mean very small values now occupy less space since we store
fewer zeros?

Thanks

On Mon, 26 Jul 2021 at 14:17, Dean Rasheed  wrote:
>
> Allow numeric scale to be negative or greater than precision.
>
> Formerly, when specifying NUMERIC(precision, scale), the scale had to
> be in the range [0, precision], which was per SQL spec. This commit
> extends the range of allowed scales to [-1000, 1000], independent of
> the precision (whose valid range remains [1, 1000]).
>
> A negative scale implies rounding before the decimal point. For
> example, a column might be declared with a scale of -3 to round values
> to the nearest thousand. Note that the display scale remains
> non-negative, so in this case the display scale will be zero, and all
> digits before the decimal point will be displayed.
>
> A scale greater than the precision supports fractional values with
> zeros immediately after the decimal point.
>
> Take the opportunity to tidy up the code that packs, unpacks and
> validates the contents of a typmod integer, encapsulating it in a
> small set of new inline functions.
>
> Bump the catversion because the allowed contents of atttypmod have
> changed for numeric columns. This isn't a change that requires a
> re-initdb, but negative scale values in the typmod would confuse old
> backends.
>
> Dean Rasheed, with additional improvements by Tom Lane. Reviewed by
> Tom Lane.
>
> Discussion: 
> https://postgr.es/m/caezatcwdnlgpkihmurf8nfofp0rftakj7kty6gczopnmfuo...@mail.gmail.com
>
> Branch
> --
> master
>
> Details
> ---
> https://git.postgresql.org/pg/commitdiff/085f931f52494e1f304e35571924efa6fcdc2b44
>
> Modified Files
> --
> doc/src/sgml/datatype.sgml |  48 ++-
> src/backend/utils/adt/numeric.c| 130 +
> src/include/catalog/catversion.h   |   2 +-
> src/include/utils/numeric.h|  16 +++-
> src/test/regress/expected/numeric.out  |  63 ++
> src/test/regress/expected/sanity_check.out |   1 +
> src/test/regress/sql/numeric.sql   |  34 
> 7 files changed, 251 insertions(+), 43 deletions(-)
>


-- 
Simon Riggshttp://www.EnterpriseDB.com/

Re: pgsql: Document a few caveats in synchronous logical replication.

[Simon Riggs]

On Thu, Jun 17, 2021 at 7:25 AM Amit Kapila  wrote:

> In a synchronous logical setup, locking [user] catalog tables can cause
> deadlock. This is because logical decoding of transactions can lock
> catalog tables to access them so exclusively locking those in transactions
> can lead to deadlock. To avoid this users must refrain from having
> exclusive locks on catalog tables.

If LOCK and TRUNCATE is advised against on all user catalog tables,
why would CLUSTER only apply to pg_class? Surely its locking level is
the same as LOCK?

The use of "[user]" isn't fully explained, so it might not be clear
that this applies to both Postgres catalog tables and any user tables
that have been nominated as catalogs. Probably worth linking to the
"Capabilities" section to explain.

It would be worth coalescing the following sections into a single
page, since they are just a few lines each:
Streaming Replication Protocol Interface
Logical Decoding SQL Interface
System Catalogs Related to Logical Decoding

> Discussion: 
> https://www.postgresql.org/message-id/2021022847.tpnb6eg3yiykzpky%40alap3.anarazel.de

Unfortunately this is a URL linking to the top of a huge discussion,
so isn't useful in locating the actual discussion.

-- 
Simon Riggshttp://www.EnterpriseDB.com/

Re: pgsql: Allow insert and update tuple routing and COPY for foreign table

[Simon Riggs]

On Wed, 24 Apr 2019 at 12:55, Etsuro Fujita 
wrote:


> > My point is that this should not be necessary.
>
> In my opinion, I think this is necessary...
>

Could we decide by looking at what FDWs are known to exist?  I hope we are
trying to avoid breakage in the smallest number of FDWs.

-- 
Simon Riggshttp://www.2ndQuadrant.com/
<http://www.2ndquadrant.com/>
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Re: pgsql: Compute XID horizon for page level index vacuum on primary.

[Simon Riggs]

On Fri, 29 Mar 2019 at 16:32, Andres Freund  wrote:

> On 2019-03-29 16:20:54 +0000, Simon Riggs wrote:
> > On Fri, 29 Mar 2019 at 16:12, Andres Freund  wrote:
> >
> >
> > > On 2019-03-29 15:58:14 +, Simon Riggs wrote:
> > > > On Fri, 29 Mar 2019 at 15:29, Andres Freund 
> wrote:
> > > > > That's far from a trivial feature imo. It seems quite possible that
> > > we'd
> > > > > end up with increased overhead, because the current logic can get
> away
> > > > > with only doing hint bit style writes - but would that be true if
> we
> > > > > started actually replacing the item pointers? Because I don't see
> any
> > > > > guarantee they couldn't cross a page boundary etc? So I think we'd
> need
> > > > > to do WAL logging during index searches, which seems prohibitively
> > > > > expensive.
> > > > >
> > > >
> > > > Don't see that.
> > > >
> > > > I was talking about reusing the first 4 bytes of an index tuple's
> > > > ItemPointerData,
> > > > which is the first field of an index tuple. Index tuples are
> MAXALIGNed,
> > > so
> > > > I can't see how that would ever cross a page boundary.
> > >
> > > They're 8 bytes, and MAXALIGN often is 4 bytes:
> > >
> >
> > xids are 4 bytes, so we're good.
>
> I literally went on to explain why that's not sufficient? You can't
> *just* replace the block number with an xid. You *also* need to set a
> flag denoting that it's an xid and dead now. Which can't fit in the same
> 4 bytes.  You either have to set it in the IndexTuple's t_info, or or in
> the ItemIdData's lp_flags. And both can be on a different sectors.  If
> the flag persists, and the xid doesn't you're going to interpret a block
> number as an xid - not great; but even worse, if the xid survives, but
> the flag doesn't, you're going to access the xid as a block - definitely
> not ok, because you're going to return wrong results.
>

Yes, I agree, I was thinking the same thing after my last comment, but was
afk. The idea requires the atomic update of at least 4 bytes plus at least
1 bit and so would require at least 8byte MAXALIGN to be useful. Your other
points suggesting that multiple things all need to be set accurately for
this to work aren't correct. The idea was that we would write a hint that
would avoid later I/O, so the overall idea is still viable.

Anyway, thinking some more, I think the whole idea of generating
lastRemovedXid is moot and there are better ways in the future of doing
this that would avoid a performance issue altogether. Clearly not PG12.

The main issue relates to the potential overhead of moving this to the
master. I agree its the right thing to do, but we should have some way of
checking it is not a performance issue in practice.

-- 
Simon Riggshttp://www.2ndQuadrant.com/
<http://www.2ndquadrant.com/>
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Re: pgsql: Compute XID horizon for page level index vacuum on primary.

[Simon Riggs]

On Fri, 29 Mar 2019 at 16:12, Andres Freund  wrote:


> On 2019-03-29 15:58:14 +0000, Simon Riggs wrote:
> > On Fri, 29 Mar 2019 at 15:29, Andres Freund  wrote:
> > > That's far from a trivial feature imo. It seems quite possible that
> we'd
> > > end up with increased overhead, because the current logic can get away
> > > with only doing hint bit style writes - but would that be true if we
> > > started actually replacing the item pointers? Because I don't see any
> > > guarantee they couldn't cross a page boundary etc? So I think we'd need
> > > to do WAL logging during index searches, which seems prohibitively
> > > expensive.
> > >
> >
> > Don't see that.
> >
> > I was talking about reusing the first 4 bytes of an index tuple's
> > ItemPointerData,
> > which is the first field of an index tuple. Index tuples are MAXALIGNed,
> so
> > I can't see how that would ever cross a page boundary.
>
> They're 8 bytes, and MAXALIGN often is 4 bytes:
>

xids are 4 bytes, so we're good.

If MAXALIGN could ever be 2 bytes, we'd have a problem.

So as a whole they definitely can cross sector boundaries. You might be
> able to argue your way out of that by saying that the blkid is going to
> be aligned, but that's not that trivial, as t_info isn't guaranteed
> that.
>
> But even so, you can't have unlogged changes that you then rely on. Even
> if there's no torn page issue. Currently BTP_HAS_GARBAGE and
> ItemIdMarkDead() are treated as hints - if we want to guarantee all
> these are accurate, I don't quite see how we'd get around WAL logging
> those.
>

You can have unlogged changes that you rely on - that is exactly how hints
work.

If the hint is lost, we do the I/O. Worst case it would be the same as what
you have now.

I'm talking about saving many I/Os - this doesn't need to provably avoid
all I/Os to work, its incremental benefit all the way.

-- 
Simon Riggshttp://www.2ndQuadrant.com/
<http://www.2ndquadrant.com/>
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Re: pgsql: Compute XID horizon for page level index vacuum on primary.

[Simon Riggs]

On Fri, 29 Mar 2019 at 15:29, Andres Freund  wrote:


> On 2019-03-29 09:37:11 +0000, Simon Riggs wrote:
>


> > While trying to understand this, I see there is an even better way to
> > optimize this. Since we are removing dead index tuples, we could alter
> the
> > killed index tuple interface so that it returns the xmax of the tuple
> being
> > marked as killed, rather than just a boolean to say it is dead.
>
> Wouldn't that quite possibly result in additional and unnecessary
> conflicts? Right now the page level horizon is computed whenever the
> page is actually reused, rather than when an item is marked as
> deleted. As it stands right now, the computed horizons are commonly very
> "old", because of that delay, leading to lower rates of conflicts.
>

I wasn't suggesting we change when the horizon is calculated, so no change
there.

The idea was to cache the data for later use, replacing the hint bit with a
hint xid.

That won't change the rate of conflicts, up or down - but it does avoid I/O.


> > Indexes can then mark the killed tuples with the xmax that killed them
> > rather than just a hint bit. This is possible since the index tuples
> > are dead and cannot be used to follow the htid to the heap, so the
> > htid is redundant and so the block number of the tid could be
> > overwritten with the xmax, zeroing the itemid. Each killed item we
> > mark with its xmax means one less heap fetch we need to perform when
> > we delete the page - it's possible we optimize that away completely by
> > doing this.
>
> That's far from a trivial feature imo. It seems quite possible that we'd
> end up with increased overhead, because the current logic can get away
> with only doing hint bit style writes - but would that be true if we
> started actually replacing the item pointers? Because I don't see any
> guarantee they couldn't cross a page boundary etc? So I think we'd need
> to do WAL logging during index searches, which seems prohibitively
> expensive.
>

Don't see that.

I was talking about reusing the first 4 bytes of an index tuple's
ItemPointerData,
which is the first field of an index tuple. Index tuples are MAXALIGNed, so
I can't see how that would ever cross a page boundary.


> And I'm also doubtful it's worth it because:
>
> > Since this point of the code is clearly going to be a performance issue
> it
> > seems like something we should do now.
>
> I've tried quite a bit to find a workload where this matters, but after
> avoiding redundant buffer accesses by sorting, and prefetching I was
> unable to do so.  What workload do you see where this would be really be
> bad? Without the performance optimization I'd found a very minor
> regression by trying to force the heap visits to happen in a pretty
> random order, but after sorting that went away.  I'm sure it's possible
> to find a case on overloaded rotational disks where you'd find a small
> regression, but I don't think it'd be particularly bad.
>

The code can do literally hundreds of random I/Os in an 8192 blocksize.
What happens with 16 or 32kB?

"Small regression" ?

-- 
Simon Riggshttp://www.2ndQuadrant.com/
<http://www.2ndquadrant.com/>
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Re: pgsql: Compute XID horizon for page level index vacuum on primary.

[Simon Riggs]

On Wed, 27 Mar 2019 at 00:06, Andres Freund  wrote:

> Compute XID horizon for page level index vacuum on primary.
>
> Previously the xid horizon was only computed during WAL replay.


This commit message was quite confusing. It took me a while to realize this
relates to btree index deletes and that what you mean is that we are
calculcating the latestRemovedXid for index entries. That is related to but
not same thing as the horizon itself. So now I understand the "was computed
only during WAL replay" since it seemed obvious that the xmin horizon was
calculcated regularly on the master, but as you say the latestRemovedXid
was not.

Now I understand, I'm happy that you've moved this from redo into mainline.
And you've optimized it, which is also important (since performance was the
original objection and why it was placed in redo). I can see you've removed
duplicate code in hash indexes as well, which is good.

The term "xid horizon" was only used once in the code in PG11. That usage
appears to be a typo, since in many other places the term "xmin horizon" is
used to mean the point at which we can finally mark tuples as dead. Now we
have some new, undocumented APIs that use the term "xid horizon" yet still
code that refers to "xmin horizon", with neither term being defined. I'm
hoping you'll do some later cleanup of that to avoid confusion.

While trying to understand this, I see there is an even better way to
optimize this. Since we are removing dead index tuples, we could alter the
killed index tuple interface so that it returns the xmax of the tuple being
marked as killed, rather than just a boolean to say it is dead. Indexes can
then mark the killed tuples with the xmax that killed them rather than just
a hint bit. This is possible since the index tuples are dead and cannot be
used to follow the htid to the heap, so the htid is redundant and so the
block number of the tid could be overwritten with the xmax, zeroing the
itemid. Each killed item we mark with its xmax means one less heap fetch we
need to perform when we delete the page - it's possible we optimize that
away completely by doing this.

Since this point of the code is clearly going to be a performance issue it
seems like something we should do now.

-- 
Simon Riggshttp://www.2ndQuadrant.com/
<http://www.2ndquadrant.com/>
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

pgsql: Remove AELs from subxids correctly on standby

[Simon Riggs]

Remove AELs from subxids correctly on standby

Issues relate only to subtransactions that hold AccessExclusiveLocks
when replayed on standby.

Prior to PG10, aborting subtransactions that held an
AccessExclusiveLock failed to release the lock until top level commit or
abort. 49bff5300d527 fixed that.

However, 49bff5300d527 also introduced a similar bug where subtransaction
commit would fail to release an AccessExclusiveLock, leaving the lock to
be removed sometimes early and sometimes late. This commit fixes
that bug also. Backpatch to PG10 needed.

Tested by observation. Note need for multi-node isolationtester to improve
test coverage for this and other HS cases.

Reported-by: Simon Riggs
Author: Simon Riggs

Branch
--
master

Details
---
https://git.postgresql.org/pg/commitdiff/15378c1a15390a2b4c315f19f1644af46c7e3a15

Modified Files
--
src/backend/access/transam/xact.c   |  6 ++
src/backend/storage/ipc/procarray.c |  5 +
src/backend/storage/ipc/standby.c   | 24 +++-
src/include/storage/standby.h   |  2 +-
4 files changed, 7 insertions(+), 30 deletions(-)

pgsql: Remove spurious code comments in standby related code

[Simon Riggs]

Remove spurious code comments in standby related code

GetRunningTransactionData() suggested that subxids were not worth
optimizing away if overflowed, yet they have already been removed
for that case.

Changes to LogAccessExclusiveLock() API forgot to remove the
prior comment when it was copied to LockAcquire().

Branch
--
master

Details
---
https://git.postgresql.org/pg/commitdiff/dc878ffedf2aa461b11b617094101e3c4cf48daa

Modified Files
--
src/backend/storage/ipc/procarray.c | 4 +---
src/backend/storage/ipc/standby.c   | 5 -
2 files changed, 1 insertion(+), 8 deletions(-)

pgsql: Remove cut-off bug from RunningTransactionData

[Simon Riggs]

Remove cut-off bug from RunningTransactionData

32ac7a118fc17f5 tried to fix a Hot Standby issue
reported by Greg Stark, but in doing so caused
a different bug to appear, noted by Andres Freund.

Revoke the core changes from 32ac7a118fc17f5,
leaving in its place a minor change in code
ordering and comments to explain for the future.

Branch
--
master

Details
---
https://git.postgresql.org/pg/commitdiff/802bde87ba3c64b28d019e8265c2d8948a027c9c

Modified Files
--
src/backend/storage/ipc/procarray.c | 23 ---
1 file changed, 16 insertions(+), 7 deletions(-)

pgsql: Revert MERGE patch

[Simon Riggs]

Revert MERGE patch

This reverts commits d204ef63776b8a00ca220adec23979091564e465,
83454e3c2b28141c0db01c7d2027e01040df5249 and a few more commits thereafter
(complete list at the end) related to MERGE feature.

While the feature was fully functional, with sufficient test coverage and
necessary documentation, it was felt that some parts of the executor and
parse-analyzer can use a different design and it wasn't possible to do that in
the available time. So it was decided to revert the patch for PG11 and retry
again in the future.

Thanks again to all reviewers and bug reporters.

List of commits reverted, in reverse chronological order:

 f1464c5380 Improve parse representation for MERGE
 ddb4158579 MERGE syntax diagram correction
 530e69e59b Allow cpluspluscheck to pass by renaming variable
 01b88b4df5 MERGE minor errata
 3af7b2b0d4 MERGE fix variable warning in non-assert builds
 a5d86181ec MERGE INSERT allows only one VALUES clause
 4b2d44031f MERGE post-commit review
 4923550c20 Tab completion for MERGE
 aa3faa3c7a WITH support in MERGE
 83454e3c2b New files for MERGE
 d204ef6377 MERGE SQL Command following SQL:2016

Author: Pavan Deolasee
Reviewed-by: Michael Paquier

Branch
--
master

Details
---
https://git.postgresql.org/pg/commitdiff/08ea7a2291db21a618d19d612c8060cda68f1892

Modified Files
--
contrib/test_decoding/expected/ddl.out |   46 -
contrib/test_decoding/sql/ddl.sql  |   16 -
doc/src/sgml/libpq.sgml|8 +-
doc/src/sgml/mvcc.sgml |   28 +-
doc/src/sgml/plpgsql.sgml  |3 +-
doc/src/sgml/ref/allfiles.sgml |1 -
doc/src/sgml/ref/create_policy.sgml|7 -
doc/src/sgml/ref/insert.sgml   |   11 +-
doc/src/sgml/ref/merge.sgml|  617 
doc/src/sgml/reference.sgml|1 -
doc/src/sgml/trigger.sgml  |   20 -
src/backend/access/heap/heapam.c   |   28 +-
src/backend/catalog/sql_features.txt   |6 +-
src/backend/commands/explain.c |   33 -
src/backend/commands/prepare.c |1 -
src/backend/commands/trigger.c |  270 +---
src/backend/executor/Makefile  |2 +-
src/backend/executor/README|   11 -
src/backend/executor/execMain.c|   17 -
src/backend/executor/execMerge.c   |  682 
src/backend/executor/execPartition.c   |  121 --
src/backend/executor/execReplication.c |4 +-
src/backend/executor/nodeModifyTable.c |  280 +---
src/backend/executor/spi.c |3 -
src/backend/nodes/copyfuncs.c  |   58 -
src/backend/nodes/equalfuncs.c |   49 -
src/backend/nodes/nodeFuncs.c  |   64 +-
src/backend/nodes/outfuncs.c   |   40 -
src/backend/nodes/readfuncs.c  |   45 -
src/backend/optimizer/plan/createplan.c|   22 +-
src/backend/optimizer/plan/planner.c   |   29 +-
src/backend/optimizer/plan/setrefs.c   |   54 -
src/backend/optimizer/prep/preptlist.c |   41 -
src/backend/optimizer/util/pathnode.c  |   11 +-
src/backend/optimizer/util/plancat.c   |4 -
src/backend/parser/Makefile|2 +-
src/backend/parser/analyze.c   |   18 +-
src/backend/parser/gram.y  |  151 +-
src/backend/parser/parse_agg.c |   10 -
src/backend/parser/parse_clause.c  |   57 +-
src/backend/parser/parse_collate.c |1 -
src/backend/parser/parse_expr.c|3 -
src/backend/parser/parse_func.c|3 -
src/backend/parser/parse_merge.c   |  654 
src/backend/parser/parse_relation.c|   10 -
src/backend/rewrite/rewriteHandler.c   |  115 +-
src/backend/rewrite/rowsecurity.c  |   97 --
src/backend/tcop/pquery.c  |5 -
src/backend/tcop/utility.c |   16 -
src/bin/psql/tab-complete.c|   87 +-
src/include/access/heapam.h|   13 +-
src/include/commands/trigger.h |6 +-
src/include/executor/execMerge.h   |   31 -
src/include/executor/execPartition.h   |1 -
src/include/executor/instrument.h  |7 +-
src/include/executor/nodeModifyTable.h |   23 -
src/include/executor/spi.h |1 -
src/include/nodes/execnodes.h  |   65 +-
src/include/nodes/nodes.h  |7 +-
src/include/nodes/parsenodes.h |   53 +-

pgsql: Improve parse representation for MERGE

[Simon Riggs]

Improve parse representation for MERGE

Separation of parser data structures from executor, as
requested by Tom Lane. Further improvements possible.

While there, implement error for multiple VALUES clauses via parser
to allow line number of error, as requested by Andres Freund.

Author: Pavan Deolasee

Discussion: 
https://www.postgresql.org/message-id/CABOikdPpqjectFchg0FyTOpsGXyPoqwgC==olkwuxgbosrd...@mail.gmail.com

Branch
--
master

Details
---
https://git.postgresql.org/pg/commitdiff/f1464c53804fa7280a7942f6ac08038440f73b11

Modified Files
--
src/backend/nodes/copyfuncs.c| 35 ++
src/backend/nodes/equalfuncs.c   | 28 ---
src/backend/nodes/nodeFuncs.c| 14 --
src/backend/nodes/outfuncs.c | 27 ---
src/backend/nodes/readfuncs.c| 35 ++
src/backend/parser/gram.y| 92 
src/backend/parser/parse_merge.c | 86 +
src/backend/rewrite/rewriteHandler.c |  4 +-
src/include/nodes/nodes.h|  3 +-
src/include/nodes/parsenodes.h   | 27 ---
src/test/regress/expected/merge.out  |  4 +-
11 files changed, 207 insertions(+), 148 deletions(-)

Re: pgsql: MERGE INSERT allows only one VALUES clause

[Simon Riggs]

On 5 April 2018 at 17:15, Andrew Gierth <and...@tao11.riddles.org.uk> wrote:
>>>>>> "Simon" == Simon Riggs <si...@2ndquadrant.com> writes:
>
>  Simon> MERGE INSERT allows only one VALUES clause
>
> That can't possibly be right; you've changed it to say that only one
> _expression_ (i.e. column) is allowed in the VALUES () list, not that
> only one _row_ is allowed. I think it was correct before.

Corrected, thanks

-- 
Simon Riggshttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

pgsql: MERGE syntax diagram correction

[Simon Riggs]

MERGE syntax diagram correction

Reported-by: Andrew Gierth

Branch
--
master

Details
---
https://git.postgresql.org/pg/commitdiff/ddb4158579e052ee35313c333256cb1f16ee65fa

Modified Files
--
doc/src/sgml/ref/merge.sgml | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

pgsql: Allow cpluspluscheck to pass by renaming variable

[Simon Riggs]

Allow cpluspluscheck to pass by renaming variable

Use of a C++ keyword as a function name caused problems

Reported-by: Álvaro Herrera

Branch
--
master

Details
---
https://git.postgresql.org/pg/commitdiff/530e69e59b07cf94a65cfde7dd1a8b1c62b44228

Modified Files
--
src/backend/parser/parse_clause.c | 14 +++---
src/include/parser/parse_clause.h |  2 +-
2 files changed, 8 insertions(+), 8 deletions(-)

pgsql: MERGE minor errata

[Simon Riggs]

MERGE minor errata

Branch
--
master

Details
---
https://git.postgresql.org/pg/commitdiff/01b88b4df5e2df0365cceaf79a039214d9f05273

Modified Files
--
src/backend/executor/README  | 4 ++--
src/backend/executor/execMerge.c | 2 +-
src/backend/executor/execPartition.c | 4 +---
3 files changed, 4 insertions(+), 6 deletions(-)

pgsql: MERGE fix variable warning in non-assert builds

[Simon Riggs]

MERGE fix variable warning in non-assert builds

Author: Jesper Pedersen

Branch
--
master

Details
---
https://git.postgresql.org/pg/commitdiff/3af7b2b0d497cfc240ffc8098ef068adb30048a2

Modified Files
--
src/backend/executor/execMerge.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)

pgsql: MERGE INSERT allows only one VALUES clause

[Simon Riggs]

MERGE INSERT allows only one VALUES clause

Doc syntax and brief mention of restriction

Branch
--
master

Details
---
https://git.postgresql.org/pg/commitdiff/a5d86181ecc9c441d4c327771c43134de59549cd

Modified Files
--
doc/src/sgml/ref/merge.sgml | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

pgsql: MERGE post-commit review

[Simon Riggs]

MERGE post-commit review

Review comments from Andres Freund

* Consolidate code into AfterTriggerGetTransitionTable()
* Rename nodeMerge.c to execMerge.c
* Rename nodeMerge.h to execMerge.h
* Move MERGE handling in ExecInitModifyTable()
  into a execMerge.c ExecInitMerge()
* Move mt_merge_subcommands flags into execMerge.h
* Rename opt_and_condition to opt_merge_when_and_condition
* Wordsmith various comments

Author: Pavan Deolasee
Reviewer: Simon Riggs

Branch
--
master

Details
---
https://git.postgresql.org/pg/commitdiff/4b2d44031f8c005f6f86364d7663858b6b5bdd14

Modified Files
--
src/backend/commands/trigger.c| 192 --
src/backend/executor/Makefile |   4 +-
src/backend/executor/README   |  11 +-
src/backend/executor/{nodeMerge.c => execMerge.c} | 302 +++---
src/backend/executor/execPartition.c  |   9 +-
src/backend/executor/nodeModifyTable.c| 109 +---
src/backend/optimizer/plan/setrefs.c  |  16 +-
src/backend/parser/gram.y |  12 +-
src/include/executor/execMerge.h  |  31 +++
src/include/executor/nodeMerge.h  |  22 --
src/include/executor/nodeModifyTable.h|   1 +
11 files changed, 384 insertions(+), 325 deletions(-)

pgsql: Tab completion for MERGE

[Simon Riggs]

Tab completion for MERGE

Author: Pavan Deolasee

Branch
--
master

Details
---
https://git.postgresql.org/pg/commitdiff/4923550c20ae6d122ae0867480a7de8b040f7d80

Modified Files
--
src/bin/psql/tab-complete.c | 87 +
1 file changed, 81 insertions(+), 6 deletions(-)

pgsql: WITH support in MERGE

[Simon Riggs]

WITH support in MERGE

Author: Peter Geoghegan
Recursive support removed, no tests
Docs added by me

Branch
--
master

Details
---
https://git.postgresql.org/pg/commitdiff/aa3faa3c7a7a49b3318059ccaf79bc1886a64707

Modified Files
--
doc/src/sgml/ref/merge.sgml |  15 +++-
src/backend/nodes/copyfuncs.c   |   1 +
src/backend/nodes/equalfuncs.c  |   1 +
src/backend/nodes/nodeFuncs.c   |   2 +
src/backend/parser/gram.y   |  11 +--
src/backend/parser/parse_merge.c|  14 
src/include/nodes/parsenodes.h  |   1 +
src/test/regress/expected/merge.out |   3 -
src/test/regress/expected/with.out  | 137 
src/test/regress/sql/with.sql   |  56 +++
10 files changed, 232 insertions(+), 9 deletions(-)

pgsql: New files for MERGE

[Simon Riggs]

New files for MERGE

Branch
--
master

Details
---
https://git.postgresql.org/pg/commitdiff/83454e3c2b28141c0db01c7d2027e01040df5249

Modified Files
--
doc/src/sgml/ref/merge.sgml|  603 +++
src/backend/executor/nodeMerge.c   |  575 +++
src/backend/parser/parse_merge.c   |  660 
src/include/executor/nodeMerge.h   |   22 +
src/include/parser/parse_merge.h   |   19 +
src/test/isolation/expected/merge-delete.out   |   97 ++
.../isolation/expected/merge-insert-update.out |   84 +
.../isolation/expected/merge-match-recheck.out |  106 ++
src/test/isolation/expected/merge-update.out   |  213 +++
src/test/isolation/specs/merge-delete.spec |   51 +
src/test/isolation/specs/merge-insert-update.spec  |   52 +
src/test/isolation/specs/merge-match-recheck.spec  |   79 +
src/test/isolation/specs/merge-update.spec |  132 ++
src/test/regress/expected/merge.out| 1673 
src/test/regress/sql/merge.sql | 1173 ++
15 files changed, 5539 insertions(+)

pgsql: MERGE SQL Command following SQL:2016

[Simon Riggs]

MERGE SQL Command following SQL:2016

MERGE performs actions that modify rows in the target table
using a source table or query. MERGE provides a single SQL
statement that can conditionally INSERT/UPDATE/DELETE rows
a task that would other require multiple PL statements.
e.g.

MERGE INTO target AS t
USING source AS s
ON t.tid = s.sid
WHEN MATCHED AND t.balance > s.delta THEN
  UPDATE SET balance = t.balance - s.delta
WHEN MATCHED THEN
  DELETE
WHEN NOT MATCHED AND s.delta > 0 THEN
  INSERT VALUES (s.sid, s.delta)
WHEN NOT MATCHED THEN
  DO NOTHING;

MERGE works with regular and partitioned tables, including
column and row security enforcement, as well as support for
row, statement and transition triggers.

MERGE is optimized for OLTP and is parameterizable, though
also useful for large scale ETL/ELT. MERGE is not intended
to be used in preference to existing single SQL commands
for INSERT, UPDATE or DELETE since there is some overhead.
MERGE can be used statically from PL/pgSQL.

MERGE does not yet support inheritance, write rules,
RETURNING clauses, updatable views or foreign tables.
MERGE follows SQL Standard per the most recent SQL:2016.

Includes full tests and documentation, including full
isolation tests to demonstrate the concurrent behavior.

This version written from scratch in 2017 by Simon Riggs,
using docs and tests originally written in 2009. Later work
from Pavan Deolasee has been both complex and deep, leaving
the lead author credit now in his hands.
Extensive discussion of concurrency from Peter Geoghegan,
with thanks for the time and effort contributed.

Various issues reported via sqlsmith by Andreas Seltenreich

Authors: Pavan Deolasee, Simon Riggs
Reviewer: Peter Geoghegan, Amit Langote, Tomas Vondra, Simon Riggs

Discussion:
https://postgr.es/m/canp8+jkitbsrb7otgt9cy2i1obfot36z0xmraqc+xrz8qb0...@mail.gmail.com
https://postgr.es/m/CAH2-WzkJdBuxj9PO=2QaO9-3h3xGbQPZ34kJH=hukrekwm-...@mail.gmail.com

Branch
--
master

Details
---
https://git.postgresql.org/pg/commitdiff/d204ef63776b8a00ca220adec23979091564e465

Modified Files
--
contrib/test_decoding/expected/ddl.out|  46 
contrib/test_decoding/sql/ddl.sql |  16 ++
doc/src/sgml/libpq.sgml   |   8 +-
doc/src/sgml/mvcc.sgml|  28 ++-
doc/src/sgml/plpgsql.sgml |   3 +-
doc/src/sgml/ref/allfiles.sgml|   1 +
doc/src/sgml/ref/create_policy.sgml   |   7 +
doc/src/sgml/ref/insert.sgml  |  11 +-
doc/src/sgml/reference.sgml   |   1 +
doc/src/sgml/trigger.sgml |  20 ++
src/backend/access/heap/heapam.c  |  28 ++-
src/backend/catalog/sql_features.txt  |   6 +-
src/backend/commands/explain.c|  33 +++
src/backend/commands/prepare.c|   1 +
src/backend/commands/trigger.c| 156 +---
src/backend/executor/Makefile |   2 +-
src/backend/executor/README   |  10 +
src/backend/executor/execMain.c   |  17 ++
src/backend/executor/execPartition.c  | 116 +
src/backend/executor/execReplication.c|   4 +-
src/backend/executor/nodeModifyTable.c| 384 ++
src/backend/executor/spi.c|   3 +
src/backend/nodes/copyfuncs.c |  40 
src/backend/nodes/equalfuncs.c|  32 +++
src/backend/nodes/nodeFuncs.c |  58 -
src/backend/nodes/outfuncs.c  |  25 ++
src/backend/nodes/readfuncs.c |  26 ++
src/backend/optimizer/plan/createplan.c   |  22 +-
src/backend/optimizer/plan/planner.c  |  29 ++-
src/backend/optimizer/plan/setrefs.c  |  54 +
src/backend/optimizer/prep/preptlist.c|  41 
src/backend/optimizer/util/pathnode.c |  11 +-
src/backend/optimizer/util/plancat.c  |   4 +
src/backend/parser/Makefile   |   2 +-
src/backend/parser/analyze.c  |  18 +-
src/backend/parser/gram.y | 158 +++-
src/backend/parser/parse_agg.c|  10 +
src/backend/parser/parse_clause.c |  45 +++-
src/backend/parser/parse_collate.c|   1 +
src/backend/parser/parse_expr.c   |   3 +
src/backend/parser/parse_func.c   |   3 +
src/backend/parser/parse_relation.c   |  10 +
src/backend/rewrite/rewriteHandler.c  | 117 -
src/backend/rewrite/rowsecurity.c |  97 
src/backend/tcop/pquery.c |   5 +
src/backend/tcop/utility.c|  16 ++
src/include/access/heapam.h   |  13 +-
src/include/commands/trigger.h|   6 +-
src/include/executor/execPartition.h  |   1 +
src/include/executor/instrument.h |   7 +-
src/include/executor/nodeModifyTable.h|  21 ++
src/include/executor/spi.h|   1 +
src/include/nodes/execnodes.h |  65 -
src/include/nodes/nodes.h |   6 +-
src/include/nodes/parsenodes.h|  37 ++-
src/i

Re: pgsql: Modified files for MERGE

[Simon Riggs]

On 2 April 2018 at 21:54, Andres Freund <and...@anarazel.de> wrote:
> Hi,
>
> On 2018-04-02 21:50:28 +0100, Simon Riggs wrote:
>> I didn't think it mattered, but clearly does. Reverted before you
>> asked.
>
> I don't think the BF failures were related to the split, though... I
> personally think it's fair to take a couple hours to attempt to mop-up
> post commit issues of a larger patch...

I diagnosed the causal issue as lack of sleep and will try again once
that is cured.

But thank you for the leeway.

-- 
Simon Riggshttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

pgsql: Revert "MERGE SQL Command following SQL:2016"

[Simon Riggs]

Revert "MERGE SQL Command following SQL:2016"

This reverts commit e6597dc3533946b98acba7871bd4ca1f7a3d4c1d.

Branch
--
master

Details
---
https://git.postgresql.org/pg/commitdiff/aa5877bb26347c58a34aee4e460eb1e1123bb096

Modified Files
--
doc/src/sgml/ref/merge.sgml|  603 ---
src/backend/executor/nodeMerge.c   |  575 ---
src/backend/parser/parse_merge.c   |  660 ---
src/include/executor/nodeMerge.h   |   22 -
src/include/parser/parse_merge.h   |   19 -
src/test/isolation/expected/merge-delete.out   |   97 --
.../isolation/expected/merge-insert-update.out |   84 -
.../isolation/expected/merge-match-recheck.out |  106 --
src/test/isolation/expected/merge-update.out   |  213 ---
src/test/isolation/specs/merge-delete.spec |   51 -
src/test/isolation/specs/merge-insert-update.spec  |   52 -
src/test/isolation/specs/merge-match-recheck.spec  |   79 -
src/test/isolation/specs/merge-update.spec |  132 --
src/test/regress/expected/merge.out| 1811 
src/test/regress/sql/merge.sql | 1173 -
15 files changed, 5677 deletions(-)

pgsql: Revert "Modified files for MERGE"

[Simon Riggs]

Revert "Modified files for MERGE"

This reverts commit 354f13855e6381d288dfaa52bcd4f2cb0fd4a5eb.

Branch
--
master

Details
---
https://git.postgresql.org/pg/commitdiff/7cf8a5c302735d193dcf901b87e03e324903c632

Modified Files
--
contrib/test_decoding/expected/ddl.out|  46 
contrib/test_decoding/sql/ddl.sql |  16 --
doc/src/sgml/libpq.sgml   |   8 +-
doc/src/sgml/mvcc.sgml|  28 +--
doc/src/sgml/plpgsql.sgml |   3 +-
doc/src/sgml/ref/allfiles.sgml|   1 -
doc/src/sgml/ref/create_policy.sgml   |   7 -
doc/src/sgml/ref/insert.sgml  |  11 +-
doc/src/sgml/reference.sgml   |   1 -
doc/src/sgml/trigger.sgml |  20 --
src/backend/access/heap/heapam.c  |  28 +--
src/backend/catalog/sql_features.txt  |   6 +-
src/backend/commands/explain.c|  33 ---
src/backend/commands/prepare.c|   1 -
src/backend/commands/trigger.c| 156 +++-
src/backend/executor/Makefile |   2 +-
src/backend/executor/README   |  10 -
src/backend/executor/execMain.c   |  17 --
src/backend/executor/execPartition.c  | 116 -
src/backend/executor/execReplication.c|   4 +-
src/backend/executor/nodeModifyTable.c| 384 --
src/backend/executor/spi.c|   3 -
src/backend/nodes/copyfuncs.c |  40 
src/backend/nodes/equalfuncs.c|  32 ---
src/backend/nodes/nodeFuncs.c |  48 +---
src/backend/nodes/outfuncs.c  |  25 --
src/backend/nodes/readfuncs.c |   6 -
src/backend/optimizer/plan/createplan.c   |  22 +-
src/backend/optimizer/plan/planner.c  |  29 +--
src/backend/optimizer/plan/setrefs.c  |  54 -
src/backend/optimizer/prep/preptlist.c|  41 
src/backend/optimizer/util/pathnode.c |  11 +-
src/backend/optimizer/util/plancat.c  |   4 -
src/backend/parser/Makefile   |   2 +-
src/backend/parser/analyze.c  |  18 +-
src/backend/parser/gram.y | 158 +---
src/backend/parser/parse_agg.c|  10 -
src/backend/parser/parse_clause.c |  45 +---
src/backend/parser/parse_collate.c|   1 -
src/backend/parser/parse_expr.c   |   3 -
src/backend/parser/parse_func.c   |   3 -
src/backend/parser/parse_relation.c   |  10 -
src/backend/rewrite/rewriteHandler.c  | 117 +
src/backend/rewrite/rowsecurity.c |  97 
src/backend/tcop/pquery.c |   5 -
src/backend/tcop/utility.c|  16 --
src/include/access/heapam.h   |  13 +-
src/include/commands/trigger.h|   6 +-
src/include/executor/execPartition.h  |   1 -
src/include/executor/instrument.h |   7 +-
src/include/executor/nodeModifyTable.h|  21 --
src/include/executor/spi.h|   1 -
src/include/nodes/execnodes.h |  65 +
src/include/nodes/nodes.h |   6 +-
src/include/nodes/parsenodes.h|  37 +--
src/include/nodes/plannodes.h |   8 +-
src/include/nodes/relation.h  |   7 +-
src/include/optimizer/pathnode.h  |   7 +-
src/include/parser/analyze.h  |   5 -
src/include/parser/kwlist.h   |   2 -
src/include/parser/parse_clause.h |   5 +-
src/include/parser/parse_node.h   |   5 +-
src/include/rewrite/rewriteHandler.h  |   1 -
src/interfaces/libpq/fe-exec.c|   9 +-
src/pl/plpgsql/src/pl_exec.c  |   5 +-
src/pl/plpgsql/src/pl_gram.y  |   8 -
src/pl/plpgsql/src/pl_scanner.c   |   1 -
src/pl/plpgsql/src/plpgsql.h  |   4 +-
src/test/isolation/isolation_schedule |   4 -
src/test/regress/expected/identity.out|  55 -
src/test/regress/expected/privileges.out  |  98 
src/test/regress/expected/rowsecurity.out | 182 --
src/test/regress/expected/rules.out   |  31 ---
src/test/regress/expected/triggers.out|  48 
src/test/regress/parallel_schedule|   2 +-
src/test/regress/serial_schedule  |   1 -
src/test/regress/sql/identity.sql |  45 
src/test/regress/sql/privileges.sql   | 108 -
src/test/regress/sql/rowsecurity.sql  | 156 
src/test/regress/sql/rules.sql|  33 ---
src/test/regress/sql/triggers.sql |  47 
src/tools/pgindent/typedefs.list  |   3 -
82 files changed, 165 insertions(+), 2570 deletions(-)

pgsql: Modified files for MERGE

[Simon Riggs]

Modified files for MERGE

Branch
--
master

Details
---
https://git.postgresql.org/pg/commitdiff/354f13855e6381d288dfaa52bcd4f2cb0fd4a5eb

Modified Files
--
contrib/test_decoding/expected/ddl.out|  46 
contrib/test_decoding/sql/ddl.sql |  16 ++
doc/src/sgml/libpq.sgml   |   8 +-
doc/src/sgml/mvcc.sgml|  28 ++-
doc/src/sgml/plpgsql.sgml |   3 +-
doc/src/sgml/ref/allfiles.sgml|   1 +
doc/src/sgml/ref/create_policy.sgml   |   7 +
doc/src/sgml/ref/insert.sgml  |  11 +-
doc/src/sgml/reference.sgml   |   1 +
doc/src/sgml/trigger.sgml |  20 ++
src/backend/access/heap/heapam.c  |  28 ++-
src/backend/catalog/sql_features.txt  |   6 +-
src/backend/commands/explain.c|  33 +++
src/backend/commands/prepare.c|   1 +
src/backend/commands/trigger.c| 156 +---
src/backend/executor/Makefile |   2 +-
src/backend/executor/README   |  10 +
src/backend/executor/execMain.c   |  17 ++
src/backend/executor/execPartition.c  | 116 +
src/backend/executor/execReplication.c|   4 +-
src/backend/executor/nodeModifyTable.c| 384 ++
src/backend/executor/spi.c|   3 +
src/backend/nodes/copyfuncs.c |  40 
src/backend/nodes/equalfuncs.c|  32 +++
src/backend/nodes/nodeFuncs.c |  48 +++-
src/backend/nodes/outfuncs.c  |  25 ++
src/backend/nodes/readfuncs.c |   6 +
src/backend/optimizer/plan/createplan.c   |  22 +-
src/backend/optimizer/plan/planner.c  |  29 ++-
src/backend/optimizer/plan/setrefs.c  |  54 +
src/backend/optimizer/prep/preptlist.c|  41 
src/backend/optimizer/util/pathnode.c |  11 +-
src/backend/optimizer/util/plancat.c  |   4 +
src/backend/parser/Makefile   |   2 +-
src/backend/parser/analyze.c  |  18 +-
src/backend/parser/gram.y | 158 +++-
src/backend/parser/parse_agg.c|  10 +
src/backend/parser/parse_clause.c |  45 +++-
src/backend/parser/parse_collate.c|   1 +
src/backend/parser/parse_expr.c   |   3 +
src/backend/parser/parse_func.c   |   3 +
src/backend/parser/parse_relation.c   |  10 +
src/backend/rewrite/rewriteHandler.c  | 117 -
src/backend/rewrite/rowsecurity.c |  97 
src/backend/tcop/pquery.c |   5 +
src/backend/tcop/utility.c|  16 ++
src/include/access/heapam.h   |  13 +-
src/include/commands/trigger.h|   6 +-
src/include/executor/execPartition.h  |   1 +
src/include/executor/instrument.h |   7 +-
src/include/executor/nodeModifyTable.h|  21 ++
src/include/executor/spi.h|   1 +
src/include/nodes/execnodes.h |  65 -
src/include/nodes/nodes.h |   6 +-
src/include/nodes/parsenodes.h|  37 ++-
src/include/nodes/plannodes.h |   8 +-
src/include/nodes/relation.h  |   7 +-
src/include/optimizer/pathnode.h  |   7 +-
src/include/parser/analyze.h  |   5 +
src/include/parser/kwlist.h   |   2 +
src/include/parser/parse_clause.h |   5 +-
src/include/parser/parse_node.h   |   5 +-
src/include/rewrite/rewriteHandler.h  |   1 +
src/interfaces/libpq/fe-exec.c|   9 +-
src/pl/plpgsql/src/pl_exec.c  |   5 +-
src/pl/plpgsql/src/pl_gram.y  |   8 +
src/pl/plpgsql/src/pl_scanner.c   |   1 +
src/pl/plpgsql/src/plpgsql.h  |   4 +-
src/test/isolation/isolation_schedule |   4 +
src/test/regress/expected/identity.out|  55 +
src/test/regress/expected/privileges.out  |  98 
src/test/regress/expected/rowsecurity.out | 182 ++
src/test/regress/expected/rules.out   |  31 +++
src/test/regress/expected/triggers.out|  48 
src/test/regress/parallel_schedule|   2 +-
src/test/regress/serial_schedule  |   1 +
src/test/regress/sql/identity.sql |  45 
src/test/regress/sql/privileges.sql   | 108 +
src/test/regress/sql/rowsecurity.sql  | 156 
src/test/regress/sql/rules.sql|  33 +++
src/test/regress/sql/triggers.sql |  47 
src/tools/pgindent/typedefs.list  |   3 +
82 files changed, 2570 insertions(+), 165 deletions(-)

pgsql: MERGE SQL Command following SQL:2016

[Simon Riggs]

MERGE SQL Command following SQL:2016

MERGE performs actions that modify rows in the target table
using a source table or query. MERGE provides a single SQL
statement that can conditionally INSERT/UPDATE/DELETE rows
a task that would other require multiple PL statements.
e.g.

MERGE INTO target AS t
USING source AS s
ON t.tid = s.sid
WHEN MATCHED AND t.balance > s.delta THEN
  UPDATE SET balance = t.balance - s.delta
WHEN MATCHED THEN
  DELETE
WHEN NOT MATCHED AND s.delta > 0 THEN
  INSERT VALUES (s.sid, s.delta)
WHEN NOT MATCHED THEN
  DO NOTHING;

MERGE works with regular and partitioned tables, including
column and row security enforcement, as well as support for
row, statement and transition triggers.

MERGE is optimized for OLTP and is parameterizable, though
also useful for large scale ETL/ELT. MERGE is not intended
to be used in preference to existing single SQL commands
for INSERT, UPDATE or DELETE since there is some overhead.
MERGE can be used statically from PL/pgSQL.

MERGE does not yet support inheritance, write rules,
RETURNING clauses, updatable views or foreign tables.
MERGE follows SQL Standard per the most recent SQL:2016.

Includes full tests and documentation, including full
isolation tests to demonstrate the concurrent behavior.

This version written from scratch in 2017 by Simon Riggs,
using docs and tests originally written in 2009. Later work
from Pavan Deolasee has been both complex and deep, leaving
the lead author credit now in his hands.
Extensive discussion of concurrency from Peter Geoghegan,
with thanks for the time and effort contributed.

Various issues reported via sqlsmith by Andreas Seltenreich

Authors: Pavan Deolasee, Simon Riggs
Reviewers: Peter Geoghegan, Amit Langote, Tomas Vondra, Simon Riggs

Discussion:
https://postgr.es/m/canp8+jkitbsrb7otgt9cy2i1obfot36z0xmraqc+xrz8qb0...@mail.gmail.com
https://postgr.es/m/CAH2-WzkJdBuxj9PO=2QaO9-3h3xGbQPZ34kJH=hukrekwm-...@mail.gmail.com

Branch
--
master

Details
---
https://git.postgresql.org/pg/commitdiff/e6597dc3533946b98acba7871bd4ca1f7a3d4c1d

Modified Files
--
doc/src/sgml/ref/merge.sgml|  603 +++
src/backend/executor/nodeMerge.c   |  575 +++
src/backend/parser/parse_merge.c   |  660 +++
src/include/executor/nodeMerge.h   |   22 +
src/include/parser/parse_merge.h   |   19 +
src/test/isolation/expected/merge-delete.out   |   97 ++
.../isolation/expected/merge-insert-update.out |   84 +
.../isolation/expected/merge-match-recheck.out |  106 ++
src/test/isolation/expected/merge-update.out   |  213 +++
src/test/isolation/specs/merge-delete.spec |   51 +
src/test/isolation/specs/merge-insert-update.spec  |   52 +
src/test/isolation/specs/merge-match-recheck.spec  |   79 +
src/test/isolation/specs/merge-update.spec |  132 ++
src/test/regress/expected/merge.out| 1811 
src/test/regress/sql/merge.sql | 1173 +
15 files changed, 5677 insertions(+)

Re: pgsql: Optimize btree insertions for common case of increasing values

[Simon Riggs]

On 29 March 2018 at 00:09, Peter Geoghegan <p...@bowt.ie> wrote:
> On Wed, Mar 28, 2018 at 11:56 AM, Peter Geoghegan <p...@bowt.ie> wrote:
>>> Previously, we agreed that P_IGNORE() is required. So I assume no issues
>>> there. The other tests seem required too for us to conclusively decide to
>>> use the cached block.
>>
>> Actually, you're right that P_INCOMPLETE_SPLIT() is the only redundant
>> one.
>
> Another issue that I have with the main test of the
> RelationSetTargetBlock() page is that nobody explains *why* we check
> that we have enough space on the page to proceed. Why would it not be
> okay if there was a page split?

...because we need the Stack to bubble up the parent insert.

> Although it's subtle, I'm pretty confident that it actually would be
> okay from a correctness point of view to allow an insertion to go
> ahead that would result in a page split -- it's possible to pass a
> NULL stack to _bt_findinsertloc() and _bt_insertonpg() while still
> getting correct behavior.
>
> * In the case of _bt_findinsertloc(), it's okay to have a NULL stack
> because you'll never take the P_INCOMPLETE_SPLIT() path for a
> rightmost page, as already explained, so the stack cannot possibly be
> used (besides, even _bt_finish_split() can deal with a NULL stack). It
> is not just an accident that it works, because _bt_search() will
> sometimes return a NULL stack when writing -- it does so when there is
> only one non-meta page, which is both a root and a leaf page. Your
> optimization is a bit similar to that, in the sense that the
> cached/target block contains a leaf page that is rightmost (root pages
> are always rightmost, and are leaf pages in the case described).
>
> * In the case of _bt_insertonpg(), it's okay to have a NULL stack
> because there is a codepath that allows _bt_insert_parent() (which is
> the only thing in _bt_insertonpg() that touches the stack) to work
> without a stack during crash recovery. That path is quite a bit
> slower, though.

It's a linear scan, so quite clearly not going to perform well on big indexes.

Why would that be worth pursuing?

> Suggested next steps to deal with this:
>
> * A minor point: I don't think you should call
> RelationSetTargetBlock() when the page P_ISROOT(), which, as I
> mentioned, is a condition that can coexist with P_ISLEAF() with very
> small B-Trees. There can be no point in doing so. No need to add
> P_ISROOT() to the main "Is cached page stale?" test within
> _bt_doinsert(), though.

True. A better test would be to not use the optimization at all on
smaller btrees by checking the level is >= 3.

> * An assertion would make me feel a lot better about depending on not
> having a page split from a significant distance.
>
> Your optimization should continue to not be used when it would result
> in a page split, but only because that would be slow. The comments
> should say so, IMV. Also, _bt_insertonpg() should have an assertion
> against a page split actually occurring when the optimization was
> used, just in case. When !BufferIsValid(cbuf), we know that we're
> being called from _bt_doinsert() (see existing assertion at the top of
> _bt_insertonpg() as an example of this), so at the point where it's
> clear a page split is needed, we should assert that there is no target
> block that we must have been passed as the target page.

This is the same issues as the NULL stack. The page split would need
to insert into parent.

> * The indentation of the main _bt_doinsert() test does not follow
> project guidelines. Please tweak that, too.
>
> That's all I have for now. I might think of something else tomorrow,
> since I haven't spent that much time on this. It would be nice to get
> a second opinion on some of this, if at all possible.

I don't see any need to change any of these things. This is a tuning
patch and none of the above affects correctness of what has been
committed.

If you can find non-performant cases that require further tuning, we
can submit another patch at a later time.

And then we can cross the bridge of checking whether what is said
above is correct or not.

-- 
Simon Riggshttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Re: pgsql: Add documentation for the JIT feature.

[Simon Riggs]

On 28 March 2018 at 22:23, Andres Freund <and...@anarazel.de> wrote:

> Add documentation for the JIT feature.

Very nice feature and most welcome but we should call it something
other than just "JIT"

JIT means Just In Time, which could be applied to many concepts and
has been in use for many years in a range of concepts. particularly in
manufacturing/logistics and project management.

The feature is JIT compilation, not just "JIT". If we had replication
with eager consensus, it would be silly to call the feature simply
"eager" with no other qualifier.

Thanks

-- 
Simon Riggshttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

pgsql: Store 2PC GID in commit/abort WAL recs for logical decoding

[Simon Riggs]

Store 2PC GID in commit/abort WAL recs for logical decoding

Store GID of 2PC in commit/abort WAL records when wal_level = logical.
This allows logical decoding to send the SAME gid to subscribers
across restarts of logical replication.

Track relica origin replay progress for 2PC.

(Edited from patch 0003 in the logical decoding 2PC series.)

Authors: Nikhil Sontakke, Stas Kelvich
Reviewed-by: Simon Riggs, Andres Freund

Branch
--
master

Details
---
https://git.postgresql.org/pg/commitdiff/1eb6d6527aae264b3e0b9c95aa70bb7a594ad1cf

Modified Files
--
src/backend/access/rmgrdesc/xactdesc.c |  39 
src/backend/access/transam/twophase.c  | 105 -
src/backend/access/transam/xact.c  |  78 ++--
src/include/access/twophase.h  |   5 +-
src/include/access/xact.h  |  27 -
5 files changed, 230 insertions(+), 24 deletions(-)

pgsql: Use pg_stat_get_xact* functions within xacts

[Simon Riggs]

Use pg_stat_get_xact* functions within xacts

Resolve build farm failures from c203d6cf81b4d7e43,
diagnosed by Tom Lane.

The output of pg_stat_get_xact_tuples_hot_updated() and friends
is not guaranteed to show anything after the transaction completes.
Data is flushed slowly to stats collector, so using them can
give timing issues.

Branch
--
master

Details
---
https://git.postgresql.org/pg/commitdiff/5b0d7f6996abfc1e3e51bac62af6076903635dc8

Modified Files
--
src/test/regress/expected/func_index.out | 9 ++---
src/test/regress/sql/func_index.sql  | 9 ++---
2 files changed, 12 insertions(+), 6 deletions(-)

Re: pgsql: Allow HOT updates for some expression indexes

[Simon Riggs]

On 27 March 2018 at 20:11, Simon Riggs <si...@2ndquadrant.com> wrote:
> Buildfarm failure seen, investigating

Error on FreeBSD only , perhaps timing-related as a result of using
pg_stat_get_xact_tuples_hot_updated()
in tests.

Will wait to collect other failures

-- 
Simon Riggshttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Re: pgsql: Improve ANALYZE's strategy for finding MCVs.

[Simon Riggs]

On 22 March 2018 at 09:42, Dean Rasheed <dean.a.rash...@gmail.com> wrote:

> Improve ANALYZE's strategy for finding MCVs.

> Thus it tends to produce
> fewer MCVs than the previous code for uniform distributions, and more
> for non-uniform distributions, reducing estimation errors in both
> cases.  In addition, the algorithm responds better to increasing the
> statistics target, allowing more values to be included in the MCV list
> when more of the table is sampled.
>
> Jeff Janes, substantially modified by me. Reviewed by John Naylor and
> Tomas Vondra.

git praise --all

-- 
Simon Riggshttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

pgsql: Default monitoring roles - errata

[Simon Riggs]

Default monitoring roles - errata

25fff40798fc4ac11a241bfd9ab0c45c085e2212 introduced
default monitoring roles. Apply these corrections:

* Allow access to pg_stat_get_wal_senders()
  by role pg_read_all_stats

* Correct comment in pg_stat_get_wal_receiver()
  to show it is no longer superuser-only.

Author: Feike Steenbergen
Reviewed-by: Michael Paquier

Apply to HEAD, then later backpatch to 10

Branch
--
REL_10_STABLE

Details
---
https://git.postgresql.org/pg/commitdiff/76e117dbed8c0fee084fbfc06f15c6c377690f59

Modified Files
--
src/backend/replication/walreceiver.c | 3 ++-
src/backend/replication/walsender.c   | 8 +---
2 files changed, 7 insertions(+), 4 deletions(-)

pgsql: Fix typo in recent commit

[Simon Riggs]

Fix typo in recent commit

Typo in 9c7d06d60680c7f00d931233873dee81fdb311c6

Reported-by: Masahiko Sawada

Branch
--
master

Details
---
https://git.postgresql.org/pg/commitdiff/4e54dd2e0a750352ce2a5c45d1cc9183e887eec3

Modified Files
--
src/backend/replication/slotfuncs.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

pgsql: Ability to advance replication slots

[Simon Riggs]

Ability to advance replication slots

Ability to advance both physical and logical replication slots using a
new user function pg_replication_slot_advance().

For logical advance that means records are consumed as fast as possible
and changes are not given to output plugin for sending. Makes 2nd phase
(after we reached SNAPBUILD_FULL_SNAPSHOT) of replication slot creation
faster, especially when there are big transactions as the reorder buffer
does not have to deal with data changes and does not have to spill to
disk.

Author: Petr Jelinek
Reviewed-by: Simon Riggs

Branch
--
master

Details
---
https://git.postgresql.org/pg/commitdiff/9c7d06d60680c7f00d931233873dee81fdb311c6

Modified Files
--
contrib/test_decoding/expected/slot.out|  30 
contrib/test_decoding/sql/slot.sql |  15 ++
doc/src/sgml/func.sgml |  19 +++
src/backend/replication/logical/decode.c   |  44 --
src/backend/replication/logical/logical.c  |  30 +++-
src/backend/replication/logical/logicalfuncs.c |   1 +
src/backend/replication/slotfuncs.c| 200 +
src/backend/replication/walsender.c|   1 +
src/include/catalog/pg_proc.h  |   2 +
src/include/replication/logical.h  |   8 +
10 files changed, 333 insertions(+), 17 deletions(-)

Re: pgsql: Implement channel binding tls-server-end-point for SCRAM

[Simon Riggs]

On 4 January 2018 at 21:02, Tom Lane <t...@sss.pgh.pa.us> wrote:
> Peter Eisentraut <pete...@gmx.net> writes:
>> Implement channel binding tls-server-end-point for SCRAM
>
> Buildfarm doesn't like this one bit.

Can't we automate these messages? Seems strange to send manual emails
every time. We do know who the commits are coming from and we have
their email address.

It would be useful to get automatic message giving a summary of
buildfarm results at 15, 30 and 60 minute intervals, even if it is
just ALL CLEAR.

-- 
Simon Riggshttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

pgsql: Add TIMELINE to backup_label file

[Simon Riggs]

Add TIMELINE to backup_label file

Allows new test to confirm timelines match

Author: Michael Paquier
Reviewed-by: David Steele

Branch
--
master

Details
---
https://git.postgresql.org/pg/commitdiff/6271fceb8a4f07dafe9d67dcf7e849b319bb2647

Modified Files
--
src/backend/access/transam/xlog.c | 50 ---
src/test/perl/PostgresNode.pm |  1 +
2 files changed, 48 insertions(+), 3 deletions(-)

pgsql: Set es_output_cid in replication worker

[Simon Riggs]

Set es_output_cid in replication worker

Allows triggers to operate correctly

Author: Petr Jelinek 
Reported-by: Konstantin Knizhnik 

Branch
--
REL_10_STABLE

Details
---
https://git.postgresql.org/pg/commitdiff/7015bb38949a44849b7a2e6c139700d20d82858b

Modified Files
--
src/backend/replication/logical/worker.c | 2 ++
1 file changed, 2 insertions(+)

pgsql: Additional docs for toast_tuple_target changes

[Simon Riggs]

Additional docs for toast_tuple_target changes

Branch
--
master

Details
---
https://git.postgresql.org/pg/commitdiff/117469006bf525c6e8dc84cb9fcbdc4c1a050878

Modified Files
--
doc/src/sgml/storage.sgml | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)

pgsql: Pad XLogReaderState's per-buffer data_bufsz more aggressively.

[Simon Riggs]

Pad XLogReaderState's per-buffer data_bufsz more aggressively.

Originally, we palloc'd this buffer just barely big enough to hold the
largest xlog backup block seen so far. We now MAXALIGN the palloc size.

The original coding could result in many repeated palloc cycles, in the
worst case where we see a series ofgradually larger xlog records.  We
ameliorate that similarly to 8735978e7aebfbc499843630131c18d1f7346c79
by imposing a minimum buffer size of BLCKSZ.

Discussion: https://postgr.es/m/e1eha4j-0006hi...@gemulon.postgresql.org

Branch
--
master

Details
---
https://git.postgresql.org/pg/commitdiff/59af8d4384ba5ae72986eab7e5cdc514a969aa05

Modified Files
--
src/backend/access/transam/xlogreader.c | 8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)

Re: pgsql: Generational memory allocator

[Simon Riggs]

On 27 November 2017 at 06:42, Tom Lane <t...@sss.pgh.pa.us> wrote:
> Simon Riggs <si...@2ndquadrant.com> writes:
>> On 27 November 2017 at 05:53, Tom Lane <t...@sss.pgh.pa.us> wrote:
>>> Well, let's not overthink this, because anything under 8K is going to
>>> be rounded up to the next power of 2 anyway by aset.c.  Based on this
>>> point I'd say that BLCKSZ/2 or BLCKSZ/4 would be reasonable candidates
>>> for the minimum.
>
>> BLCKSZ/2 seems best then.
>
> Sold, will make it so.

So you will like this next patch also, since there is related code
above that stanza.

-- 
Simon Riggshttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


data_bufsz.v1.patch
Description: Binary data

Re: pgsql: Generational memory allocator

[Simon Riggs]

On 27 November 2017 at 05:53, Tom Lane <t...@sss.pgh.pa.us> wrote:
> Simon Riggs <si...@2ndquadrant.com> writes:
>> On 27 November 2017 at 04:46, Tom Lane <t...@sss.pgh.pa.us> wrote:
>>> Well, I'm concerned about the possibility of a lot of palloc thrashing
>>> if the first bunch of records it reads happen to have steadily increasing
>>> sizes.  However, rather than doubling, it might be sufficient to set a
>>> robust minimum on the first allocation, ie use something along the lines
>>> of Max(1024, MAXALIGN(state->main_data_len)).
>
>> Agreed.
>
>> I was just researching what that number should be... and I was
>> thinking that we should use the maximum normal tuple size, which I
>> think is
>
>> TOAST_TUPLE_THRESHOLD +
>> SizeOfXLogRecord +
>> SizeOfXLogRecordDataHeaderLong
>
> Well, let's not overthink this, because anything under 8K is going to
> be rounded up to the next power of 2 anyway by aset.c.  Based on this
> point I'd say that BLCKSZ/2 or BLCKSZ/4 would be reasonable candidates
> for the minimum.

BLCKSZ/2 seems best then.

I guess that means palloc doesn't work well with BLCKSZ > 8192

-- 
Simon Riggshttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Re: pgsql: Generational memory allocator

[Simon Riggs]

On 27 November 2017 at 04:46, Tom Lane <t...@sss.pgh.pa.us> wrote:
> Simon Riggs <si...@2ndquadrant.com> writes:
>> On 26 November 2017 at 08:46, Tom Lane <t...@sss.pgh.pa.us> wrote:
>>> I've confirmed that the attached is sufficient to stop the valgrind crash
>>> on my machine.  But as I said, I think we should be more aggressive at
>>> resizing the buffer, to reduce resize cycles.  I'm inclined to start out
>>> with a buffer size of 128 or 256 or so bytes and double it when needed.
>>> Anybody have a feeling for a typical size for the "main data" part
>>> of a WAL record?
>
>> We reuse the buffer and only pfree/palloc when we need to enlarge the
>> buffer, so not sure we need to do the doubling thing and it probably
>> doesn't matter what the typical size is.
>
> Well, I'm concerned about the possibility of a lot of palloc thrashing
> if the first bunch of records it reads happen to have steadily increasing
> sizes.  However, rather than doubling, it might be sufficient to set a
> robust minimum on the first allocation, ie use something along the lines
> of Max(1024, MAXALIGN(state->main_data_len)).

Agreed.

I was just researching what that number should be... and I was
thinking that we should use the maximum normal tuple size, which I
think is

TOAST_TUPLE_THRESHOLD +
SizeOfXLogRecord +
SizeOfXLogRecordDataHeaderLong

-- 
Simon Riggshttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Re: pgsql: Generational memory allocator

[Simon Riggs]

On 25 November 2017 at 12:25, Tom Lane <t...@sss.pgh.pa.us> wrote:
> I wrote:
>> For me, this patch fixes the valgrind failures inside generation.c
>> itself, but I still see one more in the test_decoding run: ...
>> Not sure what to make of this: the stack traces make it look unrelated
>> to the GenerationContext changes, but if it's not related, how come
>> skink was passing before that patch went in?
>
> I've pushed fixes for everything that I could find wrong in generation.c
> (and there was a lot :-().

Thanks very much for doing that.  I'll review changes and comment.

-- 
Simon Riggshttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Re: pgsql: Generational memory allocator

[Simon Riggs]

On 25 November 2017 at 02:35, Andrew Dunstan
<andrew.duns...@2ndquadrant.com> wrote:
>
>
> On 11/23/2017 03:06 PM, Tom Lane wrote:
>>
>> I think it's a legitimate complaint that postmaster.log wasn't captured
>> in this failure, but that's a buildfarm script oversight and hardly
>> Andres' fault.
>>
>
> A fix for this will be in the next buildfarm release.

Thanks

-- 
Simon Riggshttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Re: pgsql: Generational memory allocator

[Simon Riggs]

On 26 November 2017 at 08:46, Tom Lane <t...@sss.pgh.pa.us> wrote:
> I wrote:
>> Instead I propose that we should make sure that the palloc request size
>> for XLogReaderState->main_data is always maxalign'd.  The existing
>> behavior in DecodeXLogRecord of palloc'ing it only just barely big
>> enough for the current record seems pretty brain-dead performance-wise
>> even without this consideration.  Generally, if we need to enlarge
>> that buffer, we should enlarge it significantly, IMO.
>
> I've confirmed that the attached is sufficient to stop the valgrind crash
> on my machine.  But as I said, I think we should be more aggressive at
> resizing the buffer, to reduce resize cycles.  I'm inclined to start out
> with a buffer size of 128 or 256 or so bytes and double it when needed.
> Anybody have a feeling for a typical size for the "main data" part
> of a WAL record?

We reuse the buffer and only pfree/palloc when we need to enlarge the
buffer, so not sure we need to do the doubling thing and it probably
doesn't matter what the typical size is.

So I think we're just good to go with your patch.

-- 
Simon Riggshttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Re: pgsql: Generational memory allocator

[Simon Riggs]

On 24 November 2017 at 09:04, Tom Lane <t...@sss.pgh.pa.us> wrote:
> Andres Freund <and...@anarazel.de> writes:
>> On 2017-11-23 22:34:57 +0100, Tomas Vondra wrote:
>>> Hmmm, I see. Presumably adding this to GenerationChunk (similarly to what we
>>> do in AllocChunkData) would address the issue:
>>>
>>> #if MAXIMUM_ALIGNOF > 4 && SIZEOF_VOID_P == 4
>>> Size padding;
>>> #endif
>>>
>>> but I have no way to verify that (no access to such machine). I wonder why
>>> SlabChunk doesn't need to do that (perhaps a comment explaining that would
>>> be appropriate?).
>
>> Can't you just compile pg on a 32bit system and manually define MAXALIGN
>> to 8 bytes?
>
> I pushed a patch that computes how much padding to add and adds it.
> (It might not really work if size_t and void * are different sizes,
> because then there could be additional padding in the struct; but
> that seems very unlikely.)

Oh, OK, thanks.

It sunk in what was needed while flying, but that's better than my efforts.

-- 
Simon Riggshttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Re: pgsql: Generational memory allocator

[Simon Riggs]

On 24 November 2017 at 07:06, Tom Lane <t...@sss.pgh.pa.us> wrote:

> Meanwhile, over on
> https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=snapper=2017-11-23%2013%3A56%3A17
>
> we have
>
> ccache gcc-4.7 -Wall -Wmissing-prototypes -Wpointer-arith 
> -Wdeclaration-after-statement -Wendif-labels -Wmissing-format-attribute 
> -Wformat-security -fno-strict-aliasing -fwrapv -fexcess-precision=standard -g 
> -g -O2 -fstack-protector --param=ssp-buffer-size=4 -Wformat 
> -Werror=format-security  -I../../../../src/include  -D_FORTIFY_SOURCE=2 
> -D_GNU_SOURCE -I/usr/include/libxml2  -I/usr/include/mit-krb5  -c -o 
> generation.o generation.c
> generation.c: In function ‘GenerationContextCreate’:
> generation.c:206:7: error: static assertion failed: "padding calculation in 
> GenerationChunk is wrong"
> make[4]: *** [generation.o] Error 1
>
> Looks to me like GenerationChunk imagines that 3*sizeof(pointer)
> must be a maxaligned quantity, which is wrong on platforms where
> MAXALIGN is bigger than sizeof(pointer).

Thanks, yes.

I can't see how to resolve that without breaking the design assumption
at line 122, "there must not be any padding to reach a MAXALIGN
boundary here!".

So I'll wait for Tomas to comment.

(I'm just about to catch a long flight, so will be offline for 24
hours, so we may need to revert this before I get back if it is
difficult to resolve.)

-- 
Simon Riggshttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

pgsql: Generational memory allocator

[Simon Riggs]

Generational memory allocator

Add new style of memory allocator, known as Generational
appropriate for use in cases where memory is allocated
and then freed in roughly oldest first order (FIFO).

Use new allocator for logical decoding’s reorderbuffer
to significantly reduce memory usage and improve performance.

Author: Tomas Vondra
Reviewed-by: Simon Riggs

Branch
--
master

Details
---
https://git.postgresql.org/pg/commitdiff/a4ccc1cef5a04cc054af83bc4582a045d5232cb3

Modified Files
--
src/backend/replication/logical/reorderbuffer.c |  80 +--
src/backend/utils/mmgr/Makefile |   2 +-
src/backend/utils/mmgr/README   |  23 +
src/backend/utils/mmgr/generation.c | 768 
src/include/nodes/memnodes.h|   4 +-
src/include/nodes/nodes.h   |   1 +
src/include/replication/reorderbuffer.h |  15 +-
src/include/utils/memutils.h|   5 +
8 files changed, 819 insertions(+), 79 deletions(-)

Re: [COMMITTERS] pgsql: Remove secondary checkpoint

[Simon Riggs]

On 20 November 2017 at 15:55, Andres Freund <and...@anarazel.de> wrote:
> On 2017-11-20 15:50:40 -0500, Simon Riggs wrote:
>> On 20 November 2017 at 08:38, Amit Kapila <amit.kapil...@gmail.com> wrote:
>> > Your patch looks correct to me.  I can reproduce the problem and
>> > verified that patch fixes the problem.  It is better to track this in
>> > CF if not already tracked.
>>
>> What email and patch is this referring to?
>
> https://www.postgresql.org/message-id/878tfcsp1t@ansel.ydns.eu

Apologies to Andreas, I didn't receive that email

-- 
Simon Riggshttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

pgsql: Fix pg_control_checkpoint from commit 4b0d28de06

[Simon Riggs]

Fix pg_control_checkpoint from commit 4b0d28de06

Author: Simon Riggs <si...@2ndquadrant.com>
Reported-By: Andreas Seltenreich <seltenre...@gmx.de>

Branch
--
master

Details
---
https://git.postgresql.org/pg/commitdiff/2ede45c3a49e484edfa143850d55eb32dba296de

Modified Files
--
src/backend/utils/misc/pg_controldata.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

Re: [COMMITTERS] pgsql: Remove secondary checkpoint

[Simon Riggs]

On 20 November 2017 at 08:38, Amit Kapila <amit.kapil...@gmail.com> wrote:
> On Sat, Nov 11, 2017 at 10:58 PM, Andreas Seltenreich
> <seltenre...@gmx.de> wrote:
>> Hi,
>>
>> sqlsmith doesn't like commit 4b0d28de06:
>>
>> ,
>> | regression=> select * from pg_control_checkpoint();
>> | server closed the connection unexpectedly
>> | TRAP: FailedAssertion("!((atti->attalign) == 's')", File: "heaptuple.c", 
>> Line: 126)
>> `
>>
>> On a build with assertions disabled, the statement fails with an error
>> instead:
>>
>> ,
>> | regression=> select * from pg_control_checkpoint();
>> | ERROR:  function return row and query-specified return row do not match
>> | DETAIL:  Returned row contains 19 attributes, but query expects 18.
>> `
>>
>> The attached patch fixes it for me.
>>
>
>
> Your patch looks correct to me.  I can reproduce the problem and
> verified that patch fixes the problem.  It is better to track this in
> CF if not already tracked.

What email and patch is this referring to?

-- 
Simon Riggshttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Re: pgsql: Parameter toast_tuple_target controls TOAST for new rows

[Simon Riggs]

On 19 November 2017 at 20:30, Tom Lane <t...@sss.pgh.pa.us> wrote:
> Andres Freund <and...@anarazel.de> writes:
>> On 2017-11-19 19:49:01 -0500, Tom Lane wrote:
>>> TBH, I would just remove those test cases.  Even if they were stable
>>> across platforms, they don't directly prove anything at all about
>>> whether the feature does what it's supposed to.
>
>> I think it might make sense to rewrite the tests so it doesn't output
>> any of the sizes, but instead just compares the size of tables with
>> different thresholds. That should be fairly reliable.
>
> Hm, what I'd try after a bit of thought is to stuff the same data
> into two different tables with different settings chosen to make it
> TOAST or not, and then just test the toast tables for zero or nonzero
> size.

Roughly that.

> Setting STORAGE = EXTERNAL on the data column to disable
> compression would help make things more stable, too.

That was already set at line 350

-- 
Simon Riggshttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Re: pgsql: Parameter toast_tuple_target controls TOAST for new rows

[Simon Riggs]

On 19 November 2017 at 19:52, Andres Freund <and...@anarazel.de> wrote:
> On 2017-11-19 19:49:01 -0500, Tom Lane wrote:
>> Andres Freund <and...@anarazel.de> writes:
>> > On 2017-11-19 19:08:48 -0500, Simon Riggs wrote:
>> >> Am investigating the few buildfarm failures
>>
>> > The tests look very sensitive to differences in tuple size due to
>> > different alignment requirements. Dependant on what MAXALIGN (and some
>> > others) is the number of tuples fitting on a page will differ.
>>
>> "Few" buildfarm failures?  It's probably going to fail on every 32-bit host.
>>
>> TBH, I would just remove those test cases.  Even if they were stable
>> across platforms, they don't directly prove anything at all about
>> whether the feature does what it's supposed to.
>>
>> (Also, scaling the results to blocksize seems unlikely to help in
>> passing on different BLCKSZ configurations...)
>
> I think it might make sense to rewrite the tests so it doesn't output
> any of the sizes, but instead just compares the size of tables with
> different thresholds. That should be fairly reliable.

Thanks for your input.

-- 
Simon Riggshttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Re: pgsql: Parameter toast_tuple_target controls TOAST for new rows

[Simon Riggs]

On 19 November 2017 at 19:49, Tom Lane <t...@sss.pgh.pa.us> wrote:
> Andres Freund <and...@anarazel.de> writes:
>> On 2017-11-19 19:08:48 -0500, Simon Riggs wrote:
>>> Am investigating the few buildfarm failures
>
>> The tests look very sensitive to differences in tuple size due to
>> different alignment requirements. Dependant on what MAXALIGN (and some
>> others) is the number of tuples fitting on a page will differ.
>
> "Few" buildfarm failures?  It's probably going to fail on every 32-bit host.
>
> TBH, I would just remove those test cases.  Even if they were stable
> across platforms, they don't directly prove anything at all about
> whether the feature does what it's supposed to.
>
> (Also, scaling the results to blocksize seems unlikely to help in
> passing on different BLCKSZ configurations...)

I just committed a change while this email arrived. I think what I've
done is essentially this.

The original test was more about demonstrating effectiveness.

-- 
Simon Riggshttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

pgsql: Reduce test variability for toast_tuple_target test

[Simon Riggs]

Reduce test variability for toast_tuple_target test

Branch
--
master

Details
---
https://git.postgresql.org/pg/commitdiff/56f34686220731eef72dfd129519b25f28406db1

Modified Files
--
src/test/regress/expected/strings.out | 50 ++-
src/test/regress/sql/strings.sql  | 26 +-
2 files changed, 26 insertions(+), 50 deletions(-)

Re: pgsql: Parameter toast_tuple_target controls TOAST for new rows

[Simon Riggs]

On 19 November 2017 at 17:52, Simon Riggs <si...@2ndquadrant.com> wrote:
> Parameter toast_tuple_target controls TOAST for new rows
>
> Specifies the point at which we try to move long column values
> into TOAST tables.
>
> No effect on existing rows.
>
> Discussion: 
> https://postgr.es/m/CANP8+jKsVmw6CX6YP9z7zqkTzcKV1+Uzr3XjKcZW=2ya00o...@mail.gmail.com
>
> Author: Simon Riggs <si...@2ndqudrant.com>
> Reviewed-by: Andrew Dunstan <andrew.duns...@2ndquadrant.com>
>
> Branch
> --
> master
>
> Details
> ---
> https://git.postgresql.org/pg/commitdiff/c2513365a0a85e77d3c21adb92fe12cfbe0d1897
>
> Modified Files
> --
> doc/src/sgml/ref/alter_table.sgml  |  2 +-
> doc/src/sgml/ref/create_table.sgml | 21 +++
> src/backend/access/common/reloptions.c | 12 +
> src/backend/access/heap/tuptoaster.c   |  2 +-
> src/include/utils/rel.h|  9 +++
> src/test/regress/expected/strings.out  | 47 ++
> src/test/regress/sql/strings.sql   | 19 ++
> 7 files changed, 110 insertions(+), 2 deletions(-)

Am investigating the few buildfarm failures

-- 
Simon Riggshttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

pgsql: Parameter toast_tuple_target controls TOAST for new rows

[Simon Riggs]

Parameter toast_tuple_target controls TOAST for new rows

Specifies the point at which we try to move long column values
into TOAST tables.

No effect on existing rows.

Discussion: 
https://postgr.es/m/CANP8+jKsVmw6CX6YP9z7zqkTzcKV1+Uzr3XjKcZW=2ya00o...@mail.gmail.com

Author: Simon Riggs <si...@2ndqudrant.com>
Reviewed-by: Andrew Dunstan <andrew.duns...@2ndquadrant.com>

Branch
--
master

Details
---
https://git.postgresql.org/pg/commitdiff/c2513365a0a85e77d3c21adb92fe12cfbe0d1897

Modified Files
--
doc/src/sgml/ref/alter_table.sgml  |  2 +-
doc/src/sgml/ref/create_table.sgml | 21 +++
src/backend/access/common/reloptions.c | 12 +
src/backend/access/heap/tuptoaster.c   |  2 +-
src/include/utils/rel.h|  9 +++
src/test/regress/expected/strings.out  | 47 ++
src/test/regress/sql/strings.sql   | 19 ++
7 files changed, 110 insertions(+), 2 deletions(-)