Re: [HACKERS] Compiler warning in costsize.c

2017-05-07 Thread David Rowley
On 11 April 2017 at 12:53, Michael Paquier wrote: > On Tue, Apr 11, 2017 at 4:02 AM, Robert Haas wrote: >> On Mon, Apr 10, 2017 at 2:09 PM, Tom Lane wrote: >>> ilm...@ilmari.org (Dagfinn Ilmari =?utf-8?Q?Manns=C3=A5ker?=)

Re: [HACKERS] Compiler warning in costsize.c

2017-04-10 Thread Michael Paquier
On Tue, Apr 11, 2017 at 4:02 AM, Robert Haas wrote: > On Mon, Apr 10, 2017 at 2:09 PM, Tom Lane wrote: >> ilm...@ilmari.org (Dagfinn Ilmari =?utf-8?Q?Manns=C3=A5ker?=) writes: >>> Why bother with the 'rte' variable at all if it's only used for the >>>

Re: [HACKERS] Compiler warning in costsize.c

2017-04-10 Thread Robert Haas
On Mon, Apr 10, 2017 at 2:09 PM, Tom Lane wrote: > ilm...@ilmari.org (Dagfinn Ilmari =?utf-8?Q?Manns=C3=A5ker?=) writes: >> Why bother with the 'rte' variable at all if it's only used for the >> Assert()ing the rtekind? > > That was proposed a few messages back. I don't like

Re: [HACKERS] Compiler warning in costsize.c

2017-04-10 Thread Tom Lane
ilm...@ilmari.org (Dagfinn Ilmari =?utf-8?Q?Manns=C3=A5ker?=) writes: > Why bother with the 'rte' variable at all if it's only used for the > Assert()ing the rtekind? That was proposed a few messages back. I don't like it because it makes these functions look different from the other

Re: [HACKERS] Compiler warning in costsize.c

2017-04-10 Thread Tom Lane
Robert Haas writes: > On Mon, Apr 10, 2017 at 8:30 AM, Michael Paquier > wrote: >> On Mon, Apr 10, 2017 at 9:05 PM, Tom Lane wrote: >>> I wonder if we shouldn't just do >>> ... >>> and eat the "useless" calculation of rte. >

Re: [HACKERS] Compiler warning in costsize.c

2017-04-10 Thread Dagfinn Ilmari Mannsåker
Tom Lane writes: > I wonder if we shouldn't just do > > RangeTblEntry *rte PG_USED_FOR_ASSERTS_ONLY; > ListCell *lc; > > /* Should only be applied to base relations that are subqueries */ > Assert(rel->relid > 0); > -#ifdef USE_ASSERT_CHECKING >

Re: [HACKERS] Compiler warning in costsize.c

2017-04-10 Thread Robert Haas
On Mon, Apr 10, 2017 at 8:30 AM, Michael Paquier wrote: > On Mon, Apr 10, 2017 at 9:05 PM, Tom Lane wrote: >> I wonder if we shouldn't just do >> >> RangeTblEntry *rte PG_USED_FOR_ASSERTS_ONLY; >> ListCell *lc; >> >> /*

Re: [HACKERS] Compiler warning in costsize.c

2017-04-10 Thread Michael Paquier
On Mon, Apr 10, 2017 at 9:05 PM, Tom Lane wrote: > I wonder if we shouldn't just do > > RangeTblEntry *rte PG_USED_FOR_ASSERTS_ONLY; > ListCell *lc; > > /* Should only be applied to base relations that are subqueries */ > Assert(rel->relid >

Re: [HACKERS] Compiler warning in costsize.c

2017-04-10 Thread Tom Lane
David Rowley writes: > On 8 April 2017 at 04:42, Tom Lane wrote: >> BTW, is it really true that only these two places produce such warnings >> on MSVC? I see about three dozen uses of PG_USED_FOR_ASSERTS_ONLY in our >> tree, and I'd have

Re: [HACKERS] Compiler warning in costsize.c

2017-04-10 Thread David Rowley
On 8 April 2017 at 04:42, Tom Lane wrote: > I'd be happier with something along the line of > > RangeTblEntry *rte; > ListCell *lc; > > /* Should only be applied to base relations that are subqueries */ > Assert(rel->relid > 0); > rte

Re: [HACKERS] Compiler warning in costsize.c

2017-04-07 Thread Tom Lane
Michael Paquier writes: > Bah. This actually fixes nothing. Attached is a different patch that > really addresses the problem, by removing the variable because we > don't want planner_rt_fetch() to run for non-Assert builds. I don't really like any of these fixes,

Re: [HACKERS] Compiler warning in costsize.c

2017-04-07 Thread Michael Paquier
On Fri, Apr 7, 2017 at 12:38 AM, Michael Paquier wrote: > On Tue, Apr 4, 2017 at 9:42 PM, Michael Paquier > wrote: >> On Wed, Apr 5, 2017 at 2:54 AM, Tom Lane wrote: >>> (I'm personally not that much in love with

Re: [HACKERS] Compiler warning in costsize.c

2017-04-07 Thread Michael Paquier
On Tue, Apr 4, 2017 at 9:42 PM, Michael Paquier wrote: > On Wed, Apr 5, 2017 at 2:54 AM, Tom Lane wrote: >> (I'm personally not that much in love with PG_USED_FOR_ASSERTS_ONLY, >> because it tends to confuse pgindent.) > > I would be incline to just

Re: [HACKERS] Compiler warning in costsize.c

2017-04-04 Thread Michael Paquier
On Wed, Apr 5, 2017 at 2:54 AM, Tom Lane wrote: > Michael Paquier writes: >> In builds where USE_ASSERT_CHECKING is not enabled, costsize.c can >> generate warnings. Here is for example with MSVC: >> src/backend/optimizer/path/costsize.c(4520):

Re: [HACKERS] Compiler warning in costsize.c

2017-04-04 Thread Tom Lane
Michael Paquier writes: > In builds where USE_ASSERT_CHECKING is not enabled, costsize.c can > generate warnings. Here is for example with MSVC: > src/backend/optimizer/path/costsize.c(4520): warning C4101: 'rte' : > unreferenced local variable

Re: [HACKERS] Compiler warning in costsize.c

2017-04-04 Thread Michael Paquier
On Tue, Apr 4, 2017 at 7:03 PM, David Rowley wrote: > On 4 April 2017 at 16:22, Michael Paquier wrote: >> Hi all, >> >> In builds where USE_ASSERT_CHECKING is not enabled, costsize.c can >> generate warnings. Here is for example with MSVC:

Re: [HACKERS] Compiler warning in costsize.c

2017-04-04 Thread David Rowley
On 4 April 2017 at 16:22, Michael Paquier wrote: > Hi all, > > In builds where USE_ASSERT_CHECKING is not enabled, costsize.c can > generate warnings. Here is for example with MSVC: > src/backend/optimizer/path/costsize.c(4520): warning C4101: 'rte' : > unreferen >