-
Notifications
You must be signed in to change notification settings - Fork 897
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
Enable real time aggregation for CAggs with joins #5243
Conversation
Codecov Report
@@ Coverage Diff @@
## main #5243 +/- ##
=======================================
Coverage 89.35% 89.35%
=======================================
Files 225 225
Lines 51925 51925
=======================================
+ Hits 46396 46399 +3
+ Misses 5529 5526 -3
Continue to review full report at Codecov.
|
e09d667
to
689feaa
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would be nice to show in tests all the CAggs definition (i.e. \d+ conditions_summary_daily_realtime
)
tsl/test/expected/cagg_joins-12.out
Outdated
DETAIL: drop cascades to table _timescaledb_internal._hyper_6_35_chunk | ||
drop cascades to table _timescaledb_internal._hyper_6_36_chunk | ||
DETAIL: drop cascades to table _timescaledb_internal._hyper_7_37_chunk | ||
drop cascades to table _timescaledb_internal._hyper_7_38_chunk |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe you want to turn off the detailed output for this part of the test. It will change more times, and potentially generate false positives. It is sufficient to know the number of objects dropped.
tsl/test/expected/cagg_joins-12.out
Outdated
NOTICE: drop cascades to 9 other objects | ||
DETAIL: drop cascades to view _timescaledb_internal._partial_view_3 | ||
NOTICE: drop cascades to 12 other objects | ||
DETAIL: drop cascades to view conditions_summary_daily_realtime | ||
drop cascades to view _timescaledb_internal._partial_view_3 | ||
drop cascades to view _timescaledb_internal._direct_view_3 | ||
drop cascades to view _timescaledb_internal._partial_view_4 | ||
drop cascades to view _timescaledb_internal._direct_view_4 | ||
drop cascades to view conditions_summary_daily_2 | ||
drop cascades to view _timescaledb_internal._partial_view_5 | ||
drop cascades to view _timescaledb_internal._direct_view_5 | ||
drop cascades to view _timescaledb_internal._partial_view_7 | ||
drop cascades to view _timescaledb_internal._direct_view_7 | ||
drop cascades to view conditions_summary_daily_2 | ||
drop cascades to view _timescaledb_internal._partial_view_6 | ||
drop cascades to view _timescaledb_internal._direct_view_6 | ||
drop cascades to view _timescaledb_internal._partial_view_8 | ||
drop cascades to view _timescaledb_internal._direct_view_8 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here, this do not add anything to the test, so you can just as well remove this output by not printing the detailed error output.
tsl/test/sql/cagg_joins.sql.in
Outdated
--Create a cagg with join between a hypertable and a normal table | ||
-- with equality condition on inner join type | ||
CREATE MATERIALIZED VIEW conditions_summary_daily_realtime | ||
WITH (timescaledb.continuous) AS | ||
SELECT time_bucket(INTERVAL '1 day', day) AS bucket, | ||
AVG(temperature), | ||
MAX(temperature), | ||
MIN(temperature), | ||
name | ||
FROM conditions, devices | ||
WHERE conditions.device_id = devices.device_id | ||
GROUP BY name, bucket; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agree with @fabriziomello here. Show the definition of the cagg so that the test demonstrates that one with real-time aggregate was actually created.
350af01
to
1f4efa0
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall it looks good to me. Please see the comment on the test improvement and add a proper commit/PR description and (if needed) also a changelog entry.
devices | ||
WHERE conditions.device_id = devices.device_id AND devices.device_id >= COALESCE(_timescaledb_internal.to_date(_timescaledb_internal.cagg_watermark(3)), '-infinity'::date) | ||
GROUP BY devices.name, (time_bucket('@ 1 day'::interval, conditions.day)); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you insert data into the real-time part of the CAGG and do a SELECT
on the CAGG to show that real-time aggregation works?
fc57aa3
to
328e7d8
Compare
328e7d8
to
9f10b65
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good.
tsl/src/continuous_aggs/create.c
Outdated
* If there is join in CAgg definition then adjust varno | ||
* to get time column from hypertable only. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Which hypertable? Both the materialized table and the "underlying" table are hypertables. Or are you thinking about the hypertable in the join? Would be good to clarify which one it is.
tsl/test/expected/cagg_joins-12.out
Outdated
DETAIL: drop cascades to table _timescaledb_internal._hyper_8_41_chunk | ||
drop cascades to table _timescaledb_internal._hyper_8_42_chunk |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would suggest to not print the detailed message here, so set verbosity to terse
.
9f10b65
to
f941045
Compare
f941045
to
ee3e2c1
Compare
This release contains new features and bug fixes since the 2.9.3 release. This release is high priority for upgrade. We strongly recommend that you upgrade as soon as possible. **Features** * #5241 Allow RETURNING clause when inserting into compressed chunks * #5245 Manage life-cycle of connections via memory contexts * #5246 Make connection establishment interruptible * #5253 Make data node command execution interruptible * #5243 Enable real-time aggregation for continuous aggregates with joins * #5262 Extend enabling compression on a continuous aggregrate with 'compress_segmentby' and 'compress_orderby' parameters **Bugfixes** * #4926 Fix corruption when inserting into compressed chunks * #5218 Add role-level security to job error log * #5214 Fix use of prepared statement in async module * #5290 Compression can't be enabled on continuous aggregates when segmentby/orderby columns need quotation * #5239 Fix next_start calculation for fixed schedules
No description provided.