From 8a5592daf1bcc65f002fce23de1ee0e533e6c6a6 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Wed, 22 Aug 2007 05:13:50 +0000 Subject: [PATCH] Remove option to change parser of an existing text search configuration. This prevents needing to do complex and poorly-defined updates of the mapping table if the new parser has different token types than the old. Per discussion. --- doc/src/sgml/ref/alter_tsconfig.sgml | 30 +------ doc/src/sgml/ref/create_tsconfig.sgml | 9 +- src/backend/commands/tsearchcmds.c | 118 +++++--------------------- src/backend/parser/gram.y | 11 +-- src/include/nodes/parsenodes.h | 4 +- 5 files changed, 33 insertions(+), 139 deletions(-) diff --git a/doc/src/sgml/ref/alter_tsconfig.sgml b/doc/src/sgml/ref/alter_tsconfig.sgml index 295ba1df64..e40f34fd61 100644 --- a/doc/src/sgml/ref/alter_tsconfig.sgml +++ b/doc/src/sgml/ref/alter_tsconfig.sgml @@ -1,5 +1,5 @@ @@ -20,9 +20,6 @@ PostgreSQL documentation -ALTER TEXT SEARCH CONFIGURATION name ( - PARSER = parser_name -) ALTER TEXT SEARCH CONFIGURATION name ADD MAPPING FOR token_type [, ... ] WITH dictionary_name [, ... ] ALTER TEXT SEARCH CONFIGURATION name @@ -43,8 +40,8 @@ ALTER TEXT SEARCH CONFIGURATION name OWNER TO ALTER TEXT SEARCH CONFIGURATION changes the definition of - a text search configuration. You can change which parser it uses, modify - its mapping from token types to dictionaries, + a text search configuration. You can modify + its mappings from token types to dictionaries, or change the configuration's name or owner. @@ -68,15 +65,6 @@ ALTER TEXT SEARCH CONFIGURATION name OWNER TO - - parser_name - - - The name of a new text search parser to use for this configuration. - - - - token_type @@ -154,19 +142,7 @@ ALTER TEXT SEARCH CONFIGURATION name OWNER TO - - - Notes - - While changing the text search parser used by a configuration is allowed, - this will only work nicely if old and new parsers use the same set of - token types. It is advisable to drop the mappings for any incompatible - token types before changing parsers. - - - - Examples diff --git a/doc/src/sgml/ref/create_tsconfig.sgml b/doc/src/sgml/ref/create_tsconfig.sgml index 49be9411d5..048cb63ce9 100644 --- a/doc/src/sgml/ref/create_tsconfig.sgml +++ b/doc/src/sgml/ref/create_tsconfig.sgml @@ -1,5 +1,5 @@ @@ -98,10 +98,9 @@ CREATE TEXT SEARCH CONFIGURATION nameNotes - It is allowed to specify both PARSER and COPY, - resulting in the specified parser being used with whatever mappings - are in the source configuration. This is generally inadvisable, - unless you know that both parsers involved use the same token type set. + The PARSER and COPY options are mutually + exclusive, because when an existing configuration is copied, its + parser selection is copied too. diff --git a/src/backend/commands/tsearchcmds.c b/src/backend/commands/tsearchcmds.c index 7c5a1c49a3..a9a31504aa 100644 --- a/src/backend/commands/tsearchcmds.c +++ b/src/backend/commands/tsearchcmds.c @@ -9,7 +9,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/commands/tsearchcmds.c,v 1.3 2007/08/22 01:39:44 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/commands/tsearchcmds.c,v 1.4 2007/08/22 05:13:50 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -46,12 +46,10 @@ #include "utils/syscache.h" -static HeapTuple UpdateTSConfiguration(AlterTSConfigurationStmt *stmt, - HeapTuple tup); static void MakeConfigurationMapping(AlterTSConfigurationStmt *stmt, - HeapTuple tup); + HeapTuple tup, Relation relMap); static void DropConfigurationMapping(AlterTSConfigurationStmt *stmt, - HeapTuple tup); + HeapTuple tup, Relation relMap); /* --------------------- TS Parser commands ------------------------ */ @@ -1265,7 +1263,6 @@ DefineTSConfiguration(List *names, List *parameters) Oid namespaceoid; char *cfgname; NameData cname; - List *sourceName = NIL; Oid sourceOid = InvalidOid; Oid prsOid = InvalidOid; Oid cfgOid; @@ -1290,7 +1287,7 @@ DefineTSConfiguration(List *names, List *parameters) if (pg_strcasecmp(defel->defname, "parser") == 0) prsOid = TSParserGetPrsid(defGetQualifiedName(defel), false); else if (pg_strcasecmp(defel->defname, "copy") == 0) - sourceName = defGetQualifiedName(defel); + sourceOid = TSConfigGetCfgid(defGetQualifiedName(defel), false); else ereport(ERROR, (errcode(ERRCODE_SYNTAX_ERROR), @@ -1298,15 +1295,18 @@ DefineTSConfiguration(List *names, List *parameters) defel->defname))); } + if (OidIsValid(sourceOid) && OidIsValid(prsOid)) + ereport(ERROR, + (errcode(ERRCODE_SYNTAX_ERROR), + errmsg("cannot specify both PARSER and COPY options"))); + /* * Look up source config if given. */ - if (sourceName) + if (OidIsValid(sourceOid)) { Form_pg_ts_config cfg; - sourceOid = TSConfigGetCfgid(sourceName, false); - tup = SearchSysCache(TSCONFIGOID, ObjectIdGetDatum(sourceOid), 0, 0, 0); @@ -1316,9 +1316,8 @@ DefineTSConfiguration(List *names, List *parameters) cfg = (Form_pg_ts_config) GETSTRUCT(tup); - /* Use source's parser if no other was specified */ - if (!OidIsValid(prsOid)) - prsOid = cfg->cfgparser; + /* use source's parser */ + prsOid = cfg->cfgparser; ReleaseSysCache(tup); } @@ -1626,8 +1625,7 @@ void AlterTSConfiguration(AlterTSConfigurationStmt *stmt) { HeapTuple tup; - HeapTuple newtup; - Relation mapRel; + Relation relMap; /* Find the configuration */ tup = GetTSConfigTuple(stmt->cfgname); @@ -1642,84 +1640,22 @@ AlterTSConfiguration(AlterTSConfigurationStmt *stmt) aclcheck_error(ACLCHECK_NOT_OWNER, ACL_KIND_TSCONFIGURATION, NameListToString(stmt->cfgname)); - /* Update fields of config tuple? */ - if (stmt->options) - newtup = UpdateTSConfiguration(stmt, tup); - else - newtup = tup; + relMap = heap_open(TSConfigMapRelationId, RowExclusiveLock); - /* Add or drop mappings? */ + /* Add or drop mappings */ if (stmt->dicts) - MakeConfigurationMapping(stmt, newtup); + MakeConfigurationMapping(stmt, tup, relMap); else if (stmt->tokentype) - DropConfigurationMapping(stmt, newtup); - - /* - * Even if we aren't changing mappings, there could already be some, - * so makeConfigurationDependencies always has to look. - */ - mapRel = heap_open(TSConfigMapRelationId, AccessShareLock); + DropConfigurationMapping(stmt, tup, relMap); /* Update dependencies */ - makeConfigurationDependencies(newtup, true, mapRel); + makeConfigurationDependencies(tup, true, relMap); - heap_close(mapRel, AccessShareLock); + heap_close(relMap, RowExclusiveLock); ReleaseSysCache(tup); } -/* - * ALTER TEXT SEARCH CONFIGURATION - update fields of pg_ts_config tuple - */ -static HeapTuple -UpdateTSConfiguration(AlterTSConfigurationStmt *stmt, HeapTuple tup) -{ - Relation cfgRel; - ListCell *pl; - Datum repl_val[Natts_pg_ts_config]; - char repl_null[Natts_pg_ts_config]; - char repl_repl[Natts_pg_ts_config]; - HeapTuple newtup; - - memset(repl_val, 0, sizeof(repl_val)); - memset(repl_null, ' ', sizeof(repl_null)); - memset(repl_repl, ' ', sizeof(repl_repl)); - - cfgRel = heap_open(TSConfigRelationId, RowExclusiveLock); - - foreach(pl, stmt->options) - { - DefElem *defel = (DefElem *) lfirst(pl); - - if (pg_strcasecmp(defel->defname, "parser") == 0) - { - Oid newPrs; - - newPrs = TSParserGetPrsid(defGetQualifiedName(defel), false); - repl_val[Anum_pg_ts_config_cfgparser - 1] = ObjectIdGetDatum(newPrs); - repl_repl[Anum_pg_ts_config_cfgparser - 1] = 'r'; - } - else - ereport(ERROR, - (errcode(ERRCODE_SYNTAX_ERROR), - errmsg("text search configuration parameter \"%s\" not recognized", - defel->defname))); - } - - newtup = heap_modifytuple(tup, RelationGetDescr(cfgRel), - repl_val, repl_null, repl_repl); - - simple_heap_update(cfgRel, &newtup->t_self, newtup); - - CatalogUpdateIndexes(cfgRel, newtup); - - heap_close(cfgRel, RowExclusiveLock); - - return newtup; -} - -/*------------------- TS Configuration mapping stuff ----------------*/ - /* * Translate a list of token type names to an array of token type numbers */ @@ -1780,10 +1716,10 @@ getTokenTypes(Oid prsId, List *tokennames) * ALTER TEXT SEARCH CONFIGURATION ADD/ALTER MAPPING */ static void -MakeConfigurationMapping(AlterTSConfigurationStmt *stmt, HeapTuple tup) +MakeConfigurationMapping(AlterTSConfigurationStmt *stmt, + HeapTuple tup, Relation relMap) { Oid cfgId = HeapTupleGetOid(tup); - Relation relMap; ScanKeyData skey[2]; SysScanDesc scan; HeapTuple maptup; @@ -1801,8 +1737,6 @@ MakeConfigurationMapping(AlterTSConfigurationStmt *stmt, HeapTuple tup) tokens = getTokenTypes(prsId, stmt->tokentype); ntoken = list_length(stmt->tokentype); - relMap = heap_open(TSConfigMapRelationId, RowExclusiveLock); - if (stmt->override) { /* @@ -1938,18 +1872,16 @@ MakeConfigurationMapping(AlterTSConfigurationStmt *stmt, HeapTuple tup) } } } - - heap_close(relMap, RowExclusiveLock); } /* * ALTER TEXT SEARCH CONFIGURATION DROP MAPPING */ static void -DropConfigurationMapping(AlterTSConfigurationStmt *stmt, HeapTuple tup) +DropConfigurationMapping(AlterTSConfigurationStmt *stmt, + HeapTuple tup, Relation relMap) { Oid cfgId = HeapTupleGetOid(tup); - Relation relMap; ScanKeyData skey[2]; SysScanDesc scan; HeapTuple maptup; @@ -1964,8 +1896,6 @@ DropConfigurationMapping(AlterTSConfigurationStmt *stmt, HeapTuple tup) tokens = getTokenTypes(prsId, stmt->tokentype); ntoken = list_length(stmt->tokentype); - relMap = heap_open(TSConfigMapRelationId, RowExclusiveLock); - i = 0; foreach(c, stmt->tokentype) { @@ -2011,8 +1941,6 @@ DropConfigurationMapping(AlterTSConfigurationStmt *stmt, HeapTuple tup) i++; } - - heap_close(relMap, RowExclusiveLock); } diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y index cf0fd39fd7..38ddf4bd48 100644 --- a/src/backend/parser/gram.y +++ b/src/backend/parser/gram.y @@ -11,7 +11,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/parser/gram.y,v 2.599 2007/08/21 15:13:42 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/parser/gram.y,v 2.600 2007/08/22 05:13:50 tgl Exp $ * * HISTORY * AUTHOR DATE MAJOR EVENT @@ -5523,14 +5523,7 @@ AlterTSDictionaryStmt: ; AlterTSConfigurationStmt: - ALTER TEXT_P SEARCH CONFIGURATION any_name definition - { - AlterTSConfigurationStmt *n = makeNode(AlterTSConfigurationStmt); - n->cfgname = $5; - n->options = $6; - $$ = (Node *)n; - } - | ALTER TEXT_P SEARCH CONFIGURATION any_name ADD_P MAPPING FOR name_list WITH any_name_list + ALTER TEXT_P SEARCH CONFIGURATION any_name ADD_P MAPPING FOR name_list WITH any_name_list { AlterTSConfigurationStmt *n = makeNode(AlterTSConfigurationStmt); n->cfgname = $5; diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h index 7ba810dbb1..79449c5584 100644 --- a/src/include/nodes/parsenodes.h +++ b/src/include/nodes/parsenodes.h @@ -7,7 +7,7 @@ * Portions Copyright (c) 1996-2007, PostgreSQL Global Development Group * Portions Copyright (c) 1994, Regents of the University of California * - * $PostgreSQL: pgsql/src/include/nodes/parsenodes.h,v 1.351 2007/08/21 01:11:28 tgl Exp $ + * $PostgreSQL: pgsql/src/include/nodes/parsenodes.h,v 1.352 2007/08/22 05:13:50 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -2054,10 +2054,8 @@ typedef struct AlterTSConfigurationStmt { NodeTag type; List *cfgname; /* qualified name (list of Value strings) */ - List *options; /* List of DefElem nodes */ /* - * These fields are used for ADD/ALTER/DROP MAPPING variants. * dicts will be non-NIL if ADD/ALTER MAPPING was specified. * If dicts is NIL, but tokentype isn't, DROP MAPPING was specified. */