Skip to content

Add StructuredTable - #12

Merged
DISTREAT merged 12 commits into
masterfrom
comptime
Jan 18, 2026
Merged

Add StructuredTable#12
DISTREAT merged 12 commits into
masterfrom
comptime

Conversation

@DISTREAT

Copy link
Copy Markdown
Owner

No description provided.

@DISTREATDISTREAT self-assigned this Jan 18, 2026
@DISTREATDISTREAT added the enhancement New feature or request label Jan 18, 2026
@DISTREATDISTREAT linked an issue Jan 18, 2026 that may be closed by this pull request
@DISTREAT
DISTREATforce-pushed the comptime branch 2 times, most recently from c159527 to e132315CompareJanuary 18, 2026 02:00

CopilotAI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR adds a new StructuredTable type that enables type-safe parsing and manipulation of CSV data through compile-time schema validation. The existing Table implementation has been refactored into separate modules to support the new functionality while maintaining backward compatibility through re-exports.

Changes:

  • Introduced StructuredTable generic type that maps CSV data to user-defined struct types with automatic type conversion and validation
  • Refactored monolithic src/root.zig into separate table.zig and schema.zig modules for better organization
  • Added comprehensive test suite for the new StructuredTable functionality and updated README documentation with usage examples

Reviewed changes

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

Show a summary per file
FileDescription
src/root.zigConverted to thin re-export module that exposes both Table and StructuredTable APIs
src/table.zigExtracted core Table implementation with bug fix for getColumnCount signature
src/schema.zigNew module implementing StructuredTable with type-safe CSV parsing and serialization
src/tests/root.zigNew test harness that imports individual test files
src/tests/table.zigUpdated table tests with corrected import and updated API usage
src/tests/schema.zigComprehensive test suite for StructuredTable covering parsing, editing, and error handling
build.zigUpdated test configuration to use new test structure with module import
README.mdAdded extensive documentation and examples for both StructuredTable and Table APIs
Comments suppressed due to low confidence (1)

src/tests/table.zig:5

  • The StructuredTable constant is imported but never used in this test file. Since this file only contains tests for the Table type (not StructuredTable), this import should be removed.

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

Comment threadsrc/tests/schema.zig Outdated
Comment threadsrc/tests/schema.zig Outdated

CopilotAI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 6 comments.


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

Comment threadsrc/schema.zig
Comment threadsrc/schema.zig Outdated
Comment threadsrc/schema.zig
Comment on lines +206 to +253
pub fn getRow(self: Self, row_index: usize) TableError!ParseResult(table_schema) {
if (row_index >= self.getRowCount()) return TableError.RowNotFound;
var out: table_schema = undefined;
inline for (schema_info.@"struct".fields) |field| {
const field_name = field.name;
const column_indexes = self.table.findColumnIndexesByValue(self.allocator, 0, field_name) catch return ParseResult(table_schema){
.@"error" = .{
.kind = StructureError.MissingColumn,
.field_name = field_name,
.field_type = @typeName(field.type),
.csv_value = null,
},
};
defer self.allocator.free(column_indexes);
if (column_indexes.len > 1) return ParseResult(table_schema){
.@"error" = .{
.kind = StructureError.AmbiguousColumn,
.field_name = field_name,
.field_type = @typeName(field.type),
.csv_value = null,
},
};
const rows = self.table.getColumnByIndex(self.allocator, column_indexes[0]) catch return ParseResult(table_schema){
.@"error" = .{
.kind = StructureError.MissingColumn,
.field_name = field_name,
.field_type = @typeName(field.type),
.csv_value = null,
},
};
defer self.allocator.free(rows);
const value = rows[row_index + 1];
const parsed = (&self).deserializeCsvValue(field.type, value) catch |err| return ParseResult(table_schema){
.@"error" = .{
.kind = err,
.field_name = field_name,
.field_type = @typeName(field.type),
.csv_value = value,
},
};
@field(out, field_name) = parsed;
}
return ParseResult(table_schema){
.ok = .{
.value = out,
},
};
}

CopilotAIJan 18, 2026

Copy link

Choose a reason for hiding this comment

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

Performance concern: the function searches for each field's column index by value on every invocation, using findColumnIndexesByValue which performs a linear search. For tables with many fields or when this function is called frequently in a loop, this results in O(n*m) complexity where n is the number of fields and m is the number of columns. Consider caching the column index mapping after the first lookup to improve performance.

Copilot uses AI. Check for mistakes.

Copy link
Copy Markdown
OwnerAuthor

Choose a reason for hiding this comment

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

Performance is not in scope atm.

Comment threadsrc/schema.zig
Comment on lines +267 to +298
pub fn editRow(self: *Self, row_index: usize, row: table_schema) TableError!ParseResult(table_schema) {
if (row_index >= self.getRowCount()) return TableError.RowNotFound;
inline for (schema_info.@"struct".fields) |field| {
const field_name = field.name;
const column_indexes = self.table.findColumnIndexesByValue(self.allocator, 0, field_name) catch return ParseResult(table_schema){
.@"error" = .{
.kind = StructureError.MissingColumn,
.field_name = field_name,
.field_type = @typeName(field.type),
.csv_value = null,
},
};
defer self.allocator.free(column_indexes);
if (column_indexes.len > 1) return ParseResult(table_schema){
.@"error" = .{
.kind = StructureError.AmbiguousColumn,
.field_name = field_name,
.field_type = @typeName(field.type),
.csv_value = null,
},
};
const column_index = column_indexes[0];
const table_index = headerAwareToTableIndex(row_index);
const value = try self.serializeCsvValue(field.type, @field(row, field_name));
try self.table.replaceValue(table_index, column_index, value);
}
return ParseResult(table_schema){
.ok = .{
.value = row,
},
};
}

CopilotAIJan 18, 2026

Copy link

Choose a reason for hiding this comment

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

Performance concern: the function searches for each field's column index by value on every invocation, using findColumnIndexesByValue which performs a linear search. For tables with many fields or when this function is called frequently in a loop, this results in O(n*m) complexity where n is the number of fields and m is the number of columns. Consider caching the column index mapping after the first lookup to improve performance.

Copilot uses AI. Check for mistakes.

Copy link
Copy Markdown
OwnerAuthor

Choose a reason for hiding this comment

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

Performance is not in scope at the moment.

Comment threadsrc/schema.zig Outdated
Comment threadsrc/schema.zig

CopilotAI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

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


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

Comment threadsrc/schema.zig
Comment on lines +129 to +143
.bool => {
const lower = std.ascii.allocLowerString(self.allocator, value) catch return TableError.OutOfMemory;
defer self.allocator.free(lower);
for ([_][]const u8{ "true", "1", "yes", "y" }) |true_word| {
if (std.mem.eql(u8, true_word, lower)) {
return true;
}
}
for ([_][]const u8{ "false", "0", "no", "n" }) |false_word| {
if (std.mem.eql(u8, false_word, lower)) {
return false;
}
}
return StructureError.UnexpectedType;
},

CopilotAIJan 18, 2026

Copy link

Choose a reason for hiding this comment

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

Memory allocated by std.ascii.allocLowerString is correctly freed with defer. However, this is called every time a boolean value is parsed. For better performance, consider converting the value to lowercase in-place in a small stack buffer since bool values are typically short strings.

Copilot uses AI. Check for mistakes.

Copy link
Copy Markdown
OwnerAuthor

Choose a reason for hiding this comment

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

Not in scope

Comment threadREADME.md
Comment threadsrc/schema.zig
Comment threadsrc/schema.zig
Comment threadsrc/schema.zig Outdated
Comment threadsrc/schema.zig Outdated
Comment threadsrc/tests/schema.zig
Comment threadsrc/tests/schema.zig
@DISTREAT
DISTREAT merged commit 2291a4f into masterJan 18, 2026
2 checks passed
@DISTREAT
DISTREAT deleted the comptime branch January 18, 2026 02:37
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancementNew feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Support Serialization and Deserialization of CSV Rows via Structs

2 participants

@DISTREAT