Fix Proposal for Issue 199

[brendonparker]

Fixes #199

Maybe a more elegant way?

Not sure the test I added fits your standards, but gives some idea of what is going on.

[codecov-commenter]

Codecov Report

Attention: Patch coverage is 4.76190% with 20 lines in your changes missing coverage. Please review.

Project coverage is 86.92%. Comparing base (e2e2e71) to head (9e62861).

Files	Patch %	Lines
...B.NET.Data/Internal/Writer/ListVectorDataWriter.cs	4.76%	20 Missing ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@             Coverage Diff             @@
##           develop     #200      +/-   ##
===========================================
- Coverage    88.63%   86.92%   -1.71%     
===========================================
  Files           56       56              
  Lines         1830     1851      +21     
  Branches       252      252              
===========================================
- Hits          1622     1609      -13     
- Misses         150      184      +34     
  Partials        58       58              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[coveralls]

Pull Request Test Coverage Report for Build 9571608010

Details

• 1 of 21 (4.76%) changed or added relevant lines in 1 file are covered.
• 2 unchanged lines in 1 file lost coverage.
• Overall coverage decreased (-1.2%) to 89.316%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
DuckDB.NET.Data/Internal/Writer/ListVectorDataWriter.cs	1	21	4.76%

Files with Coverage Reduction	New Missed Lines	%
DuckDB.NET.Data/Internal/Writer/ListVectorDataWriter.cs	2	74.51%

Totals
Change from base Build 9561880599:	-1.2%
Covered Lines:	1681
Relevant Lines:	1851

---

💛 - Coveralls

[Giorgi]

The code is exactly what I was planning to do. For tests, I wanted to suggest this but it's failing even with the fix. Not sure why.

    [Fact]
    public void ListValuesDoubleNullable()
    {
        ListValuesInternal("Double", faker => faker.Random.Double().OrNull(faker));
    }

[brendonparker]

Thanks for pointing me towards those tests. I took a quick look but it isn't apparent to me either :/

I think it may be memory related? At least on my machine the test either fails with:

The active test run was aborted. Reason: Test host process crashed

or

The active test run was aborted. Reason: Test host process crashed : Fatal error. System.AccessViolationException: Attempted to read or write protected memory. This is often an indication that other memory is corrupt.

If I change the rows from 2000 to 10 then it works/passes 🤔

[Giorgi]

I have noticed that the test runners report a wrong message when Access Violation or some other low-level error causes the failure. You'll actually need to debug the test to see what error is thrown and where. I'm also not sure why changing the row number helps. Probably it's generating some edge case data when the number of rows is higher.

[brendonparker]

Agree... not a change you want to leave/keep. I just added it to illustrate.

I need to take a break for a while. I may poke at it a bit more later to try and figure out what is going wrong.

[coveralls]

Pull Request Test Coverage Report for Build 9572221321

Details

• 1 of 21 (4.76%) changed or added relevant lines in 1 file are covered.
• 16 unchanged lines in 2 files lost coverage.
• Overall coverage decreased (-2.1%) to 88.39%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
DuckDB.NET.Data/Internal/Writer/ListVectorDataWriter.cs	1	21	4.76%

Files with Coverage Reduction	New Missed Lines	%
DuckDB.NET.Data/Internal/Writer/VectorDataWriterBase.cs	2	79.82%
DuckDB.NET.Data/Internal/Writer/ListVectorDataWriter.cs	14	62.75%

Totals
Change from base Build 9561880599:	-2.1%
Covered Lines:	1667
Relevant Lines:	1851

---

💛 - Coveralls

[Giorgi]

Found the issue!