Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #73 +/- ##
==========================================
+ Coverage 86.02% 86.26% +0.23%
==========================================
Files 92 97 +5
Lines 10291 10678 +387
Branches 537 585 +48
==========================================
+ Hits 8853 9211 +358
- Misses 1413 1428 +15
- Partials 25 39 +14
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
Adds CSV export support to the Quiver database layer, including foreign-key label resolution, date/time formatting, and enum-to-label mapping, along with a comprehensive new test suite and a new third-party CSV dependency.
Changes:
- Implement
Database::export_to_csv()and CSV writer utility (write_csv) using rapidcsv. - Add
EnumMapandValuestreaming support to enable enum label export and generic value-to-string conversion. - Add extensive CSV export tests and wire them into the test build.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/test_database_csv.cpp | New tests covering CSV export scenarios (basic, empty, FK resolution, quoting, datetime formatting, enums, nulls). |
| tests/CMakeLists.txt | Adds the new CSV test file to the test target. |
| src/database.cpp | Implements Database::export_to_csv() and updates CSV import stub logging/validation. |
| src/csv.cpp | Adds CSV writing implementation (FK/enum resolution, datetime formatting, null handling). |
| include/quiver/csv.h | Introduces CSV export API/types (DateFormatMap, FkLabelMap, write_csv). |
| include/quiver/enum_map.h / src/enum_map.cpp | Adds enum mapping type used by CSV export. |
| include/quiver/value.h / src/value.cpp | Adds operator<< for Value to support stringification. |
| include/quiver/schema.h | Adds TableDefinition::column_names() helper. |
| include/quiver/database.h | Extends export_to_csv signature to accept date format and enum map parameters. |
| src/CMakeLists.txt | Adds new source files and links rapidcsv. |
| cmake/Dependencies.cmake | Fetches rapidcsv via FetchContent. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const std::vector<std::string> column_names() const { | ||
| std::vector<std::string> names; | ||
| for (const auto& [col_name, col_def] : columns) { | ||
| names.push_back(col_name); | ||
| } | ||
| return names; | ||
| } |
There was a problem hiding this comment.
column_names() returns const std::vector<std::string> by value. Returning a const value is atypical and can inhibit moves/assignments for callers. Prefer returning a non-const std::vector<std::string> by value (or provide an output iterator/view if you want to avoid allocations).
| #include "quiver/value.h" | ||
|
|
||
| namespace quiver { | ||
|
|
||
| std::ostream& operator<<(std::ostream& os, const Value& val) { | ||
| std::visit( | ||
| [&os](const auto& v) { | ||
| using T = std::decay_t<decltype(v)>; | ||
| if constexpr (!std::is_same_v<T, std::nullptr_t>) { | ||
| os << v; | ||
| } | ||
| }, | ||
| val); |
There was a problem hiding this comment.
value.cpp uses std::decay_t and std::is_same_v but does not include <type_traits>. This relies on transitive includes from other headers, which is not guaranteed across standard library implementations. Add the missing include to make this translation unit self-contained.
1 participant
No description provided.