Skip to content

extract_from_op_expr keeps the original operator after swapping operands #643

Description

@Rjabov

TITLE:
wrappers: BETWEEN on a foreign table returns no rows (operand swap ignores the commutator)

BODY:

Bug report

Describe the bug

supabase-wrappers builds the filter it pushes to a remote system from the query's WHERE clause. When a comparison has the constant on the left and the column on the right, extract_from_op_expr swaps the operands so the column becomes Qual.field, but keeps the original operator instead of its commutator. The remote system is asked the opposite question and the query returns wrong rows, with no error.

This is easy to hit because Postgres expands X BETWEEN a AND b into X >= a AND X <= b. When X is the constant and the bounds are columns, both halves end up constant first. That is the normal way to ask which stored range contains a value: subscription periods, price tiers, validity windows.

To Reproduce

  1. Start ClickHouse and supabase/postgres:17.6.1.167 on a shared docker network.

  2. In ClickHouse:

create database demo;
create table demo.subs (id UInt64, plan String, valid_from Date, valid_to Date)
  engine = MergeTree order by id;
insert into demo.subs values
 (1,'free','2025-01-01','2025-12-31'),
 (2,'pro' ,'2026-01-01','2026-12-31'),
 (3,'team','2027-01-01','2027-12-31');
  1. In Postgres:
create extension if not exists wrappers with schema extensions;
create foreign data wrapper clickhouse_wrapper
  handler extensions.click_house_fdw_handler validator extensions.click_house_fdw_validator;
create server ch_server foreign data wrapper clickhouse_wrapper
  options (conn_string 'tcp://default:clickhouse@clickhouse:9000/demo');
create foreign table ch_subs (id bigint, plan text, valid_from date, valid_to date)
  server ch_server options (table 'subs');
  1. Run both queries:
select plan from ch_subs where date '2026-06-01' between valid_from and valid_to;
-- 0 rows

select plan from ch_subs where valid_from <= date '2026-06-01' and valid_to >= date '2026-06-01';
-- pro
  1. Check what ClickHouse was actually asked:
system flush logs;
select query from system.query_log where type = 'QueryFinish' and query like '%from subs where%';
select plan, valid_from, valid_to from subs where valid_from >= '2026-06-01' and valid_to <= '2026-06-01'
select plan, valid_from, valid_to from subs where valid_from <= '2026-06-01' and valid_to >= '2026-06-01'

Expected behavior

Both queries return pro, matching the same rows held in an ordinary Postgres table. The first should push down valid_from <= '2026-06-01' and valid_to >= '2026-06-01'.

Screenshots

Not applicable.

System information

  • wrappers 0.6.2, as bundled in supabase/postgres:17.6.1.167 (Postgres 17.6)
  • Also reproduced with wrappers built from main
  • Remotes: ClickHouse 24.8, MySQL 8.4, MongoDB 7.0
  • OS: Linux x86_64

Additional context

Cause, in supabase-wrappers/src/qual.rs, extract_from_op_expr:

let opr = get_operator(opno);
...
if is_a(right, T_Var) && !is_a(left, T_Var) && (*opr).oprcom != Oid::INVALID {
    std::mem::swap(&mut left, &mut right);   // operands swapped
}
...
operator: pgrx::name_data_to_str(&(*opr).oprname).to_string(),   // operator not swapped

oprcom is read as a gate and never used. It is the only reference to oprcom in the repo. The swap has been there since 10897b8 (2022-12-02) and qual.rs has no test covering it.

Reproduced against clickhouse_fdw, mysql_fdw and mongodb_fdw. MySQL general_log and MongoDB system.profile show the same inverted filter, and EXPLAIN (VERBOSE) prints Qual { field: "qty", operator: "<", value: Cell(I32(10)) } for where 10 < qty.

Affected: <, >, <=, >=, and any operator whose commutator is a different operator, for example @> and <@. Literal constants and bound parameters both.

Not affected: = and <> (self commutating), LIKE (oprcom is 0), IN lists (different code path), BETWEEN SYMMETRIC and NOT BETWEEN (not extracted), current_date and now() (not a Const).

get_foreign_plan leaves the clause in the plan, so Postgres rechecks it locally. The result is missing rows, never extra ones.

Possible fix

Use the operator's commutator when the operands are swapped, in extract_from_op_expr:

-                let opr = get_operator(opno);
+                let mut opr = get_operator(opno);
...
                 if is_a(right, T_Var) && !is_a(left, T_Var) && (*opr).oprcom != Oid::INVALID {
+                    // operands are swapped, so the operator has to be replaced by its
+                    // commutator, otherwise the qual is inverted: `100 > price` would be
+                    // pushed down as `price > 100` instead of `price < 100`
+                    opr = get_operator((*opr).oprcom);
                     std::mem::swap(&mut left, &mut right);
                 }

The oprcom != Oid::INVALID gate already guarantees the commutator exists, so no extra check is needed. cargo fmt --check, RUSTFLAGS="-D warnings" cargo clippy and cargo test -p supabase-wrappers --lib all pass with this applied.

I am happy to open a PR with the fix, a full explanation of the operand swap and why the commutator is required, and tests covering the swap path: each of <, >, <=, >= with the constant on the left, = and <> as unchanged controls, an operator with a distinct commutator such as @> and <@, and a bound parameter case. qual.rs currently has no test touching this branch. Let me know if you would prefer a different shape for the fix or the tests before I do.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions