ALTER TABLE INHERIT vs collations

Started by Tom Laneover 14 years ago9 messages
#1Tom Lane
tgl@sss.pgh.pa.us

Right at the moment, ALTER INHERIT doesn't verify that collations match
in a proposed inheritance child. So you can do this:

regression=# create table foo (f1 text collate "C");
CREATE TABLE
regression=# create table bar (f1 text collate "POSIX");
CREATE TABLE
regression=# alter table bar inherit foo;
ALTER TABLE

but then the planner whines about it:

regression=# select * from foo;
ERROR: attribute "f1" of relation "bar" does not match parent's collation

Does anyone think it's not a bug that ALTER TABLE lets this through?
If so, what do you think the querying semantics ought to be?

regards, tom lane

#2Thom Brown
thom@linux.com
In reply to: Tom Lane (#1)
Re: ALTER TABLE INHERIT vs collations

On 16 April 2011 23:23, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Right at the moment, ALTER INHERIT doesn't verify that collations match
in a proposed inheritance child.  So you can do this:

regression=# create table foo (f1 text collate "C");
CREATE TABLE
regression=# create table bar (f1 text collate "POSIX");
CREATE TABLE
regression=# alter table bar inherit foo;
ALTER TABLE

but then the planner whines about it:

regression=# select * from foo;
ERROR:  attribute "f1" of relation "bar" does not match parent's collation

Does anyone think it's not a bug that ALTER TABLE lets this through?
If so, what do you think the querying semantics ought to be?

An argument to not treat it as a bug might be to suggest that the
child table's column could inherit the parent table's column collation
when the query targets the parent, but revert to its own otherwise.

--
Thom Brown
Twitter: @darkixion
IRC (freenode): dark_ixion
Registered Linux user: #516935

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

#3Tom Lane
tgl@sss.pgh.pa.us
In reply to: Thom Brown (#2)
Re: ALTER TABLE INHERIT vs collations

Thom Brown <thom@linux.com> writes:

On 16 April 2011 23:23, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Does anyone think it's not a bug that ALTER TABLE lets this through?
If so, what do you think the querying semantics ought to be?

An argument to not treat it as a bug might be to suggest that the
child table's column could inherit the parent table's column collation
when the query targets the parent, but revert to its own otherwise.

That seems to me to be about on par with arguing that inheritance
shouldn't demand column type matching, but should coerce child columns
on-the-fly to their parent's type. Even if you don't think that's
horrid from a theoretical standpoint, there's a good practical reason
not to allow it: if it acts that way, then indexes on the child column
will silently not be usable for many kinds of query against the parent.
People will be tearing their hair out looking for the cause of their
performance problems ... and, no doubt, filing bugs against the planner
... when throwing an error would have helped them catch the mismatch.

I think there needs to be a pretty darn compelling use-case for such
mismatches before we should allow them. And I don't see one.

regards, tom lane

#4Thom Brown
thom@linux.com
In reply to: Tom Lane (#3)
Re: ALTER TABLE INHERIT vs collations

On 17 April 2011 00:40, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Thom Brown <thom@linux.com> writes:

On 16 April 2011 23:23, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Does anyone think it's not a bug that ALTER TABLE lets this through?
If so, what do you think the querying semantics ought to be?

An argument to not treat it as a bug might be to suggest that the
child table's column could inherit the parent table's column collation
when the query targets the parent, but revert to its own otherwise.

That seems to me to be about on par with arguing that inheritance
shouldn't demand column type matching, but should coerce child columns
on-the-fly to their parent's type.  Even if you don't think that's
horrid from a theoretical standpoint, there's a good practical reason
not to allow it: if it acts that way, then indexes on the child column
will silently not be usable for many kinds of query against the parent.
People will be tearing their hair out looking for the cause of their
performance problems ... and, no doubt, filing bugs against the planner
... when throwing an error would have helped them catch the mismatch.

I think there needs to be a pretty darn compelling use-case for such
mismatches before we should allow them.  And I don't see one.

Conceded.

--
Thom Brown
Twitter: @darkixion
IRC (freenode): dark_ixion
Registered Linux user: #516935

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

#5Robert Haas
robertmhaas@gmail.com
In reply to: Tom Lane (#1)
Re: ALTER TABLE INHERIT vs collations

On Sat, Apr 16, 2011 at 6:23 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Right at the moment, ALTER INHERIT doesn't verify that collations match
in a proposed inheritance child.  So you can do this:

regression=# create table foo (f1 text collate "C");
CREATE TABLE
regression=# create table bar (f1 text collate "POSIX");
CREATE TABLE
regression=# alter table bar inherit foo;
ALTER TABLE

but then the planner whines about it:

regression=# select * from foo;
ERROR:  attribute "f1" of relation "bar" does not match parent's collation

Does anyone think it's not a bug that ALTER TABLE lets this through?
If so, what do you think the querying semantics ought to be?

We seem to generally be treating collations as type information, so
that seems to argue that we ought to force it to match here.

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

#6Jean-Pierre Pelletier
pelletier_32@sympatico.ca
In reply to: Robert Haas (#5)
Re: ALTER TABLE INHERIT vs collations

One use case for this might be with constraint exclusion where there
would be one
partition per collation (language) with queries against the parent table always
being for exactly one language.

Not sure what the collation should be in the parent table then.

#7Peter Eisentraut
peter_e@gmx.net
In reply to: Tom Lane (#1)
Re: ALTER TABLE INHERIT vs collations

On Sat, 2011-04-16 at 18:23 -0400, Tom Lane wrote:

Right at the moment, ALTER INHERIT doesn't verify that collations
match in a proposed inheritance child.

It should be prevented, I think.

#8Peter Eisentraut
peter_e@gmx.net
In reply to: Jean-Pierre Pelletier (#6)
Re: ALTER TABLE INHERIT vs collations

On Sat, 2011-04-16 at 21:52 -0400, Jean-Pierre Pelletier wrote:

One use case for this might be with constraint exclusion where there
would be one
partition per collation (language) with queries against the parent table always
being for exactly one language.

If you really wanted that, you can always use a view to make it
explicit.

#9Tom Lane
tgl@sss.pgh.pa.us
In reply to: Peter Eisentraut (#7)
Re: ALTER TABLE INHERIT vs collations

Peter Eisentraut <peter_e@gmx.net> writes:

On Sat, 2011-04-16 at 18:23 -0400, Tom Lane wrote:

Right at the moment, ALTER INHERIT doesn't verify that collations
match in a proposed inheritance child.

It should be prevented, I think.

BTW, on looking through the source to find the cause, I see that the
reason it's not prevented is that MergeAttributesIntoExisting() failed
to check for collation match. But the other three places in tablecmds.c
that check for child column type matches do check collation too. So it
seems clear that this was just an oversight in one case.

regards, tom lane