[PATCH]Remove obsolete macro CHECKFLOATVAL in btree_gist

Started by tanghy.fnst@fujitsu.comover 4 years ago5 messageshackers
Jump to latest
#1tanghy.fnst@fujitsu.com
tanghy.fnst@fujitsu.com

Hi

Commit 6bf0bc842 replaced float.c's CHECKFLOATVAL() macro with static inline subroutines,
but the fix introduced a performance regression as Tom Lane pointed out and fixed at 607f8ce74.

Found obsolete CHECKFLOATVAL usage in contrib/btree_gist, and tried to fix it according to 607f8ce74.
The attached patch has been testified in master. All tap tests passed.

Regards,
Tang

Attachments:

0001-Remove-obsolete-macro-CHECKFLOATVAL.patchapplication/octet-stream; name=0001-Remove-obsolete-macro-CHECKFLOATVAL.patchDownload+7-22
#2tanghy.fnst@fujitsu.com
tanghy.fnst@fujitsu.com
In reply to: tanghy.fnst@fujitsu.com (#1)
RE: [PATCH]Remove obsolete macro CHECKFLOATVAL in btree_gist

On Friday, August 6, 2021 11:14 PM, tanghy.fnst@fujitsu.com <tanghy.fnst@fujitsu.com> wrote:

Commit 6bf0bc842 replaced float.c's CHECKFLOATVAL() macro with static inline subroutines,
but the fix introduced a performance regression as Tom Lane pointed out and fixed at 607f8ce74.

Found obsolete CHECKFLOATVAL usage in contrib/btree_gist, and tried to fix it according to 607f8ce74.

Added above patch in commit fest:
https://commitfest.postgresql.org/34/3287/

Regards,
Tang

#3Michael Paquier
michael@paquier.xyz
In reply to: tanghy.fnst@fujitsu.com (#2)
Re: [PATCH]Remove obsolete macro CHECKFLOATVAL in btree_gist

On Tue, Aug 17, 2021 at 11:08:59AM +0000, tanghy.fnst@fujitsu.com wrote:

On Friday, August 6, 2021 11:14 PM, tanghy.fnst@fujitsu.com <tanghy.fnst@fujitsu.com> wrote:

Commit 6bf0bc842 replaced float.c's CHECKFLOATVAL() macro with static inline subroutines,
but the fix introduced a performance regression as Tom Lane pointed out and fixed at 607f8ce74.

Found obsolete CHECKFLOATVAL usage in contrib/btree_gist, and tried to fix it according to 607f8ce74.

Added above patch in commit fest:
https://commitfest.postgresql.org/34/3287/

Yes, that does not seem wise on performance grounds. The case of
!zero_is_valid is never reached, so it seems like this code was just a
copy-paste from the float code in the backend. Your patch looks right
to me.
--
Michael

#4Michael Paquier
michael@paquier.xyz
In reply to: Michael Paquier (#3)
Re: [PATCH]Remove obsolete macro CHECKFLOATVAL in btree_gist

On Wed, Aug 18, 2021 at 10:04:04AM +0900, Michael Paquier wrote:

Yes, that does not seem wise on performance grounds. The case of
!zero_is_valid is never reached, so it seems like this code was just a
copy-paste from the float code in the backend. Your patch looks right
to me.

Applied.
--
Michael

#5tanghy.fnst@fujitsu.com
tanghy.fnst@fujitsu.com
In reply to: Michael Paquier (#4)
RE: [PATCH]Remove obsolete macro CHECKFLOATVAL in btree_gist

On Thursday, August 19, 2021 12:28 PM, Michael Paquier <michael@paquier.xyz> wrote

On Wed, Aug 18, 2021 at 10:04:04AM +0900, Michael Paquier wrote:

Yes, that does not seem wise on performance grounds. The case of
!zero_is_valid is never reached, so it seems like this code was just a
copy-paste from the float code in the backend. Your patch looks right
to me.

Applied.

Thanks for you check and commit, I've changed the patch's commit fest status to 'committed'.
https://commitfest.postgresql.org/34/3287/

Regards,
Tang