Add WITH XMLNAMESPACES support

[JapleenKaur3]

Description

Currently, Babelfish rejects WITH XMLNAMESPACES with an "unsupported" error. With this change, it is now fully supported for FOR XML (RAW, PATH, AUTO) and XML data type methods (.query, .value, .exist).

The ANTLR visitor parses the namespace declarations and builds two representations: a text[][] array literal passed to xpath() for XML methods, and a xmlns:prefix="uri" declaration string passed as a new argument to the FOR XML aggregate. The aggregate emits declarations on the appropriate XML element (ROOT when present, otherwise each row tag), and a restore_xml_prefix_colon helper in forxml.c decodes PG's identifier encoding so prefixed aliases render correctly. New 3-argument overloads of bbf_xmlquery, bbf_xmlexist, bbf_xmlvalue forward the namespace array to PG's xpath(). Validation enforces T-SQL rules: no duplicate prefixes, no empty URIs, reserved prefix rejection, xsi deduplication with ELEMENTS XSINIL, and proper XML-escaping of URIs containing &/</>.

Authored-by: Japleen Kaur amjj@amazon.com
Signed-off-by: Japleen Kaur amjj@amazon.com

Issues Resolved

BABEL-2514

Test Scenarios Covered

• Use case based - FOR XML RAW/PATH/AUTO with prefixed row names, column aliases, DEFAULT namespace, multiple prefixes, ELEMENTS/XSINIL/ROOT/TYPE, JOINs, CTEs, subqueries, UNION, GROUP BY/ORDER BY/TOP; .query/.value/.exist with prefixed XPath on variables and table columns, positional access, attribute predicates, chained method calls

• Boundary conditions - Empty result sets, all-NULL rows, long URIs, URIs with special characters, Unicode values, multiple data types

• Arbitrary inputs - Covered under boundary conditions (various data types, Unicode values, special characters in values and URIs)

• Negative test cases - Duplicate prefix, multiple DEFAULT, reserved xmlns/xml prefixes, empty URI

• Minor version upgrade tests - Added to upgrade/latest schedule

• Major version upgrade tests - NA

• Performance tests - NA

• Tooling impact - NA

• Client tests - NA

Check List

• Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is under the terms of the Apache 2.0 and PostgreSQL licenses, and grant any person obtaining a copy of the contribution permission to relicense all or a portion of my contribution to the PostgreSQL License solely to contribute all or a portion of my contribution to the PostgreSQL open source project.

For more information on following Developer Certificate of Origin and signing off your commits, please check here.

[harshdubey166]

/code-review

[kuntalghosh]

AI Code Review

Verdict: REQUEST_CHANGES

Based on the provided code changes, here are my feedback and suggestions:

Issue 1 - In tsqlIface.cpp at line ~824: URI containing double-quotes breaks the PG text array literal:

result += "\"\"," "\"" + uri + "\"";
// and later:
result += "\"" + prefix + "\",\"" + uri + "\"";

build_xmlnamespace_array_literal constructs a PostgreSQL text[][] array literal by embedding URIs directly between double-quote delimiters. If a URI contains a " character (which is legal inside a T-SQL single-quoted string, e.g. 'http://example.com/"path"'), the generated literal becomes malformed — the inner quote prematurely terminates the array element. This can cause a parse error at query execution or, worse, result in a URI being silently truncated.

The fix should escape " as \" (the backslash-escape form accepted inside PG array literals) and also escape \ as \\ for round-trip safety. Similarly, since the array literal is wrapped in single-quotes as a SQL text constant, any single-quote in the URI needs to be doubled ('').

Suggested fix:

// Add a helper:
static std::string escape_for_pg_array_literal(const std::string &s)
{
    std::string out;
    out.reserve(s.size());
    for (char c : s)
    {
        if (c == '"' || c == '\\')
            out += '\\';
        out += c;
    }
    return out;
}

// Then use it when building the literal:
result += "\"" + escape_for_pg_array_literal(prefix) + "\",\""
        + escape_for_pg_array_literal(uri) + "\"";

Also escape any ' in the URI by doubling it, since the outer wrapper is a SQL single-quoted string ('{{...}}'::_text):

// Replace single quotes in the full result string before appending the cast:
// Or better: escape ' -> '' in each uri before insertion.

Issue 2 - In tsqlIface.cpp at line ~984: build_xmlnamespace_decls_string comment says xsi is skipped but code does not skip it:

/*
 * The xsi prefix bound to http://www.w3.org/2001/XMLSchema-instance is
 * skipped because XSINIL handling emits xmlns:xsi separately on each row;
 * SQL Server avoids redeclaring it when both are specified.
 */
static std::string
build_xmlnamespace_decls_string(...)
{
    // ... no xsi-skip logic present ...

The xsi deduplication is actually handled correctly downstream in forxml.c via ns_decls_has_xsi(), which detects that xsi is already in ns_decls and skips adding the XSINIL declaration separately. The comment in build_xmlnamespace_decls_string is misleading — it claims this function skips xsi, when in fact it emits xsi and the deduplication happens elsewhere. This will confuse future maintainers.

Suggested fix: Update the comment to accurately reflect the design:

/*
 * Note: The xsi prefix (if declared) IS included in the output string.
 * Deduplication with XSINIL's xmlns:xsi declaration is handled downstream
 * in forxml.c by ns_decls_has_xsi(), which detects xsi in ns_decls and
 * suppresses the redundant XSINIL declaration.
 */

Issue 3 - In forxml.c at line ~266: restore_xml_prefix_colon palloc size may be insufficient when decoded prefix is longer than raw prefix:

result = palloc(strlen(colname) + 1);
write = result;
if (prefix_declared)
{
    memcpy(write, decoded_prefix.data, decoded_prefix.len);
    write += decoded_prefix.len;
}

The buffer is allocated as strlen(colname) + 1. The output replaces _x003A_ (7 chars) with : (1 char), so the suffix always shrinks. However, the prefix portion can theoretically grow: the raw prefix includes _xNNNN_ escape sequences (7 chars each), but decoded_prefix can expand if there are non-ASCII multi-byte representations... actually no — the decode only produces single-byte ASCII (code < 128). And the raw prefix with escapes is always >= decoded length. So this is safe. (No fix needed — noting for clarity.)

Issue 4 - New tests not added to upgrade/latest/schedule:

The PR introduces forxml-namespaces-vu-* and xml-methods-namespaces-vu-* test files (using the -vu- version-upgrade pattern), and adds upgrade SQL in babelfishpg_tsql--5.6.0--5.7.0.sql. However, these tests are NOT listed in test/JDBC/upgrade/latest/schedule. Other existing forxml tests (forxml, forxml-path, forxml-auto, etc.) are already present in that schedule. Without adding the new tests, minor version upgrade testing won't exercise the new namespace-aware function overloads and aggregate signature changes.

Suggested fix: Add the following entries to test/JDBC/upgrade/latest/schedule:

forxml-namespaces
xml-methods-namespaces

Issue 5 - In tsqlIface.cpp: build_xmlnamespace_array_literal is called multiple times for the same declaration list in enterAnother_statement:

xmlnamespace_array_literal = build_xmlnamespace_array_literal(decls);
xmlnamespace_decls_for_forxml = build_xmlnamespace_decls_string(decls);
// ... later after clear_rewritten_query_fragment:
xmlnamespace_array_literal = build_xmlnamespace_array_literal(decls);
xmlnamespace_decls_for_forxml = build_xmlnamespace_decls_string(decls);

This rebuilds the same string and re-validates (throwing on duplicates etc.) three times for the same declaration list within enterAnother_statement. It also sets xmlnamespace_declared_prefixes as a side-effect of build_xmlnamespace_array_literal, which is fragile. Consider computing once and saving the result, then restoring after clear_rewritten_query_fragment from the saved copy. This is advisory, not blocking — correctness is maintained.

Missing Tests

• No test for a URI containing a double-quote character (related to Issue 1).
• No test for a URI containing a backslash character.
• No test for a URI containing a single-quote (escaped as '' in T-SQL: 'http://example.com/it''s').

Notes

• The PR description is thorough and well-written; BABEL-2514 ticket is linked.
• Target branch BABEL_5_X_DEV is correct for a new feature.
• Test coverage is extensive: FOR XML RAW/PATH/AUTO with prefixes, ELEMENTS/XSINIL/ROOT/TYPE, JOINs, CTEs, subqueries, XML methods (.query/.value/.exist), error cases, and boundary conditions.
• The Test-Spatial_Index entry added to the upgrade schedule appears unrelated to this PR and may be from the base branch.
• The xsi deduplication behavior in forxml.c (Issue 2 comment fix) is functionally correct — no output bug exists.

Note: This AI review is meant to augment, not replace, human review. Use the suggestions with discretion.

[kuntalghosh]

AI Code Review

Verdict: REQUEST_CHANGES

Based on the provided code changes, here are my feedback and suggestions:

Issue 1 - In tsqlIface.cpp at line ~902: URI containing double-quotes or backslashes breaks the PG text array literal:

result += "\"" + prefix + "\",\"" + uri + "\"";
// and for DEFAULT:
result += "\"\"," "\"" + uri + "\"";

build_xmlnamespace_array_literal constructs a PostgreSQL text[][] array literal by embedding URIs directly between double-quote delimiters. If a URI contains a " character (which is legal inside a T-SQL single-quoted string, e.g. 'http://example.com/"path"'), the generated literal becomes malformed — the inner quote prematurely terminates the array element. This can cause a parse error at query execution or, worse, result in a URI being silently truncated. The same applies to \ which has escape significance in PG array literal parsing.

Additionally, since the array literal is wrapped in single-quotes as a SQL text constant (the result ends with }}'::_text), any single-quote in the URI (e.g. 'http://example.com/it''s' in T-SQL) needs to be doubled ('') in the output.

Suggested fix:

// Add a helper:
static std::string escape_for_pg_array_literal(const std::string &s)
{
    std::string out;
    out.reserve(s.size());
    for (char c : s)
    {
        if (c == '"' || c == '\\')
            out += '\\';
        out += c;
    }
    return out;
}

// Then use it when building the literal:
result += "\"" + escape_for_pg_array_literal(prefix) + "\",\""
        + escape_for_pg_array_literal(uri) + "\"";

// Also escape single-quotes in the final result (or per-uri) for the SQL literal wrapper.

Issue 2 - In tsqlIface.cpp at line ~944: Comment claims xsi is skipped but code emits all prefixes including xsi:

/*
 * The xsi prefix bound to http://www.w3.org/2001/XMLSchema-instance is
 * skipped because XSINIL handling emits xmlns:xsi separately on each row;
 * SQL Server avoids redeclaring it when both are specified.
 */
static std::string
build_xmlnamespace_decls_string(...)
{
    // ... iterates ALL declarations, no xsi check ...

The code iterates all declarations without any xsi-skip logic. The xsi deduplication is actually handled correctly downstream in forxml.c via ns_decls_has_xsi(), which detects that xsi is already present in ns_decls and suppresses the redundant XSINIL declaration. The comment is misleading and will confuse future maintainers.

Suggested fix:

/*
 * Note: The xsi prefix (if declared) IS included in the output string.
 * Deduplication with XSINIL's xmlns:xsi declaration is handled downstream
 * in forxml.c by ns_decls_has_xsi(), which detects xsi in ns_decls and
 * suppresses the redundant XSINIL declaration.
 */

Issue 3 - New tests not added to test/JDBC/upgrade/latest/schedule:

The PR introduces forxml-namespaces-vu-* and xml-methods-namespaces-vu-* test files (using the -vu- version-upgrade pattern), and adds upgrade SQL in babelfishpg_tsql--5.6.0--5.7.0.sql. However, these tests are NOT listed in test/JDBC/upgrade/latest/schedule. Other existing forxml tests (forxml, forxml-path, forxml-auto, etc.) are already present in that schedule at lines 227–228 and 683–686. Without adding the new tests, minor version upgrade testing won't exercise the new namespace-aware function overloads and aggregate signature changes.

Suggested fix: Add the following entries to test/JDBC/upgrade/latest/schedule:

forxml-namespaces
xml-methods-namespaces

Issue 4 - In forxml.c: Aggregate function callable by any authenticated user bypassing parser validation:

/* In tsql_query_to_xml_sfunc: */
if (PG_NARGS() > 9 && !PG_ARGISNULL(9))
{
    char *raw = text_to_cstring(PG_GETARG_TEXT_PP(9));
    if (raw[0] != '\0')
        ns_decls = raw;
}
// ... ns_decls later interpolated verbatim via appendStringInfo("<%s %s>", ...)

The ns_decls string is interpolated directly into XML output via appendStringInfo calls (in root-tag, row-tag, and level-1 element emission across RAW/PATH/AUTO modes). The C code trusts that the ANTLR parser layer has produced a well-formed xmlns:NCName="..." string. However, sys.tsql_select_for_xml_agg(...) is a public aggregate — any authenticated user can call it directly with arbitrary text in the ns_decls argument, bypassing the parser's validation and XML-escaping. This allows injecting arbitrary attributes or markup into the returned XML value.

Suggested fix: Either validate ns_decls in the C sfunc (parse as a sequence of xmlns declarations and reject everything else), or add REVOKE EXECUTE on the aggregate functions from PUBLIC in both sys_functions.sql and the upgrade script so only the extension's internal paths can invoke them.

Issue 5 - In forxml.c at line ~170: ns_decls_has_xsi uses naive substring matching on the full ns_decls string:

static bool
ns_decls_has_xsi(const char *ns_decls)
{
    if (ns_decls == NULL || ns_decls[0] == '\0')
        return false;
    return strstr(ns_decls, "xmlns:xsi=") != NULL;
}

This matches against the entire ns_decls string, including URI values. A URI containing the literal text xmlns:xsi= (e.g., 'http://example.com/xmlns:xsi=foo') would cause a false positive, suppressing the XSINIL declaration when it shouldn't be suppressed. The same concern applies to restore_xml_prefix_colon which uses strstr(ns_decls, needle) to check for prefix declarations — a URI body containing xmlns:ns1= would produce a false match.

Suggested fix: Parse ns_decls by tokenizing on known boundaries (the string is space-separated xmlns:*="..." or xmlns="..." entries), or pass a structured list of declared prefixes from the C++ layer to the C layer instead of relying on substring search.

Missing Tests

• No test for a URI containing a double-quote character (related to Issue 1).
• No test for a URI containing a backslash character.
• No test for a URI containing a single-quote (escaped as '' in T-SQL: 'http://example.com/it''s').

Notes

• Prior AI review issues from the earlier revision are re-evaluated here. Issues 1, 2, and 3 from the prior review remain unresolved. Issue 4 (buffer allocation in restore_xml_prefix_colon) was analyzed and confirmed safe — no fix needed. Issue 5 (redundant build_xmlnamespace_array_literal calls in enterAnother_statement) is advisory and remains valid but non-blocking.
• The PR description is thorough and well-written; BABEL-2514 ticket is linked.
• Target branch BABEL_5_X_DEV is correct for a new feature.
• Test coverage is extensive: FOR XML RAW/PATH/AUTO with prefixes, ELEMENTS/XSINIL/ROOT/TYPE, JOINs, CTEs, subqueries, XML methods (.query/.value/.exist), error cases, and boundary conditions.
• The bbf_xmlquery_internal refactoring to a shared static function with the namespace array as a parameter is clean and well-structured.
• The upgrade script correctly drops the old 8-arg aggregate definitions before recreating at 9 args.

Note: This AI review is meant to augment, not replace, human review. Use the suggestions with discretion.

[kuntalghosh]

AI Code Review

Verdict: REQUEST_CHANGES

Based on the provided code changes, here are my feedback and suggestions:

Issue 1 - In forxml.c at line ~165: ns_decls_has_xsi uses naive substring matching that can false-match inside URI values:

static bool
ns_decls_has_xsi(const char *ns_decls)
{
    if (ns_decls == NULL || ns_decls[0] == '\0')
        return false;
    return strstr(ns_decls, "xmlns:xsi=") != NULL;
}

The ns_decls string has the format xmlns:p1="uri1" xmlns:p2="uri2". A URI value may legitimately contain the substring xmlns:xsi= (e.g., WITH XMLNAMESPACES('http://x/?xmlns:xsi=fake' AS n)). The current strstr scan does not distinguish between declaration boundaries and URI content. A false positive would suppress the XSINIL xmlns:xsi binding, producing invalid XML (references to xsi:nil without a binding for the xsi prefix).

The same issue affects restore_xml_prefix_colon at line ~254, where strstr(ns_decls, needle) looks for xmlns:<prefix>= without anchoring to declaration boundaries.

Suggested fix:

static bool
ns_decls_has_xsi(const char *ns_decls)
{
    const char *p;
    if (ns_decls == NULL || ns_decls[0] == '\0')
        return false;
    /*
     * Scan for "xmlns:xsi=" only at declaration boundaries: either at
     * position 0 or preceded by a space (never inside a quoted URI value).
     */
    p = ns_decls;
    while ((p = strstr(p, "xmlns:xsi=")) != NULL)
    {
        if (p == ns_decls || *(p - 1) == ' ')
            return true;
        p++;
    }
    return false;
}

Apply the same anchored-search pattern to restore_xml_prefix_colon's strstr(ns_decls, needle) check.

Issue 2 - In forxml.c at line ~317: ns_decls from aggregate argument interpolated into XML output without validation at the C trust boundary:

if (PG_NARGS() > 9 && !PG_ARGISNULL(9))
{
    char *raw = text_to_cstring(PG_GETARG_TEXT_PP(9));
    if (raw[0] != '\0')
        ns_decls = raw;
}
// ... later:
appendStringInfo(state, "{<%s %s>", root_name, ns_decls);

The sys.tsql_select_for_xml_agg and sys.tsql_select_for_xml_text_agg aggregates are publicly callable (no REVOKE EXECUTE ... FROM PUBLIC). While the ANTLR rewriter produces well-formed namespace declaration strings, any authenticated user can call the aggregate directly with arbitrary text in the ns_decls argument position. That text is then interpolated verbatim into XML element tags across multiple emission paths (root tag, RAW/PATH/AUTO row tags), enabling injection of arbitrary XML attributes or elements into the output.

The typical Babelfish pattern for internal-only aggregate/function paths is either (a) C-side validation of the expected format, or (b) revoking public execute so only the extension's internal paths can invoke the function.

Suggested fix (option A — restrict access):

-- In sys_functions.sql and the upgrade script, after CREATE AGGREGATE:
REVOKE EXECUTE ON AGGREGATE sys.tsql_select_for_xml_agg(
    ANYELEMENT, int, text, boolean, text, boolean, boolean, text, text)
FROM PUBLIC;
REVOKE EXECUTE ON AGGREGATE sys.tsql_select_for_xml_text_agg(
    ANYELEMENT, int, text, boolean, text, boolean, boolean, text, text)
FROM PUBLIC;

Alternatively (option B — validate at C boundary): Parse ns_decls in tsql_query_to_xml_sfunc as a sequence of xmlns[:NCName]="..." tokens and reject anything that doesn't match.

Issue 3 - In forxml.c at line ~286: Memory leak in per-row restore_xml_prefix_colon calls:

colname = map_sql_identifier_to_xml_name(NameStr(att->attname), true, false);
colname = restore_xml_prefix_colon(colname, all_ns_decls);

When restore_xml_prefix_colon returns a new palloc'd buffer (the rewrite path), the original allocation from map_sql_identifier_to_xml_name is leaked. In the per-row paths (tsql_row_to_xml_raw, tsql_row_to_xml_path), this leak is proportional to columns × rows. For large result sets this accumulates over the aggregate context's lifetime.

Suggested fix:

char *orig_colname = map_sql_identifier_to_xml_name(NameStr(att->attname), true, false);
colname = restore_xml_prefix_colon(orig_colname, all_ns_decls);
if (colname != orig_colname)
    pfree(orig_colname);

Missing Tests

• No test for a URI containing a double-quote character (exercises escape_for_pg_array_literal correctness for the array literal path).
• No test for a URI containing a backslash character.
• No test for a URI containing xmlns:xsi= as a substring (exercises ns_decls_has_xsi against false positives).

Notes

• Prior review issues addressed in this revision: Issue 1 (array literal escaping for " and \) is fixed via escape_for_pg_array_literal. Issue 2 (misleading xsi comment) is corrected. Issue 3 (upgrade schedule entries) is resolved — both forxml-namespaces and xml-methods-namespaces now appear in test/JDBC/upgrade/latest/schedule. The advisory issue about redundant build_xmlnamespace_array_literal calls remains (acceptable, correctness maintained).
• The PR description is thorough; BABEL-2514 is linked. Target branch BABEL_5_X_DEV is correct.
• Test coverage is extensive: FOR XML RAW/PATH/AUTO with prefixes, ELEMENTS/XSINIL/ROOT/TYPE, JOINs, CTEs, subqueries, XML methods (.query/.value/.exist), error cases, boundary conditions, and Unicode values.
• The bbf_xmlquery_internal refactoring to a shared static with a namespace parameter is clean.
• The expected_dependency.out update correctly reflects the new 3-arg overloads and the 10-arg sfunc signature.

Note: This AI review is meant to augment, not replace, human review. Use the suggestions with discretion.

[harshdubey166]

/code-review

[kuntalghosh]

AI Code Review

Verdict: APPROVE

Based on the provided code changes, here are my feedback and suggestions:

Prior review issues addressed: All three previous rounds of feedback have been resolved — escape_for_pg_array_literal was added to handle " and \ in URIs, ns_decls_has_xsi was rewritten to use the declaration-walking ns_decls_has_prefix (eliminating false-matches inside URI values), and the misleading comment about xsi-skip was corrected.

Issue 1 (Advisory) - In tsqlIface.cpp at line ~4367: xsi-conflict detection uses substring search on the escaped array literal:

size_t pos = ns_str.find("\"xsi\"");
if (pos != std::string::npos)
{
    /* Found xsi prefix — extract its URI and compare */
    size_t uri_start = ns_str.find("\"", pos + 5) + 1;

The validation that rejects xsi prefix redefinition when ELEMENTS XSINIL is active searches for the literal substring "xsi" in the array literal string. After escape_for_pg_array_literal, a URI containing the characters "xsi" (e.g., 'http://x/"xsi"/y') produces \"xsi\" in the array literal, which contains "xsi" as a subsequence. This could cause a false positive — rejecting a statement that does not actually declare an xsi prefix.

The impact is low (confusing error for an exotic URI, no silent incorrect behavior), but a more robust approach would check against the already-populated xmlnamespace_declared_prefixes set which is authoritative:

Suggested fix:

if (!xmlnamespace_array_literal.empty() && !selectCtx->for_clause()->XSINIL().empty())
{
    if (xmlnamespace_declared_prefixes.count("xsi") > 0)
    {
        /* Check the URI from the original declarations rather than parsing the array literal */
        // ... or use the decls string with ns_decls_has_prefix-style logic
    }
}

Issue 2 (Advisory) - In forxml.c at lines ~760, ~908: per-row memory allocation in restore_xml_prefix_colon within RAW/PATH modes:

colname = restore_xml_prefix_colon(raw_colname, all_ns_decls);
/* restore_xml_prefix_colon may return a new buffer; free the original */
if (colname != raw_colname)
    pfree(raw_colname);

In tsql_row_to_xml_raw and tsql_row_to_xml_path, restore_xml_prefix_colon is called once per column per row. The original raw_colname is freed, but map_sql_identifier_to_xml_name + restore_xml_prefix_colon still allocate fresh memory every row for the same column names. For AUTO mode this was optimized (done once after metadata parsing), but RAW/PATH do it per-row.

For typical result sets this is negligible, but for very large result sets with many prefixed columns it accumulates until the aggregate context is freed. Consider caching restored column names (perhaps in the forxml_state) if performance profiling shows this matters.

Missing Tests

None — the test coverage is comprehensive. Tests cover:

• All three FOR XML modes (RAW, PATH, AUTO) with namespaces
• ELEMENTS, ELEMENTS XSINIL, ROOT, TYPE combinations
• DEFAULT namespace, multiple prefixes, prefixed row names and column aliases
• XML methods (.query, .value, .exist) with namespaces on variables and table columns
• Negative cases (duplicate prefix, invalid characters, reserved prefixes, empty URI, undeclared prefix in alias)
• Edge cases (false-match guard in section 24, double-quotes/backslashes/single-quotes in URIs)
• Minor version upgrade schedule entries

Notes

• The PR description is well-structured, BABEL-2514 is linked, target branch (BABEL_5_X_DEV) is correct.
• The upgrade path (babelfishpg_tsql--5.6.0--5.7.0.sql) correctly drops the 8-arg aggregate and recreates at 9 args, and adds the 3-argument namespace-aware function overloads.
• The PG_NARGS() > 9 backward compatibility logic correctly handles older callers that don't pass the 10th ns_decls argument.
• The restore_xml_prefix_colon buffer allocation is provably safe — all transformations are non-expanding (decoding _xNNNN_ from 7 chars to 1 char, and _x003A_ from 7 chars to 1 char), so palloc(strlen(colname) + 1) is always sufficient.
• The static globals (xmlnamespace_array_literal, xmlnamespace_decls_for_forxml, xmlnamespace_declared_prefixes) are cleared at statement boundaries via clear_rewritten_query_fragment() and correctly saved/restored around graft+makeSQL calls in enterAnother_statement.

Note: This AI review is meant to augment, not replace, human review. Use the suggestions with discretion.