Re: Flexible configuration for full-text search

Hi, Aleksandr!

On Mon, Jul 9, 2018 at 10:26 AM Aleksandr Parfenov <
a(dot)parfenov(at)postgrespro(dot)ru> wrote:

> A new version of the patch in the attachment. There are no changes since
> the last version except refreshing it to current HEAD.
>

I took a look at this patch. It applied cleanly, but didn't pass
regression tests.

***
/Users/smagen/projects/postgresql/env/master/src/src/test/regress/expected/misc_sanity.out
2018-07-20
13:44:54.000000000 +0300
---
/Users/smagen/projects/postgresql/env/master/src/src/test/regress/results/misc_sanity.out
2018-07-20
13:47:00.000000000 +0300
***************
*** 105,109 ****
pg_index | indpred | pg_node_tree
pg_largeobject | data | bytea
pg_largeobject_metadata | lomacl | aclitem[]
! (11 rows)

--- 105,110 ----
pg_index | indpred | pg_node_tree
pg_largeobject | data | bytea
pg_largeobject_metadata | lomacl | aclitem[]
! pg_ts_config_map | mapdicts | jsonb
! (12 rows)

It seems to be related to recent patches which adds toast tables to
majority of system tables with varlena column. Regression diff indicates
that mapdicts field of pg_ts_config_map can't be toasted. I think we
should add toast table to pg_ts_config_map table.

Also, I see that you add extra grammar rules for ALTER TEXT SEARCH
CONFIGURATION to the documentation, which allows specifying config instead
of dictionary_name.

+ALTER TEXT SEARCH CONFIGURATION <replaceable>name</replaceable>
+ ADD MAPPING FOR <replaceable
class="parameter">token_type</replaceable> [, ... ] WITH <replaceable
class="parameter">config</replaceable>
ALTER TEXT SEARCH CONFIGURATION <replaceable>name</replaceable>
ADD MAPPING FOR <replaceable
class="parameter">token_type</replaceable> [, ... ] WITH <replaceable
class="parameter">dictionary_name</replaceable> [, ... ]
+ALTER TEXT SEARCH CONFIGURATION <replaceable>name</replaceable>
+ ALTER MAPPING FOR <replaceable
class="parameter">token_type</replaceable> [, ... ] WITH <replaceable
class="parameter">config</replaceable>
ALTER TEXT SEARCH CONFIGURATION <replaceable>name</replaceable>
ALTER MAPPING FOR <replaceable
class="parameter">token_type</replaceable> [, ... ] WITH <replaceable
class="parameter">dictionary_name</replaceable> [, ... ]
ALTER TEXT SEARCH CONFIGURATION <replaceable>name</replaceable>

In the same time you declare config as following.

+ <para>
+ Formally <replaceable class="parameter">config</replaceable> is one of:
+ </para>
+ <programlisting>
+ * dictionary_name
+
+ * config { UNION | INTERSECT | EXCEPT | MAP } config
+
+ * CASE config
+ WHEN [ NO ] MATCH THEN { KEEP | config }
+ [ ELSE config ]
+ END
+ </programlisting>

That is config itself could be a dictionary_name. I think this makes
grammar rules for ALTER TEXT SEARCH CONFIGURATION redundant. We can
specify those rules to always expect config, assuming that it can be
actually a dictionary nay.

+ if (fout->remoteVersion >= 110000)

PostgreSQL 11 already passed feature freeze. Thus, we should be aimed to
PostgreSQL 12.

That's all for now, but I'm going to do more detailed code review.

------
Alexander Korotkov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company