agora inbox for pgsql-hackers@postgresql.org  
help / color / mirror / Atom feed
Re: Database Kernels and O_DIRECT
18+ messages / 12 participants
[nested] [flat]

* Re: Database Kernels and O_DIRECT
@ 2003-10-14 19:59 James Rogers <jamesr@best.com>
  2003-10-15 03:26 ` Re: Database Kernels and O_DIRECT Greg Stark <gsstark@mit.edu>
  0 siblings, 1 reply; 18+ messages in thread

From: James Rogers @ 2003-10-14 19:59 UTC (permalink / raw)
  To: pgsql-hackers

On Sun, 2003-10-12 at 15:13, Greg Stark wrote:
> There's an interesting thread on linux-kernel right now about O_DIRECT and the
> kernel i/o APIs databases need. I noticed a connection between what they were
> discussing and the earlier discussions here and the pining for an interface to
> avoid having vacuum preempt other disk i/o.
>
> Someone from Oracle is on there explaining what Oracle's needs are. Perhaps
> someone more knowledgable than myself could explain what would most help
> postgres in this area.


There is an important difference between Oracle and Postgres that makes
discussions of this complicated because the assumptions are different.

Oracle runs on top of a database kernel, whereas Postgres does not.  In
the former case, it is very useful and conducive to better performance
to have O_DIRECT and direct control of the I/O in general -- the more,
the better.  In the latter case (e.g. Postgres), it is more of a
nuisance and difficult to exploit well.

The point of having a database kernel underneath the DBMS is two-fold.  

First, it improves portability by acting as an operating system
abstraction layer, replacing OS kernel services with its own equivalents
(which may map to any number of mechanisms underneath).  It is the
reason Oracle is easily supported on so many operating systems; to port
to a new OS, they only have to modify the database kernel, and they
probably have a highly portable generic version to start with that they
can then optimize for a given platform at their leisure. All the rest of
Oracle's code only has to compile against and run on the virtual
operating system that is their database kernel.

Second, where possible, the database kernel bypasses the OS kernel
internally (e.g. O_DIRECT) and implements its own versions of the OS
kernel services that are highly-tuned for database purposes. This often
has significant performance benefits.  While it kind of looks like an OS
on top of an OS, well-written database kernels often tend to exist
almost parallel the system kernel in certain respects, only using the
system kernel where it is convenient or for future capabilities that
have been stubbed out in the database kernel.  Writing DBMS code to a
database kernel almost always produces a more scalable system than
writing to portable OS APIs because it eliminates the "lowest common
denominator" effect.

Having a database kernel isn't really important unless you are a
performance junkie or have to address really scalable database systems. 
Some more advanced DBMS features are easier to implement on a database
kernel as a pragmatic concern, because the system model being
implemented for is more database friendly. It lets the database take
advantage of the more advanced features and optimizations of whatever
operating system it is running on without the vast majority of the DBMS
code base being aware of these significant differences.

I'd like to see Postgres move to a database kernel eventually for a lot
of reasons, but it would a relatively significant change. Maybe v8? :-)

Cheers,

-James Rogers
 jamesr@best.com
 





^ permalink  raw  reply  [nested|flat] 18+ messages in thread

* Re: Database Kernels and O_DIRECT
  2003-10-14 19:59 Re: Database Kernels and O_DIRECT James Rogers <jamesr@best.com>
@ 2003-10-15 03:26 ` Greg Stark <gsstark@mit.edu>
  2003-10-15 06:31   ` Re: Database Kernels and O_DIRECT James Rogers <jamesr@best.com>
  2003-10-15 13:09   ` Re: Database Kernels and O_DIRECT Bruce Momjian <pgman@candle.pha.pa.us>
  0 siblings, 2 replies; 18+ messages in thread

From: Greg Stark @ 2003-10-15 03:26 UTC (permalink / raw)
  To: pgsql-hackers


James Rogers <jamesr@best.com> writes:
> >
> > Someone from Oracle is on there explaining what Oracle's needs are. Perhaps
> > someone more knowledgable than myself could explain what would most help
> > postgres in this area.
> 
> 
> There is an important difference between Oracle and Postgres that makes
> discussions of this complicated because the assumptions are different.

All the more reason Postgres's view of the world should maybe be represented
there. As it turns out Linus seems unsympathetic to the O_DIRECT approach and
seems more interested in building a better kernel interface to control caching
and i/o scheduling. Something that fits better with postgres's design than
Oracle's.

> the former case, it is very useful and conducive to better performance
> to have O_DIRECT and direct control of the I/O in general -- the more,
> the better.  In the latter case (e.g. Postgres), it is more of a
> nuisance and difficult to exploit well.

Actually I think it would be useful for the WAL. As I understand it there's no
point caching the WAL and every write is going to get synced anyways so
there's no point in buffering it either. The sooner the process can find out
it's been synced the better. But I'm not really 100% up on the way the WAL is
used so I could be wrong.

> The point of having a database kernel underneath the DBMS is two-fold.  
> 
> First, it improves portability by acting as an operating system
> abstraction layer, replacing OS kernel services with its own equivalents

Bah. So Oracle has to live with whatever OS features VMS had 20 years ago. It
has to reimplement whatever I/O scheduling or other strategies it wants.
Rather than being the escape from the "lowest common denominator" it is in
fact precisely the cause of it.

You describe Postgres as if abstraction is a foreign concept to it. Much
better to have well designed minimal abstractions for each of the resources
needed, rather than trying to turn every OS you meet into the first one you
met.


-- 
greg




^ permalink  raw  reply  [nested|flat] 18+ messages in thread

* Re: Database Kernels and O_DIRECT
  2003-10-14 19:59 Re: Database Kernels and O_DIRECT James Rogers <jamesr@best.com>
  2003-10-15 03:26 ` Re: Database Kernels and O_DIRECT Greg Stark <gsstark@mit.edu>
@ 2003-10-15 06:31   ` James Rogers <jamesr@best.com>
  2003-10-15 08:26     ` Re: Database Kernels and O_DIRECT James Rogers <jamesr@best.com>
  2003-10-15 15:43     ` Re: Database Kernels and O_DIRECT Tom Lane <tgl@sss.pgh.pa.us>
  1 sibling, 2 replies; 18+ messages in thread

From: James Rogers @ 2003-10-15 06:31 UTC (permalink / raw)
  To: pgsql-hackers

On 10/14/03 8:26 PM, "Greg Stark" <gsstark@mit.edu> wrote:
> 
> All the more reason Postgres's view of the world should maybe be represented
> there. As it turns out Linus seems unsympathetic to the O_DIRECT approach and
> seems more interested in building a better kernel interface to control caching
> and i/o scheduling. Something that fits better with postgres's design than
> Oracle's.


This would certainly help Postgres as currently written, but it won't have
the theoretical performance headroom of what Oracle wants.  A practical
kernel API is too narrow to be fully aware of and exploit database state.
And then there is the portability issue...

The way you want these kinds of things implemented in an operating system
kernel are somewhat orthogonal to how you want them implemented from the
perspective of a database kernel.  Typical resource use cases for an
operating system and a database engine make pretty different assumptions and
the best you'll get is a compromise that doesn't optimize either.

Making additional optimizations to the OS kernel works great for Postgres
(on Linux, at least) because currently very little is optimized in this
regard.  Basically Linus is doing some design optimization work for us.  An
improvement, but kind of a mediocre one in the big scheme of things and not
terribly portable.  If we suddenly wanted to optimize Postgres for
performance the way Oracle does, we would be a lot more keen on the O_DIRECT
approach.

 
> Actually I think it would be useful for the WAL. As I understand it there's no
> point caching the WAL and every write is going to get synced anyways so
> there's no point in buffering it either. The sooner the process can find out
> it's been synced the better. But I'm not really 100% up on the way the WAL is
> used so I could be wrong.


Aye, I think you may be correct.

 
> Bah. So Oracle has to live with whatever OS features VMS had 20 years ago. It
> has to reimplement whatever I/O scheduling or other strategies it wants.
> Rather than being the escape from the "lowest common denominator" it is in
> fact precisely the cause of it.


You appear to have completely missed the point.

The point of the abstraction layer is so they can optimize the hell out of
the database for every single platform they support without having to
rewrite a bunch of the database every time.  The database kernel API is
BETTER AND MORE OPTIMAL than the operating system API. It allows them to use
whatever memory management scheme, I/O scheme, etc is the best for every
single platform.  If "the best" happens to going to the native OS service,
then that is what they do, but most of the code doesn't need to know this if
the abstraction layer is well-designed.

Most of the code in a DBMS does not care where memory comes from, how its
managed, what the file system actually looks like, or how I/O is done.  As
long as the behavior is the same from the database kernel API it is writing
to, it is all good.  What this means from a practical standpoint is that you
don't *have* to use SysV IPC on every platform, or POSIX, or mmap, or
whatever.  You can use whatever that particular platform likes as long it
can be mapped into the database kernel API, which tends to be at a high
enough level that just about *any* reasonable implementation of an OS API
can be mapped into it with quite a bit of optimization.


> You describe Postgres as if abstraction is a foreign concept to it. Much
> better to have well designed minimal abstractions for each of the resources
> needed, rather than trying to turn every OS you meet into the first one you
> met.
 

You have a serious misconception of what a database kernel is and looks
like.

A database kernel doesn't look like the OS kernel that is mapped to it.  You
write a database kernel API that is idealized for database usage and
provides services specifically designed for the needs of a database.  It is
a high-level API, not a mirror copy of standard OS APIs; if you did that,
you wouldn't have any room to do the database kernel implementation.  You
then build an implementation of the API on the local system using whatever
operating system interfaces suit your fancy.  The API is simple enough and
small enough that this isn't particularly difficult to do in a typical case.
And you can write a default kernel that is portable "as is" to most
operating systems.

There is some abstraction in Postgres and the database is well-written, but
it isn't written in a manner that makes it easy to swap out operating system
or API models.  It is written to be portable at all levels.  A database
kernel isn't necessarily required to be portable at the very lowest level,
but it is vastly more optimizable because you aren't forced into a narrow
set of choices for interfacing with the operating system.

Operating system APIs are not particularly well-suited for databases, and if
you force a database to adhere to operating system APIs directly, you end up
with a suboptimal situation almost every single time.  You end with
implementations that you never would have done if you were targeting the
database for only that platform.  Using a database kernel lets you make
platform specific optimizations and API selections without forcing most of
the database code to be aware of it.

Perhaps more to the point, who gives a damn what optimizations Linus puts in
the Linux kernel.  What good does that do Postgres users on FreeBSD, or OSX,
or Windows?  Abstracting a database engine to a set of operating system APIs
is never going to give stellar or even results across all platforms because
the operating system APIs usually aren't written so that you could write
your database optimally.

Theoretically, it is the difference between middling performance in the
typical case and highly optimal in just about every case.  A database kernel
lets you use an operating system in the way it likes to be used rather than
using an API that you just happen to support.

Cheers,

-James Rogers
 jamesr@best.com








^ permalink  raw  reply  [nested|flat] 18+ messages in thread

* Re: Database Kernels and O_DIRECT
  2003-10-14 19:59 Re: Database Kernels and O_DIRECT James Rogers <jamesr@best.com>
  2003-10-15 03:26 ` Re: Database Kernels and O_DIRECT Greg Stark <gsstark@mit.edu>
  2003-10-15 06:31   ` Re: Database Kernels and O_DIRECT James Rogers <jamesr@best.com>
@ 2003-10-15 08:26     ` James Rogers <jamesr@best.com>
  2003-10-15 21:12       ` Re: Database Kernels and O_DIRECT Hannu Krosing <hannu@tm.ee>
  1 sibling, 1 reply; 18+ messages in thread

From: James Rogers @ 2003-10-15 08:26 UTC (permalink / raw)
  To: pgsql-hackers

On 10/14/03 11:31 PM, "James Rogers" <jamesr@best.com> wrote:
> 
> There is some abstraction in Postgres and the database is well-written, but
> it isn't written in a manner that makes it easy to swap out operating system
> or API models.  It is written to be portable at all levels.  A database
> kernel isn't necessarily required to be portable at the very lowest level,
> but it is vastly more optimizable because you aren't forced into a narrow
> set of choices for interfacing with the operating system.


Just to clarify, my post wasn't really to say that we should run out and
make Postgres use a database kernel type internal model tomorrow.  The point
of all that was that Oracle does things that way for a very good reason and
that there can be benefits that may not be immediately obvious.

It is really one of those emergent "needs" when a database engine gets to a
certain level of sophistication.  For smaller and simpler databases, you
don't really need it and the effort isn't justified.  At some point, you
cross a threshold where not only does it become justified but it becomes a
wise idea or not having it will start to punish you in a number of different
ways.  I personally think that Postgres is sitting on the cusp of "its a
wise idea", and that it is something worth thinking about in the future.

Cheers,

-James Rogers
 jamesr@best.com




^ permalink  raw  reply  [nested|flat] 18+ messages in thread

* Re: Database Kernels and O_DIRECT
  2003-10-14 19:59 Re: Database Kernels and O_DIRECT James Rogers <jamesr@best.com>
  2003-10-15 03:26 ` Re: Database Kernels and O_DIRECT Greg Stark <gsstark@mit.edu>
  2003-10-15 06:31   ` Re: Database Kernels and O_DIRECT James Rogers <jamesr@best.com>
  2003-10-15 08:26     ` Re: Database Kernels and O_DIRECT James Rogers <jamesr@best.com>
@ 2003-10-15 21:12       ` Hannu Krosing <hannu@tm.ee>
  0 siblings, 0 replies; 18+ messages in thread

From: Hannu Krosing @ 2003-10-15 21:12 UTC (permalink / raw)
  To: James Rogers <jamesr@best.com>; +Cc: pgsql-hackers

James Rogers kirjutas K, 15.10.2003 kell 11:26:
> On 10/14/03 11:31 PM, "James Rogers" <jamesr@best.com> wrote:
> > 
> > There is some abstraction in Postgres and the database is well-written, but
> > it isn't written in a manner that makes it easy to swap out operating system
> > or API models.  It is written to be portable at all levels.  A database
> > kernel isn't necessarily required to be portable at the very lowest level,
> > but it is vastly more optimizable because you aren't forced into a narrow
> > set of choices for interfacing with the operating system.
> 
> 
> Just to clarify, my post wasn't really to say that we should run out and
> make Postgres use a database kernel type internal model tomorrow.  The point
> of all that was that Oracle does things that way for a very good reason and
> that there can be benefits that may not be immediately obvious.

OTOH, what may be a perfectly good reason for Oracle, may not be it for
PostgreSQL.

For me the beauty of OS software has always been the possibility to fix
problems at the right level (kernel, library, language) , and not to
just make workarounds at another level (your application).

So getting some API's into kernel for optimizing cache usage or
writeback strategies would be much better than using raw writes and
rewriting the whole thing ourseleves. 

The newer linux kernels have several schedulers to choose from, why not
push for choice in other areas as well.

The ultimate "database kernel" could thus be a custom tuned linux kernel
;)

> It is really one of those emergent "needs" when a database engine gets to a
> certain level of sophistication.  For smaller and simpler databases, you
> don't really need it and the effort isn't justified.  At some point, you
> cross a threshold where not only does it become justified but it becomes a
> wise idea or not having it will start to punish you in a number of different
> ways.  I personally think that Postgres is sitting on the cusp of "its a
> wise idea", and that it is something worth thinking about in the future.

This thread reminds me of Linus/Tannenbaum Monolithic vs. Microkernel
argument - while theoretically Microkernels are "better" Linux could
outperform it by having the required modularity on source level, and
being an open-source project this was enough. It also beat the Mach
kernel by being there whereas microkernel based mach was too hard to
develop/debug and thus has taken way longer to mature.

--------------
Hannu




^ permalink  raw  reply  [nested|flat] 18+ messages in thread

* Re: Database Kernels and O_DIRECT
  2003-10-14 19:59 Re: Database Kernels and O_DIRECT James Rogers <jamesr@best.com>
  2003-10-15 03:26 ` Re: Database Kernels and O_DIRECT Greg Stark <gsstark@mit.edu>
  2003-10-15 06:31   ` Re: Database Kernels and O_DIRECT James Rogers <jamesr@best.com>
@ 2003-10-15 15:43     ` Tom Lane <tgl@sss.pgh.pa.us>
  2003-10-15 21:02       ` Re: Database Kernels and O_DIRECT Andrew Dunstan <andrew@dunslane.net>
  2003-10-16 00:49       ` Re: Database Kernels and O_DIRECT Sailesh Krishnamurthy <sailesh@cs.berkeley.edu>
  2003-10-26 04:12       ` Re: Database Kernels and O_DIRECT Bruce Momjian <pgman@candle.pha.pa.us>
  1 sibling, 3 replies; 18+ messages in thread

From: Tom Lane @ 2003-10-15 15:43 UTC (permalink / raw)
  To: James Rogers <jamesr@best.com>; +Cc: pgsql-hackers

James Rogers <jamesr@best.com> writes:
> If we suddenly wanted to optimize Postgres for performance the way
> Oracle does, we would be a lot more keen on the O_DIRECT approach.

This isn't ever going to happen, for the simple reason that we don't
have Oracle's manpower.  You are blithely throwing around the phrase
"database kernel" like it would be a small simple project.  In reality
you are talking about (at least) implementing our own complete
filesystem, and then doing it over again on every platform we want to
support, and then after that, optimizing it to the point of actually
being enough better than the native facilities to have been worth the
effort.  I cannot conceive of that happening in a Postgres project that
even remotely resembles the present reality, because we just don't have
the manpower; and what manpower we do have is better spent on other
tasks.  We have other things to do than re-invent the operating system
wheel.  Improving the planner, for example.

One of the first concepts I learned in CS grad school was that of
optimizing a system at multiple levels.  If the hardware guys can build
a 2X faster CPU, and the operating system guys can find a 2X improvement
in (say) filesystem performance, and then the application guys can find
a 2X improvement in their algorithms, you've got 8X total speedup, which
might have been impossible or at least vastly harder to get by working
at only one level of the system.  The lesson for Postgres is that we
should not be trying to beat the operating system guys at their own
game.  It's unclear that we can anyway, and we can certainly get more
bang for our optimization buck by working at system levels that don't
correspond to operating-system concerns.

I tend to agree with the opinion that Oracle's architecture is based on
twenty-year-old assumptions.  Back then it was reasonable to assume that
database-specific algorithms could outperform a general-purpose
operating system.  In today's environment that assumption is not a given.

			regards, tom lane



^ permalink  raw  reply  [nested|flat] 18+ messages in thread

* Re: Database Kernels and O_DIRECT
  2003-10-14 19:59 Re: Database Kernels and O_DIRECT James Rogers <jamesr@best.com>
  2003-10-15 03:26 ` Re: Database Kernels and O_DIRECT Greg Stark <gsstark@mit.edu>
  2003-10-15 06:31   ` Re: Database Kernels and O_DIRECT James Rogers <jamesr@best.com>
  2003-10-15 15:43     ` Re: Database Kernels and O_DIRECT Tom Lane <tgl@sss.pgh.pa.us>
@ 2003-10-15 21:02       ` Andrew Dunstan <andrew@dunslane.net>
  2003-10-16 05:51         ` Re: Database Kernels and O_DIRECT Manfred Spraul <manfred@colorfullife.com>
  2003-10-16 14:31         ` Re: Database Kernels and O_DIRECT Christopher Browne <cbbrowne@libertyrms.info>
  2 siblings, 2 replies; 18+ messages in thread

From: Andrew Dunstan @ 2003-10-15 21:02 UTC (permalink / raw)
  To: ; +Cc: pgsql-hackers

Tom Lane wrote:

>James Rogers <jamesr@best.com> writes:
>  
>
>>If we suddenly wanted to optimize Postgres for performance the way
>>Oracle does, we would be a lot more keen on the O_DIRECT approach.
>>    
>>
>
>This isn't ever going to happen, for the simple reason that we don't
>have Oracle's manpower.  
>
[snip - long and sensible elaboration of above statement]

I have wondered (somewhat fruitlessly) for several years about the 
possibilities of special purpose lightweight file systems that could 
relax some of the assumptions and checks used in general purpose file 
systems. Such a thing might provide most of the benefits of a "database 
kernel" without imposing anything extra on the database application layer.

Just a thought - I have no resources to make any attack on such a project.

cheers

andrew




^ permalink  raw  reply  [nested|flat] 18+ messages in thread

* Re: Database Kernels and O_DIRECT
  2003-10-14 19:59 Re: Database Kernels and O_DIRECT James Rogers <jamesr@best.com>
  2003-10-15 03:26 ` Re: Database Kernels and O_DIRECT Greg Stark <gsstark@mit.edu>
  2003-10-15 06:31   ` Re: Database Kernels and O_DIRECT James Rogers <jamesr@best.com>
  2003-10-15 15:43     ` Re: Database Kernels and O_DIRECT Tom Lane <tgl@sss.pgh.pa.us>
  2003-10-15 21:02       ` Re: Database Kernels and O_DIRECT Andrew Dunstan <andrew@dunslane.net>
@ 2003-10-16 05:51         ` Manfred Spraul <manfred@colorfullife.com>
  1 sibling, 0 replies; 18+ messages in thread

From: Manfred Spraul @ 2003-10-16 05:51 UTC (permalink / raw)
  To: Andrew Dunstan <andrew@dunslane.net>; +Cc: pgsql-hackers

Andrew Dunstan wrote:

>
> I have wondered (somewhat fruitlessly) for several years about the 
> possibilities of special purpose lightweight file systems that could 
> relax some of the assumptions and checks used in general purpose file 
> systems. Such a thing might provide most of the benefits of a 
> "database kernel" without imposing anything extra on the database 
> application layer.

CPU is usually cheap compared to disk io.

There are two things that might be worth looking into:
Oracle released their cluster filesystem (ocfs) as a GPL driver for 
Linux. It might be interesting to check how it performs if used for 
postgres, but I fear that it implicitely assumes that the bulk of the 
caching is performed by the database in user space.
And using O_DIRECT for the WAL logs - the logs are never read.

--
    Manfred





^ permalink  raw  reply  [nested|flat] 18+ messages in thread

* Re: Database Kernels and O_DIRECT
  2003-10-14 19:59 Re: Database Kernels and O_DIRECT James Rogers <jamesr@best.com>
  2003-10-15 03:26 ` Re: Database Kernels and O_DIRECT Greg Stark <gsstark@mit.edu>
  2003-10-15 06:31   ` Re: Database Kernels and O_DIRECT James Rogers <jamesr@best.com>
  2003-10-15 15:43     ` Re: Database Kernels and O_DIRECT Tom Lane <tgl@sss.pgh.pa.us>
  2003-10-15 21:02       ` Re: Database Kernels and O_DIRECT Andrew Dunstan <andrew@dunslane.net>
@ 2003-10-16 14:31         ` Christopher Browne <cbbrowne@libertyrms.info>
  1 sibling, 0 replies; 18+ messages in thread

From: Christopher Browne @ 2003-10-16 14:31 UTC (permalink / raw)
  To: pgsql-hackers

andrew@dunslane.net (Andrew Dunstan) writes:
> Tom Lane wrote:
>>James Rogers <jamesr@best.com> writes:
>>>If we suddenly wanted to optimize Postgres for performance the way
>>>Oracle does, we would be a lot more keen on the O_DIRECT approach.
>>This isn't ever going to happen, for the simple reason that we don't
>> have Oracle's manpower.
>>
> [snip - long and sensible elaboration of above statement]
>
> I have wondered (somewhat fruitlessly) for several years about the
> possibilities of special purpose lightweight file systems that could
> relax some of the assumptions and checks used in general purpose file
> systems. Such a thing might provide most of the benefits of a
> "database kernel" without imposing anything extra on the database
> application layer.
>
> Just a thought - I have no resources to make any attack on such a project.

There is an exactly relevant project for this, namely Hans Reiser's
"ReiserFS," on Linux.

http://www.namesys.com/whitepaper.html

In Version 4, they will be exporting an API that allows userspace
applications to control the use of transactional filesystem updates.

If someone were to directly build a database on top of this, one might
wind up with some sort of "ReiserSQL," which would be relatively
analagous to the "database kernel" approach.

Of course, the task would be large, and it would likely take _years_
for it to stabilize to the point of being much more than a "neat
hack."

The other neat approach that would be more relevant to PostgreSQL
would be to create a filesystem that stored data in pure blocks, with
pretty large block sizes, and low overhead for saving directory
metadata.  There isn't too terribly much interest in {a,o,m}time...
-- 
output = reverse("ofni.smrytrebil" "@" "enworbbc")
<http://dev6.int.libertyrms.com/;
Christopher Browne
(416) 646 3304 x124 (land)



^ permalink  raw  reply  [nested|flat] 18+ messages in thread

* Re: Database Kernels and O_DIRECT
  2003-10-14 19:59 Re: Database Kernels and O_DIRECT James Rogers <jamesr@best.com>
  2003-10-15 03:26 ` Re: Database Kernels and O_DIRECT Greg Stark <gsstark@mit.edu>
  2003-10-15 06:31   ` Re: Database Kernels and O_DIRECT James Rogers <jamesr@best.com>
  2003-10-15 15:43     ` Re: Database Kernels and O_DIRECT Tom Lane <tgl@sss.pgh.pa.us>
@ 2003-10-16 00:49       ` Sailesh Krishnamurthy <sailesh@cs.berkeley.edu>
  2 siblings, 0 replies; 18+ messages in thread

From: Sailesh Krishnamurthy @ 2003-10-16 00:49 UTC (permalink / raw)
  To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: James Rogers <jamesr@best.com>; pgsql-hackers

>>>>> "Tom" == Tom Lane <tgl@sss.pgh.pa.us> writes:

    Tom> I tend to agree with the opinion that Oracle's architecture
    Tom> is based on twenty-year-old assumptions.  Back then it was
    Tom> reasonable to assume that database-specific algorithms could
    Tom> outperform a general-purpose operating system.  In today's
    Tom> environment that assumption is not a given.


In fact: 

   Michael Stonebraker: Operating System Support for Database Management. 
   CACM 24(7): 412-418 (1981)

   Abstract: 

             Several operating system services are examined with a
             view toward their applicability to support of database
             management functions. These services include buffer pool
             management; the file system; scheduling, process
             management, and interprocess communication; and
             consistency control.

-- 
Pip-pip
Sailesh
http://www.cs.berkeley.edu/~sailesh





^ permalink  raw  reply  [nested|flat] 18+ messages in thread

* Re: Database Kernels and O_DIRECT
  2003-10-14 19:59 Re: Database Kernels and O_DIRECT James Rogers <jamesr@best.com>
  2003-10-15 03:26 ` Re: Database Kernels and O_DIRECT Greg Stark <gsstark@mit.edu>
  2003-10-15 06:31   ` Re: Database Kernels and O_DIRECT James Rogers <jamesr@best.com>
  2003-10-15 15:43     ` Re: Database Kernels and O_DIRECT Tom Lane <tgl@sss.pgh.pa.us>
@ 2003-10-26 04:12       ` Bruce Momjian <pgman@candle.pha.pa.us>
  2 siblings, 0 replies; 18+ messages in thread

From: Bruce Momjian @ 2003-10-26 04:12 UTC (permalink / raw)
  To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: James Rogers <jamesr@best.com>; pgsql-hackers

Tom Lane wrote:
> James Rogers <jamesr@best.com> writes:
> > If we suddenly wanted to optimize Postgres for performance the way
> > Oracle does, we would be a lot more keen on the O_DIRECT approach.
> 
> This isn't ever going to happen, for the simple reason that we don't
> have Oracle's manpower.  You are blithely throwing around the phrase
> "database kernel" like it would be a small simple project.  In reality
> you are talking about (at least) implementing our own complete
> filesystem, and then doing it over again on every platform we want to
> support, and then after that, optimizing it to the point of actually
> being enough better than the native facilities to have been worth the
> effort.  I cannot conceive of that happening in a Postgres project that
> even remotely resembles the present reality, because we just don't have
> the manpower; and what manpower we do have is better spent on other
> tasks.  We have other things to do than re-invent the operating system
> wheel.  Improving the planner, for example.

One question is what a database kernel would look like?  Would it
basically mean just taking our existing portability code, such as for
shared memory, and moving it into a separate libary with its own API? 
Don't we almost have that already?

I am just confused what would be different?  I think the only major
difference I have heard is to bypass the OS file system and memory
management.  We already bypass most of the memory management by using
palloc.

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073



^ permalink  raw  reply  [nested|flat] 18+ messages in thread

* Re: Database Kernels and O_DIRECT
  2003-10-14 19:59 Re: Database Kernels and O_DIRECT James Rogers <jamesr@best.com>
  2003-10-15 03:26 ` Re: Database Kernels and O_DIRECT Greg Stark <gsstark@mit.edu>
@ 2003-10-15 13:09   ` Bruce Momjian <pgman@candle.pha.pa.us>
  2003-10-15 14:14     ` Re: Database Kernels and O_DIRECT Paulo Scardine <paulos@cimed.ind.br>
  1 sibling, 1 reply; 18+ messages in thread

From: Bruce Momjian @ 2003-10-15 13:09 UTC (permalink / raw)
  To: Greg Stark <gsstark@mit.edu>; +Cc: pgsql-hackers

Greg Stark wrote:
> 
> James Rogers <jamesr@best.com> writes:
> > >
> > > Someone from Oracle is on there explaining what Oracle's needs are. Perhaps
> > > someone more knowledgable than myself could explain what would most help
> > > postgres in this area.
> > 
> > 
> > There is an important difference between Oracle and Postgres that makes
> > discussions of this complicated because the assumptions are different.
> 
> All the more reason Postgres's view of the world should maybe be represented
> there. As it turns out Linus seems unsympathetic to the O_DIRECT approach and
> seems more interested in building a better kernel interface to control caching
> and i/o scheduling. Something that fits better with postgres's design than
> Oracle's.

Of course, the big question is why Oracle is even there talking to
Linus, and Linus isn't asking to get PostgreSQL involved.  If you are
running an open-source project, you would think you would give favor to
other open-source projects.  Same with MySQL favortism --- if you are
writing an open-source tool, why favor a database developed/controlled
by a single company?

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073



^ permalink  raw  reply  [nested|flat] 18+ messages in thread

* Re: Database Kernels and O_DIRECT
  2003-10-14 19:59 Re: Database Kernels and O_DIRECT James Rogers <jamesr@best.com>
  2003-10-15 03:26 ` Re: Database Kernels and O_DIRECT Greg Stark <gsstark@mit.edu>
  2003-10-15 13:09   ` Re: Database Kernels and O_DIRECT Bruce Momjian <pgman@candle.pha.pa.us>
@ 2003-10-15 14:14     ` Paulo Scardine <paulos@cimed.ind.br>
  0 siblings, 0 replies; 18+ messages in thread

From: Paulo Scardine @ 2003-10-15 14:14 UTC (permalink / raw)
  To: pgsql-hackers

> Of course, the big question is why Oracle is even there talking to
> Linus, and Linus isn't asking to get PostgreSQL involved.  If you are
> running an open-source project, you would think you would give favor to
> other open-source projects.  Same with MySQL favortism --- if you are
> writing an open-source tool, why favor a database developed/controlled
> by a single company?

It's the unix style: no message, no error... If Postgres developers do not
send any message to Linus he will think Linux is doing just fine for them.

Seems that Oracle cares to improve their Linux port so they asked Linus some
features. I doubt Linus runned to Oracle asking "please, how could I help
you improve your closed software project?". Kernel folks seems to be very
busy people.

IMHO if we see any window for improvement in any OS, we should go to Linus
(or Peter or Bill Gates) and ask for it. As wrote in the original post.

Regards,
--
Paulo Scardine





^ permalink  raw  reply  [nested|flat] 18+ messages in thread

* [PATCH 6/8] Add psql, pg_dump and pg_upgrade support
@ 2018-06-18 12:57 Ildus Kurbangaliev <i.kurbangaliev@gmail.com>
  0 siblings, 0 replies; 18+ messages in thread

From: Ildus Kurbangaliev @ 2018-06-18 12:57 UTC (permalink / raw)

Signed-off-by: Ildus Kurbangaliev <i.kurbangaliev@gmail.com>
---
 src/backend/commands/compressioncmds.c     |  80 ++++++---
 src/backend/commands/tablecmds.c           |  14 +-
 src/backend/utils/adt/pg_upgrade_support.c |  10 ++
 src/bin/pg_dump/pg_backup.h                |   2 +
 src/bin/pg_dump/pg_dump.c                  | 200 ++++++++++++++++++++-
 src/bin/pg_dump/pg_dump.h                  |  17 ++
 src/bin/pg_dump/pg_dumpall.c               |   5 +
 src/bin/pg_dump/pg_restore.c               |   3 +
 src/bin/pg_dump/t/002_pg_dump.pl           |  95 ++++++++++
 src/bin/psql/describe.c                    |  42 +++++
 src/bin/psql/tab-complete.c                |   5 +-
 src/include/catalog/binary_upgrade.h       |   2 +
 src/include/catalog/pg_proc.dat            |   4 +
 13 files changed, 434 insertions(+), 45 deletions(-)

diff --git a/src/backend/commands/compressioncmds.c b/src/backend/commands/compressioncmds.c
index e1b41964f2..f3a5a1f7fb 100644
--- a/src/backend/commands/compressioncmds.c
+++ b/src/backend/commands/compressioncmds.c
@@ -36,6 +36,9 @@
 #include "utils/syscache.h"
 #include "utils/snapmgr.h"
 
+/* Set by pg_upgrade_support functions */
+Oid			binary_upgrade_next_attr_compression_oid = InvalidOid;
+
 /*
  * When conditions of compression satisfies one if builtin attribute
  * compresssion tuples the compressed attribute will be linked to
@@ -129,11 +132,12 @@ lookup_attribute_compression(Oid attrelid, AttrNumber attnum,
 					tup_amoid;
 		Datum		values[Natts_pg_attr_compression];
 		bool		nulls[Natts_pg_attr_compression];
+		char	   *amname;
 
 		heap_deform_tuple(tuple, RelationGetDescr(rel), values, nulls);
 		acoid = DatumGetObjectId(values[Anum_pg_attr_compression_acoid - 1]);
-		tup_amoid = get_am_oid(
-							   NameStr(*DatumGetName(values[Anum_pg_attr_compression_acname - 1])), false);
+		amname = NameStr(*DatumGetName(values[Anum_pg_attr_compression_acname - 1]));
+		tup_amoid = get_am_oid(amname, false);
 
 		if (previous_amoids)
 			*previous_amoids = list_append_unique_oid(*previous_amoids, tup_amoid);
@@ -150,17 +154,15 @@ lookup_attribute_compression(Oid attrelid, AttrNumber attnum,
 			if (DatumGetPointer(acoptions) == NULL)
 				result = acoid;
 		}
-		else
+		else if (DatumGetPointer(acoptions) != NULL)
 		{
 			bool		equal;
 
 			/* check if arrays for WITH options are equal */
 			equal = DatumGetBool(CallerFInfoFunctionCall2(
-														  array_eq,
-														  &arrayeq_info,
-														  InvalidOid,
-														  acoptions,
-														  values[Anum_pg_attr_compression_acoptions - 1]));
+						array_eq, &arrayeq_info, InvalidOid, acoptions,
+						values[Anum_pg_attr_compression_acoptions - 1]));
+
 			if (equal)
 				result = acoid;
 		}
@@ -227,6 +229,16 @@ CreateAttributeCompression(Form_pg_attribute att,
 	/* Try to find builtin compression first */
 	acoid = lookup_attribute_compression(0, 0, amoid, arropt, NULL);
 
+	/* no rewrite by default */
+	if (need_rewrite != NULL)
+		*need_rewrite = false;
+
+	if (IsBinaryUpgrade)
+	{
+		/* Skip the rewrite checks and searching of identical compression */
+		goto add_tuple;
+	}
+
 	/*
 	 * attrelid will be invalid on CREATE TABLE, no need for table rewrite
 	 * check.
@@ -252,16 +264,10 @@ CreateAttributeCompression(Form_pg_attribute att,
 		 */
 		if (need_rewrite != NULL)
 		{
-			/* no rewrite by default */
-			*need_rewrite = false;
-
 			Assert(preserved_amoids != NULL);
 
 			if (compression->preserve == NIL)
-			{
-				Assert(!IsBinaryUpgrade);
 				*need_rewrite = true;
-			}
 			else
 			{
 				ListCell   *cell;
@@ -294,7 +300,7 @@ CreateAttributeCompression(Form_pg_attribute att,
 				 * In binary upgrade list will not be free since it contains
 				 * Oid of builtin compression access method.
 				 */
-				if (!IsBinaryUpgrade && list_length(previous_amoids) != 0)
+				if (list_length(previous_amoids) != 0)
 					*need_rewrite = true;
 			}
 		}
@@ -303,9 +309,6 @@ CreateAttributeCompression(Form_pg_attribute att,
 		list_free(previous_amoids);
 	}
 
-	if (IsBinaryUpgrade && !OidIsValid(acoid))
-		elog(ERROR, "could not restore attribute compression data");
-
 	/* Return Oid if we already found identical compression on this column */
 	if (OidIsValid(acoid))
 	{
@@ -315,6 +318,7 @@ CreateAttributeCompression(Form_pg_attribute att,
 		return acoid;
 	}
 
+add_tuple:
 	/* Initialize buffers for new tuple values */
 	memset(values, 0, sizeof(values));
 	memset(nulls, false, sizeof(nulls));
@@ -323,13 +327,27 @@ CreateAttributeCompression(Form_pg_attribute att,
 
 	rel = heap_open(AttrCompressionRelationId, RowExclusiveLock);
 
-	acoid = GetNewOidWithIndex(rel, AttrCompressionIndexId,
-							   Anum_pg_attr_compression_acoid);
+	if (IsBinaryUpgrade)
+	{
+		/* acoid should be found in some cases */
+		if (binary_upgrade_next_attr_compression_oid < FirstNormalObjectId &&
+			(!OidIsValid(acoid) || binary_upgrade_next_attr_compression_oid != acoid))
+			elog(ERROR, "could not link to built-in attribute compression");
+
+		acoid = binary_upgrade_next_attr_compression_oid;
+	}
+	else
+	{
+		acoid = GetNewOidWithIndex(rel, AttrCompressionIndexId,
+									Anum_pg_attr_compression_acoid);
+
+	}
+
 	if (acoid < FirstNormalObjectId)
 	{
-		/* this is database initialization */
+		/* this is built-in attribute compression */
 		heap_close(rel, RowExclusiveLock);
-		return DefaultCompressionOid;
+		return acoid;
 	}
 
 	/* we need routine only to call cmcheck function */
@@ -393,8 +411,8 @@ RemoveAttributeCompression(Oid acoid)
 /*
  * CleanupAttributeCompression
  *
- * Remove entries in pg_attr_compression except current attribute compression
- * and related with specified list of access methods.
+ * Remove entries in pg_attr_compression of the column except current
+ * attribute compression and related with specified list of access methods.
  */
 void
 CleanupAttributeCompression(Oid relid, AttrNumber attnum, List *keepAmOids)
@@ -422,9 +440,7 @@ CleanupAttributeCompression(Oid relid, AttrNumber attnum, List *keepAmOids)
 	ReleaseSysCache(attrtuple);
 
 	Assert(relid > 0 && attnum > 0);
-
-	if (IsBinaryUpgrade)
-		goto builtin_removal;
+	Assert(!IsBinaryUpgrade);
 
 	rel = heap_open(AttrCompressionRelationId, RowExclusiveLock);
 
@@ -441,7 +457,10 @@ CleanupAttributeCompression(Oid relid, AttrNumber attnum, List *keepAmOids)
 	scan = systable_beginscan(rel, AttrCompressionRelidAttnumIndexId,
 							  true, NULL, 2, key);
 
-	/* Remove attribute compression tuples and collect removed Oids to list */
+	/*
+	 * Remove attribute compression tuples and collect removed Oids
+	 * to list.
+	 */
 	while (HeapTupleIsValid(tuple = systable_getnext(scan)))
 	{
 		Form_pg_attr_compression acform;
@@ -463,7 +482,10 @@ CleanupAttributeCompression(Oid relid, AttrNumber attnum, List *keepAmOids)
 	systable_endscan(scan);
 	heap_close(rel, RowExclusiveLock);
 
-	/* Now remove dependencies */
+	/*
+	 * Now remove dependencies between attribute compression (dependent)
+	 * and column.
+	 */
 	rel = heap_open(DependRelationId, RowExclusiveLock);
 	foreach(lc, removed)
 	{
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 89d53b173e..5ef48e3005 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -756,10 +756,10 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId,
 		if (colDef->identity)
 			attr->attidentity = colDef->identity;
 
-		if (relkind == RELKIND_RELATION || relkind == RELKIND_PARTITIONED_TABLE)
+		if (!IsBinaryUpgrade &&
+			(relkind == RELKIND_RELATION || relkind == RELKIND_PARTITIONED_TABLE))
 			attr->attcompression = CreateAttributeCompression(attr,
-															  colDef->compression,
-															  NULL, NULL);
+										colDef->compression, NULL, NULL);
 		else
 			attr->attcompression = InvalidOid;
 	}
@@ -13133,14 +13133,6 @@ ATExecSetCompression(AlteredTableInfo *tab,
 	/* make changes visible */
 	CommandCounterIncrement();
 
-	/*
-	 * Normally cleanup is done in rewrite but in binary upgrade we should do
-	 * it explicitly.
-	 */
-	if (IsBinaryUpgrade)
-		CleanupAttributeCompression(RelationGetRelid(rel),
-									attnum, preserved_amoids);
-
 	ObjectAddressSet(address, AttrCompressionRelationId, acoid);
 	return address;
 }
diff --git a/src/backend/utils/adt/pg_upgrade_support.c b/src/backend/utils/adt/pg_upgrade_support.c
index b8b7777c31..1082eab4dc 100644
--- a/src/backend/utils/adt/pg_upgrade_support.c
+++ b/src/backend/utils/adt/pg_upgrade_support.c
@@ -116,6 +116,16 @@ binary_upgrade_set_next_pg_authid_oid(PG_FUNCTION_ARGS)
 	PG_RETURN_VOID();
 }
 
+Datum
+binary_upgrade_set_next_attr_compression_oid(PG_FUNCTION_ARGS)
+{
+	Oid			acoid = PG_GETARG_OID(0);
+
+	CHECK_IS_BINARY_UPGRADE;
+	binary_upgrade_next_attr_compression_oid = acoid;
+	PG_RETURN_VOID();
+}
+
 Datum
 binary_upgrade_create_empty_extension(PG_FUNCTION_ARGS)
 {
diff --git a/src/bin/pg_dump/pg_backup.h b/src/bin/pg_dump/pg_backup.h
index 42cf441aaf..11b0da8221 100644
--- a/src/bin/pg_dump/pg_backup.h
+++ b/src/bin/pg_dump/pg_backup.h
@@ -78,6 +78,7 @@ typedef struct _restoreOptions
 	int			no_publications;	/* Skip publication entries */
 	int			no_security_labels; /* Skip security label entries */
 	int			no_subscriptions;	/* Skip subscription entries */
+	int			no_compression_methods; /* Skip compression methods */
 	int			strict_names;
 
 	const char *filename;
@@ -151,6 +152,7 @@ typedef struct _dumpOptions
 	int			no_security_labels;
 	int			no_publications;
 	int			no_subscriptions;
+	int			no_compression_methods;
 	int			no_synchronized_snapshots;
 	int			no_unlogged_table_data;
 	int			serializable_deferrable;
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index f0ea83e6a9..b76e059d67 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -40,11 +40,13 @@
 #include "getopt_long.h"
 
 #include "access/attnum.h"
+#include "access/cmapi.h"
 #include "access/sysattr.h"
 #include "access/transam.h"
 #include "catalog/pg_aggregate_d.h"
 #include "catalog/pg_am_d.h"
 #include "catalog/pg_attribute_d.h"
+#include "catalog/pg_attr_compression_d.h"
 #include "catalog/pg_cast_d.h"
 #include "catalog/pg_class_d.h"
 #include "catalog/pg_default_acl_d.h"
@@ -375,6 +377,7 @@ main(int argc, char **argv)
 		{"no-synchronized-snapshots", no_argument, &dopt.no_synchronized_snapshots, 1},
 		{"no-unlogged-table-data", no_argument, &dopt.no_unlogged_table_data, 1},
 		{"no-subscriptions", no_argument, &dopt.no_subscriptions, 1},
+		{"no-compression-methods", no_argument, &dopt.no_compression_methods, 1},
 		{"no-sync", no_argument, NULL, 7},
 		{"on-conflict-do-nothing", no_argument, &dopt.do_nothing, 1},
 
@@ -842,13 +845,13 @@ main(int argc, char **argv)
 	 * We rely on dependency information to help us determine a safe order, so
 	 * the initial sort is mostly for cosmetic purposes: we sort by name to
 	 * ensure that logically identical schemas will dump identically.
+	 *
+	 * If we do a parallel dump, we want the largest tables to go first.
 	 */
-	sortDumpableObjectsByTypeName(dobjs, numObjs);
-
-	/* If we do a parallel dump, we want the largest tables to go first */
 	if (archiveFormat == archDirectory && numWorkers > 1)
 		sortDataAndIndexObjectsBySize(dobjs, numObjs);
 
+	sortDumpableObjectsByTypeName(dobjs, numObjs);
 	sortDumpableObjects(dobjs, numObjs,
 						boundaryObjs[0].dumpId, boundaryObjs[1].dumpId);
 
@@ -8133,9 +8136,12 @@ getTableAttrs(Archive *fout, TableInfo *tblinfo, int numTables)
 	int			i_attcollation;
 	int			i_attfdwoptions;
 	int			i_attmissingval;
+	int			i_attcmoptions;
+	int			i_attcmname;
 	PGresult   *res;
 	int			ntups;
 	bool		hasdefaults;
+	bool		createWithCompression;
 
 	for (i = 0; i < numTables; i++)
 	{
@@ -8178,6 +8184,23 @@ getTableAttrs(Archive *fout, TableInfo *tblinfo, int numTables)
 						  "a.attislocal,\n"
 						  "pg_catalog.format_type(t.oid, a.atttypmod) AS atttypname,\n");
 
+		createWithCompression = (!dopt->binary_upgrade && fout->remoteVersion >= 120000);
+
+		if (createWithCompression)
+			appendPQExpBuffer(q,
+							  "pg_catalog.array_to_string(ARRAY("
+							  "SELECT pg_catalog.quote_ident(option_name) || "
+							  "' ' || pg_catalog.quote_literal(option_value) "
+							  "FROM pg_catalog.pg_options_to_table(c.acoptions) "
+							  "ORDER BY option_name"
+							  "), E',\n    ') AS attcmoptions,\n"
+							  "c.acname AS attcmname,\n");
+		else
+			appendPQExpBuffer(q,
+							  "NULL AS attcmoptions,\n"
+							  "NULL AS attcmname,\n");
+
+
 		if (fout->remoteVersion >= 110000)
 			appendPQExpBuffer(q,
 							  "CASE WHEN a.atthasmissing AND NOT a.attisdropped "
@@ -8228,7 +8251,13 @@ getTableAttrs(Archive *fout, TableInfo *tblinfo, int numTables)
 		appendPQExpBuffer(q,
 						  /* need left join here to not fail on dropped columns ... */
 						  "FROM pg_catalog.pg_attribute a LEFT JOIN pg_catalog.pg_type t "
-						  "ON a.atttypid = t.oid\n"
+						  "ON a.atttypid = t.oid\n");
+
+		if (createWithCompression)
+			appendPQExpBuffer(q, "LEFT JOIN pg_catalog.pg_attr_compression c "
+								 "ON a.attcompression = c.acoid\n");
+
+		appendPQExpBuffer(q,
 						  "WHERE a.attrelid = '%u'::pg_catalog.oid "
 						  "AND a.attnum > 0::pg_catalog.int2\n"
 						  "ORDER BY a.attnum",
@@ -8256,6 +8285,8 @@ getTableAttrs(Archive *fout, TableInfo *tblinfo, int numTables)
 		i_attcollation = PQfnumber(res, "attcollation");
 		i_attfdwoptions = PQfnumber(res, "attfdwoptions");
 		i_attmissingval = PQfnumber(res, "attmissingval");
+		i_attcmname = PQfnumber(res, "attcmname");
+		i_attcmoptions = PQfnumber(res, "attcmoptions");
 
 		tbinfo->numatts = ntups;
 		tbinfo->attnames = (char **) pg_malloc(ntups * sizeof(char *));
@@ -8273,9 +8304,12 @@ getTableAttrs(Archive *fout, TableInfo *tblinfo, int numTables)
 		tbinfo->attcollation = (Oid *) pg_malloc(ntups * sizeof(Oid));
 		tbinfo->attfdwoptions = (char **) pg_malloc(ntups * sizeof(char *));
 		tbinfo->attmissingval = (char **) pg_malloc(ntups * sizeof(char *));
+		tbinfo->attcmoptions = (char **) pg_malloc(ntups * sizeof(char *));
+		tbinfo->attcmnames = (char **) pg_malloc(ntups * sizeof(char *));
 		tbinfo->notnull = (bool *) pg_malloc(ntups * sizeof(bool));
 		tbinfo->inhNotNull = (bool *) pg_malloc(ntups * sizeof(bool));
 		tbinfo->attrdefs = (AttrDefInfo **) pg_malloc(ntups * sizeof(AttrDefInfo *));
+		tbinfo->attcompression = NULL;
 		hasdefaults = false;
 
 		for (j = 0; j < ntups; j++)
@@ -8301,6 +8335,8 @@ getTableAttrs(Archive *fout, TableInfo *tblinfo, int numTables)
 			tbinfo->attcollation[j] = atooid(PQgetvalue(res, j, i_attcollation));
 			tbinfo->attfdwoptions[j] = pg_strdup(PQgetvalue(res, j, i_attfdwoptions));
 			tbinfo->attmissingval[j] = pg_strdup(PQgetvalue(res, j, i_attmissingval));
+			tbinfo->attcmoptions[j] = pg_strdup(PQgetvalue(res, j, i_attcmoptions));
+			tbinfo->attcmnames[j] = pg_strdup(PQgetvalue(res, j, i_attcmname));
 			tbinfo->attrdefs[j] = NULL; /* fix below */
 			if (PQgetvalue(res, j, i_atthasdef)[0] == 't')
 				hasdefaults = true;
@@ -8518,6 +8554,104 @@ getTableAttrs(Archive *fout, TableInfo *tblinfo, int numTables)
 			}
 			PQclear(res);
 		}
+
+		/*
+		 * Get compression info
+		 */
+		if (fout->remoteVersion >= 120000 && dopt->binary_upgrade)
+		{
+			int			i_acname;
+			int			i_acoid;
+			int			i_parsedoptions;
+			int			i_curattnum;
+			int			start;
+
+			if (g_verbose)
+				write_msg(NULL, "finding compression info for table \"%s.%s\"\n",
+						  tbinfo->dobj.namespace->dobj.name,
+						  tbinfo->dobj.name);
+
+			tbinfo->attcompression = pg_malloc0(tbinfo->numatts * sizeof(AttrCompressionInfo *));
+
+			resetPQExpBuffer(q);
+			appendPQExpBuffer(q,
+				"SELECT attrelid::pg_catalog.regclass AS relname, attname,"
+				" (CASE WHEN deptype = 'i' THEN refobjsubid ELSE objsubid END) AS curattnum,"
+				" (CASE WHEN deptype = 'n' THEN attcompression = refobjid"
+				"		ELSE attcompression = objid END) AS iscurrent,"
+				" acname, acoid,"
+				" (CASE WHEN acoptions IS NOT NULL"
+				"  THEN pg_catalog.array_to_string(ARRAY("
+				"		SELECT pg_catalog.quote_ident(option_name) || "
+				"			' ' || pg_catalog.quote_literal(option_value) "
+				"		FROM pg_catalog.pg_options_to_table(acoptions) "
+				"		ORDER BY option_name"
+				"		), E',\n    ')"
+				"  ELSE NULL END) AS parsedoptions "
+				" FROM pg_depend d"
+				" JOIN pg_attribute a ON"
+				"	(classid = 'pg_class'::pg_catalog.regclass::pg_catalog.oid AND a.attrelid = d.objid"
+				"		AND a.attnum = d.objsubid AND d.deptype = 'n'"
+				"		AND d.refclassid = 'pg_attr_compression'::pg_catalog.regclass::pg_catalog.oid)"
+				"	OR (d.refclassid = 'pg_class'::pg_catalog.regclass::pg_catalog.oid"
+				"		AND d.refobjid = a.attrelid"
+				"		AND d.refobjsubid = a.attnum AND d.deptype = 'i'"
+				"		AND d.classid = 'pg_attr_compression'::pg_catalog.regclass::pg_catalog.oid)"
+				" JOIN pg_attr_compression c ON"
+				"	(d.deptype = 'i' AND d.objid = c.acoid AND a.attnum = c.acattnum"
+				"		AND a.attrelid = c.acrelid) OR"
+				"	(d.deptype = 'n' AND d.refobjid = c.acoid AND c.acattnum = 0"
+				"		AND c.acrelid = 0)"
+				" WHERE (deptype = 'n' AND d.objid = %d) OR (deptype = 'i' AND d.refobjid = %d)"
+				" ORDER BY curattnum, iscurrent;",
+				tbinfo->dobj.catId.oid, tbinfo->dobj.catId.oid);
+
+			res = ExecuteSqlQuery(fout, q->data, PGRES_TUPLES_OK);
+			ntups = PQntuples(res);
+
+			if (ntups > 0)
+			{
+				int		k;
+
+				i_acname = PQfnumber(res, "acname");
+				i_acoid = PQfnumber(res, "acoid");
+				i_parsedoptions = PQfnumber(res, "parsedoptions");
+				i_curattnum = PQfnumber(res, "curattnum");
+
+				start = 0;
+
+				for (j = 0; j < ntups; j++)
+				{
+					int		attnum = atoi(PQgetvalue(res, j, i_curattnum));
+
+					if ((j == ntups - 1) || atoi(PQgetvalue(res, j + 1, i_curattnum)) != attnum)
+					{
+						AttrCompressionInfo *cminfo = pg_malloc(sizeof(AttrCompressionInfo));
+
+						cminfo->nitems = j - start + 1;
+						cminfo->items = pg_malloc(sizeof(AttrCompressionItem *) * cminfo->nitems);
+
+						for (k = start; k < start + cminfo->nitems; k++)
+						{
+							AttrCompressionItem	*cmitem = pg_malloc0(sizeof(AttrCompressionItem));
+
+							cmitem->acname = pg_strdup(PQgetvalue(res, k, i_acname));
+							cmitem->acoid = atooid(PQgetvalue(res, k, i_acoid));
+
+							if (!PQgetisnull(res, k, i_parsedoptions))
+								cmitem->parsedoptions = pg_strdup(PQgetvalue(res, k, i_parsedoptions));
+
+							cminfo->items[k - start] = cmitem;
+						}
+
+						tbinfo->attcompression[attnum - 1] = cminfo;
+						start = j + 1;	/* start from next */
+					}
+				}
+			}
+
+			PQclear(res);
+		}
 	}
 
 	destroyPQExpBuffer(q);
@@ -12575,6 +12709,9 @@ dumpAccessMethod(Archive *fout, AccessMethodInfo *aminfo)
 		case AMTYPE_INDEX:
 			appendPQExpBuffer(q, "TYPE INDEX ");
 			break;
+		case AMTYPE_COMPRESSION:
+			appendPQExpBuffer(q, "TYPE COMPRESSION ");
+			break;
 		default:
 			write_msg(NULL, "WARNING: invalid type \"%c\" of access method \"%s\"\n",
 					  aminfo->amtype, qamname);
@@ -15500,6 +15637,14 @@ dumpTableSchema(Archive *fout, TableInfo *tbinfo)
 											   (!tbinfo->inhNotNull[j] ||
 												dopt->binary_upgrade));
 
+					/*
+					 * Compression will require a record in
+					 * pg_attr_compression
+					 */
+					bool		has_custom_compression = (tbinfo->attcmnames[j] &&
+														  ((strcmp(tbinfo->attcmnames[j], "pglz") != 0) ||
+														   nonemptyReloptions(tbinfo->attcmoptions[j])));
+
 					/*
 					 * Skip column if fully defined by reloftype or the
 					 * partition parent.
@@ -15558,6 +15703,25 @@ dumpTableSchema(Archive *fout, TableInfo *tbinfo)
 											  fmtQualifiedDumpable(coll));
 					}
 
+					/*
+					 * Compression
+					 *
+					 * In binary-upgrade mode, compression is assigned by
+					 * ALTER. Even if we're skipping compression the attribute
+					 * will get default compression. It's the task for ALTER
+					 * command to restore compression info.
+					 */
+					if (!dopt->no_compression_methods && !dopt->binary_upgrade &&
+						tbinfo->attcmnames[j] && strlen(tbinfo->attcmnames[j]) &&
+						has_custom_compression)
+					{
+						appendPQExpBuffer(q, " COMPRESSION %s",
+										  tbinfo->attcmnames[j]);
+						if (nonemptyReloptions(tbinfo->attcmoptions[j]))
+							appendPQExpBuffer(q, " WITH (%s)",
+											  tbinfo->attcmoptions[j]);
+					}
+
 					if (has_default)
 						appendPQExpBuffer(q, " DEFAULT %s",
 										  tbinfo->attrdefs[j]->adef_expr);
@@ -15973,6 +16137,34 @@ dumpTableSchema(Archive *fout, TableInfo *tbinfo)
 				appendPQExpBuffer(q, "OPTIONS (\n    %s\n);\n",
 								  tbinfo->attfdwoptions[j]);
 			}
+
+			/*
+			 * Dump per-column compression options
+			 */
+			if (tbinfo->attcompression && tbinfo->attcompression[j])
+			{
+				AttrCompressionInfo *cminfo = tbinfo->attcompression[j];
+
+				if (cminfo->nitems)
+					appendPQExpBuffer(q, "\n-- For binary upgrade, recreate compression metadata on column %s\n",
+							fmtId(tbinfo->attnames[j]));
+
+				for (int i = 0; i < cminfo->nitems; i++)
+				{
+					AttrCompressionItem *item = cminfo->items[i];
+
+					appendPQExpBuffer(q,
+						"SELECT binary_upgrade_set_next_attr_compression_oid('%d'::pg_catalog.oid);\n",
+									  item->acoid);
+					appendPQExpBuffer(q, "ALTER TABLE %s ALTER COLUMN %s\nSET COMPRESSION %s",
+									  qualrelname, fmtId(tbinfo->attnames[j]), item->acname);
+
+					if (item->parsedoptions)
+						appendPQExpBuffer(q, "\nWITH (%s);\n", item->parsedoptions);
+					else
+						appendPQExpBuffer(q, ";\n");
+				}
+			}
 		}
 	}
 
diff --git a/src/bin/pg_dump/pg_dump.h b/src/bin/pg_dump/pg_dump.h
index 1448005f30..582d661dd2 100644
--- a/src/bin/pg_dump/pg_dump.h
+++ b/src/bin/pg_dump/pg_dump.h
@@ -325,6 +325,10 @@ typedef struct _tableInfo
 	char	   *partbound;		/* partition bound definition */
 	bool		needs_override; /* has GENERATED ALWAYS AS IDENTITY */
 
+	char	  **attcmoptions;	/* per-attribute current compression options */
+	char	  **attcmnames;		/* per-attribute current compression method names */
+	struct _attrCompressionInfo **attcompression; /* per-attribute all compression data */
+
 	/*
 	 * Stuff computed only for dumpable tables.
 	 */
@@ -346,6 +350,19 @@ typedef struct _attrDefInfo
 	bool		separate;		/* true if must dump as separate item */
 } AttrDefInfo;
 
+typedef struct _attrCompressionItem
+{
+	Oid			acoid;			/* attribute compression oid */
+	char	   *acname;			/* compression access method name */
+	char	   *parsedoptions;	/* WITH options */
+} AttrCompressionItem;
+
+typedef struct _attrCompressionInfo
+{
+	int			nitems;
+	AttrCompressionItem	**items;
+} AttrCompressionInfo;
+
 typedef struct _tableDataInfo
 {
 	DumpableObject dobj;
diff --git a/src/bin/pg_dump/pg_dumpall.c b/src/bin/pg_dump/pg_dumpall.c
index eb29d318a4..61d55c7082 100644
--- a/src/bin/pg_dump/pg_dumpall.c
+++ b/src/bin/pg_dump/pg_dumpall.c
@@ -74,6 +74,7 @@ static int	no_comments = 0;
 static int	no_publications = 0;
 static int	no_security_labels = 0;
 static int	no_subscriptions = 0;
+static int	no_compression_methods = 0;
 static int	no_unlogged_table_data = 0;
 static int	no_role_passwords = 0;
 static int	server_version;
@@ -136,6 +137,7 @@ main(int argc, char *argv[])
 		{"no-role-passwords", no_argument, &no_role_passwords, 1},
 		{"no-security-labels", no_argument, &no_security_labels, 1},
 		{"no-subscriptions", no_argument, &no_subscriptions, 1},
+		{"no-compression-methods", no_argument, &no_compression_methods, 1},
 		{"no-sync", no_argument, NULL, 4},
 		{"no-unlogged-table-data", no_argument, &no_unlogged_table_data, 1},
 		{"on-conflict-do-nothing", no_argument, &on_conflict_do_nothing, 1},
@@ -406,6 +408,8 @@ main(int argc, char *argv[])
 		appendPQExpBufferStr(pgdumpopts, " --no-security-labels");
 	if (no_subscriptions)
 		appendPQExpBufferStr(pgdumpopts, " --no-subscriptions");
+	if (no_compression_methods)
+		appendPQExpBufferStr(pgdumpopts, " --no-compression-methods");
 	if (no_unlogged_table_data)
 		appendPQExpBufferStr(pgdumpopts, " --no-unlogged-table-data");
 	if (on_conflict_do_nothing)
@@ -622,6 +626,7 @@ help(void)
 	printf(_("  --no-role-passwords          do not dump passwords for roles\n"));
 	printf(_("  --no-security-labels         do not dump security label assignments\n"));
 	printf(_("  --no-subscriptions           do not dump subscriptions\n"));
+	printf(_("  --no-compression-methods     do not dump compression methods\n"));
 	printf(_("  --no-sync                    do not wait for changes to be written safely to disk\n"));
 	printf(_("  --no-tablespaces             do not dump tablespace assignments\n"));
 	printf(_("  --no-unlogged-table-data     do not dump unlogged table data\n"));
diff --git a/src/bin/pg_dump/pg_restore.c b/src/bin/pg_dump/pg_restore.c
index 501d7cea72..78758107f2 100644
--- a/src/bin/pg_dump/pg_restore.c
+++ b/src/bin/pg_dump/pg_restore.c
@@ -75,6 +75,7 @@ main(int argc, char **argv)
 	static int	no_publications = 0;
 	static int	no_security_labels = 0;
 	static int	no_subscriptions = 0;
+	static int	no_compression_methods = 0;
 	static int	strict_names = 0;
 
 	struct option cmdopts[] = {
@@ -124,6 +125,7 @@ main(int argc, char **argv)
 		{"no-publications", no_argument, &no_publications, 1},
 		{"no-security-labels", no_argument, &no_security_labels, 1},
 		{"no-subscriptions", no_argument, &no_subscriptions, 1},
+		{"no-compression-methods", no_argument, &no_compression_methods, 1},
 
 		{NULL, 0, NULL, 0}
 	};
@@ -364,6 +366,7 @@ main(int argc, char **argv)
 	opts->no_publications = no_publications;
 	opts->no_security_labels = no_security_labels;
 	opts->no_subscriptions = no_subscriptions;
+	opts->no_compression_methods = no_compression_methods;
 
 	if (if_exists && !opts->dropSchema)
 	{
diff --git a/src/bin/pg_dump/t/002_pg_dump.pl b/src/bin/pg_dump/t/002_pg_dump.pl
index ec751a7c23..432b65ef00 100644
--- a/src/bin/pg_dump/t/002_pg_dump.pl
+++ b/src/bin/pg_dump/t/002_pg_dump.pl
@@ -650,6 +650,43 @@ my %tests = (
 		},
 	},
 
+	# compression data in binary upgrade mode
+	'ALTER TABLE test_table_compression ALTER COLUMN ... SET COMPRESSION' => {
+		all_runs  => 1,
+		catch_all => 'ALTER TABLE ... commands',
+		regexp    => qr/^
+			\QCREATE TABLE dump_test.test_table_compression (\E\n
+			\s+\Qcol1 text,\E\n
+			\s+\Qcol2 text,\E\n
+			\s+\Qcol3 text,\E\n
+			\s+\Qcol4 text\E\n
+			\);
+			.*
+			\QSELECT binary_upgrade_set_next_attr_compression_oid('\E\d+\Q'::pg_catalog.oid);\E\n
+			\QALTER TABLE dump_test.test_table_compression ALTER COLUMN col1\E\n
+			\QSET COMPRESSION pglz;\E\n
+			.*
+			\QSELECT binary_upgrade_set_next_attr_compression_oid('\E\d+\Q'::pg_catalog.oid);\E\n
+			\QALTER TABLE dump_test.test_table_compression ALTER COLUMN col2\E\n
+			\QSET COMPRESSION pglz2;\E\n
+			.*
+			\QSELECT binary_upgrade_set_next_attr_compression_oid('\E\d+\Q'::pg_catalog.oid);\E\n
+			\QALTER TABLE dump_test.test_table_compression ALTER COLUMN col3\E\n
+			\QSET COMPRESSION pglz\E\n
+			\QWITH (min_input_size '1000');\E\n
+			.*
+			\QSELECT binary_upgrade_set_next_attr_compression_oid('\E\d+\Q'::pg_catalog.oid);\E\n
+			\QALTER TABLE dump_test.test_table_compression ALTER COLUMN col4\E\n
+			\QSET COMPRESSION pglz2\E\n
+			\QWITH (min_input_size '1000');\E\n
+			\QSELECT binary_upgrade_set_next_attr_compression_oid('\E\d+\Q'::pg_catalog.oid);\E\n
+			\QALTER TABLE dump_test.test_table_compression ALTER COLUMN col4\E\n
+			\QSET COMPRESSION pglz2\E\n
+			\QWITH (min_input_size '2000');\E\n
+			/xms,
+		like => { binary_upgrade => 1, },
+	},
+
 	'ALTER TABLE ONLY test_table ALTER COLUMN col1 SET STATISTICS 90' => {
 		create_order => 93,
 		create_sql =>
@@ -1400,6 +1437,17 @@ my %tests = (
 		like => { %full_runs, section_pre_data => 1, },
 	},
 
+	'CREATE ACCESS METHOD pglz2' => {
+		all_runs     => 1,
+		catch_all    => 'CREATE ... commands',
+		create_order => 52,
+		create_sql =>
+		  'CREATE ACCESS METHOD pglz2 TYPE COMPRESSION HANDLER pglzhandler;',
+		regexp =>
+		  qr/CREATE ACCESS METHOD pglz2 TYPE COMPRESSION HANDLER pglzhandler;/m,
+		like => { %full_runs, section_pre_data => 1, },
+	},
+
 	'CREATE COLLATION test0 FROM "C"' => {
 		create_order => 76,
 		create_sql   => 'CREATE COLLATION test0 FROM "C";',
@@ -2420,6 +2468,53 @@ my %tests = (
 		unlike => { exclude_dump_test_schema => 1, },
 	},
 
+	'CREATE TABLE test_table_compression' => {
+		create_order => 55,
+		create_sql   => 'CREATE TABLE dump_test.test_table_compression (
+						   col1 text,
+						   col2 text COMPRESSION pglz2,
+						   col3 text COMPRESSION pglz WITH (min_input_size \'1000\'),
+						   col4 text COMPRESSION pglz2 WITH (min_input_size \'1000\')
+					     );',
+		regexp => qr/^
+			\QCREATE TABLE dump_test.test_table_compression (\E\n
+			\s+\Qcol1 text,\E\n
+			\s+\Qcol2 text COMPRESSION pglz2,\E\n
+			\s+\Qcol3 text COMPRESSION pglz WITH (min_input_size '1000'),\E\n
+			\s+\Qcol4 text COMPRESSION pglz2 WITH (min_input_size '2000')\E\n
+			\);
+			/xm,
+		like =>
+		  { %full_runs, %dump_test_schema_runs, section_pre_data => 1, },
+		unlike => {
+			binary_upgrade		     => 1,
+			exclude_dump_test_schema => 1,
+		},
+	},
+
+	'ALTER TABLE test_table_compression' => {
+		create_order => 56,
+		create_sql   => 'ALTER TABLE dump_test.test_table_compression
+						 ALTER COLUMN col4
+						 SET COMPRESSION pglz2
+						 WITH (min_input_size \'2000\')
+						 PRESERVE (pglz2);',
+		regexp => qr/^
+			\QCREATE TABLE dump_test.test_table_compression (\E\n
+			\s+\Qcol1 text,\E\n
+			\s+\Qcol2 text COMPRESSION pglz2,\E\n
+			\s+\Qcol3 text COMPRESSION pglz WITH (min_input_size '1000'),\E\n
+			\s+\Qcol4 text COMPRESSION pglz2 WITH (min_input_size '2000')\E\n
+			\);
+			/xm,
+		like =>
+		  { %full_runs, %dump_test_schema_runs, section_pre_data => 1, },
+		unlike => {
+			binary_upgrade		     => 1,
+			exclude_dump_test_schema => 1,
+		},
+	},
+
 	'CREATE STATISTICS extended_stats_no_options' => {
 		create_order => 97,
 		create_sql   => 'CREATE STATISTICS dump_test.test_ext_stats_no_options
diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c
index 4ca0db1d0c..58bc222c0d 100644
--- a/src/bin/psql/describe.c
+++ b/src/bin/psql/describe.c
@@ -1467,6 +1467,7 @@ describeOneTableDetails(const char *schemaname,
 				fdwopts_col = -1,
 				attstorage_col = -1,
 				attstattarget_col = -1,
+				attcompression_col = -1,
 				attdescr_col = -1;
 	int			numrows;
 	struct
@@ -1835,6 +1836,24 @@ describeOneTableDetails(const char *schemaname,
 		appendPQExpBufferStr(&buf, ",\n  a.attstorage");
 		attstorage_col = cols++;
 
+		/* compresssion info */
+		if (pset.sversion >= 120000 &&
+			(tableinfo.relkind == RELKIND_RELATION ||
+			 tableinfo.relkind == RELKIND_PARTITIONED_TABLE))
+		{
+			appendPQExpBufferStr(&buf, ",\n  CASE WHEN attcompression = 0 THEN NULL ELSE "
+								 " (SELECT c.acname || "
+								 "		(CASE WHEN acoptions IS NULL "
+								 "		 THEN '' "
+								 "		 ELSE '(' || array_to_string(ARRAY(SELECT quote_ident(option_name) || ' ' || quote_literal(option_value)"
+								 "											  FROM pg_options_to_table(acoptions)), ', ') || ')'"
+								 " 		 END) "
+								 "  FROM pg_catalog.pg_attr_compression c "
+								 "  WHERE c.acoid = a.attcompression) "
+								 " END AS attcmname");
+			attcompression_col = cols++;
+		}
+
 		/* stats target, if relevant to relkind */
 		if (tableinfo.relkind == RELKIND_RELATION ||
 			tableinfo.relkind == RELKIND_INDEX ||
@@ -1954,6 +1973,8 @@ describeOneTableDetails(const char *schemaname,
 		headers[cols++] = gettext_noop("FDW options");
 	if (attstorage_col >= 0)
 		headers[cols++] = gettext_noop("Storage");
+	if (attcompression_col >= 0)
+		headers[cols++] = gettext_noop("Compression");
 	if (attstattarget_col >= 0)
 		headers[cols++] = gettext_noop("Stats target");
 	if (attdescr_col >= 0)
@@ -2025,6 +2046,27 @@ describeOneTableDetails(const char *schemaname,
 							  false, false);
 		}
 
+		/* Column compression. */
+		if (attcompression_col >= 0)
+		{
+			bool		mustfree = false;
+			const int	trunclen = 100;
+			char *val = PQgetvalue(res, i, attcompression_col);
+
+			/* truncate the options if they're too long */
+			if (strlen(val) > trunclen + 3)
+			{
+				char *trunc = pg_malloc0(trunclen + 4);
+				strncpy(trunc, val, trunclen);
+				strncpy(trunc + trunclen, "...", 4);
+
+				val = trunc;
+				mustfree = true;
+			}
+
+			printTableAddCell(&cont, val, false, mustfree);
+		}
+
 		/* Statistics target, if the relkind supports this feature */
 		if (attstattarget_col >= 0)
 			printTableAddCell(&cont, PQgetvalue(res, i, attstattarget_col),
diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index bb696f8ee9..8cfb0304a8 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -2161,11 +2161,14 @@ psql_completion(const char *text, int start, int end)
 	/* ALTER TABLE ALTER [COLUMN] <foo> SET */
 	else if (Matches7("ALTER", "TABLE", MatchAny, "ALTER", "COLUMN", MatchAny, "SET") ||
 			 Matches6("ALTER", "TABLE", MatchAny, "ALTER", MatchAny, "SET"))
-		COMPLETE_WITH_LIST5("(", "DEFAULT", "NOT NULL", "STATISTICS", "STORAGE");
+		COMPLETE_WITH_LIST6("(", "COMPRESSION", "DEFAULT", "NOT NULL", "STATISTICS", "STORAGE");
 	/* ALTER TABLE ALTER [COLUMN] <foo> SET ( */
 	else if (Matches8("ALTER", "TABLE", MatchAny, "ALTER", "COLUMN", MatchAny, "SET", "(") ||
 			 Matches7("ALTER", "TABLE", MatchAny, "ALTER", MatchAny, "SET", "("))
 		COMPLETE_WITH_LIST2("n_distinct", "n_distinct_inherited");
+	else if (Matches9("ALTER", "TABLE", MatchAny, "ALTER", "COLUMN", MatchAny, "SET", "COMPRESSION", MatchAny) ||
+			 Matches8("ALTER", "TABLE", MatchAny, "ALTER", MatchAny, "SET", "COMPRESSION", MatchAny))
+		COMPLETE_WITH_CONST("WITH (");
 	/* ALTER TABLE ALTER [COLUMN] <foo> SET STORAGE */
 	else if (Matches8("ALTER", "TABLE", MatchAny, "ALTER", "COLUMN", MatchAny, "SET", "STORAGE") ||
 			 Matches7("ALTER", "TABLE", MatchAny, "ALTER", MatchAny, "SET", "STORAGE"))
diff --git a/src/include/catalog/binary_upgrade.h b/src/include/catalog/binary_upgrade.h
index abc6e1ae1d..1e95a3863a 100644
--- a/src/include/catalog/binary_upgrade.h
+++ b/src/include/catalog/binary_upgrade.h
@@ -25,6 +25,8 @@ extern PGDLLIMPORT Oid binary_upgrade_next_toast_pg_class_oid;
 extern PGDLLIMPORT Oid binary_upgrade_next_pg_enum_oid;
 extern PGDLLIMPORT Oid binary_upgrade_next_pg_authid_oid;
 
+extern PGDLLIMPORT Oid binary_upgrade_next_attr_compression_oid;
+
 extern PGDLLIMPORT bool binary_upgrade_record_init_privs;
 
 #endif							/* BINARY_UPGRADE_H */
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index 53891aacc0..06a0576bd8 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -10061,6 +10061,10 @@
   proname => 'binary_upgrade_set_missing_value', provolatile => 'v',
   proparallel => 'u', prorettype => 'void', proargtypes => 'oid text text',
   prosrc => 'binary_upgrade_set_missing_value' },
+{ oid => '4012', descr => 'for use by pg_upgrade',
+  proname => 'binary_upgrade_set_next_attr_compression_oid', provolatile => 'v',
+  proparallel => 'r', prorettype => 'void', proargtypes => 'oid',
+  prosrc => 'binary_upgrade_set_next_attr_compression_oid' },
 
 # replication/origin.h
 { oid => '6003', descr => 'create a replication origin',
-- 
2.18.0


--MP_/tqROVSJLfUtKS/DWevR5Hf=
Content-Type: text/x-patch
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
 filename=0007-Add-tests-for-compression-methods-v19.patch



^ permalink  raw  reply  [nested|flat] 18+ messages in thread

* [PATCH 6/8] Add psql, pg_dump and pg_upgrade support
@ 2018-06-18 12:57 Ildus Kurbangaliev <i.kurbangaliev@gmail.com>
  0 siblings, 0 replies; 18+ messages in thread

From: Ildus Kurbangaliev @ 2018-06-18 12:57 UTC (permalink / raw)

Signed-off-by: Ildus Kurbangaliev <i.kurbangaliev@gmail.com>
---
 src/backend/commands/compressioncmds.c     |  80 ++++++---
 src/backend/commands/tablecmds.c           |  14 +-
 src/backend/utils/adt/pg_upgrade_support.c |  10 ++
 src/bin/pg_dump/pg_backup.h                |   2 +
 src/bin/pg_dump/pg_dump.c                  | 200 ++++++++++++++++++++-
 src/bin/pg_dump/pg_dump.h                  |  17 ++
 src/bin/pg_dump/pg_dumpall.c               |   5 +
 src/bin/pg_dump/pg_restore.c               |   3 +
 src/bin/pg_dump/t/002_pg_dump.pl           |  95 ++++++++++
 src/bin/psql/describe.c                    |  42 +++++
 src/bin/psql/tab-complete.c                |   5 +-
 src/include/catalog/binary_upgrade.h       |   2 +
 src/include/catalog/pg_proc.dat            |   4 +
 13 files changed, 434 insertions(+), 45 deletions(-)

diff --git a/src/backend/commands/compressioncmds.c b/src/backend/commands/compressioncmds.c
index e1b41964f2..f3a5a1f7fb 100644
--- a/src/backend/commands/compressioncmds.c
+++ b/src/backend/commands/compressioncmds.c
@@ -36,6 +36,9 @@
 #include "utils/syscache.h"
 #include "utils/snapmgr.h"
 
+/* Set by pg_upgrade_support functions */
+Oid			binary_upgrade_next_attr_compression_oid = InvalidOid;
+
 /*
  * When conditions of compression satisfies one if builtin attribute
  * compresssion tuples the compressed attribute will be linked to
@@ -129,11 +132,12 @@ lookup_attribute_compression(Oid attrelid, AttrNumber attnum,
 					tup_amoid;
 		Datum		values[Natts_pg_attr_compression];
 		bool		nulls[Natts_pg_attr_compression];
+		char	   *amname;
 
 		heap_deform_tuple(tuple, RelationGetDescr(rel), values, nulls);
 		acoid = DatumGetObjectId(values[Anum_pg_attr_compression_acoid - 1]);
-		tup_amoid = get_am_oid(
-							   NameStr(*DatumGetName(values[Anum_pg_attr_compression_acname - 1])), false);
+		amname = NameStr(*DatumGetName(values[Anum_pg_attr_compression_acname - 1]));
+		tup_amoid = get_am_oid(amname, false);
 
 		if (previous_amoids)
 			*previous_amoids = list_append_unique_oid(*previous_amoids, tup_amoid);
@@ -150,17 +154,15 @@ lookup_attribute_compression(Oid attrelid, AttrNumber attnum,
 			if (DatumGetPointer(acoptions) == NULL)
 				result = acoid;
 		}
-		else
+		else if (DatumGetPointer(acoptions) != NULL)
 		{
 			bool		equal;
 
 			/* check if arrays for WITH options are equal */
 			equal = DatumGetBool(CallerFInfoFunctionCall2(
-														  array_eq,
-														  &arrayeq_info,
-														  InvalidOid,
-														  acoptions,
-														  values[Anum_pg_attr_compression_acoptions - 1]));
+						array_eq, &arrayeq_info, InvalidOid, acoptions,
+						values[Anum_pg_attr_compression_acoptions - 1]));
+
 			if (equal)
 				result = acoid;
 		}
@@ -227,6 +229,16 @@ CreateAttributeCompression(Form_pg_attribute att,
 	/* Try to find builtin compression first */
 	acoid = lookup_attribute_compression(0, 0, amoid, arropt, NULL);
 
+	/* no rewrite by default */
+	if (need_rewrite != NULL)
+		*need_rewrite = false;
+
+	if (IsBinaryUpgrade)
+	{
+		/* Skip the rewrite checks and searching of identical compression */
+		goto add_tuple;
+	}
+
 	/*
 	 * attrelid will be invalid on CREATE TABLE, no need for table rewrite
 	 * check.
@@ -252,16 +264,10 @@ CreateAttributeCompression(Form_pg_attribute att,
 		 */
 		if (need_rewrite != NULL)
 		{
-			/* no rewrite by default */
-			*need_rewrite = false;
-
 			Assert(preserved_amoids != NULL);
 
 			if (compression->preserve == NIL)
-			{
-				Assert(!IsBinaryUpgrade);
 				*need_rewrite = true;
-			}
 			else
 			{
 				ListCell   *cell;
@@ -294,7 +300,7 @@ CreateAttributeCompression(Form_pg_attribute att,
 				 * In binary upgrade list will not be free since it contains
 				 * Oid of builtin compression access method.
 				 */
-				if (!IsBinaryUpgrade && list_length(previous_amoids) != 0)
+				if (list_length(previous_amoids) != 0)
 					*need_rewrite = true;
 			}
 		}
@@ -303,9 +309,6 @@ CreateAttributeCompression(Form_pg_attribute att,
 		list_free(previous_amoids);
 	}
 
-	if (IsBinaryUpgrade && !OidIsValid(acoid))
-		elog(ERROR, "could not restore attribute compression data");
-
 	/* Return Oid if we already found identical compression on this column */
 	if (OidIsValid(acoid))
 	{
@@ -315,6 +318,7 @@ CreateAttributeCompression(Form_pg_attribute att,
 		return acoid;
 	}
 
+add_tuple:
 	/* Initialize buffers for new tuple values */
 	memset(values, 0, sizeof(values));
 	memset(nulls, false, sizeof(nulls));
@@ -323,13 +327,27 @@ CreateAttributeCompression(Form_pg_attribute att,
 
 	rel = heap_open(AttrCompressionRelationId, RowExclusiveLock);
 
-	acoid = GetNewOidWithIndex(rel, AttrCompressionIndexId,
-							   Anum_pg_attr_compression_acoid);
+	if (IsBinaryUpgrade)
+	{
+		/* acoid should be found in some cases */
+		if (binary_upgrade_next_attr_compression_oid < FirstNormalObjectId &&
+			(!OidIsValid(acoid) || binary_upgrade_next_attr_compression_oid != acoid))
+			elog(ERROR, "could not link to built-in attribute compression");
+
+		acoid = binary_upgrade_next_attr_compression_oid;
+	}
+	else
+	{
+		acoid = GetNewOidWithIndex(rel, AttrCompressionIndexId,
+									Anum_pg_attr_compression_acoid);
+
+	}
+
 	if (acoid < FirstNormalObjectId)
 	{
-		/* this is database initialization */
+		/* this is built-in attribute compression */
 		heap_close(rel, RowExclusiveLock);
-		return DefaultCompressionOid;
+		return acoid;
 	}
 
 	/* we need routine only to call cmcheck function */
@@ -393,8 +411,8 @@ RemoveAttributeCompression(Oid acoid)
 /*
  * CleanupAttributeCompression
  *
- * Remove entries in pg_attr_compression except current attribute compression
- * and related with specified list of access methods.
+ * Remove entries in pg_attr_compression of the column except current
+ * attribute compression and related with specified list of access methods.
  */
 void
 CleanupAttributeCompression(Oid relid, AttrNumber attnum, List *keepAmOids)
@@ -422,9 +440,7 @@ CleanupAttributeCompression(Oid relid, AttrNumber attnum, List *keepAmOids)
 	ReleaseSysCache(attrtuple);
 
 	Assert(relid > 0 && attnum > 0);
-
-	if (IsBinaryUpgrade)
-		goto builtin_removal;
+	Assert(!IsBinaryUpgrade);
 
 	rel = heap_open(AttrCompressionRelationId, RowExclusiveLock);
 
@@ -441,7 +457,10 @@ CleanupAttributeCompression(Oid relid, AttrNumber attnum, List *keepAmOids)
 	scan = systable_beginscan(rel, AttrCompressionRelidAttnumIndexId,
 							  true, NULL, 2, key);
 
-	/* Remove attribute compression tuples and collect removed Oids to list */
+	/*
+	 * Remove attribute compression tuples and collect removed Oids
+	 * to list.
+	 */
 	while (HeapTupleIsValid(tuple = systable_getnext(scan)))
 	{
 		Form_pg_attr_compression acform;
@@ -463,7 +482,10 @@ CleanupAttributeCompression(Oid relid, AttrNumber attnum, List *keepAmOids)
 	systable_endscan(scan);
 	heap_close(rel, RowExclusiveLock);
 
-	/* Now remove dependencies */
+	/*
+	 * Now remove dependencies between attribute compression (dependent)
+	 * and column.
+	 */
 	rel = heap_open(DependRelationId, RowExclusiveLock);
 	foreach(lc, removed)
 	{
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 89d53b173e..5ef48e3005 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -756,10 +756,10 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId,
 		if (colDef->identity)
 			attr->attidentity = colDef->identity;
 
-		if (relkind == RELKIND_RELATION || relkind == RELKIND_PARTITIONED_TABLE)
+		if (!IsBinaryUpgrade &&
+			(relkind == RELKIND_RELATION || relkind == RELKIND_PARTITIONED_TABLE))
 			attr->attcompression = CreateAttributeCompression(attr,
-															  colDef->compression,
-															  NULL, NULL);
+										colDef->compression, NULL, NULL);
 		else
 			attr->attcompression = InvalidOid;
 	}
@@ -13133,14 +13133,6 @@ ATExecSetCompression(AlteredTableInfo *tab,
 	/* make changes visible */
 	CommandCounterIncrement();
 
-	/*
-	 * Normally cleanup is done in rewrite but in binary upgrade we should do
-	 * it explicitly.
-	 */
-	if (IsBinaryUpgrade)
-		CleanupAttributeCompression(RelationGetRelid(rel),
-									attnum, preserved_amoids);
-
 	ObjectAddressSet(address, AttrCompressionRelationId, acoid);
 	return address;
 }
diff --git a/src/backend/utils/adt/pg_upgrade_support.c b/src/backend/utils/adt/pg_upgrade_support.c
index b8b7777c31..1082eab4dc 100644
--- a/src/backend/utils/adt/pg_upgrade_support.c
+++ b/src/backend/utils/adt/pg_upgrade_support.c
@@ -116,6 +116,16 @@ binary_upgrade_set_next_pg_authid_oid(PG_FUNCTION_ARGS)
 	PG_RETURN_VOID();
 }
 
+Datum
+binary_upgrade_set_next_attr_compression_oid(PG_FUNCTION_ARGS)
+{
+	Oid			acoid = PG_GETARG_OID(0);
+
+	CHECK_IS_BINARY_UPGRADE;
+	binary_upgrade_next_attr_compression_oid = acoid;
+	PG_RETURN_VOID();
+}
+
 Datum
 binary_upgrade_create_empty_extension(PG_FUNCTION_ARGS)
 {
diff --git a/src/bin/pg_dump/pg_backup.h b/src/bin/pg_dump/pg_backup.h
index 42cf441aaf..11b0da8221 100644
--- a/src/bin/pg_dump/pg_backup.h
+++ b/src/bin/pg_dump/pg_backup.h
@@ -78,6 +78,7 @@ typedef struct _restoreOptions
 	int			no_publications;	/* Skip publication entries */
 	int			no_security_labels; /* Skip security label entries */
 	int			no_subscriptions;	/* Skip subscription entries */
+	int			no_compression_methods; /* Skip compression methods */
 	int			strict_names;
 
 	const char *filename;
@@ -151,6 +152,7 @@ typedef struct _dumpOptions
 	int			no_security_labels;
 	int			no_publications;
 	int			no_subscriptions;
+	int			no_compression_methods;
 	int			no_synchronized_snapshots;
 	int			no_unlogged_table_data;
 	int			serializable_deferrable;
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index f0ea83e6a9..b76e059d67 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -40,11 +40,13 @@
 #include "getopt_long.h"
 
 #include "access/attnum.h"
+#include "access/cmapi.h"
 #include "access/sysattr.h"
 #include "access/transam.h"
 #include "catalog/pg_aggregate_d.h"
 #include "catalog/pg_am_d.h"
 #include "catalog/pg_attribute_d.h"
+#include "catalog/pg_attr_compression_d.h"
 #include "catalog/pg_cast_d.h"
 #include "catalog/pg_class_d.h"
 #include "catalog/pg_default_acl_d.h"
@@ -375,6 +377,7 @@ main(int argc, char **argv)
 		{"no-synchronized-snapshots", no_argument, &dopt.no_synchronized_snapshots, 1},
 		{"no-unlogged-table-data", no_argument, &dopt.no_unlogged_table_data, 1},
 		{"no-subscriptions", no_argument, &dopt.no_subscriptions, 1},
+		{"no-compression-methods", no_argument, &dopt.no_compression_methods, 1},
 		{"no-sync", no_argument, NULL, 7},
 		{"on-conflict-do-nothing", no_argument, &dopt.do_nothing, 1},
 
@@ -842,13 +845,13 @@ main(int argc, char **argv)
 	 * We rely on dependency information to help us determine a safe order, so
 	 * the initial sort is mostly for cosmetic purposes: we sort by name to
 	 * ensure that logically identical schemas will dump identically.
+	 *
+	 * If we do a parallel dump, we want the largest tables to go first.
 	 */
-	sortDumpableObjectsByTypeName(dobjs, numObjs);
-
-	/* If we do a parallel dump, we want the largest tables to go first */
 	if (archiveFormat == archDirectory && numWorkers > 1)
 		sortDataAndIndexObjectsBySize(dobjs, numObjs);
 
+	sortDumpableObjectsByTypeName(dobjs, numObjs);
 	sortDumpableObjects(dobjs, numObjs,
 						boundaryObjs[0].dumpId, boundaryObjs[1].dumpId);
 
@@ -8133,9 +8136,12 @@ getTableAttrs(Archive *fout, TableInfo *tblinfo, int numTables)
 	int			i_attcollation;
 	int			i_attfdwoptions;
 	int			i_attmissingval;
+	int			i_attcmoptions;
+	int			i_attcmname;
 	PGresult   *res;
 	int			ntups;
 	bool		hasdefaults;
+	bool		createWithCompression;
 
 	for (i = 0; i < numTables; i++)
 	{
@@ -8178,6 +8184,23 @@ getTableAttrs(Archive *fout, TableInfo *tblinfo, int numTables)
 						  "a.attislocal,\n"
 						  "pg_catalog.format_type(t.oid, a.atttypmod) AS atttypname,\n");
 
+		createWithCompression = (!dopt->binary_upgrade && fout->remoteVersion >= 120000);
+
+		if (createWithCompression)
+			appendPQExpBuffer(q,
+							  "pg_catalog.array_to_string(ARRAY("
+							  "SELECT pg_catalog.quote_ident(option_name) || "
+							  "' ' || pg_catalog.quote_literal(option_value) "
+							  "FROM pg_catalog.pg_options_to_table(c.acoptions) "
+							  "ORDER BY option_name"
+							  "), E',\n    ') AS attcmoptions,\n"
+							  "c.acname AS attcmname,\n");
+		else
+			appendPQExpBuffer(q,
+							  "NULL AS attcmoptions,\n"
+							  "NULL AS attcmname,\n");
+
+
 		if (fout->remoteVersion >= 110000)
 			appendPQExpBuffer(q,
 							  "CASE WHEN a.atthasmissing AND NOT a.attisdropped "
@@ -8228,7 +8251,13 @@ getTableAttrs(Archive *fout, TableInfo *tblinfo, int numTables)
 		appendPQExpBuffer(q,
 						  /* need left join here to not fail on dropped columns ... */
 						  "FROM pg_catalog.pg_attribute a LEFT JOIN pg_catalog.pg_type t "
-						  "ON a.atttypid = t.oid\n"
+						  "ON a.atttypid = t.oid\n");
+
+		if (createWithCompression)
+			appendPQExpBuffer(q, "LEFT JOIN pg_catalog.pg_attr_compression c "
+								 "ON a.attcompression = c.acoid\n");
+
+		appendPQExpBuffer(q,
 						  "WHERE a.attrelid = '%u'::pg_catalog.oid "
 						  "AND a.attnum > 0::pg_catalog.int2\n"
 						  "ORDER BY a.attnum",
@@ -8256,6 +8285,8 @@ getTableAttrs(Archive *fout, TableInfo *tblinfo, int numTables)
 		i_attcollation = PQfnumber(res, "attcollation");
 		i_attfdwoptions = PQfnumber(res, "attfdwoptions");
 		i_attmissingval = PQfnumber(res, "attmissingval");
+		i_attcmname = PQfnumber(res, "attcmname");
+		i_attcmoptions = PQfnumber(res, "attcmoptions");
 
 		tbinfo->numatts = ntups;
 		tbinfo->attnames = (char **) pg_malloc(ntups * sizeof(char *));
@@ -8273,9 +8304,12 @@ getTableAttrs(Archive *fout, TableInfo *tblinfo, int numTables)
 		tbinfo->attcollation = (Oid *) pg_malloc(ntups * sizeof(Oid));
 		tbinfo->attfdwoptions = (char **) pg_malloc(ntups * sizeof(char *));
 		tbinfo->attmissingval = (char **) pg_malloc(ntups * sizeof(char *));
+		tbinfo->attcmoptions = (char **) pg_malloc(ntups * sizeof(char *));
+		tbinfo->attcmnames = (char **) pg_malloc(ntups * sizeof(char *));
 		tbinfo->notnull = (bool *) pg_malloc(ntups * sizeof(bool));
 		tbinfo->inhNotNull = (bool *) pg_malloc(ntups * sizeof(bool));
 		tbinfo->attrdefs = (AttrDefInfo **) pg_malloc(ntups * sizeof(AttrDefInfo *));
+		tbinfo->attcompression = NULL;
 		hasdefaults = false;
 
 		for (j = 0; j < ntups; j++)
@@ -8301,6 +8335,8 @@ getTableAttrs(Archive *fout, TableInfo *tblinfo, int numTables)
 			tbinfo->attcollation[j] = atooid(PQgetvalue(res, j, i_attcollation));
 			tbinfo->attfdwoptions[j] = pg_strdup(PQgetvalue(res, j, i_attfdwoptions));
 			tbinfo->attmissingval[j] = pg_strdup(PQgetvalue(res, j, i_attmissingval));
+			tbinfo->attcmoptions[j] = pg_strdup(PQgetvalue(res, j, i_attcmoptions));
+			tbinfo->attcmnames[j] = pg_strdup(PQgetvalue(res, j, i_attcmname));
 			tbinfo->attrdefs[j] = NULL; /* fix below */
 			if (PQgetvalue(res, j, i_atthasdef)[0] == 't')
 				hasdefaults = true;
@@ -8518,6 +8554,104 @@ getTableAttrs(Archive *fout, TableInfo *tblinfo, int numTables)
 			}
 			PQclear(res);
 		}
+
+		/*
+		 * Get compression info
+		 */
+		if (fout->remoteVersion >= 120000 && dopt->binary_upgrade)
+		{
+			int			i_acname;
+			int			i_acoid;
+			int			i_parsedoptions;
+			int			i_curattnum;
+			int			start;
+
+			if (g_verbose)
+				write_msg(NULL, "finding compression info for table \"%s.%s\"\n",
+						  tbinfo->dobj.namespace->dobj.name,
+						  tbinfo->dobj.name);
+
+			tbinfo->attcompression = pg_malloc0(tbinfo->numatts * sizeof(AttrCompressionInfo *));
+
+			resetPQExpBuffer(q);
+			appendPQExpBuffer(q,
+				"SELECT attrelid::pg_catalog.regclass AS relname, attname,"
+				" (CASE WHEN deptype = 'i' THEN refobjsubid ELSE objsubid END) AS curattnum,"
+				" (CASE WHEN deptype = 'n' THEN attcompression = refobjid"
+				"		ELSE attcompression = objid END) AS iscurrent,"
+				" acname, acoid,"
+				" (CASE WHEN acoptions IS NOT NULL"
+				"  THEN pg_catalog.array_to_string(ARRAY("
+				"		SELECT pg_catalog.quote_ident(option_name) || "
+				"			' ' || pg_catalog.quote_literal(option_value) "
+				"		FROM pg_catalog.pg_options_to_table(acoptions) "
+				"		ORDER BY option_name"
+				"		), E',\n    ')"
+				"  ELSE NULL END) AS parsedoptions "
+				" FROM pg_depend d"
+				" JOIN pg_attribute a ON"
+				"	(classid = 'pg_class'::pg_catalog.regclass::pg_catalog.oid AND a.attrelid = d.objid"
+				"		AND a.attnum = d.objsubid AND d.deptype = 'n'"
+				"		AND d.refclassid = 'pg_attr_compression'::pg_catalog.regclass::pg_catalog.oid)"
+				"	OR (d.refclassid = 'pg_class'::pg_catalog.regclass::pg_catalog.oid"
+				"		AND d.refobjid = a.attrelid"
+				"		AND d.refobjsubid = a.attnum AND d.deptype = 'i'"
+				"		AND d.classid = 'pg_attr_compression'::pg_catalog.regclass::pg_catalog.oid)"
+				" JOIN pg_attr_compression c ON"
+				"	(d.deptype = 'i' AND d.objid = c.acoid AND a.attnum = c.acattnum"
+				"		AND a.attrelid = c.acrelid) OR"
+				"	(d.deptype = 'n' AND d.refobjid = c.acoid AND c.acattnum = 0"
+				"		AND c.acrelid = 0)"
+				" WHERE (deptype = 'n' AND d.objid = %d) OR (deptype = 'i' AND d.refobjid = %d)"
+				" ORDER BY curattnum, iscurrent;",
+				tbinfo->dobj.catId.oid, tbinfo->dobj.catId.oid);
+
+			res = ExecuteSqlQuery(fout, q->data, PGRES_TUPLES_OK);
+			ntups = PQntuples(res);
+
+			if (ntups > 0)
+			{
+				int		k;
+
+				i_acname = PQfnumber(res, "acname");
+				i_acoid = PQfnumber(res, "acoid");
+				i_parsedoptions = PQfnumber(res, "parsedoptions");
+				i_curattnum = PQfnumber(res, "curattnum");
+
+				start = 0;
+
+				for (j = 0; j < ntups; j++)
+				{
+					int		attnum = atoi(PQgetvalue(res, j, i_curattnum));
+
+					if ((j == ntups - 1) || atoi(PQgetvalue(res, j + 1, i_curattnum)) != attnum)
+					{
+						AttrCompressionInfo *cminfo = pg_malloc(sizeof(AttrCompressionInfo));
+
+						cminfo->nitems = j - start + 1;
+						cminfo->items = pg_malloc(sizeof(AttrCompressionItem *) * cminfo->nitems);
+
+						for (k = start; k < start + cminfo->nitems; k++)
+						{
+							AttrCompressionItem	*cmitem = pg_malloc0(sizeof(AttrCompressionItem));
+
+							cmitem->acname = pg_strdup(PQgetvalue(res, k, i_acname));
+							cmitem->acoid = atooid(PQgetvalue(res, k, i_acoid));
+
+							if (!PQgetisnull(res, k, i_parsedoptions))
+								cmitem->parsedoptions = pg_strdup(PQgetvalue(res, k, i_parsedoptions));
+
+							cminfo->items[k - start] = cmitem;
+						}
+
+						tbinfo->attcompression[attnum - 1] = cminfo;
+						start = j + 1;	/* start from next */
+					}
+				}
+			}
+
+			PQclear(res);
+		}
 	}
 
 	destroyPQExpBuffer(q);
@@ -12575,6 +12709,9 @@ dumpAccessMethod(Archive *fout, AccessMethodInfo *aminfo)
 		case AMTYPE_INDEX:
 			appendPQExpBuffer(q, "TYPE INDEX ");
 			break;
+		case AMTYPE_COMPRESSION:
+			appendPQExpBuffer(q, "TYPE COMPRESSION ");
+			break;
 		default:
 			write_msg(NULL, "WARNING: invalid type \"%c\" of access method \"%s\"\n",
 					  aminfo->amtype, qamname);
@@ -15500,6 +15637,14 @@ dumpTableSchema(Archive *fout, TableInfo *tbinfo)
 											   (!tbinfo->inhNotNull[j] ||
 												dopt->binary_upgrade));
 
+					/*
+					 * Compression will require a record in
+					 * pg_attr_compression
+					 */
+					bool		has_custom_compression = (tbinfo->attcmnames[j] &&
+														  ((strcmp(tbinfo->attcmnames[j], "pglz") != 0) ||
+														   nonemptyReloptions(tbinfo->attcmoptions[j])));
+
 					/*
 					 * Skip column if fully defined by reloftype or the
 					 * partition parent.
@@ -15558,6 +15703,25 @@ dumpTableSchema(Archive *fout, TableInfo *tbinfo)
 											  fmtQualifiedDumpable(coll));
 					}
 
+					/*
+					 * Compression
+					 *
+					 * In binary-upgrade mode, compression is assigned by
+					 * ALTER. Even if we're skipping compression the attribute
+					 * will get default compression. It's the task for ALTER
+					 * command to restore compression info.
+					 */
+					if (!dopt->no_compression_methods && !dopt->binary_upgrade &&
+						tbinfo->attcmnames[j] && strlen(tbinfo->attcmnames[j]) &&
+						has_custom_compression)
+					{
+						appendPQExpBuffer(q, " COMPRESSION %s",
+										  tbinfo->attcmnames[j]);
+						if (nonemptyReloptions(tbinfo->attcmoptions[j]))
+							appendPQExpBuffer(q, " WITH (%s)",
+											  tbinfo->attcmoptions[j]);
+					}
+
 					if (has_default)
 						appendPQExpBuffer(q, " DEFAULT %s",
 										  tbinfo->attrdefs[j]->adef_expr);
@@ -15973,6 +16137,34 @@ dumpTableSchema(Archive *fout, TableInfo *tbinfo)
 				appendPQExpBuffer(q, "OPTIONS (\n    %s\n);\n",
 								  tbinfo->attfdwoptions[j]);
 			}
+
+			/*
+			 * Dump per-column compression options
+			 */
+			if (tbinfo->attcompression && tbinfo->attcompression[j])
+			{
+				AttrCompressionInfo *cminfo = tbinfo->attcompression[j];
+
+				if (cminfo->nitems)
+					appendPQExpBuffer(q, "\n-- For binary upgrade, recreate compression metadata on column %s\n",
+							fmtId(tbinfo->attnames[j]));
+
+				for (int i = 0; i < cminfo->nitems; i++)
+				{
+					AttrCompressionItem *item = cminfo->items[i];
+
+					appendPQExpBuffer(q,
+						"SELECT binary_upgrade_set_next_attr_compression_oid('%d'::pg_catalog.oid);\n",
+									  item->acoid);
+					appendPQExpBuffer(q, "ALTER TABLE %s ALTER COLUMN %s\nSET COMPRESSION %s",
+									  qualrelname, fmtId(tbinfo->attnames[j]), item->acname);
+
+					if (item->parsedoptions)
+						appendPQExpBuffer(q, "\nWITH (%s);\n", item->parsedoptions);
+					else
+						appendPQExpBuffer(q, ";\n");
+				}
+			}
 		}
 	}
 
diff --git a/src/bin/pg_dump/pg_dump.h b/src/bin/pg_dump/pg_dump.h
index 1448005f30..582d661dd2 100644
--- a/src/bin/pg_dump/pg_dump.h
+++ b/src/bin/pg_dump/pg_dump.h
@@ -325,6 +325,10 @@ typedef struct _tableInfo
 	char	   *partbound;		/* partition bound definition */
 	bool		needs_override; /* has GENERATED ALWAYS AS IDENTITY */
 
+	char	  **attcmoptions;	/* per-attribute current compression options */
+	char	  **attcmnames;		/* per-attribute current compression method names */
+	struct _attrCompressionInfo **attcompression; /* per-attribute all compression data */
+
 	/*
 	 * Stuff computed only for dumpable tables.
 	 */
@@ -346,6 +350,19 @@ typedef struct _attrDefInfo
 	bool		separate;		/* true if must dump as separate item */
 } AttrDefInfo;
 
+typedef struct _attrCompressionItem
+{
+	Oid			acoid;			/* attribute compression oid */
+	char	   *acname;			/* compression access method name */
+	char	   *parsedoptions;	/* WITH options */
+} AttrCompressionItem;
+
+typedef struct _attrCompressionInfo
+{
+	int			nitems;
+	AttrCompressionItem	**items;
+} AttrCompressionInfo;
+
 typedef struct _tableDataInfo
 {
 	DumpableObject dobj;
diff --git a/src/bin/pg_dump/pg_dumpall.c b/src/bin/pg_dump/pg_dumpall.c
index eb29d318a4..61d55c7082 100644
--- a/src/bin/pg_dump/pg_dumpall.c
+++ b/src/bin/pg_dump/pg_dumpall.c
@@ -74,6 +74,7 @@ static int	no_comments = 0;
 static int	no_publications = 0;
 static int	no_security_labels = 0;
 static int	no_subscriptions = 0;
+static int	no_compression_methods = 0;
 static int	no_unlogged_table_data = 0;
 static int	no_role_passwords = 0;
 static int	server_version;
@@ -136,6 +137,7 @@ main(int argc, char *argv[])
 		{"no-role-passwords", no_argument, &no_role_passwords, 1},
 		{"no-security-labels", no_argument, &no_security_labels, 1},
 		{"no-subscriptions", no_argument, &no_subscriptions, 1},
+		{"no-compression-methods", no_argument, &no_compression_methods, 1},
 		{"no-sync", no_argument, NULL, 4},
 		{"no-unlogged-table-data", no_argument, &no_unlogged_table_data, 1},
 		{"on-conflict-do-nothing", no_argument, &on_conflict_do_nothing, 1},
@@ -406,6 +408,8 @@ main(int argc, char *argv[])
 		appendPQExpBufferStr(pgdumpopts, " --no-security-labels");
 	if (no_subscriptions)
 		appendPQExpBufferStr(pgdumpopts, " --no-subscriptions");
+	if (no_compression_methods)
+		appendPQExpBufferStr(pgdumpopts, " --no-compression-methods");
 	if (no_unlogged_table_data)
 		appendPQExpBufferStr(pgdumpopts, " --no-unlogged-table-data");
 	if (on_conflict_do_nothing)
@@ -622,6 +626,7 @@ help(void)
 	printf(_("  --no-role-passwords          do not dump passwords for roles\n"));
 	printf(_("  --no-security-labels         do not dump security label assignments\n"));
 	printf(_("  --no-subscriptions           do not dump subscriptions\n"));
+	printf(_("  --no-compression-methods     do not dump compression methods\n"));
 	printf(_("  --no-sync                    do not wait for changes to be written safely to disk\n"));
 	printf(_("  --no-tablespaces             do not dump tablespace assignments\n"));
 	printf(_("  --no-unlogged-table-data     do not dump unlogged table data\n"));
diff --git a/src/bin/pg_dump/pg_restore.c b/src/bin/pg_dump/pg_restore.c
index 501d7cea72..78758107f2 100644
--- a/src/bin/pg_dump/pg_restore.c
+++ b/src/bin/pg_dump/pg_restore.c
@@ -75,6 +75,7 @@ main(int argc, char **argv)
 	static int	no_publications = 0;
 	static int	no_security_labels = 0;
 	static int	no_subscriptions = 0;
+	static int	no_compression_methods = 0;
 	static int	strict_names = 0;
 
 	struct option cmdopts[] = {
@@ -124,6 +125,7 @@ main(int argc, char **argv)
 		{"no-publications", no_argument, &no_publications, 1},
 		{"no-security-labels", no_argument, &no_security_labels, 1},
 		{"no-subscriptions", no_argument, &no_subscriptions, 1},
+		{"no-compression-methods", no_argument, &no_compression_methods, 1},
 
 		{NULL, 0, NULL, 0}
 	};
@@ -364,6 +366,7 @@ main(int argc, char **argv)
 	opts->no_publications = no_publications;
 	opts->no_security_labels = no_security_labels;
 	opts->no_subscriptions = no_subscriptions;
+	opts->no_compression_methods = no_compression_methods;
 
 	if (if_exists && !opts->dropSchema)
 	{
diff --git a/src/bin/pg_dump/t/002_pg_dump.pl b/src/bin/pg_dump/t/002_pg_dump.pl
index ec751a7c23..432b65ef00 100644
--- a/src/bin/pg_dump/t/002_pg_dump.pl
+++ b/src/bin/pg_dump/t/002_pg_dump.pl
@@ -650,6 +650,43 @@ my %tests = (
 		},
 	},
 
+	# compression data in binary upgrade mode
+	'ALTER TABLE test_table_compression ALTER COLUMN ... SET COMPRESSION' => {
+		all_runs  => 1,
+		catch_all => 'ALTER TABLE ... commands',
+		regexp    => qr/^
+			\QCREATE TABLE dump_test.test_table_compression (\E\n
+			\s+\Qcol1 text,\E\n
+			\s+\Qcol2 text,\E\n
+			\s+\Qcol3 text,\E\n
+			\s+\Qcol4 text\E\n
+			\);
+			.*
+			\QSELECT binary_upgrade_set_next_attr_compression_oid('\E\d+\Q'::pg_catalog.oid);\E\n
+			\QALTER TABLE dump_test.test_table_compression ALTER COLUMN col1\E\n
+			\QSET COMPRESSION pglz;\E\n
+			.*
+			\QSELECT binary_upgrade_set_next_attr_compression_oid('\E\d+\Q'::pg_catalog.oid);\E\n
+			\QALTER TABLE dump_test.test_table_compression ALTER COLUMN col2\E\n
+			\QSET COMPRESSION pglz2;\E\n
+			.*
+			\QSELECT binary_upgrade_set_next_attr_compression_oid('\E\d+\Q'::pg_catalog.oid);\E\n
+			\QALTER TABLE dump_test.test_table_compression ALTER COLUMN col3\E\n
+			\QSET COMPRESSION pglz\E\n
+			\QWITH (min_input_size '1000');\E\n
+			.*
+			\QSELECT binary_upgrade_set_next_attr_compression_oid('\E\d+\Q'::pg_catalog.oid);\E\n
+			\QALTER TABLE dump_test.test_table_compression ALTER COLUMN col4\E\n
+			\QSET COMPRESSION pglz2\E\n
+			\QWITH (min_input_size '1000');\E\n
+			\QSELECT binary_upgrade_set_next_attr_compression_oid('\E\d+\Q'::pg_catalog.oid);\E\n
+			\QALTER TABLE dump_test.test_table_compression ALTER COLUMN col4\E\n
+			\QSET COMPRESSION pglz2\E\n
+			\QWITH (min_input_size '2000');\E\n
+			/xms,
+		like => { binary_upgrade => 1, },
+	},
+
 	'ALTER TABLE ONLY test_table ALTER COLUMN col1 SET STATISTICS 90' => {
 		create_order => 93,
 		create_sql =>
@@ -1400,6 +1437,17 @@ my %tests = (
 		like => { %full_runs, section_pre_data => 1, },
 	},
 
+	'CREATE ACCESS METHOD pglz2' => {
+		all_runs     => 1,
+		catch_all    => 'CREATE ... commands',
+		create_order => 52,
+		create_sql =>
+		  'CREATE ACCESS METHOD pglz2 TYPE COMPRESSION HANDLER pglzhandler;',
+		regexp =>
+		  qr/CREATE ACCESS METHOD pglz2 TYPE COMPRESSION HANDLER pglzhandler;/m,
+		like => { %full_runs, section_pre_data => 1, },
+	},
+
 	'CREATE COLLATION test0 FROM "C"' => {
 		create_order => 76,
 		create_sql   => 'CREATE COLLATION test0 FROM "C";',
@@ -2420,6 +2468,53 @@ my %tests = (
 		unlike => { exclude_dump_test_schema => 1, },
 	},
 
+	'CREATE TABLE test_table_compression' => {
+		create_order => 55,
+		create_sql   => 'CREATE TABLE dump_test.test_table_compression (
+						   col1 text,
+						   col2 text COMPRESSION pglz2,
+						   col3 text COMPRESSION pglz WITH (min_input_size \'1000\'),
+						   col4 text COMPRESSION pglz2 WITH (min_input_size \'1000\')
+					     );',
+		regexp => qr/^
+			\QCREATE TABLE dump_test.test_table_compression (\E\n
+			\s+\Qcol1 text,\E\n
+			\s+\Qcol2 text COMPRESSION pglz2,\E\n
+			\s+\Qcol3 text COMPRESSION pglz WITH (min_input_size '1000'),\E\n
+			\s+\Qcol4 text COMPRESSION pglz2 WITH (min_input_size '2000')\E\n
+			\);
+			/xm,
+		like =>
+		  { %full_runs, %dump_test_schema_runs, section_pre_data => 1, },
+		unlike => {
+			binary_upgrade		     => 1,
+			exclude_dump_test_schema => 1,
+		},
+	},
+
+	'ALTER TABLE test_table_compression' => {
+		create_order => 56,
+		create_sql   => 'ALTER TABLE dump_test.test_table_compression
+						 ALTER COLUMN col4
+						 SET COMPRESSION pglz2
+						 WITH (min_input_size \'2000\')
+						 PRESERVE (pglz2);',
+		regexp => qr/^
+			\QCREATE TABLE dump_test.test_table_compression (\E\n
+			\s+\Qcol1 text,\E\n
+			\s+\Qcol2 text COMPRESSION pglz2,\E\n
+			\s+\Qcol3 text COMPRESSION pglz WITH (min_input_size '1000'),\E\n
+			\s+\Qcol4 text COMPRESSION pglz2 WITH (min_input_size '2000')\E\n
+			\);
+			/xm,
+		like =>
+		  { %full_runs, %dump_test_schema_runs, section_pre_data => 1, },
+		unlike => {
+			binary_upgrade		     => 1,
+			exclude_dump_test_schema => 1,
+		},
+	},
+
 	'CREATE STATISTICS extended_stats_no_options' => {
 		create_order => 97,
 		create_sql   => 'CREATE STATISTICS dump_test.test_ext_stats_no_options
diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c
index 4ca0db1d0c..58bc222c0d 100644
--- a/src/bin/psql/describe.c
+++ b/src/bin/psql/describe.c
@@ -1467,6 +1467,7 @@ describeOneTableDetails(const char *schemaname,
 				fdwopts_col = -1,
 				attstorage_col = -1,
 				attstattarget_col = -1,
+				attcompression_col = -1,
 				attdescr_col = -1;
 	int			numrows;
 	struct
@@ -1835,6 +1836,24 @@ describeOneTableDetails(const char *schemaname,
 		appendPQExpBufferStr(&buf, ",\n  a.attstorage");
 		attstorage_col = cols++;
 
+		/* compresssion info */
+		if (pset.sversion >= 120000 &&
+			(tableinfo.relkind == RELKIND_RELATION ||
+			 tableinfo.relkind == RELKIND_PARTITIONED_TABLE))
+		{
+			appendPQExpBufferStr(&buf, ",\n  CASE WHEN attcompression = 0 THEN NULL ELSE "
+								 " (SELECT c.acname || "
+								 "		(CASE WHEN acoptions IS NULL "
+								 "		 THEN '' "
+								 "		 ELSE '(' || array_to_string(ARRAY(SELECT quote_ident(option_name) || ' ' || quote_literal(option_value)"
+								 "											  FROM pg_options_to_table(acoptions)), ', ') || ')'"
+								 " 		 END) "
+								 "  FROM pg_catalog.pg_attr_compression c "
+								 "  WHERE c.acoid = a.attcompression) "
+								 " END AS attcmname");
+			attcompression_col = cols++;
+		}
+
 		/* stats target, if relevant to relkind */
 		if (tableinfo.relkind == RELKIND_RELATION ||
 			tableinfo.relkind == RELKIND_INDEX ||
@@ -1954,6 +1973,8 @@ describeOneTableDetails(const char *schemaname,
 		headers[cols++] = gettext_noop("FDW options");
 	if (attstorage_col >= 0)
 		headers[cols++] = gettext_noop("Storage");
+	if (attcompression_col >= 0)
+		headers[cols++] = gettext_noop("Compression");
 	if (attstattarget_col >= 0)
 		headers[cols++] = gettext_noop("Stats target");
 	if (attdescr_col >= 0)
@@ -2025,6 +2046,27 @@ describeOneTableDetails(const char *schemaname,
 							  false, false);
 		}
 
+		/* Column compression. */
+		if (attcompression_col >= 0)
+		{
+			bool		mustfree = false;
+			const int	trunclen = 100;
+			char *val = PQgetvalue(res, i, attcompression_col);
+
+			/* truncate the options if they're too long */
+			if (strlen(val) > trunclen + 3)
+			{
+				char *trunc = pg_malloc0(trunclen + 4);
+				strncpy(trunc, val, trunclen);
+				strncpy(trunc + trunclen, "...", 4);
+
+				val = trunc;
+				mustfree = true;
+			}
+
+			printTableAddCell(&cont, val, false, mustfree);
+		}
+
 		/* Statistics target, if the relkind supports this feature */
 		if (attstattarget_col >= 0)
 			printTableAddCell(&cont, PQgetvalue(res, i, attstattarget_col),
diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index bb696f8ee9..8cfb0304a8 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -2161,11 +2161,14 @@ psql_completion(const char *text, int start, int end)
 	/* ALTER TABLE ALTER [COLUMN] <foo> SET */
 	else if (Matches7("ALTER", "TABLE", MatchAny, "ALTER", "COLUMN", MatchAny, "SET") ||
 			 Matches6("ALTER", "TABLE", MatchAny, "ALTER", MatchAny, "SET"))
-		COMPLETE_WITH_LIST5("(", "DEFAULT", "NOT NULL", "STATISTICS", "STORAGE");
+		COMPLETE_WITH_LIST6("(", "COMPRESSION", "DEFAULT", "NOT NULL", "STATISTICS", "STORAGE");
 	/* ALTER TABLE ALTER [COLUMN] <foo> SET ( */
 	else if (Matches8("ALTER", "TABLE", MatchAny, "ALTER", "COLUMN", MatchAny, "SET", "(") ||
 			 Matches7("ALTER", "TABLE", MatchAny, "ALTER", MatchAny, "SET", "("))
 		COMPLETE_WITH_LIST2("n_distinct", "n_distinct_inherited");
+	else if (Matches9("ALTER", "TABLE", MatchAny, "ALTER", "COLUMN", MatchAny, "SET", "COMPRESSION", MatchAny) ||
+			 Matches8("ALTER", "TABLE", MatchAny, "ALTER", MatchAny, "SET", "COMPRESSION", MatchAny))
+		COMPLETE_WITH_CONST("WITH (");
 	/* ALTER TABLE ALTER [COLUMN] <foo> SET STORAGE */
 	else if (Matches8("ALTER", "TABLE", MatchAny, "ALTER", "COLUMN", MatchAny, "SET", "STORAGE") ||
 			 Matches7("ALTER", "TABLE", MatchAny, "ALTER", MatchAny, "SET", "STORAGE"))
diff --git a/src/include/catalog/binary_upgrade.h b/src/include/catalog/binary_upgrade.h
index abc6e1ae1d..1e95a3863a 100644
--- a/src/include/catalog/binary_upgrade.h
+++ b/src/include/catalog/binary_upgrade.h
@@ -25,6 +25,8 @@ extern PGDLLIMPORT Oid binary_upgrade_next_toast_pg_class_oid;
 extern PGDLLIMPORT Oid binary_upgrade_next_pg_enum_oid;
 extern PGDLLIMPORT Oid binary_upgrade_next_pg_authid_oid;
 
+extern PGDLLIMPORT Oid binary_upgrade_next_attr_compression_oid;
+
 extern PGDLLIMPORT bool binary_upgrade_record_init_privs;
 
 #endif							/* BINARY_UPGRADE_H */
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index 53891aacc0..06a0576bd8 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -10061,6 +10061,10 @@
   proname => 'binary_upgrade_set_missing_value', provolatile => 'v',
   proparallel => 'u', prorettype => 'void', proargtypes => 'oid text text',
   prosrc => 'binary_upgrade_set_missing_value' },
+{ oid => '4012', descr => 'for use by pg_upgrade',
+  proname => 'binary_upgrade_set_next_attr_compression_oid', provolatile => 'v',
+  proparallel => 'r', prorettype => 'void', proargtypes => 'oid',
+  prosrc => 'binary_upgrade_set_next_attr_compression_oid' },
 
 # replication/origin.h
 { oid => '6003', descr => 'create a replication origin',
-- 
2.18.0


--MP_/tqROVSJLfUtKS/DWevR5Hf=
Content-Type: text/x-patch
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
 filename=0007-Add-tests-for-compression-methods-v19.patch



^ permalink  raw  reply  [nested|flat] 18+ messages in thread

* [PATCH 6/8] Add psql, pg_dump and pg_upgrade support
@ 2018-06-18 12:57 Ildus Kurbangaliev <i.kurbangaliev@gmail.com>
  0 siblings, 0 replies; 18+ messages in thread

From: Ildus Kurbangaliev @ 2018-06-18 12:57 UTC (permalink / raw)

Signed-off-by: Ildus Kurbangaliev <i.kurbangaliev@gmail.com>
---
 src/backend/commands/compressioncmds.c     |  80 ++++++---
 src/backend/commands/tablecmds.c           |  14 +-
 src/backend/utils/adt/pg_upgrade_support.c |  10 ++
 src/bin/pg_dump/pg_backup.h                |   2 +
 src/bin/pg_dump/pg_dump.c                  | 195 ++++++++++++++++++++-
 src/bin/pg_dump/pg_dump.h                  |  17 ++
 src/bin/pg_dump/pg_dumpall.c               |   5 +
 src/bin/pg_dump/pg_restore.c               |   3 +
 src/bin/pg_dump/t/002_pg_dump.pl           |  95 ++++++++++
 src/bin/psql/describe.c                    |  42 +++++
 src/bin/psql/tab-complete.c                |   5 +-
 src/include/catalog/binary_upgrade.h       |   2 +
 src/include/catalog/pg_proc.dat            |   4 +
 13 files changed, 432 insertions(+), 42 deletions(-)

diff --git a/src/backend/commands/compressioncmds.c b/src/backend/commands/compressioncmds.c
index 7841c7700a..89ffa227b0 100644
--- a/src/backend/commands/compressioncmds.c
+++ b/src/backend/commands/compressioncmds.c
@@ -36,6 +36,9 @@
 #include "utils/syscache.h"
 #include "utils/snapmgr.h"
 
+/* Set by pg_upgrade_support functions */
+Oid			binary_upgrade_next_attr_compression_oid = InvalidOid;
+
 /*
  * When conditions of compression satisfies one if builtin attribute
  * compresssion tuples the compressed attribute will be linked to
@@ -129,11 +132,12 @@ lookup_attribute_compression(Oid attrelid, AttrNumber attnum,
 					tup_amoid;
 		Datum		values[Natts_pg_attr_compression];
 		bool		nulls[Natts_pg_attr_compression];
+		char	   *amname;
 
 		heap_deform_tuple(tuple, RelationGetDescr(rel), values, nulls);
 		acoid = DatumGetObjectId(values[Anum_pg_attr_compression_acoid - 1]);
-		tup_amoid = get_am_oid(
-							   NameStr(*DatumGetName(values[Anum_pg_attr_compression_acname - 1])), false);
+		amname = NameStr(*DatumGetName(values[Anum_pg_attr_compression_acname - 1]));
+		tup_amoid = get_am_oid(amname, false);
 
 		if (previous_amoids)
 			*previous_amoids = list_append_unique_oid(*previous_amoids, tup_amoid);
@@ -150,17 +154,15 @@ lookup_attribute_compression(Oid attrelid, AttrNumber attnum,
 			if (DatumGetPointer(acoptions) == NULL)
 				result = acoid;
 		}
-		else
+		else if (DatumGetPointer(acoptions) != NULL)
 		{
 			bool		equal;
 
 			/* check if arrays for WITH options are equal */
 			equal = DatumGetBool(CallerFInfoFunctionCall2(
-														  array_eq,
-														  &arrayeq_info,
-														  InvalidOid,
-														  acoptions,
-														  values[Anum_pg_attr_compression_acoptions - 1]));
+						array_eq, &arrayeq_info, InvalidOid, acoptions,
+						values[Anum_pg_attr_compression_acoptions - 1]));
+
 			if (equal)
 				result = acoid;
 		}
@@ -227,6 +229,16 @@ CreateAttributeCompression(Form_pg_attribute att,
 	/* Try to find builtin compression first */
 	acoid = lookup_attribute_compression(0, 0, amoid, arropt, NULL);
 
+	/* no rewrite by default */
+	if (need_rewrite != NULL)
+		*need_rewrite = false;
+
+	if (IsBinaryUpgrade)
+	{
+		/* Skip the rewrite checks and searching of identical compression */
+		goto add_tuple;
+	}
+
 	/*
 	 * attrelid will be invalid on CREATE TABLE, no need for table rewrite
 	 * check.
@@ -252,16 +264,10 @@ CreateAttributeCompression(Form_pg_attribute att,
 		 */
 		if (need_rewrite != NULL)
 		{
-			/* no rewrite by default */
-			*need_rewrite = false;
-
 			Assert(preserved_amoids != NULL);
 
 			if (compression->preserve == NIL)
-			{
-				Assert(!IsBinaryUpgrade);
 				*need_rewrite = true;
-			}
 			else
 			{
 				ListCell   *cell;
@@ -294,7 +300,7 @@ CreateAttributeCompression(Form_pg_attribute att,
 				 * In binary upgrade list will not be free since it contains
 				 * Oid of builtin compression access method.
 				 */
-				if (!IsBinaryUpgrade && list_length(previous_amoids) != 0)
+				if (list_length(previous_amoids) != 0)
 					*need_rewrite = true;
 			}
 		}
@@ -303,9 +309,6 @@ CreateAttributeCompression(Form_pg_attribute att,
 		list_free(previous_amoids);
 	}
 
-	if (IsBinaryUpgrade && !OidIsValid(acoid))
-		elog(ERROR, "could not restore attribute compression data");
-
 	/* Return Oid if we already found identical compression on this column */
 	if (OidIsValid(acoid))
 	{
@@ -315,6 +318,7 @@ CreateAttributeCompression(Form_pg_attribute att,
 		return acoid;
 	}
 
+add_tuple:
 	/* Initialize buffers for new tuple values */
 	memset(values, 0, sizeof(values));
 	memset(nulls, false, sizeof(nulls));
@@ -323,13 +327,27 @@ CreateAttributeCompression(Form_pg_attribute att,
 
 	rel = heap_open(AttrCompressionRelationId, RowExclusiveLock);
 
-	acoid = GetNewOidWithIndex(rel, AttrCompressionIndexId,
-							   Anum_pg_attr_compression_acoid);
+	if (IsBinaryUpgrade)
+	{
+		/* acoid should be found in some cases */
+		if (binary_upgrade_next_attr_compression_oid < FirstNormalObjectId &&
+			(!OidIsValid(acoid) || binary_upgrade_next_attr_compression_oid != acoid))
+			elog(ERROR, "could not link to built-in attribute compression");
+
+		acoid = binary_upgrade_next_attr_compression_oid;
+	}
+	else
+	{
+		acoid = GetNewOidWithIndex(rel, AttrCompressionIndexId,
+									Anum_pg_attr_compression_acoid);
+
+	}
+
 	if (acoid < FirstNormalObjectId)
 	{
-		/* this is database initialization */
+		/* this is built-in attribute compression */
 		heap_close(rel, RowExclusiveLock);
-		return DefaultCompressionOid;
+		return acoid;
 	}
 
 	/* we need routine only to call cmcheck function */
@@ -390,8 +408,8 @@ RemoveAttributeCompression(Oid acoid)
 /*
  * CleanupAttributeCompression
  *
- * Remove entries in pg_attr_compression except current attribute compression
- * and related with specified list of access methods.
+ * Remove entries in pg_attr_compression of the column except current
+ * attribute compression and related with specified list of access methods.
  */
 void
 CleanupAttributeCompression(Oid relid, AttrNumber attnum, List *keepAmOids)
@@ -419,9 +437,7 @@ CleanupAttributeCompression(Oid relid, AttrNumber attnum, List *keepAmOids)
 	ReleaseSysCache(attrtuple);
 
 	Assert(relid > 0 && attnum > 0);
-
-	if (IsBinaryUpgrade)
-		goto builtin_removal;
+	Assert(!IsBinaryUpgrade);
 
 	rel = heap_open(AttrCompressionRelationId, RowExclusiveLock);
 
@@ -438,7 +454,10 @@ CleanupAttributeCompression(Oid relid, AttrNumber attnum, List *keepAmOids)
 	scan = systable_beginscan(rel, AttrCompressionRelidAttnumIndexId,
 							  true, NULL, 2, key);
 
-	/* Remove attribute compression tuples and collect removed Oids to list */
+	/*
+	 * Remove attribute compression tuples and collect removed Oids
+	 * to list.
+	 */
 	while (HeapTupleIsValid(tuple = systable_getnext(scan)))
 	{
 		Form_pg_attr_compression acform;
@@ -460,7 +479,10 @@ CleanupAttributeCompression(Oid relid, AttrNumber attnum, List *keepAmOids)
 	systable_endscan(scan);
 	heap_close(rel, RowExclusiveLock);
 
-	/* Now remove dependencies */
+	/*
+	 * Now remove dependencies between attribute compression (dependent)
+	 * and column.
+	 */
 	rel = heap_open(DependRelationId, RowExclusiveLock);
 	foreach(lc, removed)
 	{
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 185c9650c7..b3ff97a5cb 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -798,10 +798,10 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId,
 		if (colDef->identity)
 			attr->attidentity = colDef->identity;
 
-		if (relkind == RELKIND_RELATION || relkind == RELKIND_PARTITIONED_TABLE)
+		if (!IsBinaryUpgrade &&
+			(relkind == RELKIND_RELATION || relkind == RELKIND_PARTITIONED_TABLE))
 			attr->attcompression = CreateAttributeCompression(attr,
-															  colDef->compression,
-															  NULL, NULL);
+										colDef->compression, NULL, NULL);
 		else
 			attr->attcompression = InvalidOid;
 	}
@@ -13696,14 +13696,6 @@ ATExecSetCompression(AlteredTableInfo *tab,
 	/* make changes visible */
 	CommandCounterIncrement();
 
-	/*
-	 * Normally cleanup is done in rewrite but in binary upgrade we should do
-	 * it explicitly.
-	 */
-	if (IsBinaryUpgrade)
-		CleanupAttributeCompression(RelationGetRelid(rel),
-									attnum, preserved_amoids);
-
 	ObjectAddressSet(address, AttrCompressionRelationId, acoid);
 	return address;
 }
diff --git a/src/backend/utils/adt/pg_upgrade_support.c b/src/backend/utils/adt/pg_upgrade_support.c
index 99db5ba389..0e81e70e09 100644
--- a/src/backend/utils/adt/pg_upgrade_support.c
+++ b/src/backend/utils/adt/pg_upgrade_support.c
@@ -116,6 +116,16 @@ binary_upgrade_set_next_pg_authid_oid(PG_FUNCTION_ARGS)
 	PG_RETURN_VOID();
 }
 
+Datum
+binary_upgrade_set_next_attr_compression_oid(PG_FUNCTION_ARGS)
+{
+	Oid			acoid = PG_GETARG_OID(0);
+
+	CHECK_IS_BINARY_UPGRADE;
+	binary_upgrade_next_attr_compression_oid = acoid;
+	PG_RETURN_VOID();
+}
+
 Datum
 binary_upgrade_create_empty_extension(PG_FUNCTION_ARGS)
 {
diff --git a/src/bin/pg_dump/pg_backup.h b/src/bin/pg_dump/pg_backup.h
index 7ab27391fb..2f97e244c7 100644
--- a/src/bin/pg_dump/pg_backup.h
+++ b/src/bin/pg_dump/pg_backup.h
@@ -78,6 +78,7 @@ typedef struct _restoreOptions
 	int			no_publications;	/* Skip publication entries */
 	int			no_security_labels; /* Skip security label entries */
 	int			no_subscriptions;	/* Skip subscription entries */
+	int			no_compression_methods; /* Skip compression methods */
 	int			strict_names;
 
 	const char *filename;
@@ -150,6 +151,7 @@ typedef struct _dumpOptions
 	int			no_security_labels;
 	int			no_publications;
 	int			no_subscriptions;
+	int			no_compression_methods;
 	int			no_synchronized_snapshots;
 	int			no_unlogged_table_data;
 	int			serializable_deferrable;
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 4c98ae4d7f..2e32d4e126 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -41,11 +41,13 @@
 #include "getopt_long.h"
 
 #include "access/attnum.h"
+#include "access/cmapi.h"
 #include "access/sysattr.h"
 #include "access/transam.h"
 #include "catalog/pg_aggregate_d.h"
 #include "catalog/pg_am_d.h"
 #include "catalog/pg_attribute_d.h"
+#include "catalog/pg_attr_compression_d.h"
 #include "catalog/pg_cast_d.h"
 #include "catalog/pg_class_d.h"
 #include "catalog/pg_default_acl_d.h"
@@ -389,6 +391,7 @@ main(int argc, char **argv)
 		{"no-synchronized-snapshots", no_argument, &dopt.no_synchronized_snapshots, 1},
 		{"no-unlogged-table-data", no_argument, &dopt.no_unlogged_table_data, 1},
 		{"no-subscriptions", no_argument, &dopt.no_subscriptions, 1},
+		{"no-compression-methods", no_argument, &dopt.no_compression_methods, 1},
 		{"no-sync", no_argument, NULL, 7},
 		{"on-conflict-do-nothing", no_argument, &dopt.do_nothing, 1},
 		{"rows-per-insert", required_argument, NULL, 10},
@@ -885,6 +888,8 @@ main(int argc, char **argv)
 	 * We rely on dependency information to help us determine a safe order, so
 	 * the initial sort is mostly for cosmetic purposes: we sort by name to
 	 * ensure that logically identical schemas will dump identically.
+	 *
+	 * If we do a parallel dump, we want the largest tables to go first.
 	 */
 	sortDumpableObjectsByTypeName(dobjs, numObjs);
 
@@ -8227,9 +8232,12 @@ getTableAttrs(Archive *fout, TableInfo *tblinfo, int numTables)
 	int			i_attcollation;
 	int			i_attfdwoptions;
 	int			i_attmissingval;
+	int			i_attcmoptions;
+	int			i_attcmname;
 	PGresult   *res;
 	int			ntups;
 	bool		hasdefaults;
+	bool		createWithCompression;
 
 	for (i = 0; i < numTables; i++)
 	{
@@ -8272,6 +8280,23 @@ getTableAttrs(Archive *fout, TableInfo *tblinfo, int numTables)
 						  "a.attislocal,\n"
 						  "pg_catalog.format_type(t.oid, a.atttypmod) AS atttypname,\n");
 
+		createWithCompression = (!dopt->binary_upgrade && fout->remoteVersion >= 120000);
+
+		if (createWithCompression)
+			appendPQExpBuffer(q,
+							  "pg_catalog.array_to_string(ARRAY("
+							  "SELECT pg_catalog.quote_ident(option_name) || "
+							  "' ' || pg_catalog.quote_literal(option_value) "
+							  "FROM pg_catalog.pg_options_to_table(c.acoptions) "
+							  "ORDER BY option_name"
+							  "), E',\n    ') AS attcmoptions,\n"
+							  "c.acname AS attcmname,\n");
+		else
+			appendPQExpBuffer(q,
+							  "NULL AS attcmoptions,\n"
+							  "NULL AS attcmname,\n");
+
+
 		if (fout->remoteVersion >= 110000)
 			appendPQExpBuffer(q,
 							  "CASE WHEN a.atthasmissing AND NOT a.attisdropped "
@@ -8324,7 +8349,13 @@ getTableAttrs(Archive *fout, TableInfo *tblinfo, int numTables)
 		/* need left join here to not fail on dropped columns ... */
 		appendPQExpBuffer(q,
 						  "FROM pg_catalog.pg_attribute a LEFT JOIN pg_catalog.pg_type t "
-						  "ON a.atttypid = t.oid\n"
+						  "ON a.atttypid = t.oid\n");
+
+		if (createWithCompression)
+			appendPQExpBuffer(q, "LEFT JOIN pg_catalog.pg_attr_compression c "
+								 "ON a.attcompression = c.acoid\n");
+
+		appendPQExpBuffer(q,
 						  "WHERE a.attrelid = '%u'::pg_catalog.oid "
 						  "AND a.attnum > 0::pg_catalog.int2\n"
 						  "ORDER BY a.attnum",
@@ -8352,6 +8383,8 @@ getTableAttrs(Archive *fout, TableInfo *tblinfo, int numTables)
 		i_attcollation = PQfnumber(res, "attcollation");
 		i_attfdwoptions = PQfnumber(res, "attfdwoptions");
 		i_attmissingval = PQfnumber(res, "attmissingval");
+		i_attcmname = PQfnumber(res, "attcmname");
+		i_attcmoptions = PQfnumber(res, "attcmoptions");
 
 		tbinfo->numatts = ntups;
 		tbinfo->attnames = (char **) pg_malloc(ntups * sizeof(char *));
@@ -8369,9 +8402,12 @@ getTableAttrs(Archive *fout, TableInfo *tblinfo, int numTables)
 		tbinfo->attcollation = (Oid *) pg_malloc(ntups * sizeof(Oid));
 		tbinfo->attfdwoptions = (char **) pg_malloc(ntups * sizeof(char *));
 		tbinfo->attmissingval = (char **) pg_malloc(ntups * sizeof(char *));
+		tbinfo->attcmoptions = (char **) pg_malloc(ntups * sizeof(char *));
+		tbinfo->attcmnames = (char **) pg_malloc(ntups * sizeof(char *));
 		tbinfo->notnull = (bool *) pg_malloc(ntups * sizeof(bool));
 		tbinfo->inhNotNull = (bool *) pg_malloc(ntups * sizeof(bool));
 		tbinfo->attrdefs = (AttrDefInfo **) pg_malloc(ntups * sizeof(AttrDefInfo *));
+		tbinfo->attcompression = NULL;
 		hasdefaults = false;
 
 		for (j = 0; j < ntups; j++)
@@ -8397,6 +8433,8 @@ getTableAttrs(Archive *fout, TableInfo *tblinfo, int numTables)
 			tbinfo->attcollation[j] = atooid(PQgetvalue(res, j, i_attcollation));
 			tbinfo->attfdwoptions[j] = pg_strdup(PQgetvalue(res, j, i_attfdwoptions));
 			tbinfo->attmissingval[j] = pg_strdup(PQgetvalue(res, j, i_attmissingval));
+			tbinfo->attcmoptions[j] = pg_strdup(PQgetvalue(res, j, i_attcmoptions));
+			tbinfo->attcmnames[j] = pg_strdup(PQgetvalue(res, j, i_attcmname));
 			tbinfo->attrdefs[j] = NULL; /* fix below */
 			if (PQgetvalue(res, j, i_atthasdef)[0] == 't')
 				hasdefaults = true;
@@ -8614,6 +8652,104 @@ getTableAttrs(Archive *fout, TableInfo *tblinfo, int numTables)
 			}
 			PQclear(res);
 		}
+
+		/*
+		 * Get compression info
+		 */
+		if (fout->remoteVersion >= 120000 && dopt->binary_upgrade)
+		{
+			int			i_acname;
+			int			i_acoid;
+			int			i_parsedoptions;
+			int			i_curattnum;
+			int			start;
+
+			if (g_verbose)
+				write_msg(NULL, "finding compression info for table \"%s.%s\"\n",
+						  tbinfo->dobj.namespace->dobj.name,
+						  tbinfo->dobj.name);
+
+			tbinfo->attcompression = pg_malloc0(tbinfo->numatts * sizeof(AttrCompressionInfo *));
+
+			resetPQExpBuffer(q);
+			appendPQExpBuffer(q,
+				"SELECT attrelid::pg_catalog.regclass AS relname, attname,"
+				" (CASE WHEN deptype = 'i' THEN refobjsubid ELSE objsubid END) AS curattnum,"
+				" (CASE WHEN deptype = 'n' THEN attcompression = refobjid"
+				"		ELSE attcompression = objid END) AS iscurrent,"
+				" acname, acoid,"
+				" (CASE WHEN acoptions IS NOT NULL"
+				"  THEN pg_catalog.array_to_string(ARRAY("
+				"		SELECT pg_catalog.quote_ident(option_name) || "
+				"			' ' || pg_catalog.quote_literal(option_value) "
+				"		FROM pg_catalog.pg_options_to_table(acoptions) "
+				"		ORDER BY option_name"
+				"		), E',\n    ')"
+				"  ELSE NULL END) AS parsedoptions "
+				" FROM pg_depend d"
+				" JOIN pg_attribute a ON"
+				"	(classid = 'pg_class'::pg_catalog.regclass::pg_catalog.oid AND a.attrelid = d.objid"
+				"		AND a.attnum = d.objsubid AND d.deptype = 'n'"
+				"		AND d.refclassid = 'pg_attr_compression'::pg_catalog.regclass::pg_catalog.oid)"
+				"	OR (d.refclassid = 'pg_class'::pg_catalog.regclass::pg_catalog.oid"
+				"		AND d.refobjid = a.attrelid"
+				"		AND d.refobjsubid = a.attnum AND d.deptype = 'i'"
+				"		AND d.classid = 'pg_attr_compression'::pg_catalog.regclass::pg_catalog.oid)"
+				" JOIN pg_attr_compression c ON"
+				"	(d.deptype = 'i' AND d.objid = c.acoid AND a.attnum = c.acattnum"
+				"		AND a.attrelid = c.acrelid) OR"
+				"	(d.deptype = 'n' AND d.refobjid = c.acoid AND c.acattnum = 0"
+				"		AND c.acrelid = 0)"
+				" WHERE (deptype = 'n' AND d.objid = %d) OR (deptype = 'i' AND d.refobjid = %d)"
+				" ORDER BY curattnum, iscurrent;",
+				tbinfo->dobj.catId.oid, tbinfo->dobj.catId.oid);
+
+			res = ExecuteSqlQuery(fout, q->data, PGRES_TUPLES_OK);
+			ntups = PQntuples(res);
+
+			if (ntups > 0)
+			{
+				int		k;
+
+				i_acname = PQfnumber(res, "acname");
+				i_acoid = PQfnumber(res, "acoid");
+				i_parsedoptions = PQfnumber(res, "parsedoptions");
+				i_curattnum = PQfnumber(res, "curattnum");
+
+				start = 0;
+
+				for (j = 0; j < ntups; j++)
+				{
+					int		attnum = atoi(PQgetvalue(res, j, i_curattnum));
+
+					if ((j == ntups - 1) || atoi(PQgetvalue(res, j + 1, i_curattnum)) != attnum)
+					{
+						AttrCompressionInfo *cminfo = pg_malloc(sizeof(AttrCompressionInfo));
+
+						cminfo->nitems = j - start + 1;
+						cminfo->items = pg_malloc(sizeof(AttrCompressionItem *) * cminfo->nitems);
+
+						for (k = start; k < start + cminfo->nitems; k++)
+						{
+							AttrCompressionItem	*cmitem = pg_malloc0(sizeof(AttrCompressionItem));
+
+							cmitem->acname = pg_strdup(PQgetvalue(res, k, i_acname));
+							cmitem->acoid = atooid(PQgetvalue(res, k, i_acoid));
+
+							if (!PQgetisnull(res, k, i_parsedoptions))
+								cmitem->parsedoptions = pg_strdup(PQgetvalue(res, k, i_parsedoptions));
+
+							cminfo->items[k - start] = cmitem;
+						}
+
+						tbinfo->attcompression[attnum - 1] = cminfo;
+						start = j + 1;	/* start from next */
+					}
+				}
+			}
+
+			PQclear(res);
+		}
 	}
 
 	destroyPQExpBuffer(q);
@@ -12729,6 +12865,8 @@ dumpAccessMethod(Archive *fout, AccessMethodInfo *aminfo)
 			break;
 		case AMTYPE_TABLE:
 			appendPQExpBuffer(q, "TYPE TABLE ");
+		case AMTYPE_COMPRESSION:
+			appendPQExpBuffer(q, "TYPE COMPRESSION ");
 			break;
 		default:
 			write_msg(NULL, "WARNING: invalid type \"%c\" of access method \"%s\"\n",
@@ -15650,6 +15788,14 @@ dumpTableSchema(Archive *fout, TableInfo *tbinfo)
 											   (!tbinfo->inhNotNull[j] ||
 												dopt->binary_upgrade));
 
+					/*
+					 * Compression will require a record in
+					 * pg_attr_compression
+					 */
+					bool		has_custom_compression = (tbinfo->attcmnames[j] &&
+														  ((strcmp(tbinfo->attcmnames[j], "pglz") != 0) ||
+														   nonemptyReloptions(tbinfo->attcmoptions[j])));
+
 					/*
 					 * Skip column if fully defined by reloftype or the
 					 * partition parent.
@@ -15708,6 +15854,25 @@ dumpTableSchema(Archive *fout, TableInfo *tbinfo)
 											  fmtQualifiedDumpable(coll));
 					}
 
+					/*
+					 * Compression
+					 *
+					 * In binary-upgrade mode, compression is assigned by
+					 * ALTER. Even if we're skipping compression the attribute
+					 * will get default compression. It's the task for ALTER
+					 * command to restore compression info.
+					 */
+					if (!dopt->no_compression_methods && !dopt->binary_upgrade &&
+						tbinfo->attcmnames[j] && strlen(tbinfo->attcmnames[j]) &&
+						has_custom_compression)
+					{
+						appendPQExpBuffer(q, " COMPRESSION %s",
+										  tbinfo->attcmnames[j]);
+						if (nonemptyReloptions(tbinfo->attcmoptions[j]))
+							appendPQExpBuffer(q, " WITH (%s)",
+											  tbinfo->attcmoptions[j]);
+					}
+
 					if (has_default)
 						appendPQExpBuffer(q, " DEFAULT %s",
 										  tbinfo->attrdefs[j]->adef_expr);
@@ -16123,6 +16288,34 @@ dumpTableSchema(Archive *fout, TableInfo *tbinfo)
 				appendPQExpBuffer(q, "OPTIONS (\n    %s\n);\n",
 								  tbinfo->attfdwoptions[j]);
 			}
+
+			/*
+			 * Dump per-column compression options
+			 */
+			if (tbinfo->attcompression && tbinfo->attcompression[j])
+			{
+				AttrCompressionInfo *cminfo = tbinfo->attcompression[j];
+
+				if (cminfo->nitems)
+					appendPQExpBuffer(q, "\n-- For binary upgrade, recreate compression metadata on column %s\n",
+							fmtId(tbinfo->attnames[j]));
+
+				for (int i = 0; i < cminfo->nitems; i++)
+				{
+					AttrCompressionItem *item = cminfo->items[i];
+
+					appendPQExpBuffer(q,
+						"SELECT binary_upgrade_set_next_attr_compression_oid('%d'::pg_catalog.oid);\n",
+									  item->acoid);
+					appendPQExpBuffer(q, "ALTER TABLE %s ALTER COLUMN %s\nSET COMPRESSION %s",
+									  qualrelname, fmtId(tbinfo->attnames[j]), item->acname);
+
+					if (item->parsedoptions)
+						appendPQExpBuffer(q, "\nWITH (%s);\n", item->parsedoptions);
+					else
+						appendPQExpBuffer(q, ";\n");
+				}
+			}
 		}
 
 		if (ftoptions)
diff --git a/src/bin/pg_dump/pg_dump.h b/src/bin/pg_dump/pg_dump.h
index 2e1b90acd0..810d001e84 100644
--- a/src/bin/pg_dump/pg_dump.h
+++ b/src/bin/pg_dump/pg_dump.h
@@ -326,6 +326,10 @@ typedef struct _tableInfo
 	bool		needs_override; /* has GENERATED ALWAYS AS IDENTITY */
 	char	   *amname;			/* relation access method */
 
+	char	  **attcmoptions;	/* per-attribute current compression options */
+	char	  **attcmnames;		/* per-attribute current compression method names */
+	struct _attrCompressionInfo **attcompression; /* per-attribute all compression data */
+
 	/*
 	 * Stuff computed only for dumpable tables.
 	 */
@@ -347,6 +351,19 @@ typedef struct _attrDefInfo
 	bool		separate;		/* true if must dump as separate item */
 } AttrDefInfo;
 
+typedef struct _attrCompressionItem
+{
+	Oid			acoid;			/* attribute compression oid */
+	char	   *acname;			/* compression access method name */
+	char	   *parsedoptions;	/* WITH options */
+} AttrCompressionItem;
+
+typedef struct _attrCompressionInfo
+{
+	int			nitems;
+	AttrCompressionItem	**items;
+} AttrCompressionInfo;
+
 typedef struct _tableDataInfo
 {
 	DumpableObject dobj;
diff --git a/src/bin/pg_dump/pg_dumpall.c b/src/bin/pg_dump/pg_dumpall.c
index a86965e670..f09925a85d 100644
--- a/src/bin/pg_dump/pg_dumpall.c
+++ b/src/bin/pg_dump/pg_dumpall.c
@@ -76,6 +76,7 @@ static int	no_comments = 0;
 static int	no_publications = 0;
 static int	no_security_labels = 0;
 static int	no_subscriptions = 0;
+static int	no_compression_methods = 0;
 static int	no_unlogged_table_data = 0;
 static int	no_role_passwords = 0;
 static int	server_version;
@@ -143,6 +144,7 @@ main(int argc, char *argv[])
 		{"no-role-passwords", no_argument, &no_role_passwords, 1},
 		{"no-security-labels", no_argument, &no_security_labels, 1},
 		{"no-subscriptions", no_argument, &no_subscriptions, 1},
+		{"no-compression-methods", no_argument, &no_compression_methods, 1},
 		{"no-sync", no_argument, NULL, 4},
 		{"no-unlogged-table-data", no_argument, &no_unlogged_table_data, 1},
 		{"on-conflict-do-nothing", no_argument, &on_conflict_do_nothing, 1},
@@ -432,6 +434,8 @@ main(int argc, char *argv[])
 		appendPQExpBufferStr(pgdumpopts, " --no-security-labels");
 	if (no_subscriptions)
 		appendPQExpBufferStr(pgdumpopts, " --no-subscriptions");
+	if (no_compression_methods)
+		appendPQExpBufferStr(pgdumpopts, " --no-compression-methods");
 	if (no_unlogged_table_data)
 		appendPQExpBufferStr(pgdumpopts, " --no-unlogged-table-data");
 	if (on_conflict_do_nothing)
@@ -656,6 +660,7 @@ help(void)
 	printf(_("  --no-role-passwords          do not dump passwords for roles\n"));
 	printf(_("  --no-security-labels         do not dump security label assignments\n"));
 	printf(_("  --no-subscriptions           do not dump subscriptions\n"));
+	printf(_("  --no-compression-methods     do not dump compression methods\n"));
 	printf(_("  --no-sync                    do not wait for changes to be written safely to disk\n"));
 	printf(_("  --no-tablespaces             do not dump tablespace assignments\n"));
 	printf(_("  --no-unlogged-table-data     do not dump unlogged table data\n"));
diff --git a/src/bin/pg_dump/pg_restore.c b/src/bin/pg_dump/pg_restore.c
index 428e040acb..03f84933c6 100644
--- a/src/bin/pg_dump/pg_restore.c
+++ b/src/bin/pg_dump/pg_restore.c
@@ -75,6 +75,7 @@ main(int argc, char **argv)
 	static int	no_publications = 0;
 	static int	no_security_labels = 0;
 	static int	no_subscriptions = 0;
+	static int	no_compression_methods = 0;
 	static int	strict_names = 0;
 
 	struct option cmdopts[] = {
@@ -124,6 +125,7 @@ main(int argc, char **argv)
 		{"no-publications", no_argument, &no_publications, 1},
 		{"no-security-labels", no_argument, &no_security_labels, 1},
 		{"no-subscriptions", no_argument, &no_subscriptions, 1},
+		{"no-compression-methods", no_argument, &no_compression_methods, 1},
 
 		{NULL, 0, NULL, 0}
 	};
@@ -375,6 +377,7 @@ main(int argc, char **argv)
 	opts->no_publications = no_publications;
 	opts->no_security_labels = no_security_labels;
 	opts->no_subscriptions = no_subscriptions;
+	opts->no_compression_methods = no_compression_methods;
 
 	if (if_exists && !opts->dropSchema)
 	{
diff --git a/src/bin/pg_dump/t/002_pg_dump.pl b/src/bin/pg_dump/t/002_pg_dump.pl
index de6895122e..87a397303f 100644
--- a/src/bin/pg_dump/t/002_pg_dump.pl
+++ b/src/bin/pg_dump/t/002_pg_dump.pl
@@ -651,6 +651,43 @@ my %tests = (
 		},
 	},
 
+	# compression data in binary upgrade mode
+	'ALTER TABLE test_table_compression ALTER COLUMN ... SET COMPRESSION' => {
+		all_runs  => 1,
+		catch_all => 'ALTER TABLE ... commands',
+		regexp    => qr/^
+			\QCREATE TABLE dump_test.test_table_compression (\E\n
+			\s+\Qcol1 text,\E\n
+			\s+\Qcol2 text,\E\n
+			\s+\Qcol3 text,\E\n
+			\s+\Qcol4 text\E\n
+			\);
+			.*
+			\QSELECT binary_upgrade_set_next_attr_compression_oid('\E\d+\Q'::pg_catalog.oid);\E\n
+			\QALTER TABLE dump_test.test_table_compression ALTER COLUMN col1\E\n
+			\QSET COMPRESSION pglz;\E\n
+			.*
+			\QSELECT binary_upgrade_set_next_attr_compression_oid('\E\d+\Q'::pg_catalog.oid);\E\n
+			\QALTER TABLE dump_test.test_table_compression ALTER COLUMN col2\E\n
+			\QSET COMPRESSION pglz2;\E\n
+			.*
+			\QSELECT binary_upgrade_set_next_attr_compression_oid('\E\d+\Q'::pg_catalog.oid);\E\n
+			\QALTER TABLE dump_test.test_table_compression ALTER COLUMN col3\E\n
+			\QSET COMPRESSION pglz\E\n
+			\QWITH (min_input_size '1000');\E\n
+			.*
+			\QSELECT binary_upgrade_set_next_attr_compression_oid('\E\d+\Q'::pg_catalog.oid);\E\n
+			\QALTER TABLE dump_test.test_table_compression ALTER COLUMN col4\E\n
+			\QSET COMPRESSION pglz2\E\n
+			\QWITH (min_input_size '1000');\E\n
+			\QSELECT binary_upgrade_set_next_attr_compression_oid('\E\d+\Q'::pg_catalog.oid);\E\n
+			\QALTER TABLE dump_test.test_table_compression ALTER COLUMN col4\E\n
+			\QSET COMPRESSION pglz2\E\n
+			\QWITH (min_input_size '2000');\E\n
+			/xms,
+		like => { binary_upgrade => 1, },
+	},
+
 	'ALTER TABLE ONLY test_table ALTER COLUMN col1 SET STATISTICS 90' => {
 		create_order => 93,
 		create_sql =>
@@ -1367,6 +1404,17 @@ my %tests = (
 		like => { %full_runs, section_pre_data => 1, },
 	},
 
+	'CREATE ACCESS METHOD pglz2' => {
+		all_runs     => 1,
+		catch_all    => 'CREATE ... commands',
+		create_order => 52,
+		create_sql =>
+		  'CREATE ACCESS METHOD pglz2 TYPE COMPRESSION HANDLER pglzhandler;',
+		regexp =>
+		  qr/CREATE ACCESS METHOD pglz2 TYPE COMPRESSION HANDLER pglzhandler;/m,
+		like => { %full_runs, section_pre_data => 1, },
+	},
+
 	'CREATE COLLATION test0 FROM "C"' => {
 		create_order => 76,
 		create_sql   => 'CREATE COLLATION test0 FROM "C";',
@@ -2414,6 +2462,53 @@ my %tests = (
 		unlike => { exclude_dump_test_schema => 1, },
 	},
 
+	'CREATE TABLE test_table_compression' => {
+		create_order => 55,
+		create_sql   => 'CREATE TABLE dump_test.test_table_compression (
+						   col1 text,
+						   col2 text COMPRESSION pglz2,
+						   col3 text COMPRESSION pglz WITH (min_input_size \'1000\'),
+						   col4 text COMPRESSION pglz2 WITH (min_input_size \'1000\')
+					     );',
+		regexp => qr/^
+			\QCREATE TABLE dump_test.test_table_compression (\E\n
+			\s+\Qcol1 text,\E\n
+			\s+\Qcol2 text COMPRESSION pglz2,\E\n
+			\s+\Qcol3 text COMPRESSION pglz WITH (min_input_size '1000'),\E\n
+			\s+\Qcol4 text COMPRESSION pglz2 WITH (min_input_size '2000')\E\n
+			\);
+			/xm,
+		like =>
+		  { %full_runs, %dump_test_schema_runs, section_pre_data => 1, },
+		unlike => {
+			binary_upgrade		     => 1,
+			exclude_dump_test_schema => 1,
+		},
+	},
+
+	'ALTER TABLE test_table_compression' => {
+		create_order => 56,
+		create_sql   => 'ALTER TABLE dump_test.test_table_compression
+						 ALTER COLUMN col4
+						 SET COMPRESSION pglz2
+						 WITH (min_input_size \'2000\')
+						 PRESERVE (pglz2);',
+		regexp => qr/^
+			\QCREATE TABLE dump_test.test_table_compression (\E\n
+			\s+\Qcol1 text,\E\n
+			\s+\Qcol2 text COMPRESSION pglz2,\E\n
+			\s+\Qcol3 text COMPRESSION pglz WITH (min_input_size '1000'),\E\n
+			\s+\Qcol4 text COMPRESSION pglz2 WITH (min_input_size '2000')\E\n
+			\);
+			/xm,
+		like =>
+		  { %full_runs, %dump_test_schema_runs, section_pre_data => 1, },
+		unlike => {
+			binary_upgrade		     => 1,
+			exclude_dump_test_schema => 1,
+		},
+	},
+
 	'CREATE STATISTICS extended_stats_no_options' => {
 		create_order => 97,
 		create_sql   => 'CREATE STATISTICS dump_test.test_ext_stats_no_options
diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c
index 779e48437c..428f839d03 100644
--- a/src/bin/psql/describe.c
+++ b/src/bin/psql/describe.c
@@ -1467,6 +1467,7 @@ describeOneTableDetails(const char *schemaname,
 				fdwopts_col = -1,
 				attstorage_col = -1,
 				attstattarget_col = -1,
+				attcompression_col = -1,
 				attdescr_col = -1;
 	int			numrows;
 	struct
@@ -1859,6 +1860,24 @@ describeOneTableDetails(const char *schemaname,
 		appendPQExpBufferStr(&buf, ",\n  a.attstorage");
 		attstorage_col = cols++;
 
+		/* compresssion info */
+		if (pset.sversion >= 120000 &&
+			(tableinfo.relkind == RELKIND_RELATION ||
+			 tableinfo.relkind == RELKIND_PARTITIONED_TABLE))
+		{
+			appendPQExpBufferStr(&buf, ",\n  CASE WHEN attcompression = 0 THEN NULL ELSE "
+								 " (SELECT c.acname || "
+								 "		(CASE WHEN acoptions IS NULL "
+								 "		 THEN '' "
+								 "		 ELSE '(' || array_to_string(ARRAY(SELECT quote_ident(option_name) || ' ' || quote_literal(option_value)"
+								 "											  FROM pg_options_to_table(acoptions)), ', ') || ')'"
+								 " 		 END) "
+								 "  FROM pg_catalog.pg_attr_compression c "
+								 "  WHERE c.acoid = a.attcompression) "
+								 " END AS attcmname");
+			attcompression_col = cols++;
+		}
+
 		/* stats target, if relevant to relkind */
 		if (tableinfo.relkind == RELKIND_RELATION ||
 			tableinfo.relkind == RELKIND_INDEX ||
@@ -1985,6 +2004,8 @@ describeOneTableDetails(const char *schemaname,
 		headers[cols++] = gettext_noop("FDW options");
 	if (attstorage_col >= 0)
 		headers[cols++] = gettext_noop("Storage");
+	if (attcompression_col >= 0)
+		headers[cols++] = gettext_noop("Compression");
 	if (attstattarget_col >= 0)
 		headers[cols++] = gettext_noop("Stats target");
 	if (attdescr_col >= 0)
@@ -2056,6 +2077,27 @@ describeOneTableDetails(const char *schemaname,
 							  false, false);
 		}
 
+		/* Column compression. */
+		if (attcompression_col >= 0)
+		{
+			bool		mustfree = false;
+			const int	trunclen = 100;
+			char *val = PQgetvalue(res, i, attcompression_col);
+
+			/* truncate the options if they're too long */
+			if (strlen(val) > trunclen + 3)
+			{
+				char *trunc = pg_malloc0(trunclen + 4);
+				strncpy(trunc, val, trunclen);
+				strncpy(trunc + trunclen, "...", 4);
+
+				val = trunc;
+				mustfree = true;
+			}
+
+			printTableAddCell(&cont, val, false, mustfree);
+		}
+
 		/* Statistics target, if the relkind supports this feature */
 		if (attstattarget_col >= 0)
 			printTableAddCell(&cont, PQgetvalue(res, i, attstattarget_col),
diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index 10ae21cc61..19ef736b98 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -1919,11 +1919,14 @@ psql_completion(const char *text, int start, int end)
 	/* ALTER TABLE ALTER [COLUMN] <foo> SET */
 	else if (Matches("ALTER", "TABLE", MatchAny, "ALTER", "COLUMN", MatchAny, "SET") ||
 			 Matches("ALTER", "TABLE", MatchAny, "ALTER", MatchAny, "SET"))
-		COMPLETE_WITH("(", "DEFAULT", "NOT NULL", "STATISTICS", "STORAGE");
+		COMPLETE_WITH("(", "COMPRESSION", "DEFAULT", "NOT NULL", "STATISTICS", "STORAGE");
 	/* ALTER TABLE ALTER [COLUMN] <foo> SET ( */
 	else if (Matches("ALTER", "TABLE", MatchAny, "ALTER", "COLUMN", MatchAny, "SET", "(") ||
 			 Matches("ALTER", "TABLE", MatchAny, "ALTER", MatchAny, "SET", "("))
 		COMPLETE_WITH("n_distinct", "n_distinct_inherited");
+	else if (Matches("ALTER", "TABLE", MatchAny, "ALTER", "COLUMN", MatchAny, "SET", "COMPRESSION", MatchAny) ||
+			 Matches("ALTER", "TABLE", MatchAny, "ALTER", MatchAny, "SET", "COMPRESSION", MatchAny))
+		COMPLETE_WITH("WITH (", "PRESERVE (");
 	/* ALTER TABLE ALTER [COLUMN] <foo> SET STORAGE */
 	else if (Matches("ALTER", "TABLE", MatchAny, "ALTER", "COLUMN", MatchAny, "SET", "STORAGE") ||
 			 Matches("ALTER", "TABLE", MatchAny, "ALTER", MatchAny, "SET", "STORAGE"))
diff --git a/src/include/catalog/binary_upgrade.h b/src/include/catalog/binary_upgrade.h
index 2927b7a4d3..00f91e4b90 100644
--- a/src/include/catalog/binary_upgrade.h
+++ b/src/include/catalog/binary_upgrade.h
@@ -25,6 +25,8 @@ extern PGDLLIMPORT Oid binary_upgrade_next_toast_pg_class_oid;
 extern PGDLLIMPORT Oid binary_upgrade_next_pg_enum_oid;
 extern PGDLLIMPORT Oid binary_upgrade_next_pg_authid_oid;
 
+extern PGDLLIMPORT Oid binary_upgrade_next_attr_compression_oid;
+
 extern PGDLLIMPORT bool binary_upgrade_record_init_privs;
 
 #endif							/* BINARY_UPGRADE_H */
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index 2dce2c087d..8ca3f53b96 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -9981,6 +9981,10 @@
   proname => 'binary_upgrade_set_missing_value', provolatile => 'v',
   proparallel => 'u', prorettype => 'void', proargtypes => 'oid text text',
   prosrc => 'binary_upgrade_set_missing_value' },
+{ oid => '4012', descr => 'for use by pg_upgrade',
+  proname => 'binary_upgrade_set_next_attr_compression_oid', provolatile => 'v',
+  proparallel => 'r', prorettype => 'void', proargtypes => 'oid',
+  prosrc => 'binary_upgrade_set_next_attr_compression_oid' },
 
 # conversion functions
 { oid => '4300',
-- 
2.21.0


--MP_//XvyHCr_hJMvh/tp2R3=uJa
Content-Type: text/x-patch
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
 filename=0007-Add-tests-for-compression-methods-v22.patch



^ permalink  raw  reply  [nested|flat] 18+ messages in thread

* [PATCH 6/8] Add psql, pg_dump and pg_upgrade support
@ 2018-06-18 12:57 Ildus Kurbangaliev <i.kurbangaliev@gmail.com>
  0 siblings, 0 replies; 18+ messages in thread

From: Ildus Kurbangaliev @ 2018-06-18 12:57 UTC (permalink / raw)

Signed-off-by: Ildus Kurbangaliev <i.kurbangaliev@gmail.com>
---
 src/backend/commands/compressioncmds.c     |  80 ++++++---
 src/backend/commands/tablecmds.c           |  14 +-
 src/backend/utils/adt/pg_upgrade_support.c |  10 ++
 src/bin/pg_dump/pg_backup.h                |   2 +
 src/bin/pg_dump/pg_dump.c                  | 196 ++++++++++++++++++++-
 src/bin/pg_dump/pg_dump.h                  |  17 ++
 src/bin/pg_dump/pg_dumpall.c               |   5 +
 src/bin/pg_dump/pg_restore.c               |   3 +
 src/bin/pg_dump/t/002_pg_dump.pl           |  95 ++++++++++
 src/bin/psql/describe.c                    |  42 +++++
 src/bin/psql/tab-complete.c                |   5 +-
 src/include/catalog/binary_upgrade.h       |   2 +
 src/include/catalog/pg_proc.dat            |   4 +
 13 files changed, 433 insertions(+), 42 deletions(-)

diff --git a/src/backend/commands/compressioncmds.c b/src/backend/commands/compressioncmds.c
index 7841c7700a..89ffa227b0 100644
--- a/src/backend/commands/compressioncmds.c
+++ b/src/backend/commands/compressioncmds.c
@@ -36,6 +36,9 @@
 #include "utils/syscache.h"
 #include "utils/snapmgr.h"
 
+/* Set by pg_upgrade_support functions */
+Oid			binary_upgrade_next_attr_compression_oid = InvalidOid;
+
 /*
  * When conditions of compression satisfies one if builtin attribute
  * compresssion tuples the compressed attribute will be linked to
@@ -129,11 +132,12 @@ lookup_attribute_compression(Oid attrelid, AttrNumber attnum,
 					tup_amoid;
 		Datum		values[Natts_pg_attr_compression];
 		bool		nulls[Natts_pg_attr_compression];
+		char	   *amname;
 
 		heap_deform_tuple(tuple, RelationGetDescr(rel), values, nulls);
 		acoid = DatumGetObjectId(values[Anum_pg_attr_compression_acoid - 1]);
-		tup_amoid = get_am_oid(
-							   NameStr(*DatumGetName(values[Anum_pg_attr_compression_acname - 1])), false);
+		amname = NameStr(*DatumGetName(values[Anum_pg_attr_compression_acname - 1]));
+		tup_amoid = get_am_oid(amname, false);
 
 		if (previous_amoids)
 			*previous_amoids = list_append_unique_oid(*previous_amoids, tup_amoid);
@@ -150,17 +154,15 @@ lookup_attribute_compression(Oid attrelid, AttrNumber attnum,
 			if (DatumGetPointer(acoptions) == NULL)
 				result = acoid;
 		}
-		else
+		else if (DatumGetPointer(acoptions) != NULL)
 		{
 			bool		equal;
 
 			/* check if arrays for WITH options are equal */
 			equal = DatumGetBool(CallerFInfoFunctionCall2(
-														  array_eq,
-														  &arrayeq_info,
-														  InvalidOid,
-														  acoptions,
-														  values[Anum_pg_attr_compression_acoptions - 1]));
+						array_eq, &arrayeq_info, InvalidOid, acoptions,
+						values[Anum_pg_attr_compression_acoptions - 1]));
+
 			if (equal)
 				result = acoid;
 		}
@@ -227,6 +229,16 @@ CreateAttributeCompression(Form_pg_attribute att,
 	/* Try to find builtin compression first */
 	acoid = lookup_attribute_compression(0, 0, amoid, arropt, NULL);
 
+	/* no rewrite by default */
+	if (need_rewrite != NULL)
+		*need_rewrite = false;
+
+	if (IsBinaryUpgrade)
+	{
+		/* Skip the rewrite checks and searching of identical compression */
+		goto add_tuple;
+	}
+
 	/*
 	 * attrelid will be invalid on CREATE TABLE, no need for table rewrite
 	 * check.
@@ -252,16 +264,10 @@ CreateAttributeCompression(Form_pg_attribute att,
 		 */
 		if (need_rewrite != NULL)
 		{
-			/* no rewrite by default */
-			*need_rewrite = false;
-
 			Assert(preserved_amoids != NULL);
 
 			if (compression->preserve == NIL)
-			{
-				Assert(!IsBinaryUpgrade);
 				*need_rewrite = true;
-			}
 			else
 			{
 				ListCell   *cell;
@@ -294,7 +300,7 @@ CreateAttributeCompression(Form_pg_attribute att,
 				 * In binary upgrade list will not be free since it contains
 				 * Oid of builtin compression access method.
 				 */
-				if (!IsBinaryUpgrade && list_length(previous_amoids) != 0)
+				if (list_length(previous_amoids) != 0)
 					*need_rewrite = true;
 			}
 		}
@@ -303,9 +309,6 @@ CreateAttributeCompression(Form_pg_attribute att,
 		list_free(previous_amoids);
 	}
 
-	if (IsBinaryUpgrade && !OidIsValid(acoid))
-		elog(ERROR, "could not restore attribute compression data");
-
 	/* Return Oid if we already found identical compression on this column */
 	if (OidIsValid(acoid))
 	{
@@ -315,6 +318,7 @@ CreateAttributeCompression(Form_pg_attribute att,
 		return acoid;
 	}
 
+add_tuple:
 	/* Initialize buffers for new tuple values */
 	memset(values, 0, sizeof(values));
 	memset(nulls, false, sizeof(nulls));
@@ -323,13 +327,27 @@ CreateAttributeCompression(Form_pg_attribute att,
 
 	rel = heap_open(AttrCompressionRelationId, RowExclusiveLock);
 
-	acoid = GetNewOidWithIndex(rel, AttrCompressionIndexId,
-							   Anum_pg_attr_compression_acoid);
+	if (IsBinaryUpgrade)
+	{
+		/* acoid should be found in some cases */
+		if (binary_upgrade_next_attr_compression_oid < FirstNormalObjectId &&
+			(!OidIsValid(acoid) || binary_upgrade_next_attr_compression_oid != acoid))
+			elog(ERROR, "could not link to built-in attribute compression");
+
+		acoid = binary_upgrade_next_attr_compression_oid;
+	}
+	else
+	{
+		acoid = GetNewOidWithIndex(rel, AttrCompressionIndexId,
+									Anum_pg_attr_compression_acoid);
+
+	}
+
 	if (acoid < FirstNormalObjectId)
 	{
-		/* this is database initialization */
+		/* this is built-in attribute compression */
 		heap_close(rel, RowExclusiveLock);
-		return DefaultCompressionOid;
+		return acoid;
 	}
 
 	/* we need routine only to call cmcheck function */
@@ -390,8 +408,8 @@ RemoveAttributeCompression(Oid acoid)
 /*
  * CleanupAttributeCompression
  *
- * Remove entries in pg_attr_compression except current attribute compression
- * and related with specified list of access methods.
+ * Remove entries in pg_attr_compression of the column except current
+ * attribute compression and related with specified list of access methods.
  */
 void
 CleanupAttributeCompression(Oid relid, AttrNumber attnum, List *keepAmOids)
@@ -419,9 +437,7 @@ CleanupAttributeCompression(Oid relid, AttrNumber attnum, List *keepAmOids)
 	ReleaseSysCache(attrtuple);
 
 	Assert(relid > 0 && attnum > 0);
-
-	if (IsBinaryUpgrade)
-		goto builtin_removal;
+	Assert(!IsBinaryUpgrade);
 
 	rel = heap_open(AttrCompressionRelationId, RowExclusiveLock);
 
@@ -438,7 +454,10 @@ CleanupAttributeCompression(Oid relid, AttrNumber attnum, List *keepAmOids)
 	scan = systable_beginscan(rel, AttrCompressionRelidAttnumIndexId,
 							  true, NULL, 2, key);
 
-	/* Remove attribute compression tuples and collect removed Oids to list */
+	/*
+	 * Remove attribute compression tuples and collect removed Oids
+	 * to list.
+	 */
 	while (HeapTupleIsValid(tuple = systable_getnext(scan)))
 	{
 		Form_pg_attr_compression acform;
@@ -460,7 +479,10 @@ CleanupAttributeCompression(Oid relid, AttrNumber attnum, List *keepAmOids)
 	systable_endscan(scan);
 	heap_close(rel, RowExclusiveLock);
 
-	/* Now remove dependencies */
+	/*
+	 * Now remove dependencies between attribute compression (dependent)
+	 * and column.
+	 */
 	rel = heap_open(DependRelationId, RowExclusiveLock);
 	foreach(lc, removed)
 	{
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 4ecff835c0..a778f48cbb 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -756,10 +756,10 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId,
 		if (colDef->identity)
 			attr->attidentity = colDef->identity;
 
-		if (relkind == RELKIND_RELATION || relkind == RELKIND_PARTITIONED_TABLE)
+		if (!IsBinaryUpgrade &&
+			(relkind == RELKIND_RELATION || relkind == RELKIND_PARTITIONED_TABLE))
 			attr->attcompression = CreateAttributeCompression(attr,
-															  colDef->compression,
-															  NULL, NULL);
+										colDef->compression, NULL, NULL);
 		else
 			attr->attcompression = InvalidOid;
 	}
@@ -13119,14 +13119,6 @@ ATExecSetCompression(AlteredTableInfo *tab,
 	/* make changes visible */
 	CommandCounterIncrement();
 
-	/*
-	 * Normally cleanup is done in rewrite but in binary upgrade we should do
-	 * it explicitly.
-	 */
-	if (IsBinaryUpgrade)
-		CleanupAttributeCompression(RelationGetRelid(rel),
-									attnum, preserved_amoids);
-
 	ObjectAddressSet(address, AttrCompressionRelationId, acoid);
 	return address;
 }
diff --git a/src/backend/utils/adt/pg_upgrade_support.c b/src/backend/utils/adt/pg_upgrade_support.c
index b8b7777c31..1082eab4dc 100644
--- a/src/backend/utils/adt/pg_upgrade_support.c
+++ b/src/backend/utils/adt/pg_upgrade_support.c
@@ -116,6 +116,16 @@ binary_upgrade_set_next_pg_authid_oid(PG_FUNCTION_ARGS)
 	PG_RETURN_VOID();
 }
 
+Datum
+binary_upgrade_set_next_attr_compression_oid(PG_FUNCTION_ARGS)
+{
+	Oid			acoid = PG_GETARG_OID(0);
+
+	CHECK_IS_BINARY_UPGRADE;
+	binary_upgrade_next_attr_compression_oid = acoid;
+	PG_RETURN_VOID();
+}
+
 Datum
 binary_upgrade_create_empty_extension(PG_FUNCTION_ARGS)
 {
diff --git a/src/bin/pg_dump/pg_backup.h b/src/bin/pg_dump/pg_backup.h
index ba798213be..00853989d0 100644
--- a/src/bin/pg_dump/pg_backup.h
+++ b/src/bin/pg_dump/pg_backup.h
@@ -78,6 +78,7 @@ typedef struct _restoreOptions
 	int			no_publications;	/* Skip publication entries */
 	int			no_security_labels; /* Skip security label entries */
 	int			no_subscriptions;	/* Skip subscription entries */
+	int			no_compression_methods; /* Skip compression methods */
 	int			strict_names;
 
 	const char *filename;
@@ -151,6 +152,7 @@ typedef struct _dumpOptions
 	int			no_security_labels;
 	int			no_publications;
 	int			no_subscriptions;
+	int			no_compression_methods;
 	int			no_synchronized_snapshots;
 	int			no_unlogged_table_data;
 	int			serializable_deferrable;
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index c8d01ed4a4..e59b8ca9d5 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -40,11 +40,13 @@
 #include "getopt_long.h"
 
 #include "access/attnum.h"
+#include "access/cmapi.h"
 #include "access/sysattr.h"
 #include "access/transam.h"
 #include "catalog/pg_aggregate_d.h"
 #include "catalog/pg_am_d.h"
 #include "catalog/pg_attribute_d.h"
+#include "catalog/pg_attr_compression_d.h"
 #include "catalog/pg_cast_d.h"
 #include "catalog/pg_class_d.h"
 #include "catalog/pg_default_acl_d.h"
@@ -376,6 +378,7 @@ main(int argc, char **argv)
 		{"no-synchronized-snapshots", no_argument, &dopt.no_synchronized_snapshots, 1},
 		{"no-unlogged-table-data", no_argument, &dopt.no_unlogged_table_data, 1},
 		{"no-subscriptions", no_argument, &dopt.no_subscriptions, 1},
+		{"no-compression-methods", no_argument, &dopt.no_compression_methods, 1},
 		{"no-sync", no_argument, NULL, 7},
 		{"on-conflict-do-nothing", no_argument, &dopt.do_nothing, 1},
 
@@ -843,6 +846,8 @@ main(int argc, char **argv)
 	 * We rely on dependency information to help us determine a safe order, so
 	 * the initial sort is mostly for cosmetic purposes: we sort by name to
 	 * ensure that logically identical schemas will dump identically.
+	 *
+	 * If we do a parallel dump, we want the largest tables to go first.
 	 */
 	sortDumpableObjectsByTypeName(dobjs, numObjs);
 
@@ -8143,9 +8148,12 @@ getTableAttrs(Archive *fout, TableInfo *tblinfo, int numTables)
 	int			i_attcollation;
 	int			i_attfdwoptions;
 	int			i_attmissingval;
+	int			i_attcmoptions;
+	int			i_attcmname;
 	PGresult   *res;
 	int			ntups;
 	bool		hasdefaults;
+	bool		createWithCompression;
 
 	for (i = 0; i < numTables; i++)
 	{
@@ -8188,6 +8196,23 @@ getTableAttrs(Archive *fout, TableInfo *tblinfo, int numTables)
 						  "a.attislocal,\n"
 						  "pg_catalog.format_type(t.oid, a.atttypmod) AS atttypname,\n");
 
+		createWithCompression = (!dopt->binary_upgrade && fout->remoteVersion >= 120000);
+
+		if (createWithCompression)
+			appendPQExpBuffer(q,
+							  "pg_catalog.array_to_string(ARRAY("
+							  "SELECT pg_catalog.quote_ident(option_name) || "
+							  "' ' || pg_catalog.quote_literal(option_value) "
+							  "FROM pg_catalog.pg_options_to_table(c.acoptions) "
+							  "ORDER BY option_name"
+							  "), E',\n    ') AS attcmoptions,\n"
+							  "c.acname AS attcmname,\n");
+		else
+			appendPQExpBuffer(q,
+							  "NULL AS attcmoptions,\n"
+							  "NULL AS attcmname,\n");
+
+
 		if (fout->remoteVersion >= 110000)
 			appendPQExpBuffer(q,
 							  "CASE WHEN a.atthasmissing AND NOT a.attisdropped "
@@ -8240,7 +8265,13 @@ getTableAttrs(Archive *fout, TableInfo *tblinfo, int numTables)
 		/* need left join here to not fail on dropped columns ... */
 		appendPQExpBuffer(q,
 						  "FROM pg_catalog.pg_attribute a LEFT JOIN pg_catalog.pg_type t "
-						  "ON a.atttypid = t.oid\n"
+						  "ON a.atttypid = t.oid\n");
+
+		if (createWithCompression)
+			appendPQExpBuffer(q, "LEFT JOIN pg_catalog.pg_attr_compression c "
+								 "ON a.attcompression = c.acoid\n");
+
+		appendPQExpBuffer(q,
 						  "WHERE a.attrelid = '%u'::pg_catalog.oid "
 						  "AND a.attnum > 0::pg_catalog.int2\n"
 						  "ORDER BY a.attnum",
@@ -8268,6 +8299,8 @@ getTableAttrs(Archive *fout, TableInfo *tblinfo, int numTables)
 		i_attcollation = PQfnumber(res, "attcollation");
 		i_attfdwoptions = PQfnumber(res, "attfdwoptions");
 		i_attmissingval = PQfnumber(res, "attmissingval");
+		i_attcmname = PQfnumber(res, "attcmname");
+		i_attcmoptions = PQfnumber(res, "attcmoptions");
 
 		tbinfo->numatts = ntups;
 		tbinfo->attnames = (char **) pg_malloc(ntups * sizeof(char *));
@@ -8285,9 +8318,12 @@ getTableAttrs(Archive *fout, TableInfo *tblinfo, int numTables)
 		tbinfo->attcollation = (Oid *) pg_malloc(ntups * sizeof(Oid));
 		tbinfo->attfdwoptions = (char **) pg_malloc(ntups * sizeof(char *));
 		tbinfo->attmissingval = (char **) pg_malloc(ntups * sizeof(char *));
+		tbinfo->attcmoptions = (char **) pg_malloc(ntups * sizeof(char *));
+		tbinfo->attcmnames = (char **) pg_malloc(ntups * sizeof(char *));
 		tbinfo->notnull = (bool *) pg_malloc(ntups * sizeof(bool));
 		tbinfo->inhNotNull = (bool *) pg_malloc(ntups * sizeof(bool));
 		tbinfo->attrdefs = (AttrDefInfo **) pg_malloc(ntups * sizeof(AttrDefInfo *));
+		tbinfo->attcompression = NULL;
 		hasdefaults = false;
 
 		for (j = 0; j < ntups; j++)
@@ -8313,6 +8349,8 @@ getTableAttrs(Archive *fout, TableInfo *tblinfo, int numTables)
 			tbinfo->attcollation[j] = atooid(PQgetvalue(res, j, i_attcollation));
 			tbinfo->attfdwoptions[j] = pg_strdup(PQgetvalue(res, j, i_attfdwoptions));
 			tbinfo->attmissingval[j] = pg_strdup(PQgetvalue(res, j, i_attmissingval));
+			tbinfo->attcmoptions[j] = pg_strdup(PQgetvalue(res, j, i_attcmoptions));
+			tbinfo->attcmnames[j] = pg_strdup(PQgetvalue(res, j, i_attcmname));
 			tbinfo->attrdefs[j] = NULL; /* fix below */
 			if (PQgetvalue(res, j, i_atthasdef)[0] == 't')
 				hasdefaults = true;
@@ -8530,6 +8568,104 @@ getTableAttrs(Archive *fout, TableInfo *tblinfo, int numTables)
 			}
 			PQclear(res);
 		}
+
+		/*
+		 * Get compression info
+		 */
+		if (fout->remoteVersion >= 120000 && dopt->binary_upgrade)
+		{
+			int			i_acname;
+			int			i_acoid;
+			int			i_parsedoptions;
+			int			i_curattnum;
+			int			start;
+
+			if (g_verbose)
+				write_msg(NULL, "finding compression info for table \"%s.%s\"\n",
+						  tbinfo->dobj.namespace->dobj.name,
+						  tbinfo->dobj.name);
+
+			tbinfo->attcompression = pg_malloc0(tbinfo->numatts * sizeof(AttrCompressionInfo *));
+
+			resetPQExpBuffer(q);
+			appendPQExpBuffer(q,
+				"SELECT attrelid::pg_catalog.regclass AS relname, attname,"
+				" (CASE WHEN deptype = 'i' THEN refobjsubid ELSE objsubid END) AS curattnum,"
+				" (CASE WHEN deptype = 'n' THEN attcompression = refobjid"
+				"		ELSE attcompression = objid END) AS iscurrent,"
+				" acname, acoid,"
+				" (CASE WHEN acoptions IS NOT NULL"
+				"  THEN pg_catalog.array_to_string(ARRAY("
+				"		SELECT pg_catalog.quote_ident(option_name) || "
+				"			' ' || pg_catalog.quote_literal(option_value) "
+				"		FROM pg_catalog.pg_options_to_table(acoptions) "
+				"		ORDER BY option_name"
+				"		), E',\n    ')"
+				"  ELSE NULL END) AS parsedoptions "
+				" FROM pg_depend d"
+				" JOIN pg_attribute a ON"
+				"	(classid = 'pg_class'::pg_catalog.regclass::pg_catalog.oid AND a.attrelid = d.objid"
+				"		AND a.attnum = d.objsubid AND d.deptype = 'n'"
+				"		AND d.refclassid = 'pg_attr_compression'::pg_catalog.regclass::pg_catalog.oid)"
+				"	OR (d.refclassid = 'pg_class'::pg_catalog.regclass::pg_catalog.oid"
+				"		AND d.refobjid = a.attrelid"
+				"		AND d.refobjsubid = a.attnum AND d.deptype = 'i'"
+				"		AND d.classid = 'pg_attr_compression'::pg_catalog.regclass::pg_catalog.oid)"
+				" JOIN pg_attr_compression c ON"
+				"	(d.deptype = 'i' AND d.objid = c.acoid AND a.attnum = c.acattnum"
+				"		AND a.attrelid = c.acrelid) OR"
+				"	(d.deptype = 'n' AND d.refobjid = c.acoid AND c.acattnum = 0"
+				"		AND c.acrelid = 0)"
+				" WHERE (deptype = 'n' AND d.objid = %d) OR (deptype = 'i' AND d.refobjid = %d)"
+				" ORDER BY curattnum, iscurrent;",
+				tbinfo->dobj.catId.oid, tbinfo->dobj.catId.oid);
+
+			res = ExecuteSqlQuery(fout, q->data, PGRES_TUPLES_OK);
+			ntups = PQntuples(res);
+
+			if (ntups > 0)
+			{
+				int		k;
+
+				i_acname = PQfnumber(res, "acname");
+				i_acoid = PQfnumber(res, "acoid");
+				i_parsedoptions = PQfnumber(res, "parsedoptions");
+				i_curattnum = PQfnumber(res, "curattnum");
+
+				start = 0;
+
+				for (j = 0; j < ntups; j++)
+				{
+					int		attnum = atoi(PQgetvalue(res, j, i_curattnum));
+
+					if ((j == ntups - 1) || atoi(PQgetvalue(res, j + 1, i_curattnum)) != attnum)
+					{
+						AttrCompressionInfo *cminfo = pg_malloc(sizeof(AttrCompressionInfo));
+
+						cminfo->nitems = j - start + 1;
+						cminfo->items = pg_malloc(sizeof(AttrCompressionItem *) * cminfo->nitems);
+
+						for (k = start; k < start + cminfo->nitems; k++)
+						{
+							AttrCompressionItem	*cmitem = pg_malloc0(sizeof(AttrCompressionItem));
+
+							cmitem->acname = pg_strdup(PQgetvalue(res, k, i_acname));
+							cmitem->acoid = atooid(PQgetvalue(res, k, i_acoid));
+
+							if (!PQgetisnull(res, k, i_parsedoptions))
+								cmitem->parsedoptions = pg_strdup(PQgetvalue(res, k, i_parsedoptions));
+
+							cminfo->items[k - start] = cmitem;
+						}
+
+						tbinfo->attcompression[attnum - 1] = cminfo;
+						start = j + 1;	/* start from next */
+					}
+				}
+			}
+
+			PQclear(res);
+		}
 	}
 
 	destroyPQExpBuffer(q);
@@ -12606,6 +12742,9 @@ dumpAccessMethod(Archive *fout, AccessMethodInfo *aminfo)
 		case AMTYPE_INDEX:
 			appendPQExpBuffer(q, "TYPE INDEX ");
 			break;
+		case AMTYPE_COMPRESSION:
+			appendPQExpBuffer(q, "TYPE COMPRESSION ");
+			break;
 		default:
 			write_msg(NULL, "WARNING: invalid type \"%c\" of access method \"%s\"\n",
 					  aminfo->amtype, qamname);
@@ -15531,6 +15670,14 @@ dumpTableSchema(Archive *fout, TableInfo *tbinfo)
 											   (!tbinfo->inhNotNull[j] ||
 												dopt->binary_upgrade));
 
+					/*
+					 * Compression will require a record in
+					 * pg_attr_compression
+					 */
+					bool		has_custom_compression = (tbinfo->attcmnames[j] &&
+														  ((strcmp(tbinfo->attcmnames[j], "pglz") != 0) ||
+														   nonemptyReloptions(tbinfo->attcmoptions[j])));
+
 					/*
 					 * Skip column if fully defined by reloftype or the
 					 * partition parent.
@@ -15589,6 +15736,25 @@ dumpTableSchema(Archive *fout, TableInfo *tbinfo)
 											  fmtQualifiedDumpable(coll));
 					}
 
+					/*
+					 * Compression
+					 *
+					 * In binary-upgrade mode, compression is assigned by
+					 * ALTER. Even if we're skipping compression the attribute
+					 * will get default compression. It's the task for ALTER
+					 * command to restore compression info.
+					 */
+					if (!dopt->no_compression_methods && !dopt->binary_upgrade &&
+						tbinfo->attcmnames[j] && strlen(tbinfo->attcmnames[j]) &&
+						has_custom_compression)
+					{
+						appendPQExpBuffer(q, " COMPRESSION %s",
+										  tbinfo->attcmnames[j]);
+						if (nonemptyReloptions(tbinfo->attcmoptions[j]))
+							appendPQExpBuffer(q, " WITH (%s)",
+											  tbinfo->attcmoptions[j]);
+					}
+
 					if (has_default)
 						appendPQExpBuffer(q, " DEFAULT %s",
 										  tbinfo->attrdefs[j]->adef_expr);
@@ -16004,6 +16170,34 @@ dumpTableSchema(Archive *fout, TableInfo *tbinfo)
 				appendPQExpBuffer(q, "OPTIONS (\n    %s\n);\n",
 								  tbinfo->attfdwoptions[j]);
 			}
+
+			/*
+			 * Dump per-column compression options
+			 */
+			if (tbinfo->attcompression && tbinfo->attcompression[j])
+			{
+				AttrCompressionInfo *cminfo = tbinfo->attcompression[j];
+
+				if (cminfo->nitems)
+					appendPQExpBuffer(q, "\n-- For binary upgrade, recreate compression metadata on column %s\n",
+							fmtId(tbinfo->attnames[j]));
+
+				for (int i = 0; i < cminfo->nitems; i++)
+				{
+					AttrCompressionItem *item = cminfo->items[i];
+
+					appendPQExpBuffer(q,
+						"SELECT binary_upgrade_set_next_attr_compression_oid('%d'::pg_catalog.oid);\n",
+									  item->acoid);
+					appendPQExpBuffer(q, "ALTER TABLE %s ALTER COLUMN %s\nSET COMPRESSION %s",
+									  qualrelname, fmtId(tbinfo->attnames[j]), item->acname);
+
+					if (item->parsedoptions)
+						appendPQExpBuffer(q, "\nWITH (%s);\n", item->parsedoptions);
+					else
+						appendPQExpBuffer(q, ";\n");
+				}
+			}
 		}
 	}
 
diff --git a/src/bin/pg_dump/pg_dump.h b/src/bin/pg_dump/pg_dump.h
index 685ad78669..9bff630c93 100644
--- a/src/bin/pg_dump/pg_dump.h
+++ b/src/bin/pg_dump/pg_dump.h
@@ -325,6 +325,10 @@ typedef struct _tableInfo
 	char	   *partbound;		/* partition bound definition */
 	bool		needs_override; /* has GENERATED ALWAYS AS IDENTITY */
 
+	char	  **attcmoptions;	/* per-attribute current compression options */
+	char	  **attcmnames;		/* per-attribute current compression method names */
+	struct _attrCompressionInfo **attcompression; /* per-attribute all compression data */
+
 	/*
 	 * Stuff computed only for dumpable tables.
 	 */
@@ -346,6 +350,19 @@ typedef struct _attrDefInfo
 	bool		separate;		/* true if must dump as separate item */
 } AttrDefInfo;
 
+typedef struct _attrCompressionItem
+{
+	Oid			acoid;			/* attribute compression oid */
+	char	   *acname;			/* compression access method name */
+	char	   *parsedoptions;	/* WITH options */
+} AttrCompressionItem;
+
+typedef struct _attrCompressionInfo
+{
+	int			nitems;
+	AttrCompressionItem	**items;
+} AttrCompressionInfo;
+
 typedef struct _tableDataInfo
 {
 	DumpableObject dobj;
diff --git a/src/bin/pg_dump/pg_dumpall.c b/src/bin/pg_dump/pg_dumpall.c
index 5176626476..d1e13c643a 100644
--- a/src/bin/pg_dump/pg_dumpall.c
+++ b/src/bin/pg_dump/pg_dumpall.c
@@ -74,6 +74,7 @@ static int	no_comments = 0;
 static int	no_publications = 0;
 static int	no_security_labels = 0;
 static int	no_subscriptions = 0;
+static int	no_compression_methods = 0;
 static int	no_unlogged_table_data = 0;
 static int	no_role_passwords = 0;
 static int	server_version;
@@ -136,6 +137,7 @@ main(int argc, char *argv[])
 		{"no-role-passwords", no_argument, &no_role_passwords, 1},
 		{"no-security-labels", no_argument, &no_security_labels, 1},
 		{"no-subscriptions", no_argument, &no_subscriptions, 1},
+		{"no-compression-methods", no_argument, &no_compression_methods, 1},
 		{"no-sync", no_argument, NULL, 4},
 		{"no-unlogged-table-data", no_argument, &no_unlogged_table_data, 1},
 		{"on-conflict-do-nothing", no_argument, &on_conflict_do_nothing, 1},
@@ -406,6 +408,8 @@ main(int argc, char *argv[])
 		appendPQExpBufferStr(pgdumpopts, " --no-security-labels");
 	if (no_subscriptions)
 		appendPQExpBufferStr(pgdumpopts, " --no-subscriptions");
+	if (no_compression_methods)
+		appendPQExpBufferStr(pgdumpopts, " --no-compression-methods");
 	if (no_unlogged_table_data)
 		appendPQExpBufferStr(pgdumpopts, " --no-unlogged-table-data");
 	if (on_conflict_do_nothing)
@@ -622,6 +626,7 @@ help(void)
 	printf(_("  --no-role-passwords          do not dump passwords for roles\n"));
 	printf(_("  --no-security-labels         do not dump security label assignments\n"));
 	printf(_("  --no-subscriptions           do not dump subscriptions\n"));
+	printf(_("  --no-compression-methods     do not dump compression methods\n"));
 	printf(_("  --no-sync                    do not wait for changes to be written safely to disk\n"));
 	printf(_("  --no-tablespaces             do not dump tablespace assignments\n"));
 	printf(_("  --no-unlogged-table-data     do not dump unlogged table data\n"));
diff --git a/src/bin/pg_dump/pg_restore.c b/src/bin/pg_dump/pg_restore.c
index 44012ff44d..fd45a0d7c2 100644
--- a/src/bin/pg_dump/pg_restore.c
+++ b/src/bin/pg_dump/pg_restore.c
@@ -75,6 +75,7 @@ main(int argc, char **argv)
 	static int	no_publications = 0;
 	static int	no_security_labels = 0;
 	static int	no_subscriptions = 0;
+	static int	no_compression_methods = 0;
 	static int	strict_names = 0;
 
 	struct option cmdopts[] = {
@@ -124,6 +125,7 @@ main(int argc, char **argv)
 		{"no-publications", no_argument, &no_publications, 1},
 		{"no-security-labels", no_argument, &no_security_labels, 1},
 		{"no-subscriptions", no_argument, &no_subscriptions, 1},
+		{"no-compression-methods", no_argument, &no_compression_methods, 1},
 
 		{NULL, 0, NULL, 0}
 	};
@@ -375,6 +377,7 @@ main(int argc, char **argv)
 	opts->no_publications = no_publications;
 	opts->no_security_labels = no_security_labels;
 	opts->no_subscriptions = no_subscriptions;
+	opts->no_compression_methods = no_compression_methods;
 
 	if (if_exists && !opts->dropSchema)
 	{
diff --git a/src/bin/pg_dump/t/002_pg_dump.pl b/src/bin/pg_dump/t/002_pg_dump.pl
index ec751a7c23..432b65ef00 100644
--- a/src/bin/pg_dump/t/002_pg_dump.pl
+++ b/src/bin/pg_dump/t/002_pg_dump.pl
@@ -650,6 +650,43 @@ my %tests = (
 		},
 	},
 
+	# compression data in binary upgrade mode
+	'ALTER TABLE test_table_compression ALTER COLUMN ... SET COMPRESSION' => {
+		all_runs  => 1,
+		catch_all => 'ALTER TABLE ... commands',
+		regexp    => qr/^
+			\QCREATE TABLE dump_test.test_table_compression (\E\n
+			\s+\Qcol1 text,\E\n
+			\s+\Qcol2 text,\E\n
+			\s+\Qcol3 text,\E\n
+			\s+\Qcol4 text\E\n
+			\);
+			.*
+			\QSELECT binary_upgrade_set_next_attr_compression_oid('\E\d+\Q'::pg_catalog.oid);\E\n
+			\QALTER TABLE dump_test.test_table_compression ALTER COLUMN col1\E\n
+			\QSET COMPRESSION pglz;\E\n
+			.*
+			\QSELECT binary_upgrade_set_next_attr_compression_oid('\E\d+\Q'::pg_catalog.oid);\E\n
+			\QALTER TABLE dump_test.test_table_compression ALTER COLUMN col2\E\n
+			\QSET COMPRESSION pglz2;\E\n
+			.*
+			\QSELECT binary_upgrade_set_next_attr_compression_oid('\E\d+\Q'::pg_catalog.oid);\E\n
+			\QALTER TABLE dump_test.test_table_compression ALTER COLUMN col3\E\n
+			\QSET COMPRESSION pglz\E\n
+			\QWITH (min_input_size '1000');\E\n
+			.*
+			\QSELECT binary_upgrade_set_next_attr_compression_oid('\E\d+\Q'::pg_catalog.oid);\E\n
+			\QALTER TABLE dump_test.test_table_compression ALTER COLUMN col4\E\n
+			\QSET COMPRESSION pglz2\E\n
+			\QWITH (min_input_size '1000');\E\n
+			\QSELECT binary_upgrade_set_next_attr_compression_oid('\E\d+\Q'::pg_catalog.oid);\E\n
+			\QALTER TABLE dump_test.test_table_compression ALTER COLUMN col4\E\n
+			\QSET COMPRESSION pglz2\E\n
+			\QWITH (min_input_size '2000');\E\n
+			/xms,
+		like => { binary_upgrade => 1, },
+	},
+
 	'ALTER TABLE ONLY test_table ALTER COLUMN col1 SET STATISTICS 90' => {
 		create_order => 93,
 		create_sql =>
@@ -1400,6 +1437,17 @@ my %tests = (
 		like => { %full_runs, section_pre_data => 1, },
 	},
 
+	'CREATE ACCESS METHOD pglz2' => {
+		all_runs     => 1,
+		catch_all    => 'CREATE ... commands',
+		create_order => 52,
+		create_sql =>
+		  'CREATE ACCESS METHOD pglz2 TYPE COMPRESSION HANDLER pglzhandler;',
+		regexp =>
+		  qr/CREATE ACCESS METHOD pglz2 TYPE COMPRESSION HANDLER pglzhandler;/m,
+		like => { %full_runs, section_pre_data => 1, },
+	},
+
 	'CREATE COLLATION test0 FROM "C"' => {
 		create_order => 76,
 		create_sql   => 'CREATE COLLATION test0 FROM "C";',
@@ -2420,6 +2468,53 @@ my %tests = (
 		unlike => { exclude_dump_test_schema => 1, },
 	},
 
+	'CREATE TABLE test_table_compression' => {
+		create_order => 55,
+		create_sql   => 'CREATE TABLE dump_test.test_table_compression (
+						   col1 text,
+						   col2 text COMPRESSION pglz2,
+						   col3 text COMPRESSION pglz WITH (min_input_size \'1000\'),
+						   col4 text COMPRESSION pglz2 WITH (min_input_size \'1000\')
+					     );',
+		regexp => qr/^
+			\QCREATE TABLE dump_test.test_table_compression (\E\n
+			\s+\Qcol1 text,\E\n
+			\s+\Qcol2 text COMPRESSION pglz2,\E\n
+			\s+\Qcol3 text COMPRESSION pglz WITH (min_input_size '1000'),\E\n
+			\s+\Qcol4 text COMPRESSION pglz2 WITH (min_input_size '2000')\E\n
+			\);
+			/xm,
+		like =>
+		  { %full_runs, %dump_test_schema_runs, section_pre_data => 1, },
+		unlike => {
+			binary_upgrade		     => 1,
+			exclude_dump_test_schema => 1,
+		},
+	},
+
+	'ALTER TABLE test_table_compression' => {
+		create_order => 56,
+		create_sql   => 'ALTER TABLE dump_test.test_table_compression
+						 ALTER COLUMN col4
+						 SET COMPRESSION pglz2
+						 WITH (min_input_size \'2000\')
+						 PRESERVE (pglz2);',
+		regexp => qr/^
+			\QCREATE TABLE dump_test.test_table_compression (\E\n
+			\s+\Qcol1 text,\E\n
+			\s+\Qcol2 text COMPRESSION pglz2,\E\n
+			\s+\Qcol3 text COMPRESSION pglz WITH (min_input_size '1000'),\E\n
+			\s+\Qcol4 text COMPRESSION pglz2 WITH (min_input_size '2000')\E\n
+			\);
+			/xm,
+		like =>
+		  { %full_runs, %dump_test_schema_runs, section_pre_data => 1, },
+		unlike => {
+			binary_upgrade		     => 1,
+			exclude_dump_test_schema => 1,
+		},
+	},
+
 	'CREATE STATISTICS extended_stats_no_options' => {
 		create_order => 97,
 		create_sql   => 'CREATE STATISTICS dump_test.test_ext_stats_no_options
diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c
index 4ca0db1d0c..58bc222c0d 100644
--- a/src/bin/psql/describe.c
+++ b/src/bin/psql/describe.c
@@ -1467,6 +1467,7 @@ describeOneTableDetails(const char *schemaname,
 				fdwopts_col = -1,
 				attstorage_col = -1,
 				attstattarget_col = -1,
+				attcompression_col = -1,
 				attdescr_col = -1;
 	int			numrows;
 	struct
@@ -1835,6 +1836,24 @@ describeOneTableDetails(const char *schemaname,
 		appendPQExpBufferStr(&buf, ",\n  a.attstorage");
 		attstorage_col = cols++;
 
+		/* compresssion info */
+		if (pset.sversion >= 120000 &&
+			(tableinfo.relkind == RELKIND_RELATION ||
+			 tableinfo.relkind == RELKIND_PARTITIONED_TABLE))
+		{
+			appendPQExpBufferStr(&buf, ",\n  CASE WHEN attcompression = 0 THEN NULL ELSE "
+								 " (SELECT c.acname || "
+								 "		(CASE WHEN acoptions IS NULL "
+								 "		 THEN '' "
+								 "		 ELSE '(' || array_to_string(ARRAY(SELECT quote_ident(option_name) || ' ' || quote_literal(option_value)"
+								 "											  FROM pg_options_to_table(acoptions)), ', ') || ')'"
+								 " 		 END) "
+								 "  FROM pg_catalog.pg_attr_compression c "
+								 "  WHERE c.acoid = a.attcompression) "
+								 " END AS attcmname");
+			attcompression_col = cols++;
+		}
+
 		/* stats target, if relevant to relkind */
 		if (tableinfo.relkind == RELKIND_RELATION ||
 			tableinfo.relkind == RELKIND_INDEX ||
@@ -1954,6 +1973,8 @@ describeOneTableDetails(const char *schemaname,
 		headers[cols++] = gettext_noop("FDW options");
 	if (attstorage_col >= 0)
 		headers[cols++] = gettext_noop("Storage");
+	if (attcompression_col >= 0)
+		headers[cols++] = gettext_noop("Compression");
 	if (attstattarget_col >= 0)
 		headers[cols++] = gettext_noop("Stats target");
 	if (attdescr_col >= 0)
@@ -2025,6 +2046,27 @@ describeOneTableDetails(const char *schemaname,
 							  false, false);
 		}
 
+		/* Column compression. */
+		if (attcompression_col >= 0)
+		{
+			bool		mustfree = false;
+			const int	trunclen = 100;
+			char *val = PQgetvalue(res, i, attcompression_col);
+
+			/* truncate the options if they're too long */
+			if (strlen(val) > trunclen + 3)
+			{
+				char *trunc = pg_malloc0(trunclen + 4);
+				strncpy(trunc, val, trunclen);
+				strncpy(trunc + trunclen, "...", 4);
+
+				val = trunc;
+				mustfree = true;
+			}
+
+			printTableAddCell(&cont, val, false, mustfree);
+		}
+
 		/* Statistics target, if the relkind supports this feature */
 		if (attstattarget_col >= 0)
 			printTableAddCell(&cont, PQgetvalue(res, i, attstattarget_col),
diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index a980f92e11..a9383ab939 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -1853,11 +1853,14 @@ psql_completion(const char *text, int start, int end)
 	/* ALTER TABLE ALTER [COLUMN] <foo> SET */
 	else if (Matches("ALTER", "TABLE", MatchAny, "ALTER", "COLUMN", MatchAny, "SET") ||
 			 Matches("ALTER", "TABLE", MatchAny, "ALTER", MatchAny, "SET"))
-		COMPLETE_WITH("(", "DEFAULT", "NOT NULL", "STATISTICS", "STORAGE");
+		COMPLETE_WITH("(", "COMPRESSION", "DEFAULT", "NOT NULL", "STATISTICS", "STORAGE");
 	/* ALTER TABLE ALTER [COLUMN] <foo> SET ( */
 	else if (Matches("ALTER", "TABLE", MatchAny, "ALTER", "COLUMN", MatchAny, "SET", "(") ||
 			 Matches("ALTER", "TABLE", MatchAny, "ALTER", MatchAny, "SET", "("))
 		COMPLETE_WITH("n_distinct", "n_distinct_inherited");
+	else if (Matches("ALTER", "TABLE", MatchAny, "ALTER", "COLUMN", MatchAny, "SET", "COMPRESSION", MatchAny) ||
+			 Matches("ALTER", "TABLE", MatchAny, "ALTER", MatchAny, "SET", "COMPRESSION", MatchAny))
+		COMPLETE_WITH("WITH (", "PRESERVE (");
 	/* ALTER TABLE ALTER [COLUMN] <foo> SET STORAGE */
 	else if (Matches("ALTER", "TABLE", MatchAny, "ALTER", "COLUMN", MatchAny, "SET", "STORAGE") ||
 			 Matches("ALTER", "TABLE", MatchAny, "ALTER", MatchAny, "SET", "STORAGE"))
diff --git a/src/include/catalog/binary_upgrade.h b/src/include/catalog/binary_upgrade.h
index abc6e1ae1d..1e95a3863a 100644
--- a/src/include/catalog/binary_upgrade.h
+++ b/src/include/catalog/binary_upgrade.h
@@ -25,6 +25,8 @@ extern PGDLLIMPORT Oid binary_upgrade_next_toast_pg_class_oid;
 extern PGDLLIMPORT Oid binary_upgrade_next_pg_enum_oid;
 extern PGDLLIMPORT Oid binary_upgrade_next_pg_authid_oid;
 
+extern PGDLLIMPORT Oid binary_upgrade_next_attr_compression_oid;
+
 extern PGDLLIMPORT bool binary_upgrade_record_init_privs;
 
 #endif							/* BINARY_UPGRADE_H */
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index fa82fbed05..4528a3c2c1 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -9868,6 +9868,10 @@
   proname => 'binary_upgrade_set_missing_value', provolatile => 'v',
   proparallel => 'u', prorettype => 'void', proargtypes => 'oid text text',
   prosrc => 'binary_upgrade_set_missing_value' },
+{ oid => '4012', descr => 'for use by pg_upgrade',
+  proname => 'binary_upgrade_set_next_attr_compression_oid', provolatile => 'v',
+  proparallel => 'r', prorettype => 'void', proargtypes => 'oid',
+  prosrc => 'binary_upgrade_set_next_attr_compression_oid' },
 
 # replication/origin.h
 { oid => '6003', descr => 'create a replication origin',
-- 
2.19.1


--MP_/7ZVSJ3tZdZjf_J65xpltpI_
Content-Type: text/x-patch
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
 filename=0007-Add-tests-for-compression-methods-v20.patch



^ permalink  raw  reply  [nested|flat] 18+ messages in thread

* [PATCH] Test to demonstrate bug in commit 0d3dba38c7 and to verify a fix.
@ 2026-05-12 10:27 Antonin Houska <ah@cybertec.at>
  0 siblings, 0 replies; 18+ messages in thread

From: Antonin Houska @ 2026-05-12 10:27 UTC (permalink / raw)

The fix: https://www.postgresql.org/message-id/77611.1778055944%40localhost
---
 src/backend/access/transam/xact.c             |   2 +
 src/backend/commands/repack_worker.c          |   2 +
 src/test/isolation/isolationtester.c          |   9 +-
 .../expected/repack_running_xacts.out         |  81 ++++++++++++
 .../specs/repack_running_xacts.spec           | 119 ++++++++++++++++++
 5 files changed, 212 insertions(+), 1 deletion(-)
 create mode 100644 src/test/modules/injection_points/expected/repack_running_xacts.out
 create mode 100644 src/test/modules/injection_points/specs/repack_running_xacts.spec

diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c
index 5586fbe5b07..b63ee166028 100644
--- a/src/backend/access/transam/xact.c
+++ b/src/backend/access/transam/xact.c
@@ -65,6 +65,7 @@
 #include "utils/builtins.h"
 #include "utils/combocid.h"
 #include "utils/guc.h"
+#include "utils/injection_point.h"
 #include "utils/inval.h"
 #include "utils/memutils.h"
 #include "utils/relmapper.h"
@@ -2428,6 +2429,7 @@ CommitTransaction(void)
 	 * must be done _before_ releasing locks we hold and _after_
 	 * RecordTransactionCommit.
 	 */
+	INJECTION_POINT("before-end-transaction", NULL);
 	ProcArrayEndTransaction(MyProc, latestXid);
 
 	/*
diff --git a/src/backend/commands/repack_worker.c b/src/backend/commands/repack_worker.c
index c40f8c98e06..6835626d677 100644
--- a/src/backend/commands/repack_worker.c
+++ b/src/backend/commands/repack_worker.c
@@ -26,6 +26,7 @@
 #include "storage/ipc.h"
 #include "storage/proc.h"
 #include "tcop/tcopprot.h"
+#include "utils/injection_point.h"
 #include "utils/memutils.h"
 
 #define REPL_PLUGIN_NAME   "pgrepack"
@@ -233,6 +234,7 @@ repack_setup_logical_decoding(Oid relid)
 	 * Neither prepare_write nor do_write callback nor update_progress is
 	 * useful for us.
 	 */
+	INJECTION_POINT("before-create-decoding-context", NULL);
 	ctx = CreateInitDecodingContext(REPL_PLUGIN_NAME,
 									NIL,
 									true,
diff --git a/src/test/isolation/isolationtester.c b/src/test/isolation/isolationtester.c
index 440c875b8ac..8f17ee412c9 100644
--- a/src/test/isolation/isolationtester.c
+++ b/src/test/isolation/isolationtester.c
@@ -216,15 +216,22 @@ main(int argc, char **argv)
 	 * exactly expect concurrent use of test tables.  However, autovacuum will
 	 * occasionally take AccessExclusiveLock to truncate a table, and we must
 	 * ignore that transient wait.
+	 *
+	 * If the session's backend is blocked, and if its background worker is
+	 * waiting on an injection point, we assume that the injection point is
+	 * the reason for the backend to be blocked. That's what we check in the
+	 * second query of the UNION. XXX Should we use a separate query for that?
 	 */
 	initPQExpBuffer(&wait_query);
 	appendPQExpBufferStr(&wait_query,
+						 "WITH blocking(res) AS ("
 						 "SELECT pg_catalog.pg_isolation_test_session_is_blocked($1, '{");
 	/* The spec syntax requires at least one session; assume that here. */
 	appendPQExpBufferStr(&wait_query, conns[1].backend_pid_str);
 	for (i = 2; i < nconns; i++)
 		appendPQExpBuffer(&wait_query, ",%s", conns[i].backend_pid_str);
-	appendPQExpBufferStr(&wait_query, "}')");
+	appendPQExpBufferStr(&wait_query, "}') UNION "
+						 "SELECT pg_catalog.pg_isolation_test_session_is_blocked(pid, '{}') FROM pg_stat_activity WHERE leader_pid=$1) SELECT bool_or(res) FROM blocking");
 
 	res = PQprepare(conns[0].conn, PREP_WAITING, wait_query.data, 0, NULL);
 	if (PQresultStatus(res) != PGRES_COMMAND_OK)
diff --git a/src/test/modules/injection_points/expected/repack_running_xacts.out b/src/test/modules/injection_points/expected/repack_running_xacts.out
new file mode 100644
index 00000000000..271fe2b97cb
--- /dev/null
+++ b/src/test/modules/injection_points/expected/repack_running_xacts.out
@@ -0,0 +1,81 @@
+Parsed test spec with 5 sessions
+
+starting permutation: repack s3_assign_xid wakeup_bcdc s4_changes s4_attach s4_commit s3_commit s5_assign_xid wakeup_bet s5_commit check
+injection_points_attach
+-----------------------
+                       
+(1 row)
+
+step repack: 
+	REPACK (CONCURRENTLY) repack_test;
+ <waiting ...>
+step s3_assign_xid: 
+	BEGIN;
+	INSERT INTO aux VALUES (1);
+
+step wakeup_bcdc: 
+	SELECT injection_points_wakeup('before-create-decoding-context');
+
+injection_points_wakeup
+-----------------------
+                       
+(1 row)
+
+step s4_changes: 
+	BEGIN;
+	INSERT INTO repack_test(i, j) VALUES (1, 1);
+
+step s4_attach: 
+	SELECT injection_points_set_local();
+	SELECT injection_points_attach('before-end-transaction', 'wait');
+
+injection_points_set_local
+--------------------------
+                          
+(1 row)
+
+injection_points_attach
+-----------------------
+                       
+(1 row)
+
+step s4_commit: 
+	COMMIT;
+ <waiting ...>
+step s3_commit: 
+	COMMIT;
+
+step s5_assign_xid: 
+	BEGIN;
+	INSERT INTO aux VALUES (2);
+
+step wakeup_bet: 
+	SELECT injection_points_wakeup('before-end-transaction');
+
+injection_points_wakeup
+-----------------------
+                       
+(1 row)
+
+step repack: <... completed>
+step s4_commit: <... completed>
+step s5_commit: 
+	COMMIT;
+
+step check: 
+	TABLE repack_test;
+
+i|j
+-+-
+(0 rows)
+
+injection_points_detach
+-----------------------
+                       
+(1 row)
+
+injection_points_detach
+-----------------------
+                       
+(1 row)
+
diff --git a/src/test/modules/injection_points/specs/repack_running_xacts.spec b/src/test/modules/injection_points/specs/repack_running_xacts.spec
new file mode 100644
index 00000000000..1f878514046
--- /dev/null
+++ b/src/test/modules/injection_points/specs/repack_running_xacts.spec
@@ -0,0 +1,119 @@
+setup
+{
+	CREATE EXTENSION injection_points;
+	CREATE TABLE repack_test(i int PRIMARY KEY, j int);
+	CREATE TABLE aux(i int);
+}
+
+teardown
+{
+	DROP TABLE repack_test;
+	DROP TABLE aux;
+	DROP EXTENSION injection_points;
+}
+
+session s1
+setup
+{
+	SELECT injection_points_attach('before-create-decoding-context', 'wait');
+}
+step repack
+{
+	REPACK (CONCURRENTLY) repack_test;
+}
+step check
+{
+	TABLE repack_test;
+}
+teardown
+{
+	SELECT injection_points_detach('before-create-decoding-context');
+}
+
+session s2
+step wakeup_bcdc
+{
+	SELECT injection_points_wakeup('before-create-decoding-context');
+}
+step wakeup_bet
+{
+	SELECT injection_points_wakeup('before-end-transaction');
+}
+
+session s3
+step s3_assign_xid
+{
+	BEGIN;
+	INSERT INTO aux VALUES (1);
+}
+step s3_commit
+{
+	COMMIT;
+}
+
+session s4
+step s4_changes
+{
+	BEGIN;
+	INSERT INTO repack_test(i, j) VALUES (1, 1);
+}
+# Do not attach in the setup section, that would be too soon.
+step s4_attach
+{
+	SELECT injection_points_set_local();
+	SELECT injection_points_attach('before-end-transaction', 'wait');
+}
+step s4_commit
+{
+	COMMIT;
+}
+teardown
+{
+	SELECT injection_points_detach('before-end-transaction');
+}
+
+session s5
+step s5_assign_xid
+{
+	BEGIN;
+	INSERT INTO aux VALUES (2);
+}
+step s5_commit
+{
+	COMMIT;
+}
+
+permutation
+repack
+# Assign XID so that a running transaction prevents the snapshot builder from
+# reaching CONSISTENT state immediately. It will wait for this to complete
+# after having reached BUILDING_SNAPSHOT.
+s3_assign_xid
+# Let the decoding setup start.
+wakeup_bcdc
+# Likewise, the snapshot builder will wait for the s4's xact to complete after
+# having reached FULL_SNAPSHOT. This is the problematic transaction, so let it
+# do some changes.
+s4_changes
+# Attach to the 'before-end-transaction' injection point that s4 will need
+# during commit.
+s4_attach
+# Only write commit record for s4, but do not remove the xact from procarray
+# yet. Thus the snapshot builder still needs to wait.
+s4_commit
+# Let the snapshot builder proceed to FULL_SNAPSHOT.
+s3_commit
+# Start another transaction so that CONSISTENT is not reached "directly",
+# i.e. due to no running transaction. It's important here that builder->xmin
+# does not advance.
+s5_assign_xid
+# Remove s4 xact from procarray, and thus reach the CONSISTENT state. Since
+# the COMMIT appeared in WAL too early (i.e. when the snapshot builder state
+# did not allow decoding of COMMIT records yet), the snapshot builder will
+# consider s4 running. This is also due to returning from
+# SnapBuildProcessRunningXacts() too early, w/o advancing builder->xmin.
+wakeup_bet
+# s5 is not needed anymore
+s5_commit
+# Show that the data changes performed by s4 are lost.
+check
-- 
2.47.3


--=-=-=--





^ permalink  raw  reply  [nested|flat] 18+ messages in thread


end of thread, other threads:[~2026-05-12 10:27 UTC | newest]

Thread overview: 18+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2003-10-14 19:59 Re: Database Kernels and O_DIRECT James Rogers <jamesr@best.com>
2003-10-15 03:26 ` Greg Stark <gsstark@mit.edu>
2003-10-15 06:31   ` James Rogers <jamesr@best.com>
2003-10-15 08:26     ` James Rogers <jamesr@best.com>
2003-10-15 21:12       ` Hannu Krosing <hannu@tm.ee>
2003-10-15 15:43     ` Tom Lane <tgl@sss.pgh.pa.us>
2003-10-15 21:02       ` Andrew Dunstan <andrew@dunslane.net>
2003-10-16 05:51         ` Manfred Spraul <manfred@colorfullife.com>
2003-10-16 14:31         ` Christopher Browne <cbbrowne@libertyrms.info>
2003-10-16 00:49       ` Sailesh Krishnamurthy <sailesh@cs.berkeley.edu>
2003-10-26 04:12       ` Bruce Momjian <pgman@candle.pha.pa.us>
2003-10-15 13:09   ` Bruce Momjian <pgman@candle.pha.pa.us>
2003-10-15 14:14     ` Paulo Scardine <paulos@cimed.ind.br>
2018-06-18 12:57 [PATCH 6/8] Add psql, pg_dump and pg_upgrade support Ildus Kurbangaliev <i.kurbangaliev@gmail.com>
2018-06-18 12:57 [PATCH 6/8] Add psql, pg_dump and pg_upgrade support Ildus Kurbangaliev <i.kurbangaliev@gmail.com>
2018-06-18 12:57 [PATCH 6/8] Add psql, pg_dump and pg_upgrade support Ildus Kurbangaliev <i.kurbangaliev@gmail.com>
2018-06-18 12:57 [PATCH 6/8] Add psql, pg_dump and pg_upgrade support Ildus Kurbangaliev <i.kurbangaliev@gmail.com>
2026-05-12 10:27 [PATCH] Test to demonstrate bug in commit 0d3dba38c7 and to verify a fix. Antonin Houska <ah@cybertec.at>

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox