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>
  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-15 03:26  Greg Stark <gsstark@mit.edu>
  parent: James Rogers <jamesr@best.com>
  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-15 06:31  James Rogers <jamesr@best.com>
  parent: Greg Stark <gsstark@mit.edu>
  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-15 08:26  James Rogers <jamesr@best.com>
  parent: James Rogers <jamesr@best.com>
  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-15 13:09  Bruce Momjian <pgman@candle.pha.pa.us>
  parent: Greg Stark <gsstark@mit.edu>
  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-15 14:14  Paulo Scardine <paulos@cimed.ind.br>
  parent: Bruce Momjian <pgman@candle.pha.pa.us>
  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

* Re: Database Kernels and O_DIRECT
@ 2003-10-15 15:43  Tom Lane <tgl@sss.pgh.pa.us>
  parent: James Rogers <jamesr@best.com>
  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-15 21:02  Andrew Dunstan <andrew@dunslane.net>
  parent: Tom Lane <tgl@sss.pgh.pa.us>
  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-15 21:12  Hannu Krosing <hannu@tm.ee>
  parent: James Rogers <jamesr@best.com>
  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-16 00:49  Sailesh Krishnamurthy <sailesh@cs.berkeley.edu>
  parent: Tom Lane <tgl@sss.pgh.pa.us>
  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-16 05:51  Manfred Spraul <manfred@colorfullife.com>
  parent: Andrew Dunstan <andrew@dunslane.net>
  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-16 14:31  Christopher Browne <cbbrowne@libertyrms.info>
  parent: Andrew Dunstan <andrew@dunslane.net>
  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-26 04:12  Bruce Momjian <pgman@candle.pha.pa.us>
  parent: Tom Lane <tgl@sss.pgh.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

* [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 v52 10/10] CheckLogicalDecodingRequirements: be specific about which GUC is limiting
@ 2026-04-03 19:01  Álvaro Herrera <alvherre@kurilemu.de>
  0 siblings, 0 replies; 18+ messages in thread

From: Álvaro Herrera @ 2026-04-03 19:01 UTC (permalink / raw)

---
 src/backend/commands/repack_worker.c           |  3 ++-
 src/backend/replication/logical/logical.c      |  8 +++++---
 src/backend/replication/logical/logicalfuncs.c |  2 +-
 src/backend/replication/slot.c                 | 18 ++++++++++++------
 src/backend/replication/slotfuncs.c            | 11 ++++++-----
 src/backend/replication/walsender.c            |  5 +++--
 src/include/replication/logical.h              |  3 ++-
 src/include/replication/slot.h                 |  2 +-
 8 files changed, 32 insertions(+), 20 deletions(-)

diff --git a/src/backend/commands/repack_worker.c b/src/backend/commands/repack_worker.c
index 610592a05b0..ca827223845 100644
--- a/src/backend/commands/repack_worker.c
+++ b/src/backend/commands/repack_worker.c
@@ -224,7 +224,7 @@ repack_setup_logical_decoding(Oid relid)
 	 * Make sure we can use logical decoding.
 	 */
 	CheckSlotPermissions();
-	CheckLogicalDecodingRequirements();
+	CheckLogicalDecodingRequirements(true);
 
 	/*
 	 * A single backend should not execute multiple REPACK commands at a time,
@@ -252,6 +252,7 @@ repack_setup_logical_decoding(Oid relid)
 	ctx = CreateInitDecodingContext(REPL_PLUGIN_NAME,
 									NIL,
 									true,
+									true,
 									InvalidXLogRecPtr,
 									XL_ROUTINE(.page_read = read_local_xlog_page,
 											   .segment_open = wal_segment_open,
diff --git a/src/backend/replication/logical/logical.c b/src/backend/replication/logical/logical.c
index f20a0fe70ad..a08aece5731 100644
--- a/src/backend/replication/logical/logical.c
+++ b/src/backend/replication/logical/logical.c
@@ -108,9 +108,9 @@ static void LoadOutputPlugin(OutputPluginCallbacks *callbacks, const char *plugi
  * decoding.
  */
 void
-CheckLogicalDecodingRequirements(void)
+CheckLogicalDecodingRequirements(bool repack)
 {
-	CheckSlotRequirements();
+	CheckSlotRequirements(repack);
 
 	/*
 	 * NB: Adding a new requirement likely means that RestoreSlotFromDisk()
@@ -305,6 +305,7 @@ StartupDecodingContext(List *output_plugin_options,
  * output_plugin_options -- contains options passed to the output plugin
  * need_full_snapshot -- if true, must obtain a snapshot able to read all
  *		tables; if false, one that can read only catalogs is acceptable.
+ * for_repack -- if true, we're going to be decoding for REPACK.
  * restart_lsn -- if given as invalid, it's this routine's responsibility to
  *		mark WAL as reserved by setting a convenient restart_lsn for the slot.
  *		Otherwise, we set for decoding to start from the given LSN without
@@ -325,6 +326,7 @@ LogicalDecodingContext *
 CreateInitDecodingContext(const char *plugin,
 						  List *output_plugin_options,
 						  bool need_full_snapshot,
+						  bool for_repack,
 						  XLogRecPtr restart_lsn,
 						  XLogReaderRoutine *xl_routine,
 						  LogicalOutputPluginWriterPrepareWrite prepare_write,
@@ -341,7 +343,7 @@ CreateInitDecodingContext(const char *plugin,
 	 * On a standby, this check is also required while creating the slot.
 	 * Check the comments in the function.
 	 */
-	CheckLogicalDecodingRequirements();
+	CheckLogicalDecodingRequirements(for_repack);
 
 	/* shorter lines... */
 	slot = MyReplicationSlot;
diff --git a/src/backend/replication/logical/logicalfuncs.c b/src/backend/replication/logical/logicalfuncs.c
index 9760818941d..512013b0ef0 100644
--- a/src/backend/replication/logical/logicalfuncs.c
+++ b/src/backend/replication/logical/logicalfuncs.c
@@ -115,7 +115,7 @@ pg_logical_slot_get_changes_guts(FunctionCallInfo fcinfo, bool confirm, bool bin
 
 	CheckSlotPermissions();
 
-	CheckLogicalDecodingRequirements();
+	CheckLogicalDecodingRequirements(false);
 
 	if (PG_ARGISNULL(0))
 		ereport(ERROR,
diff --git a/src/backend/replication/slot.c b/src/backend/replication/slot.c
index 2c6c6773ad2..13004ed547a 100644
--- a/src/backend/replication/slot.c
+++ b/src/backend/replication/slot.c
@@ -1669,19 +1669,25 @@ CheckLogicalSlotExists(void)
  * slots.
  */
 void
-CheckSlotRequirements(void)
+CheckSlotRequirements(bool repack)
 {
+	int		limit;
+
 	/*
 	 * NB: Adding a new requirement likely means that RestoreSlotFromDisk()
 	 * needs the same check.
 	 */
 
-	/* XXX we should be able to check exactly which type of slot we need */
-	if (max_replication_slots + max_repack_replication_slots == 0)
+	if (repack)
+		limit = max_repack_replication_slots;
+	else
+		limit = max_replication_slots;
+
+	if (limit == 0)
 		ereport(ERROR,
-				(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
-				 errmsg("replication slots can only be used if \"%s\" > 0 or \"%s\" > 0",
-						"max_replication_slots", "max_repack_replication_slots")));
+				errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+				errmsg("replication slots can only be used if \"%s\" > 0",
+					   repack ? "max_repack_replication_slots" : "max_replication_slots"));
 
 	if (wal_level < WAL_LEVEL_REPLICA)
 		ereport(ERROR,
diff --git a/src/backend/replication/slotfuncs.c b/src/backend/replication/slotfuncs.c
index 78dd3c4ea66..16fbd383735 100644
--- a/src/backend/replication/slotfuncs.c
+++ b/src/backend/replication/slotfuncs.c
@@ -90,7 +90,7 @@ pg_create_physical_replication_slot(PG_FUNCTION_ARGS)
 
 	CheckSlotPermissions();
 
-	CheckSlotRequirements();
+	CheckSlotRequirements(false);
 
 	create_physical_replication_slot(NameStr(*name),
 									 immediately_reserve,
@@ -164,6 +164,7 @@ create_logical_replication_slot(char *name, char *plugin,
 	 */
 	ctx = CreateInitDecodingContext(plugin, NIL,
 									false,	/* just catalogs is OK */
+									false,	/* not repack */
 									restart_lsn,
 									XL_ROUTINE(.page_read = read_local_xlog_page,
 											   .segment_open = wal_segment_open,
@@ -203,7 +204,7 @@ pg_create_logical_replication_slot(PG_FUNCTION_ARGS)
 
 	CheckSlotPermissions();
 
-	CheckLogicalDecodingRequirements();
+	CheckLogicalDecodingRequirements(false);
 
 	create_logical_replication_slot(NameStr(*name),
 									NameStr(*plugin),
@@ -240,7 +241,7 @@ pg_drop_replication_slot(PG_FUNCTION_ARGS)
 
 	CheckSlotPermissions();
 
-	CheckSlotRequirements();
+	CheckSlotRequirements(false);
 
 	ReplicationSlotDrop(NameStr(*name), true);
 
@@ -648,9 +649,9 @@ copy_replication_slot(FunctionCallInfo fcinfo, bool logical_slot)
 	CheckSlotPermissions();
 
 	if (logical_slot)
-		CheckLogicalDecodingRequirements();
+		CheckLogicalDecodingRequirements(false);
 	else
-		CheckSlotRequirements();
+		CheckSlotRequirements(false);
 
 	LWLockAcquire(ReplicationSlotControlLock, LW_SHARED);
 
diff --git a/src/backend/replication/walsender.c b/src/backend/replication/walsender.c
index 75ef3419a15..9d7d675fa96 100644
--- a/src/backend/replication/walsender.c
+++ b/src/backend/replication/walsender.c
@@ -1240,7 +1240,7 @@ CreateReplicationSlot(CreateReplicationSlotCmd *cmd)
 
 		Assert(cmd->kind == REPLICATION_KIND_LOGICAL);
 
-		CheckLogicalDecodingRequirements();
+		CheckLogicalDecodingRequirements(false);
 
 		/*
 		 * Initially create persistent slot as ephemeral - that allows us to
@@ -1309,6 +1309,7 @@ CreateReplicationSlot(CreateReplicationSlotCmd *cmd)
 		Assert(IsLogicalDecodingEnabled());
 
 		ctx = CreateInitDecodingContext(cmd->plugin, NIL, need_full_snapshot,
+										false,
 										InvalidXLogRecPtr,
 										XL_ROUTINE(.page_read = logical_read_xlog_page,
 												   .segment_open = WalSndSegmentOpen,
@@ -1466,7 +1467,7 @@ StartLogicalReplication(StartReplicationCmd *cmd)
 	QueryCompletion qc;
 
 	/* make sure that our requirements are still fulfilled */
-	CheckLogicalDecodingRequirements();
+	CheckLogicalDecodingRequirements(false);
 
 	Assert(!MyReplicationSlot);
 
diff --git a/src/include/replication/logical.h b/src/include/replication/logical.h
index bc9d4ece672..bc075b16741 100644
--- a/src/include/replication/logical.h
+++ b/src/include/replication/logical.h
@@ -115,11 +115,12 @@ typedef struct LogicalDecodingContext
 } LogicalDecodingContext;
 
 
-extern void CheckLogicalDecodingRequirements(void);
+extern void CheckLogicalDecodingRequirements(bool repack);
 
 extern LogicalDecodingContext *CreateInitDecodingContext(const char *plugin,
 														 List *output_plugin_options,
 														 bool need_full_snapshot,
+														 bool for_repack,
 														 XLogRecPtr restart_lsn,
 														 XLogReaderRoutine *xl_routine,
 														 LogicalOutputPluginWriterPrepareWrite prepare_write,
diff --git a/src/include/replication/slot.h b/src/include/replication/slot.h
index c316a01a807..489af7d8d6c 100644
--- a/src/include/replication/slot.h
+++ b/src/include/replication/slot.h
@@ -378,7 +378,7 @@ extern void ReplicationSlotDropAtPubNode(WalReceiverConn *wrconn, char *slotname
 extern void StartupReplicationSlots(void);
 extern void CheckPointReplicationSlots(bool is_shutdown);
 
-extern void CheckSlotRequirements(void);
+extern void CheckSlotRequirements(bool repack);
 extern void CheckSlotPermissions(void);
 extern ReplicationSlotInvalidationCause
 			GetSlotInvalidationCause(const char *cause_name);
-- 
2.47.3


--gp2pyozrd5pweboh--





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


end of thread, other threads:[~2026-04-03 19:01 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-04-03 19:01 [PATCH v52 10/10] CheckLogicalDecodingRequirements: be specific about which GUC is limiting Álvaro Herrera <alvherre@kurilemu.de>

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