Thread: handling of heap rewrites in logical decoding
A heap rewrite during a DDL command publishes changes for relations named like pg_temp_%u, which are not real tables, and this breaks replication systems that are not aware of that. We have a hack in the pgoutput plugin to ignore those, but we knew that was a hack. So here is an attempt at a more proper solution: Store a relisrewrite column in pg_class and filter on that. I have put this into reorderbuffer.c, so that it affects all plugins. An alternative would be to have each plugin handle it separately (the way it is done now), but it's hard to see why a plugin would not want this fixed behavior. A more fancy solution would be to make this column an OID that links back to the original table. Then, a DDL-aware plugin could choose replicate the rewrite rows. However, at the moment no plugin works that way, so this might be overdoing it. -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Attachment
On 25 February 2018 at 09:57, Peter Eisentraut <peter.eisentraut@2ndquadrant.com> wrote:
I'm pretty sure we _will_ want the ability to decode and stream rewrite contents for non-IMMUTABLE table rewrites.
A heap rewrite during a DDL command publishes changes for relations
named like pg_temp_%u, which are not real tables, and this breaks
replication systems that are not aware of that. We have a hack in the
pgoutput plugin to ignore those, but we knew that was a hack. So here
is an attempt at a more proper solution: Store a relisrewrite column in
pg_class and filter on that.
I have put this into reorderbuffer.c, so that it affects all plugins.
An alternative would be to have each plugin handle it separately (the
way it is done now), but it's hard to see why a plugin would not want
this fixed behavior.
A more fancy solution would be to make this column an OID that links
back to the original table. Then, a DDL-aware plugin could choose
replicate the rewrite rows. However, at the moment no plugin works that
way, so this might be overdoing it.
I'm pretty sure we _will_ want the ability to decode and stream rewrite contents for non-IMMUTABLE table rewrites.
Filtering out by default is OK by me, but I think making it impossible to decode is a mistake. So I'm all for the oid option and had written a suggestion for it before I saw you already mentioned it in the next part of your mail.
The main issue with filtering out rewrites by default is that I don't see how, if we default to ignore/filter-out, plugins would indicate "actually I want to choose about this one" or "I understand table rewrites". I'd prefer not to add another change callback.
On 2/25/18 07:27, Craig Ringer wrote: > I'm pretty sure we _will_ want the ability to decode and stream rewrite > contents for non-IMMUTABLE table rewrites. > > Filtering out by default is OK by me, but I think making it impossible > to decode is a mistake. So I'm all for the oid option and had written a > suggestion for it before I saw you already mentioned it in the next > part of your mail. > > The main issue with filtering out rewrites by default is that I don't > see how, if we default to ignore/filter-out, plugins would indicate > "actually I want to choose about this one" or "I understand table > rewrites". I'd prefer not to add another change callback. Second version, which uses an OID. I added another field to the output plugin options (next to the output_type), to indicate whether the plugin wants to receive these changes. I added some test cases to test_decoding to show how it works either way. It's a bit messy to pass this setting through to the ReorderBuffer; maybe there is a better idea. But the result seems pretty useful. -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Attachment
On 2/28/18 13:52, Peter Eisentraut wrote:> Second version, which uses an OID. I added another field to the output> plugin options (next to the output_type), to indicate whether the plugin> wants to receive these changes. I added some test cases to> test_decoding to show how it works either way. It's a bit messy to pass> this setting through to the ReorderBuffer; maybe there is a better idea.> But the result seems pretty useful. Here is a rebased update of this patch. No functionality changes compared to v2. -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Attachment
On 16 March 2018 at 08:51, Peter Eisentraut <peter.eisentraut@2ndquadrant.com> wrote:
On 2/28/18 13:52, Peter Eisentraut wrote:> Second version, which uses an
OID. I added another field to the output> plugin options (next to the
output_type), to indicate whether the plugin> wants to receive these
changes. I added some test cases to> test_decoding to show how it works
either way. It's a bit messy to pass> this setting through to the
ReorderBuffer; maybe there is a better idea.> But the result seems
pretty useful.
Here is a rebased update of this patch. No functionality changes
compared to v2.
On 16 March 2018 at 10:54, Craig Ringer <craig@2ndquadrant.com> wrote:
On 16 March 2018 at 08:51, Peter Eisentraut <peter.eisentraut@2ndquadrant.com> wrote: On 2/28/18 13:52, Peter Eisentraut wrote:> Second version, which uses an
OID. I added another field to the output> plugin options (next to the
output_type), to indicate whether the plugin> wants to receive these
changes. I added some test cases to> test_decoding to show how it works
either way. It's a bit messy to pass> this setting through to the
ReorderBuffer; maybe there is a better idea.> But the result seems
pretty useful.
Here is a rebased update of this patch. No functionality changes
compared to v2.Picking this up for review.
Why was relrewrite / Anum_pg_class_relrewrite inserted at position 28, not appended to pg_class?
I don't see how it'd cause any harm, I'm just curious about the rationale.
Otherwise, I'm totally happy with what I see here. It's always nice to get to the end of a patch and think "wait, that's all?"
Ready to go.
On 3/21/18 03:08, Craig Ringer wrote: > Why was relrewrite / Anum_pg_class_relrewrite inserted at position 28, > not appended to pg_class? > > I don't see how it'd cause any harm, I'm just curious about the rationale. Adding it at the end would not be appropriate, since those are variable-length fields. So I added it close to other fields of similar purpose. > Otherwise, I'm totally happy with what I see here. It's always nice to > get to the end of a patch and think "wait, that's all?" > > Ready to go. committed -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On 21 March 2018 at 21:20, Peter Eisentraut <peter.eisentraut@2ndquadrant.com> wrote:
On 3/21/18 03:08, Craig Ringer wrote:
> Why was relrewrite / Anum_pg_class_relrewrite inserted at position 28,
> not appended to pg_class?
>
> I don't see how it'd cause any harm, I'm just curious about the rationale.
Adding it at the end would not be appropriate, since those are
variable-length fields. So I added it close to other fields of similar
purpose.
Right, that makes sense. You're renumbering attnos anyway, so you might as well put it somewhere logical.