Thread: Cleaning up ERRCODE usage in our XML code
I noticed while working on bug #18617 [1] that we are fairly slipshod
about which SQLSTATE to report when libxml2 returns an error. There
are places using ERRCODE_INTERNAL_ERROR for easily-triggered errors;
there are different places that use different ERRCODEs for exactly
the same condition; and there are places that use different ERRCODEs
for failures from xmlXPathCompile and xmlXPathCompiledEval. I found
that this last is problematic because some errors you might expect
to be reported during XPath compilation are not detected till
execution, notably namespace-related errors. That seems more like
a libxml2 implementation artifact than something we should expect to
be stable behavior, so I think we should avoid using different
ERRCODEs.
A lot of this can be blamed on there not being any especially on-point
SQLSTATE values back when this code was first written. I learned that
recent revisions of SQL have a whole new SQLSTATE class, class 10 =
"XQuery Error", so we have an opportunity to sync up with that as well
as be more self-consistent. The spec's subclass codes in this class
seem quite fine-grained. It might be an interesting exercise to try
to teach xml_errorHandler() to translate libxml2's error->code values
into fine-grained SQLSTATEs, but I've not attempted that; I'm not
sure whether there is a close mapping between what libxml2 reports
and the set of codes the SQL committee chose. What I've done in the
attached first-draft patch is just to select one relatively generic
code in class 10, 10608 = invalid_argument_for_xquery, and use that
where it seemed apropos.
This is pretty low-priority, so I'll stash it in the next CF.
regards, tom lane
[1] https://www.postgresql.org/message-id/356363.1726333674%40sss.pgh.pa.us
diff --git a/contrib/xml2/xpath.c b/contrib/xml2/xpath.c
index f94b622d92..cc40fa5624 100644
--- a/contrib/xml2/xpath.c
+++ b/contrib/xml2/xpath.c
@@ -388,7 +388,7 @@ pgxml_xpath(text *document, xmlChar *xpath, xpath_workspace *workspace)
/* compile the path */
comppath = xmlXPathCompile(xpath);
if (comppath == NULL)
- xml_ereport(xmlerrcxt, ERROR, ERRCODE_EXTERNAL_ROUTINE_EXCEPTION,
+ xml_ereport(xmlerrcxt, ERROR, ERRCODE_INVALID_ARGUMENT_FOR_XQUERY,
"XPath Syntax Error");
/* Now evaluate the path expression. */
@@ -652,7 +652,7 @@ xpath_table(PG_FUNCTION_ARGS)
comppath = xmlXPathCompile(xpaths[j]);
if (comppath == NULL)
xml_ereport(xmlerrcxt, ERROR,
- ERRCODE_EXTERNAL_ROUTINE_EXCEPTION,
+ ERRCODE_INVALID_ARGUMENT_FOR_XQUERY,
"XPath Syntax Error");
/* Now evaluate the path expression. */
diff --git a/contrib/xml2/xslt_proc.c b/contrib/xml2/xslt_proc.c
index f30a3a42c0..e761ca5cb5 100644
--- a/contrib/xml2/xslt_proc.c
+++ b/contrib/xml2/xslt_proc.c
@@ -90,7 +90,7 @@ xslt_process(PG_FUNCTION_ARGS)
XML_PARSE_NOENT);
if (doctree == NULL)
- xml_ereport(xmlerrcxt, ERROR, ERRCODE_EXTERNAL_ROUTINE_EXCEPTION,
+ xml_ereport(xmlerrcxt, ERROR, ERRCODE_INVALID_XML_DOCUMENT,
"error parsing XML document");
/* Same for stylesheet */
@@ -99,14 +99,14 @@ xslt_process(PG_FUNCTION_ARGS)
XML_PARSE_NOENT);
if (ssdoc == NULL)
- xml_ereport(xmlerrcxt, ERROR, ERRCODE_EXTERNAL_ROUTINE_EXCEPTION,
+ xml_ereport(xmlerrcxt, ERROR, ERRCODE_INVALID_XML_DOCUMENT,
"error parsing stylesheet as XML document");
/* After this call we need not free ssdoc separately */
stylesheet = xsltParseStylesheetDoc(ssdoc);
if (stylesheet == NULL)
- xml_ereport(xmlerrcxt, ERROR, ERRCODE_EXTERNAL_ROUTINE_EXCEPTION,
+ xml_ereport(xmlerrcxt, ERROR, ERRCODE_INVALID_ARGUMENT_FOR_XQUERY,
"failed to parse stylesheet");
xslt_ctxt = xsltNewTransformContext(stylesheet, doctree);
@@ -141,7 +141,7 @@ xslt_process(PG_FUNCTION_ARGS)
NULL, NULL, xslt_ctxt);
if (restree == NULL)
- xml_ereport(xmlerrcxt, ERROR, ERRCODE_EXTERNAL_ROUTINE_EXCEPTION,
+ xml_ereport(xmlerrcxt, ERROR, ERRCODE_INVALID_ARGUMENT_FOR_XQUERY,
"failed to apply stylesheet");
resstat = xsltSaveResultToString(&resstr, &reslen, restree, stylesheet);
diff --git a/src/backend/utils/adt/xml.c b/src/backend/utils/adt/xml.c
index 1a07876cd5..3eaf9f78a8 100644
--- a/src/backend/utils/adt/xml.c
+++ b/src/backend/utils/adt/xml.c
@@ -742,7 +742,7 @@ xmltotext_with_options(xmltype *data, XmlOptionType xmloption_arg, bool indent)
{
/* If it's a document, saving is easy. */
if (xmlSaveDoc(ctxt, doc) == -1 || xmlerrcxt->err_occurred)
- xml_ereport(xmlerrcxt, ERROR, ERRCODE_INTERNAL_ERROR,
+ xml_ereport(xmlerrcxt, ERROR, ERRCODE_OUT_OF_MEMORY,
"could not save document to xmlBuffer");
}
else if (content_nodes != NULL)
@@ -785,7 +785,7 @@ xmltotext_with_options(xmltype *data, XmlOptionType xmloption_arg, bool indent)
if (xmlSaveTree(ctxt, newline) == -1 || xmlerrcxt->err_occurred)
{
xmlFreeNode(newline);
- xml_ereport(xmlerrcxt, ERROR, ERRCODE_INTERNAL_ERROR,
+ xml_ereport(xmlerrcxt, ERROR, ERRCODE_OUT_OF_MEMORY,
"could not save newline to xmlBuffer");
}
}
@@ -793,7 +793,7 @@ xmltotext_with_options(xmltype *data, XmlOptionType xmloption_arg, bool indent)
if (xmlSaveTree(ctxt, node) == -1 || xmlerrcxt->err_occurred)
{
xmlFreeNode(newline);
- xml_ereport(xmlerrcxt, ERROR, ERRCODE_INTERNAL_ERROR,
+ xml_ereport(xmlerrcxt, ERROR, ERRCODE_OUT_OF_MEMORY,
"could not save content to xmlBuffer");
}
}
@@ -1004,7 +1004,7 @@ xmlpi(const char *target, text *arg, bool arg_is_null, bool *result_is_null)
if (pg_strcasecmp(target, "xml") == 0)
ereport(ERROR,
- (errcode(ERRCODE_SYNTAX_ERROR), /* really */
+ (errcode(ERRCODE_INVALID_XML_PROCESSING_INSTRUCTION),
errmsg("invalid XML processing instruction"),
errdetail("XML processing instruction target name cannot be \"%s\".", target)));
@@ -4383,7 +4383,7 @@ xpath_internal(text *xpath_expr_text, xmltype *data, ArrayType *namespaces,
xpath_len = VARSIZE_ANY_EXHDR(xpath_expr_text);
if (xpath_len == 0)
ereport(ERROR,
- (errcode(ERRCODE_DATA_EXCEPTION),
+ (errcode(ERRCODE_INVALID_ARGUMENT_FOR_XQUERY),
errmsg("empty XPath expression")));
string = pg_xmlCharStrndup(datastr, len);
@@ -4450,7 +4450,7 @@ xpath_internal(text *xpath_expr_text, xmltype *data, ArrayType *namespaces,
xpathcomp = xmlXPathCompile(xpath_expr);
if (xpathcomp == NULL || xmlerrcxt->err_occurred)
- xml_ereport(xmlerrcxt, ERROR, ERRCODE_INTERNAL_ERROR,
+ xml_ereport(xmlerrcxt, ERROR, ERRCODE_INVALID_ARGUMENT_FOR_XQUERY,
"invalid XPath expression");
/*
@@ -4462,7 +4462,7 @@ xpath_internal(text *xpath_expr_text, xmltype *data, ArrayType *namespaces,
*/
xpathobj = xmlXPathCompiledEval(xpathcomp, xpathctx);
if (xpathobj == NULL || xmlerrcxt->err_occurred)
- xml_ereport(xmlerrcxt, ERROR, ERRCODE_INTERNAL_ERROR,
+ xml_ereport(xmlerrcxt, ERROR, ERRCODE_INVALID_ARGUMENT_FOR_XQUERY,
"could not create XPath object");
/*
@@ -4792,7 +4792,7 @@ XmlTableSetNamespace(TableFuncScanState *state, const char *name, const char *ur
if (xmlXPathRegisterNs(xtCxt->xpathcxt,
pg_xmlCharStrndup(name, strlen(name)),
pg_xmlCharStrndup(uri, strlen(uri))))
- xml_ereport(xtCxt->xmlerrcxt, ERROR, ERRCODE_DATA_EXCEPTION,
+ xml_ereport(xtCxt->xmlerrcxt, ERROR, ERRCODE_INVALID_ARGUMENT_FOR_XQUERY,
"could not set XML namespace");
#else
NO_XML_SUPPORT();
@@ -4814,14 +4814,14 @@ XmlTableSetRowFilter(TableFuncScanState *state, const char *path)
if (*path == '\0')
ereport(ERROR,
- (errcode(ERRCODE_DATA_EXCEPTION),
+ (errcode(ERRCODE_INVALID_ARGUMENT_FOR_XQUERY),
errmsg("row path filter must not be empty string")));
xstr = pg_xmlCharStrndup(path, strlen(path));
xtCxt->xpathcomp = xmlXPathCompile(xstr);
if (xtCxt->xpathcomp == NULL || xtCxt->xmlerrcxt->err_occurred)
- xml_ereport(xtCxt->xmlerrcxt, ERROR, ERRCODE_SYNTAX_ERROR,
+ xml_ereport(xtCxt->xmlerrcxt, ERROR, ERRCODE_INVALID_ARGUMENT_FOR_XQUERY,
"invalid XPath expression");
#else
NO_XML_SUPPORT();
@@ -4845,14 +4845,14 @@ XmlTableSetColumnFilter(TableFuncScanState *state, const char *path, int colnum)
if (*path == '\0')
ereport(ERROR,
- (errcode(ERRCODE_DATA_EXCEPTION),
+ (errcode(ERRCODE_INVALID_ARGUMENT_FOR_XQUERY),
errmsg("column path filter must not be empty string")));
xstr = pg_xmlCharStrndup(path, strlen(path));
xtCxt->xpathscomp[colnum] = xmlXPathCompile(xstr);
if (xtCxt->xpathscomp[colnum] == NULL || xtCxt->xmlerrcxt->err_occurred)
- xml_ereport(xtCxt->xmlerrcxt, ERROR, ERRCODE_DATA_EXCEPTION,
+ xml_ereport(xtCxt->xmlerrcxt, ERROR, ERRCODE_INVALID_ARGUMENT_FOR_XQUERY,
"invalid XPath expression");
#else
NO_XML_SUPPORT();
@@ -4879,7 +4879,7 @@ XmlTableFetchRow(TableFuncScanState *state)
{
xtCxt->xpathobj = xmlXPathCompiledEval(xtCxt->xpathcomp, xtCxt->xpathcxt);
if (xtCxt->xpathobj == NULL || xtCxt->xmlerrcxt->err_occurred)
- xml_ereport(xtCxt->xmlerrcxt, ERROR, ERRCODE_INTERNAL_ERROR,
+ xml_ereport(xtCxt->xmlerrcxt, ERROR, ERRCODE_INVALID_ARGUMENT_FOR_XQUERY,
"could not create XPath object");
xtCxt->row_count = 0;
@@ -4943,7 +4943,7 @@ XmlTableGetValue(TableFuncScanState *state, int colnum,
/* Evaluate column path */
xpathobj = xmlXPathCompiledEval(xtCxt->xpathscomp[colnum], xtCxt->xpathcxt);
if (xpathobj == NULL || xtCxt->xmlerrcxt->err_occurred)
- xml_ereport(xtCxt->xmlerrcxt, ERROR, ERRCODE_INTERNAL_ERROR,
+ xml_ereport(xtCxt->xmlerrcxt, ERROR, ERRCODE_INVALID_ARGUMENT_FOR_XQUERY,
"could not create XPath object");
/*
diff --git a/src/backend/utils/errcodes.txt b/src/backend/utils/errcodes.txt
index b43a24d4bc..97d91eb1e9 100644
--- a/src/backend/utils/errcodes.txt
+++ b/src/backend/utils/errcodes.txt
@@ -141,6 +141,12 @@ Section: Class 0Z - Diagnostics Exception
0Z000 E ERRCODE_DIAGNOSTICS_EXCEPTION diagnostics_exception
0Z002 E ERRCODE_STACKED_DIAGNOSTICS_ACCESSED_WITHOUT_ACTIVE_HANDLER
stacked_diagnostics_accessed_without_active_handler
+Section: Class 10 - XQuery Error
+
+# recent SQL versions define quite a few codes in this class, but for now
+# we are only using this generic one
+10608 E ERRCODE_INVALID_ARGUMENT_FOR_XQUERY invalid_argument_for_xquery
+
Section: Class 20 - Case Not Found
20000 E ERRCODE_CASE_NOT_FOUND case_not_found
I wrote:
> [ v1-clean-up-errcodes-for-xml.patch ]
Per cfbot, rebased over d5622acb3. No functional changes.
regards, tom lane
diff --git a/contrib/xml2/xpath.c b/contrib/xml2/xpath.c
index 0fdf735faf..ef78aa00c8 100644
--- a/contrib/xml2/xpath.c
+++ b/contrib/xml2/xpath.c
@@ -388,7 +388,7 @@ pgxml_xpath(text *document, xmlChar *xpath, xpath_workspace *workspace)
/* compile the path */
comppath = xmlXPathCtxtCompile(workspace->ctxt, xpath);
if (comppath == NULL)
- xml_ereport(xmlerrcxt, ERROR, ERRCODE_EXTERNAL_ROUTINE_EXCEPTION,
+ xml_ereport(xmlerrcxt, ERROR, ERRCODE_INVALID_ARGUMENT_FOR_XQUERY,
"XPath Syntax Error");
/* Now evaluate the path expression. */
@@ -652,7 +652,7 @@ xpath_table(PG_FUNCTION_ARGS)
comppath = xmlXPathCtxtCompile(ctxt, xpaths[j]);
if (comppath == NULL)
xml_ereport(xmlerrcxt, ERROR,
- ERRCODE_EXTERNAL_ROUTINE_EXCEPTION,
+ ERRCODE_INVALID_ARGUMENT_FOR_XQUERY,
"XPath Syntax Error");
/* Now evaluate the path expression. */
diff --git a/contrib/xml2/xslt_proc.c b/contrib/xml2/xslt_proc.c
index f30a3a42c0..e761ca5cb5 100644
--- a/contrib/xml2/xslt_proc.c
+++ b/contrib/xml2/xslt_proc.c
@@ -90,7 +90,7 @@ xslt_process(PG_FUNCTION_ARGS)
XML_PARSE_NOENT);
if (doctree == NULL)
- xml_ereport(xmlerrcxt, ERROR, ERRCODE_EXTERNAL_ROUTINE_EXCEPTION,
+ xml_ereport(xmlerrcxt, ERROR, ERRCODE_INVALID_XML_DOCUMENT,
"error parsing XML document");
/* Same for stylesheet */
@@ -99,14 +99,14 @@ xslt_process(PG_FUNCTION_ARGS)
XML_PARSE_NOENT);
if (ssdoc == NULL)
- xml_ereport(xmlerrcxt, ERROR, ERRCODE_EXTERNAL_ROUTINE_EXCEPTION,
+ xml_ereport(xmlerrcxt, ERROR, ERRCODE_INVALID_XML_DOCUMENT,
"error parsing stylesheet as XML document");
/* After this call we need not free ssdoc separately */
stylesheet = xsltParseStylesheetDoc(ssdoc);
if (stylesheet == NULL)
- xml_ereport(xmlerrcxt, ERROR, ERRCODE_EXTERNAL_ROUTINE_EXCEPTION,
+ xml_ereport(xmlerrcxt, ERROR, ERRCODE_INVALID_ARGUMENT_FOR_XQUERY,
"failed to parse stylesheet");
xslt_ctxt = xsltNewTransformContext(stylesheet, doctree);
@@ -141,7 +141,7 @@ xslt_process(PG_FUNCTION_ARGS)
NULL, NULL, xslt_ctxt);
if (restree == NULL)
- xml_ereport(xmlerrcxt, ERROR, ERRCODE_EXTERNAL_ROUTINE_EXCEPTION,
+ xml_ereport(xmlerrcxt, ERROR, ERRCODE_INVALID_ARGUMENT_FOR_XQUERY,
"failed to apply stylesheet");
resstat = xsltSaveResultToString(&resstr, &reslen, restree, stylesheet);
diff --git a/src/backend/utils/adt/xml.c b/src/backend/utils/adt/xml.c
index 41b1a5c6b0..2654c2d7ff 100644
--- a/src/backend/utils/adt/xml.c
+++ b/src/backend/utils/adt/xml.c
@@ -742,7 +742,7 @@ xmltotext_with_options(xmltype *data, XmlOptionType xmloption_arg, bool indent)
{
/* If it's a document, saving is easy. */
if (xmlSaveDoc(ctxt, doc) == -1 || xmlerrcxt->err_occurred)
- xml_ereport(xmlerrcxt, ERROR, ERRCODE_INTERNAL_ERROR,
+ xml_ereport(xmlerrcxt, ERROR, ERRCODE_OUT_OF_MEMORY,
"could not save document to xmlBuffer");
}
else if (content_nodes != NULL)
@@ -785,7 +785,7 @@ xmltotext_with_options(xmltype *data, XmlOptionType xmloption_arg, bool indent)
if (xmlSaveTree(ctxt, newline) == -1 || xmlerrcxt->err_occurred)
{
xmlFreeNode(newline);
- xml_ereport(xmlerrcxt, ERROR, ERRCODE_INTERNAL_ERROR,
+ xml_ereport(xmlerrcxt, ERROR, ERRCODE_OUT_OF_MEMORY,
"could not save newline to xmlBuffer");
}
}
@@ -793,7 +793,7 @@ xmltotext_with_options(xmltype *data, XmlOptionType xmloption_arg, bool indent)
if (xmlSaveTree(ctxt, node) == -1 || xmlerrcxt->err_occurred)
{
xmlFreeNode(newline);
- xml_ereport(xmlerrcxt, ERROR, ERRCODE_INTERNAL_ERROR,
+ xml_ereport(xmlerrcxt, ERROR, ERRCODE_OUT_OF_MEMORY,
"could not save content to xmlBuffer");
}
}
@@ -1004,7 +1004,7 @@ xmlpi(const char *target, text *arg, bool arg_is_null, bool *result_is_null)
if (pg_strcasecmp(target, "xml") == 0)
ereport(ERROR,
- (errcode(ERRCODE_SYNTAX_ERROR), /* really */
+ (errcode(ERRCODE_INVALID_XML_PROCESSING_INSTRUCTION),
errmsg("invalid XML processing instruction"),
errdetail("XML processing instruction target name cannot be \"%s\".", target)));
@@ -4383,7 +4383,7 @@ xpath_internal(text *xpath_expr_text, xmltype *data, ArrayType *namespaces,
xpath_len = VARSIZE_ANY_EXHDR(xpath_expr_text);
if (xpath_len == 0)
ereport(ERROR,
- (errcode(ERRCODE_DATA_EXCEPTION),
+ (errcode(ERRCODE_INVALID_ARGUMENT_FOR_XQUERY),
errmsg("empty XPath expression")));
string = pg_xmlCharStrndup(datastr, len);
@@ -4456,7 +4456,7 @@ xpath_internal(text *xpath_expr_text, xmltype *data, ArrayType *namespaces,
*/
xpathcomp = xmlXPathCtxtCompile(xpathctx, xpath_expr);
if (xpathcomp == NULL || xmlerrcxt->err_occurred)
- xml_ereport(xmlerrcxt, ERROR, ERRCODE_INTERNAL_ERROR,
+ xml_ereport(xmlerrcxt, ERROR, ERRCODE_INVALID_ARGUMENT_FOR_XQUERY,
"invalid XPath expression");
/*
@@ -4468,7 +4468,7 @@ xpath_internal(text *xpath_expr_text, xmltype *data, ArrayType *namespaces,
*/
xpathobj = xmlXPathCompiledEval(xpathcomp, xpathctx);
if (xpathobj == NULL || xmlerrcxt->err_occurred)
- xml_ereport(xmlerrcxt, ERROR, ERRCODE_INTERNAL_ERROR,
+ xml_ereport(xmlerrcxt, ERROR, ERRCODE_INVALID_ARGUMENT_FOR_XQUERY,
"could not create XPath object");
/*
@@ -4798,7 +4798,7 @@ XmlTableSetNamespace(TableFuncScanState *state, const char *name, const char *ur
if (xmlXPathRegisterNs(xtCxt->xpathcxt,
pg_xmlCharStrndup(name, strlen(name)),
pg_xmlCharStrndup(uri, strlen(uri))))
- xml_ereport(xtCxt->xmlerrcxt, ERROR, ERRCODE_DATA_EXCEPTION,
+ xml_ereport(xtCxt->xmlerrcxt, ERROR, ERRCODE_INVALID_ARGUMENT_FOR_XQUERY,
"could not set XML namespace");
#else
NO_XML_SUPPORT();
@@ -4820,7 +4820,7 @@ XmlTableSetRowFilter(TableFuncScanState *state, const char *path)
if (*path == '\0')
ereport(ERROR,
- (errcode(ERRCODE_DATA_EXCEPTION),
+ (errcode(ERRCODE_INVALID_ARGUMENT_FOR_XQUERY),
errmsg("row path filter must not be empty string")));
xstr = pg_xmlCharStrndup(path, strlen(path));
@@ -4830,7 +4830,7 @@ XmlTableSetRowFilter(TableFuncScanState *state, const char *path)
xtCxt->xpathcomp = xmlXPathCtxtCompile(xtCxt->xpathcxt, xstr);
if (xtCxt->xpathcomp == NULL || xtCxt->xmlerrcxt->err_occurred)
- xml_ereport(xtCxt->xmlerrcxt, ERROR, ERRCODE_SYNTAX_ERROR,
+ xml_ereport(xtCxt->xmlerrcxt, ERROR, ERRCODE_INVALID_ARGUMENT_FOR_XQUERY,
"invalid XPath expression");
#else
NO_XML_SUPPORT();
@@ -4854,7 +4854,7 @@ XmlTableSetColumnFilter(TableFuncScanState *state, const char *path, int colnum)
if (*path == '\0')
ereport(ERROR,
- (errcode(ERRCODE_DATA_EXCEPTION),
+ (errcode(ERRCODE_INVALID_ARGUMENT_FOR_XQUERY),
errmsg("column path filter must not be empty string")));
xstr = pg_xmlCharStrndup(path, strlen(path));
@@ -4864,7 +4864,7 @@ XmlTableSetColumnFilter(TableFuncScanState *state, const char *path, int colnum)
xtCxt->xpathscomp[colnum] = xmlXPathCtxtCompile(xtCxt->xpathcxt, xstr);
if (xtCxt->xpathscomp[colnum] == NULL || xtCxt->xmlerrcxt->err_occurred)
- xml_ereport(xtCxt->xmlerrcxt, ERROR, ERRCODE_DATA_EXCEPTION,
+ xml_ereport(xtCxt->xmlerrcxt, ERROR, ERRCODE_INVALID_ARGUMENT_FOR_XQUERY,
"invalid XPath expression");
#else
NO_XML_SUPPORT();
@@ -4891,7 +4891,7 @@ XmlTableFetchRow(TableFuncScanState *state)
{
xtCxt->xpathobj = xmlXPathCompiledEval(xtCxt->xpathcomp, xtCxt->xpathcxt);
if (xtCxt->xpathobj == NULL || xtCxt->xmlerrcxt->err_occurred)
- xml_ereport(xtCxt->xmlerrcxt, ERROR, ERRCODE_INTERNAL_ERROR,
+ xml_ereport(xtCxt->xmlerrcxt, ERROR, ERRCODE_INVALID_ARGUMENT_FOR_XQUERY,
"could not create XPath object");
xtCxt->row_count = 0;
@@ -4955,7 +4955,7 @@ XmlTableGetValue(TableFuncScanState *state, int colnum,
/* Evaluate column path */
xpathobj = xmlXPathCompiledEval(xtCxt->xpathscomp[colnum], xtCxt->xpathcxt);
if (xpathobj == NULL || xtCxt->xmlerrcxt->err_occurred)
- xml_ereport(xtCxt->xmlerrcxt, ERROR, ERRCODE_INTERNAL_ERROR,
+ xml_ereport(xtCxt->xmlerrcxt, ERROR, ERRCODE_INVALID_ARGUMENT_FOR_XQUERY,
"could not create XPath object");
/*
diff --git a/src/backend/utils/errcodes.txt b/src/backend/utils/errcodes.txt
index b43a24d4bc..97d91eb1e9 100644
--- a/src/backend/utils/errcodes.txt
+++ b/src/backend/utils/errcodes.txt
@@ -141,6 +141,12 @@ Section: Class 0Z - Diagnostics Exception
0Z000 E ERRCODE_DIAGNOSTICS_EXCEPTION diagnostics_exception
0Z002 E ERRCODE_STACKED_DIAGNOSTICS_ACCESSED_WITHOUT_ACTIVE_HANDLER
stacked_diagnostics_accessed_without_active_handler
+Section: Class 10 - XQuery Error
+
+# recent SQL versions define quite a few codes in this class, but for now
+# we are only using this generic one
+10608 E ERRCODE_INVALID_ARGUMENT_FOR_XQUERY invalid_argument_for_xquery
+
Section: Class 20 - Case Not Found
20000 E ERRCODE_CASE_NOT_FOUND case_not_found
> On 20 Sep 2024, at 21:00, Tom Lane <tgl@sss.pgh.pa.us> wrote: > > I wrote: >> [ v1-clean-up-errcodes-for-xml.patch ] > > Per cfbot, rebased over d5622acb3. No functional changes. Looking over these I don't see anything mis-characterized so +1 on going ahead with these. It would be neat if we end up translating xml2 errors into XQuery Error SQLSTATEs but this is a clear improvement over what we have until then. There is an ERRCODE_INTERNAL_ERROR in xml_out_internal() which seems a tad odd given that any error would be known to be parsing related and b) are caused by libxml and not internally. Not sure if it's worth bothering with but with the other ones improved it stood out. -- Daniel Gustafsson
Daniel Gustafsson <daniel@yesql.se> writes:
>> On 20 Sep 2024, at 21:00, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Per cfbot, rebased over d5622acb3. No functional changes.
> Looking over these I don't see anything mis-characterized so +1 on going ahead
> with these. It would be neat if we end up translating xml2 errors into XQuery
> Error SQLSTATEs but this is a clear improvement over what we have until then.
Thanks for looking at it!
> There is an ERRCODE_INTERNAL_ERROR in xml_out_internal() which seems a tad odd
> given that any error would be known to be parsing related and b) are caused by
> libxml and not internally. Not sure if it's worth bothering with but with the
> other ones improved it stood out.
Yeah, I looked at that but wasn't sure what to do with it. We should
have validated the decl header when the XML value was created, so if
we get here then either the value got corrupted on-disk or in-transit,
or something forgot to do that validation, or libxml has changed its
mind since then about what's a valid decl. At least some of those
cases are probably legitimately called INTERNAL_ERROR. I thought for
awhile about ERRCODE_DATA_CORRUPTED, but I'm not convinced that's a
better fit.
regards, tom lane
> On 23 Sep 2024, at 19:17, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Daniel Gustafsson <daniel@yesql.se> writes: > >> There is an ERRCODE_INTERNAL_ERROR in xml_out_internal() which seems a tad odd >> given that any error would be known to be parsing related and b) are caused by >> libxml and not internally. Not sure if it's worth bothering with but with the >> other ones improved it stood out. > > Yeah, I looked at that but wasn't sure what to do with it. We should > have validated the decl header when the XML value was created, so if > we get here then either the value got corrupted on-disk or in-transit, > or something forgot to do that validation, or libxml has changed its > mind since then about what's a valid decl. At least some of those > cases are probably legitimately called INTERNAL_ERROR. I thought for > awhile about ERRCODE_DATA_CORRUPTED, but I'm not convinced that's a > better fit. I agree that it might not be an obvious better fit, but also not an obvious worse fit. It will make it easier to filter on during fleet analysis so I would be inclined to change it, but the main value of the patch are other hunks so feel free to ignore. -- Daniel Gustafsson
Daniel Gustafsson <daniel@yesql.se> writes:
> On 23 Sep 2024, at 19:17, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Yeah, I looked at that but wasn't sure what to do with it. We should
>> have validated the decl header when the XML value was created, so if
>> we get here then either the value got corrupted on-disk or in-transit,
>> or something forgot to do that validation, or libxml has changed its
>> mind since then about what's a valid decl. At least some of those
>> cases are probably legitimately called INTERNAL_ERROR. I thought for
>> awhile about ERRCODE_DATA_CORRUPTED, but I'm not convinced that's a
>> better fit.
> I agree that it might not be an obvious better fit, but also not an obvious
> worse fit. It will make it easier to filter on during fleet analysis so I
> would be inclined to change it, but the main value of the patch are other hunks
> so feel free to ignore.
Fair enough. Pushed with ERRCODE_DATA_CORRUPTED used there.
Thanks again for reviewing.
regards, tom lane