Skip to content
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

Respect other extensions' ExecutorStart hooks #7712

Merged
merged 1 commit into from
Feb 16, 2025

Conversation

zilder
Copy link
Contributor

@zilder zilder commented Feb 14, 2025

A fix for #7667.

When we override ExecutorStart hook that has been set by another extension we have to chain-call it not to disrupt other extension's integrity.

Copy link

@akuzm, @svenklemm: please review this pull request.

Powered by pull-review

@github-actions github-actions bot requested a review from akuzm February 14, 2025 14:46
@zilder zilder force-pushed the zilder/chain_executor_start_hook branch from ccac202 to 34fa509 Compare February 14, 2025 14:49
@zilder
Copy link
Contributor Author

zilder commented Feb 14, 2025

Similar to #7694

@zilder zilder requested a review from mkindahl February 14, 2025 14:56
Copy link
Contributor

@mkindahl mkindahl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Strictly speaking, you do not need two variables for this, but patch looks good so approved.

Copy link

codecov bot commented Feb 14, 2025

Codecov Report

Attention: Patch coverage is 71.42857% with 2 lines in your changes missing coverage. Please review.

Project coverage is 81.92%. Comparing base (59f50f2) to head (c863841).
Report is 767 commits behind head on main.

Files with missing lines Patch % Lines
tsl/src/hypercore/attr_capture.c 71.42% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #7712      +/-   ##
==========================================
+ Coverage   80.06%   81.92%   +1.86%     
==========================================
  Files         190      246      +56     
  Lines       37181    45116    +7935     
  Branches     9450    11256    +1806     
==========================================
+ Hits        29770    36963    +7193     
- Misses       2997     3734     +737     
- Partials     4414     4419       +5     

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

@zilder zilder force-pushed the zilder/chain_executor_start_hook branch from 34fa509 to d7a4adb Compare February 16, 2025 18:59
When we override ExecutorStart hook that has been set by another
extension we have to chain-call it not to disrupt other extension's
integrity.
@zilder zilder force-pushed the zilder/chain_executor_start_hook branch from d7a4adb to c863841 Compare February 16, 2025 19:14
@zilder zilder merged commit 0fc733e into timescale:main Feb 16, 2025
44 of 45 checks passed
@svenklemm svenklemm added this to the v2.18.2 milestone Feb 18, 2025
svenklemm added a commit to svenklemm/timescaledb that referenced this pull request Feb 19, 2025
This release contains performance improvements and bug fixes since
the 2.18.1 release. We recommend that you upgrade at the next
available opportunity.

**Bugfixes**
* timescale#7686 Potential wrong aggregation result when using vectorized aggregation with hash grouping in reverse order
* timescale#7694 Fix ExplainHook breaking call chain
* timescale#7695 Block dropping internal compressed chunks with `drop_chunk()`
* timescale#7711 License error when using hypercore handler
* timescale#7712 Respect other extensions' ExecutorStart hooks

**Thanks**
* @davidmehren and @jflambert for reporting an issue with extension hooks
* @jflambert for reporting a bug with license errors shown in autovacuum
svenklemm added a commit to svenklemm/timescaledb that referenced this pull request Feb 19, 2025
This release contains performance improvements and bug fixes since
the 2.18.1 release. We recommend that you upgrade at the next
available opportunity.

**Bugfixes**
* timescale#7686 Potential wrong aggregation result when using vectorized aggregation with hash grouping in reverse order
* timescale#7694 Fix ExplainHook breaking call chain
* timescale#7695 Block dropping internal compressed chunks with `drop_chunk()`
* timescale#7711 License error when using hypercore handler
* timescale#7712 Respect other extensions' ExecutorStart hooks

**Thanks**
* @davidmehren and @jflambert for reporting an issue with extension hooks
* @jflambert for reporting a bug with license errors shown in autovacuum
@svenklemm svenklemm mentioned this pull request Feb 19, 2025
svenklemm added a commit to svenklemm/timescaledb that referenced this pull request Feb 19, 2025
This release contains performance improvements and bug fixes since
the 2.18.1 release. We recommend that you upgrade at the next
available opportunity.

**Bugfixes**
* timescale#7686 Potential wrong aggregation result when using vectorized aggregation with hash grouping in reverse order
* timescale#7694 Fix ExplainHook breaking call chain
* timescale#7695 Block dropping internal compressed chunks with `drop_chunk()`
* timescale#7711 License error when using hypercore handler
* timescale#7712 Respect other extensions' ExecutorStart hooks

**Thanks**
* @davidmehren and @jflambert for reporting an issue with extension hooks
* @jflambert for reporting a bug with license errors shown in autovacuum
svenklemm added a commit that referenced this pull request Feb 19, 2025
This release contains performance improvements and bug fixes since
the 2.18.1 release. We recommend that you upgrade at the next
available opportunity.

**Bugfixes**
* #7686 Potential wrong aggregation result when using vectorized aggregation with hash grouping in reverse order
* #7694 Fix ExplainHook breaking call chain
* #7695 Block dropping internal compressed chunks with `drop_chunk()`
* #7711 License error when using hypercore handler
* #7712 Respect other extensions' ExecutorStart hooks

**Thanks**
* @davidmehren and @jflambert for reporting an issue with extension hooks
* @jflambert for reporting a bug with license errors shown in autovacuum
svenklemm added a commit to svenklemm/timescaledb that referenced this pull request Feb 19, 2025
This release contains performance improvements and bug fixes since
the 2.18.1 release. We recommend that you upgrade at the next
available opportunity.

**Bugfixes**
* timescale#7686 Potential wrong aggregation result when using vectorized aggregation with hash grouping in reverse order
* timescale#7694 Fix ExplainHook breaking call chain
* timescale#7695 Block dropping internal compressed chunks with `drop_chunk()`
* timescale#7711 License error when using hypercore handler
* timescale#7712 Respect other extensions' ExecutorStart hooks

**Thanks**
* @davidmehren and @jflambert for reporting an issue with extension hooks
* @jflambert for reporting a bug with license errors shown in autovacuum
@svenklemm svenklemm mentioned this pull request Feb 19, 2025
svenklemm added a commit that referenced this pull request Feb 19, 2025
This release contains performance improvements and bug fixes since
the 2.18.1 release. We recommend that you upgrade at the next
available opportunity.

**Bugfixes**
* #7686 Potential wrong aggregation result when using vectorized aggregation with hash grouping in reverse order
* #7694 Fix ExplainHook breaking call chain
* #7695 Block dropping internal compressed chunks with `drop_chunk()`
* #7711 License error when using hypercore handler
* #7712 Respect other extensions' ExecutorStart hooks

**Thanks**
* @davidmehren and @jflambert for reporting an issue with extension hooks
* @jflambert for reporting a bug with license errors shown in autovacuum
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants