Thread: Cosmetic change in catalog/index.c
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
NotDashEscaped: You need GnuPG to verify this message
Very minor cosmetic patch to catalog/index.c: I noticed when replying to
bug #895 that the error message "relation already exists" is not
capitilized as the rest are:
ERROR: relation named "pk_exchange_batch_idx" already exists
This patch fixes that and a few other places where the capitilization
is needed.
--
Greg Sabino Mullane greg@turnstep.com
PGP Key: 0x14964AC8 200302131005
Index: index.c
===================================================================
RCS file: /projects/cvsroot/pgsql-server/src/backend/catalog/index.c,v
retrieving revision 1.208
diff -c -r1.208 index.c
*** index.c 2002/12/15 16:17:38 1.208
--- index.c 2003/02/13 15:04:27
***************
*** 235,241 ****
* here we are indexing on a normal attribute (1...n)
*/
if (atnum > natts)
! elog(ERROR, "cannot create index: column %d does not exist",
atnum);
from = heapTupDesc->attrs[AttrNumberGetAttrOffset(atnum)];
--- 235,241 ----
* here we are indexing on a normal attribute (1...n)
*/
if (atnum > natts)
! elog(ERROR, "Cannot create index: column %d does not exist",
atnum);
from = heapTupDesc->attrs[AttrNumberGetAttrOffset(atnum)];
***************
*** 540,546 ****
*/
if (indexInfo->ii_NumIndexAttrs < 1 ||
indexInfo->ii_NumKeyAttrs < 1)
! elog(ERROR, "must index at least one column");
if (!allow_system_table_mods &&
IsSystemRelation(heapRelation) &&
--- 540,546 ----
*/
if (indexInfo->ii_NumIndexAttrs < 1 ||
indexInfo->ii_NumKeyAttrs < 1)
! elog(ERROR, "Must index at least one column");
if (!allow_system_table_mods &&
IsSystemRelation(heapRelation) &&
***************
*** 558,564 ****
elog(ERROR, "Shared indexes cannot be created after initdb");
if (get_relname_relid(indexRelationName, namespaceId))
! elog(ERROR, "relation named \"%s\" already exists",
indexRelationName);
/*
--- 558,564 ----
elog(ERROR, "Shared indexes cannot be created after initdb");
if (get_relname_relid(indexRelationName, namespaceId))
! elog(ERROR, "Relation named \"%s\" already exists",
indexRelationName);
/*
***************
*** 671,677 ****
constraintType = CONSTRAINT_UNIQUE;
else
{
! elog(ERROR, "index_create: constraint must be PRIMARY or UNIQUE");
constraintType = 0; /* keep compiler quiet */
}
--- 671,677 ----
constraintType = CONSTRAINT_UNIQUE;
else
{
! elog(ERROR, "Index_create: constraint must be PRIMARY or UNIQUE");
constraintType = 0; /* keep compiler quiet */
}
***************
*** 1036,1042 ****
if (heaprel->rd_rel->relkind != RELKIND_RELATION &&
heaprel->rd_rel->relkind != RELKIND_TOASTVALUE)
! elog(ERROR, "relation %s isn't an indexable relation",
RelationGetRelationName(heaprel));
/* If pg_class.relhasindex is set, indexes are active */
--- 1036,1042 ----
if (heaprel->rd_rel->relkind != RELKIND_RELATION &&
heaprel->rd_rel->relkind != RELKIND_TOASTVALUE)
! elog(ERROR, "Relation %s isn't an indexable relation",
RelationGetRelationName(heaprel));
/* If pg_class.relhasindex is set, indexes are active */
***************
*** 1796,1808 ****
if (iRel->rd_rel->relisshared)
{
if (!IsIgnoringSystemIndexes())
! elog(ERROR, "the target relation %u is shared", indexId);
inplace = true;
}
if (iRel->rd_isnailed)
{
if (!IsIgnoringSystemIndexes())
! elog(ERROR, "the target relation %u is nailed", indexId);
inplace = true;
}
--- 1796,1808 ----
if (iRel->rd_rel->relisshared)
{
if (!IsIgnoringSystemIndexes())
! elog(ERROR, "The target relation %u is shared", indexId);
inplace = true;
}
if (iRel->rd_isnailed)
{
if (!IsIgnoringSystemIndexes())
! elog(ERROR, "The target relation %u is nailed", indexId);
inplace = true;
}
***************
*** 1919,1925 ****
deactivate_needed = true;
}
else
! elog(ERROR, "the target relation %u is shared", relid);
}
old = SetReindexProcessing(true);
--- 1919,1925 ----
deactivate_needed = true;
}
else
! elog(ERROR, "The target relation %u is shared", relid);
}
old = SetReindexProcessing(true);
-----BEGIN PGP SIGNATURE-----
Comment: http://www.turnstep.com/pgp.html
iD8DBQE+S7SDvJuQZxSWSsgRAmZFAKDcJbSmMbfWXMmekB/xopQ9Qtj8mACg19FB
+hZkHs8APOZsWA5geYlTDD8=
=ZIrU
-----END PGP SIGNATURE-----
On Thu, 2003-02-13 at 10:09, Greg Sabino Mullane wrote: > Very minor cosmetic patch to catalog/index.c: I noticed when replying to > bug #895 that the error message "relation already exists" is not > capitilized as the rest are: On the contrary, I can't see any kind of consistency in the leading capitalization in elog() messages. If you're only going to make a few arbitrary changes but not make things consistent, it doesn't seem to be worth the risk of breaking client apps that examine error message strings. > This patch fixes that and a few other places where the capitilization > is needed. Shouldn't you update translations as well? Cheers, Neil -- Neil Conway <neilc@samurai.com> || PGP Key ID: DB3C29FC
Neil Conway <neilc@samurai.com> writes:
> On the contrary, I can't see any kind of consistency in the leading
> capitalization in elog() messages.
There isn't any :-(. While I'd vote with Greg to migrate towards a
consistent use of initial caps, I seem to recall that Peter was in favor
of no initial caps ... which might help to explain why there's no
consistency in the code now ...
There's not much point in applying piecemeal changes if we don't have
a consensus on what to work toward.
regards, tom lane
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Neil Conway wrote: > On the contrary, I can't see any kind of consistency in the leading > capitalization in elog() messages. I actually only set out to fix one specific example, the lowercase 'r' in: ERROR: relation named "relation_name" already exists when trying to create an index. When trying to create a table, sequence, or view, the message is: ERROR: Relation named "relation_name" already exists so I went with the majority to force a consensus among otherwise identical error messages. However, I felt silly making a one-letter patch, so made some other changes in the file as well. > If you're only going to make a few arbitrary changes but not make things > consistent, it doesn't seem to be worth the risk of breaking client apps > that examine error message strings. Perhaps we need to re-examine error codes? Apps parsing message strings seems dangerous. On the other hand, I would hope that a simple case change would not break too many of them. And in the above example, the same error is already returned two different ways. Tom Lane writes: > [consistency] There isn't any :-(. While I'd vote with Greg to migrate > towards a consistent use of initial caps, I seem to recall that Peter > was in favor of no initial caps ... which might help to explain why > there's no consistency in the code now ... My argument for initial caps is this: the error codes represent complete sentences, and look better capitalized. The only exception should be when an internal lowercase function name begins the error message, such as: elog(ERROR, "index_drop: cache lookup failed for index %u", I am not totally opposed to leading with all lowercase if someone were to present me with a good argument; more important is to pick one way and stick with it to avoid problems like the one noted above. - -- Greg Sabino Mullane greg@turnstep.com PGP Key: 0x14964AC8 200302140930 -----BEGIN PGP SIGNATURE----- Comment: http://www.turnstep.com/pgp.html iD8DBQE+TP4zvJuQZxSWSsgRAsXJAJ9IQh5y74QOsweEVjrE5/uax5kTnACgqZLr 6FPZYZZAt1+cb1uR3QxxNL8= =Toij -----END PGP SIGNATURE-----
greg@turnstep.com writes:
> My argument for initial caps is this: the error codes represent complete
> sentences, and look better capitalized. The only exception should be
> when an internal lowercase function name begins the error message, such
> as:
> elog(ERROR, "index_drop: cache lookup failed for index %u",
I would like to migrate *away* from mentioning internal function names
in the error message text at all. Ideally, information about the
location where the error was reported would be available separately from
the primary message text (I would like elog() to be a macro that can
invoke __FILE__ and __LINE__ behind the scenes). See past discussions
about revising the error message protocol so that multiple independent
fields can be delivered.
regards, tom lane
On Fri, 2003-02-14 at 09:37, greg@turnstep.com wrote: > Perhaps we need to re-examine error codes? Apps parsing message strings > seems dangerous. Yes, error codes would definitely be good. Any takers on implementing them? :-) Cheers, Neil -- Neil Conway <neilc@samurai.com> || PGP Key ID: DB3C29FC
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 > Yes, error codes would definitely be good. Any takers on > implementing them? :-) I plant to take a look at this later this week. No promises, however! - -- Greg Sabino Mullane greg@turnstep.com PGP Key: 0x14964AC8 200302171656 -----BEGIN PGP SIGNATURE----- Comment: http://www.turnstep.com/pgp.html iD8DBQE+UCn7vJuQZxSWSsgRAr58AKCfs4TBrGG+5VuqusbxJd+17dFibgCbBu9b C1rGTNKX8taRfzBjLXvbXB0= =Eyzs -----END PGP SIGNATURE-----