agora inbox for [email protected]
help / color / mirror / Atom feed[PATCH v12 07/17] Reduce scope of BitmapHeapScan tbmiterator local variables
30+ messages / 5 participants
[nested] [flat]
* [PATCH v6 08/14] Reduce scope of BitmapHeapScan tbmiterator local variables
@ 2024-02-13 15:17 Melanie Plageman <[email protected]>
0 siblings, 0 replies; 30+ messages in thread
From: Melanie Plageman @ 2024-02-13 15:17 UTC (permalink / raw)
To simplify the diff of a future commit which will move the TBMIterators
into the scan descriptor, define them in a narrower scope now.
---
src/backend/executor/nodeBitmapHeapscan.c | 20 +++++++++-----------
1 file changed, 9 insertions(+), 11 deletions(-)
diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c
index 08e346ae7b3..248ff90904b 100644
--- a/src/backend/executor/nodeBitmapHeapscan.c
+++ b/src/backend/executor/nodeBitmapHeapscan.c
@@ -71,8 +71,6 @@ BitmapHeapNext(BitmapHeapScanState *node)
ExprContext *econtext;
TableScanDesc scan;
TIDBitmap *tbm;
- TBMIterator *tbmiterator = NULL;
- TBMSharedIterator *shared_tbmiterator = NULL;
TBMIterateResult *tbmres;
TupleTableSlot *slot;
ParallelBitmapHeapState *pstate = node->pstate;
@@ -85,10 +83,6 @@ BitmapHeapNext(BitmapHeapScanState *node)
slot = node->ss.ss_ScanTupleSlot;
scan = node->ss.ss_currentScanDesc;
tbm = node->tbm;
- if (pstate == NULL)
- tbmiterator = node->tbmiterator;
- else
- shared_tbmiterator = node->shared_tbmiterator;
tbmres = node->tbmres;
/*
@@ -105,6 +99,9 @@ BitmapHeapNext(BitmapHeapScanState *node)
*/
if (!node->initialized)
{
+ TBMIterator *tbmiterator = NULL;
+ TBMSharedIterator *shared_tbmiterator = NULL;
+
if (!pstate)
{
tbm = (TIDBitmap *) MultiExecProcNode(outerPlanState(node));
@@ -113,7 +110,7 @@ BitmapHeapNext(BitmapHeapScanState *node)
elog(ERROR, "unrecognized result from subplan");
node->tbm = tbm;
- node->tbmiterator = tbmiterator = tbm_begin_iterate(tbm);
+ tbmiterator = tbm_begin_iterate(tbm);
node->tbmres = tbmres = NULL;
#ifdef USE_PREFETCH
@@ -166,8 +163,7 @@ BitmapHeapNext(BitmapHeapScanState *node)
}
/* Allocate a private iterator and attach the shared state to it */
- node->shared_tbmiterator = shared_tbmiterator =
- tbm_attach_shared_iterate(dsa, pstate->tbmiterator);
+ shared_tbmiterator = tbm_attach_shared_iterate(dsa, pstate->tbmiterator);
node->tbmres = tbmres = NULL;
#ifdef USE_PREFETCH
@@ -206,6 +202,8 @@ BitmapHeapNext(BitmapHeapScanState *node)
extra_flags);
}
+ node->tbmiterator = tbmiterator;
+ node->shared_tbmiterator = shared_tbmiterator;
node->initialized = true;
}
@@ -221,9 +219,9 @@ BitmapHeapNext(BitmapHeapScanState *node)
if (tbmres == NULL)
{
if (!pstate)
- node->tbmres = tbmres = tbm_iterate(tbmiterator);
+ node->tbmres = tbmres = tbm_iterate(node->tbmiterator);
else
- node->tbmres = tbmres = tbm_shared_iterate(shared_tbmiterator);
+ node->tbmres = tbmres = tbm_shared_iterate(node->shared_tbmiterator);
if (tbmres == NULL)
{
/* no more entries in the bitmap */
--
2.40.1
--w4wcjcocxsm37usi
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v6-0009-Remove-table_scan_bitmap_next_tuple-parameter-tbm.patch"
^ permalink raw reply [nested|flat] 30+ messages in thread
* [PATCH v8 07/17] Reduce scope of BitmapHeapScan tbmiterator local variables
@ 2024-02-13 15:17 Melanie Plageman <[email protected]>
0 siblings, 0 replies; 30+ messages in thread
From: Melanie Plageman @ 2024-02-13 15:17 UTC (permalink / raw)
To simplify the diff of a future commit which will move the TBMIterators
into the scan descriptor, define them in a narrower scope now.
---
src/backend/executor/nodeBitmapHeapscan.c | 20 +++++++++-----------
1 file changed, 9 insertions(+), 11 deletions(-)
diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c
index c95e3412da..49938c9ed4 100644
--- a/src/backend/executor/nodeBitmapHeapscan.c
+++ b/src/backend/executor/nodeBitmapHeapscan.c
@@ -71,8 +71,6 @@ BitmapHeapNext(BitmapHeapScanState *node)
ExprContext *econtext;
TableScanDesc scan;
TIDBitmap *tbm;
- TBMIterator *tbmiterator = NULL;
- TBMSharedIterator *shared_tbmiterator = NULL;
TBMIterateResult *tbmres;
TupleTableSlot *slot;
ParallelBitmapHeapState *pstate = node->pstate;
@@ -85,10 +83,6 @@ BitmapHeapNext(BitmapHeapScanState *node)
slot = node->ss.ss_ScanTupleSlot;
scan = node->ss.ss_currentScanDesc;
tbm = node->tbm;
- if (pstate == NULL)
- tbmiterator = node->tbmiterator;
- else
- shared_tbmiterator = node->shared_tbmiterator;
tbmres = node->tbmres;
/*
@@ -105,6 +99,9 @@ BitmapHeapNext(BitmapHeapScanState *node)
*/
if (!node->initialized)
{
+ TBMIterator *tbmiterator = NULL;
+ TBMSharedIterator *shared_tbmiterator = NULL;
+
if (!pstate)
{
tbm = (TIDBitmap *) MultiExecProcNode(outerPlanState(node));
@@ -113,7 +110,7 @@ BitmapHeapNext(BitmapHeapScanState *node)
elog(ERROR, "unrecognized result from subplan");
node->tbm = tbm;
- node->tbmiterator = tbmiterator = tbm_begin_iterate(tbm);
+ tbmiterator = tbm_begin_iterate(tbm);
node->tbmres = tbmres = NULL;
#ifdef USE_PREFETCH
@@ -166,8 +163,7 @@ BitmapHeapNext(BitmapHeapScanState *node)
}
/* Allocate a private iterator and attach the shared state to it */
- node->shared_tbmiterator = shared_tbmiterator =
- tbm_attach_shared_iterate(dsa, pstate->tbmiterator);
+ shared_tbmiterator = tbm_attach_shared_iterate(dsa, pstate->tbmiterator);
node->tbmres = tbmres = NULL;
#ifdef USE_PREFETCH
@@ -206,6 +202,8 @@ BitmapHeapNext(BitmapHeapScanState *node)
extra_flags);
}
+ node->tbmiterator = tbmiterator;
+ node->shared_tbmiterator = shared_tbmiterator;
node->initialized = true;
}
@@ -221,9 +219,9 @@ BitmapHeapNext(BitmapHeapScanState *node)
if (tbmres == NULL)
{
if (!pstate)
- node->tbmres = tbmres = tbm_iterate(tbmiterator);
+ node->tbmres = tbmres = tbm_iterate(node->tbmiterator);
else
- node->tbmres = tbmres = tbm_shared_iterate(shared_tbmiterator);
+ node->tbmres = tbmres = tbm_shared_iterate(node->shared_tbmiterator);
if (tbmres == NULL)
{
/* no more entries in the bitmap */
--
2.40.1
--xqq4defy3uncu6k6
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v8-0008-Remove-table_scan_bitmap_next_tuple-parameter-tbm.patch"
^ permalink raw reply [nested|flat] 30+ messages in thread
* [PATCH v7 07/13] Reduce scope of BitmapHeapScan tbmiterator local variables
@ 2024-02-13 15:17 Melanie Plageman <[email protected]>
0 siblings, 0 replies; 30+ messages in thread
From: Melanie Plageman @ 2024-02-13 15:17 UTC (permalink / raw)
To simplify the diff of a future commit which will move the TBMIterators
into the scan descriptor, define them in a narrower scope now.
---
src/backend/executor/nodeBitmapHeapscan.c | 20 +++++++++-----------
1 file changed, 9 insertions(+), 11 deletions(-)
diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c
index b2397fe2054..94a5e2da17c 100644
--- a/src/backend/executor/nodeBitmapHeapscan.c
+++ b/src/backend/executor/nodeBitmapHeapscan.c
@@ -71,8 +71,6 @@ BitmapHeapNext(BitmapHeapScanState *node)
ExprContext *econtext;
TableScanDesc scan;
TIDBitmap *tbm;
- TBMIterator *tbmiterator = NULL;
- TBMSharedIterator *shared_tbmiterator = NULL;
TBMIterateResult *tbmres;
TupleTableSlot *slot;
ParallelBitmapHeapState *pstate = node->pstate;
@@ -85,10 +83,6 @@ BitmapHeapNext(BitmapHeapScanState *node)
slot = node->ss.ss_ScanTupleSlot;
scan = node->ss.ss_currentScanDesc;
tbm = node->tbm;
- if (pstate == NULL)
- tbmiterator = node->tbmiterator;
- else
- shared_tbmiterator = node->shared_tbmiterator;
tbmres = node->tbmres;
/*
@@ -105,6 +99,9 @@ BitmapHeapNext(BitmapHeapScanState *node)
*/
if (!node->initialized)
{
+ TBMIterator *tbmiterator = NULL;
+ TBMSharedIterator *shared_tbmiterator = NULL;
+
if (!pstate)
{
tbm = (TIDBitmap *) MultiExecProcNode(outerPlanState(node));
@@ -113,7 +110,7 @@ BitmapHeapNext(BitmapHeapScanState *node)
elog(ERROR, "unrecognized result from subplan");
node->tbm = tbm;
- node->tbmiterator = tbmiterator = tbm_begin_iterate(tbm);
+ tbmiterator = tbm_begin_iterate(tbm);
node->tbmres = tbmres = NULL;
#ifdef USE_PREFETCH
@@ -166,8 +163,7 @@ BitmapHeapNext(BitmapHeapScanState *node)
}
/* Allocate a private iterator and attach the shared state to it */
- node->shared_tbmiterator = shared_tbmiterator =
- tbm_attach_shared_iterate(dsa, pstate->tbmiterator);
+ shared_tbmiterator = tbm_attach_shared_iterate(dsa, pstate->tbmiterator);
node->tbmres = tbmres = NULL;
#ifdef USE_PREFETCH
@@ -206,6 +202,8 @@ BitmapHeapNext(BitmapHeapScanState *node)
extra_flags);
}
+ node->tbmiterator = tbmiterator;
+ node->shared_tbmiterator = shared_tbmiterator;
node->initialized = true;
}
@@ -221,9 +219,9 @@ BitmapHeapNext(BitmapHeapScanState *node)
if (tbmres == NULL)
{
if (!pstate)
- node->tbmres = tbmres = tbm_iterate(tbmiterator);
+ node->tbmres = tbmres = tbm_iterate(node->tbmiterator);
else
- node->tbmres = tbmres = tbm_shared_iterate(shared_tbmiterator);
+ node->tbmres = tbmres = tbm_shared_iterate(node->shared_tbmiterator);
if (tbmres == NULL)
{
/* no more entries in the bitmap */
--
2.40.1
--kqqpqghcwbcc3dt5
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v7-0008-Remove-table_scan_bitmap_next_tuple-parameter-tbm.patch"
^ permalink raw reply [nested|flat] 30+ messages in thread
* [PATCH v7 07/13] Reduce scope of BitmapHeapScan tbmiterator local variables
@ 2024-02-13 15:17 Melanie Plageman <[email protected]>
0 siblings, 0 replies; 30+ messages in thread
From: Melanie Plageman @ 2024-02-13 15:17 UTC (permalink / raw)
To simplify the diff of a future commit which will move the TBMIterators
into the scan descriptor, define them in a narrower scope now.
---
src/backend/executor/nodeBitmapHeapscan.c | 20 +++++++++-----------
1 file changed, 9 insertions(+), 11 deletions(-)
diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c
index b2397fe2054..94a5e2da17c 100644
--- a/src/backend/executor/nodeBitmapHeapscan.c
+++ b/src/backend/executor/nodeBitmapHeapscan.c
@@ -71,8 +71,6 @@ BitmapHeapNext(BitmapHeapScanState *node)
ExprContext *econtext;
TableScanDesc scan;
TIDBitmap *tbm;
- TBMIterator *tbmiterator = NULL;
- TBMSharedIterator *shared_tbmiterator = NULL;
TBMIterateResult *tbmres;
TupleTableSlot *slot;
ParallelBitmapHeapState *pstate = node->pstate;
@@ -85,10 +83,6 @@ BitmapHeapNext(BitmapHeapScanState *node)
slot = node->ss.ss_ScanTupleSlot;
scan = node->ss.ss_currentScanDesc;
tbm = node->tbm;
- if (pstate == NULL)
- tbmiterator = node->tbmiterator;
- else
- shared_tbmiterator = node->shared_tbmiterator;
tbmres = node->tbmres;
/*
@@ -105,6 +99,9 @@ BitmapHeapNext(BitmapHeapScanState *node)
*/
if (!node->initialized)
{
+ TBMIterator *tbmiterator = NULL;
+ TBMSharedIterator *shared_tbmiterator = NULL;
+
if (!pstate)
{
tbm = (TIDBitmap *) MultiExecProcNode(outerPlanState(node));
@@ -113,7 +110,7 @@ BitmapHeapNext(BitmapHeapScanState *node)
elog(ERROR, "unrecognized result from subplan");
node->tbm = tbm;
- node->tbmiterator = tbmiterator = tbm_begin_iterate(tbm);
+ tbmiterator = tbm_begin_iterate(tbm);
node->tbmres = tbmres = NULL;
#ifdef USE_PREFETCH
@@ -166,8 +163,7 @@ BitmapHeapNext(BitmapHeapScanState *node)
}
/* Allocate a private iterator and attach the shared state to it */
- node->shared_tbmiterator = shared_tbmiterator =
- tbm_attach_shared_iterate(dsa, pstate->tbmiterator);
+ shared_tbmiterator = tbm_attach_shared_iterate(dsa, pstate->tbmiterator);
node->tbmres = tbmres = NULL;
#ifdef USE_PREFETCH
@@ -206,6 +202,8 @@ BitmapHeapNext(BitmapHeapScanState *node)
extra_flags);
}
+ node->tbmiterator = tbmiterator;
+ node->shared_tbmiterator = shared_tbmiterator;
node->initialized = true;
}
@@ -221,9 +219,9 @@ BitmapHeapNext(BitmapHeapScanState *node)
if (tbmres == NULL)
{
if (!pstate)
- node->tbmres = tbmres = tbm_iterate(tbmiterator);
+ node->tbmres = tbmres = tbm_iterate(node->tbmiterator);
else
- node->tbmres = tbmres = tbm_shared_iterate(shared_tbmiterator);
+ node->tbmres = tbmres = tbm_shared_iterate(node->shared_tbmiterator);
if (tbmres == NULL)
{
/* no more entries in the bitmap */
--
2.40.1
--kqqpqghcwbcc3dt5
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v7-0008-Remove-table_scan_bitmap_next_tuple-parameter-tbm.patch"
^ permalink raw reply [nested|flat] 30+ messages in thread
* [PATCH v10 07/17] Reduce scope of BitmapHeapScan tbmiterator local variables
@ 2024-02-13 15:17 Melanie Plageman <[email protected]>
0 siblings, 0 replies; 30+ messages in thread
From: Melanie Plageman @ 2024-02-13 15:17 UTC (permalink / raw)
To simplify the diff of a future commit which will move the TBMIterators
into the scan descriptor, define them in a narrower scope now.
---
src/backend/executor/nodeBitmapHeapscan.c | 20 +++++++++-----------
1 file changed, 9 insertions(+), 11 deletions(-)
diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c
index c95e3412da..49938c9ed4 100644
--- a/src/backend/executor/nodeBitmapHeapscan.c
+++ b/src/backend/executor/nodeBitmapHeapscan.c
@@ -71,8 +71,6 @@ BitmapHeapNext(BitmapHeapScanState *node)
ExprContext *econtext;
TableScanDesc scan;
TIDBitmap *tbm;
- TBMIterator *tbmiterator = NULL;
- TBMSharedIterator *shared_tbmiterator = NULL;
TBMIterateResult *tbmres;
TupleTableSlot *slot;
ParallelBitmapHeapState *pstate = node->pstate;
@@ -85,10 +83,6 @@ BitmapHeapNext(BitmapHeapScanState *node)
slot = node->ss.ss_ScanTupleSlot;
scan = node->ss.ss_currentScanDesc;
tbm = node->tbm;
- if (pstate == NULL)
- tbmiterator = node->tbmiterator;
- else
- shared_tbmiterator = node->shared_tbmiterator;
tbmres = node->tbmres;
/*
@@ -105,6 +99,9 @@ BitmapHeapNext(BitmapHeapScanState *node)
*/
if (!node->initialized)
{
+ TBMIterator *tbmiterator = NULL;
+ TBMSharedIterator *shared_tbmiterator = NULL;
+
if (!pstate)
{
tbm = (TIDBitmap *) MultiExecProcNode(outerPlanState(node));
@@ -113,7 +110,7 @@ BitmapHeapNext(BitmapHeapScanState *node)
elog(ERROR, "unrecognized result from subplan");
node->tbm = tbm;
- node->tbmiterator = tbmiterator = tbm_begin_iterate(tbm);
+ tbmiterator = tbm_begin_iterate(tbm);
node->tbmres = tbmres = NULL;
#ifdef USE_PREFETCH
@@ -166,8 +163,7 @@ BitmapHeapNext(BitmapHeapScanState *node)
}
/* Allocate a private iterator and attach the shared state to it */
- node->shared_tbmiterator = shared_tbmiterator =
- tbm_attach_shared_iterate(dsa, pstate->tbmiterator);
+ shared_tbmiterator = tbm_attach_shared_iterate(dsa, pstate->tbmiterator);
node->tbmres = tbmres = NULL;
#ifdef USE_PREFETCH
@@ -206,6 +202,8 @@ BitmapHeapNext(BitmapHeapScanState *node)
extra_flags);
}
+ node->tbmiterator = tbmiterator;
+ node->shared_tbmiterator = shared_tbmiterator;
node->initialized = true;
}
@@ -221,9 +219,9 @@ BitmapHeapNext(BitmapHeapScanState *node)
if (tbmres == NULL)
{
if (!pstate)
- node->tbmres = tbmres = tbm_iterate(tbmiterator);
+ node->tbmres = tbmres = tbm_iterate(node->tbmiterator);
else
- node->tbmres = tbmres = tbm_shared_iterate(shared_tbmiterator);
+ node->tbmres = tbmres = tbm_shared_iterate(node->shared_tbmiterator);
if (tbmres == NULL)
{
/* no more entries in the bitmap */
--
2.40.1
--3o7pc6dfau5a5hry
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v10-0008-Remove-table_scan_bitmap_next_tuple-parameter-tb.patch"
^ permalink raw reply [nested|flat] 30+ messages in thread
* [PATCH v10 07/17] Reduce scope of BitmapHeapScan tbmiterator local variables
@ 2024-02-13 15:17 Melanie Plageman <[email protected]>
0 siblings, 0 replies; 30+ messages in thread
From: Melanie Plageman @ 2024-02-13 15:17 UTC (permalink / raw)
To simplify the diff of a future commit which will move the TBMIterators
into the scan descriptor, define them in a narrower scope now.
---
src/backend/executor/nodeBitmapHeapscan.c | 20 +++++++++-----------
1 file changed, 9 insertions(+), 11 deletions(-)
diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c
index c95e3412da..49938c9ed4 100644
--- a/src/backend/executor/nodeBitmapHeapscan.c
+++ b/src/backend/executor/nodeBitmapHeapscan.c
@@ -71,8 +71,6 @@ BitmapHeapNext(BitmapHeapScanState *node)
ExprContext *econtext;
TableScanDesc scan;
TIDBitmap *tbm;
- TBMIterator *tbmiterator = NULL;
- TBMSharedIterator *shared_tbmiterator = NULL;
TBMIterateResult *tbmres;
TupleTableSlot *slot;
ParallelBitmapHeapState *pstate = node->pstate;
@@ -85,10 +83,6 @@ BitmapHeapNext(BitmapHeapScanState *node)
slot = node->ss.ss_ScanTupleSlot;
scan = node->ss.ss_currentScanDesc;
tbm = node->tbm;
- if (pstate == NULL)
- tbmiterator = node->tbmiterator;
- else
- shared_tbmiterator = node->shared_tbmiterator;
tbmres = node->tbmres;
/*
@@ -105,6 +99,9 @@ BitmapHeapNext(BitmapHeapScanState *node)
*/
if (!node->initialized)
{
+ TBMIterator *tbmiterator = NULL;
+ TBMSharedIterator *shared_tbmiterator = NULL;
+
if (!pstate)
{
tbm = (TIDBitmap *) MultiExecProcNode(outerPlanState(node));
@@ -113,7 +110,7 @@ BitmapHeapNext(BitmapHeapScanState *node)
elog(ERROR, "unrecognized result from subplan");
node->tbm = tbm;
- node->tbmiterator = tbmiterator = tbm_begin_iterate(tbm);
+ tbmiterator = tbm_begin_iterate(tbm);
node->tbmres = tbmres = NULL;
#ifdef USE_PREFETCH
@@ -166,8 +163,7 @@ BitmapHeapNext(BitmapHeapScanState *node)
}
/* Allocate a private iterator and attach the shared state to it */
- node->shared_tbmiterator = shared_tbmiterator =
- tbm_attach_shared_iterate(dsa, pstate->tbmiterator);
+ shared_tbmiterator = tbm_attach_shared_iterate(dsa, pstate->tbmiterator);
node->tbmres = tbmres = NULL;
#ifdef USE_PREFETCH
@@ -206,6 +202,8 @@ BitmapHeapNext(BitmapHeapScanState *node)
extra_flags);
}
+ node->tbmiterator = tbmiterator;
+ node->shared_tbmiterator = shared_tbmiterator;
node->initialized = true;
}
@@ -221,9 +219,9 @@ BitmapHeapNext(BitmapHeapScanState *node)
if (tbmres == NULL)
{
if (!pstate)
- node->tbmres = tbmres = tbm_iterate(tbmiterator);
+ node->tbmres = tbmres = tbm_iterate(node->tbmiterator);
else
- node->tbmres = tbmres = tbm_shared_iterate(shared_tbmiterator);
+ node->tbmres = tbmres = tbm_shared_iterate(node->shared_tbmiterator);
if (tbmres == NULL)
{
/* no more entries in the bitmap */
--
2.40.1
--3o7pc6dfau5a5hry
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v10-0008-Remove-table_scan_bitmap_next_tuple-parameter-tb.patch"
^ permalink raw reply [nested|flat] 30+ messages in thread
* [PATCH v9 07/17] Reduce scope of BitmapHeapScan tbmiterator local variables
@ 2024-02-13 15:17 Melanie Plageman <[email protected]>
0 siblings, 0 replies; 30+ messages in thread
From: Melanie Plageman @ 2024-02-13 15:17 UTC (permalink / raw)
To simplify the diff of a future commit which will move the TBMIterators
into the scan descriptor, define them in a narrower scope now.
---
src/backend/executor/nodeBitmapHeapscan.c | 20 +++++++++-----------
1 file changed, 9 insertions(+), 11 deletions(-)
diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c
index c95e3412da..49938c9ed4 100644
--- a/src/backend/executor/nodeBitmapHeapscan.c
+++ b/src/backend/executor/nodeBitmapHeapscan.c
@@ -71,8 +71,6 @@ BitmapHeapNext(BitmapHeapScanState *node)
ExprContext *econtext;
TableScanDesc scan;
TIDBitmap *tbm;
- TBMIterator *tbmiterator = NULL;
- TBMSharedIterator *shared_tbmiterator = NULL;
TBMIterateResult *tbmres;
TupleTableSlot *slot;
ParallelBitmapHeapState *pstate = node->pstate;
@@ -85,10 +83,6 @@ BitmapHeapNext(BitmapHeapScanState *node)
slot = node->ss.ss_ScanTupleSlot;
scan = node->ss.ss_currentScanDesc;
tbm = node->tbm;
- if (pstate == NULL)
- tbmiterator = node->tbmiterator;
- else
- shared_tbmiterator = node->shared_tbmiterator;
tbmres = node->tbmres;
/*
@@ -105,6 +99,9 @@ BitmapHeapNext(BitmapHeapScanState *node)
*/
if (!node->initialized)
{
+ TBMIterator *tbmiterator = NULL;
+ TBMSharedIterator *shared_tbmiterator = NULL;
+
if (!pstate)
{
tbm = (TIDBitmap *) MultiExecProcNode(outerPlanState(node));
@@ -113,7 +110,7 @@ BitmapHeapNext(BitmapHeapScanState *node)
elog(ERROR, "unrecognized result from subplan");
node->tbm = tbm;
- node->tbmiterator = tbmiterator = tbm_begin_iterate(tbm);
+ tbmiterator = tbm_begin_iterate(tbm);
node->tbmres = tbmres = NULL;
#ifdef USE_PREFETCH
@@ -166,8 +163,7 @@ BitmapHeapNext(BitmapHeapScanState *node)
}
/* Allocate a private iterator and attach the shared state to it */
- node->shared_tbmiterator = shared_tbmiterator =
- tbm_attach_shared_iterate(dsa, pstate->tbmiterator);
+ shared_tbmiterator = tbm_attach_shared_iterate(dsa, pstate->tbmiterator);
node->tbmres = tbmres = NULL;
#ifdef USE_PREFETCH
@@ -206,6 +202,8 @@ BitmapHeapNext(BitmapHeapScanState *node)
extra_flags);
}
+ node->tbmiterator = tbmiterator;
+ node->shared_tbmiterator = shared_tbmiterator;
node->initialized = true;
}
@@ -221,9 +219,9 @@ BitmapHeapNext(BitmapHeapScanState *node)
if (tbmres == NULL)
{
if (!pstate)
- node->tbmres = tbmres = tbm_iterate(tbmiterator);
+ node->tbmres = tbmres = tbm_iterate(node->tbmiterator);
else
- node->tbmres = tbmres = tbm_shared_iterate(shared_tbmiterator);
+ node->tbmres = tbmres = tbm_shared_iterate(node->shared_tbmiterator);
if (tbmres == NULL)
{
/* no more entries in the bitmap */
--
2.40.1
--7mdtsjmrzitrgzgx
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v9-0008-Remove-table_scan_bitmap_next_tuple-parameter-tbm.patch"
^ permalink raw reply [nested|flat] 30+ messages in thread
* [PATCH v11 07/17] Reduce scope of BitmapHeapScan tbmiterator local variables
@ 2024-02-13 15:17 Melanie Plageman <[email protected]>
0 siblings, 0 replies; 30+ messages in thread
From: Melanie Plageman @ 2024-02-13 15:17 UTC (permalink / raw)
To simplify the diff of a future commit which will move the TBMIterators
into the scan descriptor, define them in a narrower scope now.
---
src/backend/executor/nodeBitmapHeapscan.c | 20 +++++++++-----------
1 file changed, 9 insertions(+), 11 deletions(-)
diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c
index c95e3412da..49938c9ed4 100644
--- a/src/backend/executor/nodeBitmapHeapscan.c
+++ b/src/backend/executor/nodeBitmapHeapscan.c
@@ -71,8 +71,6 @@ BitmapHeapNext(BitmapHeapScanState *node)
ExprContext *econtext;
TableScanDesc scan;
TIDBitmap *tbm;
- TBMIterator *tbmiterator = NULL;
- TBMSharedIterator *shared_tbmiterator = NULL;
TBMIterateResult *tbmres;
TupleTableSlot *slot;
ParallelBitmapHeapState *pstate = node->pstate;
@@ -85,10 +83,6 @@ BitmapHeapNext(BitmapHeapScanState *node)
slot = node->ss.ss_ScanTupleSlot;
scan = node->ss.ss_currentScanDesc;
tbm = node->tbm;
- if (pstate == NULL)
- tbmiterator = node->tbmiterator;
- else
- shared_tbmiterator = node->shared_tbmiterator;
tbmres = node->tbmres;
/*
@@ -105,6 +99,9 @@ BitmapHeapNext(BitmapHeapScanState *node)
*/
if (!node->initialized)
{
+ TBMIterator *tbmiterator = NULL;
+ TBMSharedIterator *shared_tbmiterator = NULL;
+
if (!pstate)
{
tbm = (TIDBitmap *) MultiExecProcNode(outerPlanState(node));
@@ -113,7 +110,7 @@ BitmapHeapNext(BitmapHeapScanState *node)
elog(ERROR, "unrecognized result from subplan");
node->tbm = tbm;
- node->tbmiterator = tbmiterator = tbm_begin_iterate(tbm);
+ tbmiterator = tbm_begin_iterate(tbm);
node->tbmres = tbmres = NULL;
#ifdef USE_PREFETCH
@@ -166,8 +163,7 @@ BitmapHeapNext(BitmapHeapScanState *node)
}
/* Allocate a private iterator and attach the shared state to it */
- node->shared_tbmiterator = shared_tbmiterator =
- tbm_attach_shared_iterate(dsa, pstate->tbmiterator);
+ shared_tbmiterator = tbm_attach_shared_iterate(dsa, pstate->tbmiterator);
node->tbmres = tbmres = NULL;
#ifdef USE_PREFETCH
@@ -206,6 +202,8 @@ BitmapHeapNext(BitmapHeapScanState *node)
extra_flags);
}
+ node->tbmiterator = tbmiterator;
+ node->shared_tbmiterator = shared_tbmiterator;
node->initialized = true;
}
@@ -221,9 +219,9 @@ BitmapHeapNext(BitmapHeapScanState *node)
if (tbmres == NULL)
{
if (!pstate)
- node->tbmres = tbmres = tbm_iterate(tbmiterator);
+ node->tbmres = tbmres = tbm_iterate(node->tbmiterator);
else
- node->tbmres = tbmres = tbm_shared_iterate(shared_tbmiterator);
+ node->tbmres = tbmres = tbm_shared_iterate(node->shared_tbmiterator);
if (tbmres == NULL)
{
/* no more entries in the bitmap */
--
2.40.1
--owzzsiozz6hgpp7e
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v11-0008-Remove-table_scan_bitmap_next_tuple-parameter-tb.patch"
^ permalink raw reply [nested|flat] 30+ messages in thread
* [PATCH v12 07/17] Reduce scope of BitmapHeapScan tbmiterator local variables
@ 2024-02-13 15:17 Melanie Plageman <[email protected]>
0 siblings, 0 replies; 30+ messages in thread
From: Melanie Plageman @ 2024-02-13 15:17 UTC (permalink / raw)
To simplify the diff of a future commit which will move the TBMIterators
into the scan descriptor, define them in a narrower scope now.
---
src/backend/executor/nodeBitmapHeapscan.c | 20 +++++++++-----------
1 file changed, 9 insertions(+), 11 deletions(-)
diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c
index c95e3412da8..49938c9ed41 100644
--- a/src/backend/executor/nodeBitmapHeapscan.c
+++ b/src/backend/executor/nodeBitmapHeapscan.c
@@ -71,8 +71,6 @@ BitmapHeapNext(BitmapHeapScanState *node)
ExprContext *econtext;
TableScanDesc scan;
TIDBitmap *tbm;
- TBMIterator *tbmiterator = NULL;
- TBMSharedIterator *shared_tbmiterator = NULL;
TBMIterateResult *tbmres;
TupleTableSlot *slot;
ParallelBitmapHeapState *pstate = node->pstate;
@@ -85,10 +83,6 @@ BitmapHeapNext(BitmapHeapScanState *node)
slot = node->ss.ss_ScanTupleSlot;
scan = node->ss.ss_currentScanDesc;
tbm = node->tbm;
- if (pstate == NULL)
- tbmiterator = node->tbmiterator;
- else
- shared_tbmiterator = node->shared_tbmiterator;
tbmres = node->tbmres;
/*
@@ -105,6 +99,9 @@ BitmapHeapNext(BitmapHeapScanState *node)
*/
if (!node->initialized)
{
+ TBMIterator *tbmiterator = NULL;
+ TBMSharedIterator *shared_tbmiterator = NULL;
+
if (!pstate)
{
tbm = (TIDBitmap *) MultiExecProcNode(outerPlanState(node));
@@ -113,7 +110,7 @@ BitmapHeapNext(BitmapHeapScanState *node)
elog(ERROR, "unrecognized result from subplan");
node->tbm = tbm;
- node->tbmiterator = tbmiterator = tbm_begin_iterate(tbm);
+ tbmiterator = tbm_begin_iterate(tbm);
node->tbmres = tbmres = NULL;
#ifdef USE_PREFETCH
@@ -166,8 +163,7 @@ BitmapHeapNext(BitmapHeapScanState *node)
}
/* Allocate a private iterator and attach the shared state to it */
- node->shared_tbmiterator = shared_tbmiterator =
- tbm_attach_shared_iterate(dsa, pstate->tbmiterator);
+ shared_tbmiterator = tbm_attach_shared_iterate(dsa, pstate->tbmiterator);
node->tbmres = tbmres = NULL;
#ifdef USE_PREFETCH
@@ -206,6 +202,8 @@ BitmapHeapNext(BitmapHeapScanState *node)
extra_flags);
}
+ node->tbmiterator = tbmiterator;
+ node->shared_tbmiterator = shared_tbmiterator;
node->initialized = true;
}
@@ -221,9 +219,9 @@ BitmapHeapNext(BitmapHeapScanState *node)
if (tbmres == NULL)
{
if (!pstate)
- node->tbmres = tbmres = tbm_iterate(tbmiterator);
+ node->tbmres = tbmres = tbm_iterate(node->tbmiterator);
else
- node->tbmres = tbmres = tbm_shared_iterate(shared_tbmiterator);
+ node->tbmres = tbmres = tbm_shared_iterate(node->shared_tbmiterator);
if (tbmres == NULL)
{
/* no more entries in the bitmap */
--
2.40.1
--6jpz2j246qmht4bt
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v12-0008-Remove-table_scan_bitmap_next_tuple-parameter-tb.patch"
^ permalink raw reply [nested|flat] 30+ messages in thread
* [PATCH v6 08/14] Reduce scope of BitmapHeapScan tbmiterator local variables
@ 2024-02-13 15:17 Melanie Plageman <[email protected]>
0 siblings, 0 replies; 30+ messages in thread
From: Melanie Plageman @ 2024-02-13 15:17 UTC (permalink / raw)
To simplify the diff of a future commit which will move the TBMIterators
into the scan descriptor, define them in a narrower scope now.
---
src/backend/executor/nodeBitmapHeapscan.c | 20 +++++++++-----------
1 file changed, 9 insertions(+), 11 deletions(-)
diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c
index 08e346ae7b3..248ff90904b 100644
--- a/src/backend/executor/nodeBitmapHeapscan.c
+++ b/src/backend/executor/nodeBitmapHeapscan.c
@@ -71,8 +71,6 @@ BitmapHeapNext(BitmapHeapScanState *node)
ExprContext *econtext;
TableScanDesc scan;
TIDBitmap *tbm;
- TBMIterator *tbmiterator = NULL;
- TBMSharedIterator *shared_tbmiterator = NULL;
TBMIterateResult *tbmres;
TupleTableSlot *slot;
ParallelBitmapHeapState *pstate = node->pstate;
@@ -85,10 +83,6 @@ BitmapHeapNext(BitmapHeapScanState *node)
slot = node->ss.ss_ScanTupleSlot;
scan = node->ss.ss_currentScanDesc;
tbm = node->tbm;
- if (pstate == NULL)
- tbmiterator = node->tbmiterator;
- else
- shared_tbmiterator = node->shared_tbmiterator;
tbmres = node->tbmres;
/*
@@ -105,6 +99,9 @@ BitmapHeapNext(BitmapHeapScanState *node)
*/
if (!node->initialized)
{
+ TBMIterator *tbmiterator = NULL;
+ TBMSharedIterator *shared_tbmiterator = NULL;
+
if (!pstate)
{
tbm = (TIDBitmap *) MultiExecProcNode(outerPlanState(node));
@@ -113,7 +110,7 @@ BitmapHeapNext(BitmapHeapScanState *node)
elog(ERROR, "unrecognized result from subplan");
node->tbm = tbm;
- node->tbmiterator = tbmiterator = tbm_begin_iterate(tbm);
+ tbmiterator = tbm_begin_iterate(tbm);
node->tbmres = tbmres = NULL;
#ifdef USE_PREFETCH
@@ -166,8 +163,7 @@ BitmapHeapNext(BitmapHeapScanState *node)
}
/* Allocate a private iterator and attach the shared state to it */
- node->shared_tbmiterator = shared_tbmiterator =
- tbm_attach_shared_iterate(dsa, pstate->tbmiterator);
+ shared_tbmiterator = tbm_attach_shared_iterate(dsa, pstate->tbmiterator);
node->tbmres = tbmres = NULL;
#ifdef USE_PREFETCH
@@ -206,6 +202,8 @@ BitmapHeapNext(BitmapHeapScanState *node)
extra_flags);
}
+ node->tbmiterator = tbmiterator;
+ node->shared_tbmiterator = shared_tbmiterator;
node->initialized = true;
}
@@ -221,9 +219,9 @@ BitmapHeapNext(BitmapHeapScanState *node)
if (tbmres == NULL)
{
if (!pstate)
- node->tbmres = tbmres = tbm_iterate(tbmiterator);
+ node->tbmres = tbmres = tbm_iterate(node->tbmiterator);
else
- node->tbmres = tbmres = tbm_shared_iterate(shared_tbmiterator);
+ node->tbmres = tbmres = tbm_shared_iterate(node->shared_tbmiterator);
if (tbmres == NULL)
{
/* no more entries in the bitmap */
--
2.40.1
--w4wcjcocxsm37usi
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v6-0009-Remove-table_scan_bitmap_next_tuple-parameter-tbm.patch"
^ permalink raw reply [nested|flat] 30+ messages in thread
* [PATCH v9 07/17] Reduce scope of BitmapHeapScan tbmiterator local variables
@ 2024-02-13 15:17 Melanie Plageman <[email protected]>
0 siblings, 0 replies; 30+ messages in thread
From: Melanie Plageman @ 2024-02-13 15:17 UTC (permalink / raw)
To simplify the diff of a future commit which will move the TBMIterators
into the scan descriptor, define them in a narrower scope now.
---
src/backend/executor/nodeBitmapHeapscan.c | 20 +++++++++-----------
1 file changed, 9 insertions(+), 11 deletions(-)
diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c
index c95e3412da..49938c9ed4 100644
--- a/src/backend/executor/nodeBitmapHeapscan.c
+++ b/src/backend/executor/nodeBitmapHeapscan.c
@@ -71,8 +71,6 @@ BitmapHeapNext(BitmapHeapScanState *node)
ExprContext *econtext;
TableScanDesc scan;
TIDBitmap *tbm;
- TBMIterator *tbmiterator = NULL;
- TBMSharedIterator *shared_tbmiterator = NULL;
TBMIterateResult *tbmres;
TupleTableSlot *slot;
ParallelBitmapHeapState *pstate = node->pstate;
@@ -85,10 +83,6 @@ BitmapHeapNext(BitmapHeapScanState *node)
slot = node->ss.ss_ScanTupleSlot;
scan = node->ss.ss_currentScanDesc;
tbm = node->tbm;
- if (pstate == NULL)
- tbmiterator = node->tbmiterator;
- else
- shared_tbmiterator = node->shared_tbmiterator;
tbmres = node->tbmres;
/*
@@ -105,6 +99,9 @@ BitmapHeapNext(BitmapHeapScanState *node)
*/
if (!node->initialized)
{
+ TBMIterator *tbmiterator = NULL;
+ TBMSharedIterator *shared_tbmiterator = NULL;
+
if (!pstate)
{
tbm = (TIDBitmap *) MultiExecProcNode(outerPlanState(node));
@@ -113,7 +110,7 @@ BitmapHeapNext(BitmapHeapScanState *node)
elog(ERROR, "unrecognized result from subplan");
node->tbm = tbm;
- node->tbmiterator = tbmiterator = tbm_begin_iterate(tbm);
+ tbmiterator = tbm_begin_iterate(tbm);
node->tbmres = tbmres = NULL;
#ifdef USE_PREFETCH
@@ -166,8 +163,7 @@ BitmapHeapNext(BitmapHeapScanState *node)
}
/* Allocate a private iterator and attach the shared state to it */
- node->shared_tbmiterator = shared_tbmiterator =
- tbm_attach_shared_iterate(dsa, pstate->tbmiterator);
+ shared_tbmiterator = tbm_attach_shared_iterate(dsa, pstate->tbmiterator);
node->tbmres = tbmres = NULL;
#ifdef USE_PREFETCH
@@ -206,6 +202,8 @@ BitmapHeapNext(BitmapHeapScanState *node)
extra_flags);
}
+ node->tbmiterator = tbmiterator;
+ node->shared_tbmiterator = shared_tbmiterator;
node->initialized = true;
}
@@ -221,9 +219,9 @@ BitmapHeapNext(BitmapHeapScanState *node)
if (tbmres == NULL)
{
if (!pstate)
- node->tbmres = tbmres = tbm_iterate(tbmiterator);
+ node->tbmres = tbmres = tbm_iterate(node->tbmiterator);
else
- node->tbmres = tbmres = tbm_shared_iterate(shared_tbmiterator);
+ node->tbmres = tbmres = tbm_shared_iterate(node->shared_tbmiterator);
if (tbmres == NULL)
{
/* no more entries in the bitmap */
--
2.40.1
--7mdtsjmrzitrgzgx
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v9-0008-Remove-table_scan_bitmap_next_tuple-parameter-tbm.patch"
^ permalink raw reply [nested|flat] 30+ messages in thread
* [PATCH v11 07/17] Reduce scope of BitmapHeapScan tbmiterator local variables
@ 2024-02-13 15:17 Melanie Plageman <[email protected]>
0 siblings, 0 replies; 30+ messages in thread
From: Melanie Plageman @ 2024-02-13 15:17 UTC (permalink / raw)
To simplify the diff of a future commit which will move the TBMIterators
into the scan descriptor, define them in a narrower scope now.
---
src/backend/executor/nodeBitmapHeapscan.c | 20 +++++++++-----------
1 file changed, 9 insertions(+), 11 deletions(-)
diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c
index c95e3412da..49938c9ed4 100644
--- a/src/backend/executor/nodeBitmapHeapscan.c
+++ b/src/backend/executor/nodeBitmapHeapscan.c
@@ -71,8 +71,6 @@ BitmapHeapNext(BitmapHeapScanState *node)
ExprContext *econtext;
TableScanDesc scan;
TIDBitmap *tbm;
- TBMIterator *tbmiterator = NULL;
- TBMSharedIterator *shared_tbmiterator = NULL;
TBMIterateResult *tbmres;
TupleTableSlot *slot;
ParallelBitmapHeapState *pstate = node->pstate;
@@ -85,10 +83,6 @@ BitmapHeapNext(BitmapHeapScanState *node)
slot = node->ss.ss_ScanTupleSlot;
scan = node->ss.ss_currentScanDesc;
tbm = node->tbm;
- if (pstate == NULL)
- tbmiterator = node->tbmiterator;
- else
- shared_tbmiterator = node->shared_tbmiterator;
tbmres = node->tbmres;
/*
@@ -105,6 +99,9 @@ BitmapHeapNext(BitmapHeapScanState *node)
*/
if (!node->initialized)
{
+ TBMIterator *tbmiterator = NULL;
+ TBMSharedIterator *shared_tbmiterator = NULL;
+
if (!pstate)
{
tbm = (TIDBitmap *) MultiExecProcNode(outerPlanState(node));
@@ -113,7 +110,7 @@ BitmapHeapNext(BitmapHeapScanState *node)
elog(ERROR, "unrecognized result from subplan");
node->tbm = tbm;
- node->tbmiterator = tbmiterator = tbm_begin_iterate(tbm);
+ tbmiterator = tbm_begin_iterate(tbm);
node->tbmres = tbmres = NULL;
#ifdef USE_PREFETCH
@@ -166,8 +163,7 @@ BitmapHeapNext(BitmapHeapScanState *node)
}
/* Allocate a private iterator and attach the shared state to it */
- node->shared_tbmiterator = shared_tbmiterator =
- tbm_attach_shared_iterate(dsa, pstate->tbmiterator);
+ shared_tbmiterator = tbm_attach_shared_iterate(dsa, pstate->tbmiterator);
node->tbmres = tbmres = NULL;
#ifdef USE_PREFETCH
@@ -206,6 +202,8 @@ BitmapHeapNext(BitmapHeapScanState *node)
extra_flags);
}
+ node->tbmiterator = tbmiterator;
+ node->shared_tbmiterator = shared_tbmiterator;
node->initialized = true;
}
@@ -221,9 +219,9 @@ BitmapHeapNext(BitmapHeapScanState *node)
if (tbmres == NULL)
{
if (!pstate)
- node->tbmres = tbmres = tbm_iterate(tbmiterator);
+ node->tbmres = tbmres = tbm_iterate(node->tbmiterator);
else
- node->tbmres = tbmres = tbm_shared_iterate(shared_tbmiterator);
+ node->tbmres = tbmres = tbm_shared_iterate(node->shared_tbmiterator);
if (tbmres == NULL)
{
/* no more entries in the bitmap */
--
2.40.1
--owzzsiozz6hgpp7e
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v11-0008-Remove-table_scan_bitmap_next_tuple-parameter-tb.patch"
^ permalink raw reply [nested|flat] 30+ messages in thread
* [PATCH v8 07/17] Reduce scope of BitmapHeapScan tbmiterator local variables
@ 2024-02-13 15:17 Melanie Plageman <[email protected]>
0 siblings, 0 replies; 30+ messages in thread
From: Melanie Plageman @ 2024-02-13 15:17 UTC (permalink / raw)
To simplify the diff of a future commit which will move the TBMIterators
into the scan descriptor, define them in a narrower scope now.
---
src/backend/executor/nodeBitmapHeapscan.c | 20 +++++++++-----------
1 file changed, 9 insertions(+), 11 deletions(-)
diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c
index c95e3412da..49938c9ed4 100644
--- a/src/backend/executor/nodeBitmapHeapscan.c
+++ b/src/backend/executor/nodeBitmapHeapscan.c
@@ -71,8 +71,6 @@ BitmapHeapNext(BitmapHeapScanState *node)
ExprContext *econtext;
TableScanDesc scan;
TIDBitmap *tbm;
- TBMIterator *tbmiterator = NULL;
- TBMSharedIterator *shared_tbmiterator = NULL;
TBMIterateResult *tbmres;
TupleTableSlot *slot;
ParallelBitmapHeapState *pstate = node->pstate;
@@ -85,10 +83,6 @@ BitmapHeapNext(BitmapHeapScanState *node)
slot = node->ss.ss_ScanTupleSlot;
scan = node->ss.ss_currentScanDesc;
tbm = node->tbm;
- if (pstate == NULL)
- tbmiterator = node->tbmiterator;
- else
- shared_tbmiterator = node->shared_tbmiterator;
tbmres = node->tbmres;
/*
@@ -105,6 +99,9 @@ BitmapHeapNext(BitmapHeapScanState *node)
*/
if (!node->initialized)
{
+ TBMIterator *tbmiterator = NULL;
+ TBMSharedIterator *shared_tbmiterator = NULL;
+
if (!pstate)
{
tbm = (TIDBitmap *) MultiExecProcNode(outerPlanState(node));
@@ -113,7 +110,7 @@ BitmapHeapNext(BitmapHeapScanState *node)
elog(ERROR, "unrecognized result from subplan");
node->tbm = tbm;
- node->tbmiterator = tbmiterator = tbm_begin_iterate(tbm);
+ tbmiterator = tbm_begin_iterate(tbm);
node->tbmres = tbmres = NULL;
#ifdef USE_PREFETCH
@@ -166,8 +163,7 @@ BitmapHeapNext(BitmapHeapScanState *node)
}
/* Allocate a private iterator and attach the shared state to it */
- node->shared_tbmiterator = shared_tbmiterator =
- tbm_attach_shared_iterate(dsa, pstate->tbmiterator);
+ shared_tbmiterator = tbm_attach_shared_iterate(dsa, pstate->tbmiterator);
node->tbmres = tbmres = NULL;
#ifdef USE_PREFETCH
@@ -206,6 +202,8 @@ BitmapHeapNext(BitmapHeapScanState *node)
extra_flags);
}
+ node->tbmiterator = tbmiterator;
+ node->shared_tbmiterator = shared_tbmiterator;
node->initialized = true;
}
@@ -221,9 +219,9 @@ BitmapHeapNext(BitmapHeapScanState *node)
if (tbmres == NULL)
{
if (!pstate)
- node->tbmres = tbmres = tbm_iterate(tbmiterator);
+ node->tbmres = tbmres = tbm_iterate(node->tbmiterator);
else
- node->tbmres = tbmres = tbm_shared_iterate(shared_tbmiterator);
+ node->tbmres = tbmres = tbm_shared_iterate(node->shared_tbmiterator);
if (tbmres == NULL)
{
/* no more entries in the bitmap */
--
2.40.1
--xqq4defy3uncu6k6
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v8-0008-Remove-table_scan_bitmap_next_tuple-parameter-tbm.patch"
^ permalink raw reply [nested|flat] 30+ messages in thread
* [PATCH v8 07/17] Reduce scope of BitmapHeapScan tbmiterator local variables
@ 2024-02-13 15:17 Melanie Plageman <[email protected]>
0 siblings, 0 replies; 30+ messages in thread
From: Melanie Plageman @ 2024-02-13 15:17 UTC (permalink / raw)
To simplify the diff of a future commit which will move the TBMIterators
into the scan descriptor, define them in a narrower scope now.
---
src/backend/executor/nodeBitmapHeapscan.c | 20 +++++++++-----------
1 file changed, 9 insertions(+), 11 deletions(-)
diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c
index c95e3412da..49938c9ed4 100644
--- a/src/backend/executor/nodeBitmapHeapscan.c
+++ b/src/backend/executor/nodeBitmapHeapscan.c
@@ -71,8 +71,6 @@ BitmapHeapNext(BitmapHeapScanState *node)
ExprContext *econtext;
TableScanDesc scan;
TIDBitmap *tbm;
- TBMIterator *tbmiterator = NULL;
- TBMSharedIterator *shared_tbmiterator = NULL;
TBMIterateResult *tbmres;
TupleTableSlot *slot;
ParallelBitmapHeapState *pstate = node->pstate;
@@ -85,10 +83,6 @@ BitmapHeapNext(BitmapHeapScanState *node)
slot = node->ss.ss_ScanTupleSlot;
scan = node->ss.ss_currentScanDesc;
tbm = node->tbm;
- if (pstate == NULL)
- tbmiterator = node->tbmiterator;
- else
- shared_tbmiterator = node->shared_tbmiterator;
tbmres = node->tbmres;
/*
@@ -105,6 +99,9 @@ BitmapHeapNext(BitmapHeapScanState *node)
*/
if (!node->initialized)
{
+ TBMIterator *tbmiterator = NULL;
+ TBMSharedIterator *shared_tbmiterator = NULL;
+
if (!pstate)
{
tbm = (TIDBitmap *) MultiExecProcNode(outerPlanState(node));
@@ -113,7 +110,7 @@ BitmapHeapNext(BitmapHeapScanState *node)
elog(ERROR, "unrecognized result from subplan");
node->tbm = tbm;
- node->tbmiterator = tbmiterator = tbm_begin_iterate(tbm);
+ tbmiterator = tbm_begin_iterate(tbm);
node->tbmres = tbmres = NULL;
#ifdef USE_PREFETCH
@@ -166,8 +163,7 @@ BitmapHeapNext(BitmapHeapScanState *node)
}
/* Allocate a private iterator and attach the shared state to it */
- node->shared_tbmiterator = shared_tbmiterator =
- tbm_attach_shared_iterate(dsa, pstate->tbmiterator);
+ shared_tbmiterator = tbm_attach_shared_iterate(dsa, pstate->tbmiterator);
node->tbmres = tbmres = NULL;
#ifdef USE_PREFETCH
@@ -206,6 +202,8 @@ BitmapHeapNext(BitmapHeapScanState *node)
extra_flags);
}
+ node->tbmiterator = tbmiterator;
+ node->shared_tbmiterator = shared_tbmiterator;
node->initialized = true;
}
@@ -221,9 +219,9 @@ BitmapHeapNext(BitmapHeapScanState *node)
if (tbmres == NULL)
{
if (!pstate)
- node->tbmres = tbmres = tbm_iterate(tbmiterator);
+ node->tbmres = tbmres = tbm_iterate(node->tbmiterator);
else
- node->tbmres = tbmres = tbm_shared_iterate(shared_tbmiterator);
+ node->tbmres = tbmres = tbm_shared_iterate(node->shared_tbmiterator);
if (tbmres == NULL)
{
/* no more entries in the bitmap */
--
2.40.1
--xqq4defy3uncu6k6
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v8-0008-Remove-table_scan_bitmap_next_tuple-parameter-tbm.patch"
^ permalink raw reply [nested|flat] 30+ messages in thread
* [PATCH v10 07/17] Reduce scope of BitmapHeapScan tbmiterator local variables
@ 2024-02-13 15:17 Melanie Plageman <[email protected]>
0 siblings, 0 replies; 30+ messages in thread
From: Melanie Plageman @ 2024-02-13 15:17 UTC (permalink / raw)
To simplify the diff of a future commit which will move the TBMIterators
into the scan descriptor, define them in a narrower scope now.
---
src/backend/executor/nodeBitmapHeapscan.c | 20 +++++++++-----------
1 file changed, 9 insertions(+), 11 deletions(-)
diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c
index c95e3412da..49938c9ed4 100644
--- a/src/backend/executor/nodeBitmapHeapscan.c
+++ b/src/backend/executor/nodeBitmapHeapscan.c
@@ -71,8 +71,6 @@ BitmapHeapNext(BitmapHeapScanState *node)
ExprContext *econtext;
TableScanDesc scan;
TIDBitmap *tbm;
- TBMIterator *tbmiterator = NULL;
- TBMSharedIterator *shared_tbmiterator = NULL;
TBMIterateResult *tbmres;
TupleTableSlot *slot;
ParallelBitmapHeapState *pstate = node->pstate;
@@ -85,10 +83,6 @@ BitmapHeapNext(BitmapHeapScanState *node)
slot = node->ss.ss_ScanTupleSlot;
scan = node->ss.ss_currentScanDesc;
tbm = node->tbm;
- if (pstate == NULL)
- tbmiterator = node->tbmiterator;
- else
- shared_tbmiterator = node->shared_tbmiterator;
tbmres = node->tbmres;
/*
@@ -105,6 +99,9 @@ BitmapHeapNext(BitmapHeapScanState *node)
*/
if (!node->initialized)
{
+ TBMIterator *tbmiterator = NULL;
+ TBMSharedIterator *shared_tbmiterator = NULL;
+
if (!pstate)
{
tbm = (TIDBitmap *) MultiExecProcNode(outerPlanState(node));
@@ -113,7 +110,7 @@ BitmapHeapNext(BitmapHeapScanState *node)
elog(ERROR, "unrecognized result from subplan");
node->tbm = tbm;
- node->tbmiterator = tbmiterator = tbm_begin_iterate(tbm);
+ tbmiterator = tbm_begin_iterate(tbm);
node->tbmres = tbmres = NULL;
#ifdef USE_PREFETCH
@@ -166,8 +163,7 @@ BitmapHeapNext(BitmapHeapScanState *node)
}
/* Allocate a private iterator and attach the shared state to it */
- node->shared_tbmiterator = shared_tbmiterator =
- tbm_attach_shared_iterate(dsa, pstate->tbmiterator);
+ shared_tbmiterator = tbm_attach_shared_iterate(dsa, pstate->tbmiterator);
node->tbmres = tbmres = NULL;
#ifdef USE_PREFETCH
@@ -206,6 +202,8 @@ BitmapHeapNext(BitmapHeapScanState *node)
extra_flags);
}
+ node->tbmiterator = tbmiterator;
+ node->shared_tbmiterator = shared_tbmiterator;
node->initialized = true;
}
@@ -221,9 +219,9 @@ BitmapHeapNext(BitmapHeapScanState *node)
if (tbmres == NULL)
{
if (!pstate)
- node->tbmres = tbmres = tbm_iterate(tbmiterator);
+ node->tbmres = tbmres = tbm_iterate(node->tbmiterator);
else
- node->tbmres = tbmres = tbm_shared_iterate(shared_tbmiterator);
+ node->tbmres = tbmres = tbm_shared_iterate(node->shared_tbmiterator);
if (tbmres == NULL)
{
/* no more entries in the bitmap */
--
2.40.1
--3o7pc6dfau5a5hry
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v10-0008-Remove-table_scan_bitmap_next_tuple-parameter-tb.patch"
^ permalink raw reply [nested|flat] 30+ messages in thread
* [PATCH v7 07/13] Reduce scope of BitmapHeapScan tbmiterator local variables
@ 2024-02-13 15:17 Melanie Plageman <[email protected]>
0 siblings, 0 replies; 30+ messages in thread
From: Melanie Plageman @ 2024-02-13 15:17 UTC (permalink / raw)
To simplify the diff of a future commit which will move the TBMIterators
into the scan descriptor, define them in a narrower scope now.
---
src/backend/executor/nodeBitmapHeapscan.c | 20 +++++++++-----------
1 file changed, 9 insertions(+), 11 deletions(-)
diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c
index b2397fe2054..94a5e2da17c 100644
--- a/src/backend/executor/nodeBitmapHeapscan.c
+++ b/src/backend/executor/nodeBitmapHeapscan.c
@@ -71,8 +71,6 @@ BitmapHeapNext(BitmapHeapScanState *node)
ExprContext *econtext;
TableScanDesc scan;
TIDBitmap *tbm;
- TBMIterator *tbmiterator = NULL;
- TBMSharedIterator *shared_tbmiterator = NULL;
TBMIterateResult *tbmres;
TupleTableSlot *slot;
ParallelBitmapHeapState *pstate = node->pstate;
@@ -85,10 +83,6 @@ BitmapHeapNext(BitmapHeapScanState *node)
slot = node->ss.ss_ScanTupleSlot;
scan = node->ss.ss_currentScanDesc;
tbm = node->tbm;
- if (pstate == NULL)
- tbmiterator = node->tbmiterator;
- else
- shared_tbmiterator = node->shared_tbmiterator;
tbmres = node->tbmres;
/*
@@ -105,6 +99,9 @@ BitmapHeapNext(BitmapHeapScanState *node)
*/
if (!node->initialized)
{
+ TBMIterator *tbmiterator = NULL;
+ TBMSharedIterator *shared_tbmiterator = NULL;
+
if (!pstate)
{
tbm = (TIDBitmap *) MultiExecProcNode(outerPlanState(node));
@@ -113,7 +110,7 @@ BitmapHeapNext(BitmapHeapScanState *node)
elog(ERROR, "unrecognized result from subplan");
node->tbm = tbm;
- node->tbmiterator = tbmiterator = tbm_begin_iterate(tbm);
+ tbmiterator = tbm_begin_iterate(tbm);
node->tbmres = tbmres = NULL;
#ifdef USE_PREFETCH
@@ -166,8 +163,7 @@ BitmapHeapNext(BitmapHeapScanState *node)
}
/* Allocate a private iterator and attach the shared state to it */
- node->shared_tbmiterator = shared_tbmiterator =
- tbm_attach_shared_iterate(dsa, pstate->tbmiterator);
+ shared_tbmiterator = tbm_attach_shared_iterate(dsa, pstate->tbmiterator);
node->tbmres = tbmres = NULL;
#ifdef USE_PREFETCH
@@ -206,6 +202,8 @@ BitmapHeapNext(BitmapHeapScanState *node)
extra_flags);
}
+ node->tbmiterator = tbmiterator;
+ node->shared_tbmiterator = shared_tbmiterator;
node->initialized = true;
}
@@ -221,9 +219,9 @@ BitmapHeapNext(BitmapHeapScanState *node)
if (tbmres == NULL)
{
if (!pstate)
- node->tbmres = tbmres = tbm_iterate(tbmiterator);
+ node->tbmres = tbmres = tbm_iterate(node->tbmiterator);
else
- node->tbmres = tbmres = tbm_shared_iterate(shared_tbmiterator);
+ node->tbmres = tbmres = tbm_shared_iterate(node->shared_tbmiterator);
if (tbmres == NULL)
{
/* no more entries in the bitmap */
--
2.40.1
--kqqpqghcwbcc3dt5
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v7-0008-Remove-table_scan_bitmap_next_tuple-parameter-tbm.patch"
^ permalink raw reply [nested|flat] 30+ messages in thread
* [PATCH v9 07/17] Reduce scope of BitmapHeapScan tbmiterator local variables
@ 2024-02-13 15:17 Melanie Plageman <[email protected]>
0 siblings, 0 replies; 30+ messages in thread
From: Melanie Plageman @ 2024-02-13 15:17 UTC (permalink / raw)
To simplify the diff of a future commit which will move the TBMIterators
into the scan descriptor, define them in a narrower scope now.
---
src/backend/executor/nodeBitmapHeapscan.c | 20 +++++++++-----------
1 file changed, 9 insertions(+), 11 deletions(-)
diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c
index c95e3412da..49938c9ed4 100644
--- a/src/backend/executor/nodeBitmapHeapscan.c
+++ b/src/backend/executor/nodeBitmapHeapscan.c
@@ -71,8 +71,6 @@ BitmapHeapNext(BitmapHeapScanState *node)
ExprContext *econtext;
TableScanDesc scan;
TIDBitmap *tbm;
- TBMIterator *tbmiterator = NULL;
- TBMSharedIterator *shared_tbmiterator = NULL;
TBMIterateResult *tbmres;
TupleTableSlot *slot;
ParallelBitmapHeapState *pstate = node->pstate;
@@ -85,10 +83,6 @@ BitmapHeapNext(BitmapHeapScanState *node)
slot = node->ss.ss_ScanTupleSlot;
scan = node->ss.ss_currentScanDesc;
tbm = node->tbm;
- if (pstate == NULL)
- tbmiterator = node->tbmiterator;
- else
- shared_tbmiterator = node->shared_tbmiterator;
tbmres = node->tbmres;
/*
@@ -105,6 +99,9 @@ BitmapHeapNext(BitmapHeapScanState *node)
*/
if (!node->initialized)
{
+ TBMIterator *tbmiterator = NULL;
+ TBMSharedIterator *shared_tbmiterator = NULL;
+
if (!pstate)
{
tbm = (TIDBitmap *) MultiExecProcNode(outerPlanState(node));
@@ -113,7 +110,7 @@ BitmapHeapNext(BitmapHeapScanState *node)
elog(ERROR, "unrecognized result from subplan");
node->tbm = tbm;
- node->tbmiterator = tbmiterator = tbm_begin_iterate(tbm);
+ tbmiterator = tbm_begin_iterate(tbm);
node->tbmres = tbmres = NULL;
#ifdef USE_PREFETCH
@@ -166,8 +163,7 @@ BitmapHeapNext(BitmapHeapScanState *node)
}
/* Allocate a private iterator and attach the shared state to it */
- node->shared_tbmiterator = shared_tbmiterator =
- tbm_attach_shared_iterate(dsa, pstate->tbmiterator);
+ shared_tbmiterator = tbm_attach_shared_iterate(dsa, pstate->tbmiterator);
node->tbmres = tbmres = NULL;
#ifdef USE_PREFETCH
@@ -206,6 +202,8 @@ BitmapHeapNext(BitmapHeapScanState *node)
extra_flags);
}
+ node->tbmiterator = tbmiterator;
+ node->shared_tbmiterator = shared_tbmiterator;
node->initialized = true;
}
@@ -221,9 +219,9 @@ BitmapHeapNext(BitmapHeapScanState *node)
if (tbmres == NULL)
{
if (!pstate)
- node->tbmres = tbmres = tbm_iterate(tbmiterator);
+ node->tbmres = tbmres = tbm_iterate(node->tbmiterator);
else
- node->tbmres = tbmres = tbm_shared_iterate(shared_tbmiterator);
+ node->tbmres = tbmres = tbm_shared_iterate(node->shared_tbmiterator);
if (tbmres == NULL)
{
/* no more entries in the bitmap */
--
2.40.1
--7mdtsjmrzitrgzgx
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v9-0008-Remove-table_scan_bitmap_next_tuple-parameter-tbm.patch"
^ permalink raw reply [nested|flat] 30+ messages in thread
* [PATCH v6 08/14] Reduce scope of BitmapHeapScan tbmiterator local variables
@ 2024-02-13 15:17 Melanie Plageman <[email protected]>
0 siblings, 0 replies; 30+ messages in thread
From: Melanie Plageman @ 2024-02-13 15:17 UTC (permalink / raw)
To simplify the diff of a future commit which will move the TBMIterators
into the scan descriptor, define them in a narrower scope now.
---
src/backend/executor/nodeBitmapHeapscan.c | 20 +++++++++-----------
1 file changed, 9 insertions(+), 11 deletions(-)
diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c
index 08e346ae7b3..248ff90904b 100644
--- a/src/backend/executor/nodeBitmapHeapscan.c
+++ b/src/backend/executor/nodeBitmapHeapscan.c
@@ -71,8 +71,6 @@ BitmapHeapNext(BitmapHeapScanState *node)
ExprContext *econtext;
TableScanDesc scan;
TIDBitmap *tbm;
- TBMIterator *tbmiterator = NULL;
- TBMSharedIterator *shared_tbmiterator = NULL;
TBMIterateResult *tbmres;
TupleTableSlot *slot;
ParallelBitmapHeapState *pstate = node->pstate;
@@ -85,10 +83,6 @@ BitmapHeapNext(BitmapHeapScanState *node)
slot = node->ss.ss_ScanTupleSlot;
scan = node->ss.ss_currentScanDesc;
tbm = node->tbm;
- if (pstate == NULL)
- tbmiterator = node->tbmiterator;
- else
- shared_tbmiterator = node->shared_tbmiterator;
tbmres = node->tbmres;
/*
@@ -105,6 +99,9 @@ BitmapHeapNext(BitmapHeapScanState *node)
*/
if (!node->initialized)
{
+ TBMIterator *tbmiterator = NULL;
+ TBMSharedIterator *shared_tbmiterator = NULL;
+
if (!pstate)
{
tbm = (TIDBitmap *) MultiExecProcNode(outerPlanState(node));
@@ -113,7 +110,7 @@ BitmapHeapNext(BitmapHeapScanState *node)
elog(ERROR, "unrecognized result from subplan");
node->tbm = tbm;
- node->tbmiterator = tbmiterator = tbm_begin_iterate(tbm);
+ tbmiterator = tbm_begin_iterate(tbm);
node->tbmres = tbmres = NULL;
#ifdef USE_PREFETCH
@@ -166,8 +163,7 @@ BitmapHeapNext(BitmapHeapScanState *node)
}
/* Allocate a private iterator and attach the shared state to it */
- node->shared_tbmiterator = shared_tbmiterator =
- tbm_attach_shared_iterate(dsa, pstate->tbmiterator);
+ shared_tbmiterator = tbm_attach_shared_iterate(dsa, pstate->tbmiterator);
node->tbmres = tbmres = NULL;
#ifdef USE_PREFETCH
@@ -206,6 +202,8 @@ BitmapHeapNext(BitmapHeapScanState *node)
extra_flags);
}
+ node->tbmiterator = tbmiterator;
+ node->shared_tbmiterator = shared_tbmiterator;
node->initialized = true;
}
@@ -221,9 +219,9 @@ BitmapHeapNext(BitmapHeapScanState *node)
if (tbmres == NULL)
{
if (!pstate)
- node->tbmres = tbmres = tbm_iterate(tbmiterator);
+ node->tbmres = tbmres = tbm_iterate(node->tbmiterator);
else
- node->tbmres = tbmres = tbm_shared_iterate(shared_tbmiterator);
+ node->tbmres = tbmres = tbm_shared_iterate(node->shared_tbmiterator);
if (tbmres == NULL)
{
/* no more entries in the bitmap */
--
2.40.1
--w4wcjcocxsm37usi
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v6-0009-Remove-table_scan_bitmap_next_tuple-parameter-tbm.patch"
^ permalink raw reply [nested|flat] 30+ messages in thread
* Re: Parametrization minimum password lenght
@ 2024-11-25 10:15 Emanuele Musella <[email protected]>
2024-11-25 17:35 ` Re: Parametrization minimum password lenght Emanuele Musella <[email protected]>
0 siblings, 1 reply; 30+ messages in thread
From: Emanuele Musella @ 2024-11-25 10:15 UTC (permalink / raw)
To: [email protected]; +Cc: Bertrand Drouvot <[email protected]>
Thank you Bertrand for your feedbacks. We are looking for CFbot part so we
can compile it like CFbot.
For now we have fixed all points.
Thank you
Il giorno mar 19 nov 2024 alle ore 20:28 Bertrand Drouvot <
[email protected]> ha scritto:
> Hi,
>
> On Mon, Nov 18, 2024 at 05:21:18PM +0100, Emanuele Musella wrote:
> > We notice some errors on CFBot results.
>
> FWIW, you can run "cfbot like" tests on your own repo (see [1]).
>
> > In attached the errors fixed
>
> Thanks for the updated version!
>
> A few random comments:
>
> === 1
>
> trailing whitespace:
>
> $ git apply min_password_length_v7.patch
> min_password_length_v7.patch:130: trailing whitespace.
> There is a configuration parameter that control the behavior
> warning: 1 line adds whitespace errors.
>
> === 2
>
> + * Author: Maurizio Boriani <[email protected]>
> + * Author: Emanuele Musella <[email protected]>
>
> Same comment as in [2].
>
> === 3
>
> - int pwdlen = strlen(password);
> + int pwdlen = pg_mbstrlen(password);
>
> Sorry if I was not clear in [2], but I meant to say to keep using strlen()
> to be
> consistent with the current behavior.
>
> === 4
>
> + GUC_UNIT_BYTE,
>
> this is correct if strlen() is used (see above comment).
>
> === 5
>
> + 0, INT_MAX,
>
> INT_MAX seems too large and 0 too low. Maybe we should not allow less than
> it
> was before the patch (8). For the max, maybe something like
> PG_MAX_AUTH_TOKEN_LENGTH?
> (see the comment in src/backend/libpq/auth.c)
>
> === 6
>
> + There is a configuration parameter that control the behavior
> + <filename>passwordcheck</filename>
>
> s/behavior/behavior of/?
>
> === 7
>
> + <varname>passwordcheck.min_password_length</varname> is the minimum
> length
> + of accepted password on database users.
> + If not setted the default is 8 bytes.
>
> What about? "is the minimum password length in bytes. The default is 8."
>
> === 7
>
> +
> +<programlisting>
> +# postgresql.conf
> +session_preload_libraries = 'passwordcheck'
> +passwordcheck.min_password_length = 12
> +
> +</programlisting>
>
> What about a sentence before? Something like for auto_explain means "In
> ordinary
> usage, these parameters are set in postgresql.conf,............"
>
> [1]: https://github.com/postgres/postgres/blob/master/src/tools/ci/README
> [2]:
> https://www.postgresql.org/message-id/ZzsZZY3YrO6hinnT%40ip-10-97-1-34.eu-west-3.compute.internal
>
> Regards,
>
> --
> Bertrand Drouvot
> PostgreSQL Contributors Team
> RDS Open Source Databases
> Amazon Web Services: https://aws.amazon.com
>
Attachments:
[application/octet-stream] min_password_length_v8.patch (4.4K, ../../CA+ugDNzwu_8_ZMfj4oGOfqMEJJii1u3tLg2a2o5+PVnk8zYcQQ@mail.gmail.com/3-min_password_length_v8.patch)
download | inline diff:
diff --git a/contrib/passwordcheck/expected/passwordcheck.out b/contrib/passwordcheck/expected/passwordcheck.out
index 2027681daf..8f1f0baaab 100644
--- a/contrib/passwordcheck/expected/passwordcheck.out
+++ b/contrib/passwordcheck/expected/passwordcheck.out
@@ -1,9 +1,10 @@
LOAD 'passwordcheck';
+SET passwordcheck.min_password_length = 12;
CREATE USER regress_passwordcheck_user1;
-- ok
ALTER USER regress_passwordcheck_user1 PASSWORD 'a_nice_long_password';
-- error: too short
-ALTER USER regress_passwordcheck_user1 PASSWORD 'tooshrt';
+ALTER USER regress_passwordcheck_user1 PASSWORD 'tooshort';
ERROR: password is too short
-- error: contains user name
ALTER USER regress_passwordcheck_user1 PASSWORD 'xyzregress_passwordcheck_user1';
diff --git a/contrib/passwordcheck/passwordcheck.c b/contrib/passwordcheck/passwordcheck.c
index 0785618f2a..f9a2c2db33 100644
--- a/contrib/passwordcheck/passwordcheck.c
+++ b/contrib/passwordcheck/passwordcheck.c
@@ -20,17 +20,23 @@
#include <crack.h>
#endif
+#include "commands/explain.h"
#include "commands/user.h"
#include "fmgr.h"
#include "libpq/crypt.h"
+#include "mb/pg_wchar.h"
+#include "utils/guc.h"
PG_MODULE_MAGIC;
/* Saved hook value in case of unload */
static check_password_hook_type prev_check_password_hook = NULL;
-/* passwords shorter than this will be rejected */
-#define MIN_PWD_LENGTH 8
+/* GUC variables */
+static int min_pwd_len;
+
+/* Max password length */
+#define PG_MAX_PASSWORD_LENGTH 128
/*
* check_password
@@ -93,7 +99,7 @@ check_password(const char *username,
#endif
/* enforce minimum length */
- if (pwdlen < MIN_PWD_LENGTH)
+ if (pwdlen < min_pwd_len)
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
errmsg("password is too short")));
@@ -142,6 +148,21 @@ check_password(const char *username,
void
_PG_init(void)
{
+ /* Define custom GUC variables. */
+ DefineCustomIntVariable("passwordcheck.min_password_length",
+ "Sets the minimum allowed password length.",
+ NULL,
+ &min_pwd_len,
+ 8,
+ 8, PG_MAX_PASSWORD_LENGTH,
+ PGC_SUSET,
+ GUC_UNIT_BYTE,
+ NULL,
+ NULL,
+ NULL);
+
+ MarkGUCPrefixReserved("passwordcheck");
+
/* activate password checks when the module is loaded */
prev_check_password_hook = check_password_hook;
check_password_hook = check_password;
diff --git a/contrib/passwordcheck/sql/passwordcheck.sql b/contrib/passwordcheck/sql/passwordcheck.sql
deleted file mode 100644
index 1fbd6b0e96..0000000000
--- a/contrib/passwordcheck/sql/passwordcheck.sql
+++ /dev/null
@@ -1,23 +0,0 @@
-LOAD 'passwordcheck';
-
-CREATE USER regress_passwordcheck_user1;
-
--- ok
-ALTER USER regress_passwordcheck_user1 PASSWORD 'a_nice_long_password';
-
--- error: too short
-ALTER USER regress_passwordcheck_user1 PASSWORD 'tooshrt';
-
--- error: contains user name
-ALTER USER regress_passwordcheck_user1 PASSWORD 'xyzregress_passwordcheck_user1';
-
--- error: contains only letters
-ALTER USER regress_passwordcheck_user1 PASSWORD 'alessnicelongpassword';
-
--- encrypted ok (password is "secret")
-ALTER USER regress_passwordcheck_user1 PASSWORD 'md592350e12ac34e52dd598f90893bb3ae7';
-
--- error: password is user name
-ALTER USER regress_passwordcheck_user1 PASSWORD 'md507a112732ed9f2087fa90b192d44e358';
-
-DROP USER regress_passwordcheck_user1;
diff --git a/doc/src/sgml/passwordcheck.sgml b/doc/src/sgml/passwordcheck.sgml
index 601f489227..160553b8e9 100644
--- a/doc/src/sgml/passwordcheck.sgml
+++ b/doc/src/sgml/passwordcheck.sgml
@@ -59,4 +59,30 @@
</para>
</caution>
+ <sect2 id="passwordcheck-configuration-parameters">
+ <title>Configuration Parameters</title>
+
+ <para>
+ There is a configuration parameter that control the behavior of
+ <filename>passwordcheck</filename>. This is the minumum password length.
+ </para>
+
+ <variablelist>
+ <varlistentry id="passwordcheck-configuration-parameters-min-password-length">
+ <term>
+ <varname>passwordcheck.min_password_length</varname> (<type>integer</type>)
+ <indexterm>
+ <primary><varname>passwordcheck.min_password_length</varname> configuration parameter</primary>
+ </indexterm>
+ </term>
+ <listitem>
+ <para>
+ <varname>passwordcheck.min_password_length</varname> is the minimum length
+ of accepted password on database users.
+ If not setted the default is 8.
+ </para>
+ </listitem>
+ </varlistentry>
+ </variablelist>
+ </sect2>
</sect1>
^ permalink raw reply [nested|flat] 30+ messages in thread
* Re: Parametrization minimum password lenght
2024-11-25 10:15 Re: Parametrization minimum password lenght Emanuele Musella <[email protected]>
@ 2024-11-25 17:35 ` Emanuele Musella <[email protected]>
2024-12-18 20:56 ` Re: Parametrization minimum password lenght Nathan Bossart <[email protected]>
0 siblings, 1 reply; 30+ messages in thread
From: Emanuele Musella @ 2024-11-25 17:35 UTC (permalink / raw)
To: [email protected]; +Cc: Bertrand Drouvot <[email protected]>
Sorry here the right attachment
Il giorno lun 25 nov 2024 alle ore 11:15 Emanuele Musella <
[email protected]> ha scritto:
> Thank you Bertrand for your feedbacks. We are looking for CFbot part so we
> can compile it like CFbot.
>
> For now we have fixed all points.
>
> Thank you
>
> Il giorno mar 19 nov 2024 alle ore 20:28 Bertrand Drouvot <
> [email protected]> ha scritto:
>
>> Hi,
>>
>> On Mon, Nov 18, 2024 at 05:21:18PM +0100, Emanuele Musella wrote:
>> > We notice some errors on CFBot results.
>>
>> FWIW, you can run "cfbot like" tests on your own repo (see [1]).
>>
>> > In attached the errors fixed
>>
>> Thanks for the updated version!
>>
>> A few random comments:
>>
>> === 1
>>
>> trailing whitespace:
>>
>> $ git apply min_password_length_v7.patch
>> min_password_length_v7.patch:130: trailing whitespace.
>> There is a configuration parameter that control the behavior
>> warning: 1 line adds whitespace errors.
>>
>> === 2
>>
>> + * Author: Maurizio Boriani <[email protected]>
>> + * Author: Emanuele Musella <[email protected]>
>>
>> Same comment as in [2].
>>
>> === 3
>>
>> - int pwdlen = strlen(password);
>> + int pwdlen = pg_mbstrlen(password);
>>
>> Sorry if I was not clear in [2], but I meant to say to keep using
>> strlen() to be
>> consistent with the current behavior.
>>
>> === 4
>>
>> + GUC_UNIT_BYTE,
>>
>> this is correct if strlen() is used (see above comment).
>>
>> === 5
>>
>> + 0, INT_MAX,
>>
>> INT_MAX seems too large and 0 too low. Maybe we should not allow less
>> than it
>> was before the patch (8). For the max, maybe something like
>> PG_MAX_AUTH_TOKEN_LENGTH?
>> (see the comment in src/backend/libpq/auth.c)
>>
>> === 6
>>
>> + There is a configuration parameter that control the behavior
>> + <filename>passwordcheck</filename>
>>
>> s/behavior/behavior of/?
>>
>> === 7
>>
>> + <varname>passwordcheck.min_password_length</varname> is the
>> minimum length
>> + of accepted password on database users.
>> + If not setted the default is 8 bytes.
>>
>> What about? "is the minimum password length in bytes. The default is 8."
>>
>> === 7
>>
>> +
>> +<programlisting>
>> +# postgresql.conf
>> +session_preload_libraries = 'passwordcheck'
>> +passwordcheck.min_password_length = 12
>> +
>> +</programlisting>
>>
>> What about a sentence before? Something like for auto_explain means "In
>> ordinary
>> usage, these parameters are set in postgresql.conf,............"
>>
>> [1]: https://github.com/postgres/postgres/blob/master/src/tools/ci/README
>> [2]:
>> https://www.postgresql.org/message-id/ZzsZZY3YrO6hinnT%40ip-10-97-1-34.eu-west-3.compute.internal
>>
>> Regards,
>>
>> --
>> Bertrand Drouvot
>> PostgreSQL Contributors Team
>> RDS Open Source Databases
>> Amazon Web Services: https://aws.amazon.com
>>
>
Attachments:
[application/octet-stream] min_password_length_v9.patch (4.1K, ../../CA+ugDNzgsF49E2qZ_aPUyV3EepS9RcD4Lp91pnbR4Z8+=FTFRQ@mail.gmail.com/3-min_password_length_v9.patch)
download | inline diff:
diff --git a/contrib/passwordcheck/expected/passwordcheck.out b/contrib/passwordcheck/expected/passwordcheck.out
index 2027681daf..8f1f0baaab 100644
--- a/contrib/passwordcheck/expected/passwordcheck.out
+++ b/contrib/passwordcheck/expected/passwordcheck.out
@@ -1,9 +1,10 @@
LOAD 'passwordcheck';
+SET passwordcheck.min_password_length = 12;
CREATE USER regress_passwordcheck_user1;
-- ok
ALTER USER regress_passwordcheck_user1 PASSWORD 'a_nice_long_password';
-- error: too short
-ALTER USER regress_passwordcheck_user1 PASSWORD 'tooshrt';
+ALTER USER regress_passwordcheck_user1 PASSWORD 'tooshort';
ERROR: password is too short
-- error: contains user name
ALTER USER regress_passwordcheck_user1 PASSWORD 'xyzregress_passwordcheck_user1';
diff --git a/contrib/passwordcheck/passwordcheck.c b/contrib/passwordcheck/passwordcheck.c
index 0785618f2a..f9a2c2db33 100644
--- a/contrib/passwordcheck/passwordcheck.c
+++ b/contrib/passwordcheck/passwordcheck.c
@@ -20,17 +20,23 @@
#include <crack.h>
#endif
+#include "commands/explain.h"
#include "commands/user.h"
#include "fmgr.h"
#include "libpq/crypt.h"
+#include "mb/pg_wchar.h"
+#include "utils/guc.h"
PG_MODULE_MAGIC;
/* Saved hook value in case of unload */
static check_password_hook_type prev_check_password_hook = NULL;
-/* passwords shorter than this will be rejected */
-#define MIN_PWD_LENGTH 8
+/* GUC variables */
+static int min_pwd_len;
+
+/* Max password length */
+#define PG_MAX_PASSWORD_LENGTH 128
/*
* check_password
@@ -93,7 +99,7 @@ check_password(const char *username,
#endif
/* enforce minimum length */
- if (pwdlen < MIN_PWD_LENGTH)
+ if (pwdlen < min_pwd_len)
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
errmsg("password is too short")));
@@ -142,6 +148,21 @@ check_password(const char *username,
void
_PG_init(void)
{
+ /* Define custom GUC variables. */
+ DefineCustomIntVariable("passwordcheck.min_password_length",
+ "Sets the minimum allowed password length.",
+ NULL,
+ &min_pwd_len,
+ 8,
+ 8, PG_MAX_PASSWORD_LENGTH,
+ PGC_SUSET,
+ GUC_UNIT_BYTE,
+ NULL,
+ NULL,
+ NULL);
+
+ MarkGUCPrefixReserved("passwordcheck");
+
/* activate password checks when the module is loaded */
prev_check_password_hook = check_password_hook;
check_password_hook = check_password;
diff --git a/contrib/passwordcheck/sql/passwordcheck.sql b/contrib/passwordcheck/sql/passwordcheck.sql
index 1fbd6b0e96..c151685c74 100644
--- a/contrib/passwordcheck/sql/passwordcheck.sql
+++ b/contrib/passwordcheck/sql/passwordcheck.sql
@@ -1,12 +1,12 @@
LOAD 'passwordcheck';
-
+SET passwordcheck.min_password_length = 12;
CREATE USER regress_passwordcheck_user1;
-- ok
ALTER USER regress_passwordcheck_user1 PASSWORD 'a_nice_long_password';
-- error: too short
-ALTER USER regress_passwordcheck_user1 PASSWORD 'tooshrt';
+ALTER USER regress_passwordcheck_user1 PASSWORD 'tooshort';
-- error: contains user name
ALTER USER regress_passwordcheck_user1 PASSWORD 'xyzregress_passwordcheck_user1';
diff --git a/doc/src/sgml/passwordcheck.sgml b/doc/src/sgml/passwordcheck.sgml
index 601f489227..160553b8e9 100644
--- a/doc/src/sgml/passwordcheck.sgml
+++ b/doc/src/sgml/passwordcheck.sgml
@@ -59,4 +59,30 @@
</para>
</caution>
+ <sect2 id="passwordcheck-configuration-parameters">
+ <title>Configuration Parameters</title>
+
+ <para>
+ There is a configuration parameter that control the behavior of
+ <filename>passwordcheck</filename>. This is the minumum password length.
+ </para>
+
+ <variablelist>
+ <varlistentry id="passwordcheck-configuration-parameters-min-password-length">
+ <term>
+ <varname>passwordcheck.min_password_length</varname> (<type>integer</type>)
+ <indexterm>
+ <primary><varname>passwordcheck.min_password_length</varname> configuration parameter</primary>
+ </indexterm>
+ </term>
+ <listitem>
+ <para>
+ <varname>passwordcheck.min_password_length</varname> is the minimum length
+ of accepted password on database users.
+ If not setted the default is 8.
+ </para>
+ </listitem>
+ </varlistentry>
+ </variablelist>
+ </sect2>
</sect1>
^ permalink raw reply [nested|flat] 30+ messages in thread
* Re: Parametrization minimum password lenght
2024-11-25 10:15 Re: Parametrization minimum password lenght Emanuele Musella <[email protected]>
2024-11-25 17:35 ` Re: Parametrization minimum password lenght Emanuele Musella <[email protected]>
@ 2024-12-18 20:56 ` Nathan Bossart <[email protected]>
2024-12-19 07:25 ` Re: Parametrization minimum password lenght Bertrand Drouvot <[email protected]>
2024-12-19 10:22 ` Re: Parametrization minimum password lenght Emanuele Musella <[email protected]>
0 siblings, 2 replies; 30+ messages in thread
From: Nathan Bossart @ 2024-12-18 20:56 UTC (permalink / raw)
To: Emanuele Musella <[email protected]>; +Cc: [email protected]; Bertrand Drouvot <[email protected]>
Here is what I have staged for commit.
--
nathan
From 537260d0492ad3bda2c2fcbfd00c6b0511f5e1b5 Mon Sep 17 00:00:00 2001
From: Nathan Bossart <[email protected]>
Date: Wed, 18 Dec 2024 14:49:20 -0600
Subject: [PATCH v10 1/1] Add passwordcheck.min_password_length.
This new parameter can be used to change the minimum allowed
password length (in bytes). Note that it has no effect if a user
supplies a pre-encrypted password.
Author: Emanuele Musella, Maurizio Boriani
Reviewed-by: Tomas Vondra, Bertrand Drouvot
Discussion: https://postgr.es/m/CA%2BugDNyYtHOtWCqVD3YkSVYDWD_1fO8Jm_ahsDGA5dXhbDPwrQ%40mail.gmail.com
---
.../passwordcheck/expected/passwordcheck.out | 3 ++
.../expected/passwordcheck_1.out | 3 ++
contrib/passwordcheck/passwordcheck.c | 22 +++++++++--
contrib/passwordcheck/sql/passwordcheck.sql | 4 ++
doc/src/sgml/passwordcheck.sgml | 37 +++++++++++++++++++
5 files changed, 65 insertions(+), 4 deletions(-)
diff --git a/contrib/passwordcheck/expected/passwordcheck.out b/contrib/passwordcheck/expected/passwordcheck.out
index dfb2ccfe00..ad1ac400ba 100644
--- a/contrib/passwordcheck/expected/passwordcheck.out
+++ b/contrib/passwordcheck/expected/passwordcheck.out
@@ -6,6 +6,9 @@ ALTER USER regress_passwordcheck_user1 PASSWORD 'a_nice_long_password';
-- error: too short
ALTER USER regress_passwordcheck_user1 PASSWORD 'tooshrt';
ERROR: password is too short
+-- ok
+SET passwordcheck.min_password_length = 6;
+ALTER USER regress_passwordcheck_user1 PASSWORD 'v_shrt';
-- error: contains user name
ALTER USER regress_passwordcheck_user1 PASSWORD 'xyzregress_passwordcheck_user1';
ERROR: password must not contain user name
diff --git a/contrib/passwordcheck/expected/passwordcheck_1.out b/contrib/passwordcheck/expected/passwordcheck_1.out
index 9519d60a49..e97c2c74d7 100644
--- a/contrib/passwordcheck/expected/passwordcheck_1.out
+++ b/contrib/passwordcheck/expected/passwordcheck_1.out
@@ -6,6 +6,9 @@ ALTER USER regress_passwordcheck_user1 PASSWORD 'a_nice_long_password';
-- error: too short
ALTER USER regress_passwordcheck_user1 PASSWORD 'tooshrt';
ERROR: password is too short
+-- ok
+SET passwordcheck.min_password_length = 5;
+ALTER USER regress_passwordcheck_user1 PASSWORD 'v_shrt';
-- error: contains user name
ALTER USER regress_passwordcheck_user1 PASSWORD 'xyzregress_passwordcheck_user1';
ERROR: password must not contain user name
diff --git a/contrib/passwordcheck/passwordcheck.c b/contrib/passwordcheck/passwordcheck.c
index 0785618f2a..a0103806d0 100644
--- a/contrib/passwordcheck/passwordcheck.c
+++ b/contrib/passwordcheck/passwordcheck.c
@@ -15,6 +15,7 @@
#include "postgres.h"
#include <ctype.h>
+#include <limits.h>
#ifdef USE_CRACKLIB
#include <crack.h>
@@ -26,11 +27,11 @@
PG_MODULE_MAGIC;
-/* Saved hook value in case of unload */
+/* Original hook */
static check_password_hook_type prev_check_password_hook = NULL;
-/* passwords shorter than this will be rejected */
-#define MIN_PWD_LENGTH 8
+/* GUC variables */
+static int min_password_length = 8;
/*
* check_password
@@ -93,7 +94,7 @@ check_password(const char *username,
#endif
/* enforce minimum length */
- if (pwdlen < MIN_PWD_LENGTH)
+ if (pwdlen < min_password_length)
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
errmsg("password is too short")));
@@ -142,6 +143,19 @@ check_password(const char *username,
void
_PG_init(void)
{
+ /* Define custom GUC variables. */
+ DefineCustomIntVariable("passwordcheck.min_password_length",
+ "Minimum allowed password length.",
+ NULL,
+ &min_password_length,
+ 8,
+ 0, INT_MAX,
+ PGC_SUSET,
+ GUC_UNIT_BYTE,
+ NULL, NULL, NULL);
+
+ MarkGUCPrefixReserved("passwordcheck");
+
/* activate password checks when the module is loaded */
prev_check_password_hook = check_password_hook;
check_password_hook = check_password;
diff --git a/contrib/passwordcheck/sql/passwordcheck.sql b/contrib/passwordcheck/sql/passwordcheck.sql
index 5953ece5c2..21ad8d452b 100644
--- a/contrib/passwordcheck/sql/passwordcheck.sql
+++ b/contrib/passwordcheck/sql/passwordcheck.sql
@@ -9,6 +9,10 @@ ALTER USER regress_passwordcheck_user1 PASSWORD 'a_nice_long_password';
-- error: too short
ALTER USER regress_passwordcheck_user1 PASSWORD 'tooshrt';
+-- ok
+SET passwordcheck.min_password_length = 6;
+ALTER USER regress_passwordcheck_user1 PASSWORD 'v_shrt';
+
-- error: contains user name
ALTER USER regress_passwordcheck_user1 PASSWORD 'xyzregress_passwordcheck_user1';
diff --git a/doc/src/sgml/passwordcheck.sgml b/doc/src/sgml/passwordcheck.sgml
index 601f489227..7ea3241046 100644
--- a/doc/src/sgml/passwordcheck.sgml
+++ b/doc/src/sgml/passwordcheck.sgml
@@ -59,4 +59,41 @@
</para>
</caution>
+ <sect2 id="passwordcheck-configuration-parameters">
+ <title>Configuration Parameters</title>
+
+ <variablelist>
+ <varlistentry>
+ <term>
+ <varname>passwordcheck.min_password_length</varname> (<type>integer</type>)
+ <indexterm>
+ <primary><varname>passwordcheck.min_password_length</varname> configuration parameter</primary>
+ </indexterm>
+ </term>
+ <listitem>
+ <para>
+ The minimum acceptable password length in bytes. The default is 8. Only
+ superusers can change this setting.
+ </para>
+ <note>
+ <para>
+ This parameter has no effect if a user supplies a pre-encrypted
+ password.
+ </para>
+ </note>
+ </listitem>
+ </varlistentry>
+ </variablelist>
+
+ <para>
+ In ordinary usage, this parameter is set in
+ <filename>postgresql.conf</filename>, but superusers can alter it on-the-fly
+ within their own sessions. Typical usage might be:
+ </para>
+
+<programlisting>
+# postgresql.conf
+passwordcheck.min_password_length = 12
+</programlisting>
+ </sect2>
</sect1>
--
2.39.5 (Apple Git-154)
Attachments:
[text/plain] v10-0001-Add-passwordcheck.min_password_length.patch (5.8K, ../../Z2M2-CoenHUsqz2G@nathan/2-v10-0001-Add-passwordcheck.min_password_length.patch)
download | inline diff:
From 537260d0492ad3bda2c2fcbfd00c6b0511f5e1b5 Mon Sep 17 00:00:00 2001
From: Nathan Bossart <[email protected]>
Date: Wed, 18 Dec 2024 14:49:20 -0600
Subject: [PATCH v10 1/1] Add passwordcheck.min_password_length.
This new parameter can be used to change the minimum allowed
password length (in bytes). Note that it has no effect if a user
supplies a pre-encrypted password.
Author: Emanuele Musella, Maurizio Boriani
Reviewed-by: Tomas Vondra, Bertrand Drouvot
Discussion: https://postgr.es/m/CA%2BugDNyYtHOtWCqVD3YkSVYDWD_1fO8Jm_ahsDGA5dXhbDPwrQ%40mail.gmail.com
---
.../passwordcheck/expected/passwordcheck.out | 3 ++
.../expected/passwordcheck_1.out | 3 ++
contrib/passwordcheck/passwordcheck.c | 22 +++++++++--
contrib/passwordcheck/sql/passwordcheck.sql | 4 ++
doc/src/sgml/passwordcheck.sgml | 37 +++++++++++++++++++
5 files changed, 65 insertions(+), 4 deletions(-)
diff --git a/contrib/passwordcheck/expected/passwordcheck.out b/contrib/passwordcheck/expected/passwordcheck.out
index dfb2ccfe00..ad1ac400ba 100644
--- a/contrib/passwordcheck/expected/passwordcheck.out
+++ b/contrib/passwordcheck/expected/passwordcheck.out
@@ -6,6 +6,9 @@ ALTER USER regress_passwordcheck_user1 PASSWORD 'a_nice_long_password';
-- error: too short
ALTER USER regress_passwordcheck_user1 PASSWORD 'tooshrt';
ERROR: password is too short
+-- ok
+SET passwordcheck.min_password_length = 6;
+ALTER USER regress_passwordcheck_user1 PASSWORD 'v_shrt';
-- error: contains user name
ALTER USER regress_passwordcheck_user1 PASSWORD 'xyzregress_passwordcheck_user1';
ERROR: password must not contain user name
diff --git a/contrib/passwordcheck/expected/passwordcheck_1.out b/contrib/passwordcheck/expected/passwordcheck_1.out
index 9519d60a49..e97c2c74d7 100644
--- a/contrib/passwordcheck/expected/passwordcheck_1.out
+++ b/contrib/passwordcheck/expected/passwordcheck_1.out
@@ -6,6 +6,9 @@ ALTER USER regress_passwordcheck_user1 PASSWORD 'a_nice_long_password';
-- error: too short
ALTER USER regress_passwordcheck_user1 PASSWORD 'tooshrt';
ERROR: password is too short
+-- ok
+SET passwordcheck.min_password_length = 5;
+ALTER USER regress_passwordcheck_user1 PASSWORD 'v_shrt';
-- error: contains user name
ALTER USER regress_passwordcheck_user1 PASSWORD 'xyzregress_passwordcheck_user1';
ERROR: password must not contain user name
diff --git a/contrib/passwordcheck/passwordcheck.c b/contrib/passwordcheck/passwordcheck.c
index 0785618f2a..a0103806d0 100644
--- a/contrib/passwordcheck/passwordcheck.c
+++ b/contrib/passwordcheck/passwordcheck.c
@@ -15,6 +15,7 @@
#include "postgres.h"
#include <ctype.h>
+#include <limits.h>
#ifdef USE_CRACKLIB
#include <crack.h>
@@ -26,11 +27,11 @@
PG_MODULE_MAGIC;
-/* Saved hook value in case of unload */
+/* Original hook */
static check_password_hook_type prev_check_password_hook = NULL;
-/* passwords shorter than this will be rejected */
-#define MIN_PWD_LENGTH 8
+/* GUC variables */
+static int min_password_length = 8;
/*
* check_password
@@ -93,7 +94,7 @@ check_password(const char *username,
#endif
/* enforce minimum length */
- if (pwdlen < MIN_PWD_LENGTH)
+ if (pwdlen < min_password_length)
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
errmsg("password is too short")));
@@ -142,6 +143,19 @@ check_password(const char *username,
void
_PG_init(void)
{
+ /* Define custom GUC variables. */
+ DefineCustomIntVariable("passwordcheck.min_password_length",
+ "Minimum allowed password length.",
+ NULL,
+ &min_password_length,
+ 8,
+ 0, INT_MAX,
+ PGC_SUSET,
+ GUC_UNIT_BYTE,
+ NULL, NULL, NULL);
+
+ MarkGUCPrefixReserved("passwordcheck");
+
/* activate password checks when the module is loaded */
prev_check_password_hook = check_password_hook;
check_password_hook = check_password;
diff --git a/contrib/passwordcheck/sql/passwordcheck.sql b/contrib/passwordcheck/sql/passwordcheck.sql
index 5953ece5c2..21ad8d452b 100644
--- a/contrib/passwordcheck/sql/passwordcheck.sql
+++ b/contrib/passwordcheck/sql/passwordcheck.sql
@@ -9,6 +9,10 @@ ALTER USER regress_passwordcheck_user1 PASSWORD 'a_nice_long_password';
-- error: too short
ALTER USER regress_passwordcheck_user1 PASSWORD 'tooshrt';
+-- ok
+SET passwordcheck.min_password_length = 6;
+ALTER USER regress_passwordcheck_user1 PASSWORD 'v_shrt';
+
-- error: contains user name
ALTER USER regress_passwordcheck_user1 PASSWORD 'xyzregress_passwordcheck_user1';
diff --git a/doc/src/sgml/passwordcheck.sgml b/doc/src/sgml/passwordcheck.sgml
index 601f489227..7ea3241046 100644
--- a/doc/src/sgml/passwordcheck.sgml
+++ b/doc/src/sgml/passwordcheck.sgml
@@ -59,4 +59,41 @@
</para>
</caution>
+ <sect2 id="passwordcheck-configuration-parameters">
+ <title>Configuration Parameters</title>
+
+ <variablelist>
+ <varlistentry>
+ <term>
+ <varname>passwordcheck.min_password_length</varname> (<type>integer</type>)
+ <indexterm>
+ <primary><varname>passwordcheck.min_password_length</varname> configuration parameter</primary>
+ </indexterm>
+ </term>
+ <listitem>
+ <para>
+ The minimum acceptable password length in bytes. The default is 8. Only
+ superusers can change this setting.
+ </para>
+ <note>
+ <para>
+ This parameter has no effect if a user supplies a pre-encrypted
+ password.
+ </para>
+ </note>
+ </listitem>
+ </varlistentry>
+ </variablelist>
+
+ <para>
+ In ordinary usage, this parameter is set in
+ <filename>postgresql.conf</filename>, but superusers can alter it on-the-fly
+ within their own sessions. Typical usage might be:
+ </para>
+
+<programlisting>
+# postgresql.conf
+passwordcheck.min_password_length = 12
+</programlisting>
+ </sect2>
</sect1>
--
2.39.5 (Apple Git-154)
^ permalink raw reply [nested|flat] 30+ messages in thread
* Re: Parametrization minimum password lenght
2024-11-25 10:15 Re: Parametrization minimum password lenght Emanuele Musella <[email protected]>
2024-11-25 17:35 ` Re: Parametrization minimum password lenght Emanuele Musella <[email protected]>
2024-12-18 20:56 ` Re: Parametrization minimum password lenght Nathan Bossart <[email protected]>
@ 2024-12-19 07:25 ` Bertrand Drouvot <[email protected]>
2024-12-19 15:36 ` Re: Parametrization minimum password lenght Nathan Bossart <[email protected]>
1 sibling, 1 reply; 30+ messages in thread
From: Bertrand Drouvot @ 2024-12-19 07:25 UTC (permalink / raw)
To: Nathan Bossart <[email protected]>; +Cc: Emanuele Musella <[email protected]>; [email protected]
Hi,
On Wed, Dec 18, 2024 at 02:56:24PM -0600, Nathan Bossart wrote:
> Here is what I have staged for commit.
Thanks!
A few comments:
=== 1
+ if (pwdlen < min_password_length)
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
errmsg("password is too short")));
Now that the minimum password length is not "hardcoded" anymore, I wonder if it
wouldn't be better to provide more details here (pwdlen and min_password_length).
Suggestion in on_top_of_0001.txt attached.
=== 2
+ /* Define custom GUC variables. */
+ DefineCustomIntVariable("passwordcheck.min_password_length",
+ "Minimum allowed password length.",
+ NULL,
+ &min_password_length,
+ 8,
+ 0, INT_MAX,
Since password must contain both letters and nonletters, 0 seems too low. I
wonder if 2 is not a better value (done in on_top_of_0001.txt attached).
Also, it seems to me that INT_MAX is too large (as mentioned in [1]), but that's
probably a nit.
[1]: https://www.postgresql.org/message-id/Zzzmw4IAvrypmFO4%40ip-10-97-1-34.eu-west-3.compute.internal
Regards,
--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
diff --git a/contrib/passwordcheck/expected/passwordcheck.out b/contrib/passwordcheck/expected/passwordcheck.out
index ad1ac400ba..5d7a9496c9 100644
--- a/contrib/passwordcheck/expected/passwordcheck.out
+++ b/contrib/passwordcheck/expected/passwordcheck.out
@@ -5,7 +5,7 @@ CREATE USER regress_passwordcheck_user1;
ALTER USER regress_passwordcheck_user1 PASSWORD 'a_nice_long_password';
-- error: too short
ALTER USER regress_passwordcheck_user1 PASSWORD 'tooshrt';
-ERROR: password is too short
+ERROR: password is too short: 7 (< 8)
-- ok
SET passwordcheck.min_password_length = 6;
ALTER USER regress_passwordcheck_user1 PASSWORD 'v_shrt';
diff --git a/contrib/passwordcheck/passwordcheck.c b/contrib/passwordcheck/passwordcheck.c
index a0103806d0..5027fa35c8 100644
--- a/contrib/passwordcheck/passwordcheck.c
+++ b/contrib/passwordcheck/passwordcheck.c
@@ -97,7 +97,7 @@ check_password(const char *username,
if (pwdlen < min_password_length)
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("password is too short")));
+ errmsg("password is too short: %d (< %d)", pwdlen, min_password_length)));
/* check if the password contains the username */
if (strstr(password, username))
@@ -149,7 +149,7 @@ _PG_init(void)
NULL,
&min_password_length,
8,
- 0, INT_MAX,
+ 2, INT_MAX,
PGC_SUSET,
GUC_UNIT_BYTE,
NULL, NULL, NULL);
Attachments:
[text/plain] on_top_of_0001.txt (1.4K, ../../[email protected]/2-on_top_of_0001.txt)
download | inline diff:
diff --git a/contrib/passwordcheck/expected/passwordcheck.out b/contrib/passwordcheck/expected/passwordcheck.out
index ad1ac400ba..5d7a9496c9 100644
--- a/contrib/passwordcheck/expected/passwordcheck.out
+++ b/contrib/passwordcheck/expected/passwordcheck.out
@@ -5,7 +5,7 @@ CREATE USER regress_passwordcheck_user1;
ALTER USER regress_passwordcheck_user1 PASSWORD 'a_nice_long_password';
-- error: too short
ALTER USER regress_passwordcheck_user1 PASSWORD 'tooshrt';
-ERROR: password is too short
+ERROR: password is too short: 7 (< 8)
-- ok
SET passwordcheck.min_password_length = 6;
ALTER USER regress_passwordcheck_user1 PASSWORD 'v_shrt';
diff --git a/contrib/passwordcheck/passwordcheck.c b/contrib/passwordcheck/passwordcheck.c
index a0103806d0..5027fa35c8 100644
--- a/contrib/passwordcheck/passwordcheck.c
+++ b/contrib/passwordcheck/passwordcheck.c
@@ -97,7 +97,7 @@ check_password(const char *username,
if (pwdlen < min_password_length)
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("password is too short")));
+ errmsg("password is too short: %d (< %d)", pwdlen, min_password_length)));
/* check if the password contains the username */
if (strstr(password, username))
@@ -149,7 +149,7 @@ _PG_init(void)
NULL,
&min_password_length,
8,
- 0, INT_MAX,
+ 2, INT_MAX,
PGC_SUSET,
GUC_UNIT_BYTE,
NULL, NULL, NULL);
^ permalink raw reply [nested|flat] 30+ messages in thread
* Re: Parametrization minimum password lenght
2024-11-25 10:15 Re: Parametrization minimum password lenght Emanuele Musella <[email protected]>
2024-11-25 17:35 ` Re: Parametrization minimum password lenght Emanuele Musella <[email protected]>
2024-12-18 20:56 ` Re: Parametrization minimum password lenght Nathan Bossart <[email protected]>
2024-12-19 07:25 ` Re: Parametrization minimum password lenght Bertrand Drouvot <[email protected]>
@ 2024-12-19 15:36 ` Nathan Bossart <[email protected]>
2024-12-19 15:57 ` Re: Parametrization minimum password lenght Nathan Bossart <[email protected]>
0 siblings, 1 reply; 30+ messages in thread
From: Nathan Bossart @ 2024-12-19 15:36 UTC (permalink / raw)
To: Bertrand Drouvot <[email protected]>; +Cc: Emanuele Musella <[email protected]>; [email protected]
On Thu, Dec 19, 2024 at 07:25:30AM +0000, Bertrand Drouvot wrote:
> + if (pwdlen < min_password_length)
> ereport(ERROR,
> (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
> errmsg("password is too short")));
>
> Now that the minimum password length is not "hardcoded" anymore, I wonder if it
> wouldn't be better to provide more details here (pwdlen and min_password_length).
>
> Suggestion in on_top_of_0001.txt attached.
Yeah, good point.
> + /* Define custom GUC variables. */
> + DefineCustomIntVariable("passwordcheck.min_password_length",
> + "Minimum allowed password length.",
> + NULL,
> + &min_password_length,
> + 8,
> + 0, INT_MAX,
>
> Since password must contain both letters and nonletters, 0 seems too low. I
> wonder if 2 is not a better value (done in on_top_of_0001.txt attached).
>
> Also, it seems to me that INT_MAX is too large (as mentioned in [1]), but that's
> probably a nit.
I don't think we need to restrict the allowed values here. It's true that
the lower bound will be effectively 2 due to the other checks, but that's
not the responsibility of this one to enforce. Those other checks will
likely become configurable at some point, too.
> - errmsg("password is too short")));
> + errmsg("password is too short: %d (< %d)", pwdlen, min_password_length)));
I think we should explicitly point to the parameter and note its current
value.
--
nathan
^ permalink raw reply [nested|flat] 30+ messages in thread
* Re: Parametrization minimum password lenght
2024-11-25 10:15 Re: Parametrization minimum password lenght Emanuele Musella <[email protected]>
2024-11-25 17:35 ` Re: Parametrization minimum password lenght Emanuele Musella <[email protected]>
2024-12-18 20:56 ` Re: Parametrization minimum password lenght Nathan Bossart <[email protected]>
2024-12-19 07:25 ` Re: Parametrization minimum password lenght Bertrand Drouvot <[email protected]>
2024-12-19 15:36 ` Re: Parametrization minimum password lenght Nathan Bossart <[email protected]>
@ 2024-12-19 15:57 ` Nathan Bossart <[email protected]>
2024-12-20 09:15 ` Re: Parametrization minimum password lenght Bertrand Drouvot <[email protected]>
2024-12-25 05:15 ` Re: Parametrization minimum password lenght Japin Li <[email protected]>
0 siblings, 2 replies; 30+ messages in thread
From: Nathan Bossart @ 2024-12-19 15:57 UTC (permalink / raw)
To: Bertrand Drouvot <[email protected]>; +Cc: Emanuele Musella <[email protected]>; [email protected]
On Thu, Dec 19, 2024 at 09:36:17AM -0600, Nathan Bossart wrote:
> On Thu, Dec 19, 2024 at 07:25:30AM +0000, Bertrand Drouvot wrote:
>> - errmsg("password is too short")));
>> + errmsg("password is too short: %d (< %d)", pwdlen, min_password_length)));
>
> I think we should explicitly point to the parameter and note its current
> value.
Like so.
--
nathan
From ce67f3ff5fca47b443195f5bd1932429294bdbfb Mon Sep 17 00:00:00 2001
From: Nathan Bossart <[email protected]>
Date: Wed, 18 Dec 2024 14:49:20 -0600
Subject: [PATCH v11 1/1] Add passwordcheck.min_password_length.
This new parameter can be used to change the minimum allowed
password length (in bytes). Note that it has no effect if a user
supplies a pre-encrypted password.
Author: Emanuele Musella, Maurizio Boriani
Reviewed-by: Tomas Vondra, Bertrand Drouvot
Discussion: https://postgr.es/m/CA%2BugDNyYtHOtWCqVD3YkSVYDWD_1fO8Jm_ahsDGA5dXhbDPwrQ%40mail.gmail.com
---
.../passwordcheck/expected/passwordcheck.out | 4 ++
.../expected/passwordcheck_1.out | 4 ++
contrib/passwordcheck/passwordcheck.c | 26 ++++++++++---
contrib/passwordcheck/sql/passwordcheck.sql | 4 ++
doc/src/sgml/passwordcheck.sgml | 37 +++++++++++++++++++
5 files changed, 70 insertions(+), 5 deletions(-)
diff --git a/contrib/passwordcheck/expected/passwordcheck.out b/contrib/passwordcheck/expected/passwordcheck.out
index dfb2ccfe00..83472c76d2 100644
--- a/contrib/passwordcheck/expected/passwordcheck.out
+++ b/contrib/passwordcheck/expected/passwordcheck.out
@@ -6,6 +6,10 @@ ALTER USER regress_passwordcheck_user1 PASSWORD 'a_nice_long_password';
-- error: too short
ALTER USER regress_passwordcheck_user1 PASSWORD 'tooshrt';
ERROR: password is too short
+DETAIL: password must be at least "passwordcheck.min_password_length" (8) bytes long
+-- ok
+SET passwordcheck.min_password_length = 6;
+ALTER USER regress_passwordcheck_user1 PASSWORD 'v_shrt';
-- error: contains user name
ALTER USER regress_passwordcheck_user1 PASSWORD 'xyzregress_passwordcheck_user1';
ERROR: password must not contain user name
diff --git a/contrib/passwordcheck/expected/passwordcheck_1.out b/contrib/passwordcheck/expected/passwordcheck_1.out
index 9519d60a49..fb12ec45cc 100644
--- a/contrib/passwordcheck/expected/passwordcheck_1.out
+++ b/contrib/passwordcheck/expected/passwordcheck_1.out
@@ -6,6 +6,10 @@ ALTER USER regress_passwordcheck_user1 PASSWORD 'a_nice_long_password';
-- error: too short
ALTER USER regress_passwordcheck_user1 PASSWORD 'tooshrt';
ERROR: password is too short
+DETAIL: password must be at least "passwordcheck.min_password_length" (8) bytes long
+-- ok
+SET passwordcheck.min_password_length = 6;
+ALTER USER regress_passwordcheck_user1 PASSWORD 'v_shrt';
-- error: contains user name
ALTER USER regress_passwordcheck_user1 PASSWORD 'xyzregress_passwordcheck_user1';
ERROR: password must not contain user name
diff --git a/contrib/passwordcheck/passwordcheck.c b/contrib/passwordcheck/passwordcheck.c
index 0785618f2a..d79ecb00b6 100644
--- a/contrib/passwordcheck/passwordcheck.c
+++ b/contrib/passwordcheck/passwordcheck.c
@@ -15,6 +15,7 @@
#include "postgres.h"
#include <ctype.h>
+#include <limits.h>
#ifdef USE_CRACKLIB
#include <crack.h>
@@ -26,11 +27,11 @@
PG_MODULE_MAGIC;
-/* Saved hook value in case of unload */
+/* Original hook */
static check_password_hook_type prev_check_password_hook = NULL;
-/* passwords shorter than this will be rejected */
-#define MIN_PWD_LENGTH 8
+/* GUC variables */
+static int min_password_length = 8;
/*
* check_password
@@ -93,10 +94,12 @@ check_password(const char *username,
#endif
/* enforce minimum length */
- if (pwdlen < MIN_PWD_LENGTH)
+ if (pwdlen < min_password_length)
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("password is too short")));
+ errmsg("password is too short"),
+ errdetail("password must be at least \"passwordcheck.min_password_length\" (%d) bytes long",
+ min_password_length)));
/* check if the password contains the username */
if (strstr(password, username))
@@ -142,6 +145,19 @@ check_password(const char *username,
void
_PG_init(void)
{
+ /* Define custom GUC variables. */
+ DefineCustomIntVariable("passwordcheck.min_password_length",
+ "Minimum allowed password length.",
+ NULL,
+ &min_password_length,
+ 8,
+ 0, INT_MAX,
+ PGC_SUSET,
+ GUC_UNIT_BYTE,
+ NULL, NULL, NULL);
+
+ MarkGUCPrefixReserved("passwordcheck");
+
/* activate password checks when the module is loaded */
prev_check_password_hook = check_password_hook;
check_password_hook = check_password;
diff --git a/contrib/passwordcheck/sql/passwordcheck.sql b/contrib/passwordcheck/sql/passwordcheck.sql
index 5953ece5c2..21ad8d452b 100644
--- a/contrib/passwordcheck/sql/passwordcheck.sql
+++ b/contrib/passwordcheck/sql/passwordcheck.sql
@@ -9,6 +9,10 @@ ALTER USER regress_passwordcheck_user1 PASSWORD 'a_nice_long_password';
-- error: too short
ALTER USER regress_passwordcheck_user1 PASSWORD 'tooshrt';
+-- ok
+SET passwordcheck.min_password_length = 6;
+ALTER USER regress_passwordcheck_user1 PASSWORD 'v_shrt';
+
-- error: contains user name
ALTER USER regress_passwordcheck_user1 PASSWORD 'xyzregress_passwordcheck_user1';
diff --git a/doc/src/sgml/passwordcheck.sgml b/doc/src/sgml/passwordcheck.sgml
index 601f489227..7ea3241046 100644
--- a/doc/src/sgml/passwordcheck.sgml
+++ b/doc/src/sgml/passwordcheck.sgml
@@ -59,4 +59,41 @@
</para>
</caution>
+ <sect2 id="passwordcheck-configuration-parameters">
+ <title>Configuration Parameters</title>
+
+ <variablelist>
+ <varlistentry>
+ <term>
+ <varname>passwordcheck.min_password_length</varname> (<type>integer</type>)
+ <indexterm>
+ <primary><varname>passwordcheck.min_password_length</varname> configuration parameter</primary>
+ </indexterm>
+ </term>
+ <listitem>
+ <para>
+ The minimum acceptable password length in bytes. The default is 8. Only
+ superusers can change this setting.
+ </para>
+ <note>
+ <para>
+ This parameter has no effect if a user supplies a pre-encrypted
+ password.
+ </para>
+ </note>
+ </listitem>
+ </varlistentry>
+ </variablelist>
+
+ <para>
+ In ordinary usage, this parameter is set in
+ <filename>postgresql.conf</filename>, but superusers can alter it on-the-fly
+ within their own sessions. Typical usage might be:
+ </para>
+
+<programlisting>
+# postgresql.conf
+passwordcheck.min_password_length = 12
+</programlisting>
+ </sect2>
</sect1>
--
2.39.5 (Apple Git-154)
Attachments:
[text/plain] v11-0001-Add-passwordcheck.min_password_length.patch (6.2K, ../../Z2RCa15Qut91n43u@nathan/2-v11-0001-Add-passwordcheck.min_password_length.patch)
download | inline diff:
From ce67f3ff5fca47b443195f5bd1932429294bdbfb Mon Sep 17 00:00:00 2001
From: Nathan Bossart <[email protected]>
Date: Wed, 18 Dec 2024 14:49:20 -0600
Subject: [PATCH v11 1/1] Add passwordcheck.min_password_length.
This new parameter can be used to change the minimum allowed
password length (in bytes). Note that it has no effect if a user
supplies a pre-encrypted password.
Author: Emanuele Musella, Maurizio Boriani
Reviewed-by: Tomas Vondra, Bertrand Drouvot
Discussion: https://postgr.es/m/CA%2BugDNyYtHOtWCqVD3YkSVYDWD_1fO8Jm_ahsDGA5dXhbDPwrQ%40mail.gmail.com
---
.../passwordcheck/expected/passwordcheck.out | 4 ++
.../expected/passwordcheck_1.out | 4 ++
contrib/passwordcheck/passwordcheck.c | 26 ++++++++++---
contrib/passwordcheck/sql/passwordcheck.sql | 4 ++
doc/src/sgml/passwordcheck.sgml | 37 +++++++++++++++++++
5 files changed, 70 insertions(+), 5 deletions(-)
diff --git a/contrib/passwordcheck/expected/passwordcheck.out b/contrib/passwordcheck/expected/passwordcheck.out
index dfb2ccfe00..83472c76d2 100644
--- a/contrib/passwordcheck/expected/passwordcheck.out
+++ b/contrib/passwordcheck/expected/passwordcheck.out
@@ -6,6 +6,10 @@ ALTER USER regress_passwordcheck_user1 PASSWORD 'a_nice_long_password';
-- error: too short
ALTER USER regress_passwordcheck_user1 PASSWORD 'tooshrt';
ERROR: password is too short
+DETAIL: password must be at least "passwordcheck.min_password_length" (8) bytes long
+-- ok
+SET passwordcheck.min_password_length = 6;
+ALTER USER regress_passwordcheck_user1 PASSWORD 'v_shrt';
-- error: contains user name
ALTER USER regress_passwordcheck_user1 PASSWORD 'xyzregress_passwordcheck_user1';
ERROR: password must not contain user name
diff --git a/contrib/passwordcheck/expected/passwordcheck_1.out b/contrib/passwordcheck/expected/passwordcheck_1.out
index 9519d60a49..fb12ec45cc 100644
--- a/contrib/passwordcheck/expected/passwordcheck_1.out
+++ b/contrib/passwordcheck/expected/passwordcheck_1.out
@@ -6,6 +6,10 @@ ALTER USER regress_passwordcheck_user1 PASSWORD 'a_nice_long_password';
-- error: too short
ALTER USER regress_passwordcheck_user1 PASSWORD 'tooshrt';
ERROR: password is too short
+DETAIL: password must be at least "passwordcheck.min_password_length" (8) bytes long
+-- ok
+SET passwordcheck.min_password_length = 6;
+ALTER USER regress_passwordcheck_user1 PASSWORD 'v_shrt';
-- error: contains user name
ALTER USER regress_passwordcheck_user1 PASSWORD 'xyzregress_passwordcheck_user1';
ERROR: password must not contain user name
diff --git a/contrib/passwordcheck/passwordcheck.c b/contrib/passwordcheck/passwordcheck.c
index 0785618f2a..d79ecb00b6 100644
--- a/contrib/passwordcheck/passwordcheck.c
+++ b/contrib/passwordcheck/passwordcheck.c
@@ -15,6 +15,7 @@
#include "postgres.h"
#include <ctype.h>
+#include <limits.h>
#ifdef USE_CRACKLIB
#include <crack.h>
@@ -26,11 +27,11 @@
PG_MODULE_MAGIC;
-/* Saved hook value in case of unload */
+/* Original hook */
static check_password_hook_type prev_check_password_hook = NULL;
-/* passwords shorter than this will be rejected */
-#define MIN_PWD_LENGTH 8
+/* GUC variables */
+static int min_password_length = 8;
/*
* check_password
@@ -93,10 +94,12 @@ check_password(const char *username,
#endif
/* enforce minimum length */
- if (pwdlen < MIN_PWD_LENGTH)
+ if (pwdlen < min_password_length)
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("password is too short")));
+ errmsg("password is too short"),
+ errdetail("password must be at least \"passwordcheck.min_password_length\" (%d) bytes long",
+ min_password_length)));
/* check if the password contains the username */
if (strstr(password, username))
@@ -142,6 +145,19 @@ check_password(const char *username,
void
_PG_init(void)
{
+ /* Define custom GUC variables. */
+ DefineCustomIntVariable("passwordcheck.min_password_length",
+ "Minimum allowed password length.",
+ NULL,
+ &min_password_length,
+ 8,
+ 0, INT_MAX,
+ PGC_SUSET,
+ GUC_UNIT_BYTE,
+ NULL, NULL, NULL);
+
+ MarkGUCPrefixReserved("passwordcheck");
+
/* activate password checks when the module is loaded */
prev_check_password_hook = check_password_hook;
check_password_hook = check_password;
diff --git a/contrib/passwordcheck/sql/passwordcheck.sql b/contrib/passwordcheck/sql/passwordcheck.sql
index 5953ece5c2..21ad8d452b 100644
--- a/contrib/passwordcheck/sql/passwordcheck.sql
+++ b/contrib/passwordcheck/sql/passwordcheck.sql
@@ -9,6 +9,10 @@ ALTER USER regress_passwordcheck_user1 PASSWORD 'a_nice_long_password';
-- error: too short
ALTER USER regress_passwordcheck_user1 PASSWORD 'tooshrt';
+-- ok
+SET passwordcheck.min_password_length = 6;
+ALTER USER regress_passwordcheck_user1 PASSWORD 'v_shrt';
+
-- error: contains user name
ALTER USER regress_passwordcheck_user1 PASSWORD 'xyzregress_passwordcheck_user1';
diff --git a/doc/src/sgml/passwordcheck.sgml b/doc/src/sgml/passwordcheck.sgml
index 601f489227..7ea3241046 100644
--- a/doc/src/sgml/passwordcheck.sgml
+++ b/doc/src/sgml/passwordcheck.sgml
@@ -59,4 +59,41 @@
</para>
</caution>
+ <sect2 id="passwordcheck-configuration-parameters">
+ <title>Configuration Parameters</title>
+
+ <variablelist>
+ <varlistentry>
+ <term>
+ <varname>passwordcheck.min_password_length</varname> (<type>integer</type>)
+ <indexterm>
+ <primary><varname>passwordcheck.min_password_length</varname> configuration parameter</primary>
+ </indexterm>
+ </term>
+ <listitem>
+ <para>
+ The minimum acceptable password length in bytes. The default is 8. Only
+ superusers can change this setting.
+ </para>
+ <note>
+ <para>
+ This parameter has no effect if a user supplies a pre-encrypted
+ password.
+ </para>
+ </note>
+ </listitem>
+ </varlistentry>
+ </variablelist>
+
+ <para>
+ In ordinary usage, this parameter is set in
+ <filename>postgresql.conf</filename>, but superusers can alter it on-the-fly
+ within their own sessions. Typical usage might be:
+ </para>
+
+<programlisting>
+# postgresql.conf
+passwordcheck.min_password_length = 12
+</programlisting>
+ </sect2>
</sect1>
--
2.39.5 (Apple Git-154)
^ permalink raw reply [nested|flat] 30+ messages in thread
* Re: Parametrization minimum password lenght
2024-11-25 10:15 Re: Parametrization minimum password lenght Emanuele Musella <[email protected]>
2024-11-25 17:35 ` Re: Parametrization minimum password lenght Emanuele Musella <[email protected]>
2024-12-18 20:56 ` Re: Parametrization minimum password lenght Nathan Bossart <[email protected]>
2024-12-19 07:25 ` Re: Parametrization minimum password lenght Bertrand Drouvot <[email protected]>
2024-12-19 15:36 ` Re: Parametrization minimum password lenght Nathan Bossart <[email protected]>
2024-12-19 15:57 ` Re: Parametrization minimum password lenght Nathan Bossart <[email protected]>
@ 2024-12-20 09:15 ` Bertrand Drouvot <[email protected]>
1 sibling, 0 replies; 30+ messages in thread
From: Bertrand Drouvot @ 2024-12-20 09:15 UTC (permalink / raw)
To: Nathan Bossart <[email protected]>; +Cc: Emanuele Musella <[email protected]>; [email protected]
On Thu, Dec 19, 2024 at 09:57:31AM -0600, Nathan Bossart wrote:
> On Thu, Dec 19, 2024 at 09:36:17AM -0600, Nathan Bossart wrote:
> > On Thu, Dec 19, 2024 at 07:25:30AM +0000, Bertrand Drouvot wrote:
> >> - errmsg("password is too short")));
> >> + errmsg("password is too short: %d (< %d)", pwdlen, min_password_length)));
> >
> > I think we should explicitly point to the parameter and note its current
> > value.
>
> Like so.
LGTM.
Regards,
--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
^ permalink raw reply [nested|flat] 30+ messages in thread
* Re: Parametrization minimum password lenght
2024-11-25 10:15 Re: Parametrization minimum password lenght Emanuele Musella <[email protected]>
2024-11-25 17:35 ` Re: Parametrization minimum password lenght Emanuele Musella <[email protected]>
2024-12-18 20:56 ` Re: Parametrization minimum password lenght Nathan Bossart <[email protected]>
2024-12-19 07:25 ` Re: Parametrization minimum password lenght Bertrand Drouvot <[email protected]>
2024-12-19 15:36 ` Re: Parametrization minimum password lenght Nathan Bossart <[email protected]>
2024-12-19 15:57 ` Re: Parametrization minimum password lenght Nathan Bossart <[email protected]>
@ 2024-12-25 05:15 ` Japin Li <[email protected]>
2025-01-06 19:32 ` Re: Parametrization minimum password lenght Nathan Bossart <[email protected]>
1 sibling, 1 reply; 30+ messages in thread
From: Japin Li @ 2024-12-25 05:15 UTC (permalink / raw)
To: Nathan Bossart <[email protected]>; +Cc: Bertrand Drouvot <[email protected]>; Emanuele Musella <[email protected]>; [email protected]
On Thu, 19 Dec 2024 at 09:57, Nathan Bossart <[email protected]> wrote:
> On Thu, Dec 19, 2024 at 09:36:17AM -0600, Nathan Bossart wrote:
>> On Thu, Dec 19, 2024 at 07:25:30AM +0000, Bertrand Drouvot wrote:
>>> - errmsg("password is too short")));
>>> + errmsg("password is too short: %d (< %d)", pwdlen, min_password_length)));
>>
>> I think we should explicitly point to the parameter and note its current
>> value.
>
> Like so.
It seems that the v11 cannot apply on master since 7f97b4734, and rebase is needed.
$ git am ~/v11-0001-Add-passwordcheck.min_password_length.patch
Applying: Add passwordcheck.min_password_length.
error: patch failed: contrib/passwordcheck/passwordcheck.c:26
error: contrib/passwordcheck/passwordcheck.c: patch does not apply
Patch failed at 0001 Add passwordcheck.min_password_length.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".
The comment of prev_check_password_hook has been changed which case the apply failed.
--
Regrads,
Japin Li
^ permalink raw reply [nested|flat] 30+ messages in thread
* Re: Parametrization minimum password lenght
2024-11-25 10:15 Re: Parametrization minimum password lenght Emanuele Musella <[email protected]>
2024-11-25 17:35 ` Re: Parametrization minimum password lenght Emanuele Musella <[email protected]>
2024-12-18 20:56 ` Re: Parametrization minimum password lenght Nathan Bossart <[email protected]>
2024-12-19 07:25 ` Re: Parametrization minimum password lenght Bertrand Drouvot <[email protected]>
2024-12-19 15:36 ` Re: Parametrization minimum password lenght Nathan Bossart <[email protected]>
2024-12-19 15:57 ` Re: Parametrization minimum password lenght Nathan Bossart <[email protected]>
2024-12-25 05:15 ` Re: Parametrization minimum password lenght Japin Li <[email protected]>
@ 2025-01-06 19:32 ` Nathan Bossart <[email protected]>
2025-01-07 21:08 ` Re: Parametrization minimum password lenght Nathan Bossart <[email protected]>
0 siblings, 1 reply; 30+ messages in thread
From: Nathan Bossart @ 2025-01-06 19:32 UTC (permalink / raw)
To: Japin Li <[email protected]>; +Cc: Bertrand Drouvot <[email protected]>; Emanuele Musella <[email protected]>; [email protected]
On Wed, Dec 25, 2024 at 01:15:53PM +0800, Japin Li wrote:
> It seems that the v11 cannot apply on master since 7f97b4734, and rebase is needed.
>
> $ git am ~/v11-0001-Add-passwordcheck.min_password_length.patch
> Applying: Add passwordcheck.min_password_length.
> error: patch failed: contrib/passwordcheck/passwordcheck.c:26
> error: contrib/passwordcheck/passwordcheck.c: patch does not apply
> Patch failed at 0001 Add passwordcheck.min_password_length.
> hint: Use 'git am --show-current-patch=diff' to see the failed patch
> When you have resolved this problem, run "git am --continue".
> If you prefer to skip this patch, run "git am --skip" instead.
> To restore the original branch and stop patching, run "git am --abort".
>
>
> The comment of prev_check_password_hook has been changed which case the apply failed.
Here is a rebased version of the patch. Barring objections, I am planning
to commit this soon.
--
nathan
From fd7d382fc463fc47ee1c18695d78a2203723f46b Mon Sep 17 00:00:00 2001
From: Nathan Bossart <[email protected]>
Date: Mon, 6 Jan 2025 13:29:58 -0600
Subject: [PATCH v12 1/1] Add passwordcheck.min_password_length.
This new parameter can be used to change the minimum allowed
password length (in bytes). Note that it has no effect if a user
supplies a pre-encrypted password.
Author: Emanuele Musella, Maurizio Boriani
Reviewed-by: Tomas Vondra, Bertrand Drouvot, Japin Li
Discussion: https://postgr.es/m/CA%2BugDNyYtHOtWCqVD3YkSVYDWD_1fO8Jm_ahsDGA5dXhbDPwrQ%40mail.gmail.com
---
.../passwordcheck/expected/passwordcheck.out | 4 ++
.../expected/passwordcheck_1.out | 4 ++
contrib/passwordcheck/passwordcheck.c | 24 ++++++++++--
contrib/passwordcheck/sql/passwordcheck.sql | 4 ++
doc/src/sgml/passwordcheck.sgml | 37 +++++++++++++++++++
5 files changed, 69 insertions(+), 4 deletions(-)
diff --git a/contrib/passwordcheck/expected/passwordcheck.out b/contrib/passwordcheck/expected/passwordcheck.out
index dfb2ccfe008..83472c76d27 100644
--- a/contrib/passwordcheck/expected/passwordcheck.out
+++ b/contrib/passwordcheck/expected/passwordcheck.out
@@ -6,6 +6,10 @@ ALTER USER regress_passwordcheck_user1 PASSWORD 'a_nice_long_password';
-- error: too short
ALTER USER regress_passwordcheck_user1 PASSWORD 'tooshrt';
ERROR: password is too short
+DETAIL: password must be at least "passwordcheck.min_password_length" (8) bytes long
+-- ok
+SET passwordcheck.min_password_length = 6;
+ALTER USER regress_passwordcheck_user1 PASSWORD 'v_shrt';
-- error: contains user name
ALTER USER regress_passwordcheck_user1 PASSWORD 'xyzregress_passwordcheck_user1';
ERROR: password must not contain user name
diff --git a/contrib/passwordcheck/expected/passwordcheck_1.out b/contrib/passwordcheck/expected/passwordcheck_1.out
index 9519d60a495..fb12ec45cc4 100644
--- a/contrib/passwordcheck/expected/passwordcheck_1.out
+++ b/contrib/passwordcheck/expected/passwordcheck_1.out
@@ -6,6 +6,10 @@ ALTER USER regress_passwordcheck_user1 PASSWORD 'a_nice_long_password';
-- error: too short
ALTER USER regress_passwordcheck_user1 PASSWORD 'tooshrt';
ERROR: password is too short
+DETAIL: password must be at least "passwordcheck.min_password_length" (8) bytes long
+-- ok
+SET passwordcheck.min_password_length = 6;
+ALTER USER regress_passwordcheck_user1 PASSWORD 'v_shrt';
-- error: contains user name
ALTER USER regress_passwordcheck_user1 PASSWORD 'xyzregress_passwordcheck_user1';
ERROR: password must not contain user name
diff --git a/contrib/passwordcheck/passwordcheck.c b/contrib/passwordcheck/passwordcheck.c
index 96ada5b98c6..3db42a5b99d 100644
--- a/contrib/passwordcheck/passwordcheck.c
+++ b/contrib/passwordcheck/passwordcheck.c
@@ -15,6 +15,7 @@
#include "postgres.h"
#include <ctype.h>
+#include <limits.h>
#ifdef USE_CRACKLIB
#include <crack.h>
@@ -29,8 +30,8 @@ PG_MODULE_MAGIC;
/* Saved hook value */
static check_password_hook_type prev_check_password_hook = NULL;
-/* passwords shorter than this will be rejected */
-#define MIN_PWD_LENGTH 8
+/* GUC variables */
+static int min_password_length = 8;
/*
* check_password
@@ -93,10 +94,12 @@ check_password(const char *username,
#endif
/* enforce minimum length */
- if (pwdlen < MIN_PWD_LENGTH)
+ if (pwdlen < min_password_length)
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("password is too short")));
+ errmsg("password is too short"),
+ errdetail("password must be at least \"passwordcheck.min_password_length\" (%d) bytes long",
+ min_password_length)));
/* check if the password contains the username */
if (strstr(password, username))
@@ -142,6 +145,19 @@ check_password(const char *username,
void
_PG_init(void)
{
+ /* Define custom GUC variables. */
+ DefineCustomIntVariable("passwordcheck.min_password_length",
+ "Minimum allowed password length.",
+ NULL,
+ &min_password_length,
+ 8,
+ 0, INT_MAX,
+ PGC_SUSET,
+ GUC_UNIT_BYTE,
+ NULL, NULL, NULL);
+
+ MarkGUCPrefixReserved("passwordcheck");
+
/* activate password checks when the module is loaded */
prev_check_password_hook = check_password_hook;
check_password_hook = check_password;
diff --git a/contrib/passwordcheck/sql/passwordcheck.sql b/contrib/passwordcheck/sql/passwordcheck.sql
index 5953ece5c26..21ad8d452b5 100644
--- a/contrib/passwordcheck/sql/passwordcheck.sql
+++ b/contrib/passwordcheck/sql/passwordcheck.sql
@@ -9,6 +9,10 @@ ALTER USER regress_passwordcheck_user1 PASSWORD 'a_nice_long_password';
-- error: too short
ALTER USER regress_passwordcheck_user1 PASSWORD 'tooshrt';
+-- ok
+SET passwordcheck.min_password_length = 6;
+ALTER USER regress_passwordcheck_user1 PASSWORD 'v_shrt';
+
-- error: contains user name
ALTER USER regress_passwordcheck_user1 PASSWORD 'xyzregress_passwordcheck_user1';
diff --git a/doc/src/sgml/passwordcheck.sgml b/doc/src/sgml/passwordcheck.sgml
index 601f4892272..7ea32410463 100644
--- a/doc/src/sgml/passwordcheck.sgml
+++ b/doc/src/sgml/passwordcheck.sgml
@@ -59,4 +59,41 @@
</para>
</caution>
+ <sect2 id="passwordcheck-configuration-parameters">
+ <title>Configuration Parameters</title>
+
+ <variablelist>
+ <varlistentry>
+ <term>
+ <varname>passwordcheck.min_password_length</varname> (<type>integer</type>)
+ <indexterm>
+ <primary><varname>passwordcheck.min_password_length</varname> configuration parameter</primary>
+ </indexterm>
+ </term>
+ <listitem>
+ <para>
+ The minimum acceptable password length in bytes. The default is 8. Only
+ superusers can change this setting.
+ </para>
+ <note>
+ <para>
+ This parameter has no effect if a user supplies a pre-encrypted
+ password.
+ </para>
+ </note>
+ </listitem>
+ </varlistentry>
+ </variablelist>
+
+ <para>
+ In ordinary usage, this parameter is set in
+ <filename>postgresql.conf</filename>, but superusers can alter it on-the-fly
+ within their own sessions. Typical usage might be:
+ </para>
+
+<programlisting>
+# postgresql.conf
+passwordcheck.min_password_length = 12
+</programlisting>
+ </sect2>
</sect1>
--
2.39.5 (Apple Git-154)
Attachments:
[text/plain] v12-0001-Add-passwordcheck.min_password_length.patch (6.2K, ../../Z3wvwLceb6brtR0P@nathan/2-v12-0001-Add-passwordcheck.min_password_length.patch)
download | inline diff:
From fd7d382fc463fc47ee1c18695d78a2203723f46b Mon Sep 17 00:00:00 2001
From: Nathan Bossart <[email protected]>
Date: Mon, 6 Jan 2025 13:29:58 -0600
Subject: [PATCH v12 1/1] Add passwordcheck.min_password_length.
This new parameter can be used to change the minimum allowed
password length (in bytes). Note that it has no effect if a user
supplies a pre-encrypted password.
Author: Emanuele Musella, Maurizio Boriani
Reviewed-by: Tomas Vondra, Bertrand Drouvot, Japin Li
Discussion: https://postgr.es/m/CA%2BugDNyYtHOtWCqVD3YkSVYDWD_1fO8Jm_ahsDGA5dXhbDPwrQ%40mail.gmail.com
---
.../passwordcheck/expected/passwordcheck.out | 4 ++
.../expected/passwordcheck_1.out | 4 ++
contrib/passwordcheck/passwordcheck.c | 24 ++++++++++--
contrib/passwordcheck/sql/passwordcheck.sql | 4 ++
doc/src/sgml/passwordcheck.sgml | 37 +++++++++++++++++++
5 files changed, 69 insertions(+), 4 deletions(-)
diff --git a/contrib/passwordcheck/expected/passwordcheck.out b/contrib/passwordcheck/expected/passwordcheck.out
index dfb2ccfe008..83472c76d27 100644
--- a/contrib/passwordcheck/expected/passwordcheck.out
+++ b/contrib/passwordcheck/expected/passwordcheck.out
@@ -6,6 +6,10 @@ ALTER USER regress_passwordcheck_user1 PASSWORD 'a_nice_long_password';
-- error: too short
ALTER USER regress_passwordcheck_user1 PASSWORD 'tooshrt';
ERROR: password is too short
+DETAIL: password must be at least "passwordcheck.min_password_length" (8) bytes long
+-- ok
+SET passwordcheck.min_password_length = 6;
+ALTER USER regress_passwordcheck_user1 PASSWORD 'v_shrt';
-- error: contains user name
ALTER USER regress_passwordcheck_user1 PASSWORD 'xyzregress_passwordcheck_user1';
ERROR: password must not contain user name
diff --git a/contrib/passwordcheck/expected/passwordcheck_1.out b/contrib/passwordcheck/expected/passwordcheck_1.out
index 9519d60a495..fb12ec45cc4 100644
--- a/contrib/passwordcheck/expected/passwordcheck_1.out
+++ b/contrib/passwordcheck/expected/passwordcheck_1.out
@@ -6,6 +6,10 @@ ALTER USER regress_passwordcheck_user1 PASSWORD 'a_nice_long_password';
-- error: too short
ALTER USER regress_passwordcheck_user1 PASSWORD 'tooshrt';
ERROR: password is too short
+DETAIL: password must be at least "passwordcheck.min_password_length" (8) bytes long
+-- ok
+SET passwordcheck.min_password_length = 6;
+ALTER USER regress_passwordcheck_user1 PASSWORD 'v_shrt';
-- error: contains user name
ALTER USER regress_passwordcheck_user1 PASSWORD 'xyzregress_passwordcheck_user1';
ERROR: password must not contain user name
diff --git a/contrib/passwordcheck/passwordcheck.c b/contrib/passwordcheck/passwordcheck.c
index 96ada5b98c6..3db42a5b99d 100644
--- a/contrib/passwordcheck/passwordcheck.c
+++ b/contrib/passwordcheck/passwordcheck.c
@@ -15,6 +15,7 @@
#include "postgres.h"
#include <ctype.h>
+#include <limits.h>
#ifdef USE_CRACKLIB
#include <crack.h>
@@ -29,8 +30,8 @@ PG_MODULE_MAGIC;
/* Saved hook value */
static check_password_hook_type prev_check_password_hook = NULL;
-/* passwords shorter than this will be rejected */
-#define MIN_PWD_LENGTH 8
+/* GUC variables */
+static int min_password_length = 8;
/*
* check_password
@@ -93,10 +94,12 @@ check_password(const char *username,
#endif
/* enforce minimum length */
- if (pwdlen < MIN_PWD_LENGTH)
+ if (pwdlen < min_password_length)
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("password is too short")));
+ errmsg("password is too short"),
+ errdetail("password must be at least \"passwordcheck.min_password_length\" (%d) bytes long",
+ min_password_length)));
/* check if the password contains the username */
if (strstr(password, username))
@@ -142,6 +145,19 @@ check_password(const char *username,
void
_PG_init(void)
{
+ /* Define custom GUC variables. */
+ DefineCustomIntVariable("passwordcheck.min_password_length",
+ "Minimum allowed password length.",
+ NULL,
+ &min_password_length,
+ 8,
+ 0, INT_MAX,
+ PGC_SUSET,
+ GUC_UNIT_BYTE,
+ NULL, NULL, NULL);
+
+ MarkGUCPrefixReserved("passwordcheck");
+
/* activate password checks when the module is loaded */
prev_check_password_hook = check_password_hook;
check_password_hook = check_password;
diff --git a/contrib/passwordcheck/sql/passwordcheck.sql b/contrib/passwordcheck/sql/passwordcheck.sql
index 5953ece5c26..21ad8d452b5 100644
--- a/contrib/passwordcheck/sql/passwordcheck.sql
+++ b/contrib/passwordcheck/sql/passwordcheck.sql
@@ -9,6 +9,10 @@ ALTER USER regress_passwordcheck_user1 PASSWORD 'a_nice_long_password';
-- error: too short
ALTER USER regress_passwordcheck_user1 PASSWORD 'tooshrt';
+-- ok
+SET passwordcheck.min_password_length = 6;
+ALTER USER regress_passwordcheck_user1 PASSWORD 'v_shrt';
+
-- error: contains user name
ALTER USER regress_passwordcheck_user1 PASSWORD 'xyzregress_passwordcheck_user1';
diff --git a/doc/src/sgml/passwordcheck.sgml b/doc/src/sgml/passwordcheck.sgml
index 601f4892272..7ea32410463 100644
--- a/doc/src/sgml/passwordcheck.sgml
+++ b/doc/src/sgml/passwordcheck.sgml
@@ -59,4 +59,41 @@
</para>
</caution>
+ <sect2 id="passwordcheck-configuration-parameters">
+ <title>Configuration Parameters</title>
+
+ <variablelist>
+ <varlistentry>
+ <term>
+ <varname>passwordcheck.min_password_length</varname> (<type>integer</type>)
+ <indexterm>
+ <primary><varname>passwordcheck.min_password_length</varname> configuration parameter</primary>
+ </indexterm>
+ </term>
+ <listitem>
+ <para>
+ The minimum acceptable password length in bytes. The default is 8. Only
+ superusers can change this setting.
+ </para>
+ <note>
+ <para>
+ This parameter has no effect if a user supplies a pre-encrypted
+ password.
+ </para>
+ </note>
+ </listitem>
+ </varlistentry>
+ </variablelist>
+
+ <para>
+ In ordinary usage, this parameter is set in
+ <filename>postgresql.conf</filename>, but superusers can alter it on-the-fly
+ within their own sessions. Typical usage might be:
+ </para>
+
+<programlisting>
+# postgresql.conf
+passwordcheck.min_password_length = 12
+</programlisting>
+ </sect2>
</sect1>
--
2.39.5 (Apple Git-154)
^ permalink raw reply [nested|flat] 30+ messages in thread
* Re: Parametrization minimum password lenght
2024-11-25 10:15 Re: Parametrization minimum password lenght Emanuele Musella <[email protected]>
2024-11-25 17:35 ` Re: Parametrization minimum password lenght Emanuele Musella <[email protected]>
2024-12-18 20:56 ` Re: Parametrization minimum password lenght Nathan Bossart <[email protected]>
2024-12-19 07:25 ` Re: Parametrization minimum password lenght Bertrand Drouvot <[email protected]>
2024-12-19 15:36 ` Re: Parametrization minimum password lenght Nathan Bossart <[email protected]>
2024-12-19 15:57 ` Re: Parametrization minimum password lenght Nathan Bossart <[email protected]>
2024-12-25 05:15 ` Re: Parametrization minimum password lenght Japin Li <[email protected]>
2025-01-06 19:32 ` Re: Parametrization minimum password lenght Nathan Bossart <[email protected]>
@ 2025-01-07 21:08 ` Nathan Bossart <[email protected]>
0 siblings, 0 replies; 30+ messages in thread
From: Nathan Bossart @ 2025-01-07 21:08 UTC (permalink / raw)
To: Japin Li <[email protected]>; +Cc: Bertrand Drouvot <[email protected]>; Emanuele Musella <[email protected]>; [email protected]
Committed.
--
nathan
^ permalink raw reply [nested|flat] 30+ messages in thread
* Re: Parametrization minimum password lenght
2024-11-25 10:15 Re: Parametrization minimum password lenght Emanuele Musella <[email protected]>
2024-11-25 17:35 ` Re: Parametrization minimum password lenght Emanuele Musella <[email protected]>
2024-12-18 20:56 ` Re: Parametrization minimum password lenght Nathan Bossart <[email protected]>
@ 2024-12-19 10:22 ` Emanuele Musella <[email protected]>
2024-12-19 15:40 ` Re: Parametrization minimum password lenght Nathan Bossart <[email protected]>
1 sibling, 1 reply; 30+ messages in thread
From: Emanuele Musella @ 2024-12-19 10:22 UTC (permalink / raw)
To: Nathan Bossart <[email protected]>; +Cc: [email protected]; Bertrand Drouvot <[email protected]>
Hi Nathan,
thank you.
It seems to me you are working on an older version of patch.
In attached the latest one for your reference.
Thanks for your support
Best regards
Emanuele Musella
Il giorno mer 18 dic 2024 alle ore 21:56 Nathan Bossart <
[email protected]> ha scritto:
> Here is what I have staged for commit.
>
> --
> nathan
>
Attachments:
[application/octet-stream] min_password_length_v9.patch (4.1K, ../../CA+ugDNx=c7MFQJHXaOaeYwz-viBjEt7EcY-apYFgHZoT4V76Og@mail.gmail.com/3-min_password_length_v9.patch)
download | inline diff:
diff --git a/contrib/passwordcheck/expected/passwordcheck.out b/contrib/passwordcheck/expected/passwordcheck.out
index 2027681daf..8f1f0baaab 100644
--- a/contrib/passwordcheck/expected/passwordcheck.out
+++ b/contrib/passwordcheck/expected/passwordcheck.out
@@ -1,9 +1,10 @@
LOAD 'passwordcheck';
+SET passwordcheck.min_password_length = 12;
CREATE USER regress_passwordcheck_user1;
-- ok
ALTER USER regress_passwordcheck_user1 PASSWORD 'a_nice_long_password';
-- error: too short
-ALTER USER regress_passwordcheck_user1 PASSWORD 'tooshrt';
+ALTER USER regress_passwordcheck_user1 PASSWORD 'tooshort';
ERROR: password is too short
-- error: contains user name
ALTER USER regress_passwordcheck_user1 PASSWORD 'xyzregress_passwordcheck_user1';
diff --git a/contrib/passwordcheck/passwordcheck.c b/contrib/passwordcheck/passwordcheck.c
index 0785618f2a..f9a2c2db33 100644
--- a/contrib/passwordcheck/passwordcheck.c
+++ b/contrib/passwordcheck/passwordcheck.c
@@ -20,17 +20,23 @@
#include <crack.h>
#endif
+#include "commands/explain.h"
#include "commands/user.h"
#include "fmgr.h"
#include "libpq/crypt.h"
+#include "mb/pg_wchar.h"
+#include "utils/guc.h"
PG_MODULE_MAGIC;
/* Saved hook value in case of unload */
static check_password_hook_type prev_check_password_hook = NULL;
-/* passwords shorter than this will be rejected */
-#define MIN_PWD_LENGTH 8
+/* GUC variables */
+static int min_pwd_len;
+
+/* Max password length */
+#define PG_MAX_PASSWORD_LENGTH 128
/*
* check_password
@@ -93,7 +99,7 @@ check_password(const char *username,
#endif
/* enforce minimum length */
- if (pwdlen < MIN_PWD_LENGTH)
+ if (pwdlen < min_pwd_len)
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
errmsg("password is too short")));
@@ -142,6 +148,21 @@ check_password(const char *username,
void
_PG_init(void)
{
+ /* Define custom GUC variables. */
+ DefineCustomIntVariable("passwordcheck.min_password_length",
+ "Sets the minimum allowed password length.",
+ NULL,
+ &min_pwd_len,
+ 8,
+ 8, PG_MAX_PASSWORD_LENGTH,
+ PGC_SUSET,
+ GUC_UNIT_BYTE,
+ NULL,
+ NULL,
+ NULL);
+
+ MarkGUCPrefixReserved("passwordcheck");
+
/* activate password checks when the module is loaded */
prev_check_password_hook = check_password_hook;
check_password_hook = check_password;
diff --git a/contrib/passwordcheck/sql/passwordcheck.sql b/contrib/passwordcheck/sql/passwordcheck.sql
index 1fbd6b0e96..c151685c74 100644
--- a/contrib/passwordcheck/sql/passwordcheck.sql
+++ b/contrib/passwordcheck/sql/passwordcheck.sql
@@ -1,12 +1,12 @@
LOAD 'passwordcheck';
-
+SET passwordcheck.min_password_length = 12;
CREATE USER regress_passwordcheck_user1;
-- ok
ALTER USER regress_passwordcheck_user1 PASSWORD 'a_nice_long_password';
-- error: too short
-ALTER USER regress_passwordcheck_user1 PASSWORD 'tooshrt';
+ALTER USER regress_passwordcheck_user1 PASSWORD 'tooshort';
-- error: contains user name
ALTER USER regress_passwordcheck_user1 PASSWORD 'xyzregress_passwordcheck_user1';
diff --git a/doc/src/sgml/passwordcheck.sgml b/doc/src/sgml/passwordcheck.sgml
index 601f489227..160553b8e9 100644
--- a/doc/src/sgml/passwordcheck.sgml
+++ b/doc/src/sgml/passwordcheck.sgml
@@ -59,4 +59,30 @@
</para>
</caution>
+ <sect2 id="passwordcheck-configuration-parameters">
+ <title>Configuration Parameters</title>
+
+ <para>
+ There is a configuration parameter that control the behavior of
+ <filename>passwordcheck</filename>. This is the minumum password length.
+ </para>
+
+ <variablelist>
+ <varlistentry id="passwordcheck-configuration-parameters-min-password-length">
+ <term>
+ <varname>passwordcheck.min_password_length</varname> (<type>integer</type>)
+ <indexterm>
+ <primary><varname>passwordcheck.min_password_length</varname> configuration parameter</primary>
+ </indexterm>
+ </term>
+ <listitem>
+ <para>
+ <varname>passwordcheck.min_password_length</varname> is the minimum length
+ of accepted password on database users.
+ If not setted the default is 8.
+ </para>
+ </listitem>
+ </varlistentry>
+ </variablelist>
+ </sect2>
</sect1>
^ permalink raw reply [nested|flat] 30+ messages in thread
* Re: Parametrization minimum password lenght
2024-11-25 10:15 Re: Parametrization minimum password lenght Emanuele Musella <[email protected]>
2024-11-25 17:35 ` Re: Parametrization minimum password lenght Emanuele Musella <[email protected]>
2024-12-18 20:56 ` Re: Parametrization minimum password lenght Nathan Bossart <[email protected]>
2024-12-19 10:22 ` Re: Parametrization minimum password lenght Emanuele Musella <[email protected]>
@ 2024-12-19 15:40 ` Nathan Bossart <[email protected]>
0 siblings, 0 replies; 30+ messages in thread
From: Nathan Bossart @ 2024-12-19 15:40 UTC (permalink / raw)
To: Emanuele Musella <[email protected]>; +Cc: [email protected]; Bertrand Drouvot <[email protected]>
On Thu, Dec 19, 2024 at 11:22:02AM +0100, Emanuele Musella wrote:
> It seems to me you are working on an older version of patch.
>
> In attached the latest one for your reference.
I used the v9 patch as the basis for v10. There are a few small edits,
such as removing the upper bound for the parameter and expanding the
documentation, but the feature itself remains intact.
--
nathan
^ permalink raw reply [nested|flat] 30+ messages in thread
end of thread, other threads:[~2025-01-07 21:08 UTC | newest]
Thread overview: 30+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2024-02-13 15:17 [PATCH v6 08/14] Reduce scope of BitmapHeapScan tbmiterator local variables Melanie Plageman <[email protected]>
2024-02-13 15:17 [PATCH v8 07/17] Reduce scope of BitmapHeapScan tbmiterator local variables Melanie Plageman <[email protected]>
2024-02-13 15:17 [PATCH v7 07/13] Reduce scope of BitmapHeapScan tbmiterator local variables Melanie Plageman <[email protected]>
2024-02-13 15:17 [PATCH v7 07/13] Reduce scope of BitmapHeapScan tbmiterator local variables Melanie Plageman <[email protected]>
2024-02-13 15:17 [PATCH v10 07/17] Reduce scope of BitmapHeapScan tbmiterator local variables Melanie Plageman <[email protected]>
2024-02-13 15:17 [PATCH v10 07/17] Reduce scope of BitmapHeapScan tbmiterator local variables Melanie Plageman <[email protected]>
2024-02-13 15:17 [PATCH v9 07/17] Reduce scope of BitmapHeapScan tbmiterator local variables Melanie Plageman <[email protected]>
2024-02-13 15:17 [PATCH v11 07/17] Reduce scope of BitmapHeapScan tbmiterator local variables Melanie Plageman <[email protected]>
2024-02-13 15:17 [PATCH v12 07/17] Reduce scope of BitmapHeapScan tbmiterator local variables Melanie Plageman <[email protected]>
2024-02-13 15:17 [PATCH v6 08/14] Reduce scope of BitmapHeapScan tbmiterator local variables Melanie Plageman <[email protected]>
2024-02-13 15:17 [PATCH v9 07/17] Reduce scope of BitmapHeapScan tbmiterator local variables Melanie Plageman <[email protected]>
2024-02-13 15:17 [PATCH v11 07/17] Reduce scope of BitmapHeapScan tbmiterator local variables Melanie Plageman <[email protected]>
2024-02-13 15:17 [PATCH v8 07/17] Reduce scope of BitmapHeapScan tbmiterator local variables Melanie Plageman <[email protected]>
2024-02-13 15:17 [PATCH v8 07/17] Reduce scope of BitmapHeapScan tbmiterator local variables Melanie Plageman <[email protected]>
2024-02-13 15:17 [PATCH v10 07/17] Reduce scope of BitmapHeapScan tbmiterator local variables Melanie Plageman <[email protected]>
2024-02-13 15:17 [PATCH v7 07/13] Reduce scope of BitmapHeapScan tbmiterator local variables Melanie Plageman <[email protected]>
2024-02-13 15:17 [PATCH v9 07/17] Reduce scope of BitmapHeapScan tbmiterator local variables Melanie Plageman <[email protected]>
2024-02-13 15:17 [PATCH v6 08/14] Reduce scope of BitmapHeapScan tbmiterator local variables Melanie Plageman <[email protected]>
2024-11-25 10:15 Re: Parametrization minimum password lenght Emanuele Musella <[email protected]>
2024-11-25 17:35 ` Re: Parametrization minimum password lenght Emanuele Musella <[email protected]>
2024-12-18 20:56 ` Re: Parametrization minimum password lenght Nathan Bossart <[email protected]>
2024-12-19 07:25 ` Re: Parametrization minimum password lenght Bertrand Drouvot <[email protected]>
2024-12-19 15:36 ` Re: Parametrization minimum password lenght Nathan Bossart <[email protected]>
2024-12-19 15:57 ` Re: Parametrization minimum password lenght Nathan Bossart <[email protected]>
2024-12-20 09:15 ` Re: Parametrization minimum password lenght Bertrand Drouvot <[email protected]>
2024-12-25 05:15 ` Re: Parametrization minimum password lenght Japin Li <[email protected]>
2025-01-06 19:32 ` Re: Parametrization minimum password lenght Nathan Bossart <[email protected]>
2025-01-07 21:08 ` Re: Parametrization minimum password lenght Nathan Bossart <[email protected]>
2024-12-19 10:22 ` Re: Parametrization minimum password lenght Emanuele Musella <[email protected]>
2024-12-19 15:40 ` Re: Parametrization minimum password lenght Nathan Bossart <[email protected]>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox