Artur, * Artur Zakirov (a.zaki...@postgrespro.ru) wrote: > 2016-11-19 21:28 GMT+03:00 Michael Paquier <michael.paqu...@gmail.com>: > > On Thu, Nov 17, 2016 at 1:17 PM, Alvaro Herrera > > <alvhe...@2ndquadrant.com> wrote: > >> It's a bug. You're right that we need to handle the object class > >> somewhere. Perhaps I failed to realize that tsconfigs could get > >> altered. > > > > It seems to me that the thing to be careful of here is how a new > > OBJECT_TSCONFIGURATIONMAP should use ObjectAddress. It does not seem > > that complicated, but it needs some work. > > -- > > Michael > > After some investigation it seems to me that OBJECT_TSCONFIGURATIONMAP > can't be added for pg_ts_config_map. Because this catalog hasn't Oids.
I started looking into this, in part because it's a bug fix. > But this bug can be easily fixed (patch attached). I think in > AlterTSConfiguration() TSConfigRelationId should be used instead of > TSConfigMapRelationId. Secondly, in ProcessUtilitySlow() we can use > commandCollected = true. Because configuration entry is added in > EventTriggerCollectAlterTSConfig() into > currentEventTriggerState->commandList. We should definitely be using TSConfigRelationId there, as the tuple we have the OID of is from pg_ts_config, not from pg_ts_config_map. You're also right that we need to set commandCollected = true since the MakConfigurationMapping() and DropConfigurationMapping() functions get called from AlterTSConfiguration and, as you say, they call EventTriggerCollectAlterTSConfig(). Looks like the InvokeObjectPostAlterHook() call has been around since 9.3, so we'll need to back-patch it that far, while the other changes go back to 9.5. Did you happen to look at adding a regression test for this to test_ddl_deparse? > This patch only fixes the bug. But I think I also can do a patch which > will give pg_ts_config_map entries with > pg_event_trigger_ddl_commands() if someone wants. It can be done using > new entry in the CollectedCommandType structure maybe. While that sounds like a good idea, it seems like it's more a feature addition rather than a bugfix, no? Thanks! Stephen
signature.asc
Description: Digital signature
