pg.ddx.io  pgsql-hackers@postgresql.org mailing list archive  
help / color / mirror / Atom feed
Should we remove unused code?
3+ messages / 2 participants
[nested] [flat]

Commits from this thread:
  2026-03-31 dde69621c3  Remove HASH_SEGMENT option
* Should we remove unused code?
@ 2016-04-18 15:07  Aleksander Alekseev <a.alekseev@postgrespro.ru>
  0 siblings, 1 reply; 3+ messages in thread

From: Aleksander Alekseev @ 2016-04-18 15:07 UTC (permalink / raw)
  To: pgsql-hackers

Hello

Today I've read this post:

http://blog.2ndquadrant.com/code-coverage/

I think its great and that everyone in this mailing list should
familiarize themselves with it.

Reading the coverage report, I discovered that some code is never
executed. Sometimes its OK (error reporting in out-of-memory cases),
sometimes its not (not enough tests). But there are also cases I'm not
sure about.

For instance there are two flags - HASH_SEGMENT and HASH_FFACTOR that
are in fact never used (see attachment). I wonder whether we should
keep code like this or not. 

What do you think?

-- 
Best regards,
Aleksander Alekseev
http://eax.me/

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Attachments:

remove-unused-code.patch (text/x-patch, 1.4K) download diff --git a/src/backend/utils/hash/dynahash.c b/src/backend/utils/hash/dynahash.c index 1a9f70c..9b32ceb 100644 --- a/src/backend/utils/hash/dynahash.c +++ b/src/backend/utils/hash/dynahash.c @@ -452,16 +452,6 @@ hash_create(const char *tabname, long nelem, HASHCTL *info, int flags) hctl->num_partitions = info->num_partitions; } - if (flags & HASH_SEGMENT) - { - hctl->ssize = info->ssize; - hctl->sshift = my_log2(info->ssize); - /* ssize had better be a power of 2 */ - Assert(hctl->ssize == (1L << hctl->sshift)); - } - if (flags & HASH_FFACTOR) - hctl->ffactor = info->ffactor; - /* * SHM hash tables have fixed directory size passed by the caller. */ diff --git a/src/include/utils/hsearch.h b/src/include/utils/hsearch.h index 007ba2c..0646a72 100644 --- a/src/include/utils/hsearch.h +++ b/src/include/utils/hsearch.h @@ -81,9 +81,7 @@ typedef struct HASHCTL /* Flags to indicate which parameters are supplied */ #define HASH_PARTITION 0x0001 /* Hashtable is used w/partitioned locking */ -#define HASH_SEGMENT 0x0002 /* Set segment size */ #define HASH_DIRSIZE 0x0004 /* Set directory size (initial and max) */ -#define HASH_FFACTOR 0x0008 /* Set fill factor */ #define HASH_ELEM 0x0010 /* Set keysize and entrysize */ #define HASH_BLOBS 0x0020 /* Select support functions for binary keys */ #define HASH_FUNCTION 0x0040 /* Set user defined hash function */
^ permalink raw reply [nested|flat] 3+ messages in thread

* Re: Should we remove unused code?
@ 2016-04-18 15:18  Tom Lane <tgl@sss.pgh.pa.us>
  parent: Aleksander Alekseev <a.alekseev@postgrespro.ru>
  0 siblings, 1 reply; 3+ messages in thread

From: Tom Lane @ 2016-04-18 15:18 UTC (permalink / raw)
  To: Aleksander Alekseev <a.alekseev@postgrespro.ru>; +Cc: pgsql-hackers

Aleksander Alekseev <a.alekseev@postgrespro.ru> writes:
> For instance there are two flags - HASH_SEGMENT and HASH_FFACTOR that
> are in fact never used (see attachment). I wonder whether we should
> keep code like this or not. 

> What do you think?

The fact that make check-world doesn't run any code that uses some
feature isn't a great argument for removing it.  Consider third-party
extensions, for starters.

			regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers



^ permalink  raw  reply  [nested|flat] 3+ messages in thread

* Re: Should we remove unused code?
@ 2016-04-18 15:37  Aleksander Alekseev <a.alekseev@postgrespro.ru>
  parent: Tom Lane <tgl@sss.pgh.pa.us>
  0 siblings, 0 replies; 3+ messages in thread

From: Aleksander Alekseev @ 2016-04-18 15:37 UTC (permalink / raw)
  To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: pgsql-hackers

Hello, Tom.

Thanks for your reply.

> The fact that make check-world doesn't run any code that uses some
> feature isn't a great argument for removing it.  Consider third-party
> extensions, for starters.

What is current policy regarding such sort of things? Is everything
that is in .h files considered a public API and should be backward
compatible? In this case - for how long, forever or until next major
release? Or lets say there is some API that is not used neither
internally nor by any package available on PGXN. Can we break API then?

-- 
Best regards,
Aleksander Alekseev
http://eax.me/


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers



^ permalink  raw  reply  [nested|flat] 3+ messages in thread


end of thread, other threads:[~2016-04-18 15:37 UTC | newest]

Thread overview: 3+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2016-04-18 15:07 Should we remove unused code? Aleksander Alekseev <a.alekseev@postgrespro.ru>
2016-04-18 15:18 ` Tom Lane <tgl@sss.pgh.pa.us>
2016-04-18 15:37   ` Aleksander Alekseev <a.alekseev@postgrespro.ru>

This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox