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

Fix year not multiple of day/month in Hierarchical CAggs #5255

Merged
merged 1 commit into from Feb 2, 2023

Conversation

konskov
Copy link
Contributor

@konskov konskov commented Jan 31, 2023

Previously all intervals were converted to seconds using "epoch"
with date_part. However, this treats a year as 365.25 days to
account for leap years, leading to the unexpected situation that
a year is not a multiple of a day or a month.

Fixed by treating month-only intervals as multiples of 30 days.

Fixes #5231

@konskov konskov force-pushed the fix_year_multiple branch 2 times, most recently from de154a8 to 6a38429 Compare January 31, 2023 08:48
@codecov
Copy link

codecov bot commented Jan 31, 2023

Codecov Report

Merging #5255 (5b60343) into main (9133319) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #5255   +/-   ##
=======================================
  Coverage   89.00%   89.00%           
=======================================
  Files         225      225           
  Lines       51838    51842    +4     
=======================================
+ Hits        46138    46144    +6     
+ Misses       5700     5698    -2     
Impacted Files Coverage Δ
tsl/src/continuous_aggs/create.c 88.11% <100.00%> (+0.04%) ⬆️
src/loader/bgw_message_queue.c 86.36% <0.00%> (-2.85%) ⬇️
src/bgw/scheduler.c 83.88% <0.00%> (-1.28%) ⬇️
tsl/src/bgw_policy/job.c 87.45% <0.00%> (-0.05%) ⬇️
tsl/src/reorder.c 85.71% <0.00%> (+0.21%) ⬆️
tsl/src/fdw/relinfo.c 95.87% <0.00%> (+5.67%) ⬆️

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 9133319...5b60343. Read the comment docs.

@konskov konskov changed the title treat all months as 30 days Fix year not multiple of day/month in nested CAgg Jan 31, 2023
@konskov konskov force-pushed the fix_year_multiple branch 2 times, most recently from eb58ef0 to 42cc548 Compare January 31, 2023 09:02
@konskov konskov marked this pull request as ready for review January 31, 2023 09:02
@github-actions
Copy link

@RafiaSabih, @fabriziomello: please review this pull request.

Powered by pull-review

Copy link
Contributor

@fabriziomello fabriziomello left a comment

Choose a reason for hiding this comment

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

You should also add those new tests to cagg_on_cagg_dist_ht.sql

@konskov konskov force-pushed the fix_year_multiple branch 2 times, most recently from 02efd48 to b7c065b Compare February 1, 2023 13:25
@konskov
Copy link
Contributor Author

konskov commented Feb 1, 2023

Added the tests to cagg_on_cagg_dist_ht as well.

Previously all intervals were converted to seconds using "epoch"
with date_part. However, this treats a year as 365.25 days to
account for leap years, leading to the unexpected situation that
a year is not a multiple of a day or a month.

Fixed by treating month-only intervals as multiples of 30 days.

Fixes timescale#5231
@konskov konskov added this to the TimescaleDB 2.9.3 milestone Feb 2, 2023
@konskov konskov merged commit 6bc8980 into timescale:main Feb 2, 2023
@timescale-automation
Copy link

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

Git status

HEAD detached at origin/2.9.x
You are currently cherry-picking commit 6bc89802.
  (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/src/continuous_aggs/create.c
	modified:   tsl/test/expected/cagg_on_cagg.out
	modified:   tsl/test/expected/cagg_on_cagg_dist_ht.out
	modified:   tsl/test/sql/cagg_on_cagg.sql
	modified:   tsl/test/sql/cagg_on_cagg_dist_ht.sql

Unmerged paths:
  (use "git add <file>..." to mark resolution)
	both modified:   CHANGELOG.md


Job log

@timescale-automation timescale-automation added the auto-backport-not-done Automated backport of this PR has failed non-retriably (e.g. conflicts) label Feb 2, 2023
@akuzm akuzm removed the auto-backport-not-done Automated backport of this PR has failed non-retriably (e.g. conflicts) label Feb 2, 2023
@akuzm
Copy link
Member

akuzm commented Feb 2, 2023

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

Git status

HEAD detached at akuzm/2.9.x
You are currently cherry-picking commit 6bc898021.
  (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/src/continuous_aggs/create.c
	modified:   tsl/test/expected/cagg_on_cagg.out
	modified:   tsl/test/expected/cagg_on_cagg_dist_ht.out
	modified:   tsl/test/sql/cagg_on_cagg.sql
	modified:   tsl/test/sql/cagg_on_cagg_dist_ht.sql

Unmerged paths:
  (use "git add <file>..." to mark resolution)
	both modified:   CHANGELOG.md

Untracked files:
  (use "git add <file>..." to include in what will be committed)
	tsl/src/remote/row_by_row_fetcher.c
	tsl/src/remote/row_by_row_fetcher.h
	tsl/test/sql/dist_remote_error-15.sql
	tsl/test/sql/dist_remote_error.text


@akuzm akuzm added auto-backport-not-done Automated backport of this PR has failed non-retriably (e.g. conflicts) and removed auto-backport-not-done Automated backport of this PR has failed non-retriably (e.g. conflicts) labels Feb 2, 2023
lkshminarayanan added a commit to lkshminarayanan/timescaledb that referenced this pull request Feb 3, 2023
This release contains bug fixes since the 2.9.2 release.
We recommend that you upgrade at the next available opportunity.

**Bugfixes**
* timescale#4804 Skip bucketing when start or end of refresh job is null
* timescale#5108 Fix column ordering in compressed table index
* timescale#5187 Don't enable clang-tidy by default
* timescale#5255 Fix year not multiple of day/month in nested CAgg
* timescale#5259 Lock down search_path in SPI calls
@lkshminarayanan lkshminarayanan mentioned this pull request Feb 3, 2023
lkshminarayanan added a commit to lkshminarayanan/timescaledb that referenced this pull request Feb 3, 2023
This release contains bug fixes since the 2.9.2 release.
We recommend that you upgrade at the next available opportunity.

**Bugfixes**
* timescale#4804 Skip bucketing when start or end of refresh job is null
* timescale#5108 Fix column ordering in compressed table index
* timescale#5187 Don't enable clang-tidy by default
* timescale#5255 Fix year not multiple of day/month in nested CAgg
* timescale#5259 Lock down search_path in SPI calls
lkshminarayanan added a commit to lkshminarayanan/timescaledb that referenced this pull request Feb 3, 2023
This release contains a critical securtiy fix(timescale#5259) and other bug fixes since
the 2.9.2 release.

This release is high priority for upgrade. We strongly recommend that you
upgrade as soon as possible.

**Bugfixes**
* timescale#4804 Skip bucketing when start or end of refresh job is null
* timescale#5108 Fix column ordering in compressed table index
* timescale#5187 Don't enable clang-tidy by default
* timescale#5255 Fix year not multiple of day/month in nested CAgg
* timescale#5259 Lock down search_path in SPI calls
lkshminarayanan added a commit to lkshminarayanan/timescaledb that referenced this pull request Feb 3, 2023
This release contains a critical securtiy fix(timescale#5259) and other bug fixes since
the 2.9.2 release.

This release is high priority for upgrade. We strongly recommend that you
upgrade as soon as possible.

**Bugfixes**
* timescale#4804 Skip bucketing when start or end of refresh job is null
* timescale#5108 Fix column ordering in compressed table index
* timescale#5187 Don't enable clang-tidy by default
* timescale#5255 Fix year not multiple of day/month in nested CAgg
* timescale#5259 Lock down search_path in SPI calls
lkshminarayanan added a commit to lkshminarayanan/timescaledb that referenced this pull request Feb 3, 2023
This release contains a critical securtiy fix(timescale#5259) and other bug fixes
since the 2.9.2 release.

This release is high priority for upgrade. We strongly recommend that
you upgrade as soon as possible.

**Bugfixes**
* timescale#4804 Skip bucketing when start or end of refresh job is null
* timescale#5108 Fix column ordering in compressed table index
* timescale#5187 Don't enable clang-tidy by default
* timescale#5255 Fix year not multiple of day/month in nested CAgg
* timescale#5259 Lock down search_path in SPI calls
lkshminarayanan added a commit to lkshminarayanan/timescaledb that referenced this pull request Feb 3, 2023
This release contains bug fixes since the 2.9.2 release.
This release is high priority for upgrade. We strongly recommend that you
upgrade as soon as possible.

**Bugfixes**
* timescale#4804 Skip bucketing when start or end of refresh job is null
* timescale#5108 Fix column ordering in compressed table index not following the order of a multi-column segment by definition
* timescale#5187 Don't enable clang-tidy by default
* timescale#5255 Fix year not being considered as a multiple of day/month in hierarchical continuous aggregates
* timescale#5259 Lock down search_path in SPI calls
lkshminarayanan added a commit that referenced this pull request Feb 3, 2023
This release contains bug fixes since the 2.9.2 release.
This release is high priority for upgrade. We strongly recommend that you
upgrade as soon as possible.

**Bugfixes**
* #4804 Skip bucketing when start or end of refresh job is null
* #5108 Fix column ordering in compressed table index not following the order of a multi-column segment by definition
* #5187 Don't enable clang-tidy by default
* #5255 Fix year not being considered as a multiple of day/month in hierarchical continuous aggregates
* #5259 Lock down search_path in SPI calls
lkshminarayanan added a commit to lkshminarayanan/timescaledb that referenced this pull request Feb 3, 2023
This release contains bug fixes since the 2.9.2 release.
This release is high priority for upgrade. We strongly recommend that you
upgrade as soon as possible.

**Bugfixes**
* timescale#4804 Skip bucketing when start or end of refresh job is null
* timescale#5108 Fix column ordering in compressed table index not following the order of a multi-column segment by definition
* timescale#5187 Don't enable clang-tidy by default
* timescale#5255 Fix year not being considered as a multiple of day/month in hierarchical continuous aggregates
* timescale#5259 Lock down search_path in SPI calls
@fabriziomello fabriziomello changed the title Fix year not multiple of day/month in nested CAgg Fix year not multiple of day/month in Hierarchical CAggs Feb 4, 2023
lkshminarayanan added a commit to lkshminarayanan/timescaledb that referenced this pull request Feb 6, 2023
This release contains bug fixes since the 2.9.2 release.
This release is high priority for upgrade. We strongly recommend that you
upgrade as soon as possible.

**Bugfixes**
* timescale#4804 Skip bucketing when start or end of refresh job is null
* timescale#5108 Fix column ordering in compressed table index not following the order of a multi-column segment by definition
* timescale#5187 Don't enable clang-tidy by default
* timescale#5255 Fix year not being considered as a multiple of day/month in hierarchical continuous aggregates
* timescale#5259 Lock down search_path in SPI calls
lkshminarayanan added a commit that referenced this pull request Feb 6, 2023
This release contains bug fixes since the 2.9.2 release.
This release is high priority for upgrade. We strongly recommend that you
upgrade as soon as possible.

**Bugfixes**
* #4804 Skip bucketing when start or end of refresh job is null
* #5108 Fix column ordering in compressed table index not following the order of a multi-column segment by definition
* #5187 Don't enable clang-tidy by default
* #5255 Fix year not being considered as a multiple of day/month in hierarchical continuous aggregates
* #5259 Lock down search_path in SPI calls
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: CAGG on CAGG Error Part 2
6 participants