[PATCH] Renumber confusing value for GUC_UNIT_BYTE
The GUC units are currently defined like:
#define GUC_UNIT_KB 0x1000 /* value is in kilobytes */
#define GUC_UNIT_BLOCKS 0x2000 /* value is in blocks */
#define GUC_UNIT_XBLOCKS 0x3000 /* value is in xlog blocks */
#define GUC_UNIT_MB 0x4000 /* value is in megabytes */
#define GUC_UNIT_BYTE 0x8000 /* value is in bytes */
#define GUC_UNIT_MEMORY 0xF000 /* mask for size-related units */
#define GUC_UNIT_MS 0x10000 /* value is in milliseconds */
#define GUC_UNIT_S 0x20000 /* value is in seconds */
#define GUC_UNIT_MIN 0x30000 /* value is in minutes */
#define GUC_UNIT_TIME 0xF0000 /* mask for time-related units */
0x3000 and 0x30000 seemed wrong, since they're a combination of other flags
rather than being an independant power of two.
But actually, these aren't flags: they're tested in a "case" statement for
equality, not in a bitwise & test.
So the outlier is actually 0x8000, added at:
|commit 6e7baa322773ff8c79d4d8883c99fdeff5bfa679
|Author: Andres Freund <andres@anarazel.de>
|Date: Tue Sep 12 12:13:12 2017 -0700
|
| Introduce BYTES unit for GUCs.
It looks like that originated here:
/messages/by-id/CAOG9ApEu8bXVwBxkOO9J7ZpM76TASK_vFMEEiCEjwhMmSLiaqQ@mail.gmail.com
commit 162e4838103e7957cccfe7868fc28397b55ca1d7
Author: Justin Pryzby <pryzbyj@telsasoft.com>
Date: Wed Jul 20 09:27:24 2022 -0500
Renumber confusing value for GUC_UNIT_BYTE
It had a power-of-two value, which looks right, and causes the other values
which aren't powers-of-two to look wrong. But this is tested for equality and
not a bitwise test.
See also:
6e7baa322773ff8c79d4d8883c99fdeff5bfa679
/messages/by-id/CAOG9ApEu8bXVwBxkOO9J7ZpM76TASK_vFMEEiCEjwhMmSLiaqQ@mail.gmail.com
diff --git a/src/include/utils/guc.h b/src/include/utils/guc.h
index 4d0920c42e2..be928fac881 100644
--- a/src/include/utils/guc.h
+++ b/src/include/utils/guc.h
@@ -219,11 +219,12 @@ typedef enum
#define GUC_DISALLOW_IN_AUTO_FILE 0x0800 /* can't set in
* PG_AUTOCONF_FILENAME */+/* GUC_UNIT_* are not flags - they're tested for equality */
#define GUC_UNIT_KB 0x1000 /* value is in kilobytes */
#define GUC_UNIT_BLOCKS 0x2000 /* value is in blocks */
#define GUC_UNIT_XBLOCKS 0x3000 /* value is in xlog blocks */
#define GUC_UNIT_MB 0x4000 /* value is in megabytes */
-#define GUC_UNIT_BYTE 0x8000 /* value is in bytes */
+#define GUC_UNIT_BYTE 0x5000 /* value is in bytes */
#define GUC_UNIT_MEMORY 0xF000 /* mask for size-related units */#define GUC_UNIT_MS 0x10000 /* value is in milliseconds */
On 20.07.22 16:52, Justin Pryzby wrote:
+/* GUC_UNIT_* are not flags - they're tested for equality */
Well, there is GUC_UNIT_MEMORY, etc. so there is an additional
constraint beyond just "pick any number". I'm not sure that "flag" and
"tested for equality" are really antonyms anyway.
I think renumbering this makes sense. We could just leave the comment
as is if we don't come up with a better wording.
Show quoted text
#define GUC_UNIT_KB 0x1000 /* value is in kilobytes */ #define GUC_UNIT_BLOCKS 0x2000 /* value is in blocks */ #define GUC_UNIT_XBLOCKS 0x3000 /* value is in xlog blocks */ #define GUC_UNIT_MB 0x4000 /* value is in megabytes */ -#define GUC_UNIT_BYTE 0x8000 /* value is in bytes */ +#define GUC_UNIT_BYTE 0x5000 /* value is in bytes */ #define GUC_UNIT_MEMORY 0xF000 /* mask for size-related units */#define GUC_UNIT_MS 0x10000 /* value is in milliseconds */
Peter Eisentraut <peter.eisentraut@enterprisedb.com> writes:
I think renumbering this makes sense. We could just leave the comment
as is if we don't come up with a better wording.
+1, I see no need to change the comment. We just need to establish
the precedent that values within the GUC_UNIT_MEMORY field can be
chosen sequentially.
regards, tom lane
On Tue, Sep 06, 2022 at 01:57:53AM -0400, Tom Lane wrote:
Peter Eisentraut <peter.eisentraut@enterprisedb.com> writes:
I think renumbering this makes sense. We could just leave the comment
as is if we don't come up with a better wording.+1, I see no need to change the comment. We just need to establish
the precedent that values within the GUC_UNIT_MEMORY field can be
chosen sequentially.
+1.
--
MichaelOn 06.09.22 08:27, Michael Paquier wrote:
On Tue, Sep 06, 2022 at 01:57:53AM -0400, Tom Lane wrote:
Peter Eisentraut <peter.eisentraut@enterprisedb.com> writes:
I think renumbering this makes sense. We could just leave the comment
as is if we don't come up with a better wording.+1, I see no need to change the comment. We just need to establish
the precedent that values within the GUC_UNIT_MEMORY field can be
chosen sequentially.+1.
committed without the comment change
On Wed, Sep 07, 2022 at 11:11:37AM +0200, Peter Eisentraut wrote:
On 06.09.22 08:27, Michael Paquier wrote:
On Tue, Sep 06, 2022 at 01:57:53AM -0400, Tom Lane wrote:
Peter Eisentraut <peter.eisentraut@enterprisedb.com> writes:
I think renumbering this makes sense. We could just leave the comment
as is if we don't come up with a better wording.+1, I see no need to change the comment. We just need to establish
the precedent that values within the GUC_UNIT_MEMORY field can be
chosen sequentially.+1.
committed without the comment change
Thank you