Skip to content

Fix FOR XML RAW('') empty element name handling and all-NULL row output #4680

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
JapleenKaur3 wants to merge 6 commits into
base: BABEL_5_X_DEV
Choose a base branch
from
+142 −24
Open

Fix FOR XML RAW('') empty element name handling and all-NULL row output #4680

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
f29c9e5
all null case
Mar 17, 2026
68ba403
empty element name
Mar 23, 2026
2abd649
edit before file
Mar 23, 2026
f875658
change comment
Mar 26, 2026
4df6fac
adding test cases
Mar 27, 2026
1dbae8b
remove blank line
Mar 27, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
64 changes: 52 additions & 12 deletions contrib/babelfishpg_tsql/src/tsql_for/forxml.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,8 @@ tsql_row_to_xml_raw(StringInfo state, Datum record, const char *element_name, bo
TupleDesc tupdesc;
HeapTupleData tmptup;
HeapTuple tuple;

bool allnull = true;

td = DatumGetHeapTupleHeader(record);

/* Extract rowtype info and find a tupdesc */
Expand All @@ -228,19 +229,34 @@ tsql_row_to_xml_raw(StringInfo state, Datum record, const char *element_name, bo
tmptup.t_data = td;
tuple = &tmptup;

/* Output opening tag */
if (elements)
/*
* Empty element name without ELEMENTS mode is not allowed — attribute-centric
* serialization requires a row tag name.
*/
if (element_name[0] == '\0' && !elements)
{
/* ELEMENTS mode: <row><col>value</col></row> */
if (xsinil)
appendStringInfo(state, "<%s " XML_XMLNS_XSI ">", element_name);
else
appendStringInfo(state, "<%s>", element_name);
ereport(ERROR,
(errcode(ERRCODE_INVALID_XML_PROCESSING_INSTRUCTION),
errmsg("Row tag omission (empty row tag name) cannot be used "
"with attribute-centric FOR XML serialization.")));
}
else

/* Output opening tag (only when element_name is non-empty) */
if (element_name[0] != '\0')
{
/* ATTRIBUTES mode: <row col="value"/> */
appendStringInfo(state, "<%s", element_name);
if (elements)
{
/* ELEMENTS mode: <row><col>value</col></row> */
if (xsinil)
appendStringInfo(state, "<%s " XML_XMLNS_XSI ">", element_name);
else
appendStringInfo(state, "<%s>", element_name);
}
else
{
/* ATTRIBUTES mode: <row col="value"/> */
appendStringInfo(state, "<%s", element_name);
}
}

for (int i = 0; i < tupdesc->natts; i++)
Expand All @@ -265,6 +281,7 @@ tsql_row_to_xml_raw(StringInfo state, Datum record, const char *element_name, bo
/* ELEMENTS mode output */
if (!isnull)
{
allnull = false;
/* Normal element: <col>value</col> */
appendStringInfo(state, "<%s>%s</%s>",
colname,
Expand All @@ -273,6 +290,7 @@ tsql_row_to_xml_raw(StringInfo state, Datum record, const char *element_name, bo
}
else if (xsinil)
{
allnull = false;
/* XSINIL: <col xsi:nil="true"/> */
appendStringInfo(state, "<%s " XML_XSI_NIL "/>", colname);
}
Expand All @@ -292,7 +310,29 @@ tsql_row_to_xml_raw(StringInfo state, Datum record, const char *element_name, bo

/* Output closing tag */
if (elements)
appendStringInfo(state, "</%s>", element_name);
{
if (element_name[0] == '\0')
{
/*
* Empty element name with ELEMENTS: no wrapper tag needed.
* Just output the child elements directly, same as PATH('').
*/
}
else if (allnull)
{
/*
* If all column values are NULL, produce a self-closing element
* like SQL Server does: <row/>. Replace the '>' in the already
* appended opening tag with '/' and append '>'.
*/
state->data[state->len - 1] = '/';
appendStringInfoChar(state, '>');
Comment on lines +328 to +329
Copy link
Copy Markdown
Contributor

@Deepesh125 Deepesh125 Mar 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how is this correct?

Copy link
Copy Markdown
Contributor Author

@JapleenKaur3 JapleenKaur3 Mar 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The opening tag in ELEMENTS mode appends <row> so the buffer ends with >. When all columns are NULL, we need to turn <row> into <row/>

Copy link
Copy Markdown
Contributor

@Deepesh125 Deepesh125 Mar 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I meant why are we updating stringInfo data directly state->data[state->len - 1] = '/';? what is surety that it wouldn't overwrite existing data?

Copy link
Copy Markdown
Contributor Author

@JapleenKaur3 JapleenKaur3 Mar 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This code only runs when allnull is true, so no child elements have been appended after the opening tag. The last character in the buffer at this point is always the > from <row>, so we're only replacing that specific character.
Also, this is the same pattern already used in tsql_row_to_xml_path() for the same all-NULL case

}
else
{
appendStringInfo(state, "</%s>", element_name);
}
}
else
appendStringInfoString(state, "/>");

Expand Down
12 changes: 6 additions & 6 deletions test/JDBC/expected/forxml-raw-elements-before-17_9-or-18_3-vu-verify.out
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ SELECT NULL AS a, NULL AS b FOR XML RAW, ELEMENTS;
GO
~~START~~
ntext
<row></row>
<row/>
~~END~~


Expand All @@ -170,7 +170,7 @@ SELECT NULL AS a, NULL AS b, NULL AS c, NULL AS d FOR XML RAW, ELEMENTS;
GO
~~START~~
ntext
<row></row>
<row/>
~~END~~


Expand Down Expand Up @@ -198,7 +198,7 @@ SELECT NULL AS a, NULL AS b FOR XML RAW, ELEMENTS ABSENT;
GO
~~START~~
ntext
<row></row>
<row/>
~~END~~


Expand Down Expand Up @@ -510,7 +510,7 @@ SELECT name, salary FROM forxml_raw_elements_t1 FOR XML RAW, ELEMENTS;
GO
~~START~~
ntext
<row><name>John</name><salary>50000</salary></row><row><name>Jane</name><salary>60000</salary></row><row><name>Bob</name></row><row><name>Alice</name><salary>70000</salary></row><row></row>
<row><name>John</name><salary>50000</salary></row><row><name>Jane</name><salary>60000</salary></row><row><name>Bob</name></row><row><name>Alice</name><salary>70000</salary></row><row/>
~~END~~


Expand Down Expand Up @@ -950,7 +950,7 @@ SELECT CAST(NULL AS VARCHAR(10)) AS null_val FOR XML RAW, ELEMENTS;
GO
~~START~~
ntext
<row></row>
<row/>
~~END~~


Expand All @@ -971,7 +971,7 @@ SELECT 1 AS a, 2 AS b FOR XML RAW(''), ELEMENTS;
GO
~~START~~
ntext
<><a>1</a><b>2</b></>
<a>1</a><b>2</b>
~~END~~


Expand Down
22 changes: 16 additions & 6 deletions test/JDBC/expected/forxml-raw-elements-vu-verify.out
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ SELECT NULL AS a, NULL AS b FOR XML RAW, ELEMENTS;
GO
~~START~~
ntext
<row></row>
<row/>
~~END~~


Expand All @@ -170,7 +170,7 @@ SELECT NULL AS a, NULL AS b, NULL AS c, NULL AS d FOR XML RAW, ELEMENTS;
GO
~~START~~
ntext
<row></row>
<row/>
~~END~~


Expand Down Expand Up @@ -198,7 +198,7 @@ SELECT NULL AS a, NULL AS b FOR XML RAW, ELEMENTS ABSENT;
GO
~~START~~
ntext
<row></row>
<row/>
~~END~~


Expand Down Expand Up @@ -510,7 +510,7 @@ SELECT name, salary FROM forxml_raw_elements_t1 FOR XML RAW, ELEMENTS;
GO
~~START~~
ntext
<row><name>John</name><salary>50000</salary></row><row><name>Jane</name><salary>60000</salary></row><row><name>Bob</name></row><row><name>Alice</name><salary>70000</salary></row><row></row>
<row><name>John</name><salary>50000</salary></row><row><name>Jane</name><salary>60000</salary></row><row><name>Bob</name></row><row><name>Alice</name><salary>70000</salary></row><row/>
~~END~~


Expand Down Expand Up @@ -950,7 +950,7 @@ SELECT CAST(NULL AS VARCHAR(10)) AS null_val FOR XML RAW, ELEMENTS;
GO
~~START~~
ntext
<row></row>
<row/>
~~END~~


Expand All @@ -971,10 +971,20 @@ SELECT 1 AS a, 2 AS b FOR XML RAW(''), ELEMENTS;
GO
~~START~~
ntext
<><a>1</a><b>2</b></>
<a>1</a><b>2</b>
~~END~~


-- Empty element name without ELEMENTS (attribute-centric) - should error
SELECT 1 AS a, 2 AS b FOR XML RAW('');
GO
~~START~~
ntext
~~ERROR (Code: 33557097)~~

~~ERROR (Message: Row tag omission (empty row tag name) cannot be used with attribute-centric FOR XML serialization.)~~


-- Element name with spaces
SELECT 1 AS a FOR XML RAW('element name'), ELEMENTS;
GO
Expand Down
4 changes: 4 additions & 0 deletions test/JDBC/input/forxml/forxml-raw-elements-vu-verify.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -414,6 +414,10 @@ GO
SELECT 1 AS a, 2 AS b FOR XML RAW(''), ELEMENTS;
GO

-- Empty element name without ELEMENTS (attribute-centric) - should error
Copy link
Copy Markdown
Contributor

@Deepesh125 Deepesh125 Mar 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

where do we have tests for all nulls under attribute mode? and with element mode but with empty element name?

Copy link
Copy Markdown
Contributor Author

@JapleenKaur3 JapleenKaur3 Mar 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've added both tests now.

SELECT 1 AS a, 2 AS b FOR XML RAW('');
GO

-- Element name with spaces
SELECT 1 AS a FOR XML RAW('element name'), ELEMENTS;
GO
Expand Down
Loading