Thread: Re: [GENERAL] possible to create multivalued index from xpath() results in 8.3?

Am Mittwoch, 21. November 2007 schrieb Tom Lane:
> "Matt Magoffin" <postgresql.org@msqr.us> writes:
> > Ugh, you're right of course! Somehow I had this wrong. So I tried to
> > create an index on the xml[] result by casting to text[] but I got the
> > "function must be immutable" error. Is there any reason the xml[] to
> > text[] cast is not immutable?
>
> Hmm ... I see that xmltotext() is marked 'stable' in pg_proc.h,
> but texttoxml() is marked 'immutable', which is at best inconsistent.
> It looks to me like they both depend on the GUC setting "xmloption",
> which would mean they should both be stable.  Peter, is there a bug
> there?

Yeah, that doesn't look right.

> Also, is there a way to get rid of the GUC dependency so that 
> there's a reasonably safe way to index XML values?

We could drop the dependency in xmltotext() with little loss of functionality.  
The spec doesn't allow casts between xml and text (varchar) at all.  The way 
I appear to have derived the current behavior from the spec is that this is 
interpreted as an implicit XMLSERIALIZE call in the context of a prepared 
statement, which is defined to observe the XML option, as per clause 17.3 
(part 14).  This was the clostest piece of spec that described conversion 
from xml to character types.  Now with the xpath functionality, there is 
certainly a strong use case for ignoring this altogether and just serializing 
with the XML option set to "content".

-- 
Peter Eisentraut
http://developer.postgresql.org/~petere/

Peter Eisentraut <peter_e@gmx.net> writes:
> The spec doesn't allow casts between xml and text (varchar) at all.  The way 
> I appear to have derived the current behavior from the spec is that this is 
> interpreted as an implicit XMLSERIALIZE call in the context of a prepared 
> statement, which is defined to observe the XML option, as per clause 17.3 
> (part 14).  This was the clostest piece of spec that described conversion 
> from xml to character types.  Now with the xpath functionality, there is 
> certainly a strong use case for ignoring this altogether and just serializing
> with the XML option set to "content".

Given the actual behavior of xmltotext_with_xmloption, it certainly
seems like a pretty useless error check.  Also, xml_out doesn't behave
that way, so why should xmltotext?

The volatility markings of xml_in and texttoxml seem wrong too.
It looks to me like we need:
xml_in        should be STABLE because it depends on xmloptionxml_recv    ditto (OK already)xml_out        correctly(?)
markedIMMUTABLExml_send    is STABLE, OK because it depends on client_encodingtexttoxml    should be STABLE because it
dependson xmloptionxmltotext    remove xmloption dependency, mark as IMMUTABLE
 

Should we force initdb to correct these pg_proc entries, or just quietly
change pg_proc.h?
        regards, tom lane

Tom Lane wrote:
> Given the actual behavior of xmltotext_with_xmloption, it certainly
> seems like a pretty useless error check.  Also, xml_out doesn't behave
> that way, so why should xmltotext?
>
> The volatility markings of xml_in and texttoxml seem wrong too.

This is the patch that came out of it.

> Should we force initdb to correct these pg_proc entries, or just quietly
> change pg_proc.h?

Considering the extent of the changes, I'd be in favor of forcing an initdb.

-- 
Peter Eisentraut
http://developer.postgresql.org/~petere/

Peter Eisentraut <peter_e@gmx.net> writes:
> Tom Lane wrote:
>> Should we force initdb to correct these pg_proc entries, or just quietly
>> change pg_proc.h?

> Considering the extent of the changes, I'd be in favor of forcing an initdb.

Well, if you're going to change the contents of pg_cast then there is
little choice.  I was considering something less invasive ...
        regards, tom lane

Am Freitag, 23. November 2007 schrieb Tom Lane:
> Peter Eisentraut <peter_e@gmx.net> writes:
> > Tom Lane wrote:
> >> Should we force initdb to correct these pg_proc entries, or just quietly
> >> change pg_proc.h?
> >
> > Considering the extent of the changes, I'd be in favor of forcing an
> > initdb.
>
> Well, if you're going to change the contents of pg_cast then there is
> little choice.  I was considering something less invasive ...

I will hang on to this patch for a few more days to see if any invasive 
catalog changes come out of the quote_literal/set_config discussion.  If not, 
I'll consider a less invasive solution.

-- 
Peter Eisentraut
http://developer.postgresql.org/~petere/

Peter Eisentraut <peter_e@gmx.net> writes:
>> Peter Eisentraut <peter_e@gmx.net> writes:
> Tom Lane wrote:
>> Well, if you're going to change the contents of pg_cast then there is
>> little choice.  I was considering something less invasive ...

> I will hang on to this patch for a few more days to see if any invasive 
> catalog changes come out of the quote_literal/set_config discussion.  If not,
> I'll consider a less invasive solution.

ATM it seems that consensus is to change quote_literal, so we may as
well adopt the cleaner solution for fixing the xml functions too.

Anyone out there who wants to argue against forcing initdb for RC1?
If so, better speak up now.
        regards, tom lane