multinode query not preserving timestamp type on remote causing bad plan

[phemmer]

Relevant system information:

• OS: Debian 10/Buster
• PostgreSQL version (output of postgres --version): postgres (PostgreSQL) 13.4 (Debian 13.4-1.pgdg100+1)
• TimescaleDB version (output of \dx in psql): 2.4.1
• Installation method: apt

Describe the bug
When using now()::timestamp in a where clause, the remote node is receiving a timestamptz value which is then cast to a timestamp. This causes the query planner to generate a plan that is extremely inefficient.

To Reproduce
Steps to reproduce the behavior:

create table test (time timestamp, v int);
select create_distributed_hypertable('test','time', chunk_time_interval => interval '1m', replication_factor => (select count(*) from timescaledb_information.data_nodes)::integer);
insert into test values (now() - interval '3m', 0),(now() - interval '2m', 1),(now() - interval '1m', 2),(now(), 3);
alter table test set (timescaledb.compress);
select compress_chunk(show_chunks('test', older_than => interval '1m'));
explain verbose select * from test where time > now()::timestamp - interval '1m';

Take the Remote SQL from the plan, and explain that on the remote node.

^ The compression is not strictly necessary, but it helps demonstrate the severity of the issue.

Expected behavior
Remote plan should only scan chunks for the restricted time period.

Actual behavior
All chunks are scanned, and compressed chunks are scanned using a sequential scan.

Screenshots
Here's the explain output from the remote query:

explain SELECT "time", v FROM public.test WHERE _timescaledb_internal.chunks_in(public.test.*, ARRAY[5307, 5309]) AND (("time" > (('2021-09-18 01:28:25.182707+00'::timestamptz)::timestamp without time zone - '00:01:00'::interval)));

Append  (cost=0.02..3544.56 rows=700680 width=12)
  ->  Custom Scan (DecompressChunk) on _dist_hyper_3002_4935_chunk  (cost=0.02..17.00 rows=700000 width=12)
        Filter: ("time" > (('2021-09-18 01:28:25.182707+00'::timestamp with time zone)::timestamp without time zone - '00:01:00'::interval))
        ->  Seq Scan on compress_hyper_133_5311_chunk  (cost=0.00..17.00 rows=700 width=84)
  ->  Index Scan using _dist_hyper_3002_4937_chunk_test_time_idx on _dist_hyper_3002_4937_chunk  (cost=0.16..24.16 rows=680 width=12)
        Index Cond: ("time" > (('2021-09-18 01:28:25.182707+00'::timestamp with time zone)::timestamp without time zone - '00:01:00'::interval))

Here's the output if I take off the ::timestamptz:

explain SELECT "time", v FROM public.test WHERE _timescaledb_internal.chunks_in(public.test.*, ARRAY[5307, 5309]) AND (("time" > (('2021-09-18 01:28:25.182707+00')::timestamp without time zone - '00:01:00'::interval)));

Index Scan using _dist_hyper_3002_4937_chunk_test_time_idx on _dist_hyper_3002_4937_chunk  (cost=0.15..24.15 rows=680 width=12)
  Index Cond: ("time" > '2021-09-18 01:27:25.182707'::timestamp without time zone)

Additional context
Originally I was attempting to solve the problem described in #118, which stated that as of 0.4.0, as long as the types are consistent, the plan should not scan unnecessary chunks. So I attempted to solve this by casting to timestamp, which apparently does not work.

I'm not sure if the issue lies on the access node side, causing the unnecessary casting in (('2021-09-18 01:28:25.182707+00'::timestamptz)::timestamp without time zone, or if the issue is on the data node side, with the planner not realizing that the resulting value is a timestamp and not timestamptz. Or both.

[akuzm]

Looks like the proximate cause is that the timestamptz -> timestamp conversion is not immutable (uses current global timezone from settings), that's why the filter expression is not folded to a constant on data nodes. If you specify the timezone explicitly with timezone, the chunks are filtered:

test=# explain (verbose, costs off) select * from test where time > timezone('Europe/Moscow', now()) - interval '1m';
                                                                                 QUERY PLAN                                                                                  
─────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
 Custom Scan (AsyncAppend)
   Output: test."time", test.v
   ->  Append
         ->  Custom Scan (DataNodeScan) on public.test test_1
               Output: test_1."time", test_1.v
               Data node: data1
               Chunks: _dist_hyper_2_107_chunk, _dist_hyper_2_109_chunk
               Remote SQL: SELECT "time", v FROM public.test WHERE _timescaledb_internal.chunks_in(public.test.*, ARRAY[54, 56]) AND (("time" > (timezone('Europe/Moscow'::t…
…ext, ('2021-09-27 17:54:01.872433+03'::timestamptz)) - '00:01:00'::interval)))
         ->  Custom Scan (DataNodeScan) on public.test test_2
               Output: test_2."time", test_2.v
               Data node: data2
               Chunks: _dist_hyper_2_108_chunk, _dist_hyper_2_110_chunk
               Remote SQL: SELECT "time", v FROM public.test WHERE _timescaledb_internal.chunks_in(public.test.*, ARRAY[55, 57]) AND (("time" > (timezone('Europe/Moscow'::t…
…ext, ('2021-09-27 17:54:01.872433+03'::timestamptz)) - '00:01:00'::interval)))


On a data node:
test=# explain (verbose, costs off) SELECT "time", v FROM public.test WHERE _timescaledb_internal.chunks_in(public.test.*, ARRAY[54, 56]) AND (("time" > (timezone('Europe/Moscow'::text, ('2021-09-27 17:47:17.24296+03'::timestamptz)) - '00:01:00'::interval)));
          QUERY PLAN           
───────────────────────────────
 Result
   Output: test."time", test.v
   One-Time Filter: false

On the other hand, now() is not immutable by itself, only stable, but maybe we have special logic for it...

[akuzm]

We have a mention of this problem on private repo: https://github.com/timescale/timescaledb-private/issues/523

[phemmer]

Looks like the proximate cause is that the timestamptz -> timestamp conversion is not immutable (uses current global timezone from settings), that's why the filter expression is not folded to a constant on data nodes.

Hrm, that's quite the edge case, but it does make sense. However wouldn't that be just why it's not sent across to the remote node as a simplified ::timestamp? Since it's what postgres defines as "stable", shouldn't it just be evaluated once, and thus can be used to determine which chunks need scanning, and doesn't need to be evaluated on every single record within all chunks?

[phemmer]

Another thought: This smells like dangerous behavior with regards to user expectations.

When I use now() (or anything which provides a timestamp/timestamptz), I expect that to reference a specific instant in time. If when the query reaches the data node, that time references a different instant, this would cause very unexpected results.

Given that we see now() is being converted to a constant prior to dispatching to the data nodes, and is being converted from timestamptz to timestamp, this would result in it referencing a different instant if the timezone between the access node and the data nodes isn't consistent.