feat: support rows_per_table=0 to allow subtables with zero rows#77
feat: support rows_per_table=0 to allow subtables with zero rows#77YamingPei wants to merge 7 commits into
Conversation
There was a problem hiding this comment.
Code Review
This pull request updates the configuration parser to allow rows_per_table to be set to 0 (previously it had to be positive or -1) and adds corresponding unit tests. The reviewer suggests automatically disabling the data cache when rows_per_table is set to 0 to prevent potential division-by-zero errors or undefined behavior in the caching logic, and recommends adding a test assertion to verify this automatic disabling.
There was a problem hiding this comment.
Pull request overview
Expands GenerationConfig YAML parsing to accept rows_per_table: 0, enabling subtables to be created without any rows inserted. Previously only positive values or -1 (unlimited) were allowed.
Changes:
- Relax validation in
ConfigParsersorows_per_table == 0is accepted; update the error message accordingly. - Add three parser tests covering
rows_per_tablevalues0,-1, and-2(invalid).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/parameter/inc/ConfigParser.hpp | Changes val <= 0 check to val < 0 and updates the error message to "non-negative or -1". |
| src/parameter/test/TestConfigParser.cpp | Adds and registers tests for zero, unlimited, and invalid negative rows_per_table values. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #77 +/- ##
==========================================
- Coverage 86.15% 86.11% -0.04%
==========================================
Files 214 214
Lines 11257 11262 +5
Branches 4913 4804 -109
==========================================
Hits 9698 9698
- Misses 1559 1564 +5
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Description
Support rows_per_table=0 to allow subtables with zero rows.
Issue(s)
Checklist
Please check the items in the checklist if applicable.