Fix segfault when deleting from compressed chunk

[sb230132]

During UPDATE/DELETE on compressed hypertables, we iterate over plan
tree to collect all scan nodes. For each scan nodes there can be
filter conditions.

Prior to this patch we collect only first filter condition and use
for first chunk which may be wrong. In this patch as and when we
encounter a target scan node, we immediately process those chunks.

Fixes #5640

[codecov]

Codecov Report

Merging #5643 (6feab56) into main (90f585e) will decrease coverage by 0.03%.
The diff coverage is 97.14%.

@@            Coverage Diff             @@
##             main    #5643      +/-   ##
==========================================
- Coverage   90.98%   90.96%   -0.03%     
==========================================
  Files         230      230              
  Lines       54460    54446      -14     
==========================================
- Hits        49550    49525      -25     
- Misses       4910     4921      +11     

Impacted Files	Coverage Δ
tsl/src/compression/compression.h	0.00% <ø> (ø)
tsl/src/init.c	96.29% <ø> (ø)
tsl/src/compression/compression.c	95.79% <96.96%> (-0.06%)	⬇️
src/nodes/hypertable_modify.c	72.95% <100.00%> (-0.64%)	⬇️

... and 6 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

[github-actions]

@mkindahl, @gayyappan: please review this pull request.

Powered by pull-review

[antekresic]

Needs tests that cover this specific issue.

[sb230132]

Needs tests that cover this specific issue.

For segfault there don't seem to be any testcase for now. However i found testcase which results in unwanted decompression on join tables. For this i have added testcase.

[svenklemm]

This only fixes part of the issues mentioned by the ticket and does not adress the problems with JOINS.

[mkindahl]

Not sure why you disable the changelog check. You need to add one eventually anyway.

[sb230132]

Not sure why you disable the changelog check. You need to add one eventually anyway.

Fixed.

[sb230132]

Not sure why you disable the changelog check. You need to add one eventually anyway.

This only fixes part of the issues mentioned by the ticket and does not adress the problems with JOINS.

For segfault, iam unable to get a testcase. However now i save filters for each chunk separately and process each chunk against its own filters so, the issue of segfault occurring due to mismatch in attribute number should not occur.

[svenklemm]

Not sure why you disable the changelog check. You need to add one eventually anyway.

We don't need changelog entry since the bug is not present in any released version.

[antekresic]

Looks good, we can postpone compression test refactoring in a followup PR.

[svenklemm]

Some bugs will need to be fixed in followup PRs but approving to unblock other followup PRs.

[kgyrtkirk]

I was thinking that I've already posted this....can be handled later as a follow up as well

[kgyrtkirk]

what's the return value contract of this method? (its not documented...)

...but anyway - I think it would be better to instead of passing a writeable bool ptr; have that as return value?

note that: currently I see 3 return-s in this method; from which:

• 2 doesn't free scankeys
  • not necessarily an issue - I think you can just remove the if+pfree
• 1 may not end with an ERROR ; and can exit the method without closing the heapScan - what could be the consequences of that?

...and I now wonder why that method tries to mask concurrent deletes/updates on the compressed table - instead of erroring out straight away? as it is now it may leave the table in an inconsistent state...

                        if (IsolationUsesXactSnapshot())
                                ereport(ERROR,
                                                (errcode(ERRCODE_T_R_SERIALIZATION_FAILURE),
                                                 errmsg("could not serialize access due to concurrent update")));

[timescale-automation]

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

Git status

HEAD detached at origin/2.10.x
You are currently cherry-picking commit 769f9fe6.
  (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:
	modified:   tsl/test/sql/compression.sql

Unmerged paths:
  (use "git add/rm <file>..." as appropriate to mark resolution)
	both modified:   src/cross_module_fn.h
	both modified:   src/nodes/hypertable_modify.c
	both modified:   tsl/src/compression/compression.c
	both modified:   tsl/src/compression/compression.h
	both modified:   tsl/src/init.c
	both modified:   tsl/test/expected/compression.out
	deleted by us:   tsl/test/expected/compression_update_delete.out
	deleted by us:   tsl/test/sql/compression_update_delete.sql

Job log