feat: STRUCT widening and SAX rich-typing for read_xml(union_by_name) by bendrucker · Pull Request #75 · teaguesterling/duckdb_webbed

bendrucker · 2026-05-23T03:53:29Z

Summary

read_xml(..., union_by_name := true) collapsed any cross-file STRUCT shape disagreement to VARCHAR, silently flattening one side's data to concatenated text and dropping the other side entirely. This change adds a recursive type-merge step and threads structured types through the SAX path so files above maximum_file_size participate in widening instead of bailing out.

What changed

XMLSchemaInference::MergeXMLColumnType recursively unions STRUCT fields (first-seen casing wins) and LIST element types. For scalars it picks a candidate via ForceMaxLogicalType, then keeps it only when both sides have an implicit cast path to it (CastRules::ImplicitCast). That gives the cast ladder (INTEGER -> DOUBLE, DATE -> TIMESTAMP, etc., as discussed in Type inference should promote DATE to TIMESTAMP when both are present #39 / Refactor GetMostSpecificType to use DuckDB's LogicalType::ForceMaxLogicalType #41). Pairs with no cast path (e.g. DATE vs INTEGER) fall back to VARCHAR, so non-temporal data is not parsed as DATE at extract time. This avoids TryGetMaxLogicalTypeUnchecked, which only exists on newer DuckDB.
MergeXMLColumnType also reconciles cardinality mismatches. XML has no up-front schema, so an element occurring once infers as a scalar T while the same element occurring 2+ times in another file infers as LIST<T>. The merge now promotes the singleton to a one-element list and widens the element types to LIST<T'> rather than colliding to VARCHAR. A reconcilable guard limits this to compatible shapes (same complex kind, or both scalar), so a genuine mismatch such as STRUCT vs LIST<VARCHAR> still falls back to VARCHAR.
Schema inference builds a repeated element's STRUCT fields from the union of child names across all occurrences (CollectChildNamesInOrder), in first-seen document order, instead of reading only the first occurrence. An optional child absent from the first occurrence but present in a later one is now part of the inferred struct (nullable) instead of being dropped at inference and discarded at extraction. This applies to both read_xml and read_html and is independent of union_by_name.
MergeInferredColumns extracted from the two duplicate union-merge blocks in the read_xml and read_html binds.
ExtractStructFromNode uses case-insensitive child lookup so cross-file widening (which collapses case-distinct names per DuckDB's STRUCT semantics) finds data under either casing.
When a column is widened to VARCHAR but the source node has element children, the DOM extractor now serializes via xmlNodeDump instead of concatenating descendant text.
SAX path now produces rich types: the accumulator captures nested fragments in current_xml_values / current_xml_lists; InferSchemaFromStream emits them structurally into the synthetic XML so DOM inference reconstructs the STRUCT shape; AccumulatorToRow re-parses each fragment via a new ExtractValueFromXmlFragment helper. Every branch of AccumulatorToRow reconciles the text and nested-XML accumulator maps so no occurrence is silently dropped and the SAX values match DOM: the LIST branch combines text-shape and XML-shape items, the STRUCT branch falls back to text content (yielding DOM's non-NULL struct shell with NULL fields) when a field has no element children, and a column widened to VARCHAR serializes its repeated text and fragment items instead of emitting NULL.

Tests

test/sql/xml_union_by_name.test covers:

disjoint and overlapping STRUCT fields
nested STRUCT(STRUCT(...))
LIST<STRUCT>
the scalar promotion ladder
STRUCT-vs-VARCHAR collisions (with the new fragment serialization)
LIST-vs-STRUCT collisions
case-insensitive field collapse
a cross-mode case where one file is forced through SAX via maximum_file_size and the other through DOM

test/sql/xml_union_struct_list_cardinality.test covers the singleton-vs-LIST<STRUCT> widening: type is a widened LIST<STRUCT> in both file orders, all elements survive, and a nested field that is itself singleton-vs-list widens too.

test/sql/xml_inference_all_siblings.test covers single-file inference of an optional child that appears only in a later occurrence.

test/sql/xml_sax_accumulator_parity.test covers SAX/DOM parity for a text-only STRUCT field and for list-shaped columns that widen to VARCHAR, exercising both accumulator-map fallbacks.

test/sql/html_complex_types.test previously documented the all-occurrences gap as a known limitation (a skill-item field present only in the second section was excluded); its disabled assertions are now enabled and pass.

HTML parity test added under test/html/struct_widen/.

Pre-review checklist

Run against the original real-world (Salesforce) XML dataset that surfaced the bug and confirm the structured columns extract correctly end-to-end.

Out of scope / follow-ups considered

Single tagged-union accumulator map. The SAX accumulator uses parallel maps (current_values vs current_xml_values, and the matching list maps). The cleaner shape is one map of (name) -> Variant<Text, XmlFragment> that preserves document order. Today, when a single field has both text and XML items, the branches combine them but the source-document order between the two kinds is not preserved. Tracked in read_xml SAX rich-typing: namespace declarations dropped on reparsed fragments and list order lost for mixed text/XML occurrences #77.
Per-row libxml2 parser context reuse. ExtractValueFromXmlFragment pays a xmlNewParserCtxt / xmlCtxtReadMemory / free cycle per row. For large SAX files with STRUCT columns this is the dominant cost.
Namespace declarations on re-parsed fragments. With namespaces := 'keep', captured inner XML can reference prefixes without their xmlns: declarations, so ExtractValueFromXmlFragment fails to parse and the value drops to NULL (this includes the new STRUCT text fallback). Default strip mode is unaffected. Tracked in read_xml SAX rich-typing: namespace declarations dropped on reparsed fragments and list order lost for mixed text/XML occurrences #77.
InferSchemaFromStream buffers the whole synthetic document before handing it to DOM inference. Could stream-feed instead.
MAP widening. The XML inference layer does not produce MAP columns, so the merge does not try to handle them.

Copilot

Pull request overview

This PR fixes read_xml(..., union_by_name := true) type-collapsing behavior by introducing recursive STRUCT/LIST type merging during multi-file schema unioning, and by extending the SAX streaming path to preserve and materialize nested structured values (so large files also participate in widening instead of degrading to VARCHAR).

Changes:

Add XMLSchemaInference::MergeXMLColumnType to recursively merge STRUCT fields and LIST element types, and to use DuckDB’s implicit-cast ladder where available (fallback to VARCHAR when not).
Refactor duplicate union-by-name merge logic into MergeInferredColumns and apply it to both XML and HTML binds.
Upgrade SAX handling to capture nested XML fragments, emit them structurally during inference, and re-parse them during row materialization; plus add regression tests for XML/HTML.

Reviewed changes

Copilot reviewed 27 out of 27 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
src/xml_schema_inference.cpp	Implements recursive type merge + fragment parsing helper + improved VARCHAR extraction for widened complex values.
src/include/xml_schema_inference.hpp	Exposes new merge + fragment-extraction APIs used by the reader paths.
src/xml_sax_reader.cpp	Adds “rich typing” for SAX by tracking nested XML fragments and re-parsing them into STRUCT/LIST values.
src/include/xml_sax_reader.hpp	Extends the accumulator to track nested-XML scalars/lists separately from text scalars/lists.
src/xml_reader_functions.cpp	Deduplicates union schema merging and prevents SAX usage when schema contains unsupported types.
test/sql/xml_union_by_name.test	Adds union_by_name STRUCT/LIST widening and scalar promotion regression coverage (including SAX-vs-DOM cross-mode).
test/sql/html_union_by_name.test	Adds HTML parity coverage for STRUCT widening under union_by_name.
test/xml/struct_widen_a1.xml	Test fixture for disjoint STRUCT field widening.
test/xml/struct_widen_a2.xml	Test fixture for disjoint STRUCT field widening.
test/xml/struct_widen_b1.xml	Test fixture for scalar widening inside STRUCT.
test/xml/struct_widen_b2.xml	Test fixture for scalar widening inside STRUCT.
test/xml/struct_widen_c1.xml	Test fixture for nested STRUCT widening.
test/xml/struct_widen_c2.xml	Test fixture for nested STRUCT widening.
test/xml/struct_widen_d1.xml	Test fixture for LIST widening.
test/xml/struct_widen_d2.xml	Test fixture for LIST widening.
test/xml/struct_widen_e1.xml	Test fixture for scalar promotion (INTEGER side).
test/xml/struct_widen_e2.xml	Test fixture for scalar promotion (DOUBLE side).
test/xml/struct_widen_f2.xml	Test fixture for scalar collision fallback to VARCHAR.
test/xml/struct_widen_g1.xml	Test fixture for STRUCT-vs-VARCHAR collision (STRUCT-shaped).
test/xml/struct_widen_g2.xml	Test fixture for STRUCT-vs-VARCHAR collision (text-shaped).
test/xml/struct_widen_h1.xml	Test fixture for LIST-vs-STRUCT collision.
test/xml/struct_widen_case1.xml	Test fixture for case-insensitive STRUCT field collapse (Foo).
test/xml/struct_widen_case2.xml	Test fixture for case-insensitive STRUCT field collapse (foo).
test/xml/struct_widen_sax_small.xml	Small XML fixture to exercise DOM path in SAX-vs-DOM union test.
test/xml/struct_widen_sax_big.xml	Large XML fixture to force SAX path in SAX-vs-DOM union test.
test/html/struct_widen/a1.html	HTML fixture for STRUCT widening parity.
test/html/struct_widen/a2.html	HTML fixture for STRUCT widening parity.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

teaguesterling · 2026-05-23T22:21:40Z

Thanks for putting this together @bendrucker ! Starting some review

bendrucker · 2026-05-24T21:27:58Z

Addressed all three Copilot review comments, plus fixed a portability issue I introduced that broke the duckdb-stable-build (linux_arm64) job.

I'll still plan to run this branch over the real-world XML dataset where I originally encountered the current behavior before marking as ready.

read_xml(..., union_by_name := true) collapsed any cross-file STRUCT shape disagreement to VARCHAR with concatenated text. This change adds a recursive type merge and lifts the SAX path to produce STRUCT / LIST<STRUCT> so large files participate in widening instead of bailing out. - MergeXMLColumnType unions STRUCT fields and LIST element types; scalar widening uses TryGetMaxLogicalTypeUnchecked for the implicit-cast ladder and falls back to VARCHAR for pairs with no cast path. - ExtractStructFromNode uses case-insensitive child lookup so cross-file widening (case-insensitive collapse per DuckDB's STRUCT semantics) finds data under either casing. - VARCHAR widening of element-children-bearing nodes serializes via xmlNodeDump instead of concatenating descendant text. - SAX accumulator captures nested fragments in parallel current_xml_values / current_xml_lists; InferSchemaFromStream emits them structurally so DOM inference reconstructs the STRUCT shape; AccumulatorToRow re-parses each fragment via ExtractValueFromXmlFragment. LIST extraction combines text-shape and XML-shape items. Addresses teaguesterling#39 / teaguesterling#41 via the implicit-cast ladder (INTEGER->DOUBLE, DATE->TIMESTAMP, etc.) while avoiding the score-fallback regression where DATE+INTEGER would have collapsed to DATE and thrown at extract.

- Replace LogicalType::TryGetMaxLogicalTypeUnchecked with a portable combination of ForceMaxLogicalType + CastRules::ImplicitCast verification so MergeXMLColumnType compiles against DuckDB v1.4-andium (which has no Unchecked variant). When the candidate is not implicitly castable from both inputs we fall back to VARCHAR, matching the prior intent. - Update the MergeXMLColumnType doc comment to describe the actual cast-verification logic instead of pointing at ForceMaxLogicalType alone. - In the widened-to-VARCHAR fragment serializer, dump every child node (skipping libxml2-marked blank text nodes) with format=0 so mixed content like "foo <b/> bar" survives without pretty-print whitespace. - In the SAX list-of-complex branch, XML-escape raw text items before wrapping them for ExtractValueFromXmlFragment so '&' / '<' / '>' do not break the synthetic wrapper.

teaguesterling · 2026-05-28T19:38:27Z

So glad to see the builds working here! Want to run make format to fix the formatting issues? If not, I can pull and run once it's ready for review.

bendrucker · 2026-05-29T04:04:53Z

Ran make format.

bendrucker · 2026-05-29T04:07:30Z

I should have time to test this on my XML data set tomorrow and make sure it handles everything.

read_xml(union_by_name) collapsed a column to VARCHAR when an element inferred as a scalar STRUCT in one file (single occurrence) but a LIST<STRUCT> in another (multiple occurrences), keeping only the first child and dropping the rest. XML has no up-front schema, so single- vs multi-occurrence of the same element is inherently ambiguous. MergeXMLColumnType now reconciles a scalar T against LIST<T> by treating the singleton as a one-element list and widening element types to LIST<T'>. A reconcilable guard restricts this to compatible shapes (same complex kind, or both scalar) so a genuine collision such as STRUCT vs LIST<VARCHAR> still falls back to VARCHAR. Verified against real Salesforce metadata: profiles/permissionsets objectPermissions and CustomObject fields now widen to STRUCT[] instead of collapsing, with all elements preserved.

…irst read_xml/read_html inferred a repeated element's STRUCT fields by walking only the first occurrence's children. An optional child absent from the first occurrence but present in a later one was dropped from the inferred struct, and its values were silently discarded during extraction. This happened within a single file, independent of union_by_name. InferColumnType now builds the field set via CollectChildNamesInOrder, which scans every occurrence and collects the union of distinct child names in first-seen document order. Both the LIST<STRUCT> and single STRUCT inference paths use it. Extraction is type-driven and already fills absent fields with NULL, so no extraction change is needed. Updates html_complex_types.test, which previously encoded this gap as a documented limitation (skill-item, present only in the second section, was excluded); its TODO(future) assertions are now enabled. Verified against real Salesforce metadata: a single Profile whose first objectPermissions block omits viewAllFields now infers the field and preserves all 242 of its values.

Copilot

Pull request overview

Copilot reviewed 35 out of 35 changed files in this pull request and generated 2 comments.

bendrucker · 2026-05-30T00:20:50Z

+			// LIST column. Combine text-shaped items (current_lists / current_values) and
+			// nested-XML items (current_xml_lists / current_xml_values). Document order between
+			// text and XML items is not preserved (the accumulator stores them in parallel maps);
+			// items within a single map keep their relative order.
+			auto child_type = ListType::GetChildType(col_type);


Deferring this. Order is preserved within each kind. Only a field mixing bare text and nested-XML siblings loses order between the two groups, which none of the validated Salesforce lists do (uniform complex items). Tracked in #77 for the tagged-sequence accumulator fix before release.

bendrucker · 2026-05-30T00:20:43Z

+	// Wrap the fragment in its element so the parser sees a single root and so ExtractValueFromNode
+	// receives a node whose ->children are the original siblings.
+	std::string wrapped;
+	wrapped.reserve(inner_xml.size() + wrapper_name.size() * 2 + 5);
+	wrapped.append("<")
+	    .append(wrapper_name)
+	    .append(">")
+	    .append(inner_xml)
+	    .append("</")
+	    .append(wrapper_name)
+	    .append(">");
+
+	XMLDocRAII doc(wrapped);
+	if (!doc.doc) {
+		return Value(target_type);
+	}


Deferring this. It only triggers with namespaces := 'keep'. The default strip path removes prefixes before capture, so reparsing is safe there. Confirmed it doesn't affect the Salesforce dataset this PR was validated against, which used default strip mode. Tracked in #77 for a fix (gate to strip, or inject the active namespace declarations into the wrapper).

teaguesterling · 2026-05-29T23:56:11Z

This is looking good! Feel free to mark as "Ready for review" when you think it's ready to merge! I'll start prepping a community extensions repo update once it's all good to go!

bendrucker · 2026-05-29T23:57:08Z

Great, planning to do another pass myself this evening, and then I'll mark it.

The STRUCT branch and the scalar/VARCHAR fallback each read only a subset of the four accumulator maps, so the SAX streaming path diverged from DOM: - The STRUCT branch returned a fully-NULL struct for a text-only occurrence instead of DOM's non-NULL all-fields-NULL shell. It now falls back to GetValue, wrapping the escaped text through ExtractValueFromXmlFragment. - The scalar/VARCHAR fallback dropped every repeated occurrence when union_by_name widened a SAX-inferred list to VARCHAR. It now serializes both GetListValues (text) and GetXmlListValues (nested XML) items, mirroring the LIST branch. Adds test/sql/xml_sax_accumulator_parity.test covering both paths.

bendrucker · 2026-05-30T04:22:35Z

Took a pass at the follow ups, feel free to merge that branch into this PR if you want, or merge this as-is and I'll open a follow up to close #77. Much rarer edge cases so if you want to take more time to review that it would be fine to leave in a patch release too.

teaguesterling

Reviewed the full series. The recursive STRUCT/LIST merge, the cardinality reconciliation, the all-occurrences CollectChildNamesInOrder fix (with the previously-disabled html_complex_types.test assertions re-enabled), and the SAX-DOM parity work in AccumulatorToRow all hang together. The portable ForceMaxLogicalType + CastRules::ImplicitCast formulation is exactly the right call for the v1.4-andium → v1.5.x window, and the reconcilable guard cleanly keeps singleton-vs-LIST widening from over-reaching into genuine STRUCT-vs-LIST collisions. Test coverage matches the changes one-to-one (xml_union_by_name, xml_union_struct_list_cardinality, xml_inference_all_siblings, xml_sax_accumulator_parity, the HTML parity fixtures). All 16 CI jobs green.

Cleared the stale pending review from May 23 — both of those questions are answered by the current code: the XmlEscapeText/XmlEscapeAttr helpers in a45a70a cover the escaping concern in xml_sax_reader.cpp, and the stylistic difference vs. ExtractValueFromXmlFragment in xml_schema_inference.cpp is intentional — that site re-wraps already-serialized XML fragments rather than synthesizing from arbitrary text.

Merging as-is. Please open the sax-ns-order follow-up against main when you're ready and I'll review #77 there — both commits look strong to me already (the FieldOccurrence refactor is the cleaner-shape accumulator you flagged as out-of-scope in this PR's own description, and the ns-decl injection is gated on namespaces:='keep' so default behavior is untouched).

Note for ordering with #79 (the v1.5.3 bump I have open in parallel): no conflict expected — your portable cast formulation builds cleanly on v1.5.3, and the v1.5.3 PR doesn't touch any of the schema-inference or SAX code paths.

Thank you for this!

@bendrucker

- vcpkg.json manifest 2.0.1 -> 2.1.0 - RELEASE_NOTES_v2.1.0.md covering the four merged PRs since v2.0.1: - #75 STRUCT widening + SAX rich-typing (@bendrucker) - #76 DuckDB main v1.6 build compatibility (@bendrucker) - #77 SAX edge cases (FieldOccurrence + ns decls) via #80 (@bendrucker) - #79 duckdb v1.5.3 stable bump + MSVC C++17 + mingw exclusion Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

teaguesterling requested a review from Copilot May 23, 2026 22:16

Copilot started reviewing on behalf of teaguesterling May 23, 2026 22:16 View session

Copilot AI reviewed May 23, 2026

View reviewed changes

Comment thread src/include/xml_schema_inference.hpp

Comment thread src/xml_schema_inference.cpp

Comment thread src/xml_sax_reader.cpp

bendrucker added 2 commits May 28, 2026 11:30

bendrucker force-pushed the struct-widening branch from ed17f83 to a45a70a Compare May 28, 2026 18:35

style: apply make format (clang-format 11)

b3ac4f1

bendrucker added 2 commits May 29, 2026 16:21

bendrucker requested a review from Copilot May 29, 2026 23:51

Copilot AI reviewed May 29, 2026

View reviewed changes

bendrucker mentioned this pull request May 30, 2026

read_xml SAX rich-typing: namespace declarations dropped on reparsed fragments and list order lost for mixed text/XML occurrences #77

Closed

bendrucker marked this pull request as ready for review May 30, 2026 02:12

teaguesterling enabled auto-merge May 30, 2026 02:13

teaguesterling approved these changes May 31, 2026

View reviewed changes

teaguesterling merged commit ab97dc9 into teaguesterling:main May 31, 2026
16 checks passed

teaguesterling mentioned this pull request May 31, 2026

fix: close #77 SAX rich-typing edge cases (FieldOccurrence refactor + ns-decl injection) #80

Merged

teaguesterling mentioned this pull request Jun 1, 2026

webbed: bump to v2.1.0 (STRUCT widening + SAX rich-typing + DuckDB v1.5.3) duckdb/community-extensions#1994

Merged

Conversation

bendrucker commented May 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

What changed

Tests

Pre-review checklist

Out of scope / follow-ups considered

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

teaguesterling commented May 23, 2026

Uh oh!

bendrucker commented May 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

teaguesterling commented May 28, 2026

Uh oh!

bendrucker commented May 29, 2026

Uh oh!

bendrucker commented May 29, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

bendrucker May 30, 2026

Choose a reason for hiding this comment

Uh oh!

bendrucker May 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

teaguesterling commented May 29, 2026

Uh oh!

bendrucker commented May 29, 2026

Uh oh!

bendrucker commented May 30, 2026

Uh oh!

teaguesterling left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

bendrucker commented May 23, 2026 •

edited

Loading

bendrucker commented May 24, 2026 •

edited

Loading

bendrucker May 30, 2026 •

edited

Loading