From 926d21eeebfc1ba914abcc58fb64f5894edb4a5e Mon Sep 17 00:00:00 2001
From: "Victor M. Varela" <vmvarela@gmail.com>
Date: Fri, 8 May 2026 08:51:09 +0200
Subject: [PATCH 1/2] fix: make --xml-root and --xml-row work for XML input
 parsing
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Previously these flags only controlled XML output element names.
The parser always used the actual document root and accepted any
element as a row, making it impossible to query nested structures
like RSS feeds (<rss> → <channel> → <item>).

Changes:
- Add XmlParser.skipElementBody() to skip a complete element tree
- Add XmlParser.navigateToRoot() to descend into a named container
- Add row_tag_filter to XmlParser.nextRow() to skip non-matching elements
- Update loadXmlInput, getXmlColumnNames, summarizeXml to accept
  optional xml_root and xml_row parameters (null = legacy behaviour)
- Thread xml_root_input / xml_row_input from CLI args through
  ParsedArgs, ColumnsArgs, and ValidateArgs to all call sites
- Fix test_xml_no_rows to use <results> root (consistent with default)
- Add integration tests 114 and 115 for nested navigation

Closes #139
---
 build.zig    |  31 +++++-
 src/main.zig |  30 +++++-
 src/xml.zig  | 292 ++++++++++++++++++++++++++++++++++++++-------------
 3 files changed, 273 insertions(+), 80 deletions(-)

diff --git a/build.zig b/build.zig
index 1ac00a5..9cc3997 100644
--- a/build.zig
+++ b/build.zig
@@ -1066,9 +1066,11 @@ pub fn build(b: *std.Build) void {
     test_xml_validate.step.dependOn(b.getInstallStep());
     test_step.dependOn(&test_xml_validate.step);
 
-    // Integration test 104: --xml-root and --xml-row customize element names
+    // Integration test 104: --xml-root and --xml-row customize element names for output,
+    //                        and navigate nested XML for input
     const test_xml_custom_elements = b.addSystemCommand(&.{
         "bash", "-c",
+        // Output: custom element names appear in the XML
         \\result=$(printf 'name,age\nAlice,30\n' \
         \\    | ./zig-out/bin/sql-pipe -O xml --xml-root data --xml-row record 'SELECT * FROM t')
         \\echo "$result" | grep -q '<data>' && echo "$result" | grep -q '<record>' && echo "$result" | grep -q '</data>'
@@ -1108,7 +1110,7 @@ pub fn build(b: *std.Build) void {
     // Integration test 108: Root sin rows → error con "no row elements"
     const test_xml_no_rows = b.addSystemCommand(&.{
         "bash", "-c",
-        \\msg=$(printf '<root></root>' | ./zig-out/bin/sql-pipe -I xml 'SELECT 1' 2>&1; echo "EXIT:$?")
+        \\msg=$(printf '<results></results>' | ./zig-out/bin/sql-pipe -I xml 'SELECT 1' 2>&1; echo "EXIT:$?")
         \\echo "$msg" | grep -q 'no row elements' && echo "$msg" | grep -qv 'EXIT:0'
     });
     test_xml_no_rows.step.dependOn(b.getInstallStep());
@@ -1162,6 +1164,31 @@ pub fn build(b: *std.Build) void {
     test_xml_float_as_int.step.dependOn(b.getInstallStep());
     test_step.dependOn(&test_xml_float_as_int.step);
 
+    // Integration test 114: --xml-root navigates nested XML for input (RSS-like structure)
+    const test_xml_nested_navigation = b.addSystemCommand(&.{
+        "bash", "-c",
+        // Feed with <feed><channel><item> structure; --xml-root channel --xml-row item
+        // selects only item elements from inside channel, skipping <title> etc.
+        \\doc='<feed><channel><title>My Feed</title><item><name>Alice</name><age>30</age></item><item><name>Bob</name><age>25</age></item></channel></feed>'
+        \\result=$(printf '%s' "$doc" \
+        \\    | ./zig-out/bin/sql-pipe -I xml --xml-root channel --xml-row item \
+        \\    'SELECT name || ":" || age FROM t ORDER BY name')
+        \\[ "$result" = "$(printf 'Alice:30\nBob:25')" ]
+    });
+    test_xml_nested_navigation.step.dependOn(b.getInstallStep());
+    test_step.dependOn(&test_xml_nested_navigation.step);
+
+    // Integration test 115: --xml-root / --xml-row with --validate counts only matching rows
+    const test_xml_nested_validate = b.addSystemCommand(&.{
+        "bash", "-c",
+        \\doc='<feed><channel><title>T</title><item><val>1</val></item><item><val>2</val></item></channel></feed>'
+        \\result=$(printf '%s' "$doc" \
+        \\    | ./zig-out/bin/sql-pipe -I xml --xml-root channel --xml-row item --validate)
+        \\echo "$result" | grep -q 'OK: 2 rows'
+    });
+    test_xml_nested_validate.step.dependOn(b.getInstallStep());
+    test_step.dependOn(&test_xml_nested_validate.step);
+
     // Unit tests for the RFC 4180 CSV parser (src/csv.zig)
     const unit_tests = b.addTest(.{
         .root_module = b.createModule(.{
diff --git a/src/main.zig b/src/main.zig
index 9f2e132..086d36e 100644
--- a/src/main.zig
+++ b/src/main.zig
@@ -105,6 +105,10 @@ const ParsedArgs = struct {
     xml_root: []const u8,
     /// Row element name for XML output (default: "row").
     xml_row: []const u8,
+    /// Root element to navigate to for XML input; null = use actual document root.
+    xml_root_input: ?[]const u8,
+    /// Row tag filter for XML input; null = accept any direct child element as a row.
+    xml_row_input: ?[]const u8,
 };
 
 /// Arguments for `--columns` mode.
@@ -115,6 +119,10 @@ const ColumnsArgs = struct {
     verbose: bool,
     /// Input format (default: csv).
     input_format: InputFormat,
+    /// Root element to navigate to for XML input; null = use actual document root.
+    xml_root_input: ?[]const u8,
+    /// Row tag filter for XML input; null = accept any direct child element as a row.
+    xml_row_input: ?[]const u8,
 };
 
 /// Arguments for `--validate` mode.
@@ -125,6 +133,10 @@ const ValidateArgs = struct {
     type_inference: bool,
     /// Input format (default: csv).
     input_format: InputFormat,
+    /// Root element to navigate to for XML input; null = use actual document root.
+    xml_root_input: ?[]const u8,
+    /// Row tag filter for XML input; null = accept any direct child element as a row.
+    xml_row_input: ?[]const u8,
 };
 
 /// Arguments for `--sample` mode.
@@ -298,6 +310,8 @@ fn parseArgs(args: []const [:0]const u8) SqlPipeError!ArgsResult {
     var output: ?[]const u8 = null;
     var xml_root: []const u8 = "results";
     var xml_row: []const u8 = "row";
+    var xml_root_input: ?[]const u8 = null;
+    var xml_row_input: ?[]const u8 = null;
     var sample_mode = false;
     var sample_n: usize = 10;
 
@@ -400,14 +414,18 @@ fn parseArgs(args: []const [:0]const u8) SqlPipeError!ArgsResult {
             i += 1;
             if (i >= args.len) return error.MissingXmlFlagValue;
             xml_root = args[i];
+            xml_root_input = args[i];
         } else if (std.mem.startsWith(u8, arg, "--xml-root=")) {
             xml_root = arg["--xml-root=".len..];
+            xml_root_input = arg["--xml-root=".len..];
         } else if (std.mem.eql(u8, arg, "--xml-row")) {
             i += 1;
             if (i >= args.len) return error.MissingXmlFlagValue;
             xml_row = args[i];
+            xml_row_input = args[i];
         } else if (std.mem.startsWith(u8, arg, "--xml-row=")) {
             xml_row = arg["--xml-row=".len..];
+            xml_row_input = arg["--xml-row=".len..];
         } else {
             if (query == null) query = arg;
         }
@@ -471,6 +489,8 @@ fn parseArgs(args: []const [:0]const u8) SqlPipeError!ArgsResult {
             .delimiter = delimiter,
             .verbose = verbose,
             .input_format = input_format,
+            .xml_root_input = xml_root_input,
+            .xml_row_input = xml_row_input,
         } };
 
     // --validate mode: parse CSV and print summary
@@ -479,6 +499,8 @@ fn parseArgs(args: []const [:0]const u8) SqlPipeError!ArgsResult {
             .delimiter = delimiter,
             .type_inference = type_inference,
             .input_format = input_format,
+            .xml_root_input = xml_root_input,
+            .xml_row_input = xml_row_input,
         } };
 
     // --sample mode: print schema + first n rows and exit
@@ -503,6 +525,8 @@ fn parseArgs(args: []const [:0]const u8) SqlPipeError!ArgsResult {
         .output = output,
         .xml_root = xml_root,
         .xml_row = xml_row,
+        .xml_root_input = xml_root_input,
+        .xml_row_input = xml_row_input,
     } };
 }
 
@@ -1509,7 +1533,7 @@ fn runColumns(
             var stdin_buf: [4096]u8 = undefined;
             var stdin_file_reader = std.Io.File.reader(std.Io.File.stdin(), io, &stdin_buf);
 
-            const names = xml.getXmlColumnNames(allocator, &stdin_file_reader.interface, stderr_writer);
+            const names = xml.getXmlColumnNames(allocator, &stdin_file_reader.interface, args.xml_root_input, args.xml_row_input, stderr_writer);
             defer {
                 for (names) |name| allocator.free(name);
                 allocator.free(names);
@@ -1799,7 +1823,7 @@ fn runValidate(
             var stdin_buf: [4096]u8 = undefined;
             var stdin_file_reader = std.Io.File.reader(std.Io.File.stdin(), io, &stdin_buf);
 
-            const summary = xml.summarizeXml(allocator, &stdin_file_reader.interface, stderr_writer);
+            const summary = xml.summarizeXml(allocator, &stdin_file_reader.interface, args.xml_root_input, args.xml_row_input, stderr_writer);
             defer {
                 for (summary.col_names) |name| allocator.free(name);
                 allocator.free(summary.col_names);
@@ -2019,7 +2043,7 @@ fn run(
         .xml => blk: {
             var stdin_buf: [4096]u8 = undefined;
             var stdin_reader = std.Io.File.reader(std.Io.File.stdin(), io, &stdin_buf);
-            break :blk xml.loadXmlInput(allocator, &stdin_reader.interface, db, parsed.max_rows, stderr_writer);
+            break :blk xml.loadXmlInput(allocator, &stdin_reader.interface, db, parsed.xml_root_input, parsed.xml_row_input, parsed.max_rows, stderr_writer);
         },
     };
 
diff --git a/src/xml.zig b/src/xml.zig
index 399e2f2..e570d16 100644
--- a/src/xml.zig
+++ b/src/xml.zig
@@ -223,7 +223,7 @@ pub fn writeXmlFooter(writer: *std.Io.Writer, root_name: []const u8) !void {
 ///   var p = XmlParser.init(data);
 ///   p.skipPrologue(err_writer);
 ///   const root = p.readRootOpen(err_writer);
-///   while (try p.nextRow(allocator, root, err_writer)) |cols| {
+///   while (try p.nextRow(allocator, root, null, err_writer)) |cols| {
 ///       defer { for (cols) |col| { if (col.value) |v| allocator.free(v); } allocator.free(cols); }
 ///       // use cols[i].name and cols[i].value
 ///   }
@@ -449,6 +449,59 @@ pub const XmlParser = struct {
         self.fatalAt("unexpected end of input: unclosed element '{s}'", err_writer, .{elem_name});
     }
 
+    // ─── Element skip ────────────────────────────────────
+
+    /// Skip the body and closing tag of an element.
+    ///
+    /// Pre:  positioned just after the element's opening tag '>'
+    /// Post: positioned just after the element's closing '</tag>'
+    ///       properly handles nested elements, comments, CDATA, and PIs
+    fn skipElementBody(self: *XmlParser, tag: []const u8, err_writer: *std.Io.Writer) void {
+        // depth counts unclosed nested elements inside the one we are skipping
+        var depth: usize = 0;
+        // Loop invariant: depth = number of open nested elements not yet closed
+        // Bounding function: self.data.len - self.pos (finite input)
+        while (self.pos < self.data.len) {
+            if (self.peek().? != '<') {
+                self.advance();
+                continue;
+            }
+            if (self.startsWith("<!--")) {
+                self.skipComment(err_writer);
+            } else if (self.startsWith("<![CDATA[")) {
+                for ("<![CDATA[") |_| self.advance();
+                self.skipUntilStr("]]>", err_writer);
+            } else if (self.startsWith("<?")) {
+                self.skipProcessingInstruction(err_writer);
+            } else if (self.startsWith("</")) {
+                if (depth == 0) {
+                    // Closing tag of the element we are skipping
+                    self.advance();
+                    self.advance(); // "</"
+                    self.skipWs();
+                    _ = self.readName(err_writer);
+                    self.skipWs();
+                    if (self.peek() == '>') self.advance();
+                    return;
+                }
+                // Closing tag of a nested element
+                depth -= 1;
+                self.advance();
+                self.advance(); // "</"
+                _ = self.readName(err_writer);
+                self.skipWs();
+                if (self.peek() == '>') self.advance();
+            } else {
+                // Opening tag of a nested element (possibly self-closing)
+                self.advance(); // '<'
+                _ = self.readName(err_writer);
+                const self_closing = self.skipAttrsClose(err_writer);
+                if (!self_closing) depth += 1;
+            }
+        }
+        self.fatalAt("unexpected end of input: unclosed element '{s}'", err_writer, .{tag});
+    }
+
     // ─── High-level API ──────────────────────────────────
 
     /// Skip the XML prologue: declaration, processing instructions, comments.
@@ -470,100 +523,170 @@ pub const XmlParser = struct {
         return name;
     }
 
+    /// Navigate to the named container element for row iteration.
+    ///
+    /// Pre:  skipPrologue() has been called; positioned at the first '<'
+    /// Post: positioned just after the opening '>' of the xml_root element
+    ///
+    /// If the actual document root matches xml_root, it is consumed directly.
+    /// Otherwise the actual root is consumed and its direct children are scanned
+    /// until xml_root is found; all non-matching children are skipped entirely.
+    /// Fatal if xml_root is not found or is self-closing.
+    pub fn navigateToRoot(self: *XmlParser, xml_root: []const u8, err_writer: *std.Io.Writer) void {
+        if (self.peek() != '<') self.fatalAt("expected '<' to start root element", err_writer, .{});
+        self.advance(); // '<'
+        const actual_root = self.readName(err_writer);
+        const actual_self_closing = self.skipAttrsClose(err_writer);
+
+        // Fast path: actual root already is the target container
+        if (std.mem.eql(u8, actual_root, xml_root)) {
+            if (actual_self_closing)
+                self.fatalAt("element '{s}' is self-closing (no rows possible)", err_writer, .{xml_root});
+            return;
+        }
+
+        if (actual_self_closing)
+            self.fatalAt("element '{s}' not found (actual root '{s}' is self-closing)", err_writer, .{ xml_root, actual_root });
+
+        // Search direct children of the actual root for xml_root
+        // Loop invariant: all direct children before current position have been examined
+        // Bounding function: distance to end of actual root element (finite)
+        while (true) {
+            self.skipWsAndMisc(err_writer);
+            if (self.peek() == null)
+                self.fatalAt("element '{s}' not found in document", err_writer, .{xml_root});
+            // Reached end of actual root without finding the target
+            if (self.startsWith("</"))
+                self.fatalAt("element '{s}' not found under root element '{s}'", err_writer, .{ xml_root, actual_root });
+            if (self.peek() != '<')
+                self.fatalAt("expected '<' to start element inside '{s}'", err_writer, .{actual_root});
+            self.advance(); // '<'
+            const child_tag = self.readName(err_writer);
+            const child_self_closing = self.skipAttrsClose(err_writer);
+            if (std.mem.eql(u8, child_tag, xml_root)) {
+                if (child_self_closing)
+                    self.fatalAt("element '{s}' is self-closing (no rows possible)", err_writer, .{xml_root});
+                return;
+            }
+            // Skip this non-matching child entirely
+            if (!child_self_closing) self.skipElementBody(child_tag, err_writer);
+        }
+    }
+
     /// Read the next row element from the XML stream.
     ///
     /// Pre:  positioned after the root opening tag (or a previous row's closing tag)
     /// Post: returns null when the root closing tag is reached (no more rows)
-    ///       returns an owned slice of Column structs for the next row
+    ///       returns an owned slice of Column structs for the next matching row
     ///       caller must free each col.value (when non-null) and the slice itself
+    ///
+    /// row_tag_filter: when non-null, elements not matching this tag are skipped
     pub fn nextRow(
         self: *XmlParser,
         allocator: std.mem.Allocator,
         root_name: []const u8,
+        row_tag_filter: ?[]const u8,
         err_writer: *std.Io.Writer,
     ) !?[]Column {
-        self.skipWsAndMisc(err_writer);
-        if (self.peek() == null)
-            self.fatalAt("unexpected end of input: missing '</{s}>'", err_writer, .{root_name});
-
-        // Root closing tag → end of rows
-        if (self.startsWith("</")) {
-            self.advance();
-            self.advance(); // "</"
-            self.skipWs();
-            const close_name = self.readName(err_writer);
-            if (!std.mem.eql(u8, close_name, root_name))
-                self.fatalAt("expected '</{s}>' but found '</{s}>'", err_writer, .{ root_name, close_name });
-            self.skipWs();
-            if (self.peek() == '>') self.advance();
-            return null;
-        }
+        // Loop invariant: rows before current position have been processed or skipped
+        // Bounding function: distance to root closing tag (finite)
+        while (true) {
+            self.skipWsAndMisc(err_writer);
+            if (self.peek() == null)
+                self.fatalAt("unexpected end of input: missing '</{s}>'", err_writer, .{root_name});
 
-        // Row opening tag
-        if (self.peek() != '<') self.fatalAt("expected '<' to start row element", err_writer, .{});
-        self.advance(); // '<'
-        const row_tag = self.readName(err_writer);
-        const row_self_close = self.skipAttrsClose(err_writer);
+            // Root closing tag → end of rows
+            if (self.startsWith("</")) {
+                self.advance();
+                self.advance(); // "</"
+                self.skipWs();
+                const close_name = self.readName(err_writer);
+                if (!std.mem.eql(u8, close_name, root_name))
+                    self.fatalAt("expected '</{s}>' but found '</{s}>'", err_writer, .{ root_name, close_name });
+                self.skipWs();
+                if (self.peek() == '>') self.advance();
+                return null;
+            }
 
-        var cols: std.ArrayList(Column) = .empty;
-        errdefer {
-            for (cols.items) |col| if (col.value) |v| allocator.free(v);
-            cols.deinit(allocator);
-        }
+            // Row opening tag
+            if (self.peek() != '<') self.fatalAt("expected '<' to start row element", err_writer, .{});
+            self.advance(); // '<'
+            const row_tag = self.readName(err_writer);
+            const row_self_close = self.skipAttrsClose(err_writer);
+
+            // Skip elements that don't match the row tag filter
+            if (row_tag_filter) |filter| {
+                if (!std.mem.eql(u8, row_tag, filter)) {
+                    if (!row_self_close) self.skipElementBody(row_tag, err_writer);
+                    continue;
+                }
+            }
 
-        if (!row_self_close) {
-            // Loop invariant: cols contains all column elements of this row parsed so far
-            // Bounding function: distance to row closing tag
-            while (true) {
-                self.skipWsAndMisc(err_writer);
-                if (self.peek() == null)
-                    self.fatalAt("unexpected end of input in row element", err_writer, .{});
+            var cols: std.ArrayList(Column) = .empty;
+            errdefer {
+                for (cols.items) |col| if (col.value) |v| allocator.free(v);
+                cols.deinit(allocator);
+            }
 
-                // Row closing tag
-                if (self.startsWith("</")) {
-                    self.advance();
-                    self.advance(); // "</"
-                    self.skipWs();
-                    const close_row = self.readName(err_writer);
-                    if (!std.mem.eql(u8, close_row, row_tag))
-                        self.fatalAt("expected '</{s}>' but found '</{s}>'", err_writer, .{ row_tag, close_row });
-                    self.skipWs();
-                    if (self.peek() == '>') self.advance();
-                    break;
-                }
+            if (!row_self_close) {
+                // Loop invariant: cols contains all column elements of this row parsed so far
+                // Bounding function: distance to row closing tag
+                while (true) {
+                    self.skipWsAndMisc(err_writer);
+                    if (self.peek() == null)
+                        self.fatalAt("unexpected end of input in row element", err_writer, .{});
+
+                    // Row closing tag
+                    if (self.startsWith("</")) {
+                        self.advance();
+                        self.advance(); // "</"
+                        self.skipWs();
+                        const close_row = self.readName(err_writer);
+                        if (!std.mem.eql(u8, close_row, row_tag))
+                            self.fatalAt("expected '</{s}>' but found '</{s}>'", err_writer, .{ row_tag, close_row });
+                        self.skipWs();
+                        if (self.peek() == '>') self.advance();
+                        break;
+                    }
 
-                // Column opening tag
-                if (self.peek() != '<')
-                    self.fatalAt("expected '<' to start column element", err_writer, .{});
-                self.advance(); // '<'
-                const col_tag = self.readName(err_writer);
-                const col_self_close = self.skipAttrsClose(err_writer);
+                    // Column opening tag
+                    if (self.peek() != '<')
+                        self.fatalAt("expected '<' to start column element", err_writer, .{});
+                    self.advance(); // '<'
+                    const col_tag = self.readName(err_writer);
+                    const col_self_close = self.skipAttrsClose(err_writer);
 
-                const value: ?[]u8 = if (col_self_close)
-                    null
-                else
-                    try self.readContent(allocator, err_writer, col_tag);
+                    const value: ?[]u8 = if (col_self_close)
+                        null
+                    else
+                        try self.readContent(allocator, err_writer, col_tag);
 
-                try cols.append(allocator, .{ .name = col_tag, .value = value });
+                    try cols.append(allocator, .{ .name = col_tag, .value = value });
+                }
             }
-        }
 
-        const owned = try cols.toOwnedSlice(allocator);
-        return owned;
+            const owned = try cols.toOwnedSlice(allocator);
+            return owned;
+        }
     }
 };
 
 // ─── Public input functions ───────────────────────────
 
-/// getXmlColumnNames(allocator, reader, stderr_writer) → [][]const u8
+/// getXmlColumnNames(allocator, reader, xml_root, xml_row, stderr_writer) → [][]const u8
 ///
 /// Pre:  reader is positioned at the start of a row-based XML document
 /// Post: returns an allocated slice of column names (from the first row);
 ///       caller must free each name and the slice
 ///       aborts on any parse or I/O error
+///
+/// xml_root: when non-null, navigate to this element as the row container
+/// xml_row:  when non-null, only elements with this tag are treated as rows
 pub fn getXmlColumnNames(
     allocator: std.mem.Allocator,
     reader: *std.Io.Reader,
+    xml_root: ?[]const u8,
+    xml_row: ?[]const u8,
     stderr_writer: *std.Io.Writer,
 ) [][]const u8 {
     var buf: std.ArrayList(u8) = .empty;
@@ -579,9 +702,12 @@ pub fn getXmlColumnNames(
 
     var p = XmlParser.init(buf.items);
     p.skipPrologue(stderr_writer);
-    const root = p.readRootOpen(stderr_writer);
+    const root_name: []const u8 = if (xml_root) |r| blk: {
+        p.navigateToRoot(r, stderr_writer);
+        break :blk r;
+    } else p.readRootOpen(stderr_writer);
 
-    const cols = p.nextRow(allocator, root, stderr_writer) catch
+    const cols = p.nextRow(allocator, root_name, xml_row, stderr_writer) catch
         fatal("out of memory parsing XML", stderr_writer, exit_parse, .{});
     if (cols == null) fatal("XML document has no row elements", stderr_writer, exit_parse, .{});
     defer {
@@ -607,14 +733,19 @@ pub const XmlSummary = struct {
     col_names: [][]const u8,
 };
 
-/// summarizeXml(allocator, reader, stderr_writer) → XmlSummary
+/// summarizeXml(allocator, reader, xml_root, xml_row, stderr_writer) → XmlSummary
 ///
 /// Pre:  reader is positioned at the start of a row-based XML document
 /// Post: parses the entire document; returns row count and column names
 ///       aborts on any parse or I/O error
+///
+/// xml_root: when non-null, navigate to this element as the row container
+/// xml_row:  when non-null, only elements with this tag are treated as rows
 pub fn summarizeXml(
     allocator: std.mem.Allocator,
     reader: *std.Io.Reader,
+    xml_root: ?[]const u8,
+    xml_row: ?[]const u8,
     stderr_writer: *std.Io.Writer,
 ) XmlSummary {
     var buf: std.ArrayList(u8) = .empty;
@@ -630,7 +761,10 @@ pub fn summarizeXml(
 
     var p = XmlParser.init(buf.items);
     p.skipPrologue(stderr_writer);
-    const root = p.readRootOpen(stderr_writer);
+    const root_name: []const u8 = if (xml_root) |r| blk: {
+        p.navigateToRoot(r, stderr_writer);
+        break :blk r;
+    } else p.readRootOpen(stderr_writer);
 
     var row_count: usize = 0;
     var col_names: ?[][]const u8 = null;
@@ -638,7 +772,7 @@ pub fn summarizeXml(
     // Loop invariant: row_count = rows processed so far; col_names set after first row
     // Bounding function: rows remaining in the XML document (finite)
     while (true) {
-        const cols = p.nextRow(allocator, root, stderr_writer) catch
+        const cols = p.nextRow(allocator, root_name, xml_row, stderr_writer) catch
             fatal("out of memory parsing XML", stderr_writer, exit_parse, .{});
         if (cols == null) break;
         defer {
@@ -662,7 +796,7 @@ pub fn summarizeXml(
     return .{ .row_count = row_count, .col_names = col_names.? };
 }
 
-/// loadXmlInput(allocator, reader, db, max_rows, stderr_writer) → usize
+/// loadXmlInput(allocator, reader, db, xml_root, xml_row, max_rows, stderr_writer) → usize
 ///
 /// Pre:  reader is positioned at the start of a row-based XML document
 ///       db is an open, empty SQLite database
@@ -670,10 +804,15 @@ pub fn summarizeXml(
 ///       all row elements are inserted; transaction is committed
 ///       result = number of rows inserted
 ///       aborts the process on any parse, I/O, or SQL error
+///
+/// xml_root: when non-null, navigate to this element as the row container
+/// xml_row:  when non-null, only elements with this tag are treated as rows
 pub fn loadXmlInput(
     allocator: std.mem.Allocator,
     reader: *std.Io.Reader,
     db: *c.sqlite3,
+    xml_root: ?[]const u8,
+    xml_row: ?[]const u8,
     max_rows: ?usize,
     stderr_writer: *std.Io.Writer,
 ) usize {
@@ -690,7 +829,10 @@ pub fn loadXmlInput(
 
     var p = XmlParser.init(buf.items);
     p.skipPrologue(stderr_writer);
-    const root_name = p.readRootOpen(stderr_writer);
+    const root_name: []const u8 = if (xml_root) |r| blk: {
+        p.navigateToRoot(r, stderr_writer);
+        break :blk r;
+    } else p.readRootOpen(stderr_writer);
 
     // Column names determined from the first row; kept for schema consistency
     var col_names: ?[][]const u8 = null;
@@ -712,7 +854,7 @@ pub fn loadXmlInput(
     //   in_transaction = true after the first insert
     // Bounding function: row elements remaining in the document (finite)
     while (true) {
-        const cols = p.nextRow(allocator, root_name, stderr_writer) catch
+        const cols = p.nextRow(allocator, root_name, xml_row, stderr_writer) catch
             fatal("out of memory parsing XML", stderr_writer, exit_parse, .{});
         if (cols == null) break;
 
@@ -855,7 +997,7 @@ test "XmlParser.nextRow: simple row with two columns" {
     const root = p.readRootOpen(&err_writer);
     try std.testing.expectEqualStrings("results", root);
 
-    const cols = try p.nextRow(allocator, root, &err_writer);
+    const cols = try p.nextRow(allocator, root, null, &err_writer);
     try std.testing.expect(cols != null);
     defer {
         for (cols.?) |col| if (col.value) |v| allocator.free(v);
@@ -868,7 +1010,7 @@ test "XmlParser.nextRow: simple row with two columns" {
     try std.testing.expectEqualStrings("30", cols.?[1].value.?);
 
     // No more rows
-    const next = try p.nextRow(allocator, root, &err_writer);
+    const next = try p.nextRow(allocator, root, null, &err_writer);
     try std.testing.expect(next == null);
 }
 
@@ -882,7 +1024,7 @@ test "XmlParser.nextRow: self-closing column is null" {
     p.skipPrologue(&err_writer);
     const root = p.readRootOpen(&err_writer);
 
-    const cols = try p.nextRow(allocator, root, &err_writer);
+    const cols = try p.nextRow(allocator, root, null, &err_writer);
     try std.testing.expect(cols != null);
     defer {
         for (cols.?) |col| if (col.value) |v| allocator.free(v);
@@ -904,7 +1046,7 @@ test "XmlParser.nextRow: entities decoded in content" {
     p.skipPrologue(&err_writer);
     const root = p.readRootOpen(&err_writer);
 
-    const cols = try p.nextRow(allocator, root, &err_writer);
+    const cols = try p.nextRow(allocator, root, null, &err_writer);
     try std.testing.expect(cols != null);
     defer {
         for (cols.?) |col| if (col.value) |v| allocator.free(v);
@@ -923,7 +1065,7 @@ test "XmlParser.nextRow: empty document returns null" {
     p.skipPrologue(&err_writer);
     const root = p.readRootOpen(&err_writer);
 
-    const cols = try p.nextRow(allocator, root, &err_writer);
+    const cols = try p.nextRow(allocator, root, null, &err_writer);
     try std.testing.expect(cols == null);
 }
 

From 3dcb9b9d9c572ba11a7881d0cd34d5c0e512fb51 Mon Sep 17 00:00:00 2001
From: "Victor M. Varela" <vmvarela@gmail.com>
Date: Fri, 8 May 2026 09:16:55 +0200
Subject: [PATCH 2/2] fix: address oracle code review findings for PR #140

- skipElementBody: validate closing tag name against expected tag (was discarding it)
- skipElementBody: replace fragile peek().? pattern with safe orelse-break idiom
- navigateToRoot: skip text nodes between siblings instead of fataling
- navigateToRoot: improve error message to say 'not found as a direct child of'
- getXmlColumnNames/summarizeXml/loadXmlInput: emit tailored error when --xml-row
  filter matches nothing ('check --xml-row value') vs generic 'no row elements'
- Unit tests: skipElementBody (body skip, nested elements), navigateToRoot
  (fast path, nested child, text nodes), nextRow with row_tag_filter
- Integration tests 116-120: --xml-root alone, --xml-row alone, filter-no-match
  error, --columns with both flags, fast path when --xml-root equals actual root
- Fix test 104 comment (output-only, not input)
---
 build.zig   |  56 +++++++++++++++++++++-
 src/xml.zig | 130 +++++++++++++++++++++++++++++++++++++++++++++++-----
 2 files changed, 173 insertions(+), 13 deletions(-)

diff --git a/build.zig b/build.zig
index 9cc3997..c723f2f 100644
--- a/build.zig
+++ b/build.zig
@@ -1066,8 +1066,7 @@ pub fn build(b: *std.Build) void {
     test_xml_validate.step.dependOn(b.getInstallStep());
     test_step.dependOn(&test_xml_validate.step);
 
-    // Integration test 104: --xml-root and --xml-row customize element names for output,
-    //                        and navigate nested XML for input
+    // Integration test 104: --xml-root and --xml-row customize element names for output
     const test_xml_custom_elements = b.addSystemCommand(&.{
         "bash", "-c",
         // Output: custom element names appear in the XML
@@ -1189,6 +1188,59 @@ pub fn build(b: *std.Build) void {
     test_xml_nested_validate.step.dependOn(b.getInstallStep());
     test_step.dependOn(&test_xml_nested_validate.step);
 
+    // Integration test 116: --xml-root alone (no --xml-row) navigates to container
+    const test_xml_root_alone = b.addSystemCommand(&.{
+        "bash", "-c",
+        \\doc='<feed><data><row><name>Alice</name></row><row><name>Bob</name></row></data></feed>'
+        \\result=$(printf '%s' "$doc" \
+        \\    | ./zig-out/bin/sql-pipe -I xml --xml-root data 'SELECT name FROM t ORDER BY name')
+        \\[ "$result" = "$(printf 'Alice\nBob')" ]
+    });
+    test_xml_root_alone.step.dependOn(b.getInstallStep());
+    test_step.dependOn(&test_xml_root_alone.step);
+
+    // Integration test 117: --xml-row alone (no --xml-root) filters rows by tag
+    const test_xml_row_alone = b.addSystemCommand(&.{
+        "bash", "-c",
+        \\doc='<results><item><name>Alice</name></item><meta><x>1</x></meta><item><name>Bob</name></item></results>'
+        \\result=$(printf '%s' "$doc" \
+        \\    | ./zig-out/bin/sql-pipe -I xml --xml-row item 'SELECT name FROM t ORDER BY name')
+        \\[ "$result" = "$(printf 'Alice\nBob')" ]
+    });
+    test_xml_row_alone.step.dependOn(b.getInstallStep());
+    test_step.dependOn(&test_xml_row_alone.step);
+
+    // Integration test 118: --xml-row with no matching elements exits non-zero with clear message
+    const test_xml_row_no_match = b.addSystemCommand(&.{
+        "bash", "-c",
+        \\msg=$(printf '<results><row><name>Alice</name></row></results>' \
+        \\    | ./zig-out/bin/sql-pipe -I xml --xml-row wrong 'SELECT 1' 2>&1; echo "EXIT:$?")
+        \\echo "$msg" | grep -q "'wrong'" && echo "$msg" | grep -q 'check --xml-row' && echo "$msg" | grep -qv 'EXIT:0'
+    });
+    test_xml_row_no_match.step.dependOn(b.getInstallStep());
+    test_step.dependOn(&test_xml_row_no_match.step);
+
+    // Integration test 119: --columns with --xml-root and --xml-row
+    const test_xml_columns_with_flags = b.addSystemCommand(&.{
+        "bash", "-c",
+        \\doc='<feed><channel><item><name>Alice</name><age>30</age></item></channel></feed>'
+        \\result=$(printf '%s' "$doc" \
+        \\    | ./zig-out/bin/sql-pipe -I xml --xml-root channel --xml-row item --columns)
+        \\[ "$result" = "$(printf 'name\nage')" ]
+    });
+    test_xml_columns_with_flags.step.dependOn(b.getInstallStep());
+    test_step.dependOn(&test_xml_columns_with_flags.step);
+
+    // Integration test 120: --xml-root matching the actual document root (fast path)
+    const test_xml_root_fast_path = b.addSystemCommand(&.{
+        "bash", "-c",
+        \\result=$(printf '<results><row><name>Alice</name></row></results>' \
+        \\    | ./zig-out/bin/sql-pipe -I xml --xml-root results 'SELECT name FROM t')
+        \\[ "$result" = "Alice" ]
+    });
+    test_xml_root_fast_path.step.dependOn(b.getInstallStep());
+    test_step.dependOn(&test_xml_root_fast_path.step);
+
     // Unit tests for the RFC 4180 CSV parser (src/csv.zig)
     const unit_tests = b.addTest(.{
         .root_module = b.createModule(.{
diff --git a/src/xml.zig b/src/xml.zig
index e570d16..95e99ad 100644
--- a/src/xml.zig
+++ b/src/xml.zig
@@ -461,8 +461,9 @@ pub const XmlParser = struct {
         var depth: usize = 0;
         // Loop invariant: depth = number of open nested elements not yet closed
         // Bounding function: self.data.len - self.pos (finite input)
-        while (self.pos < self.data.len) {
-            if (self.peek().? != '<') {
+        while (true) {
+            const ch = self.peek() orelse break;
+            if (ch != '<') {
                 self.advance();
                 continue;
             }
@@ -475,13 +476,16 @@ pub const XmlParser = struct {
                 self.skipProcessingInstruction(err_writer);
             } else if (self.startsWith("</")) {
                 if (depth == 0) {
-                    // Closing tag of the element we are skipping
+                    // Closing tag of the element we are skipping — validate name
                     self.advance();
                     self.advance(); // "</"
                     self.skipWs();
-                    _ = self.readName(err_writer);
+                    const close_name = self.readName(err_writer);
                     self.skipWs();
-                    if (self.peek() == '>') self.advance();
+                    if (self.peek() != '>') self.fatalAt("expected '>' after closing tag name", err_writer, .{});
+                    self.advance();
+                    if (!std.mem.eql(u8, close_name, tag))
+                        self.fatalAt("expected '</{s}>' but found '</{s}>'", err_writer, .{ tag, close_name });
                     return;
                 }
                 // Closing tag of a nested element
@@ -557,9 +561,12 @@ pub const XmlParser = struct {
                 self.fatalAt("element '{s}' not found in document", err_writer, .{xml_root});
             // Reached end of actual root without finding the target
             if (self.startsWith("</"))
-                self.fatalAt("element '{s}' not found under root element '{s}'", err_writer, .{ xml_root, actual_root });
-            if (self.peek() != '<')
-                self.fatalAt("expected '<' to start element inside '{s}'", err_writer, .{actual_root});
+                self.fatalAt("element '{s}' not found as a direct child of '{s}'", err_writer, .{ xml_root, actual_root });
+            // Skip text nodes between sibling elements
+            if (self.peek() != '<') {
+                while (self.peek()) |skip_ch| if (skip_ch != '<') self.advance() else break;
+                continue;
+            }
             self.advance(); // '<'
             const child_tag = self.readName(err_writer);
             const child_self_closing = self.skipAttrsClose(err_writer);
@@ -709,7 +716,12 @@ pub fn getXmlColumnNames(
 
     const cols = p.nextRow(allocator, root_name, xml_row, stderr_writer) catch
         fatal("out of memory parsing XML", stderr_writer, exit_parse, .{});
-    if (cols == null) fatal("XML document has no row elements", stderr_writer, exit_parse, .{});
+    if (cols == null) {
+        if (xml_row) |row_tag|
+            fatal("XML document has no '{s}' elements (check --xml-row value)", stderr_writer, exit_parse, .{row_tag})
+        else
+            fatal("XML document has no row elements", stderr_writer, exit_parse, .{});
+    }
     defer {
         for (cols.?) |col| if (col.value) |v| allocator.free(v);
         allocator.free(cols.?);
@@ -792,7 +804,12 @@ pub fn summarizeXml(
         }
     }
 
-    if (col_names == null) fatal("XML document has no row elements", stderr_writer, exit_parse, .{});
+    if (col_names == null) {
+        if (xml_row) |row_tag|
+            fatal("XML document has no '{s}' elements (check --xml-row value)", stderr_writer, exit_parse, .{row_tag})
+        else
+            fatal("XML document has no row elements", stderr_writer, exit_parse, .{});
+    }
     return .{ .row_count = row_count, .col_names = col_names.? };
 }
 
@@ -916,7 +933,12 @@ pub fn loadXmlInput(
             fatal("{s}", stderr_writer, exit_sql, .{std.mem.span(c.sqlite3_errmsg(db))});
     }
 
-    if (col_names == null) fatal("XML document has no row elements", stderr_writer, exit_parse, .{});
+    if (col_names == null) {
+        if (xml_row) |row_tag|
+            fatal("XML document has no '{s}' elements (check --xml-row value)", stderr_writer, exit_parse, .{row_tag})
+        else
+            fatal("XML document has no row elements", stderr_writer, exit_parse, .{});
+    }
     if (in_transaction) commitTransaction(db, stderr_writer);
     return rows_inserted;
 }
@@ -1079,3 +1101,89 @@ test "XmlParser: XML declaration in prologue" {
     const root = p.readRootOpen(&err_writer);
     try std.testing.expectEqualStrings("results", root);
 }
+
+test "XmlParser.skipElementBody: skips body and validates closing tag" {
+    var err_buf: [256]u8 = undefined;
+    var err_writer: std.Io.Writer = .fixed(&err_buf);
+
+    // Parser positioned just after <skip>'s '>' — body + closing tag + next element
+    const input = "<inner>text</inner></skip><keep/>";
+    var p = XmlParser.init(input);
+    p.skipElementBody("skip", &err_writer);
+    // Should now be positioned at "<keep/>"
+    try std.testing.expectEqualStrings("<keep/>", p.data[p.pos..]);
+}
+
+test "XmlParser.skipElementBody: handles nested elements" {
+    var err_buf: [256]u8 = undefined;
+    var err_writer: std.Io.Writer = .fixed(&err_buf);
+
+    const input = "<a><b><c>x</c></b></a></skip>tail";
+    var p = XmlParser.init(input);
+    p.skipElementBody("skip", &err_writer);
+    try std.testing.expectEqualStrings("tail", p.data[p.pos..]);
+}
+
+test "XmlParser.navigateToRoot: fast path — actual root matches" {
+    var err_buf: [256]u8 = undefined;
+    var err_writer: std.Io.Writer = .fixed(&err_buf);
+
+    const input = "<channel><item><title>Test</title></item></channel>";
+    var p = XmlParser.init(input);
+    p.skipPrologue(&err_writer);
+    p.navigateToRoot("channel", &err_writer);
+    try std.testing.expectEqualStrings("<item><title>Test</title></item></channel>", p.data[p.pos..]);
+}
+
+test "XmlParser.navigateToRoot: finds nested container as direct child" {
+    var err_buf: [256]u8 = undefined;
+    var err_writer: std.Io.Writer = .fixed(&err_buf);
+
+    const input = "<rss><meta><v>1</v></meta><channel><item><n>T</n></item></channel></rss>";
+    var p = XmlParser.init(input);
+    p.skipPrologue(&err_writer);
+    p.navigateToRoot("channel", &err_writer);
+    try std.testing.expectEqualStrings("<item><n>T</n></item></channel></rss>", p.data[p.pos..]);
+}
+
+test "XmlParser.navigateToRoot: handles text nodes between siblings" {
+    var err_buf: [256]u8 = undefined;
+    var err_writer: std.Io.Writer = .fixed(&err_buf);
+
+    const input = "<rss>some text<channel><item/></channel></rss>";
+    var p = XmlParser.init(input);
+    p.skipPrologue(&err_writer);
+    p.navigateToRoot("channel", &err_writer);
+    try std.testing.expectEqualStrings("<item/></channel></rss>", p.data[p.pos..]);
+}
+
+test "XmlParser.nextRow: row_tag_filter skips non-matching elements" {
+    const allocator = std.testing.allocator;
+    var err_buf: [256]u8 = undefined;
+    var err_writer: std.Io.Writer = .fixed(&err_buf);
+
+    const input = "<root><meta><x>1</x></meta><item><name>Alice</name></item>" ++
+        "<meta><x>2</x></meta><item><name>Bob</name></item></root>";
+    var p = XmlParser.init(input);
+    p.skipPrologue(&err_writer);
+    const root = p.readRootOpen(&err_writer);
+
+    const row1 = try p.nextRow(allocator, root, "item", &err_writer);
+    try std.testing.expect(row1 != null);
+    defer {
+        for (row1.?) |col| if (col.value) |v| allocator.free(v);
+        allocator.free(row1.?);
+    }
+    try std.testing.expectEqualStrings("Alice", row1.?[0].value.?);
+
+    const row2 = try p.nextRow(allocator, root, "item", &err_writer);
+    try std.testing.expect(row2 != null);
+    defer {
+        for (row2.?) |col| if (col.value) |v| allocator.free(v);
+        allocator.free(row2.?);
+    }
+    try std.testing.expectEqualStrings("Bob", row2.?[0].value.?);
+
+    const row3 = try p.nextRow(allocator, root, "item", &err_writer);
+    try std.testing.expect(row3 == null);
+}