Обсуждение: remove the unneeded header file math.h in binaryheap.c

Hi Hackers,

I found in file binaryheap.c， file "math.h" was included, but it is uncessary, so I removed it 

and filing a patch to address the issue.

I have verified that, build is OK.

Please see the attachment and help review it.

thanks a lot!

liujinyang

On 2026-Jan-13, liujinyang wrote:

> Hi Hackers,
> 
> I found in file binaryheap.c， file "math.h" was included, but it is uncessary, so I removed it 
> and filing a patch to address the issue.

Fun.  This was already unnecessary at commit 7a2fe9bd0371 which
introduced the file.  It seems Abhijit had that include in his first
version [1] because he was using floor().   Robert rewrote it later [2]
and replaced that with straight arithmetic, making the include
unnecessary, but forgot to remove it.

[1] https://postgr.es/m/20121114131112.GA27771@toroid.org 
[2] https://postgr.es/m/CA%2BTgmobvR7XW9fjj2RNY7sKK-VAG5nahfai_zV51rHVLDNvaBg%40mail.gmail.com

-- 
Álvaro Herrera               48°01'N 7°57'E  —  https://www.EnterpriseDB.com/

发件人：Álvaro Herrera <alvherre@kurilemu.de> 发件时间：2026年1月14日 01:18 收件人：liujinyang <21043272@qq.com> 抄送：pgsql-hackers <pgsql-hackers@lists.postgresql.org> 主题：Re: remove the unneeded header file math.h in binaryheap.c

On 2026-Jan-13, liujinyang wrote:

> Hi Hackers,
> 
> I found in file binaryheap.c， file "math.h" was included, but it is uncessary, so I removed it 
> and filing a patch to address the issue.

Fun.  This was already unnecessary at commit 7a2fe9bd0371 which
introduced the file.  It seems Abhijit had that include in his first
version [1] because he was using floor().   Robert rewrote it later [2]
and replaced that with straight arithmetic, making the include
unnecessary, but forgot to remove it.

[1] https://postgr.es/m/20121114131112.GA27771@toroid.org 
[2] https://postgr.es/m/CA%2BTgmobvR7XW9fjj2RNY7sKK-VAG5nahfai_zV51rHVLDNvaBg%40mail.gmail.com

-- 
Álvaro Herrera               48°01'N 7°57'E  —  https://www.EnterpriseDB.com/

On 2026-01-15, liujinyang wrote:
> Hi Alvora,
> thanks for your review.
> So wha't the next should I do for commit it, because this is my first patch.

Nothing.  I'll push it soon, no worries.

In the future, if patches don't get committed in a day or so, you can add them to a commitfest so that they aren't
forgotten-- see https://commitfest.postgresql.org. 

--
Álvaro Herrera

On 2026-Jan-15, Álvaro Herrera wrote:

> On 2026-01-15, liujinyang wrote:
> > Hi Alvora, 
> > thanks for your review.
> > So wha't the next should I do for commit it, because this is my first patch.
> 
> Nothing.  I'll push it soon, no worries.

BTW how did you notice that the include wasn't necessary?

I looked around to see if there are more unnecessary inclusions of that
header and found a few candidates.  Things still compile and pass tests
for me, but of course they may fail on other platforms.
https://cirrus-ci.com/build/4766393398198272

-- 
Álvaro Herrera               48°01'N 7°57'E  —  https://www.EnterpriseDB.com/

Hi Álvaro Herrera,

I removed the <math.h> include from two files `dt_common.c` and `timestamp.c`, and the code compiles successfully in my
environment.
Would you consider adding this to the patch?

```
postgres@zxm-VMware-Virtual-Platform:~/code/postgres$ git diff
diff --git a/src/interfaces/ecpg/pgtypeslib/dt_common.c b/src/interfaces/ecpg/pgtypeslib/dt_common.c
index c4119ab7932..0e26ed67cb1 100644
--- a/src/interfaces/ecpg/pgtypeslib/dt_common.c
+++ b/src/interfaces/ecpg/pgtypeslib/dt_common.c
@@ -4,7 +4,6 @@
 
 #include <time.h>
 #include <ctype.h>
-#include <math.h>
 
 #include "common/string.h"
 #include "dt.h"
diff --git a/src/interfaces/ecpg/pgtypeslib/timestamp.c b/src/interfaces/ecpg/pgtypeslib/timestamp.c
index 7cf433266f4..087725a44e7 100644
--- a/src/interfaces/ecpg/pgtypeslib/timestamp.c
+++ b/src/interfaces/ecpg/pgtypeslib/timestamp.c
@@ -5,7 +5,6 @@
 
 #include <time.h>
 #include <limits.h>
-#include <math.h>
 
 #ifdef __FAST_MATH__
 #error -ffast-math is known to break this code
```

--
Regards,
Man Zeng
www.openhalo.org

Hi,

On 2026-01-15 23:08:28 +0800, zengman wrote:
> I removed the <math.h> include from two files `dt_common.c` and `timestamp.c`, and the code compiles successfully in
myenvironment.
 
> Would you consider adding this to the patch?

Just because removing a platform include file works in some environment is
*NOT* sufficient to remove platform include files. There are a lot of
variations between platforms about when some include files are implicitly
included via other include files. Just removing them because it works on one
platform ends up with more niche operating systems failing to build, which we
may only figure out months or even years down the road.

At the very least you need to figure out why the includes where added and why
that reason is not present anymore.

Greetings,

Andres Freund

=?utf-8?Q?=C3=81lvaro?= Herrera <alvherre@kurilemu.de> writes:
> I looked around to see if there are more unnecessary inclusions of that
> header and found a few candidates.  Things still compile and pass tests
> for me, but of course they may fail on other platforms.

Yeah ... my recollection is that some of our inclusions of <math.h>,
and also of <limits.h>, were needed because assorted platforms didn't
follow the C/POSIX standards about which headers should provide which
symbols.  Maybe that's all cleaned up now.  I'd worry about Solaris
and AIX as being the most likely stragglers.

Another likely source of obsolete <math.h> usages is in the datetime
code, as a leftover from our old floating-point timestamps.

            regards, tom lane

On 2026-Jan-15, Andres Freund wrote:

> Hi,
> 
> On 2026-01-15 23:08:28 +0800, zengman wrote:
> > I removed the <math.h> include from two files `dt_common.c` and
> > `timestamp.c`, and the code compiles successfully in my environment.
> > Would you consider adding this to the patch?

I would rather not touch ecpg.  (I skipped those files purposefully.)

> Just because removing a platform include file works in some environment is
> *NOT* sufficient to remove platform include files. There are a lot of
> variations between platforms about when some include files are implicitly
> included via other include files. Just removing them because it works on one
> platform ends up with more niche operating systems failing to build, which we
> may only figure out months or even years down the road.
> 
> At the very least you need to figure out why the includes where added and why
> that reason is not present anymore.

I'm willing to bet that a few of those (looking at contrib_gist) were
just bogus back then, because there was no sane rule being kept,
especially btree_gist.h was a mess.  Links
postgr.es/c/a22d76d96ada
https://www.postgresql.org/message-id/flat/Pine.BSO.4.63.0607132104010.30094%40leary2.csoft.net
postgr.es/c/66c15dfda1a7

Some others appear to be misunderstandings -- (e.g. intarray, ltree) use
abs() which is in stdlib.h (included by c.h) rather than math.h, as
fabs() is.   c.f. commit f14aad5169ba.

The change in segparse.y is a mistake.  That one uses HUGE_VAL and so it
needs math.h.

vacuumlazy.c had it by 5374d097de4d which added ceil().  No longer used.

xlogrecovery.c got it in 70e81861fadd as it was split from xlog.c (which
used ceil() itself).  Not really needed.

xlogwait.c is new; has never needed it.  Copy-pasto from xlog.c perhaps?
(Tested by adding #define _MATH_H and verifying that it still compiles.)

define.c got it in 7385619f14ec.  It used pow().  No longer (was removed
in 6eb8d255d217).

nodeBitmapHeapscan.c got it in c29aff959dc6.  Used rint().  No longer
(was removed in b09ff53667ff).

nodeSubplan.c got it in ae643747b16a.  Used ceil().  No longer (removed
in 299d1716525c).

knapsack.c got it at creation in b5635948ab16.  Unclear why.  I suspect
it was made unnecessary during development but not removed.

readfuncs.c got it at the beginning of time in d31084e9d11.  Maybe
because of atoi / atof / atol?  Completely different file nowadays.

GEQO got it at creation in 29138eeb3ca2.  Used ceil().  No longer.
geqo_pool.c doesn't appear to have needed it, probably just careless
development.

indxpath.c got it at down of time in d31084e9d11.  I'm betting it wasn't
needed then, as it isn't now.

joinpath.c got it then too.  It used ceil()/sqrt().  No more (removed in
c2f0d565f319).

createplan.c got it in 2c2161a47d47.  Used rint(). Was removed in
8c314b9853c2.

pathnode.c got it in d31084e9d11.  Used pow().  Was removed in
166b5c1def56.

mcv.c got it at creation in 7300a699502f.  Cargo-culted?  (sqrt is used
in comments, maybe the actual call was removed during development?).
This code was written by a mathematician, maybe they're a fan of math.h.

numutils.c got it in d31084e9d11.  Used pow().  No longer.  Code is
completely unrelated nowadays.

tid.c got it pointlessly in 7aee5ed3b73f.  It added calls to strtol and
strtoul, which are in stdlib.h, not math.h.  Shrug.

binaryheap.c was already explained.

date.h got it as part of a22d76d96ada.  It had rint(), was removed by
b9d092c962ea.

-- 
Álvaro Herrera               48°01'N 7°57'E  —  https://www.EnterpriseDB.com/

On 2026-Jan-15, Tom Lane wrote:

> Yeah ... my recollection is that some of our inclusions of <math.h>,
> and also of <limits.h>, were needed because assorted platforms didn't
> follow the C/POSIX standards about which headers should provide which
> symbols.  Maybe that's all cleaned up now.  I'd worry about Solaris
> and AIX as being the most likely stragglers.

I think we have enough Solaris in the buildfarm to know if there are any
problems.  As for AIX, we don't support it currently, and I guess we'll
know from the developers if we need any patches there.

> Another likely source of obsolete <math.h> usages is in the datetime
> code, as a leftover from our old floating-point timestamps.

Yeah, there was one in date.h related to that, which I mentioned in my
reply to Andres.

I have just pushed it.  Now it's time for the buildfarm to speak ...


Thanks liujinyang, and welcome -- may your Postgres contribution days be
long and fertile.

-- 
Álvaro Herrera               48°01'N 7°57'E  —  https://www.EnterpriseDB.com/
"No nos atrevemos a muchas cosas porque son difíciles,
pero son difíciles porque no nos atrevemos a hacerlas" (Séneca)

Hi,

> > > I removed the <math.h> include from two files `dt_common.c` and
> > > `timestamp.c`, and the code compiles successfully in my environment.
> > > Would you consider adding this to the patch?
> 
> I would rather not touch ecpg.  (I skipped those files purposefully.)

Ok, then let's leave this part untouched. But I still sorted out the timeline of changes related to <math.h> in these
twofiles:
 
```
dt_common.c
cb8b1299a353: Added <math.h> to support the JROUND macro (which relies on the rint() function and requires the function
prototypefrom <math.h>)
 
313ed1ed9498: Removed usage of the JROUND macro (replaced with *fsec = time - *sec;), so the rint() function is no
longerneeded
 

timestamp.c
2e6f97560a8: <math.h> was included when the file was created to support the JROUND macro
313ed1ed9498: Renamed JROUND to TSROUND and added usage of the ceil() function
b9d092c962e: Removed all floating-point timestamp related code (including TSROUND and ceil())
```
Feel free to use this if needed later.

> Just because removing a platform include file works in some environment is
> *NOT* sufficient to remove platform include files. There are a lot of
> variations between platforms about when some include files are implicitly
> included via other include files. Just removing them because it works on one
> platform ends up with more niche operating systems failing to build, which we
> may only figure out months or even years down the road.
>
> At the very least you need to figure out why the includes where added and why
> that reason is not present anymore.

You're right - sorry for the oversight.


Thank you all for your guidance.

--
Regards,
Man Zeng
www.openhalo.org