Re: Another issue with invalid XML values

On Fri, Jun 24, 2011 at 04:15:35PM +0200, Florian Pflug wrote:
> On Jun24, 2011, at 13:24 , Noah Misch wrote:
> > On Wed, Jun 22, 2011 at 12:47:42PM +0200, Florian Pflug wrote:

> > There's also the " parser error :" difference; would that also be easy to
> > re-add? (I'm assuming it's not a constant but depends on the xmlErrorDomain.)
>
> It's the string representation of the error domain and error level. Unfortunately,
> the logic which builds that string representation is contained in the static
> function xmlReportError() deep within libxml, and that function doesn't seem
> to be called at all for errors reported via a structured error handler :-(
>
> So re-adding that would mean duplicating that code within our xml.c, which
> seems undesirable...

Yes, that seems like sufficient trouble to not undertake merely for the sake of
a cosmetic error message match.

> >> ! typedef enum
> >> ! {
> >> ! PG_XML_STRICTNESS_NONE /* No error handling */,
> >> ! PG_XML_STRICTNESS_LEGACY /* Ignore notices/warnings/errors unless
> >> ! * function result indicates error condition
> >> ! */,
> >> ! PG_XML_STRICTNESS_WELLFORMED/* Ignore non-parser notices/warnings/errors */,
> >> ! PG_XML_STRICTNESS_ALL /* Report all notices/warnings/errors */,
> >> ! } PgXmlStrictnessType;
> >
> > We don't generally prefix backend names with Pg/PG. Also, I think the word
> > "Type" in "PgXmlStrictnessType" is redundant. How about just XmlStrictness?
>
> I agree with removing the "Type" suffix. I'm not so sure about the "Pg"
> prefix, though. I've added that after actually hitting a conflict between
> one of this enum's constants and some constant defined by libxml. Admittedly,
> that was *before* I renamed the type to "PgXmlStrictnessType", so removing
> the "Pg" prefix now would probably work. But it seems a bit close for
> comfort - libxml defines a ton of constants named XML_something.
>
> Still, if you feel that the "Pg" prefix looks to weird and believe that it's
> better to wait until there's an actual clash before renaming things, I won't
> object further. Just wanted to bring the issue to your attention...

I do think it looks weird, but I agree that the potential for conflict is
notably higher than normal. I'm fine with having the prefix.

Thanks,
nm