DROP COLUMN

On Mon, 2002-07-15 at 11:30, Christopher Kings-Lynne wrote:

OK, more DROP COLUMN funny business:

Assuming that selects, updates and deletes all ignore the dropped column,
what happens with things like alter table statements?

You can still quite happily set the default for a dropped column, etc.

Will I have to add a dropped column check in everywhere that a command is
able to target a column. ie. create index, cluster, alter table, etc,
etc.? Or is there an easier way?

Each utility statement does some kind of a SearchSysCache() to determine
the status of the column (whether it exists or not).

You may want to write a wrapper function in lsyscache.c that returns the
status of the column (dropped or not). Perhaps the att tuple could be
fetched through this function (processed on the way out) -- though
lsyscache routines tend to return simple items.

Rod Taylor wrote:

On Mon, 2002-07-15 at 11:30, Christopher Kings-Lynne wrote:

OK, more DROP COLUMN funny business:

Assuming that selects, updates and deletes all ignore the dropped column,
what happens with things like alter table statements?

You can still quite happily set the default for a dropped column, etc.

Will I have to add a dropped column check in everywhere that a command is
able to target a column. ie. create index, cluster, alter table, etc,
etc.? Or is there an easier way?

Each utility statement does some kind of a SearchSysCache() to determine
the status of the column (whether it exists or not).

You may want to write a wrapper function in lsyscache.c that returns the
status of the column (dropped or not). Perhaps the att tuple could be
fetched through this function (processed on the way out) -- though
lsyscache routines tend to return simple items.

Excellent idea. That's how temp tables worked, by bypassing the
syscache. I wonder if you could just prevent dropped columns from being
returned by the syscache. That may work just fine.

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

Bruce Momjian <pgman@candle.pha.pa.us> writes:

Excellent idea. That's how temp tables worked, by bypassing the
syscache. I wonder if you could just prevent dropped columns from being
returned by the syscache. That may work just fine.

No, it will break all the places that need to see dropped columns.

I agree that a wrapper function is probably an appropriate solution,
but only some of the calls of SearchSysCache should use it.

regards, tom lane

Bruce Momjian <pgman@candle.pha.pa.us> writes:

Excellent idea. That's how temp tables worked, by bypassing the
syscache. I wonder if you could just prevent dropped columns from being
returned by the syscache. That may work just fine.

No, it will break all the places that need to see dropped columns.

I agree that a wrapper function is probably an appropriate solution,
but only some of the calls of SearchSysCache should use it.

What like add another parameter to SearchSysCache*?

Another question: How do I fill out the ObjectAddress when trying to drop
related objects?

eg:

object.classId = ??;
object.objectId = ??;
object.objectSubId = ??;

performDeletion(&object, behavior);

Chris

"Christopher Kings-Lynne" <chriskl@familyhealth.com.au> writes:

I agree that a wrapper function is probably an appropriate solution,
but only some of the calls of SearchSysCache should use it.

What like add another parameter to SearchSysCache*?

Definitely *not*; I don't want to kluge up every call to SearchSysCache
with a feature that's only relevant to a small number of them.

Another question: How do I fill out the ObjectAddress when trying to drop
related objects?

A column would be classId = RelOid_pg_class, objectId = OID of relation,
objectSubId = column's attnum.

BTW, it occurred to me recently that most of the column-specific
AlterTable operations will happily try to alter system columns (eg,
OID). In most cases this makes no sense and should be forbidden.
It definitely makes no sense for DROP COLUMN...

regards, tom lane

Tom Lane wrote:

"Christopher Kings-Lynne" <chriskl@familyhealth.com.au> writes:

I agree that a wrapper function is probably an appropriate solution,
but only some of the calls of SearchSysCache should use it.

What like add another parameter to SearchSysCache*?

Definitely *not*; I don't want to kluge up every call to SearchSysCache
with a feature that's only relevant to a small number of them.

Uh, then what? The only idea I had was to set a static boolean variable in
syscache.c that controls whether droppped columns are returned, and have
a enable/disable functions that can turn it on/off. The only problem is
that an elog inside a syscache lookup would leave that value set.

My only other idea is to make a syscache that is like ATTNAME except
that it doesn't return a dropped column. I could probably code that up
if you wish.

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

Uh, then what? The only idea I had was to set a static boolean
variable in
syscache.c that controls whether droppped columns are returned, and have
a enable/disable functions that can turn it on/off. The only problem is
that an elog inside a syscache lookup would leave that value set.

My only other idea is to make a syscache that is like ATTNAME except
that it doesn't return a dropped column. I could probably code that up
if you wish.

That'd be cool.

I guess the thing is that either way, I will need to manually change every
single instance where a dropped column should be avoided. So, really
there's not much difference between me changing the SysCache search to use
ATTNAMEUNDROPPED or whatever, or just checking the attisdropped field of the
tuple in the same way that you must always check that attnum > 0.

In fact, looking at it logically...if all the commands currently are
required to check that they're not modifiying a system column, then why not
add the requirement that they must also not modify dropped columns? I can
do a careful doc search and try to make sure I've touched everything...

Chris

Christopher Kings-Lynne wrote:

Uh, then what? The only idea I had was to set a static boolean
variable in
syscache.c that controls whether droppped columns are returned, and have
a enable/disable functions that can turn it on/off. The only problem is
that an elog inside a syscache lookup would leave that value set.

My only other idea is to make a syscache that is like ATTNAME except
that it doesn't return a dropped column. I could probably code that up
if you wish.

That'd be cool.

I guess the thing is that either way, I will need to manually change every
single instance where a dropped column should be avoided. So, really
there's not much difference between me changing the SysCache search to use
ATTNAMEUNDROPPED or whatever, or just checking the attisdropped field of the
tuple in the same way that you must always check that attnum > 0.

In fact, looking at it logically...if all the commands currently are
required to check that they're not modifiying a system column, then why not
add the requirement that they must also not modify dropped columns? I can
do a careful doc search and try to make sure I've touched everything...

Makes sense. Of course, we could make a syscache that didn't return
system columns either.

Actually, the original argument for negative attno's for dropped columns
was exactly for this case, that the system column check would catch
dropped columns too, but it causes other problems that are harder to fix
so we _dropped_ the idea.

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

Bruce Momjian <pgman@candle.pha.pa.us> writes:

Tom Lane wrote:

Definitely *not*; I don't want to kluge up every call to SearchSysCache
with a feature that's only relevant to a small number of them.

Uh, then what?

Then we make a wrapper function. Something like

GetUndeletedColumnByName(relid,attname)

replaces SearchSysCache(ATTNAME,...) in those places where you don't
want to see deleted columns. It'd return NULL if it finds a column
tuple but sees it's deleted.

GetUndeletedColumnByNum(relid,attnum)

replaces SearchSysCache(ATTNUM,...) similarly.

My only other idea is to make a syscache that is like ATTNAME except
that it doesn't return a dropped column.

That would mean duplicate storage of tuples inside the catcache...

regards, tom lane

Tom Lane wrote:

Bruce Momjian <pgman@candle.pha.pa.us> writes:

Tom Lane wrote:

Definitely *not*; I don't want to kluge up every call to SearchSysCache
with a feature that's only relevant to a small number of them.

Uh, then what?

Then we make a wrapper function. Something like

GetUndeletedColumnByName(relid,attname)

replaces SearchSysCache(ATTNAME,...) in those places where you don't
want to see deleted columns. It'd return NULL if it finds a column
tuple but sees it's deleted.

GetUndeletedColumnByNum(relid,attnum)

replaces SearchSysCache(ATTNUM,...) similarly.

Good idea.

My only other idea is to make a syscache that is like ATTNAME except
that it doesn't return a dropped column.

That would mean duplicate storage of tuples inside the catcache...

No, I was thinking of something that did the normal ATTNAME lookup in
the syscache code, then returned NULL on dropped columns; similar to
your idea but done inside the syscache code rather than in a separate
function.

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

Actually, the original argument for negative attno's for dropped columns
was exactly for this case, that the system column check would catch
dropped columns too, but it causes other problems that are harder to fix
so we _dropped_ the idea.

Well, negative attnums are a good idea and yes, you sort of avoid all these
problems. However, the backend is _full_ of stuff like this:

if (attnum < 0)
elog(ERROR, "Cannot footle system attribute.");

But the problem is that we'd have to change all of them anyway in a negative
attnum implementation, since they're not system attributes, they're dropped
columns.

But let's not start another thread about this!!

Chris

In fact, looking at it logically...if all the commands currently are
required to check that they're not modifiying a system column,

then why not

add the requirement that they must also not modify dropped

columns? I can

do a careful doc search and try to make sure I've touched everything...

Makes sense. Of course, we could make a syscache that didn't return
system columns either.

Actually - are you certain that every command uses a SearchSysCache and not
some other weirdness? If we have to do the odd exception, then maybe we
should do them all as 'exceptions'?

Chris

Christopher Kings-Lynne wrote:

In fact, looking at it logically...if all the commands currently are
required to check that they're not modifiying a system column,

then why not

add the requirement that they must also not modify dropped

columns? I can

do a careful doc search and try to make sure I've touched everything...

Makes sense. Of course, we could make a syscache that didn't return
system columns either.

Actually - are you certain that every command uses a SearchSysCache and not
some other weirdness? If we have to do the odd exception, then maybe we
should do them all as 'exceptions'?

I actually don't know. I know all the table name lookups do use
syscache or temp tables wouldn't have worked. ;-)

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

"Christopher Kings-Lynne" <chriskl@familyhealth.com.au> writes:

Actually - are you certain that every command uses a SearchSysCache and not
some other weirdness?

They probably don't. You'll have to look closely at places that use the
TupleDesc from a relcache entry.

regards, tom lane

-----Original Message-----
From: Bruce Momjian

Christopher Kings-Lynne wrote:

Uh, then what? The only idea I had was to set a static boolean
variable in
syscache.c that controls whether droppped columns are

returned, and have

a enable/disable functions that can turn it on/off. The only

problem is

that an elog inside a syscache lookup would leave that value set.

My only other idea is to make a syscache that is like ATTNAME except
that it doesn't return a dropped column. I could probably

code that up

if you wish.

That'd be cool.

I guess the thing is that either way, I will need to manually

change every

single instance where a dropped column should be avoided. So, really
there's not much difference between me changing the SysCache

search to use

ATTNAMEUNDROPPED or whatever, or just checking the attisdropped

field of the

tuple in the same way that you must always check that attnum > 0.

In fact, looking at it logically...if all the commands currently are
required to check that they're not modifiying a system column,

then why not

add the requirement that they must also not modify dropped

columns? I can

do a careful doc search and try to make sure I've touched everything...

Makes sense. Of course, we could make a syscache that didn't return
system columns either.

Actually, the original argument for negative attno's for dropped columns
was exactly for this case, that the system column check would catch
dropped columns too,

but it causes other problems that are harder to fix
so we _dropped_ the idea.

What does this mean ?
BTW would we do nothing for clients after all ?

regards,
Hiroshi Inoue

Hiroshi Inoue wrote:

Makes sense. Of course, we could make a syscache that didn't return
system columns either.

Actually, the original argument for negative attno's for dropped columns
was exactly for this case, that the system column check would catch
dropped columns too,

but it causes other problems that are harder to fix
so we _dropped_ the idea.

What does this mean ?

Client programmers prefered the dropped flag rather than negative
attno's so we went with that.

BTW would we do nothing for clients after all ?

Clients will now need to check that dropped flag.

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

On Tue, 2002-07-16 at 18:30, Bruce Momjian wrote:

Hiroshi Inoue wrote:

Makes sense. Of course, we could make a syscache that didn't return
system columns either.

Actually, the original argument for negative attno's for dropped columns
was exactly for this case, that the system column check would catch
dropped columns too,

but it causes other problems that are harder to fix
so we _dropped_ the idea.

What does this mean ?

Client programmers prefered the dropped flag rather than negative
attno's so we went with that.

While you are at it,could you add another flag is_system ?

<evil grin>

-----------
Hannu

Bruce Momjian wrote:

Hiroshi Inoue wrote:

Makes sense. Of course, we could make a syscache that didn't return
system columns either.

Actually, the original argument for negative attno's for dropped columns
was exactly for this case, that the system column check would catch
dropped columns too,

but it causes other problems that are harder to fix
so we _dropped_ the idea.

What does this mean ?

Client programmers prefered the dropped flag rather than negative
attno's so we went with that.

What I asked you is what *harder to fix* means.

BTW would we do nothing for clients after all ?

Clients will now need to check that dropped flag.

Clients would have to check the flag everywhere
pg_attribute appears.
Why should clients do such a thing ?

regards,
Hiroshi Inoue
http://w2422.nsk.ne.jp/~inoue/

Hiroshi Inoue wrote:

Bruce Momjian wrote:

Hiroshi Inoue wrote:

Makes sense. Of course, we could make a syscache that didn't return
system columns either.

Actually, the original argument for negative attno's for dropped columns
was exactly for this case, that the system column check would catch
dropped columns too,

but it causes other problems that are harder to fix
so we _dropped_ the idea.

What does this mean ?

Client programmers prefered the dropped flag rather than negative
attno's so we went with that.

What I asked you is what *harder to fix* means.

Uh, some said that having attno's like 1,2,3,5,7,8,9 with gaps would
cause coding problems in client applications, and that was easier to
have the numbers as 1-9 and check a flag if the column is dropped. Why
that is easier than having gaps, I don't understand. I voted for the
gaps (with negative attno's) but client coders liked the flag, so we
went with that.

BTW would we do nothing for clients after all ?

Clients will now need to check that dropped flag.

Clients would have to check the flag everywhere
pg_attribute appears.
Why should clients do such a thing ?

Well, good question. They could easily skip the dropped columns if we
used negative attno's because they usually already skip system columns.
However, they prefered a specific dropped column flag and positive
attno's. I don't know why. They would have to explain.

From my perspective, when client coders like Dave Page and others say
they would prefer the flag to the negative attno's, I don't have to
understand. I just take their word for it.

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026