Обсуждение: GROUP BY ROLLUP queries on views trigger full table scans (index usage not optimized)

On Mon, Nov 17, 2025 at 5:02 PM Haowu Ge <gehaowu@bitmoe.com> wrote:
> # 8. Query the view table to compute the average unit price for material 'PI',using a simple GROUP BY (rollup)
> EXPLAIN ANALYZE
> SELECT material_id, AVG(unit_price) as avg_price
> FROM materials_view
> WHERE material_id = 'PI'
> GROUP BY ROLLUP(material_id);

> GroupAggregate  (cost=0.00..1097.39 rows=251 width=64) (actual time=3.983..3.985 rows=2.00 loops=1)
>   Group Key: materials.material_id
>   Group Key: ()
>   Buffers: shared hit=468
>   ->  Seq Scan on materials  (cost=0.00..1093.00 rows=250 width=48) (actual time=0.018..3.970 rows=1.00 loops=1)
>         Filter: ((material_id)::text = 'PI'::text)
>         Rows Removed by Filter: 49999
>         Buffers: shared hit=468
> Planning Time: 0.117 ms
> Execution Time: 4.029 ms

Thanks for the report.  The reason why this query cannot use an index
scan is that, when the parent query uses grouping sets, all subquery
outputs are wrapped in PlaceHolderVars.  This is to ensure that these
expressions retain their separate identity so that they can match to
the expected grouping set columns (cf. cc5d98525).  However, it also
prevents the WHERE clause from matching the index column, which is why
the index cannot be used.

Prior to v18, we wrapped only the subquery outputs that were non-Var
expressions.  That is why this query was able to use an index scan.
However, that behavior could produce incorrect results when the
subquery's targetlist contained two or more identical Var expressions.
In addition, if the targetlist included any non-Var expressions, we
could also fail to match the WHERE clause to the corresponding index
expressions.

After looking at this query more closely, it seems that there is no
need to wrap the expressions in the WHERE clause in the first place.
It should be sufficient to wrap only the expressions that are used
logically above the aggregation step, ie, those in the targetlist and
in the havingQual.  However, pullup_replace_vars currently lacks the
infrastructure to do that.

Another possible fix is to detect whether the subquery's targetlist
contains two or more identical Var expressions and, if not, fall back
to wrapping only the non-Var expressions.  This would restore the
behavior that existed prior to v18 for this query.

Alternatively, we could teach match_index_to_operand to unwrap PHV
expressions when matching them to indexes.  This would also allow the
WHERE clause to match indexes for non-Var expressions, which is an
improvement over the behavior prior to v18.  But I haven't fully
thought through whether this approach is safe in all cases,
particularly when outer joins are involved.

Any thoughts?

- Richard

On Tue, Nov 18, 2025 at 5:27 PM Richard Guo <guofenglinux@gmail.com> wrote:
> Alternatively, we could teach match_index_to_operand to unwrap PHV
> expressions when matching them to indexes.  This would also allow the
> WHERE clause to match indexes for non-Var expressions, which is an
> improvement over the behavior prior to v18.  But I haven't fully
> thought through whether this approach is safe in all cases,
> particularly when outer joins are involved.

I believe this approach is safe, because a PlaceHolderVar appearing in
a relation-scan-level expression is effectively a no-op.  To play it
safe, we could verify that the PlaceHolderVar is not marked nullable
and that its syntactic scope matches this index, and only strip such
PlaceHolderVars (though it's unclear if this is really necessary).

Attached is a patch that implements this approach.  With this patch,
the reported query can use an index scan again.  Additionally, it
enables planner to explore index scans in other cases that were
previously not possible, even before v18.  For example:

create table t (a int);
insert into t select i from generate_series(1,1000)i;
create index on t (coalesce(a, 1));

analyze t;

explain (costs off)
select * from t t1 left join
  (select coalesce(a, 1) as c, * from t t2) s on true
where t1.a > s.c;
                  QUERY PLAN
-----------------------------------------------
 Nested Loop
   ->  Seq Scan on t t1
   ->  Index Scan using t_coalesce_idx on t t2
         Index Cond: (COALESCE(a, 1) < t1.a)
(4 rows)

The expression coalesce(a, 1) is wrapped in a PlaceHolderVar, and
previously it was not possible to match it to the index expression.

- Richard

On Fri, Dec 5, 2025 at 11:17 AM Haowu Ge <gehaowu@bitmoe.com> wrote:
> I tried applying the patch myself, compiled and installed it,
> but it doesn’t seem to resolve the issue of GROUP BY ROLLUP leading to a full table scan.

I see why that failed.  In your example scenario, the operand ends up
as RelabelType(PlaceHolderVar(Var)).  The v1 patch attempted to strip
PlaceHolderVar nodes first and then RelabelType nodes.  So the outer
RelabelType blocked the logic from seeing and stripping the inner
PlaceHolderVar, causing the match to fail.

You may think we can reverse the stripping order to fix this, but we
cannot, because these nodes can be interleaved in arbitrary ways.  For
instance:

CREATE TABLE t (a int, b int);
CREATE INDEX ON t (a);

SELECT * FROM (SELECT a::oid AS x FROM t)
WHERE x::int = 1
GROUP BY ROLLUP(x);

The operand is RT(PHV(RT(Var))).  To handle such interleaving, we may
need a loop that repeatedly peels off both node types from the top of
the stack until the underlying operand is revealed.

However, even a robust top-level loop is insufficient because PHVs can
be buried deeply within an expression tree.  This is particularly
problematic for expression indexes.  Consider an index on (a+1) with
the following query:

CREATE INDEX ON t ((a+1));

SELECT * FROM (SELECT a AS x FROM t)
WHERE x + 1 = 2
GROUP BY ROLLUP(x);

In this case, the operand becomes OpExpr(+, PHV(Var), Const(1)).  The
top-level stripping cannot reach this nested PHV node, which causes
the match to fail at the end, because match_index_to_operand() relies
on equal() to verify the match.

Therefore, we may need to recurse into the operand expression tree to
perform a deep strip of the PHVs, leveraging expression_tree_mutator.

A significant concern with this deep-strip approach is the performance
cost.  match_index_to_operand() lies on the planner's hot path, and
unconditionally invoking expression_tree_mutator for every check would
introduce unacceptable overhead: that involves memory allocation and
tree copying.

To mitigate this, we can avoid the stripping logic when there are no
PHVs to remove.  We can check root->glob->lastPHId == 0 to determine
if there's no PHVs at all.  Unfortunately, the "root" parameter is not
available in match_index_to_operand(), and modifying the function
signature to pass it down would break ABI compatibility, which should
not happen in a patch for back-patching.  (We can do that on master
though.)

As a viable alternative, maybe we can use a lightweight, read-only
expression walker on the operand tree first to detect the presence of
any PHVs.  We only trigger the expensive deep mutation if this
preliminary check confirms that it is actually necessary.

I'm wondering whether we need to deep-strip RelabelType nodes as well.
Is it possible to have a case where the operand OpExpr(+, RT(a), 1)
fails to match an index expression OpExpr(+, a, 1)?  Even if so, maybe
we do not need to deep-strip RelabelType nodes.  There is a distinct
difference between these two node types: PHVs are artifacts injected
purely by the planner and users have no control over them, while
RelabelType nodes usually reflect explicit types or casts derived from
the user's query.  If the user defines the index as (a::text || b),
they are expected to write WHERE (a::text || b) = ....

Any thoughts?

Hi Tom, I wonder if you can provide some insights?

- Richard

On Mon, Dec 8, 2025 at 3:10 PM Richard Guo <guofenglinux@gmail.com> wrote:
> As a viable alternative, maybe we can use a lightweight, read-only
> expression walker on the operand tree first to detect the presence of
> any PHVs.  We only trigger the expensive deep mutation if this
> preliminary check confirms that it is actually necessary.

Here is an updated patch along these lines.

Haowu, could you please verify if it fixes your query?

- Richard

On Tue, Dec 9, 2025 at 5:54 PM Haowu Ge <gehaowu@bitmoe.com> wrote:
> I built it via dpkg-buildpackage,
> but I'm unable to generate a proper installation package with the v2 patch

Hmm, I don't have much insight into the installation issue.  I tested
the v2 patch on both v18 and master, and it fixes your query.

EXPLAIN (COSTS OFF)
SELECT material_id, AVG(unit_price) as avg_price
FROM materials_view
WHERE material_id = 'PI'
GROUP BY ROLLUP(material_id);
                       QUERY PLAN
--------------------------------------------------------
 GroupAggregate
   Group Key: materials.material_id
   Group Key: ()
   ->  Index Scan using materials_pkey on materials
         Index Cond: ((material_id)::text = 'PI'::text)
(5 rows)

Regarding back-patching, I believe this issue exists before v18, but
it seems that the change in v18 made it common enough to notice,
especially in queries with grouping sets.  Given the lack of reports
for versions prior to v18, I'm inclined to back-patch this only to
v18.

Any thoughts?

- Richard

On Thu, Dec 11, 2025 at 11:32 AM Haowu Ge <gehaowu@bitmoe.com> wrote:
> Based on the "Minimal Reproducible Example" principle,
> I previously created a simplified version of the example,
> which you successfully fixed. However, during actual testing,
> I found that the behavior does not meet expectations.
> Specifically, I observed inconsistent index behavior when using a view compared to querying the underlying table
directly.

This seems to be a separate issue.  The query against the view chooses
different indexes than the query against the underlying table.  This
isn't the index capability issue you initially reported, but rather a
cost estimation issue.

Upon closer look, the issue is that the planner fails to look through
PHVs when looking up statistical data.  Since examine_variable()
relies on strict structural matching, the presence of PHVs prevents
the planner from matching the expression to the table columns.  As a
result, the view-based query falls back to default selectivity
estimates, leading to a poor plan choice.

To fix, we can strip all PHVs when looking up statistical data.  This
is safe during estimation because PHVs are transparent for the purpose
of statistics lookup; they do not alter the value distribution of the
underlying expression.

Regarding implementation, I considered reusing the stripper for index
operands in 0001 but decided to use a dedicated function.  While the
traversal structure is similar, the logic is fundamentally different.
Stripping PHVs for index matching requires strict checks on
phnullingrels and phexpr to ensure execution correctness.  For
statistics lookup, however, we can be permissive and unconditionally
strip all PHVs.  It could be argued that we can combine them into a
single function using flags, but that seems to unnecessarily entangle
two unrelated logics.

There is one plan change in the regression tests with 0002, but that
is precisely because the row count estimates become more accurate with
this patch.  For instance:

on master:
->  Seq Scan on parttbl1 parttbl  (cost=0.00..41.88 rows=13 width=8)
                          (actual time=0.034..0.036 rows=1.00 loops=1)


on patched:
->  Seq Scan on parttbl1 parttbl  (cost=0.00..41.88 rows=1 width=8)
                          (actual time=0.062..0.064 rows=1.00 loops=1)

- Richard

On Thu, Dec 12, 2025 at 9:28 AM Richard Guo <guofenglinux(at)gmail(dot)com> wrote:
> This seems to be a separate issue.  The query against the view chooses
> different indexes than the query against the underlying table.  This
> isn't the index capability issue you initially reported, but rather a
> cost estimation issue.

Hello Richard,

After testing, your patch can resolve the issue with the sample case I raised earlier.
Thank you.

However, there is another problem (I thought my example was sufficient, but it actually wasn’t).
In my actual scenario, my view includes join conditions,
and the new patch still does not achieve the same index behavior as in PostgreSQL 16.11.
So it might still require your help to look into what the issue is.
Thank you.

--- Regarding the attachment ---
Consistent with the original attachment: demo-v4-0001-partitioned_table_view_Test_Data_and_Examples.sql.txt
New example: demo-v4-0002-View_with_join_Examples.sql.txt

--- EXPLAIN ANALYZE ---

EXPLAIN ANALYZE from pg18.1 (Seq Scan on invoices_2020 and Rows Removed by Filter: 1000)

  ->  Hash Right Join  (cost=260.06..286.43 rows=289 width=20) (actual time=2.040..2.060 rows=116.00 loops=1)
        Hash Cond: ((materials2.material_id)::text = (invoices.material_code)::text)
        Buffers: shared hit=79
        ->  Seq Scan on materials2  (cost=0.00..14.90 rows=490 width=58) (actual time=0.012..0.013 rows=0.00 loops=1)
              Buffers: shared hit=1
        ->  Hash  (cost=258.59..258.59 rows=118 width=29) (actual time=2.022..2.024 rows=116.00 loops=1)
              Buckets: 1024  Batches: 1  Memory Usage: 15kB
              Buffers: shared hit=78
              ->  Append  (cost=0.00..258.59 rows=118 width=29) (actual time=1.119..1.973 rows=116.00 loops=1)
                    Buffers: shared hit=78
                    ->  Seq Scan on invoices_2020 invoices_1  (cost=0.00..43.00 rows=1 width=29) (actual
time=0.243..0.243rows=0.00 loops=1) 
                          Filter: (((sales_org)::text = ANY ('{DF01,DG01}'::text[])) AND ((data_control_scope)::text =
'DD_OG'::text)AND ((valuation_order_flag)::text = '1'::text) AND ((approval_status)::text = 'y'::text) AND
(((document_date>= '2024-06-01'::date) AND (document_date <= '2024-12-31'::date)) OR ((document_date >=
'2025-01-01'::date)AND (document_date <= '2025-12-09'::date)))) 
                          Rows Removed by Filter: 1000
                          Buffers: shared hit=13
                    ->  Seq Scan on invoices_2021 invoices_2  (cost=0.00..43.00 rows=1 width=29) (actual
time=0.255..0.255rows=0.00 loops=1) 
                          Filter: (((sales_org)::text = ANY ('{DF01,DG01}'::text[])) AND ((data_control_scope)::text =
'DD_OG'::text)AND ((valuation_order_flag)::text = '1'::text) AND ((approval_status)::text = 'y'::text) AND
(((document_date>= '2024-06-01'::date) AND (document_date <= '2024-12-31'::date)) OR ((document_date >=
'2025-01-01'::date)AND (document_date <= '2025-12-09'::date)))) 
                          Rows Removed by Filter: 1000
                          Buffers: shared hit=13



EXPLAIN ANALYZE from pg16.11 (Recheck Cond: (((document_date >= '202)

  ->  Hash Right Join  (cost=108.69..128.52 rows=309 width=10) (actual time=0.235..0.248 rows=120 loops=1)
        Hash Cond: ((materials2.material_id)::text = (invoices.material_code)::text)
        ->  Seq Scan on materials2  (cost=0.00..14.90 rows=490 width=58) (actual time=0.005..0.005 rows=0 loops=1)
        ->  Hash  (cost=107.11..107.11 rows=126 width=19) (actual time=0.226..0.230 rows=120 loops=1)
              Buckets: 1024  Batches: 1  Memory Usage: 15kB
              ->  Append  (cost=8.57..107.11 rows=126 width=19) (actual time=0.039..0.211 rows=120 loops=1)
                    ->  Bitmap Heap Scan on invoices_2020 invoices_1  (cost=8.57..12.60 rows=1 width=19) (actual
time=0.007..0.007rows=0 loops=1) 
                          Recheck Cond: (((document_date >= '2024-06-01'::date) AND (document_date <=
'2024-12-31'::date))OR ((document_date >= '2025-01-01'::date) AND (document_date <= '2025-12-09'::date))) 
                          Filter: (((sales_org)::text = ANY ('{DF01,DG01}'::text[])) AND ((data_control_scope)::text =
'DD_OG'::text)AND ((valuation_order_flag)::text = '1'::text) AND ((approval_status)::text = 'y'::text)) 
                          ->  BitmapOr  (cost=8.57..8.57 rows=1 width=0) (actual time=0.005..0.006 rows=0 loops=1)
                                ->  Bitmap Index Scan on idx_invoices_2020_date  (cost=0.00..4.29 rows=1 width=0)
(actualtime=0.004..0.004 rows=0 loops=1) 
                                      Index Cond: ((document_date >= '2024-06-01'::date) AND (document_date <=
'2024-12-31'::date))
                                ->  Bitmap Index Scan on idx_invoices_2020_date  (cost=0.00..4.29 rows=1 width=0)
(actualtime=0.001..0.001 rows=0 loops=1) 
                                      Index Cond: ((document_date >= '2025-01-01'::date) AND (document_date <=
'2025-12-09'::date))

Thanks & Best Regards
_________________________________________________________________________________
Haowu Ge (BG5FRG) | Homepage: https://www.gehaowu.com | PGP:7A06 1F6E DF09 D8A8

On Mon, Dec 15, 2025 at 10:23 AM Haowu Ge <gehaowu@bitmoe.com> wrote:
> On Thu, Dec 12, 2025 at 9:28 AM Richard Guo <guofenglinux(at)gmail(dot)com> wrote:
> > This seems to be a separate issue.  The query against the view chooses
> > different indexes than the query against the underlying table.  This
> > isn't the index capability issue you initially reported, but rather a
> > cost estimation issue.

> After testing, your patch can resolve the issue with the sample case I raised earlier.
> Thank you.
>
> However, there is another problem (I thought my example was sufficient, but it actually wasn’t).
> In my actual scenario, my view includes join conditions,
> and the new patch still does not achieve the same index behavior as in PostgreSQL 16.11.

This happens because the v3 patch is conservative: it only strips a
PHV if its syntactic scope (phrels) strictly matches the index
relation.  In query:

select 1 from (select t1.a as x from t t1, t t2) where x = 1 group by rollup(x);

The syntactic scope of PHV(t1.a) includes both {t1, t2}.  As a result,
the check fails when matching against t1's index, which prevents the
PHV from being stripped and the index from being used.

I considered checking PlaceHolderInfo.ph_eval_at instead, which would
be more accurate.  However, finding the corresponding PlaceHolderInfo
requires access to root, and passing root into match_index_to_operand
would break ABI compatibility, which we want to avoid.

Upon closer look, it seems that we do not need the syntactic scope
check at all, as long as we ensure that phnullingrels is empty.  In
this case, the PHV is either a no-op (from a reduced outer join) or
used for identity separation (grouping sets).  In either case, the PHV
is transparent regarding the value, so it should not prevent the
underlying expression from matching the index.

Any thoughts?

- Richard