From: | Florian Pflug <fgp(at)phlo(dot)org> |
---|---|
To: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
Cc: | Noah Misch <noah(at)2ndQuadrant(dot)com>, Bernd Helmle <mailings(at)oopsware(dot)de>, PG Hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: Another issue with invalid XML values |
Date: | 2011-07-26 14:41:48 |
Message-ID: | F8B9185F-87F1-4F5E-B3E8-6896F043B629@phlo.org |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Jul26, 2011, at 16:22 , Tom Lane wrote:
> Florian Pflug <fgp(at)phlo(dot)org> writes:
>> On further reflection, instead of checking whether we can restore the error
>> handler in pg_xml_init_library(), we could simply upgrade the elog(WARNING)
>> in pg_xml_done() to ereport(ERROR), and include a hint that explains the
>> probably cause.
>
>> The upside being that we only fail when we actually need to restore the
>> error handler. Since there's one caller (parse_xml_decl) who calls
>> pg_xml_init_library() but not pg_xml_init()/pg_xml_done(), the difference
>> isn't only academic. At least XML would output will continue to work
>> work after libxml is upgraded from < 2.7.4 to >= 2.7.4.
>
> Good point. But what about failing in pg_xml_init? That is, after
> calling xmlSetStructuredErrorFunc, check that it set the variable we
> expected it to set.
Yeah, that's even better. Will do it that way.
What about the suggested upgrade of the elog(ERROR) in xml_errorHandler
to elog(FATAL)? Shall I do that too, or leave it for now?
> The purpose of the check in pg_xml_done is not to detect library issues,
> but to detect omitted save/restore pairs and similar coding mistakes.
> I don't want to upgrade it to an ERROR, and I don't want to confuse
> people by hinting that the problem is with libxml.
I can see the concern about possible confusion, and I agree that
pg_xml_init(), right after we set our error handler, is the most logical
place to test whether we can read it back.
I guess I don't really see the benefit of the check in pg_xml_done()
triggering a WARNING instead of an ERROR, but since we won't be hijacking
that check now anyway, I don't mind it being a WARNING either.
best regards,
Florian Pflug
From | Date | Subject | |
---|---|---|---|
Next Message | Tom Lane | 2011-07-26 14:51:58 | Re: Check constraints on partition parents only? |
Previous Message | Alvaro Herrera | 2011-07-26 14:39:18 | Re: vacuumlo patch |