Hello. Here is the patch set. At Wed, 23 May 2018 11:20:24 -0400, Tom Lane <t...@sss.pgh.pa.us> wrote in <4979.1527088...@sss.pgh.pa.us> > After thinking about this some more, I'd like to propose that we change > the English output to be "column COLNAME of <relation>", using code > similar to what you suggested for O_POLICY etc. I know that I've been > momentarily confused more than once by looking at obj_description output > and thinking "what, the whole relation depends on this? ... oh, no, it's > just the one column". It would be better if the head-word were "column". > If that leads to better translations in other languages, fine, but in > any case this'd be an improvement for English. > > > I'll clean-up the two thinkgs and post the result later. > > OK, I'll await your patch before doing more here.
Constraints also have namespace but I understand it is just a decoration so I left it alone. 1. Remove tranlation obstracles. 2. Show qualified names for all possible object types. 3. Changes "relation R column C" to "column C of relation R". Extended statistics's name qualification is not excercised in the current regression test. I found that the last one you sugeested makes error messages far cleaner, easy to grasp the meaning at a glance. regards. -- Kyotaro Horiguchi NTT Open Source Software Center
>From cdb956ac2da784f37b110f61da8915819d3ff107 Mon Sep 17 00:00:00 2001 From: Kyotaro Horiguchi <horiguchi.kyot...@lab.ntt.co.jp> Date: Thu, 24 May 2018 19:23:40 +0900 Subject: [PATCH 1/3] Make object names more translatable getObjectDescription() is making description for some object classes in a fashon that is hard-to-translate into some languages. This patch changes it easier to translate. This changes "default" for "default value" along with that. --- src/backend/catalog/objectaddress.c | 74 ++++++++++++++++++++++++++-------- src/test/regress/expected/sequence.out | 4 +- 2 files changed, 59 insertions(+), 19 deletions(-) diff --git a/src/backend/catalog/objectaddress.c b/src/backend/catalog/objectaddress.c index d371c47842..3b27ed7cf4 100644 --- a/src/backend/catalog/objectaddress.c +++ b/src/backend/catalog/objectaddress.c @@ -2829,7 +2829,7 @@ getObjectDescription(const ObjectAddress *object) colobject.objectId = attrdef->adrelid; colobject.objectSubId = attrdef->adnum; - appendStringInfo(&buffer, _("default for %s"), + appendStringInfo(&buffer, _("default value for %s"), getObjectDescription(&colobject)); systable_endscan(adscan); @@ -3016,6 +3016,7 @@ getObjectDescription(const ObjectAddress *object) SysScanDesc rcscan; HeapTuple tup; Form_pg_rewrite rule; + StringInfoData reldesc; ruleDesc = heap_open(RewriteRelationId, AccessShareLock); @@ -3035,9 +3036,17 @@ getObjectDescription(const ObjectAddress *object) rule = (Form_pg_rewrite) GETSTRUCT(tup); - appendStringInfo(&buffer, _("rule %s on "), - NameStr(rule->rulename)); - getRelationDescription(&buffer, rule->ev_class); + /* + * You might think this could be written simpler, but we + * should give a chance to reorder the words in translation. + */ + initStringInfo(&reldesc); + getRelationDescription(&reldesc, rule->ev_class); + + appendStringInfo(&buffer, _("rule %s on %s"), + NameStr(rule->rulename), + reldesc.data); + pfree(reldesc.data); systable_endscan(rcscan); heap_close(ruleDesc, AccessShareLock); @@ -3051,6 +3060,7 @@ getObjectDescription(const ObjectAddress *object) SysScanDesc tgscan; HeapTuple tup; Form_pg_trigger trig; + StringInfoData reldesc; trigDesc = heap_open(TriggerRelationId, AccessShareLock); @@ -3070,9 +3080,17 @@ getObjectDescription(const ObjectAddress *object) trig = (Form_pg_trigger) GETSTRUCT(tup); - appendStringInfo(&buffer, _("trigger %s on "), - NameStr(trig->tgname)); - getRelationDescription(&buffer, trig->tgrelid); + /* + * You might think this could be written simpler, but we + * should give a chance to reorder the words in translation. + */ + initStringInfo(&reldesc); + getRelationDescription(&reldesc, trig->tgrelid); + + appendStringInfo(&buffer, _("trigger %s on %s"), + NameStr(trig->tgname), + reldesc.data); + pfree(reldesc.data); systable_endscan(tgscan); heap_close(trigDesc, AccessShareLock); @@ -3256,6 +3274,8 @@ getObjectDescription(const ObjectAddress *object) SysScanDesc rcscan; HeapTuple tup; Form_pg_default_acl defacl; + StringInfo acldesc = &buffer; + StringInfoData tmpbuf; defaclrel = heap_open(DefaultAclRelationId, AccessShareLock); @@ -3275,36 +3295,43 @@ getObjectDescription(const ObjectAddress *object) defacl = (Form_pg_default_acl) GETSTRUCT(tup); + if (OidIsValid(defacl->defaclnamespace)) + { + /* We combine acldesc with schema name later */ + acldesc = &tmpbuf; + initStringInfo(acldesc); + } + switch (defacl->defaclobjtype) { case DEFACLOBJ_RELATION: - appendStringInfo(&buffer, + appendStringInfo(acldesc, _("default privileges on new relations belonging to role %s"), GetUserNameFromId(defacl->defaclrole, false)); break; case DEFACLOBJ_SEQUENCE: - appendStringInfo(&buffer, + appendStringInfo(acldesc, _("default privileges on new sequences belonging to role %s"), GetUserNameFromId(defacl->defaclrole, false)); break; case DEFACLOBJ_FUNCTION: - appendStringInfo(&buffer, + appendStringInfo(acldesc, _("default privileges on new functions belonging to role %s"), GetUserNameFromId(defacl->defaclrole, false)); break; case DEFACLOBJ_TYPE: - appendStringInfo(&buffer, + appendStringInfo(acldesc, _("default privileges on new types belonging to role %s"), GetUserNameFromId(defacl->defaclrole, false)); break; case DEFACLOBJ_NAMESPACE: - appendStringInfo(&buffer, + appendStringInfo(acldesc, _("default privileges on new schemas belonging to role %s"), GetUserNameFromId(defacl->defaclrole, false)); break; default: /* shouldn't get here */ - appendStringInfo(&buffer, + appendStringInfo(acldesc, _("default privileges belonging to role %s"), GetUserNameFromId(defacl->defaclrole, false)); break; @@ -3312,9 +3339,17 @@ getObjectDescription(const ObjectAddress *object) if (OidIsValid(defacl->defaclnamespace)) { + /* + * You might think this could be written simpler, but we + * should give a chance to reorder the words in + * translation. + */ + Assert(&buffer != acldesc); appendStringInfo(&buffer, - _(" in schema %s"), + _("%s in schema %s"), + acldesc->data, get_namespace_name(defacl->defaclnamespace)); + pfree(acldesc->data); } systable_endscan(rcscan); @@ -3356,6 +3391,7 @@ getObjectDescription(const ObjectAddress *object) SysScanDesc sscan; HeapTuple tuple; Form_pg_policy form_policy; + StringInfoData reldesc; policy_rel = heap_open(PolicyRelationId, AccessShareLock); @@ -3375,9 +3411,13 @@ getObjectDescription(const ObjectAddress *object) form_policy = (Form_pg_policy) GETSTRUCT(tuple); - appendStringInfo(&buffer, _("policy %s on "), - NameStr(form_policy->polname)); - getRelationDescription(&buffer, form_policy->polrelid); + initStringInfo(&reldesc); + getRelationDescription(&reldesc, form_policy->polrelid); + + appendStringInfo(&buffer, _("policy %s on %s"), + NameStr(form_policy->polname), + reldesc.data); + pfree(reldesc.data); systable_endscan(sscan); heap_close(policy_rel, AccessShareLock); diff --git a/src/test/regress/expected/sequence.out b/src/test/regress/expected/sequence.out index ca5ea063fa..f2e20a7e85 100644 --- a/src/test/regress/expected/sequence.out +++ b/src/test/regress/expected/sequence.out @@ -293,11 +293,11 @@ CREATE TEMP TABLE t1 ( -- Both drops should fail, but with different error messages: DROP SEQUENCE t1_f1_seq; ERROR: cannot drop sequence t1_f1_seq because other objects depend on it -DETAIL: default for table t1 column f1 depends on sequence t1_f1_seq +DETAIL: default value for table t1 column f1 depends on sequence t1_f1_seq HINT: Use DROP ... CASCADE to drop the dependent objects too. DROP SEQUENCE myseq2; ERROR: cannot drop sequence myseq2 because other objects depend on it -DETAIL: default for table t1 column f2 depends on sequence myseq2 +DETAIL: default value for table t1 column f2 depends on sequence myseq2 HINT: Use DROP ... CASCADE to drop the dependent objects too. -- This however will work: DROP SEQUENCE myseq3; -- 2.16.3
>From 2a57b2edbd814ca2609ec9fb1efd99d7f626c694 Mon Sep 17 00:00:00 2001 From: Kyotaro Horiguchi <horiguchi.kyot...@lab.ntt.co.jp> Date: Thu, 24 May 2018 21:06:04 +0900 Subject: [PATCH 2/3] Qualify name of all objects that can define namespace Name of some kinds of object were missing qualification in getObjectDescription. This patch make them properly qualified. --- src/backend/catalog/objectaddress.c | 90 ++++++++++++++++++++++++++++--- src/test/regress/expected/alter_table.out | 10 ++-- 2 files changed, 88 insertions(+), 12 deletions(-) diff --git a/src/backend/catalog/objectaddress.c b/src/backend/catalog/objectaddress.c index 3b27ed7cf4..98850df39c 100644 --- a/src/backend/catalog/objectaddress.c +++ b/src/backend/catalog/objectaddress.c @@ -2738,6 +2738,7 @@ getObjectDescription(const ObjectAddress *object) { HeapTuple collTup; Form_pg_collation coll; + char *nspname; collTup = SearchSysCache1(COLLOID, ObjectIdGetDatum(object->objectId)); @@ -2745,8 +2746,16 @@ getObjectDescription(const ObjectAddress *object) elog(ERROR, "cache lookup failed for collation %u", object->objectId); coll = (Form_pg_collation) GETSTRUCT(collTup); + + /* Qualify the name if not visible in search path */ + if (CollationIsVisible(object->objectId)) + nspname = NULL; + else + nspname = get_namespace_name(coll->collnamespace); + appendStringInfo(&buffer, _("collation %s"), - NameStr(coll->collname)); + quote_qualified_identifier(nspname, + NameStr(coll->collname))); ReleaseSysCache(collTup); break; } @@ -2786,14 +2795,25 @@ getObjectDescription(const ObjectAddress *object) case OCLASS_CONVERSION: { HeapTuple conTup; + Form_pg_conversion conv; + char *nspname; conTup = SearchSysCache1(CONVOID, ObjectIdGetDatum(object->objectId)); if (!HeapTupleIsValid(conTup)) elog(ERROR, "cache lookup failed for conversion %u", object->objectId); + conv = (Form_pg_conversion) GETSTRUCT(conTup); + + /* Qualify the name if not visible in search path */ + if (ConversionIsVisible(object->objectId)) + nspname = NULL; + else + nspname = get_namespace_name(conv->connamespace); + appendStringInfo(&buffer, _("conversion %s"), - NameStr(((Form_pg_conversion) GETSTRUCT(conTup))->conname)); + quote_qualified_identifier(nspname, + NameStr(conv->conname))); ReleaseSysCache(conTup); break; } @@ -3113,6 +3133,7 @@ getObjectDescription(const ObjectAddress *object) { HeapTuple stxTup; Form_pg_statistic_ext stxForm; + char *nspname; stxTup = SearchSysCache1(STATEXTOID, ObjectIdGetDatum(object->objectId)); @@ -3122,8 +3143,15 @@ getObjectDescription(const ObjectAddress *object) stxForm = (Form_pg_statistic_ext) GETSTRUCT(stxTup); + /* Qualify the name if not visible in search path */ + if (StatisticsObjIsVisible(object->objectId)) + nspname = NULL; + else + nspname = get_namespace_name(stxForm->stxnamespace); + appendStringInfo(&buffer, _("statistics object %s"), - NameStr(stxForm->stxname)); + quote_qualified_identifier(nspname, + NameStr(stxForm->stxname))); ReleaseSysCache(stxTup); break; @@ -3132,14 +3160,26 @@ getObjectDescription(const ObjectAddress *object) case OCLASS_TSPARSER: { HeapTuple tup; + Form_pg_ts_parser prsForm; + char *nspname; tup = SearchSysCache1(TSPARSEROID, ObjectIdGetDatum(object->objectId)); if (!HeapTupleIsValid(tup)) elog(ERROR, "cache lookup failed for text search parser %u", object->objectId); + + prsForm = (Form_pg_ts_parser) GETSTRUCT(tup); + + /* Qualify the name if not visible in search path */ + if (TSParserIsVisible(object->objectId)) + nspname = NULL; + else + nspname = get_namespace_name(prsForm->prsnamespace); + appendStringInfo(&buffer, _("text search parser %s"), - NameStr(((Form_pg_ts_parser) GETSTRUCT(tup))->prsname)); + quote_qualified_identifier(nspname, + NameStr(prsForm->prsname))); ReleaseSysCache(tup); break; } @@ -3147,14 +3187,26 @@ getObjectDescription(const ObjectAddress *object) case OCLASS_TSDICT: { HeapTuple tup; + Form_pg_ts_dict dictForm; + char *nspname; tup = SearchSysCache1(TSDICTOID, ObjectIdGetDatum(object->objectId)); if (!HeapTupleIsValid(tup)) elog(ERROR, "cache lookup failed for text search dictionary %u", object->objectId); + + dictForm = (Form_pg_ts_dict) GETSTRUCT(tup); + + /* Qualify the name if not visible in search path */ + if (TSDictionaryIsVisible(object->objectId)) + nspname = NULL; + else + nspname = get_namespace_name(dictForm->dictnamespace); + appendStringInfo(&buffer, _("text search dictionary %s"), - NameStr(((Form_pg_ts_dict) GETSTRUCT(tup))->dictname)); + quote_qualified_identifier(nspname, + NameStr(dictForm->dictname))); ReleaseSysCache(tup); break; } @@ -3162,14 +3214,26 @@ getObjectDescription(const ObjectAddress *object) case OCLASS_TSTEMPLATE: { HeapTuple tup; + Form_pg_ts_template tmplForm; + char *nspname; tup = SearchSysCache1(TSTEMPLATEOID, ObjectIdGetDatum(object->objectId)); if (!HeapTupleIsValid(tup)) elog(ERROR, "cache lookup failed for text search template %u", object->objectId); + + tmplForm = (Form_pg_ts_template) GETSTRUCT(tup); + + /* Qualify the name if not visible in search path */ + if (TSTemplateIsVisible(object->objectId)) + nspname = NULL; + else + nspname = get_namespace_name(tmplForm->tmplnamespace); + appendStringInfo(&buffer, _("text search template %s"), - NameStr(((Form_pg_ts_template) GETSTRUCT(tup))->tmplname)); + quote_qualified_identifier(nspname, + NameStr(tmplForm->tmplname))); ReleaseSysCache(tup); break; } @@ -3177,14 +3241,26 @@ getObjectDescription(const ObjectAddress *object) case OCLASS_TSCONFIG: { HeapTuple tup; + Form_pg_ts_config cfgForm; + char *nspname; tup = SearchSysCache1(TSCONFIGOID, ObjectIdGetDatum(object->objectId)); if (!HeapTupleIsValid(tup)) elog(ERROR, "cache lookup failed for text search configuration %u", object->objectId); + + cfgForm = (Form_pg_ts_config) GETSTRUCT(tup); + + /* Qualify the name if not visible in search path */ + if (TSConfigIsVisible(object->objectId)) + nspname = NULL; + else + nspname = get_namespace_name(cfgForm->cfgnamespace); + appendStringInfo(&buffer, _("text search configuration %s"), - NameStr(((Form_pg_ts_config) GETSTRUCT(tup))->cfgname)); + quote_qualified_identifier(nspname, + NameStr(cfgForm->cfgname))); ReleaseSysCache(tup); break; } diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out index 50b9443e2d..376194c48a 100644 --- a/src/test/regress/expected/alter_table.out +++ b/src/test/regress/expected/alter_table.out @@ -2680,11 +2680,11 @@ drop cascades to operator family alter2.ctype_hash_ops for access method hash drop cascades to type alter2.ctype drop cascades to function alter2.same(alter2.ctype,alter2.ctype) drop cascades to operator alter2.=(alter2.ctype,alter2.ctype) -drop cascades to conversion ascii_to_utf8 -drop cascades to text search parser prs -drop cascades to text search configuration cfg -drop cascades to text search template tmpl -drop cascades to text search dictionary dict +drop cascades to conversion alter2.ascii_to_utf8 +drop cascades to text search parser alter2.prs +drop cascades to text search configuration alter2.cfg +drop cascades to text search template alter2.tmpl +drop cascades to text search dictionary alter2.dict -- -- composite types -- -- 2.16.3
>From 8eda8d29e1fdfeaed9fde6f12967416aecfb6449 Mon Sep 17 00:00:00 2001 From: Kyotaro Horiguchi <horiguchi.kyot...@lab.ntt.co.jp> Date: Thu, 24 May 2018 21:35:10 +0900 Subject: [PATCH 3/3] Change object description of columns Previously columns were descripted as "relation R column C", but it sometimes confusing that it reads as a relation R, not a column C. This patch transposes the description as "column C of relation R". --- src/backend/catalog/objectaddress.c | 16 +++++++++++++--- src/test/regress/expected/alter_table.out | 6 +++--- src/test/regress/expected/collate.out | 2 +- src/test/regress/expected/domain.out | 8 ++++---- src/test/regress/expected/sequence.out | 4 ++-- src/test/regress/expected/triggers.out | 8 ++++---- 6 files changed, 27 insertions(+), 17 deletions(-) diff --git a/src/backend/catalog/objectaddress.c b/src/backend/catalog/objectaddress.c index 98850df39c..61fe283d74 100644 --- a/src/backend/catalog/objectaddress.c +++ b/src/backend/catalog/objectaddress.c @@ -2681,12 +2681,22 @@ getObjectDescription(const ObjectAddress *object) switch (getObjectClass(object)) { case OCLASS_CLASS: - getRelationDescription(&buffer, object->objectId); if (object->objectSubId != 0) - appendStringInfo(&buffer, _(" column %s"), + { + StringInfoData reldesc; + + initStringInfo(&reldesc); + getRelationDescription(&reldesc, object->objectId); + appendStringInfo(&buffer, _("column %s of %s"), get_attname(object->objectId, object->objectSubId, - false)); + false), + reldesc.data); + pfree(reldesc.data); + } + else + getRelationDescription(&buffer, object->objectId); + break; case OCLASS_PROC: diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out index 376194c48a..702bf9fe98 100644 --- a/src/test/regress/expected/alter_table.out +++ b/src/test/regress/expected/alter_table.out @@ -1844,7 +1844,7 @@ select * from foo; (1 row) drop domain mytype cascade; -NOTICE: drop cascades to table foo column f2 +NOTICE: drop cascades to column f2 of table foo select * from foo; f1 | f3 ----+---- @@ -2870,8 +2870,8 @@ DROP TABLE test_tbl2_subclass; CREATE TYPE test_typex AS (a int, b text); CREATE TABLE test_tblx (x int, y test_typex check ((y).a > 0)); ALTER TYPE test_typex DROP ATTRIBUTE a; -- fails -ERROR: cannot drop composite type test_typex column a because other objects depend on it -DETAIL: constraint test_tblx_y_check on table test_tblx depends on composite type test_typex column a +ERROR: cannot drop column a of composite type test_typex because other objects depend on it +DETAIL: constraint test_tblx_y_check on table test_tblx depends on column a of composite type test_typex HINT: Use DROP ... CASCADE to drop the dependent objects too. ALTER TYPE test_typex DROP ATTRIBUTE a CASCADE; NOTICE: drop cascades to constraint test_tblx_y_check on table test_tblx diff --git a/src/test/regress/expected/collate.out b/src/test/regress/expected/collate.out index f045f2b291..fcbe3a5cc8 100644 --- a/src/test/regress/expected/collate.out +++ b/src/test/regress/expected/collate.out @@ -631,7 +631,7 @@ DROP COLLATION mycoll1; CREATE TABLE collate_test23 (f1 text collate mycoll2); DROP COLLATION mycoll2; -- fail ERROR: cannot drop collation mycoll2 because other objects depend on it -DETAIL: table collate_test23 column f1 depends on collation mycoll2 +DETAIL: column f1 of table collate_test23 depends on collation mycoll2 HINT: Use DROP ... CASCADE to drop the dependent objects too. -- invalid: non-lowercase quoted identifiers CREATE COLLATION case_coll ("Lc_Collate" = "POSIX", "Lc_Ctype" = "POSIX"); diff --git a/src/test/regress/expected/domain.out b/src/test/regress/expected/domain.out index f4eebb75cf..0b5a9041b0 100644 --- a/src/test/regress/expected/domain.out +++ b/src/test/regress/expected/domain.out @@ -298,8 +298,8 @@ ERROR: operator does not exist: character varying > double precision HINT: No operator matches the given name and argument types. You might need to add explicit type casts. alter type comptype alter attribute r type bigint; alter type comptype drop attribute r; -- fail -ERROR: cannot drop composite type comptype column r because other objects depend on it -DETAIL: constraint c1 depends on composite type comptype column r +ERROR: cannot drop column r of composite type comptype because other objects depend on it +DETAIL: constraint c1 depends on column r of composite type comptype HINT: Use DROP ... CASCADE to drop the dependent objects too. alter type comptype drop attribute i; select conname, obj_description(oid, 'pg_constraint') from pg_constraint @@ -645,8 +645,8 @@ alter domain dnotnulltest drop not null; update domnotnull set col1 = null; drop domain dnotnulltest cascade; NOTICE: drop cascades to 2 other objects -DETAIL: drop cascades to table domnotnull column col1 -drop cascades to table domnotnull column col2 +DETAIL: drop cascades to column col1 of table domnotnull +drop cascades to column col2 of table domnotnull -- Test ALTER DOMAIN .. DEFAULT .. create table domdeftest (col1 ddef1); insert into domdeftest default values; diff --git a/src/test/regress/expected/sequence.out b/src/test/regress/expected/sequence.out index f2e20a7e85..a0d2b22d3c 100644 --- a/src/test/regress/expected/sequence.out +++ b/src/test/regress/expected/sequence.out @@ -293,11 +293,11 @@ CREATE TEMP TABLE t1 ( -- Both drops should fail, but with different error messages: DROP SEQUENCE t1_f1_seq; ERROR: cannot drop sequence t1_f1_seq because other objects depend on it -DETAIL: default value for table t1 column f1 depends on sequence t1_f1_seq +DETAIL: default value for column f1 of table t1 depends on sequence t1_f1_seq HINT: Use DROP ... CASCADE to drop the dependent objects too. DROP SEQUENCE myseq2; ERROR: cannot drop sequence myseq2 because other objects depend on it -DETAIL: default value for table t1 column f2 depends on sequence myseq2 +DETAIL: default value for column f2 of table t1 depends on sequence myseq2 HINT: Use DROP ... CASCADE to drop the dependent objects too. -- This however will work: DROP SEQUENCE myseq3; diff --git a/src/test/regress/expected/triggers.out b/src/test/regress/expected/triggers.out index 1f8caef2d7..bf271d536e 100644 --- a/src/test/regress/expected/triggers.out +++ b/src/test/regress/expected/triggers.out @@ -557,10 +557,10 @@ LINE 2: FOR EACH STATEMENT WHEN (OLD.* IS DISTINCT FROM NEW.*) ^ -- check dependency restrictions ALTER TABLE main_table DROP COLUMN b; -ERROR: cannot drop table main_table column b because other objects depend on it -DETAIL: trigger after_upd_b_row_trig on table main_table depends on table main_table column b -trigger after_upd_a_b_row_trig on table main_table depends on table main_table column b -trigger after_upd_b_stmt_trig on table main_table depends on table main_table column b +ERROR: cannot drop column b of table main_table because other objects depend on it +DETAIL: trigger after_upd_b_row_trig on table main_table depends on column b of table main_table +trigger after_upd_a_b_row_trig on table main_table depends on column b of table main_table +trigger after_upd_b_stmt_trig on table main_table depends on column b of table main_table HINT: Use DROP ... CASCADE to drop the dependent objects too. -- this should succeed, but we'll roll it back to keep the triggers around begin; -- 2.16.3