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.

Sign up for GitHub

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

Jump to bottom

Fix havingqual processing for caggs #3430

Merged
merged 1 commit into from
Aug 17, 2021
Merged

Fix havingqual processing for caggs #3430

merged 1 commit into from
Aug 17, 2021

Conversation

gayyappan
Copy link
Contributor

If the targetlist for the cagg query has both subexprs and exprs
from the having clause, the havingqual for the partial view
is generated incorrectly. Fix this issue by checking havingqual
against all the entries in the targetlist instead of first match.

Fixes #2655

@gayyappan gayyappan requested a review from a team as a code owner July 20, 2021 21:31
@gayyappan gayyappan requested review from erimatnor, mkindahl and nikkhils and removed request for a team July 20, 2021 21:31
@gayyappan gayyappan self-assigned this Jul 20, 2021
@codecov
Copy link

codecov bot commented Jul 20, 2021 •
edited

Codecov Report

Merging #3430 (eb9a60d) into master (bff87a8) will decrease coverage by 0.02%.
The diff coverage is 100.00%.

❗ Current head eb9a60d differs from pull request most recent head 657085c. Consider uploading reports for the commit 657085c to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3430      +/-   ##
==========================================
- Coverage   90.73%   90.71%   -0.03%     
==========================================
  Files         212      212              
  Lines       36258    36267       +9     
==========================================
- Hits        32900    32898       -2     
- Misses       3358     3369      +11
Impacted Files Coverage Δ
tsl/src/continuous_aggs/create.c 96.70% <100.00%> (-0.22%) ⬇️
src/loader/bgw_launcher.c 89.47% <0.00%> (-2.48%) ⬇️
tsl/src/nodes/data_node_dispatch.c 97.08% <0.00%> (-0.25%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c221c78...657085c. Read the comment docs.

mkindahl
mkindahl reviewed Jul 21, 2021
View reviewed changes
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.

LGTM, but can you add a test case for the situation you describe and add some clarifications to the comment so that it is easier to follow?

tsl/src/continuous_aggs/create.c Outdated Show resolved Hide resolved
@gayyappan gayyappan requested a review from mkindahl July 21, 2021 15:27
mkindahl
mkindahl approved these changes Jul 21, 2021
View reviewed changes
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.

LGTM

@gayyappan gayyappan marked this pull request as draft July 23, 2021 18:28
@gayyappan gayyappan force-pushed the bugfix_3 branch 4 times, most recently from 5260ebf to 8075b07 Compare August 12, 2021 18:42
@gayyappan gayyappan marked this pull request as ready for review August 12, 2021 18:52
@gayyappan
Fix havingqual processing for caggs
657085c 
If the targetlist for the cagg query has both subexprs and exprs
from the having clause, the havingqual for the partial view
is generated incorrectly. Fix this issue by checking havingqual
against all the entries in the targetlist instead of first match.

Fixes timescale#2655
@gayyappan gayyappan merged commit 217ba46 into timescale:master Aug 17, 2021
mkindahl added a commit to mkindahl/timescaledb that referenced this pull request Aug 19, 2021
@mkindahl
Release 2.4.1
616a657 
This release contains bug fixes since the 2.4.0 release. We deem it
high priority to upgrade since it is needed to support PostgreSQL 12.8
and 13.4.

**Bugfixes**
* timescale#3430 Fix havingqual processing for continuous aggregates
* timescale#3468 Disable tests by default if tools are not found
* timescale#3468 Fix crash while tracking alter table commands
* timescale#3494 Improve error message when adding data nodes
* timescale#3498 Fix continuous agg bgw job failure for PG 12.8 and 13.4

**Thanks**
* @brianbenns for reporting a segfault with continuous aggregates
mkindahl added a commit to mkindahl/timescaledb that referenced this pull request Aug 19, 2021
@mkindahl
Release 2.4.1
f932187 
This release contains bug fixes since the 2.4.0 release. We deem it
high priority to upgrade since it is needed to support PostgreSQL 12.8
and 13.4.

**Bugfixes**
* timescale#3430 Fix havingqual processing for continuous aggregates
* timescale#3468 Disable tests by default if tools are not found
* timescale#3468 Fix crash while tracking alter table commands
* timescale#3494 Improve error message when adding data nodes
* timescale#3498 Fix continuous agg bgw job failure for PG 12.8 and 13.4

**Thanks**
* @brianbenns for reporting a segfault with continuous aggregates
mkindahl added a commit to mkindahl/timescaledb that referenced this pull request Aug 19, 2021
@mkindahl
Release 2.4.1
ce1c1aa 
This release contains bug fixes since the 2.4.0 release. We deem it
high priority to upgrade since it is needed to support PostgreSQL 12.8
and 13.4.

**Bugfixes**
* timescale#3430 Fix havingqual processing for continuous aggregates
* timescale#3468 Disable tests by default if tools are not found
* timescale#3468 Fix crash while tracking alter table commands
* timescale#3494 Improve error message when adding data nodes
* timescale#3498 Fix continuous agg bgw job failure for PG 12.8 and 13.4

**Thanks**
* @brianbenns for reporting a segfault with continuous aggregates
mkindahl added a commit to mkindahl/timescaledb that referenced this pull request Aug 19, 2021
@mkindahl
Release 2.4.1
743a661 
This release contains bug fixes since the 2.4.0 release. We deem it
high priority to upgrade since it is needed to support PostgreSQL 12.8
and 13.4.

**Bugfixes**
* timescale#3430 Fix havingqual processing for continuous aggregates
* timescale#3468 Disable tests by default if tools are not found
* timescale#3468 Fix crash while tracking alter table commands
* timescale#3494 Improve error message when adding data nodes
* timescale#3498 Fix continuous agg bgw job failure for PG 12.8 and 13.4

**Thanks**
* @brianbenns for reporting a segfault with continuous aggregates
mkindahl added a commit to mkindahl/timescaledb that referenced this pull request Aug 19, 2021
@mkindahl
Release 2.4.1
0433083 
This release contains bug fixes since the 2.4.0 release. We deem it
high priority to upgrade since it is needed to support PostgreSQL 12.8
and 13.4.

**Bugfixes**
* timescale#3430 Fix havingqual processing for continuous aggregates
* timescale#3468 Disable tests by default if tools are not found
* timescale#3468 Fix crash while tracking alter table commands
* timescale#3494 Improve error message when adding data nodes
* timescale#3498 Fix continuous agg bgw job failure for PG 12.8 and 13.4

**Thanks**
* @brianbenns for reporting a segfault with continuous aggregates
mkindahl added a commit to mkindahl/timescaledb that referenced this pull request Aug 19, 2021
@mkindahl
Release 2.4.1
2ee0252 
This release contains bug fixes since the 2.4.0 release. We deem it
high priority to upgrade since it is needed to support PostgreSQL 12.8
and 13.4.

**Bugfixes**
* timescale#3430 Fix havingqual processing for continuous aggregates
* timescale#3468 Disable tests by default if tools are not found
* timescale#3468 Fix crash while tracking alter table commands
* timescale#3494 Improve error message when adding data nodes
* timescale#3498 Fix continuous agg bgw job failure for PG 12.8 and 13.4

**Thanks**
* @brianbenns for reporting a segfault with continuous aggregates
mkindahl added a commit to mkindahl/timescaledb that referenced this pull request Aug 19, 2021
@mkindahl
Release 2.4.1
2b5170c 
This release contains bug fixes since the 2.4.0 release. We deem it
high priority to upgrade since it is needed to support PostgreSQL 12.8
and 13.4.

**Bugfixes**
* timescale#3430 Fix havingqual processing for continuous aggregates
* timescale#3468 Disable tests by default if tools are not found
* timescale#3468 Fix crash while tracking alter table commands
* timescale#3494 Improve error message when adding data nodes
* timescale#3498 Fix continuous agg bgw job failure for PG 12.8 and 13.4

**Thanks**
* @brianbenns for reporting a segfault with continuous aggregates
mkindahl added a commit to mkindahl/timescaledb that referenced this pull request Aug 19, 2021
@mkindahl
Release 2.4.1
80b48fa 
This release contains bug fixes since the 2.4.0 release. We deem it
high priority to upgrade since it is needed to support PostgreSQL 12.8
and 13.4.

**Bugfixes**
* timescale#3430 Fix havingqual processing for continuous aggregates
* timescale#3468 Disable tests by default if tools are not found
* timescale#3468 Fix crash while tracking alter table commands
* timescale#3494 Improve error message when adding data nodes
* timescale#3498 Fix continuous agg bgw job failure for PG 12.8 and 13.4

**Thanks**
* @brianbenns for reporting a segfault with continuous aggregates
mkindahl added a commit to mkindahl/timescaledb that referenced this pull request Aug 19, 2021
@mkindahl
Release 2.4.1
107f241 
This release contains bug fixes since the 2.4.0 release. We deem it
high priority to upgrade since it is needed to support PostgreSQL 12.8
and 13.4.

**Bugfixes**
* timescale#3430 Fix havingqual processing for continuous aggregates
* timescale#3468 Disable tests by default if tools are not found
* timescale#3468 Fix crash while tracking alter table commands
* timescale#3494 Improve error message when adding data nodes
* timescale#3498 Fix continuous agg bgw job failure for PG 12.8 and 13.4

**Thanks**
* @brianbenns for reporting a segfault with continuous aggregates
mkindahl added a commit to mkindahl/timescaledb that referenced this pull request Aug 19, 2021
@mkindahl
Release 2.4.1
facea96 
This release contains bug fixes since the 2.4.0 release.  We deem it
high priority to upgrade.

The release fixes continous aggregate refresh for postgres 12.8 and
13.4, a crash with ALTER TABLE commands and a crash with continuous
aggregates with HAVING clause.

**Bugfixes**
* timescale#3430 Fix havingqual processing for continuous aggregates
* timescale#3468 Disable tests by default if tools are not found
* timescale#3462 Fix crash while tracking alter table commands
* timescale#3489 Fix continuous agg bgw job failure for PG 12.8 and 13.4
* timescale#3494 Improve error message when adding data nodes

**Thanks**
* @brianbenns for reporting a segfault with continuous aggregates
* @brianbenns for reporting a segfault with continuous aggregates
* @usego for reporting an issue with continuous aggregate refresh on PG 13.4
mkindahl added a commit that referenced this pull request Aug 19, 2021
@mkindahl
Release 2.4.1
4e0954e 
This release contains bug fixes since the 2.4.0 release.  We deem it
high priority to upgrade.

The release fixes continous aggregate refresh for postgres 12.8 and
13.4, a crash with ALTER TABLE commands and a crash with continuous
aggregates with HAVING clause.

**Bugfixes**
* #3430 Fix havingqual processing for continuous aggregates
* #3468 Disable tests by default if tools are not found
* #3462 Fix crash while tracking alter table commands
* #3489 Fix continuous agg bgw job failure for PG 12.8 and 13.4
* #3494 Improve error message when adding data nodes

**Thanks**
* @brianbenns for reporting a segfault with continuous aggregates
* @brianbenns for reporting a segfault with continuous aggregates
* @usego for reporting an issue with continuous aggregate refresh on PG 13.4
mkindahl added a commit to mkindahl/timescaledb that referenced this pull request Aug 19, 2021
@mkindahl
Release 2.4.1
dd29b85 
This release contains bug fixes since the 2.4.0 release.  We deem it
high priority to upgrade.

The release fixes continous aggregate refresh for postgres 12.8 and
13.4, a crash with ALTER TABLE commands and a crash with continuous
aggregates with HAVING clause.

**Bugfixes**
* timescale#3430 Fix havingqual processing for continuous aggregates
* timescale#3468 Disable tests by default if tools are not found
* timescale#3462 Fix crash while tracking alter table commands
* timescale#3489 Fix continuous agg bgw job failure for PG 12.8 and 13.4
* timescale#3494 Improve error message when adding data nodes

**Thanks**
* @brianbenns for reporting a segfault with continuous aggregates
* @brianbenns for reporting a segfault with continuous aggregates
* @usego for reporting an issue with continuous aggregate refresh on PG 13.4
mkindahl added a commit that referenced this pull request Aug 19, 2021
@mkindahl
Release 2.4.1
818a232 
This release contains bug fixes since the 2.4.0 release.  We deem it
high priority to upgrade.

The release fixes continous aggregate refresh for postgres 12.8 and
13.4, a crash with ALTER TABLE commands and a crash with continuous
aggregates with HAVING clause.

**Bugfixes**
* #3430 Fix havingqual processing for continuous aggregates
* #3468 Disable tests by default if tools are not found
* #3462 Fix crash while tracking alter table commands
* #3489 Fix continuous agg bgw job failure for PG 12.8 and 13.4
* #3494 Improve error message when adding data nodes

**Thanks**
* @brianbenns for reporting a segfault with continuous aggregates
* @usego for reporting an issue with continuous aggregate refresh on PG 13.4
mkindahl added a commit that referenced this pull request Aug 19, 2021
@mkindahl
Release 2.4.1
63a6585 
This release contains bug fixes since the 2.4.0 release.  We deem it
high priority to upgrade.

The release fixes continous aggregate refresh for postgres 12.8 and
13.4, a crash with ALTER TABLE commands and a crash with continuous
aggregates with HAVING clause.

**Bugfixes**
* #3430 Fix havingqual processing for continuous aggregates
* #3468 Disable tests by default if tools are not found
* #3462 Fix crash while tracking alter table commands
* #3489 Fix continuous agg bgw job failure for PG 12.8 and 13.4
* #3494 Improve error message when adding data nodes

**Thanks**
* @brianbenns for reporting a segfault with continuous aggregates
* @brianbenns for reporting a segfault with continuous aggregates
* @usego for reporting an issue with continuous aggregate refresh on PG 13.4
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mkindahl mkindahl mkindahl approved these changes

@svenklemm svenklemm svenklemm approved these changes

@erimatnor erimatnor Awaiting requested review from erimatnor

@nikkhils nikkhils Awaiting requested review from nikkhils

Assignees

@gayyappan gayyappan

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Segmentation Fault on Continuous Aggregate Data Refresh
3 participants
@gayyappan @mkindahl @svenklemm