Cleaning up ERRCODE usage in our XML code - Mailing list pgsql-hackers

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