Date: Wed, 21 Dec 2016 12:34:01 -0500
From: Stephen Frost <sfrost@snowman.net>
To: Artur Zakirov <a.zakirov@postgrespro.ru>
Cc: Michael Paquier <michael.paquier@gmail.com>,
	Alvaro Herrera <alvherre@2ndquadrant.com>,
	PostgreSQL Hackers <pgsql-hackers@postgresql.org>
Subject: Re: [BUG?] pg_event_trigger_ddl_commands() error with
 ALTER TEXT SEARCH CONFIGURATION
Message-ID: <20161221173401.GI18360@tamriel.snowman.net>
References: <57a71eba-f2c7-e7fd-6fc0-2126ec0b39bd@postgrespro.ru>
 <20161117211754.6zdetsqtrmt552bz@alvherre.pgsql>
 <CAB7nPqSD2-Ny+98Bz_+vP5YW+Nm9Q2ekPFxR88WRrF865yEn7Q@mail.gmail.com>
 <CAKNkYnxhzHLE0Vf_eX6SdR-Mskx3q6brREwqm-CN1dsVDH9Auw@mail.gmail.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="s7VmGO2m7mcnuX8Q"
Content-Disposition: inline
In-Reply-To: 
 <CAKNkYnxhzHLE0Vf_eX6SdR-Mskx3q6brREwqm-CN1dsVDH9Auw@mail.gmail.com>
User-Agent: Mutt/1.5.21 (2010-09-15)
Precedence: bulk
Sender: pgsql-hackers-owner@postgresql.org


--s7VmGO2m7mcnuX8Q
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

Artur,

* Artur Zakirov (a.zakirov@postgrespro.ru) wrote:
> 2016-11-19 21:28 GMT+03:00 Michael Paquier <michael.paquier@gmail.com>:
> > On Thu, Nov 17, 2016 at 1:17 PM, Alvaro Herrera
> > <alvherre@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
>=20
> 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 =3D 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 =3D 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

--s7VmGO2m7mcnuX8Q
Content-Type: application/pgp-signature; name="signature.asc"
Content-Description: Digital signature

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1

iQIcBAEBCgAGBQJYWr0JAAoJEO1sijiDR2RVx40QAI423XgviZTl9u9dfKOpL4Dh
+HQ3r4IsJqeFLG8l/FCv29ut9Cxhmq76CZOC0lG2onsYWKqgp/PV9DzjFDgU2mNi
9WD7g4UpHjaDXqwAAjcFSKIv3gYGhjMH1pbqtPc96+BgWC2ChtNT7sjMqpQXTNmW
SbNNcL2dHkeGRu+YfKH4cjAocEU9nD98VmVtEgCKVzTbYFFlLzsFt+XLwjndUCBI
zGKF+xXSAzBmCv61JcYWOfGOTV2bPjjF2ayoAcRceMH/6fs51FPlz5Q6DQGI7Nfb
aC7FfKL3yz7ZLkhG8JNMHC2B7TxbNYWaarhV4+ytsYJQ5+6/RuYg9MWKkufxZy7C
v1zobkWwKAKa53izcx89+yuJPf8eOHn9x+eBOV6HIOsR45fdIZGMh5ECEZMS8bCv
dQ1uz44M0jwVIiIrI90oadrd0uaxoM6uZmq2TeSBrS9/3WSFzgYVXdOxjDA5wKeD
6RM01+s1UJrUi+Ck54/J9fL7gEbXN52fMiYcMdm2OKADpIg3OP5RwSaZrJxGn/XC
sAfP6icf1UqMOHZOcmrd8G7aoowkbeiQN09Sr7CWNSBRfH96Ocs1jj28MCsvme17
MjM3FmYcjkmpliGhMVb6tQ6JlLiFCVV6nluS3AsAseml0WbM7DcUCSXrxRYinRqj
91PAB35YBsM9dg4g12Ng
=9BYB
-----END PGP SIGNATURE-----

--s7VmGO2m7mcnuX8Q--