-
Notifications
You must be signed in to change notification settings - Fork 885
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add Ensure to fix coverity defect #7338
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #7338 +/- ##
==========================================
+ Coverage 80.06% 82.59% +2.52%
==========================================
Files 190 229 +39
Lines 37181 42727 +5546
Branches 9450 10734 +1284
==========================================
+ Hits 29770 35290 +5520
- Misses 2997 3159 +162
+ Partials 4414 4278 -136 ☔ View full report in Codecov by Sentry. |
src/import/ht_hypertable_modify.c
Outdated
#if PG17_GE | ||
if (oldtuple != NULL) | ||
ExecForceStoreHeapTuple(oldtuple, resultRelInfo->ri_oldTupleSlot, | ||
false); | ||
else | ||
#endif |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why was this added? This commit only talks about adding an Ensure
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added to match upstream. Let me mention this in the commit message as well for clarity.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmmm... Even those are very simple changes I think it should be splitted into two different PRs:
- with the coverity fix
- with the upstream stuff adding proper PG17 label (it is good to have it separately for future checks)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've split this into two PRs.
c9aa746
to
6c49b66
Compare
Coverity detected a possible null pointer dereference. It doesn't seem like this can be triggered, so added an `Ensure` clause.
6c49b66
to
42c22ad
Compare
Coverity detected a possible null pointer dereference. It doesn't seem like this can be triggered, so added an
Ensure
clause.Disable-check: force-changelog-file