Re: [PATCH] Add pretty-printed XML output option

On 09.03.23 21:21, Tom Lane wrote:
> Peter Smith <smithpb2250@gmail.com> writes:
>> The patch v19 LGTM.
> Another thing that's mildly irking me is that the current
> factorization of this code will result in xml_parse'ing the data
> twice, if you have both DOCUMENT and INDENT specified.  We could
> consider avoiding that if we merged the indentation functionality
> into xmltotext_with_xmloption, but it's probably premature to do so
> when we haven't figured out how to get the output right --- we might
> end up needing two xml_parse calls anyway with different parameters,
> perhaps.

Just a thought: since xmlserialize_indent also calls xml_parse() to 
build the xmlDocPtr, couldn't we simply bypass 
xmltotext_with_xmloption() in case of INDENT is specified?

Something like this:

diff --git a/src/backend/executor/execExprInterp.c 
b/src/backend/executor/execExprInterp.c
index 19351fe..ea808dd 100644
--- a/src/backend/executor/execExprInterp.c
+++ b/src/backend/executor/execExprInterp.c
@@ -3829,6 +3829,7 @@ ExecEvalXmlExpr(ExprState *state, ExprEvalStep *op)
         {
                 Datum      *argvalue = op->d.xmlexpr.argvalue;
                 bool       *argnull = op->d.xmlexpr.argnull;
+                               text       *result;

                 /* argument type is known to be xml */
                 Assert(list_length(xexpr->args) == 1);
@@ -3837,8 +3838,14 @@ ExecEvalXmlExpr(ExprState *state, ExprEvalStep *op)
                         return;
                 value = argvalue[0];

-                               *op->resvalue = 
PointerGetDatum(xmltotext_with_xmloption(DatumGetXmlP(value),
- xexpr->xmloption));
+                               if (xexpr->indent)
+                                       result = 
xmlserialize_indent(DatumGetXmlP(value),
+ xexpr->xmloption);
+                               else
+                                       result = 
xmltotext_with_xmloption(DatumGetXmlP(value),
+ xexpr->xmloption);
+
+                               *op->resvalue = PointerGetDatum(result);
                 *op->resnull = false;
         }

         break;