Different error messages executing CREATE TABLE or ALTER TABLE to create a column "xmin"

Started by Giuseppe Sucamelialmost 14 years ago9 messages

brush.tyler@gmail.com

almost 14 years ago

Hi all,

trying to create a table with a column xmin I get the
following error message:

test=> create table lx (xmin int);
ERROR: column name "xmin" conflicts with a system
column name

Instead I get a different (and less understandable) error
message if I try to add a column named xmin to an
existent table:

test=> create table lx (i int);
CREATE TABLE
test=> alter table lx add xmin int;
ERROR: column "xmin" of relation "lx" already exists.

The same problem occurs using "xmax" as column name.

I'm on Ubuntu 11.04.
Tried on both PostgreSQL 8.4.10 and 9.1.2

Regards.

--
Giuseppe Sucameli

Marc Balmer

marc@msys.ch

almost 14 years ago

In reply to: Giuseppe Sucameli (#1)

Re: Different error messages executing CREATE TABLE or ALTER TABLE to create a column "xmin"

Am 22.01.12 14:22, schrieb Giuseppe Sucameli:

Hi all,

trying to create a table with a column xmin I get the
following error message:

test=> create table lx (xmin int);
ERROR: column name "xmin" conflicts with a system
column name

Instead I get a different (and less understandable) error
message if I try to add a column named xmin to an
existent table:

test=> create table lx (i int);
CREATE TABLE
test=> alter table lx add xmin int;
ERROR: column "xmin" of relation "lx" already exists.

The same problem occurs using "xmax" as column name.

I'm on Ubuntu 11.04.
Tried on both PostgreSQL 8.4.10 and 9.1.2

That is not a bug, but a feature. See section 5.4 of the documentation
"System Columns":

"Every table has several system columns that are implicitly defined by
the system. Therefore, these names cannot be used as names of
user-defined columns. (Note that these restrictions are separate from
whether the name is a key word or not; quoting a name will not allow you
to escape these restrictions.) You do not really need to be concerned
about these columns; just know they exist."

and further down:

"xmin

The identity (transaction ID) of the inserting transaction for this row
version. (A row version is an individual state of a row; each update of
a row creates a new row version for the same logical row.)"

Vik Reykja

vikreykja@gmail.com

almost 14 years ago

In reply to: Marc Balmer (#2)

Re: Different error messages executing CREATE TABLE or ALTER TABLE to create a column "xmin"

On Mon, Jan 23, 2012 at 11:25, Marc Balmer <marc@msys.ch> wrote:

Am 22.01.12 14:22, schrieb Giuseppe Sucameli:

test=> create table lx (xmin int);
ERROR: column name "xmin" conflicts with a system
column name

test=> create table lx (i int);
CREATE TABLE
test=> alter table lx add xmin int;
ERROR: column "xmin" of relation "lx" already exists.

That is not a bug, but a feature.

I see it as a message bug. Why wouldn't ALTER TABLE also tell us that xmin
is a system column? It makes things much more clear for newbies who don't
see the column yet are told it exists if they're also told it's a system
column.

I would try to cook up a patch but I have no skills :-(

Giuseppe Sucameli

brush.tyler@gmail.com

almost 14 years ago

In reply to: Vik Reykja (#3)

Re: Different error messages executing CREATE TABLE or ALTER TABLE to create a column "xmin"

Hi Marc,

On Mon, Jan 23, 2012 at 3:04 PM, Vik Reykja <vikreykja@gmail.com> wrote:

On Mon, Jan 23, 2012 at 11:25, Marc Balmer <marc@msys.ch> wrote:

Am 22.01.12 14:22, schrieb Giuseppe Sucameli:

test=> create table lx (i int);
CREATE TABLE
test=> alter table lx add xmin int;
ERROR: column "xmin" of relation "lx" already exists.

That is not a bug, but a feature.

I see it as a message bug. Why wouldn't ALTER TABLE also tell us that xmin
is a system column? It makes things much more clear for newbies who don't
see the column yet are told it exists if they're also told it's a system
column.

I agree with Vik, the CREATE TABLE tells "xmin" is a system column,
why wouldn't ALTER TABLE do the same?

This would be a feature if CREATE TABLE didn't tell us that
xmin is a system column, otherwise is a bug.

I would try to cook up a patch but I have no skills :-(

I'm going to write a patch to fix this problem.
Regards.

--
Giuseppe Sucameli

Giuseppe Sucameli

brush.tyler@gmail.com

almost 14 years ago

In reply to: Giuseppe Sucameli (#1)

1 attachment(s)

Hi hackers,

the attached patch fixes the problem I explained in pgsql-bugs (forwarded).
It's a trivial problem, so no initial patch design was required.

Hope to see my patch applied.
Thanks for your work.

Regards.

---------- Forwarded message ----------
From: Giuseppe Sucameli <brush.tyler@gmail.com>
Date: Sun, Jan 22, 2012 at 2:22 PM
Subject: Different error messages executing CREATE TABLE or ALTER
TABLE to create a column "xmin"
To: pgsql-bugs@postgresql.org

Hi all,

trying to create a table with a column xmin I get the
following error message:

test=> create table lx (xmin int);
ERROR: column name "xmin" conflicts with a system
column name

Instead I get a different (and less understandable) error
message if I try to add a column named xmin to an
existent table:

test=> create table lx (i int);
CREATE TABLE
test=> alter table lx add xmin int;
ERROR: column "xmin" of relation "lx" already exists.

The same problem occurs using "xmax" as column name.

I'm on Ubuntu 11.04.
Tried on both PostgreSQL 8.4.10 and 9.1.2

Regards.

--
Giuseppe Sucameli

Attachments:

postgresql_syscol_message.diffapplication/octet-stream; name=postgresql_syscol_message.diffDownload

From f9368a06b7e608d13a6e9718c9b95878eae9bd9c Mon Sep 17 00:00:00 2001
From: Giuseppe Sucameli <brush.tyler@gmail.com>
Date: Tue, 24 Jan 2012 14:45:50 +0100
Subject: better error message executing ALTER TABLE to add/rename a column named like a system column

---
 src/backend/commands/tablecmds.c     |   34 ++++++++++++++++++++++++++++++++++
 src/test/regress/expected/errors.out |    2 +-
 2 files changed, 35 insertions(+), 1 deletions(-)

diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index cc210f0..3431a89 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -2258,6 +2258,24 @@ renameatt_internal(Oid myrelid,
 
 	/* new name should not already exist */
 
+	/*
+	 * first check for collision with system attribute names
+	 *
+	 * Skip this for a view or type relation, since those don't have system
+	 * attributes.
+	 */
+	if (targetrelation->rd_rel->relkind != RELKIND_VIEW
+		&& targetrelation->rd_rel->relkind != RELKIND_COMPOSITE_TYPE)
+	{
+		if (SystemAttributeByName(newattname,
+								  targetrelation->rd_rel->relhasoids) != NULL)
+			ereport(ERROR,
+					(errcode(ERRCODE_DUPLICATE_COLUMN),
+					 errmsg("column name \"%s\" conflicts with a system column name",
+							newattname)));
+	}
+
+
 	/* this test is deliberately not attisdropped-aware */
 	if (SearchSysCacheExists2(ATTNAME,
 							  ObjectIdGetDatum(myrelid),
@@ -4294,6 +4312,22 @@ ATExecAddColumn(List **wqueue, AlteredTableInfo *tab, Relation rel,
 	relkind = ((Form_pg_class) GETSTRUCT(reltup))->relkind;
 
 	/*
+	 * first check for collision with system attribute names
+	 *
+	 * Skip this for a view or type relation, since those don't have system
+	 * attributes.
+	 */
+	if (relkind != RELKIND_VIEW && relkind != RELKIND_COMPOSITE_TYPE)
+	{
+		if (SystemAttributeByName(colDef->colname,
+								  ((Form_pg_class) GETSTRUCT(reltup))->relhasoids && !isOid) != NULL)
+			ereport(ERROR,
+					(errcode(ERRCODE_DUPLICATE_COLUMN),
+					 errmsg("column name \"%s\" conflicts with a system column name",
+							colDef->colname)));
+	}
+
+	/*
 	 * this test is deliberately not attisdropped-aware, since if one tries to
 	 * add a column matching a dropped column name, it's gonna fail anyway.
 	 */
diff --git a/src/test/regress/expected/errors.out b/src/test/regress/expected/errors.out
index 4a10c6a..fa0bd82 100644
--- a/src/test/regress/expected/errors.out
+++ b/src/test/regress/expected/errors.out
@@ -109,7 +109,7 @@ alter table emp rename column salary to manager;
 ERROR:  column "manager" of relation "stud_emp" already exists
 -- conflict
 alter table emp rename column salary to oid;
-ERROR:  column "oid" of relation "stud_emp" already exists
+ERROR:  column name "oid" conflicts with a system column name
 --
 -- TRANSACTION STUFF
 -- not in a xact
-- 
1.7.4.1

Vik Reykja

vikreykja@gmail.com

almost 14 years ago

In reply to: Giuseppe Sucameli (#4)

Re: Different error messages executing CREATE TABLE or ALTER TABLE to create a column "xmin"

On Tue, Jan 24, 2012 at 14:41, Giuseppe Sucameli <brush.tyler@gmail.com>wrote:

I would try to cook up a patch but I have no skills :-(

I'm going to write a patch to fix this problem.

I managed to put something together and have posted it on hackers.

Giuseppe Sucameli

brush.tyler@gmail.com

almost 14 years ago

In reply to: Vik Reykja (#6)

Re: Different error messages executing CREATE TABLE or ALTER TABLE to create a column "xmin"

Hi Vik,

On Wed, Jan 25, 2012 at 12:28 AM, Vik Reykja <vikreykja@gmail.com> wrote:

On Tue, Jan 24, 2012 at 14:41, Giuseppe Sucameli <brush.tyler@gmail.com>
wrote:

I would try to cook up a patch but I have no skills :-(

I'm going to write a patch to fix this problem.

I managed to put something together and have posted it on hackers.

thinking you have no skills, I wrote a patch too and posted it to
hackers ML about 2 days ago, but I didn't subscribe that list so
it is stalled, waiting for someone's approval.

For this reason my email is not displayed in the archives.
I'm so sorry both have wasted our time in writing 2 patches for
the same bug.

Regards.

--
Giuseppe Sucameli

Vik Reykja

vikreykja@gmail.com

almost 14 years ago

In reply to: Giuseppe Sucameli (#7)

Re: Different error messages executing CREATE TABLE or ALTER TABLE to create a column "xmin"

On Wed, Jan 25, 2012 at 22:19, Giuseppe Sucameli <brush.tyler@gmail.com>wrote:

thinking you have no skills, I wrote a patch too and posted it to
hackers ML about 2 days ago, but I didn't subscribe that list so
it is stalled, waiting for someone's approval.

For this reason my email is not displayed in the archives.
I'm so sorry both have wasted our time in writing 2 patches for
the same bug.

I think your patch is more complete than mine so it's definitely not
wasted. I spent several hours on mine and now I have a little bit of
skills :-)

Thank you for your effort; I'm sorry if you feel you have wasted your time.

Robert Haas

robertmhaas@gmail.com

almost 14 years ago

In reply to: Vik Reykja (#8)

Re: Different error messages executing CREATE TABLE or ALTER TABLE to create a column "xmin"

On Thu, Jan 26, 2012 at 5:43 AM, Vik Reykja <vikreykja@gmail.com> wrote:

On Wed, Jan 25, 2012 at 22:19, Giuseppe Sucameli <brush.tyler@gmail.com>
wrote:

thinking you have no skills, I wrote a patch too and posted it to
hackers ML about 2 days ago, but I didn't subscribe that list so
it is stalled, waiting for someone's approval.

For this reason my email is not displayed in the archives.
I'm so sorry both have wasted our time in writing 2 patches for
the same bug.

I think your patch is more complete than mine so it's definitely not
wasted. I spent several hours on mine and now I have a little bit of skills
:-)

Thank you for your effort; I'm sorry if you feel you have wasted your time.

I think Vik's patch is better, because it doesn't rely as much on
things that happen to be true today - like which relkinds have system
columns associated with them. But I do agree we ought to handle both
the ADD COLUMN and RENAME COLUMN cases, and as such have posted an
update of Vik's patch on the other thread.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Compan