Add check for NULL offset for caggs built on top of caggs (#7272)

[zilder]

The check was missing causing instance crash due to null pointer dereference in cases when one cagg was built on top of another and either of them had non-NULL offset and the other had NULL offset. See #7272 for an example.

Closes #7272

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 92.31%. Comparing base (59f50f2) to head (299d5b2).
Report is 362 commits behind head on main.

Additional details and impacted files

@@             Coverage Diff             @@
##             main    #7290       +/-   ##
===========================================
+ Coverage   80.06%   92.31%   +12.25%     
===========================================
  Files         190      205       +15     
  Lines       37181    38597     +1416     
  Branches     9450    10001      +551     
===========================================
+ Hits        29770    35632     +5862     
+ Misses       2997     2961       -36     
+ Partials     4414        4     -4410     

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

[erimatnor]

Approving since this change seems to work, but I have some suggestions for possible improvements for clarity.

[erimatnor]

While this probably works, I am wondering if we shouldn't check this further up around line 1044.

It seems one of the bucket_time_offsets is NULL, so I think we should have NULL check on the original C pointer before trying to convert to a Datum. We should probably not call IntervalPGetDatum() on a NULL pointer to begin with and check the bucket_time_offset_pointer instead. One alternative is to also use a NullableDatum to pass this info down.

[zilder]

Thanks for the review, @erimatnor!

I think using NullableDatum would make sense for datums stored by value. Interval is stored as a pointer so we can just compare it with NULL directly. And IntervalPGetDatum just converts the pointer type to datum, doesn't dereference it.

#define IntervalPGetDatum(X) PointerGetDatum(X)

So while I could implement what you are suggesting, I'm not sure it is really needed here.

[fabriziomello]

@zilder please add a changelog entry in the .unreleased folder and add @snyrkill to the thanks session since he reported the bug.

[timescale-automation]

Automated backport to 2.16.x not done: cherry-pick failed.

Git status

HEAD detached at origin/2.16.x
You are currently cherry-picking commit 7c5dbbbd0.
  (fix conflicts and run "git cherry-pick --continue")
  (use "git cherry-pick --skip" to skip this patch)
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)

Changes to be committed:
	new file:   .unreleased/pr_7290
	modified:   tsl/src/continuous_aggs/common.c

Unmerged paths:
  (use "git add/rm <file>..." as appropriate to mark resolution)
	both modified:   tsl/test/expected/cagg_query.out
	deleted by us:   tsl/test/expected/cagg_query_using_merge.out
	deleted by us:   tsl/test/sql/include/cagg_query_common.sql

Job log