Push down ORDER BY for realtime caggs

[svenklemm]

Previously ordered queries on realtime caggs would always lead to full table scan as the query plan would have a sort with the limit on top. With this patch this gets changed so that the ORDER BY and LIMIT can be pushed down so the query can benefit from the ordered append optimization and does not require full table scan.

Fixes #4861

[codecov]

Codecov Report

Attention: Patch coverage is 94.11765% with 3 lines in your changes missing coverage. Please review.

Project coverage is 82.56%. Comparing base (59f50f2) to head (b38bd04).
Report is 552 commits behind head on main.

Files with missing lines	Patch %	Lines
tsl/src/continuous_aggs/planner.c	93.33%	1 Missing and 2 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #7271      +/-   ##
==========================================
+ Coverage   80.06%   82.56%   +2.49%     
==========================================
  Files         190      229      +39     
  Lines       37181    42726    +5545     
  Branches     9450    10733    +1283     
==========================================
+ Hits        29770    35277    +5507     
- Misses       2997     3168     +171     
+ Partials     4414     4281     -133     

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

[akuzm]

This looks incorrect, an Append node is not guaranteed to produce ordered output, so it can't be on top when there's an ORDER BY.

[svenklemm]

This is the idea of ordered append, when ordering by time and under the assumption that the individual children are ordered you can just append individual children and preserve ordering

[svenklemm]

the first child is the materialized cagg and the 2nd child is the realtime-part, the materialized part will always be ordered before the realtime-part when ordering by time.

[akuzm]

Should it have pathkeys then? Looks like it doesn't, no "Order" in explain unlike the below nodes.

[svenklemm]

Hmm atm it doesnt but it would be useful if it had them if this was a subquery in a bigger query. I removed the pathkeys from that level because postgres will turn it into mergeappend which means it will execute all children and prevent early abort with LIMIT

[akuzm]

Yeah it kind of useless on top, but the explain looks incorrect w/o the Order, and pathkeys also guarantee that no other optimization is applied that can break the order, e.g. there's an assertion in PG code that you can't have parallel ordered append.

Why does it turn into merge append? There's an option in create_append_path to make the normal Append with pathkeys, maybe we can use that somehow?

[svenklemm]

postgres is not aware that the children are already ordered based on the position in the append so it puts a mergeappend there cause its only aware of the local sorting not the global sorting. path creation is done by postgres here and we only amend the initial parse tree a bit before actual path generation

[fabriziomello]

@svenklemm I've made some tests and looks like it is returning wrong results.

Given the following setup:

CREATE TABLE temperature (
  time TIMESTAMPTZ NOT NULL,
  location TEXT NOT NULL,
  temperature DOUBLE PRECISION NOT NULL
);

SELECT create_hypertable('temperature', 'time');

INSERT INTO temperature SELECT time, 'X' as location, 10 AS temperature FROM generate_series('2023-09-23 21:24:43.905416-03'::timestamptz, '2024-09-23 21:24:43.905416-03'::timestamptz, interval '1 minute') as time;
INSERT INTO temperature SELECT time, 'Y' as location, 10 AS temperature FROM generate_series('2023-09-23 21:24:43.905416-03'::timestamptz, '2024-09-23 21:24:43.905416-03'::timestamptz, interval '1 minute') as time;
INSERT INTO temperature SELECT time, 'Z' as location, 10 AS temperature FROM generate_series('2023-09-23 21:24:43.905416-03'::timestamptz, '2024-09-23 21:24:43.905416-03'::timestamptz, interval '1 minute') as time;

CREATE INDEX temperature_location_time_idx ON temperature(location, time DESC);

CREATE MATERIALIZED VIEW temperature_5min
WITH (timescaledb.continuous, timescaledb.materialized_only=false)
AS SELECT time_bucket('5 min', time) as xtime, location, min(temperature), max(temperature)
FROM temperature
GROUP BY xtime, location;

First run the query with the optimization OFF:

fabrizio=# SET timescaledb.enable_cagg_sort_pushdown TO off;
SET
fabrizio=# SELECT * FROM temperature_5min WHERE location = 'Y' ORDER BY xtime DESC LIMIT 10;
         xtime          | location | min | max 
------------------------+----------+-----+-----
 2024-09-23 21:20:00-03 | Y        |  10 |  10
 2024-09-23 21:15:00-03 | Y        |  10 |  10
 2024-09-23 21:10:00-03 | Y        |  10 |  10
 2024-09-23 21:05:00-03 | Y        |  10 |  10
 2024-09-23 21:00:00-03 | Y        |  10 |  10
 2024-09-23 20:55:00-03 | Y        |  10 |  10
 2024-09-23 20:50:00-03 | Y        |  10 |  10
 2024-09-23 20:45:00-03 | Y        |  10 |  10
 2024-09-23 20:40:00-03 | Y        |  10 |  10
 2024-09-23 20:35:00-03 | Y        |  10 |  10
(10 rows)

Then running with the new optimization return different results (less tuples):

fabrizio=# DISCARD ALL;
DISCARD ALL
fabrizio=# SET timescaledb.enable_cagg_sort_pushdown TO on;
SET
fabrizio=# SELECT * FROM temperature_5min WHERE location = 'Y' ORDER BY xtime DESC LIMIT 10;
         xtime          | location | min | max 
------------------------+----------+-----+-----
 2024-09-23 21:20:00-03 | Y        |  10 |  10
 2024-09-23 21:15:00-03 | Y        |  10 |  10
 2024-09-23 21:10:00-03 | Y        |  10 |  10
(3 rows)

[svenklemm]

Looks like the LIMIT was pushed down below the constraint checks leading to wrong results. I've removed the limit pushdown from the PR as its not needed when we can pushdown the order by and use ordered append

[akuzm]

Do you know why Postgres doesn't do MergeAppend here? It's not as good as the new Ordered Append plan, but might be another missing optimization to explore.

[vanderhoop]

Any chance this will get backported to the last couple minor versions? 😄

[fabriziomello]

Any chance this will get backported to the last couple minor versions? 😄

Unfortunately no. It is a new feature and will be part of the next 2.18.0 release. Not sure yet when it will be released tough.