Re: Is it expected behavior index only scan shows "OPERATOR(pg_catalog." for EXPLAIN? - Mailing list pgsql-hackers

On 7/8/24 13:03, Masahiro.Ikeda@nttdata.com wrote:
> Hi,
> 
> While I'm researching about [1], I found there are inconsistent EXPLAIN outputs.
> Here is an example which shows " OPERATOR(pg_catalog.". Though it's not wrong,
> I feel like there is no consistency in the output format.
> 
> -- A reproduce procedure
> create temp table btree_bpchar (f1 text collate "C");
> create index on btree_bpchar(f1 bpchar_ops);
> insert into btree_bpchar values ('foo'), ('fool'), ('bar'), ('quux');
> set enable_seqscan to false;
> set enable_bitmapscan to false;
> set enable_indexonlyscan to false; -- or true
> explain (costs off)
> select * from btree_bpchar where f1::bpchar like 'foo';
> 
> -- Index Scan result
>                       QUERY PLAN                      
> ------------------------------------------------------
>  Index Scan using btree_bpchar_f1_idx on btree_bpchar
>    Index Cond: ((f1)::bpchar = 'foo'::bpchar)
>    Filter: ((f1)::bpchar ~~ 'foo'::text)
> (3 rows)
> 
> -- Index Only Scan result which has 'OPERATOR'
>                         QUERY PLAN                         
> -----------------------------------------------------------
>  Index Only Scan using btree_bpchar_f1_idx on btree_bpchar
>    Index Cond: (f1 OPERATOR(pg_catalog.=) 'foo'::bpchar)        -- Here is the point.
>    Filter: ((f1)::bpchar ~~ 'foo'::text)
> (3 rows)
> 

This apparently comes from generate_operator_name() in ruleutils.c,
where the OPERATOR() decorator is added if:

 /*
  * The idea here is to schema-qualify only if the parser would fail to
  * resolve the correct operator given the unqualified op name with the
  * specified argtypes.
  */

So clearly, the code believes just the operator name could be ambiguous,
so it adds the namespace too. Why exactly it is considered ambiguous I
don't know, but perhaps you have other applicable operators in the
search_path, or something like that?

> 
> IIUC, the index only scan use fixed_indexquals, which is removed "RelabelType" nodes,
> for EXPLAIN so that get_rule_expr() could not understand the left argument of the operator
> (f1 if the above case) can be displayed with arg::resulttype and it doesn't need to
> show "OPERATOR(pg_catalog.)".
> 
> I've attached PoC patch to show a simple solution. It just adds a new member "indexqualorig"
> to the index only scan node like the index scan and the bitmap index scan. But, since I'm
> a beginner about the planner, I might have misunderstood something or there should be better
> ways.
> 

I honestly don't know if this is the correct solution. It seems to me
handling this at the EXPLAIN level might just mask the issue - it's not
clear to me why adding "indexqualorig" would remove the ambiguity (if
there's one). Perhaps it might be better to find why the ruleutils.c
code thinks the OPERATOR() is necessary, and then improve/fix that?

regards

-- 
Tomas Vondra
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company