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

On 09.03.23 21:21, Tom Lane wrote:
> Peter Smith <smithpb2250(at)gmail(dot)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;