On Jul26, 2011, at 16:22 , Tom Lane wrote: > Florian Pflug <f...@phlo.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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers