Re: Compiler warning in costsize.c

Robert Haas <robertmhaas(at)gmail(dot)com> writes:
> On Mon, Apr 10, 2017 at 8:30 AM, Michael Paquier
> <michael(dot)paquier(at)gmail(dot)com> wrote:
>> On Mon, Apr 10, 2017 at 9:05 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>>> I wonder if we shouldn't just do
>>> ...
>>> and eat the "useless" calculation of rte.

> -1 from me. I'm not a big fan of useless calculation just because it
> happens to be needed in an Assert-enabled build.

Well, those planner_rt_fetch() calls are going to reduce to a simple
array lookup, so it seems rather extreme to insist on contorting the
code just to avoid that. It's not like these functions are trivially
cheap otherwise.

In fact, I kind of wonder why we're using planner_rt_fetch() at all in
costsize.c, rather than "root->simple_rte_array[rel->relid]". Maybe at
one time these functions were invokable before reaching query_planner(),
but we don't do that anymore. (Just to be sure, I stuck
"Assert(root->simple_rte_array)" into each costsize.c function that uses
planner_rt_fetch, and it still passes check-world.)

So now my proposal is

/* Should only be applied to base relations that are subqueries */
Assert(rel->relid > 0);
-#ifdef USE_ASSERT_CHECKING
- rte = planner_rt_fetch(rel->relid, root);
+ rte = root->simple_rte_array[rel->relid];
Assert(rte->rtekind == RTE_SUBQUERY);
-#endif

and make the rest of costsize.c look like that too.

regards, tom lane