Track file trailer only in debug builds #5422
Conversation
Codecov Report
@@ Coverage Diff @@
## main #5422 +/- ##
==========================================
- Coverage 90.68% 90.67% -0.02%
==========================================
Files 227 227
Lines 52618 52613 -5
==========================================
- Hits 47719 47709 -10
- Misses 4899 4904 +5
... and 7 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
jnidzwetzki
force-pushed
the
file_trailer_assert
branch
2 times, most recently
from
bfa8fea
to
21a86a9
Compare
| @@ -449,7 +454,9 @@ copy_fetcher_complete(CopyFetcher *fetcher) | |||
| * nor the expected number of columns. This provides an extra | |||
| * check against somehow getting out of sync with the data. | |||
| */ | |||
| #ifdef USE_ASSERT_CHECKING | |||
| fetcher->file_trailer_received = true; | |||
There was a problem hiding this comment.
Below you have a comment saying that the loop will not be executed in some cases, so maybe we can not execute it always?
The reasons we needed this flag is that the final iteration of the loop was supposed to check for EOF. Now that you are doing this check in place, we don't need the final iteration at all, right?
There was a problem hiding this comment.
This is correct. The final loop should check for EOF but it was not always executed. Now, we perform the read operation directly here and update the EOF state accordingly. I updated the comment to make this easier to understand.
The commit 96574a7 changes the handling of the file_trailer_received flag. It is now only used in asserts and not in any other kind of logic. This patch encapsulates the file_trailer_received in a USE_ASSERT_CHECKING macro.
jnidzwetzki
force-pushed
the
file_trailer_assert
branch
from
21a86a9
to
1f0f3db
Compare
The commit 96574a7 changes the handling of the
file_trailer_receivedflag. Itis now only used in asserts and not in any other kind of logic. This
patch encapsulates the
file_trailer_receivedin aUSE_ASSERT_CHECKINGmacro.
Disable-check: force-changelog-changed