public inbox for [email protected]
help / color / mirror / Atom feed[PATCH v5 1/2] Report progress of CREATE INDEX operations
17+ messages / 7 participants
[nested] [flat]
* [PATCH v5 1/2] Report progress of CREATE INDEX operations
@ 2019-01-02 19:14 Alvaro Herrera <[email protected]>
0 siblings, 0 replies; 17+ messages in thread
From: Alvaro Herrera @ 2019-01-02 19:14 UTC (permalink / raw)
---
contrib/amcheck/verify_nbtree.c | 2 +-
contrib/bloom/blinsert.c | 2 +-
contrib/bloom/blutils.c | 1 +
doc/src/sgml/indexam.sgml | 13 ++
doc/src/sgml/monitoring.sgml | 227 +++++++++++++++++++++++++-
src/backend/access/brin/brin.c | 5 +-
src/backend/access/gin/gininsert.c | 2 +-
src/backend/access/gin/ginutil.c | 1 +
src/backend/access/gist/gist.c | 1 +
src/backend/access/gist/gistbuild.c | 2 +-
src/backend/access/hash/hash.c | 3 +-
src/backend/access/nbtree/nbtree.c | 9 +
src/backend/access/nbtree/nbtsort.c | 57 ++++++-
src/backend/access/nbtree/nbtutils.c | 24 +++
src/backend/access/spgist/spginsert.c | 2 +-
src/backend/access/spgist/spgutils.c | 1 +
src/backend/catalog/index.c | 147 ++++++++++++++++-
src/backend/catalog/system_views.sql | 27 +++
src/backend/commands/indexcmds.c | 52 +++++-
src/backend/storage/ipc/standby.c | 2 +-
src/backend/storage/lmgr/lmgr.c | 46 +++++-
src/backend/storage/lmgr/lock.c | 7 +-
src/backend/utils/adt/amutils.c | 23 +++
src/backend/utils/adt/pgstatfuncs.c | 2 +
src/include/access/amapi.h | 4 +
src/include/access/genam.h | 1 +
src/include/access/nbtree.h | 11 ++
src/include/catalog/index.h | 2 +
src/include/catalog/pg_proc.dat | 10 +-
src/include/commands/progress.h | 36 ++++
src/include/pgstat.h | 5 +-
src/include/storage/lmgr.h | 4 +-
src/include/storage/lock.h | 2 +-
33 files changed, 695 insertions(+), 38 deletions(-)
diff --git a/contrib/amcheck/verify_nbtree.c b/contrib/amcheck/verify_nbtree.c
index 964200a7678..99f6ed6bc44 100644
--- a/contrib/amcheck/verify_nbtree.c
+++ b/contrib/amcheck/verify_nbtree.c
@@ -534,7 +534,7 @@ bt_check_every_level(Relation rel, Relation heaprel, bool readonly,
RelationGetRelationName(state->rel),
RelationGetRelationName(state->heaprel));
- IndexBuildHeapScan(state->heaprel, state->rel, indexinfo, true,
+ IndexBuildHeapScan(state->heaprel, state->rel, indexinfo, true, false,
bt_tuple_present_callback, (void *) state, scan);
ereport(DEBUG1,
diff --git a/contrib/bloom/blinsert.c b/contrib/bloom/blinsert.c
index e43fbe0005f..947ee74881f 100644
--- a/contrib/bloom/blinsert.c
+++ b/contrib/bloom/blinsert.c
@@ -141,7 +141,7 @@ blbuild(Relation heap, Relation index, IndexInfo *indexInfo)
initCachedPage(&buildstate);
/* Do the heap scan */
- reltuples = IndexBuildHeapScan(heap, index, indexInfo, true,
+ reltuples = IndexBuildHeapScan(heap, index, indexInfo, true, false,
bloomBuildCallback, (void *) &buildstate,
NULL);
diff --git a/contrib/bloom/blutils.c b/contrib/bloom/blutils.c
index 64583765787..697a37a384b 100644
--- a/contrib/bloom/blutils.c
+++ b/contrib/bloom/blutils.c
@@ -132,6 +132,7 @@ blhandler(PG_FUNCTION_ARGS)
amroutine->amcostestimate = blcostestimate;
amroutine->amoptions = bloptions;
amroutine->amproperty = NULL;
+ amroutine->ambuildphasename = NULL;
amroutine->amvalidate = blvalidate;
amroutine->ambeginscan = blbeginscan;
amroutine->amrescan = blrescan;
diff --git a/doc/src/sgml/indexam.sgml b/doc/src/sgml/indexam.sgml
index 05102724ead..fa4a3d0d131 100644
--- a/doc/src/sgml/indexam.sgml
+++ b/doc/src/sgml/indexam.sgml
@@ -127,6 +127,7 @@ typedef struct IndexAmRoutine
amcostestimate_function amcostestimate;
amoptions_function amoptions;
amproperty_function amproperty; /* can be NULL */
+ ambuildphasename_function ambuildphasename; /* can be NULL */
amvalidate_function amvalidate;
ambeginscan_function ambeginscan;
amrescan_function amrescan;
@@ -468,6 +469,18 @@ amproperty (Oid index_oid, int attno,
<para>
<programlisting>
+char *
+ambuildphasename (int64 phasenum);
+</programlisting>
+ Return the textual name of the given build phase number.
+ The phase numbers are those reported during an index build via the
+ <function>pgstat_progress_update_param</function> interface.
+ The phase names are then exposed in the
+ <structname>pg_stat_progress_create_index</structname> view.
+ </para>
+
+ <para>
+<programlisting>
bool
amvalidate (Oid opclassoid);
</programlisting>
diff --git a/doc/src/sgml/monitoring.sgml b/doc/src/sgml/monitoring.sgml
index 0e73cdcddab..47708570f84 100644
--- a/doc/src/sgml/monitoring.sgml
+++ b/doc/src/sgml/monitoring.sgml
@@ -336,6 +336,14 @@ postgres 27093 0.0 0.0 30096 2752 ? Ss 11:34 0:00 postgres: ser
</entry>
</row>
+ <row>
+ <entry><structname>pg_stat_progress_create_index</structname><indexterm><primary>pg_stat_progress_create_index</primary></indexterm></entry>
+ <entry>One row for each backend running <command>CREATE INDEX</command>, showing
+ current progress.
+ See <xref linkend='create-index-progress-reporting'/>.
+ </entry>
+ </row>
+
<row>
<entry><structname>pg_stat_progress_vacuum</structname><indexterm><primary>pg_stat_progress_vacuum</primary></indexterm></entry>
<entry>One row for each backend (including autovacuum worker processes) running
@@ -3376,11 +3384,224 @@ SELECT pg_stat_get_backend_pid(s.backendid) AS pid,
<para>
<productname>PostgreSQL</productname> has the ability to report the progress of
- certain commands during command execution. Currently, the only command
- which supports progress reporting is <command>VACUUM</command>. This may be
- expanded in the future.
+ certain commands during command execution. Currently, the only commands
+ which support progress reporting are <command>CREATE INDEX</command> and
+ <command>VACUUM</command>. This may be expanded in the future.
</para>
+ <sect2 id="create-index-progress-reporting">
+ <title>CREATE INDEX Progress Reporting</title>
+
+ <para>
+ Whenever <command>CREATE INDEX</command> is running, the
+ <structname>pg_stat_progress_create_index</structname> view will contain
+ one row for each backend that is currently creating indexes. The tables
+ below describe the information that will be reported and provide information
+ about how to interpret it.
+ </para>
+
+ <table id="pg-stat-progress-create-index-view" xreflabel="pg_stat_progress_create_index">
+ <title><structname>pg_stat_progress_create_index</structname> View</title>
+ <tgroup cols="3">
+ <thead>
+ <row>
+ <entry>Column</entry>
+ <entry>Type</entry>
+ <entry>Description</entry>
+ </row>
+ </thead>
+
+ <tbody>
+ <row>
+ <entry><structfield>pid</structfield></entry>
+ <entry><type>integer</type></entry>
+ <entry>Process ID of backend.</entry>
+ </row>
+ <row>
+ <entry><structfield>datid</structfield></entry>
+ <entry><type>oid</type></entry>
+ <entry>OID of the database to which this backend is connected.</entry>
+ </row>
+ <row>
+ <entry><structfield>datname</structfield></entry>
+ <entry><type>name</type></entry>
+ <entry>Name of the database to which this backend is connected.</entry>
+ </row>
+ <row>
+ <entry><structfield>relid</structfield></entry>
+ <entry><type>oid</type></entry>
+ <entry>OID of the table on which the index is being created.</entry>
+ </row>
+ <row>
+ <entry><structfield>phase</structfield></entry>
+ <entry><type>text</type></entry>
+ <entry>
+ Current processing phase of index creation. See <xref linkend='create-index-phases'/>.
+ </entry>
+ </row>
+ <row>
+ <entry><structfield>lockers_total</structfield></entry>
+ <entry><type>bigint</type></entry>
+ <entry>
+ Total number of lockers to wait for, when applicable.
+ </entry>
+ </row>
+ <row>
+ <entry><structfield>lockers_done</structfield></entry>
+ <entry><type>bigint</type></entry>
+ <entry>
+ Number of lockers already waited for.
+ </entry>
+ </row>
+ <row>
+ <entry><structfield>current_locked_pid</structfield></entry>
+ <entry><type>bigint</type></entry>
+ <entry>
+ Process ID of the locker currently being waited for.
+ </entry>
+ </row>
+ <row>
+ <entry><structfield>blocks_total</structfield></entry>
+ <entry><type>bigint</type></entry>
+ <entry>
+ Total number of blocks to be processed in the current phase.
+ </entry>
+ </row>
+ <row>
+ <entry><structfield>blocks_done</structfield></entry>
+ <entry><type>bigint</type></entry>
+ <entry>
+ Number of blocks already processed in the current phase.
+ </entry>
+ </row>
+ <row>
+ <entry><structfield>tuples_total</structfield></entry>
+ <entry><type>bigint</type></entry>
+ <entry>
+ Total number of tuples to be processed in the current phase.
+ </entry>
+ </row>
+ <row>
+ <entry><structfield>tuples_done</structfield></entry>
+ <entry><type>bigint</type></entry>
+ <entry>
+ Number of tuples already processed in the current phase.
+ </entry>
+ </row>
+ <row>
+ <entry><structfield>partitions_total</structfield></entry>
+ <entry><type>bigint</type></entry>
+ <entry>
+ When creating an index on a partitioned, this column is set to the
+ total number of partitions on which the index is to be created.
+ </entry>
+ </row>
+ <row>
+ <entry><structfield>partitions_done</structfield></entry>
+ <entry><type>bigint</type></entry>
+ <entry>
+ When creating an index on a partitioned, this column is set to the
+ number of partitions on which the index has been completed.
+ </entry>
+ </row>
+ </tbody>
+ </tgroup>
+ </table>
+
+ <table id="create-index-phases">
+ <title>CREATE INDEX phases</title>
+ <tgroup cols="2">
+ <thead>
+ <row>
+ <entry>Phase</entry>
+ <entry>Description</entry>
+ </row>
+ </thead>
+ <tbody>
+ <row>
+ <entry><literal>initializing</literal></entry>
+ <entry>
+ <command>CREATE INDEX</command> is preparing to create the index. This
+ phase is expected to be very brief.
+ </entry>
+ </row>
+ <row>
+ <entry><literal>waiting for old snapshots</literal></entry>
+ <entry>
+ <command>CREATE INDEX CONCURRENTLY</command> is waiting for transactions
+ that can potentially see the table to release their snapshots.
+ This phase is skipped when not in concurrent mode.
+ Columns <structname>lockers_total</structname>, <structname>lockers_done</structname>
+ and <structname>current_locker_pid</structname> contain the progress
+ information for this phase.
+ </entry>
+ </row>
+ <row>
+ <entry><literal>building index</literal></entry>
+ <entry>
+ The index is being built by the access method-specific code. In this phase,
+ access methods that support progress reporting fill in their own progress data,
+ and the subphase is indicated in this column. Typically,
+ <structname>blocks_total</structname> and <structname>blocks_done</structname>
+ will contain progress data, as well as potentially
+ <structname>tuples_total</structname> and <structname>tuples_done</structname>.
+ </entry>
+ </row>
+ <row>
+ <entry><literal>waiting for writer snapshots</literal></entry>
+ <entry>
+ <command>CREATE INDEX CONCURRENTLY</command> is waiting for transactions
+ that can potentially write into the table to release their snapshots.
+ This phase is skipped when not in concurrent mode.
+ Columns <structname>lockers_total</structname>, <structname>lockers_done</structname>
+ and <structname>current_locker_pid</structname> contain the progress
+ information for this phase.
+ </entry>
+ </row>
+ <row>
+ <entry><literal>validating index scan</literal></entry>
+ <entry>
+ <command>CREATE INDEX CONCURRENTLY</command> is scanning the index searching
+ for tuples that need to be validated.
+ This phase is skipped when not in concurrent mode.
+ Columns <structname>blocks_total</structname> (set to the total size of the index)
+ and <structname>blocks_done</structname> contain the progress information for this phase.
+ </entry>
+ </row>
+ <row>
+ <entry><literal>sorting index scan results</literal></entry>
+ <entry>
+ <command>CREATE INDEX CONCURRENTLY</command> is sorting the output of the
+ previous phase.
+ </entry>
+ </row>
+ <row>
+ <entry><literal>validating index heapscan</literal></entry>
+ <entry>
+ <command>CREATE INDEX CONCURRENTLY</command> is scanning the table
+ to validate the index tuples collected in the previous two phases.
+ This phase is skipped when not in concurrent mode.
+ Columns <structname>blocks_total</structname> (set to the total size of the table)
+ and <structname>blocks_done</structname> contain the progress information for this phase.
+ </entry>
+ </row>
+ <row>
+ <entry><literal>waiting for reader snapshots</literal></entry>
+ <entry>
+ <command>CREATE INDEX CONCURRENTLY</command> is waiting for transactions
+ that can potentially see the table to release their snapshots. This
+ phase is skipped when not in concurrent mode.
+ Columns <structname>lockers_total</structname>, <structname>lockers_done</structname>
+ and <structname>current_locker_pid</structname> contain the progress
+ information for this phase.
+ </entry>
+ </row>
+ </tbody>
+ </tgroup>
+ </table>
+
+ </sect2>
+
<sect2 id="vacuum-progress-reporting">
<title>VACUUM Progress Reporting</title>
diff --git a/src/backend/access/brin/brin.c b/src/backend/access/brin/brin.c
index 8f008dd0080..5d1aff34080 100644
--- a/src/backend/access/brin/brin.c
+++ b/src/backend/access/brin/brin.c
@@ -111,6 +111,7 @@ brinhandler(PG_FUNCTION_ARGS)
amroutine->amcostestimate = brincostestimate;
amroutine->amoptions = brinoptions;
amroutine->amproperty = NULL;
+ amroutine->ambuildphasename = NULL;
amroutine->amvalidate = brinvalidate;
amroutine->ambeginscan = brinbeginscan;
amroutine->amrescan = brinrescan;
@@ -718,7 +719,7 @@ brinbuild(Relation heap, Relation index, IndexInfo *indexInfo)
* Now scan the relation. No syncscan allowed here because we want the
* heap blocks in physical order.
*/
- reltuples = IndexBuildHeapScan(heap, index, indexInfo, false,
+ reltuples = IndexBuildHeapScan(heap, index, indexInfo, false, false,
brinbuildCallback, (void *) state, NULL);
/* process the final batch */
@@ -1234,7 +1235,7 @@ summarize_range(IndexInfo *indexInfo, BrinBuildState *state, Relation heapRel,
* by transactions that are still in progress, among other corner cases.
*/
state->bs_currRangeStart = heapBlk;
- IndexBuildHeapRangeScan(heapRel, state->bs_irel, indexInfo, false, true,
+ IndexBuildHeapRangeScan(heapRel, state->bs_irel, indexInfo, false, true, false,
heapBlk, scanNumBlks,
brinbuildCallback, (void *) state, NULL);
diff --git a/src/backend/access/gin/gininsert.c b/src/backend/access/gin/gininsert.c
index 524ac5be8b5..838de4c1ec3 100644
--- a/src/backend/access/gin/gininsert.c
+++ b/src/backend/access/gin/gininsert.c
@@ -394,7 +394,7 @@ ginbuild(Relation heap, Relation index, IndexInfo *indexInfo)
* Do the heap scan. We disallow sync scan here because dataPlaceToPage
* prefers to receive tuples in TID order.
*/
- reltuples = IndexBuildHeapScan(heap, index, indexInfo, false,
+ reltuples = IndexBuildHeapScan(heap, index, indexInfo, false, false,
ginBuildCallback, (void *) &buildstate, NULL);
/* dump remaining entries to the index */
diff --git a/src/backend/access/gin/ginutil.c b/src/backend/access/gin/ginutil.c
index afc20232ace..d2360eeafb0 100644
--- a/src/backend/access/gin/ginutil.c
+++ b/src/backend/access/gin/ginutil.c
@@ -64,6 +64,7 @@ ginhandler(PG_FUNCTION_ARGS)
amroutine->amcostestimate = gincostestimate;
amroutine->amoptions = ginoptions;
amroutine->amproperty = NULL;
+ amroutine->ambuildphasename = NULL;
amroutine->amvalidate = ginvalidate;
amroutine->ambeginscan = ginbeginscan;
amroutine->amrescan = ginrescan;
diff --git a/src/backend/access/gist/gist.c b/src/backend/access/gist/gist.c
index b75b3a8dacd..0dc36af1e0c 100644
--- a/src/backend/access/gist/gist.c
+++ b/src/backend/access/gist/gist.c
@@ -87,6 +87,7 @@ gisthandler(PG_FUNCTION_ARGS)
amroutine->amcostestimate = gistcostestimate;
amroutine->amoptions = gistoptions;
amroutine->amproperty = gistproperty;
+ amroutine->ambuildphasename = NULL;
amroutine->amvalidate = gistvalidate;
amroutine->ambeginscan = gistbeginscan;
amroutine->amrescan = gistrescan;
diff --git a/src/backend/access/gist/gistbuild.c b/src/backend/access/gist/gistbuild.c
index bd142a3560d..015f874cc93 100644
--- a/src/backend/access/gist/gistbuild.c
+++ b/src/backend/access/gist/gistbuild.c
@@ -204,7 +204,7 @@ gistbuild(Relation heap, Relation index, IndexInfo *indexInfo)
/*
* Do the heap scan.
*/
- reltuples = IndexBuildHeapScan(heap, index, indexInfo, true,
+ reltuples = IndexBuildHeapScan(heap, index, indexInfo, true, false,
gistBuildCallback, (void *) &buildstate, NULL);
/*
diff --git a/src/backend/access/hash/hash.c b/src/backend/access/hash/hash.c
index f1f01a0956d..fc7db5d6a13 100644
--- a/src/backend/access/hash/hash.c
+++ b/src/backend/access/hash/hash.c
@@ -82,6 +82,7 @@ hashhandler(PG_FUNCTION_ARGS)
amroutine->amcostestimate = hashcostestimate;
amroutine->amoptions = hashoptions;
amroutine->amproperty = NULL;
+ amroutine->ambuildphasename = NULL;
amroutine->amvalidate = hashvalidate;
amroutine->ambeginscan = hashbeginscan;
amroutine->amrescan = hashrescan;
@@ -159,7 +160,7 @@ hashbuild(Relation heap, Relation index, IndexInfo *indexInfo)
buildstate.heapRel = heap;
/* do the heap scan */
- reltuples = IndexBuildHeapScan(heap, index, indexInfo, true,
+ reltuples = IndexBuildHeapScan(heap, index, indexInfo, true, false,
hashbuildCallback, (void *) &buildstate, NULL);
if (buildstate.spool)
diff --git a/src/backend/access/nbtree/nbtree.c b/src/backend/access/nbtree/nbtree.c
index 98917de2efd..f9d32c958f6 100644
--- a/src/backend/access/nbtree/nbtree.c
+++ b/src/backend/access/nbtree/nbtree.c
@@ -22,6 +22,7 @@
#include "access/nbtxlog.h"
#include "access/relscan.h"
#include "access/xlog.h"
+#include "commands/progress.h"
#include "commands/vacuum.h"
#include "miscadmin.h"
#include "nodes/execnodes.h"
@@ -133,6 +134,7 @@ bthandler(PG_FUNCTION_ARGS)
amroutine->amcostestimate = btcostestimate;
amroutine->amoptions = btoptions;
amroutine->amproperty = btproperty;
+ amroutine->ambuildphasename = btbuildphasename;
amroutine->amvalidate = btvalidate;
amroutine->ambeginscan = btbeginscan;
amroutine->amrescan = btrescan;
@@ -1021,6 +1023,10 @@ btvacuumscan(IndexVacuumInfo *info, IndexBulkDeleteResult *stats,
if (needLock)
UnlockRelationForExtension(rel, ExclusiveLock);
+ if (info->report_progress)
+ pgstat_progress_update_param(PROGRESS_SCAN_BLOCKS_TOTAL,
+ num_pages);
+
/* Quit if we've scanned the whole relation */
if (blkno >= num_pages)
break;
@@ -1028,6 +1034,9 @@ btvacuumscan(IndexVacuumInfo *info, IndexBulkDeleteResult *stats,
for (; blkno < num_pages; blkno++)
{
btvacuumpage(&vstate, blkno, blkno);
+ if (info->report_progress)
+ pgstat_progress_update_param(PROGRESS_SCAN_BLOCKS_DONE,
+ blkno);
}
}
diff --git a/src/backend/access/nbtree/nbtsort.c b/src/backend/access/nbtree/nbtsort.c
index dc398e11867..309a2082063 100644
--- a/src/backend/access/nbtree/nbtsort.c
+++ b/src/backend/access/nbtree/nbtsort.c
@@ -65,6 +65,7 @@
#include "access/xlog.h"
#include "access/xloginsert.h"
#include "catalog/index.h"
+#include "commands/progress.h"
#include "miscadmin.h"
#include "pgstat.h"
#include "storage/smgr.h"
@@ -288,7 +289,8 @@ static double _bt_parallel_heapscan(BTBuildState *buildstate,
static void _bt_leader_participate_as_worker(BTBuildState *buildstate);
static void _bt_parallel_scan_and_sort(BTSpool *btspool, BTSpool *btspool2,
BTShared *btshared, Sharedsort *sharedsort,
- Sharedsort *sharedsort2, int sortmem);
+ Sharedsort *sharedsort2, int sortmem,
+ bool progress);
/*
@@ -384,6 +386,10 @@ _bt_spools_heapscan(Relation heap, Relation index, BTBuildState *buildstate,
/* Save as primary spool */
buildstate->spool = btspool;
+ /* Report heap scan phase started */
+ pgstat_progress_update_param(PROGRESS_CREATEIDX_SUBPHASE,
+ PROGRESS_BTREE_PHASE_INDEXBUILD_HEAPSCAN);
+
/* Attempt to launch parallel worker scan when required */
if (indexInfo->ii_ParallelWorkers > 0)
_bt_begin_parallel(buildstate, indexInfo->ii_Concurrent,
@@ -470,13 +476,31 @@ _bt_spools_heapscan(Relation heap, Relation index, BTBuildState *buildstate,
/* Fill spool using either serial or parallel heap scan */
if (!buildstate->btleader)
- reltuples = IndexBuildHeapScan(heap, index, indexInfo, true,
+ reltuples = IndexBuildHeapScan(heap, index, indexInfo, true, true,
_bt_build_callback, (void *) buildstate,
NULL);
else
reltuples = _bt_parallel_heapscan(buildstate,
&indexInfo->ii_BrokenHotChain);
+ /*
+ * Set the progress target for the next phase. Reset the block number
+ * values set by IndexBuildHeapScan
+ */
+ {
+ const int index[] = {
+ PROGRESS_CREATEIDX_TUPLES_TOTAL,
+ PROGRESS_SCAN_BLOCKS_TOTAL,
+ PROGRESS_SCAN_BLOCKS_DONE
+ };
+ const int64 val[] = {
+ buildstate->indtuples,
+ 0, 0
+ };
+
+ pgstat_progress_update_multi_param(3, index, val);
+ }
+
/* okay, all heap tuples are spooled */
if (buildstate->spool2 && !buildstate->havedead)
{
@@ -525,9 +549,15 @@ _bt_leafbuild(BTSpool *btspool, BTSpool *btspool2)
}
#endif /* BTREE_BUILD_STATS */
+ pgstat_progress_update_param(PROGRESS_CREATEIDX_SUBPHASE,
+ PROGRESS_BTREE_PHASE_PERFORMSORT_1);
tuplesort_performsort(btspool->sortstate);
if (btspool2)
+ {
+ pgstat_progress_update_param(PROGRESS_CREATEIDX_SUBPHASE,
+ PROGRESS_BTREE_PHASE_PERFORMSORT_2);
tuplesort_performsort(btspool2->sortstate);
+ }
wstate.heap = btspool->heap;
wstate.index = btspool->index;
@@ -543,6 +573,8 @@ _bt_leafbuild(BTSpool *btspool, BTSpool *btspool2)
wstate.btws_pages_written = 0;
wstate.btws_zeropage = NULL; /* until needed */
+ pgstat_progress_update_param(PROGRESS_CREATEIDX_SUBPHASE,
+ PROGRESS_BTREE_PHASE_LEAF_LOAD);
_bt_load(&wstate, btspool, btspool2);
}
@@ -1078,6 +1110,7 @@ _bt_load(BTWriteState *wstate, BTSpool *btspool, BTSpool *btspool2)
keysz = IndexRelationGetNumberOfKeyAttributes(wstate->index);
ScanKey indexScanKey = NULL;
SortSupport sortKeys;
+ long tuples_done = 0L;
if (merge)
{
@@ -1170,6 +1203,10 @@ _bt_load(BTWriteState *wstate, BTSpool *btspool, BTSpool *btspool2)
_bt_buildadd(wstate, state, itup2);
itup2 = tuplesort_getindextuple(btspool2->sortstate, true);
}
+
+ /* Report progress */
+ pgstat_progress_update_param(PROGRESS_CREATEIDX_TUPLES_DONE,
+ ++tuples_done);
}
pfree(sortKeys);
}
@@ -1184,6 +1221,10 @@ _bt_load(BTWriteState *wstate, BTSpool *btspool, BTSpool *btspool2)
state = _bt_pagestate(wstate, 0);
_bt_buildadd(wstate, state, itup);
+
+ /* Report progress */
+ pgstat_progress_update_param(PROGRESS_CREATEIDX_TUPLES_DONE,
+ ++tuples_done);
}
}
@@ -1318,6 +1359,10 @@ _bt_begin_parallel(BTBuildState *buildstate, bool isconcurrent, int request)
btshared->brokenhotchain = false;
heap_parallelscan_initialize(&btshared->heapdesc, btspool->heap, snapshot);
+ /* Report total number of blocks to scan */
+ pgstat_progress_update_param(PROGRESS_SCAN_BLOCKS_TOTAL,
+ btshared->heapdesc.phs_nblocks);
+
/*
* Store shared tuplesort-private state, for which we reserved space.
* Then, initialize opaque state using tuplesort routine.
@@ -1493,7 +1538,7 @@ _bt_leader_participate_as_worker(BTBuildState *buildstate)
/* Perform work common to all participants */
_bt_parallel_scan_and_sort(leaderworker, leaderworker2, btleader->btshared,
btleader->sharedsort, btleader->sharedsort2,
- sortmem);
+ sortmem, true);
#ifdef BTREE_BUILD_STATS
if (log_btree_build_stats)
@@ -1584,7 +1629,7 @@ _bt_parallel_build_main(dsm_segment *seg, shm_toc *toc)
/* Perform sorting of spool, and possibly a spool2 */
sortmem = maintenance_work_mem / btshared->scantuplesortstates;
_bt_parallel_scan_and_sort(btspool, btspool2, btshared, sharedsort,
- sharedsort2, sortmem);
+ sharedsort2, sortmem, false);
#ifdef BTREE_BUILD_STATS
if (log_btree_build_stats)
@@ -1613,7 +1658,7 @@ _bt_parallel_build_main(dsm_segment *seg, shm_toc *toc)
static void
_bt_parallel_scan_and_sort(BTSpool *btspool, BTSpool *btspool2,
BTShared *btshared, Sharedsort *sharedsort,
- Sharedsort *sharedsort2, int sortmem)
+ Sharedsort *sharedsort2, int sortmem, bool progress)
{
SortCoordinate coordinate;
BTBuildState buildstate;
@@ -1672,7 +1717,7 @@ _bt_parallel_scan_and_sort(BTSpool *btspool, BTSpool *btspool2,
indexInfo->ii_Concurrent = btshared->isconcurrent;
scan = heap_beginscan_parallel(btspool->heap, &btshared->heapdesc);
reltuples = IndexBuildHeapScan(btspool->heap, btspool->index, indexInfo,
- true, _bt_build_callback,
+ true, progress, _bt_build_callback,
(void *) &buildstate, scan);
/*
diff --git a/src/backend/access/nbtree/nbtutils.c b/src/backend/access/nbtree/nbtutils.c
index 2c05fb5e451..295c22b2e5b 100644
--- a/src/backend/access/nbtree/nbtutils.c
+++ b/src/backend/access/nbtree/nbtutils.c
@@ -20,6 +20,7 @@
#include "access/nbtree.h"
#include "access/reloptions.h"
#include "access/relscan.h"
+#include "commands/progress.h"
#include "miscadmin.h"
#include "utils/array.h"
#include "utils/lsyscache.h"
@@ -2082,6 +2083,29 @@ btproperty(Oid index_oid, int attno,
}
}
+/*
+ * btbuildphasename() -- Return name of index build phase.
+ */
+char *
+btbuildphasename(int64 phasenum)
+{
+ switch (phasenum)
+ {
+ case PROGRESS_CREATEIDX_SUBPHASE_INITIALIZE:
+ return "initializing (1 of 5)";
+ case PROGRESS_BTREE_PHASE_INDEXBUILD_HEAPSCAN:
+ return "table scan (2 of 5)";
+ case PROGRESS_BTREE_PHASE_PERFORMSORT_1:
+ return "sorting tuples, spool 1 (3 of 5)";
+ case PROGRESS_BTREE_PHASE_PERFORMSORT_2:
+ return "sorting tuples, spool 2 (4 of 5)";
+ case PROGRESS_BTREE_PHASE_LEAF_LOAD:
+ return "btree tuple loading (5 of 5)";
+ default:
+ return NULL;
+ }
+}
+
/*
* _bt_nonkey_truncate() -- create tuple without non-key suffix attributes.
*
diff --git a/src/backend/access/spgist/spginsert.c b/src/backend/access/spgist/spginsert.c
index f428a151385..1bc671c7238 100644
--- a/src/backend/access/spgist/spginsert.c
+++ b/src/backend/access/spgist/spginsert.c
@@ -142,7 +142,7 @@ spgbuild(Relation heap, Relation index, IndexInfo *indexInfo)
"SP-GiST build temporary context",
ALLOCSET_DEFAULT_SIZES);
- reltuples = IndexBuildHeapScan(heap, index, indexInfo, true,
+ reltuples = IndexBuildHeapScan(heap, index, indexInfo, true, false,
spgistBuildCallback, (void *) &buildstate,
NULL);
diff --git a/src/backend/access/spgist/spgutils.c b/src/backend/access/spgist/spgutils.c
index 8e63c1fad25..45472db147b 100644
--- a/src/backend/access/spgist/spgutils.c
+++ b/src/backend/access/spgist/spgutils.c
@@ -67,6 +67,7 @@ spghandler(PG_FUNCTION_ARGS)
amroutine->amcostestimate = spgcostestimate;
amroutine->amoptions = spgoptions;
amroutine->amproperty = spgproperty;
+ amroutine->ambuildphasename = NULL;
amroutine->amvalidate = spgvalidate;
amroutine->ambeginscan = spgbeginscan;
amroutine->amrescan = spgrescan;
diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c
index d16c3d0ea50..110f13d1e46 100644
--- a/src/backend/catalog/index.c
+++ b/src/backend/catalog/index.c
@@ -49,8 +49,9 @@
#include "catalog/pg_trigger.h"
#include "catalog/pg_type.h"
#include "catalog/storage.h"
-#include "commands/tablecmds.h"
#include "commands/event_trigger.h"
+#include "commands/progress.h"
+#include "commands/tablecmds.h"
#include "commands/trigger.h"
#include "executor/executor.h"
#include "miscadmin.h"
@@ -58,6 +59,7 @@
#include "nodes/nodeFuncs.h"
#include "optimizer/optimizer.h"
#include "parser/parser.h"
+#include "pgstat.h"
#include "rewrite/rewriteManip.h"
#include "storage/bufmgr.h"
#include "storage/lmgr.h"
@@ -1597,7 +1599,7 @@ index_drop(Oid indexId, bool concurrent)
* to acquire an exclusive lock on our table. The lock code will
* detect deadlock and error out properly.
*/
- WaitForLockers(heaplocktag, AccessExclusiveLock);
+ WaitForLockers(heaplocktag, AccessExclusiveLock, true);
/*
* No more predicate locks will be acquired on this index, and we're
@@ -1641,7 +1643,7 @@ index_drop(Oid indexId, bool concurrent)
* Wait till every transaction that saw the old index state has
* finished.
*/
- WaitForLockers(heaplocktag, AccessExclusiveLock);
+ WaitForLockers(heaplocktag, AccessExclusiveLock, true);
/*
* Re-open relations to allow us to complete our actions.
@@ -2291,6 +2293,25 @@ index_build(Relation heapRelation,
save_sec_context | SECURITY_RESTRICTED_OPERATION);
save_nestlevel = NewGUCNestLevel();
+ /* Set up initial progress report status */
+ {
+ const int index[] = {
+ PROGRESS_CREATEIDX_PHASE,
+ PROGRESS_CREATEIDX_SUBPHASE,
+ PROGRESS_CREATEIDX_TUPLES_DONE,
+ PROGRESS_CREATEIDX_TUPLES_TOTAL,
+ PROGRESS_SCAN_BLOCKS_DONE,
+ PROGRESS_SCAN_BLOCKS_TOTAL
+ };
+ const int64 val[] = {
+ PROGRESS_CREATEIDX_PHASE_BUILD,
+ PROGRESS_CREATEIDX_SUBPHASE_INITIALIZE,
+ 0, 0, 0, 0
+ };
+
+ pgstat_progress_update_multi_param(6, index, val);
+ }
+
/*
* Call the access method's build procedure
*/
@@ -2417,6 +2438,12 @@ index_build(Relation heapRelation,
* do so here because the AM might reject some of the tuples for its own
* reasons, such as being unable to store NULLs.
*
+ * If 'progress', we update the PROGRESS_SCAN_BLOCKS_DONE counter as we go
+ * along. Also, if that flag is true and a scan descriptor is not passed (ie.
+ * when not doing a parallel scan), the PROGRESS_SCAN_BLOCKS_TOTAL counter is
+ * updated at the beginning. For parallel scans, caller is expected to have
+ * set the total number of blocks prior to calling this function.
+ *
* A side effect is to set indexInfo->ii_BrokenHotChain to true if we detect
* any potentially broken HOT chains. Currently, we set this if there are
* any RECENTLY_DEAD or DELETE_IN_PROGRESS entries in a HOT chain, without
@@ -2428,13 +2455,14 @@ IndexBuildHeapScan(Relation heapRelation,
Relation indexRelation,
IndexInfo *indexInfo,
bool allow_sync,
+ bool progress,
IndexBuildCallback callback,
void *callback_state,
HeapScanDesc scan)
{
return IndexBuildHeapRangeScan(heapRelation, indexRelation,
indexInfo, allow_sync,
- false,
+ false, progress,
0, InvalidBlockNumber,
callback, callback_state, scan);
}
@@ -2445,6 +2473,13 @@ IndexBuildHeapScan(Relation heapRelation,
* passing InvalidBlockNumber as numblocks. Note that restricting the range
* to scan cannot be done when requesting syncscan.
*
+ * If 'progress', we update the PROGRESS_SCAN_BLOCKS_DONE counter as we go
+ * along. Also, if that flag is true and a scan descriptor is not passed (ie.
+ * when not doing a parallel scan), the PROGRESS_SCAN_BLOCKS_TOTAL counter is
+ * updated at the beginning. For parallel scans, caller is expected to have
+ * set the total number of blocks prior to calling this function. Note this
+ * only works for full-relation scans.
+ *
* When "anyvisible" mode is requested, all tuples visible to any transaction
* are indexed and counted as live, including those inserted or deleted by
* transactions that are still in progress.
@@ -2455,6 +2490,7 @@ IndexBuildHeapRangeScan(Relation heapRelation,
IndexInfo *indexInfo,
bool allow_sync,
bool anyvisible,
+ bool progress,
BlockNumber start_blockno,
BlockNumber numblocks,
IndexBuildCallback callback,
@@ -2476,6 +2512,8 @@ IndexBuildHeapRangeScan(Relation heapRelation,
TransactionId OldestXmin;
BlockNumber root_blkno = InvalidBlockNumber;
OffsetNumber root_offsets[MaxHeapTuplesPerPage];
+ BlockNumber blocks_done = 0;
+ BlockNumber previous_blkno = InvalidBlockNumber;
/*
* sanity checks
@@ -2545,6 +2583,13 @@ IndexBuildHeapRangeScan(Relation heapRelation,
NULL, /* scan key */
true, /* buffer access strategy OK */
allow_sync); /* syncscan OK? */
+
+ if (progress)
+ {
+ Assert(scan->rs_numblocks == InvalidBlockNumber);
+ pgstat_progress_update_param(PROGRESS_SCAN_BLOCKS_TOTAL,
+ scan->rs_nblocks);
+ }
}
else
{
@@ -2592,6 +2637,46 @@ IndexBuildHeapRangeScan(Relation heapRelation,
CHECK_FOR_INTERRUPTS();
+ /* Report scan progress, if asked to. */
+ if (progress &&
+ ((previous_blkno == InvalidBlockNumber) ||
+ (scan->rs_cblock != previous_blkno)))
+ {
+ /* we only do progress for full table scans */
+ Assert(numblocks == InvalidBlockNumber);
+
+ /*
+ * Report the number of blocks we've moved forward.
+ *
+ * Parallel workers cause the leader process to skip some blocks,
+ * so we subtract the current block number to the previous one to
+ * determine how many have been read in total; but be careful when
+ * we wrap around the last block in the table.
+ */
+ if (scan->rs_cblock > previous_blkno)
+ blocks_done += scan->rs_cblock - previous_blkno;
+ else if (previous_blkno == InvalidBlockNumber)
+ {
+ /*
+ * How many blocks have been read since the scan started.
+ * Should normally be zero.
+ */
+ blocks_done += scan->rs_cblock -
+ (scan->rs_parallel ? scan->rs_parallel->phs_startblock :
+ scan->rs_startblock);
+ Assert(blocks_done == 0);
+ }
+ else
+ {
+ /* wrapped around */
+ blocks_done += scan->rs_nblocks - previous_blkno + scan->rs_cblock;
+ }
+
+ pgstat_progress_update_param(PROGRESS_SCAN_BLOCKS_DONE,
+ blocks_done);
+ previous_blkno = scan->rs_cblock;
+ }
+
/*
* When dealing with a HOT-chain of updated tuples, we want to index
* the values of the live tuple (if any), but index it under the TID
@@ -3133,6 +3218,21 @@ validate_index(Oid heapId, Oid indexId, Snapshot snapshot)
int save_sec_context;
int save_nestlevel;
+ {
+ const int index[] = {
+ PROGRESS_CREATEIDX_PHASE,
+ PROGRESS_CREATEIDX_TUPLES_DONE,
+ PROGRESS_CREATEIDX_TUPLES_TOTAL,
+ PROGRESS_SCAN_BLOCKS_DONE,
+ PROGRESS_SCAN_BLOCKS_TOTAL
+ };
+ const int64 val[] = {
+ PROGRESS_CREATEIDX_PHASE_VALIDATE_IDXSCAN,
+ 0, 0, 0, 0
+ };
+ pgstat_progress_update_multi_param(5, index, val);
+ }
+
/* Open and lock the parent heap relation */
heapRelation = table_open(heapId, ShareUpdateExclusiveLock);
/* And the target index relation */
@@ -3163,6 +3263,7 @@ validate_index(Oid heapId, Oid indexId, Snapshot snapshot)
*/
ivinfo.index = indexRelation;
ivinfo.analyze_only = false;
+ ivinfo.report_progress = true; /* XXX only for btree? */
ivinfo.estimated_count = true;
ivinfo.message_level = DEBUG2;
ivinfo.num_heap_tuples = heapRelation->rd_rel->reltuples;
@@ -3180,15 +3281,39 @@ validate_index(Oid heapId, Oid indexId, Snapshot snapshot)
NULL, false);
state.htups = state.itups = state.tups_inserted = 0;
+ /* ambulkdelete updates progress metrics */
(void) index_bulk_delete(&ivinfo, NULL,
validate_index_callback, (void *) &state);
/* Execute the sort */
+ {
+ const int index[] = {
+ PROGRESS_CREATEIDX_PHASE,
+ PROGRESS_SCAN_BLOCKS_DONE,
+ PROGRESS_SCAN_BLOCKS_TOTAL
+ };
+ const int64 val[] = {
+ PROGRESS_CREATEIDX_PHASE_SORT_IDXSCAN,
+ 0, 0
+ };
+
+ pgstat_progress_update_multi_param(3, index, val);
+ }
tuplesort_performsort(state.tuplesort);
/*
- * Now scan the heap and "merge" it with the index
+ * Now scan the heap and "merge" it with the index.
*/
+ {
+ const int index[] = {
+ PROGRESS_CREATEIDX_PHASE,
+ };
+ const int64 val[] = {
+ PROGRESS_CREATEIDX_PHASE_HEAPSCAN_VALIDATE
+ };
+
+ pgstat_progress_update_multi_param(1, index, val);
+ }
validate_index_heapscan(heapRelation,
indexRelation,
indexInfo,
@@ -3291,6 +3416,7 @@ validate_index_heapscan(Relation heapRelation,
BlockNumber root_blkno = InvalidBlockNumber;
OffsetNumber root_offsets[MaxHeapTuplesPerPage];
bool in_index[MaxHeapTuplesPerPage];
+ BlockNumber previous_blkno = InvalidBlockNumber;
/* state variables for the merge */
ItemPointer indexcursor = NULL;
@@ -3330,6 +3456,9 @@ validate_index_heapscan(Relation heapRelation,
true, /* buffer access strategy OK */
false); /* syncscan not OK */
+ pgstat_progress_update_param(PROGRESS_SCAN_BLOCKS_TOTAL,
+ scan->rs_nblocks);
+
/*
* Scan all tuples matching the snapshot.
*/
@@ -3343,6 +3472,14 @@ validate_index_heapscan(Relation heapRelation,
state->htups += 1;
+ if ((previous_blkno == InvalidBlockNumber) ||
+ (scan->rs_cblock != previous_blkno))
+ {
+ pgstat_progress_update_param(PROGRESS_SCAN_BLOCKS_DONE,
+ scan->rs_cblock);
+ previous_blkno = scan->rs_cblock;
+ }
+
/*
* As commented in IndexBuildHeapScan, we should index heap-only
* tuples under the TIDs of their root tuples; so when we advance onto
diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql
index 3e229c693c4..3cbe6f8af2a 100644
--- a/src/backend/catalog/system_views.sql
+++ b/src/backend/catalog/system_views.sql
@@ -906,6 +906,33 @@ CREATE VIEW pg_stat_progress_vacuum AS
FROM pg_stat_get_progress_info('VACUUM') AS S
LEFT JOIN pg_database D ON S.datid = D.oid;
+CREATE VIEW pg_stat_progress_create_index AS
+ SELECT
+ S.pid AS pid, S.datid AS datid, D.datname AS datname,
+ S.relid AS relid,
+ CASE S.param2 WHEN 0 THEN 'initializing (phase 1 of 8)'
+ WHEN 1 THEN 'waiting for old snapshots (phase 2 of 8)'
+ WHEN 2 THEN 'building index (3 of 8)' ||
+ COALESCE((': ' || pg_indexam_progress_phasename(S.param1::oid, S.param3)),
+ '')
+ WHEN 3 THEN 'waiting for writer snapshots (phase 4 of 8)'
+ WHEN 4 THEN 'index validation: scan index (phase 5 of 8)'
+ WHEN 5 THEN 'index validation: sort index scan results (phase 6 of 8)'
+ WHEN 6 THEN 'index validation: scan heap (phase 7 of 8)'
+ WHEN 7 THEN 'waiting for reader snapshots (phase 8 of 8)'
+ END as phase,
+ S.param4 AS lockers_total,
+ S.param5 AS lockers_done,
+ S.param6 AS current_locker_pid,
+ S.param7 AS blocks_total,
+ S.param8 AS blocks_done,
+ S.param9 AS tuples_total,
+ S.param10 AS tuples_done,
+ S.param11 AS partitions_total,
+ S.param12 AS partitions_done
+ FROM pg_stat_get_progress_info('CREATE INDEX') AS S
+ LEFT JOIN pg_database D ON S.datid = D.oid;
+
CREATE VIEW pg_user_mappings AS
SELECT
U.oid AS umid,
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index 5dcedc337aa..8f8587ac1e3 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -35,6 +35,7 @@
#include "commands/dbcommands.h"
#include "commands/defrem.h"
#include "commands/event_trigger.h"
+#include "commands/progress.h"
#include "commands/tablecmds.h"
#include "commands/tablespace.h"
#include "mb/pg_wchar.h"
@@ -46,10 +47,12 @@
#include "parser/parse_func.h"
#include "parser/parse_oper.h"
#include "partitioning/partdesc.h"
+#include "pgstat.h"
#include "rewrite/rewriteManip.h"
#include "storage/lmgr.h"
#include "storage/proc.h"
#include "storage/procarray.h"
+#include "storage/sinvaladt.h"
#include "utils/acl.h"
#include "utils/builtins.h"
#include "utils/fmgroids.h"
@@ -368,6 +371,15 @@ DefineIndex(Oid relationId,
Snapshot snapshot;
int i;
+
+ /*
+ * Start progress report. If we're building a partition, this was already
+ * done.
+ */
+ if (!OidIsValid(parentIndexId))
+ pgstat_progress_start_command(PROGRESS_COMMAND_CREATE_INDEX,
+ relationId);
+
/*
* count key attributes in index
*/
@@ -584,6 +596,9 @@ DefineIndex(Oid relationId,
accessMethodId = accessMethodForm->oid;
amRoutine = GetIndexAmRoutine(accessMethodForm->amhandler);
+ pgstat_progress_update_param(PROGRESS_CREATEIDX_ACCESS_METHOD_OID,
+ accessMethodId);
+
if (stmt->unique && !amRoutine->amcanunique)
ereport(ERROR,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
@@ -864,6 +879,11 @@ DefineIndex(Oid relationId,
if (!OidIsValid(indexRelationId))
{
table_close(rel, NoLock);
+
+ /* If this is the top-level index, we're done */
+ if (!OidIsValid(parentIndexId))
+ pgstat_progress_end_command();
+
return address;
}
@@ -889,6 +909,9 @@ DefineIndex(Oid relationId,
TupleDesc parentDesc;
Oid *opfamOids;
+ pgstat_progress_update_param(PROGRESS_CREATEIDX_PARTITIONS_TOTAL,
+ nparts);
+
memcpy(part_oids, partdesc->oids, sizeof(Oid) * nparts);
parentDesc = CreateTupleDescCopy(RelationGetDescr(rel));
@@ -1039,6 +1062,8 @@ DefineIndex(Oid relationId,
skip_build, quiet);
}
+ pgstat_progress_update_param(PROGRESS_CREATEIDX_PARTITIONS_DONE,
+ i + 1);
pfree(attmap);
}
@@ -1073,6 +1098,8 @@ DefineIndex(Oid relationId,
* Indexes on partitioned tables are not themselves built, so we're
* done here.
*/
+ if (!OidIsValid(parentIndexId))
+ pgstat_progress_end_command();
return address;
}
@@ -1080,6 +1107,11 @@ DefineIndex(Oid relationId,
{
/* Close the heap and we're done, in the non-concurrent case */
table_close(rel, NoLock);
+
+ /* If this is the top-level index, we're done. */
+ if (!OidIsValid(parentIndexId))
+ pgstat_progress_end_command();
+
return address;
}
@@ -1131,7 +1163,9 @@ DefineIndex(Oid relationId,
* exclusive lock on our table. The lock code will detect deadlock and
* error out properly.
*/
- WaitForLockers(heaplocktag, ShareLock);
+ pgstat_progress_update_param(PROGRESS_CREATEIDX_PHASE,
+ PROGRESS_CREATEIDX_PHASE_WAIT_1);
+ WaitForLockers(heaplocktag, ShareLock, true);
/*
* At this moment we are sure that there are no transactions with the
@@ -1195,7 +1229,9 @@ DefineIndex(Oid relationId,
* We once again wait until no transaction can have the table open with
* the index marked as read-only for updates.
*/
- WaitForLockers(heaplocktag, ShareLock);
+ pgstat_progress_update_param(PROGRESS_CREATEIDX_PHASE,
+ PROGRESS_CREATEIDX_PHASE_WAIT_2);
+ WaitForLockers(heaplocktag, ShareLock, true);
/*
* Now take the "reference snapshot" that will be used by validate_index()
@@ -1281,6 +1317,9 @@ DefineIndex(Oid relationId,
old_snapshots = GetCurrentVirtualXIDs(limitXmin, true, false,
PROC_IS_AUTOVACUUM | PROC_IN_VACUUM,
&n_old_snapshots);
+ pgstat_progress_update_param(PROGRESS_CREATEIDX_PHASE,
+ PROGRESS_CREATEIDX_PHASE_WAIT_3);
+ pgstat_progress_update_param(PROGRESS_WAITFOR_TOTAL, n_old_snapshots);
for (i = 0; i < n_old_snapshots; i++)
{
@@ -1316,7 +1355,14 @@ DefineIndex(Oid relationId,
}
if (VirtualTransactionIdIsValid(old_snapshots[i]))
+ {
+ PGPROC *holder = BackendIdGetProc(old_snapshots[i].backendId);
+ pgstat_progress_update_param(PROGRESS_WAITFOR_CURRENT_PID,
+ holder->pid);
VirtualXactLock(old_snapshots[i], true);
+ }
+
+ pgstat_progress_update_param(PROGRESS_WAITFOR_DONE, i + 1);
}
/*
@@ -1339,6 +1385,8 @@ DefineIndex(Oid relationId,
*/
UnlockRelationIdForSession(&heaprelid, ShareUpdateExclusiveLock);
+ pgstat_progress_end_command();
+
return address;
}
diff --git a/src/backend/storage/ipc/standby.c b/src/backend/storage/ipc/standby.c
index 4d10e57a803..a0a2b964703 100644
--- a/src/backend/storage/ipc/standby.c
+++ b/src/backend/storage/ipc/standby.c
@@ -401,7 +401,7 @@ ResolveRecoveryConflictWithLock(LOCKTAG locktag)
*/
VirtualTransactionId *backends;
- backends = GetLockConflicts(&locktag, AccessExclusiveLock);
+ backends = GetLockConflicts(&locktag, AccessExclusiveLock, NULL);
ResolveRecoveryConflictWithVirtualXIDs(backends,
PROCSIG_RECOVERY_CONFLICT_LOCK);
}
diff --git a/src/backend/storage/lmgr/lmgr.c b/src/backend/storage/lmgr/lmgr.c
index e688ba81170..0b04b093782 100644
--- a/src/backend/storage/lmgr/lmgr.c
+++ b/src/backend/storage/lmgr/lmgr.c
@@ -19,9 +19,12 @@
#include "access/transam.h"
#include "access/xact.h"
#include "catalog/catalog.h"
+#include "commands/progress.h"
#include "miscadmin.h"
+#include "pgstat.h"
#include "storage/lmgr.h"
#include "storage/procarray.h"
+#include "storage/sinvaladt.h"
#include "utils/inval.h"
@@ -857,10 +860,12 @@ XactLockTableWaitErrorCb(void *arg)
* after we obtained our initial list of lockers, we will not wait for them.
*/
void
-WaitForLockersMultiple(List *locktags, LOCKMODE lockmode)
+WaitForLockersMultiple(List *locktags, LOCKMODE lockmode, bool progress)
{
List *holders = NIL;
ListCell *lc;
+ int total = 0;
+ int done = 0;
/* Done if no locks to wait for */
if (list_length(locktags) == 0)
@@ -870,10 +875,17 @@ WaitForLockersMultiple(List *locktags, LOCKMODE lockmode)
foreach(lc, locktags)
{
LOCKTAG *locktag = lfirst(lc);
+ int count;
- holders = lappend(holders, GetLockConflicts(locktag, lockmode));
+ holders = lappend(holders,
+ GetLockConflicts(locktag, lockmode,
+ progress ? &count : NULL));
+ total += count;
}
+ if (progress)
+ pgstat_progress_update_param(PROGRESS_WAITFOR_TOTAL, total);
+
/*
* Note: GetLockConflicts() never reports our own xid, hence we need not
* check for that. Also, prepared xacts are not reported, which is fine
@@ -887,10 +899,36 @@ WaitForLockersMultiple(List *locktags, LOCKMODE lockmode)
while (VirtualTransactionIdIsValid(*lockholders))
{
+ /*
+ * If requested, publish who we're going to wait for. This is not
+ * 100% accurate if they're already gone, but we don't care.
+ */
+ if (progress)
+ {
+ PGPROC *holder = BackendIdGetProc(lockholders->backendId);
+
+ pgstat_progress_update_param(PROGRESS_WAITFOR_CURRENT_PID,
+ holder->pid);
+ }
VirtualXactLock(*lockholders, true);
lockholders++;
+
+ if (progress)
+ pgstat_progress_update_param(PROGRESS_WAITFOR_DONE, ++done);
}
}
+ if (progress)
+ {
+ const int index[] = {
+ PROGRESS_WAITFOR_TOTAL,
+ PROGRESS_WAITFOR_DONE,
+ PROGRESS_WAITFOR_CURRENT_PID
+ };
+ const int64 values[] = {
+ 0, 0, 0
+ };
+ pgstat_progress_update_multi_param(3, index, values);
+ }
list_free_deep(holders);
}
@@ -901,12 +939,12 @@ WaitForLockersMultiple(List *locktags, LOCKMODE lockmode)
* Same as WaitForLockersMultiple, for a single lock tag.
*/
void
-WaitForLockers(LOCKTAG heaplocktag, LOCKMODE lockmode)
+WaitForLockers(LOCKTAG heaplocktag, LOCKMODE lockmode, bool progress)
{
List *l;
l = list_make1(&heaplocktag);
- WaitForLockersMultiple(l, lockmode);
+ WaitForLockersMultiple(l, lockmode, progress);
list_free(l);
}
diff --git a/src/backend/storage/lmgr/lock.c b/src/backend/storage/lmgr/lock.c
index 78fdbd6ff88..c8958766f1e 100644
--- a/src/backend/storage/lmgr/lock.c
+++ b/src/backend/storage/lmgr/lock.c
@@ -2807,6 +2807,7 @@ FastPathGetRelationLockEntry(LOCALLOCK *locallock)
* xacts merely awaiting such a lock are NOT reported.
*
* The result array is palloc'd and is terminated with an invalid VXID.
+ * *countp, if not null, is updated to the number of items set.
*
* Of course, the result could be out of date by the time it's returned,
* so use of this function has to be thought about carefully.
@@ -2817,7 +2818,7 @@ FastPathGetRelationLockEntry(LOCALLOCK *locallock)
* uses of the result.
*/
VirtualTransactionId *
-GetLockConflicts(const LOCKTAG *locktag, LOCKMODE lockmode)
+GetLockConflicts(const LOCKTAG *locktag, LOCKMODE lockmode, int *countp)
{
static VirtualTransactionId *vxids;
LOCKMETHODID lockmethodid = locktag->locktag_lockmethodid;
@@ -2964,6 +2965,8 @@ GetLockConflicts(const LOCKTAG *locktag, LOCKMODE lockmode)
LWLockRelease(partitionLock);
vxids[count].backendId = InvalidBackendId;
vxids[count].localTransactionId = InvalidLocalTransactionId;
+ if (countp)
+ *countp = count;
return vxids;
}
@@ -3019,6 +3022,8 @@ GetLockConflicts(const LOCKTAG *locktag, LOCKMODE lockmode)
vxids[count].backendId = InvalidBackendId;
vxids[count].localTransactionId = InvalidLocalTransactionId;
+ if (countp)
+ *countp = count;
return vxids;
}
diff --git a/src/backend/utils/adt/amutils.c b/src/backend/utils/adt/amutils.c
index 060ffe501ec..e81d6cc0562 100644
--- a/src/backend/utils/adt/amutils.c
+++ b/src/backend/utils/adt/amutils.c
@@ -445,3 +445,26 @@ pg_index_column_has_property(PG_FUNCTION_ARGS)
return indexam_property(fcinfo, propname, InvalidOid, relid, attno);
}
+
+/*
+ * Return the name of the given phase, as used for progress reporting by the
+ * given AM.
+ */
+Datum
+pg_indexam_progress_phasename(PG_FUNCTION_ARGS)
+{
+ Oid amoid = PG_GETARG_OID(0);
+ int32 phasenum = PG_GETARG_INT32(1);
+ IndexAmRoutine *routine;
+ char *name;
+
+ routine = GetIndexAmRoutineByAmId(amoid, true);
+ if (routine == NULL || !routine->ambuildphasename)
+ PG_RETURN_NULL();
+
+ name = routine->ambuildphasename(phasenum);
+ if (!name)
+ PG_RETURN_NULL();
+
+ PG_RETURN_TEXT_P(CStringGetTextDatum(name));
+}
diff --git a/src/backend/utils/adt/pgstatfuncs.c b/src/backend/utils/adt/pgstatfuncs.c
index 69f72657792..05f9da6a4ad 100644
--- a/src/backend/utils/adt/pgstatfuncs.c
+++ b/src/backend/utils/adt/pgstatfuncs.c
@@ -468,6 +468,8 @@ pg_stat_get_progress_info(PG_FUNCTION_ARGS)
/* Translate command name into command type code. */
if (pg_strcasecmp(cmd, "VACUUM") == 0)
cmdtype = PROGRESS_COMMAND_VACUUM;
+ else if (pg_strcasecmp(cmd, "CREATE INDEX") == 0)
+ cmdtype = PROGRESS_COMMAND_CREATE_INDEX;
else
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
diff --git a/src/include/access/amapi.h b/src/include/access/amapi.h
index 653ddc976ba..09a7404267c 100644
--- a/src/include/access/amapi.h
+++ b/src/include/access/amapi.h
@@ -108,6 +108,9 @@ typedef bool (*amproperty_function) (Oid index_oid, int attno,
IndexAMProperty prop, const char *propname,
bool *res, bool *isnull);
+/* name of phase as used in progress reporting */
+typedef char *(*ambuildphasename_function) (int64 phasenum);
+
/* validate definition of an opclass for this AM */
typedef bool (*amvalidate_function) (Oid opclassoid);
@@ -213,6 +216,7 @@ typedef struct IndexAmRoutine
amcostestimate_function amcostestimate;
amoptions_function amoptions;
amproperty_function amproperty; /* can be NULL */
+ ambuildphasename_function ambuildphasename; /* can be NULL */
amvalidate_function amvalidate;
ambeginscan_function ambeginscan;
amrescan_function amrescan;
diff --git a/src/include/access/genam.h b/src/include/access/genam.h
index c4aba39496f..f77d9eea8e8 100644
--- a/src/include/access/genam.h
+++ b/src/include/access/genam.h
@@ -45,6 +45,7 @@ typedef struct IndexVacuumInfo
{
Relation index; /* the index being vacuumed */
bool analyze_only; /* ANALYZE (without any actual vacuum) */
+ bool report_progress; /* emit progress.h status reports */
bool estimated_count; /* num_heap_tuples is an estimate */
int message_level; /* ereport level for progress messages */
double num_heap_tuples; /* tuples remaining in heap */
diff --git a/src/include/access/nbtree.h b/src/include/access/nbtree.h
index 60622ea7906..223aa138837 100644
--- a/src/include/access/nbtree.h
+++ b/src/include/access/nbtree.h
@@ -488,6 +488,16 @@ typedef BTScanOpaqueData *BTScanOpaque;
#define SK_BT_DESC (INDOPTION_DESC << SK_BT_INDOPTION_SHIFT)
#define SK_BT_NULLS_FIRST (INDOPTION_NULLS_FIRST << SK_BT_INDOPTION_SHIFT)
+/*
+ * Constant definition for progress reporting. Phase numbers must match
+ * btbuildphasename.
+ */
+/* PROGRESS_CREATEIDX_SUBPHASE_INITIALIZE is 1 (see progress.h) */
+#define PROGRESS_BTREE_PHASE_INDEXBUILD_HEAPSCAN 2
+#define PROGRESS_BTREE_PHASE_PERFORMSORT_1 3
+#define PROGRESS_BTREE_PHASE_PERFORMSORT_2 4
+#define PROGRESS_BTREE_PHASE_LEAF_LOAD 5
+
/*
* external entry points for btree, in nbtree.c
*/
@@ -600,6 +610,7 @@ extern bytea *btoptions(Datum reloptions, bool validate);
extern bool btproperty(Oid index_oid, int attno,
IndexAMProperty prop, const char *propname,
bool *res, bool *isnull);
+extern char *btbuildphasename(int64 phasenum);
extern IndexTuple _bt_nonkey_truncate(Relation rel, IndexTuple itup);
extern bool _bt_check_natts(Relation rel, Page page, OffsetNumber offnum);
diff --git a/src/include/catalog/index.h b/src/include/catalog/index.h
index 330c481a8b7..fb713d50f96 100644
--- a/src/include/catalog/index.h
+++ b/src/include/catalog/index.h
@@ -114,6 +114,7 @@ extern double IndexBuildHeapScan(Relation heapRelation,
Relation indexRelation,
IndexInfo *indexInfo,
bool allow_sync,
+ bool progress,
IndexBuildCallback callback,
void *callback_state,
struct HeapScanDescData *scan);
@@ -122,6 +123,7 @@ extern double IndexBuildHeapRangeScan(Relation heapRelation,
IndexInfo *indexInfo,
bool allow_sync,
bool anyvisible,
+ bool progress,
BlockNumber start_blockno,
BlockNumber end_blockno,
IndexBuildCallback callback,
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index 5bb56b2c639..0250f8533a9 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -917,6 +917,10 @@
proname => 'pg_index_column_has_property', provolatile => 's',
prorettype => 'bool', proargtypes => 'regclass int4 text',
prosrc => 'pg_index_column_has_property' },
+{ oid => '676', descr => 'return name of given index build phase',
+ proname => 'pg_indexam_progress_phasename', provolatile => 'i',
+ prorettype => 'text', proargtypes => 'oid int8',
+ prosrc => 'pg_indexam_progress_phasename' },
{ oid => '339',
proname => 'poly_same', prorettype => 'bool',
@@ -5079,9 +5083,9 @@
proname => 'pg_stat_get_progress_info', prorows => '100', proretset => 't',
provolatile => 's', proparallel => 'r', prorettype => 'record',
proargtypes => 'text',
- proallargtypes => '{text,int4,oid,oid,int8,int8,int8,int8,int8,int8,int8,int8,int8,int8}',
- proargmodes => '{i,o,o,o,o,o,o,o,o,o,o,o,o,o}',
- proargnames => '{cmdtype,pid,datid,relid,param1,param2,param3,param4,param5,param6,param7,param8,param9,param10}',
+ proallargtypes => '{text,int4,oid,oid,int8,int8,int8,int8,int8,int8,int8,int8,int8,int8,int8,int8}',
+ proargmodes => '{i,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o}',
+ proargnames => '{cmdtype,pid,datid,relid,param1,param2,param3,param4,param5,param6,param7,param8,param9,param10,param11,param12}',
prosrc => 'pg_stat_get_progress_info' },
{ oid => '3099',
descr => 'statistics: information about currently active replication',
diff --git a/src/include/commands/progress.h b/src/include/commands/progress.h
index 9858b36a383..56bd6125594 100644
--- a/src/include/commands/progress.h
+++ b/src/include/commands/progress.h
@@ -34,4 +34,40 @@
#define PROGRESS_VACUUM_PHASE_TRUNCATE 5
#define PROGRESS_VACUUM_PHASE_FINAL_CLEANUP 6
+
+/* Progress parameters for CREATE INDEX */
+#define PROGRESS_CREATEIDX_ACCESS_METHOD_OID 0
+#define PROGRESS_CREATEIDX_PHASE 1 /* AM-agnostic phase # */
+#define PROGRESS_CREATEIDX_SUBPHASE 2 /* phase # filled by AM */
+/* 3, 4 and 5 reserved for "waitfor" metrics */
+/* 6 and 7 reserved for "block number" metrics */
+#define PROGRESS_CREATEIDX_TUPLES_TOTAL 8
+#define PROGRESS_CREATEIDX_TUPLES_DONE 9
+#define PROGRESS_CREATEIDX_PARTITIONS_TOTAL 10
+#define PROGRESS_CREATEIDX_PARTITIONS_DONE 11
+
+/* Phases of CREATE INDEX */
+#define PROGRESS_CREATEIDX_PHASE_WAIT_1 1
+#define PROGRESS_CREATEIDX_PHASE_BUILD 2
+#define PROGRESS_CREATEIDX_PHASE_WAIT_2 3
+#define PROGRESS_CREATEIDX_PHASE_VALIDATE_IDXSCAN 4
+#define PROGRESS_CREATEIDX_PHASE_SORT_IDXSCAN 5
+#define PROGRESS_CREATEIDX_PHASE_HEAPSCAN_VALIDATE 6
+#define PROGRESS_CREATEIDX_PHASE_WAIT_3 7
+
+/*
+ * Subphases of CREATE INDEX, for index_build.
+ */
+#define PROGRESS_CREATEIDX_SUBPHASE_INITIALIZE 1
+/* Additional phases are defined by each AM */
+
+/* Lock holder wait counts */
+#define PROGRESS_WAITFOR_TOTAL 3
+#define PROGRESS_WAITFOR_DONE 4
+#define PROGRESS_WAITFOR_CURRENT_PID 5
+
+/* Block numbers in a generic relation scan */
+#define PROGRESS_SCAN_BLOCKS_TOTAL 6
+#define PROGRESS_SCAN_BLOCKS_DONE 7
+
#endif
diff --git a/src/include/pgstat.h b/src/include/pgstat.h
index 88a75fb798e..f931ead1c27 100644
--- a/src/include/pgstat.h
+++ b/src/include/pgstat.h
@@ -934,10 +934,11 @@ typedef enum
typedef enum ProgressCommandType
{
PROGRESS_COMMAND_INVALID,
- PROGRESS_COMMAND_VACUUM
+ PROGRESS_COMMAND_VACUUM,
+ PROGRESS_COMMAND_CREATE_INDEX
} ProgressCommandType;
-#define PGSTAT_NUM_PROGRESS_PARAM 10
+#define PGSTAT_NUM_PROGRESS_PARAM 12
/* ----------
* Shared-memory data structures
diff --git a/src/include/storage/lmgr.h b/src/include/storage/lmgr.h
index 3d705faba5c..4f2872de35f 100644
--- a/src/include/storage/lmgr.h
+++ b/src/include/storage/lmgr.h
@@ -78,8 +78,8 @@ extern void XactLockTableWait(TransactionId xid, Relation rel,
extern bool ConditionalXactLockTableWait(TransactionId xid);
/* Lock VXIDs, specified by conflicting locktags */
-extern void WaitForLockers(LOCKTAG heaplocktag, LOCKMODE lockmode);
-extern void WaitForLockersMultiple(List *locktags, LOCKMODE lockmode);
+extern void WaitForLockers(LOCKTAG heaplocktag, LOCKMODE lockmode, bool progress);
+extern void WaitForLockersMultiple(List *locktags, LOCKMODE lockmode, bool progress);
/* Lock an XID for tuple insertion (used to wait for an insertion to finish) */
extern uint32 SpeculativeInsertionLockAcquire(TransactionId xid);
diff --git a/src/include/storage/lock.h b/src/include/storage/lock.h
index 16b927cb801..e117b391774 100644
--- a/src/include/storage/lock.h
+++ b/src/include/storage/lock.h
@@ -544,7 +544,7 @@ extern bool LockHeldByMe(const LOCKTAG *locktag, LOCKMODE lockmode);
extern bool LockHasWaiters(const LOCKTAG *locktag,
LOCKMODE lockmode, bool sessionLock);
extern VirtualTransactionId *GetLockConflicts(const LOCKTAG *locktag,
- LOCKMODE lockmode);
+ LOCKMODE lockmode, int *countp);
extern void AtPrepare_Locks(void);
extern void PostPrepare_Locks(TransactionId xid);
extern int LockCheckConflicts(LockMethod lockMethodTable,
--
2.17.1
--3V7upXqbjpZ4EhLz
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v5-0002-report-progress-of-hash-indexes.patch"
^ permalink raw reply [nested|flat] 17+ messages in thread
* [PATCH v12 2/7] Row pattern recognition patch (parse/analysis).
@ 2023-12-04 11:23 Tatsuo Ishii <[email protected]>
0 siblings, 0 replies; 17+ messages in thread
From: Tatsuo Ishii @ 2023-12-04 11:23 UTC (permalink / raw)
---
src/backend/parser/parse_agg.c | 7 +
src/backend/parser/parse_clause.c | 278 +++++++++++++++++++++++++++++-
src/backend/parser/parse_expr.c | 4 +
src/backend/parser/parse_func.c | 3 +
4 files changed, 291 insertions(+), 1 deletion(-)
diff --git a/src/backend/parser/parse_agg.c b/src/backend/parser/parse_agg.c
index 9bbad33fbd..28fb5e0d71 100644
--- a/src/backend/parser/parse_agg.c
+++ b/src/backend/parser/parse_agg.c
@@ -575,6 +575,10 @@ check_agglevels_and_constraints(ParseState *pstate, Node *expr)
errkind = true;
break;
+ case EXPR_KIND_RPR_DEFINE:
+ errkind = true;
+ break;
+
/*
* There is intentionally no default: case here, so that the
* compiler will warn if we add a new ParseExprKind without
@@ -964,6 +968,9 @@ transformWindowFuncCall(ParseState *pstate, WindowFunc *wfunc,
case EXPR_KIND_CYCLE_MARK:
errkind = true;
break;
+ case EXPR_KIND_RPR_DEFINE:
+ errkind = true;
+ break;
/*
* There is intentionally no default: case here, so that the
diff --git a/src/backend/parser/parse_clause.c b/src/backend/parser/parse_clause.c
index 334b9b42bd..c5d3c10683 100644
--- a/src/backend/parser/parse_clause.c
+++ b/src/backend/parser/parse_clause.c
@@ -100,7 +100,10 @@ static WindowClause *findWindowClause(List *wclist, const char *name);
static Node *transformFrameOffset(ParseState *pstate, int frameOptions,
Oid rangeopfamily, Oid rangeopcintype, Oid *inRangeFunc,
Node *clause);
-
+static void transformRPR(ParseState *pstate, WindowClause *wc, WindowDef *windef, List **targetlist);
+static List *transformDefineClause(ParseState *pstate, WindowClause *wc, WindowDef *windef, List **targetlist);
+static void transformPatternClause(ParseState *pstate, WindowClause *wc, WindowDef *windef);
+static List *transformMeasureClause(ParseState *pstate, WindowClause *wc, WindowDef *windef);
/*
* transformFromClause -
@@ -2950,6 +2953,10 @@ transformWindowDefinitions(ParseState *pstate,
rangeopfamily, rangeopcintype,
&wc->endInRangeFunc,
windef->endOffset);
+
+ /* Process Row Pattern Recognition related clauses */
+ transformRPR(pstate, wc, windef, targetlist);
+
wc->runCondition = NIL;
wc->winref = winref;
@@ -3815,3 +3822,272 @@ transformFrameOffset(ParseState *pstate, int frameOptions,
return node;
}
+
+/*
+ * transformRPR
+ * Process Row Pattern Recognition related clauses
+ */
+static void
+transformRPR(ParseState *pstate, WindowClause *wc, WindowDef *windef, List **targetlist)
+{
+ /*
+ * Window definition exists?
+ */
+ if (windef == NULL)
+ return;
+
+ /*
+ * Row Pattern Common Syntax clause exists?
+ */
+ if (windef->rpCommonSyntax == NULL)
+ return;
+
+ /* Check Frame option. Frame must start at current row */
+ if ((wc->frameOptions & FRAMEOPTION_START_CURRENT_ROW) == 0)
+ ereport(ERROR,
+ (errcode(ERRCODE_SYNTAX_ERROR),
+ errmsg("FRAME must start at current row when row patttern recognition is used")));
+
+ /* Transform AFTER MACH SKIP TO clause */
+ wc->rpSkipTo = windef->rpCommonSyntax->rpSkipTo;
+
+ /* Transform SEEK or INITIAL clause */
+ wc->initial = windef->rpCommonSyntax->initial;
+
+ /* Transform DEFINE clause into list of TargetEntry's */
+ wc->defineClause = transformDefineClause(pstate, wc, windef, targetlist);
+
+ /* Check PATTERN clause and copy to patternClause */
+ transformPatternClause(pstate, wc, windef);
+
+ /* Transform MEASURE clause */
+ transformMeasureClause(pstate, wc, windef);
+}
+
+/*
+ * transformDefineClause Process DEFINE clause and transform ResTarget into
+ * list of TargetEntry.
+ *
+ * XXX we only support column reference in row pattern definition search
+ * condition, e.g. "price". <row pattern definition variable name>.<column
+ * reference> is not supported, e.g. "A.price".
+ */
+static List *
+transformDefineClause(ParseState *pstate, WindowClause *wc, WindowDef *windef, List **targetlist)
+{
+ /* DEFINE variable name initials */
+ static char *defineVariableInitials = "abcdefghijklmnopqrstuvwxyz";
+
+ ListCell *lc, *l;
+ ResTarget *restarget, *r;
+ List *restargets;
+ List *defineClause;
+ char *name;
+ int initialLen;
+ int i;
+
+ /*
+ * If Row Definition Common Syntax exists, DEFINE clause must exist.
+ * (the raw parser should have already checked it.)
+ */
+ Assert(windef->rpCommonSyntax->rpDefs != NULL);
+
+ /*
+ * Check and add "A AS A IS TRUE" if pattern variable is missing in DEFINE
+ * per the SQL standard.
+ */
+ restargets = NIL;
+ foreach(lc, windef->rpCommonSyntax->rpPatterns)
+ {
+ A_Expr *a;
+ bool found = false;
+
+ if (!IsA(lfirst(lc), A_Expr))
+ ereport(ERROR,
+ errmsg("node type is not A_Expr"));
+
+ a = (A_Expr *)lfirst(lc);
+ name = strVal(a->lexpr);
+
+ foreach(l, windef->rpCommonSyntax->rpDefs)
+ {
+ restarget = (ResTarget *)lfirst(l);
+
+ if (!strcmp(restarget->name, name))
+ {
+ found = true;
+ break;
+ }
+ }
+
+ if (!found)
+ {
+ /*
+ * "name" is missing. So create "name AS name IS TRUE" ResTarget
+ * node and add it to the temporary list.
+ */
+ A_Const *n;
+
+ restarget = makeNode(ResTarget);
+ n = makeNode(A_Const);
+ n->val.boolval.type = T_Boolean;
+ n->val.boolval.boolval = true;
+ n->location = -1;
+ restarget->name = pstrdup(name);
+ restarget->indirection = NIL;
+ restarget->val = (Node *)n;
+ restarget->location = -1;
+ restargets = lappend((List *)restargets, restarget);
+ }
+ }
+
+ if (list_length(restargets) >= 1)
+ {
+ /* add missing DEFINEs */
+ windef->rpCommonSyntax->rpDefs = list_concat(windef->rpCommonSyntax->rpDefs,
+ restargets);
+ list_free(restargets);
+ }
+
+ /*
+ * Check for duplicate row pattern definition variables. The standard
+ * requires that no two row pattern definition variable names shall be
+ * equivalent.
+ */
+ restargets = NIL;
+ foreach(lc, windef->rpCommonSyntax->rpDefs)
+ {
+ restarget = (ResTarget *)lfirst(lc);
+ name = restarget->name;
+
+ /*
+ * Add DEFINE expression (Restarget->val) to the targetlist as a
+ * TargetEntry if it does not exist yet. Planner will add the column
+ * ref var node to the outer plan's target list later on. This makes
+ * DEFINE expression could access the outer tuple while evaluating
+ * PATTERN.
+ *
+ * XXX: adding whole expressions of DEFINE to the plan.targetlist is
+ * not so good, because it's not necessary to evalute the expression
+ * in the target list while running the plan. We should extract the
+ * var nodes only then add them to the plan.targetlist.
+ */
+ findTargetlistEntrySQL99(pstate, (Node *)restarget->val, targetlist, EXPR_KIND_RPR_DEFINE);
+
+ /*
+ * Make sure that the row pattern definition search condition is a
+ * boolean expression.
+ */
+ transformWhereClause(pstate, restarget->val,
+ EXPR_KIND_RPR_DEFINE, "DEFINE");
+
+ foreach(l, restargets)
+ {
+ char *n;
+
+ r = (ResTarget *) lfirst(l);
+ n = r->name;
+
+ if (!strcmp(n, name))
+ ereport(ERROR,
+ (errcode(ERRCODE_SYNTAX_ERROR),
+ errmsg("row pattern definition variable name \"%s\" appears more than once in DEFINE clause",
+ name),
+ parser_errposition(pstate, exprLocation((Node *)r))));
+ }
+ restargets = lappend(restargets, restarget);
+ }
+ list_free(restargets);
+
+ /*
+ * Create list of row pattern DEFINE variable name's initial.
+ * We assign [a-z] to them (up to 26 variable names are allowed).
+ */
+ restargets = NIL;
+ i = 0;
+ initialLen = strlen(defineVariableInitials);
+
+ foreach(lc, windef->rpCommonSyntax->rpDefs)
+ {
+ char initial[2];
+
+ restarget = (ResTarget *)lfirst(lc);
+ name = restarget->name;
+
+ if (i >= initialLen)
+ {
+ ereport(ERROR,
+ (errcode(ERRCODE_SYNTAX_ERROR),
+ errmsg("number of row pattern definition variable names exceeds %d", initialLen),
+ parser_errposition(pstate, exprLocation((Node *)restarget))));
+ }
+ initial[0] = defineVariableInitials[i++];
+ initial[1] = '\0';
+ wc->defineInitial = lappend(wc->defineInitial, makeString(pstrdup(initial)));
+ }
+
+ defineClause = transformTargetList(pstate, windef->rpCommonSyntax->rpDefs,
+ EXPR_KIND_RPR_DEFINE);
+
+ /* mark column origins */
+ markTargetListOrigins(pstate, defineClause);
+
+ /* mark all nodes in the DEFINE clause tree with collation information */
+ assign_expr_collations(pstate, (Node *)defineClause);
+
+ return defineClause;
+}
+
+/*
+ * transformPatternClause
+ * Process PATTERN clause and return PATTERN clause in the raw parse tree
+ */
+static void
+transformPatternClause(ParseState *pstate, WindowClause *wc, WindowDef *windef)
+{
+ ListCell *lc;
+
+ /*
+ * Row Pattern Common Syntax clause exists?
+ */
+ if (windef->rpCommonSyntax == NULL)
+ return;
+
+ wc->patternVariable = NIL;
+ wc->patternRegexp = NIL;
+ foreach(lc, windef->rpCommonSyntax->rpPatterns)
+ {
+ A_Expr *a;
+ char *name;
+ char *regexp;
+
+ if (!IsA(lfirst(lc), A_Expr))
+ ereport(ERROR,
+ errmsg("node type is not A_Expr"));
+
+ a = (A_Expr *)lfirst(lc);
+ name = strVal(a->lexpr);
+
+ wc->patternVariable = lappend(wc->patternVariable, makeString(pstrdup(name)));
+ regexp = strVal(lfirst(list_head(a->name)));
+ wc->patternRegexp = lappend(wc->patternRegexp, makeString(pstrdup(regexp)));
+ }
+}
+
+/*
+ * transformMeasureClause
+ * Process MEASURE clause
+ * XXX MEASURE clause is not supported yet
+ */
+static List *
+transformMeasureClause(ParseState *pstate, WindowClause *wc, WindowDef *windef)
+{
+ if (windef->rowPatternMeasures == NIL)
+ return NIL;
+
+ ereport(ERROR,
+ (errcode(ERRCODE_SYNTAX_ERROR),
+ errmsg("%s","MEASURE clause is not supported yet"),
+ parser_errposition(pstate, exprLocation((Node *)windef->rowPatternMeasures))));
+ return NIL;
+}
diff --git a/src/backend/parser/parse_expr.c b/src/backend/parser/parse_expr.c
index 64c582c344..18b58ac263 100644
--- a/src/backend/parser/parse_expr.c
+++ b/src/backend/parser/parse_expr.c
@@ -557,6 +557,7 @@ transformColumnRef(ParseState *pstate, ColumnRef *cref)
case EXPR_KIND_COPY_WHERE:
case EXPR_KIND_GENERATED_COLUMN:
case EXPR_KIND_CYCLE_MARK:
+ case EXPR_KIND_RPR_DEFINE:
/* okay */
break;
@@ -1770,6 +1771,7 @@ transformSubLink(ParseState *pstate, SubLink *sublink)
case EXPR_KIND_VALUES:
case EXPR_KIND_VALUES_SINGLE:
case EXPR_KIND_CYCLE_MARK:
+ case EXPR_KIND_RPR_DEFINE:
/* okay */
break;
case EXPR_KIND_CHECK_CONSTRAINT:
@@ -3149,6 +3151,8 @@ ParseExprKindName(ParseExprKind exprKind)
return "GENERATED AS";
case EXPR_KIND_CYCLE_MARK:
return "CYCLE";
+ case EXPR_KIND_RPR_DEFINE:
+ return "DEFINE";
/*
* There is intentionally no default: case here, so that the
diff --git a/src/backend/parser/parse_func.c b/src/backend/parser/parse_func.c
index 6c29471bb3..086431f91b 100644
--- a/src/backend/parser/parse_func.c
+++ b/src/backend/parser/parse_func.c
@@ -2656,6 +2656,9 @@ check_srf_call_placement(ParseState *pstate, Node *last_srf, int location)
case EXPR_KIND_CYCLE_MARK:
errkind = true;
break;
+ case EXPR_KIND_RPR_DEFINE:
+ errkind = true;
+ break;
/*
* There is intentionally no default: case here, so that the
--
2.25.1
----Next_Part(Fri_Dec__8_10_16_13_2023_489)--
Content-Type: Text/X-Patch; charset=us-ascii
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
filename="v12-0003-Row-pattern-recognition-patch-planner.patch"
^ permalink raw reply [nested|flat] 17+ messages in thread
* Re: Should rolpassword be toastable?
@ 2024-09-20 14:06 Jonathan S. Katz <[email protected]>
2024-09-20 16:09 ` Re: Should rolpassword be toastable? Nathan Bossart <[email protected]>
0 siblings, 1 reply; 17+ messages in thread
From: Jonathan S. Katz @ 2024-09-20 14:06 UTC (permalink / raw)
To: Michael Paquier <[email protected]>; Nathan Bossart <[email protected]>; +Cc: Tom Lane <[email protected]>; Alexander Lakhin <[email protected]>; pgsql-hackers
On 9/20/24 1:23 AM, Michael Paquier wrote:
> On Thu, Sep 19, 2024 at 09:46:00PM -0500, Nathan Bossart wrote:
>> On Thu, Sep 19, 2024 at 07:37:55PM -0400, Jonathan S. Katz wrote:
>>>> Shouldn't we enforce the limit in every case in encrypt_password,
>>>> not just this one? (I do agree that encrypt_password is an okay
>>>> place to enforce it.)
>>
>> Yeah, that seems like a good idea. I've attached a more fleshed-out patch
>> set that applies the limit in all cases.
>
> Not sure. Is this really something we absolutely need? Sure, this
> generates a better error when inserting a record too long to
> pg_authid, but removing the toast relation is enough to avoid the
> problems one would see when authenticating. Not sure if this argument
> is enough to count as an objection, just sharing some doubts :)
The errors from lack of TOAST are confusing to users. Why can't we have
a user friendly error here?
Jonathan
Attachments:
[application/pgp-signature] OpenPGP_signature.asc (840B, ../../[email protected]/2-OpenPGP_signature.asc)
download
^ permalink raw reply [nested|flat] 17+ messages in thread
* Re: Should rolpassword be toastable?
2024-09-20 14:06 Re: Should rolpassword be toastable? Jonathan S. Katz <[email protected]>
@ 2024-09-20 16:09 ` Nathan Bossart <[email protected]>
2024-09-20 16:27 ` Re: Should rolpassword be toastable? Tom Lane <[email protected]>
0 siblings, 1 reply; 17+ messages in thread
From: Nathan Bossart @ 2024-09-20 16:09 UTC (permalink / raw)
To: Jonathan S. Katz <[email protected]>; +Cc: Michael Paquier <[email protected]>; Tom Lane <[email protected]>; Alexander Lakhin <[email protected]>; pgsql-hackers
On Fri, Sep 20, 2024 at 10:06:28AM -0400, Jonathan S. Katz wrote:
> On 9/20/24 1:23 AM, Michael Paquier wrote:
>> Not sure. Is this really something we absolutely need? Sure, this
>> generates a better error when inserting a record too long to
>> pg_authid, but removing the toast relation is enough to avoid the
>> problems one would see when authenticating. Not sure if this argument
>> is enough to count as an objection, just sharing some doubts :)
>
> The errors from lack of TOAST are confusing to users. Why can't we have a
> user friendly error here?
If I wanted to argue against adding a user-friendly error, I'd point out
that it's highly unlikely anyone is actually trying to use super long
hashes unless they are trying to break things, and it's just another
arbitrary limit that we'll need to maintain/enforce. But on the off-chance
that someone is building a custom driver that generates long hashes for
whatever reason, I'd imagine that a clear error would be more helpful than
"row is too big."
Here is a v3 patch set that fixes the test comment and a compiler warning
in cfbot.
--
nathan
From bb3aad9105b1997bc088403437ac6294e22809d9 Mon Sep 17 00:00:00 2001
From: Nathan Bossart <[email protected]>
Date: Thu, 19 Sep 2024 20:59:10 -0500
Subject: [PATCH v3 1/2] place limit on password hash length
---
src/backend/libpq/crypt.c | 60 ++++++++++++++++++--------
src/include/libpq/crypt.h | 10 +++++
src/test/regress/expected/password.out | 7 +++
src/test/regress/sql/password.sql | 4 ++
4 files changed, 63 insertions(+), 18 deletions(-)
diff --git a/src/backend/libpq/crypt.c b/src/backend/libpq/crypt.c
index 629e51e00b..753e5c11da 100644
--- a/src/backend/libpq/crypt.c
+++ b/src/backend/libpq/crypt.c
@@ -116,7 +116,7 @@ encrypt_password(PasswordType target_type, const char *role,
const char *password)
{
PasswordType guessed_type = get_password_type(password);
- char *encrypted_password;
+ char *encrypted_password = NULL;
const char *errstr = NULL;
if (guessed_type != PASSWORD_TYPE_PLAINTEXT)
@@ -125,32 +125,56 @@ encrypt_password(PasswordType target_type, const char *role,
* Cannot convert an already-encrypted password from one format to
* another, so return it as it is.
*/
- return pstrdup(password);
+ encrypted_password = pstrdup(password);
}
-
- switch (target_type)
+ else
{
- case PASSWORD_TYPE_MD5:
- encrypted_password = palloc(MD5_PASSWD_LEN + 1);
+ switch (target_type)
+ {
+ case PASSWORD_TYPE_MD5:
+ encrypted_password = palloc(MD5_PASSWD_LEN + 1);
- if (!pg_md5_encrypt(password, role, strlen(role),
- encrypted_password, &errstr))
- elog(ERROR, "password encryption failed: %s", errstr);
- return encrypted_password;
+ if (!pg_md5_encrypt(password, role, strlen(role),
+ encrypted_password, &errstr))
+ elog(ERROR, "password encryption failed: %s", errstr);
+ break;
- case PASSWORD_TYPE_SCRAM_SHA_256:
- return pg_be_scram_build_secret(password);
+ case PASSWORD_TYPE_SCRAM_SHA_256:
+ encrypted_password = pg_be_scram_build_secret(password);
+ break;
- case PASSWORD_TYPE_PLAINTEXT:
- elog(ERROR, "cannot encrypt password with 'plaintext'");
+ case PASSWORD_TYPE_PLAINTEXT:
+ elog(ERROR, "cannot encrypt password with 'plaintext'");
+ break;
+ }
}
+ Assert(encrypted_password);
+
/*
- * This shouldn't happen, because the above switch statements should
- * handle every combination of source and target password types.
+ * Valid password hashes may be very long, but we don't want to store
+ * anything that might need out-of-line storage, since de-TOASTing won't
+ * work during authentication because we haven't selected a database yet
+ * and cannot read pg_class. 256 bytes should be more than enough for all
+ * practical use, so fail for anything longer.
*/
- elog(ERROR, "cannot encrypt password to requested type");
- return NULL; /* keep compiler quiet */
+ if (encrypted_password && /* keep compiler quiet */
+ strlen(encrypted_password) > MAX_ENCRYPTED_PASSWORD_LEN)
+ {
+ /*
+ * We don't expect any of our own hashing routines to produce hashes
+ * that are too long.
+ */
+ Assert(guessed_type != PASSWORD_TYPE_PLAINTEXT);
+
+ ereport(ERROR,
+ (errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
+ errmsg("encrypted password is too long"),
+ errdetail("Encrypted passwords must be no longer than %d characters.",
+ MAX_ENCRYPTED_PASSWORD_LEN)));
+ }
+
+ return encrypted_password;
}
/*
diff --git a/src/include/libpq/crypt.h b/src/include/libpq/crypt.h
index f744de4d20..a2c99cd16a 100644
--- a/src/include/libpq/crypt.h
+++ b/src/include/libpq/crypt.h
@@ -15,6 +15,16 @@
#include "datatype/timestamp.h"
+/*
+ * Valid password hashes may be very long, but we don't want to store anything
+ * that might need out-of-line storage, since de-TOASTing won't work during
+ * authentication because we haven't selected a database yet and cannot read
+ * pg_class. 256 bytes should be more than enough for all practical use, and
+ * our own password encryption routines should never produce hashes longer than
+ * this.
+ */
+#define MAX_ENCRYPTED_PASSWORD_LEN (256)
+
/*
* Types of password hashes or secrets.
*
diff --git a/src/test/regress/expected/password.out b/src/test/regress/expected/password.out
index 924d6e001d..8ab995a162 100644
--- a/src/test/regress/expected/password.out
+++ b/src/test/regress/expected/password.out
@@ -127,6 +127,13 @@ SELECT rolname, rolpassword not like '%A6xHKoH/494E941doaPOYg==%' as is_rolpassw
regress_passwd_sha_len2 | t
(3 rows)
+-- Test that valid hashes that are too long are rejected
+CREATE ROLE regress_passwd10 PASSWORD 'SCRAM-SHA-256$00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000004096:NuDacwYSUxeOeFUEf3ivTQ==$Wgvq3OCYrJI6eUfvKlAzn4p/j3mzgWzXbVnWeFK1qhY=:r1qSP0j2QojCjLpFUjI0i6ckInvxJDKoyWnN3zF8WCM=';
+ERROR: encrypted password is too long
+DETAIL: Encrypted passwords must be no longer than 256 characters.
+ALTER ROLE regress_passwd9 PASSWORD 'SCRAM-SHA-256$00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000004096:NuDacwYSUxeOeFUEf3ivTQ==$Wgvq3OCYrJI6eUfvKlAzn4p/j3mzgWzXbVnWeFK1qhY=:r1qSP0j2QojCjLpFUjI0i6ckInvxJDKoyWnN3zF8WCM=';
+ERROR: encrypted password is too long
+DETAIL: Encrypted passwords must be no longer than 256 characters.
DROP ROLE regress_passwd1;
DROP ROLE regress_passwd2;
DROP ROLE regress_passwd3;
diff --git a/src/test/regress/sql/password.sql b/src/test/regress/sql/password.sql
index bb82aa4aa2..442c903c00 100644
--- a/src/test/regress/sql/password.sql
+++ b/src/test/regress/sql/password.sql
@@ -95,6 +95,10 @@ SELECT rolname, rolpassword not like '%A6xHKoH/494E941doaPOYg==%' as is_rolpassw
WHERE rolname LIKE 'regress_passwd_sha_len%'
ORDER BY rolname;
+-- Test that valid hashes that are too long are rejected
+CREATE ROLE regress_passwd10 PASSWORD 'SCRAM-SHA-256$00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000004096:NuDacwYSUxeOeFUEf3ivTQ==$Wgvq3OCYrJI6eUfvKlAzn4p/j3mzgWzXbVnWeFK1qhY=:r1qSP0j2QojCjLpFUjI0i6ckInvxJDKoyWnN3zF8WCM=';
+ALTER ROLE regress_passwd9 PASSWORD 'SCRAM-SHA-256$00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000004096:NuDacwYSUxeOeFUEf3ivTQ==$Wgvq3OCYrJI6eUfvKlAzn4p/j3mzgWzXbVnWeFK1qhY=:r1qSP0j2QojCjLpFUjI0i6ckInvxJDKoyWnN3zF8WCM=';
+
DROP ROLE regress_passwd1;
DROP ROLE regress_passwd2;
DROP ROLE regress_passwd3;
--
2.39.5 (Apple Git-154)
From 7e1736701e270d40cc1009ff4760317a8a366663 Mon Sep 17 00:00:00 2001
From: Nathan Bossart <[email protected]>
Date: Thu, 19 Sep 2024 21:09:30 -0500
Subject: [PATCH v3 2/2] remove pg_authid's TOAST table
XXX: NEEDS CATVERSION BUMP
---
src/backend/catalog/catalog.c | 4 +---
src/include/catalog/pg_authid.h | 2 --
src/test/regress/expected/create_index.out | 14 +++++++-------
src/test/regress/expected/misc_sanity.out | 5 ++++-
src/test/regress/expected/tablespace.out | 12 ++++++------
src/test/regress/sql/create_index.sql | 10 +++++-----
src/test/regress/sql/misc_sanity.sql | 2 ++
src/test/regress/sql/tablespace.sql | 8 ++++----
8 files changed, 29 insertions(+), 28 deletions(-)
diff --git a/src/backend/catalog/catalog.c b/src/backend/catalog/catalog.c
index 6c39434a30..d6b07a7865 100644
--- a/src/backend/catalog/catalog.c
+++ b/src/backend/catalog/catalog.c
@@ -300,9 +300,7 @@ IsSharedRelation(Oid relationId)
relationId == TablespaceOidIndexId)
return true;
/* These are their toast tables and toast indexes */
- if (relationId == PgAuthidToastTable ||
- relationId == PgAuthidToastIndex ||
- relationId == PgDatabaseToastTable ||
+ if (relationId == PgDatabaseToastTable ||
relationId == PgDatabaseToastIndex ||
relationId == PgDbRoleSettingToastTable ||
relationId == PgDbRoleSettingToastIndex ||
diff --git a/src/include/catalog/pg_authid.h b/src/include/catalog/pg_authid.h
index e08863f78a..e846d75731 100644
--- a/src/include/catalog/pg_authid.h
+++ b/src/include/catalog/pg_authid.h
@@ -55,8 +55,6 @@ CATALOG(pg_authid,1260,AuthIdRelationId) BKI_SHARED_RELATION BKI_ROWTYPE_OID(284
*/
typedef FormData_pg_authid *Form_pg_authid;
-DECLARE_TOAST_WITH_MACRO(pg_authid, 4175, 4176, PgAuthidToastTable, PgAuthidToastIndex);
-
DECLARE_UNIQUE_INDEX(pg_authid_rolname_index, 2676, AuthIdRolnameIndexId, pg_authid, btree(rolname name_ops));
DECLARE_UNIQUE_INDEX_PKEY(pg_authid_oid_index, 2677, AuthIdOidIndexId, pg_authid, btree(oid oid_ops));
diff --git a/src/test/regress/expected/create_index.out b/src/test/regress/expected/create_index.out
index cf6eac5734..d3358dfc39 100644
--- a/src/test/regress/expected/create_index.out
+++ b/src/test/regress/expected/create_index.out
@@ -2664,10 +2664,10 @@ REINDEX TABLE CONCURRENTLY pg_class; -- no catalog relation
ERROR: cannot reindex system catalogs concurrently
REINDEX INDEX CONCURRENTLY pg_class_oid_index; -- no catalog index
ERROR: cannot reindex system catalogs concurrently
--- These are the toast table and index of pg_authid.
-REINDEX TABLE CONCURRENTLY pg_toast.pg_toast_1260; -- no catalog toast table
+-- These are the toast table and index of pg_database.
+REINDEX TABLE CONCURRENTLY pg_toast.pg_toast_1262; -- no catalog toast table
ERROR: cannot reindex system catalogs concurrently
-REINDEX INDEX CONCURRENTLY pg_toast.pg_toast_1260_index; -- no catalog toast index
+REINDEX INDEX CONCURRENTLY pg_toast.pg_toast_1262_index; -- no catalog toast index
ERROR: cannot reindex system catalogs concurrently
REINDEX SYSTEM CONCURRENTLY postgres; -- not allowed for SYSTEM
ERROR: cannot reindex system catalogs concurrently
@@ -2974,10 +2974,10 @@ ERROR: must be owner of schema schema_to_reindex
RESET ROLE;
GRANT USAGE ON SCHEMA pg_toast TO regress_reindexuser;
SET SESSION ROLE regress_reindexuser;
-REINDEX TABLE pg_toast.pg_toast_1260;
-ERROR: permission denied for table pg_toast_1260
-REINDEX INDEX pg_toast.pg_toast_1260_index;
-ERROR: permission denied for index pg_toast_1260_index
+REINDEX TABLE pg_toast.pg_toast_1262;
+ERROR: permission denied for table pg_toast_1262
+REINDEX INDEX pg_toast.pg_toast_1262_index;
+ERROR: permission denied for index pg_toast_1262_index
-- Clean up
RESET ROLE;
REVOKE USAGE ON SCHEMA pg_toast FROM regress_reindexuser;
diff --git a/src/test/regress/expected/misc_sanity.out b/src/test/regress/expected/misc_sanity.out
index 45e7492877..b032a3f476 100644
--- a/src/test/regress/expected/misc_sanity.out
+++ b/src/test/regress/expected/misc_sanity.out
@@ -40,6 +40,8 @@ WHERE refclassid = 0 OR refobjid = 0 OR
-- 2. pg_largeobject and pg_largeobject_metadata. Large object catalogs
-- and toast tables are mutually exclusive and large object data is handled
-- as user data by pg_upgrade, which would cause failures.
+-- 3. pg_authid, since its toast table cannot be accessed when it would be
+-- needed, i.e., during authentication before we've selected a database.
SELECT relname, attname, atttypid::regtype
FROM pg_class c JOIN pg_attribute a ON c.oid = attrelid
WHERE c.oid < 16384 AND
@@ -53,12 +55,13 @@ ORDER BY 1, 2;
pg_attribute | attfdwoptions | text[]
pg_attribute | attmissingval | anyarray
pg_attribute | attoptions | text[]
+ pg_authid | rolpassword | text
pg_class | relacl | aclitem[]
pg_class | reloptions | text[]
pg_class | relpartbound | pg_node_tree
pg_largeobject | data | bytea
pg_largeobject_metadata | lomacl | aclitem[]
-(9 rows)
+(10 rows)
-- system catalogs without primary keys
--
diff --git a/src/test/regress/expected/tablespace.out b/src/test/regress/expected/tablespace.out
index 9aabb85349..dd535d41a3 100644
--- a/src/test/regress/expected/tablespace.out
+++ b/src/test/regress/expected/tablespace.out
@@ -51,13 +51,13 @@ ERROR: cannot move system relation "pg_authid_rolname_index"
REINDEX (TABLESPACE regress_tblspace) TABLE CONCURRENTLY pg_authid;
ERROR: cannot reindex system catalogs concurrently
-- toast relations, fail
-REINDEX (TABLESPACE regress_tblspace) INDEX pg_toast.pg_toast_1260_index;
-ERROR: cannot move system relation "pg_toast_1260_index"
-REINDEX (TABLESPACE regress_tblspace) INDEX CONCURRENTLY pg_toast.pg_toast_1260_index;
+REINDEX (TABLESPACE regress_tblspace) INDEX pg_toast.pg_toast_1262_index;
+ERROR: cannot move system relation "pg_toast_1262_index"
+REINDEX (TABLESPACE regress_tblspace) INDEX CONCURRENTLY pg_toast.pg_toast_1262_index;
ERROR: cannot reindex system catalogs concurrently
-REINDEX (TABLESPACE regress_tblspace) TABLE pg_toast.pg_toast_1260;
-ERROR: cannot move system relation "pg_toast_1260_index"
-REINDEX (TABLESPACE regress_tblspace) TABLE CONCURRENTLY pg_toast.pg_toast_1260;
+REINDEX (TABLESPACE regress_tblspace) TABLE pg_toast.pg_toast_1262;
+ERROR: cannot move system relation "pg_toast_1262_index"
+REINDEX (TABLESPACE regress_tblspace) TABLE CONCURRENTLY pg_toast.pg_toast_1262;
ERROR: cannot reindex system catalogs concurrently
-- system catalog, fail
REINDEX (TABLESPACE pg_global) TABLE pg_authid;
diff --git a/src/test/regress/sql/create_index.sql b/src/test/regress/sql/create_index.sql
index e296891cab..fe162cc7c3 100644
--- a/src/test/regress/sql/create_index.sql
+++ b/src/test/regress/sql/create_index.sql
@@ -1126,9 +1126,9 @@ REINDEX TABLE CONCURRENTLY concur_reindex_tab;
COMMIT;
REINDEX TABLE CONCURRENTLY pg_class; -- no catalog relation
REINDEX INDEX CONCURRENTLY pg_class_oid_index; -- no catalog index
--- These are the toast table and index of pg_authid.
-REINDEX TABLE CONCURRENTLY pg_toast.pg_toast_1260; -- no catalog toast table
-REINDEX INDEX CONCURRENTLY pg_toast.pg_toast_1260_index; -- no catalog toast index
+-- These are the toast table and index of pg_database.
+REINDEX TABLE CONCURRENTLY pg_toast.pg_toast_1262; -- no catalog toast table
+REINDEX INDEX CONCURRENTLY pg_toast.pg_toast_1262_index; -- no catalog toast index
REINDEX SYSTEM CONCURRENTLY postgres; -- not allowed for SYSTEM
REINDEX (CONCURRENTLY) SYSTEM postgres; -- ditto
REINDEX (CONCURRENTLY) SYSTEM; -- ditto
@@ -1305,8 +1305,8 @@ REINDEX SCHEMA schema_to_reindex;
RESET ROLE;
GRANT USAGE ON SCHEMA pg_toast TO regress_reindexuser;
SET SESSION ROLE regress_reindexuser;
-REINDEX TABLE pg_toast.pg_toast_1260;
-REINDEX INDEX pg_toast.pg_toast_1260_index;
+REINDEX TABLE pg_toast.pg_toast_1262;
+REINDEX INDEX pg_toast.pg_toast_1262_index;
-- Clean up
RESET ROLE;
diff --git a/src/test/regress/sql/misc_sanity.sql b/src/test/regress/sql/misc_sanity.sql
index 16f3a7c0c1..135793871b 100644
--- a/src/test/regress/sql/misc_sanity.sql
+++ b/src/test/regress/sql/misc_sanity.sql
@@ -43,6 +43,8 @@ WHERE refclassid = 0 OR refobjid = 0 OR
-- 2. pg_largeobject and pg_largeobject_metadata. Large object catalogs
-- and toast tables are mutually exclusive and large object data is handled
-- as user data by pg_upgrade, which would cause failures.
+-- 3. pg_authid, since its toast table cannot be accessed when it would be
+-- needed, i.e., during authentication before we've selected a database.
SELECT relname, attname, atttypid::regtype
FROM pg_class c JOIN pg_attribute a ON c.oid = attrelid
diff --git a/src/test/regress/sql/tablespace.sql b/src/test/regress/sql/tablespace.sql
index d274d9615e..c8b83788f0 100644
--- a/src/test/regress/sql/tablespace.sql
+++ b/src/test/regress/sql/tablespace.sql
@@ -40,10 +40,10 @@ REINDEX (TABLESPACE regress_tblspace) TABLE CONCURRENTLY pg_am;
REINDEX (TABLESPACE regress_tblspace) TABLE pg_authid;
REINDEX (TABLESPACE regress_tblspace) TABLE CONCURRENTLY pg_authid;
-- toast relations, fail
-REINDEX (TABLESPACE regress_tblspace) INDEX pg_toast.pg_toast_1260_index;
-REINDEX (TABLESPACE regress_tblspace) INDEX CONCURRENTLY pg_toast.pg_toast_1260_index;
-REINDEX (TABLESPACE regress_tblspace) TABLE pg_toast.pg_toast_1260;
-REINDEX (TABLESPACE regress_tblspace) TABLE CONCURRENTLY pg_toast.pg_toast_1260;
+REINDEX (TABLESPACE regress_tblspace) INDEX pg_toast.pg_toast_1262_index;
+REINDEX (TABLESPACE regress_tblspace) INDEX CONCURRENTLY pg_toast.pg_toast_1262_index;
+REINDEX (TABLESPACE regress_tblspace) TABLE pg_toast.pg_toast_1262;
+REINDEX (TABLESPACE regress_tblspace) TABLE CONCURRENTLY pg_toast.pg_toast_1262;
-- system catalog, fail
REINDEX (TABLESPACE pg_global) TABLE pg_authid;
REINDEX (TABLESPACE pg_global) TABLE CONCURRENTLY pg_authid;
--
2.39.5 (Apple Git-154)
Attachments:
[text/plain] v3-0001-place-limit-on-password-hash-length.patch (7.0K, ../../Zu2eT2H8OT3OXauc@nathan/2-v3-0001-place-limit-on-password-hash-length.patch)
download | inline diff:
From bb3aad9105b1997bc088403437ac6294e22809d9 Mon Sep 17 00:00:00 2001
From: Nathan Bossart <[email protected]>
Date: Thu, 19 Sep 2024 20:59:10 -0500
Subject: [PATCH v3 1/2] place limit on password hash length
---
src/backend/libpq/crypt.c | 60 ++++++++++++++++++--------
src/include/libpq/crypt.h | 10 +++++
src/test/regress/expected/password.out | 7 +++
src/test/regress/sql/password.sql | 4 ++
4 files changed, 63 insertions(+), 18 deletions(-)
diff --git a/src/backend/libpq/crypt.c b/src/backend/libpq/crypt.c
index 629e51e00b..753e5c11da 100644
--- a/src/backend/libpq/crypt.c
+++ b/src/backend/libpq/crypt.c
@@ -116,7 +116,7 @@ encrypt_password(PasswordType target_type, const char *role,
const char *password)
{
PasswordType guessed_type = get_password_type(password);
- char *encrypted_password;
+ char *encrypted_password = NULL;
const char *errstr = NULL;
if (guessed_type != PASSWORD_TYPE_PLAINTEXT)
@@ -125,32 +125,56 @@ encrypt_password(PasswordType target_type, const char *role,
* Cannot convert an already-encrypted password from one format to
* another, so return it as it is.
*/
- return pstrdup(password);
+ encrypted_password = pstrdup(password);
}
-
- switch (target_type)
+ else
{
- case PASSWORD_TYPE_MD5:
- encrypted_password = palloc(MD5_PASSWD_LEN + 1);
+ switch (target_type)
+ {
+ case PASSWORD_TYPE_MD5:
+ encrypted_password = palloc(MD5_PASSWD_LEN + 1);
- if (!pg_md5_encrypt(password, role, strlen(role),
- encrypted_password, &errstr))
- elog(ERROR, "password encryption failed: %s", errstr);
- return encrypted_password;
+ if (!pg_md5_encrypt(password, role, strlen(role),
+ encrypted_password, &errstr))
+ elog(ERROR, "password encryption failed: %s", errstr);
+ break;
- case PASSWORD_TYPE_SCRAM_SHA_256:
- return pg_be_scram_build_secret(password);
+ case PASSWORD_TYPE_SCRAM_SHA_256:
+ encrypted_password = pg_be_scram_build_secret(password);
+ break;
- case PASSWORD_TYPE_PLAINTEXT:
- elog(ERROR, "cannot encrypt password with 'plaintext'");
+ case PASSWORD_TYPE_PLAINTEXT:
+ elog(ERROR, "cannot encrypt password with 'plaintext'");
+ break;
+ }
}
+ Assert(encrypted_password);
+
/*
- * This shouldn't happen, because the above switch statements should
- * handle every combination of source and target password types.
+ * Valid password hashes may be very long, but we don't want to store
+ * anything that might need out-of-line storage, since de-TOASTing won't
+ * work during authentication because we haven't selected a database yet
+ * and cannot read pg_class. 256 bytes should be more than enough for all
+ * practical use, so fail for anything longer.
*/
- elog(ERROR, "cannot encrypt password to requested type");
- return NULL; /* keep compiler quiet */
+ if (encrypted_password && /* keep compiler quiet */
+ strlen(encrypted_password) > MAX_ENCRYPTED_PASSWORD_LEN)
+ {
+ /*
+ * We don't expect any of our own hashing routines to produce hashes
+ * that are too long.
+ */
+ Assert(guessed_type != PASSWORD_TYPE_PLAINTEXT);
+
+ ereport(ERROR,
+ (errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
+ errmsg("encrypted password is too long"),
+ errdetail("Encrypted passwords must be no longer than %d characters.",
+ MAX_ENCRYPTED_PASSWORD_LEN)));
+ }
+
+ return encrypted_password;
}
/*
diff --git a/src/include/libpq/crypt.h b/src/include/libpq/crypt.h
index f744de4d20..a2c99cd16a 100644
--- a/src/include/libpq/crypt.h
+++ b/src/include/libpq/crypt.h
@@ -15,6 +15,16 @@
#include "datatype/timestamp.h"
+/*
+ * Valid password hashes may be very long, but we don't want to store anything
+ * that might need out-of-line storage, since de-TOASTing won't work during
+ * authentication because we haven't selected a database yet and cannot read
+ * pg_class. 256 bytes should be more than enough for all practical use, and
+ * our own password encryption routines should never produce hashes longer than
+ * this.
+ */
+#define MAX_ENCRYPTED_PASSWORD_LEN (256)
+
/*
* Types of password hashes or secrets.
*
diff --git a/src/test/regress/expected/password.out b/src/test/regress/expected/password.out
index 924d6e001d..8ab995a162 100644
--- a/src/test/regress/expected/password.out
+++ b/src/test/regress/expected/password.out
@@ -127,6 +127,13 @@ SELECT rolname, rolpassword not like '%A6xHKoH/494E941doaPOYg==%' as is_rolpassw
regress_passwd_sha_len2 | t
(3 rows)
+-- Test that valid hashes that are too long are rejected
+CREATE ROLE regress_passwd10 PASSWORD 'SCRAM-SHA-256$00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000004096:NuDacwYSUxeOeFUEf3ivTQ==$Wgvq3OCYrJI6eUfvKlAzn4p/j3mzgWzXbVnWeFK1qhY=:r1qSP0j2QojCjLpFUjI0i6ckInvxJDKoyWnN3zF8WCM=';
+ERROR: encrypted password is too long
+DETAIL: Encrypted passwords must be no longer than 256 characters.
+ALTER ROLE regress_passwd9 PASSWORD 'SCRAM-SHA-256$00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000004096:NuDacwYSUxeOeFUEf3ivTQ==$Wgvq3OCYrJI6eUfvKlAzn4p/j3mzgWzXbVnWeFK1qhY=:r1qSP0j2QojCjLpFUjI0i6ckInvxJDKoyWnN3zF8WCM=';
+ERROR: encrypted password is too long
+DETAIL: Encrypted passwords must be no longer than 256 characters.
DROP ROLE regress_passwd1;
DROP ROLE regress_passwd2;
DROP ROLE regress_passwd3;
diff --git a/src/test/regress/sql/password.sql b/src/test/regress/sql/password.sql
index bb82aa4aa2..442c903c00 100644
--- a/src/test/regress/sql/password.sql
+++ b/src/test/regress/sql/password.sql
@@ -95,6 +95,10 @@ SELECT rolname, rolpassword not like '%A6xHKoH/494E941doaPOYg==%' as is_rolpassw
WHERE rolname LIKE 'regress_passwd_sha_len%'
ORDER BY rolname;
+-- Test that valid hashes that are too long are rejected
+CREATE ROLE regress_passwd10 PASSWORD 'SCRAM-SHA-256$00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000004096:NuDacwYSUxeOeFUEf3ivTQ==$Wgvq3OCYrJI6eUfvKlAzn4p/j3mzgWzXbVnWeFK1qhY=:r1qSP0j2QojCjLpFUjI0i6ckInvxJDKoyWnN3zF8WCM=';
+ALTER ROLE regress_passwd9 PASSWORD 'SCRAM-SHA-256$00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000004096:NuDacwYSUxeOeFUEf3ivTQ==$Wgvq3OCYrJI6eUfvKlAzn4p/j3mzgWzXbVnWeFK1qhY=:r1qSP0j2QojCjLpFUjI0i6ckInvxJDKoyWnN3zF8WCM=';
+
DROP ROLE regress_passwd1;
DROP ROLE regress_passwd2;
DROP ROLE regress_passwd3;
--
2.39.5 (Apple Git-154)
[text/plain] v3-0002-remove-pg_authid-s-TOAST-table.patch (9.8K, ../../Zu2eT2H8OT3OXauc@nathan/3-v3-0002-remove-pg_authid-s-TOAST-table.patch)
download | inline diff:
From 7e1736701e270d40cc1009ff4760317a8a366663 Mon Sep 17 00:00:00 2001
From: Nathan Bossart <[email protected]>
Date: Thu, 19 Sep 2024 21:09:30 -0500
Subject: [PATCH v3 2/2] remove pg_authid's TOAST table
XXX: NEEDS CATVERSION BUMP
---
src/backend/catalog/catalog.c | 4 +---
src/include/catalog/pg_authid.h | 2 --
src/test/regress/expected/create_index.out | 14 +++++++-------
src/test/regress/expected/misc_sanity.out | 5 ++++-
src/test/regress/expected/tablespace.out | 12 ++++++------
src/test/regress/sql/create_index.sql | 10 +++++-----
src/test/regress/sql/misc_sanity.sql | 2 ++
src/test/regress/sql/tablespace.sql | 8 ++++----
8 files changed, 29 insertions(+), 28 deletions(-)
diff --git a/src/backend/catalog/catalog.c b/src/backend/catalog/catalog.c
index 6c39434a30..d6b07a7865 100644
--- a/src/backend/catalog/catalog.c
+++ b/src/backend/catalog/catalog.c
@@ -300,9 +300,7 @@ IsSharedRelation(Oid relationId)
relationId == TablespaceOidIndexId)
return true;
/* These are their toast tables and toast indexes */
- if (relationId == PgAuthidToastTable ||
- relationId == PgAuthidToastIndex ||
- relationId == PgDatabaseToastTable ||
+ if (relationId == PgDatabaseToastTable ||
relationId == PgDatabaseToastIndex ||
relationId == PgDbRoleSettingToastTable ||
relationId == PgDbRoleSettingToastIndex ||
diff --git a/src/include/catalog/pg_authid.h b/src/include/catalog/pg_authid.h
index e08863f78a..e846d75731 100644
--- a/src/include/catalog/pg_authid.h
+++ b/src/include/catalog/pg_authid.h
@@ -55,8 +55,6 @@ CATALOG(pg_authid,1260,AuthIdRelationId) BKI_SHARED_RELATION BKI_ROWTYPE_OID(284
*/
typedef FormData_pg_authid *Form_pg_authid;
-DECLARE_TOAST_WITH_MACRO(pg_authid, 4175, 4176, PgAuthidToastTable, PgAuthidToastIndex);
-
DECLARE_UNIQUE_INDEX(pg_authid_rolname_index, 2676, AuthIdRolnameIndexId, pg_authid, btree(rolname name_ops));
DECLARE_UNIQUE_INDEX_PKEY(pg_authid_oid_index, 2677, AuthIdOidIndexId, pg_authid, btree(oid oid_ops));
diff --git a/src/test/regress/expected/create_index.out b/src/test/regress/expected/create_index.out
index cf6eac5734..d3358dfc39 100644
--- a/src/test/regress/expected/create_index.out
+++ b/src/test/regress/expected/create_index.out
@@ -2664,10 +2664,10 @@ REINDEX TABLE CONCURRENTLY pg_class; -- no catalog relation
ERROR: cannot reindex system catalogs concurrently
REINDEX INDEX CONCURRENTLY pg_class_oid_index; -- no catalog index
ERROR: cannot reindex system catalogs concurrently
--- These are the toast table and index of pg_authid.
-REINDEX TABLE CONCURRENTLY pg_toast.pg_toast_1260; -- no catalog toast table
+-- These are the toast table and index of pg_database.
+REINDEX TABLE CONCURRENTLY pg_toast.pg_toast_1262; -- no catalog toast table
ERROR: cannot reindex system catalogs concurrently
-REINDEX INDEX CONCURRENTLY pg_toast.pg_toast_1260_index; -- no catalog toast index
+REINDEX INDEX CONCURRENTLY pg_toast.pg_toast_1262_index; -- no catalog toast index
ERROR: cannot reindex system catalogs concurrently
REINDEX SYSTEM CONCURRENTLY postgres; -- not allowed for SYSTEM
ERROR: cannot reindex system catalogs concurrently
@@ -2974,10 +2974,10 @@ ERROR: must be owner of schema schema_to_reindex
RESET ROLE;
GRANT USAGE ON SCHEMA pg_toast TO regress_reindexuser;
SET SESSION ROLE regress_reindexuser;
-REINDEX TABLE pg_toast.pg_toast_1260;
-ERROR: permission denied for table pg_toast_1260
-REINDEX INDEX pg_toast.pg_toast_1260_index;
-ERROR: permission denied for index pg_toast_1260_index
+REINDEX TABLE pg_toast.pg_toast_1262;
+ERROR: permission denied for table pg_toast_1262
+REINDEX INDEX pg_toast.pg_toast_1262_index;
+ERROR: permission denied for index pg_toast_1262_index
-- Clean up
RESET ROLE;
REVOKE USAGE ON SCHEMA pg_toast FROM regress_reindexuser;
diff --git a/src/test/regress/expected/misc_sanity.out b/src/test/regress/expected/misc_sanity.out
index 45e7492877..b032a3f476 100644
--- a/src/test/regress/expected/misc_sanity.out
+++ b/src/test/regress/expected/misc_sanity.out
@@ -40,6 +40,8 @@ WHERE refclassid = 0 OR refobjid = 0 OR
-- 2. pg_largeobject and pg_largeobject_metadata. Large object catalogs
-- and toast tables are mutually exclusive and large object data is handled
-- as user data by pg_upgrade, which would cause failures.
+-- 3. pg_authid, since its toast table cannot be accessed when it would be
+-- needed, i.e., during authentication before we've selected a database.
SELECT relname, attname, atttypid::regtype
FROM pg_class c JOIN pg_attribute a ON c.oid = attrelid
WHERE c.oid < 16384 AND
@@ -53,12 +55,13 @@ ORDER BY 1, 2;
pg_attribute | attfdwoptions | text[]
pg_attribute | attmissingval | anyarray
pg_attribute | attoptions | text[]
+ pg_authid | rolpassword | text
pg_class | relacl | aclitem[]
pg_class | reloptions | text[]
pg_class | relpartbound | pg_node_tree
pg_largeobject | data | bytea
pg_largeobject_metadata | lomacl | aclitem[]
-(9 rows)
+(10 rows)
-- system catalogs without primary keys
--
diff --git a/src/test/regress/expected/tablespace.out b/src/test/regress/expected/tablespace.out
index 9aabb85349..dd535d41a3 100644
--- a/src/test/regress/expected/tablespace.out
+++ b/src/test/regress/expected/tablespace.out
@@ -51,13 +51,13 @@ ERROR: cannot move system relation "pg_authid_rolname_index"
REINDEX (TABLESPACE regress_tblspace) TABLE CONCURRENTLY pg_authid;
ERROR: cannot reindex system catalogs concurrently
-- toast relations, fail
-REINDEX (TABLESPACE regress_tblspace) INDEX pg_toast.pg_toast_1260_index;
-ERROR: cannot move system relation "pg_toast_1260_index"
-REINDEX (TABLESPACE regress_tblspace) INDEX CONCURRENTLY pg_toast.pg_toast_1260_index;
+REINDEX (TABLESPACE regress_tblspace) INDEX pg_toast.pg_toast_1262_index;
+ERROR: cannot move system relation "pg_toast_1262_index"
+REINDEX (TABLESPACE regress_tblspace) INDEX CONCURRENTLY pg_toast.pg_toast_1262_index;
ERROR: cannot reindex system catalogs concurrently
-REINDEX (TABLESPACE regress_tblspace) TABLE pg_toast.pg_toast_1260;
-ERROR: cannot move system relation "pg_toast_1260_index"
-REINDEX (TABLESPACE regress_tblspace) TABLE CONCURRENTLY pg_toast.pg_toast_1260;
+REINDEX (TABLESPACE regress_tblspace) TABLE pg_toast.pg_toast_1262;
+ERROR: cannot move system relation "pg_toast_1262_index"
+REINDEX (TABLESPACE regress_tblspace) TABLE CONCURRENTLY pg_toast.pg_toast_1262;
ERROR: cannot reindex system catalogs concurrently
-- system catalog, fail
REINDEX (TABLESPACE pg_global) TABLE pg_authid;
diff --git a/src/test/regress/sql/create_index.sql b/src/test/regress/sql/create_index.sql
index e296891cab..fe162cc7c3 100644
--- a/src/test/regress/sql/create_index.sql
+++ b/src/test/regress/sql/create_index.sql
@@ -1126,9 +1126,9 @@ REINDEX TABLE CONCURRENTLY concur_reindex_tab;
COMMIT;
REINDEX TABLE CONCURRENTLY pg_class; -- no catalog relation
REINDEX INDEX CONCURRENTLY pg_class_oid_index; -- no catalog index
--- These are the toast table and index of pg_authid.
-REINDEX TABLE CONCURRENTLY pg_toast.pg_toast_1260; -- no catalog toast table
-REINDEX INDEX CONCURRENTLY pg_toast.pg_toast_1260_index; -- no catalog toast index
+-- These are the toast table and index of pg_database.
+REINDEX TABLE CONCURRENTLY pg_toast.pg_toast_1262; -- no catalog toast table
+REINDEX INDEX CONCURRENTLY pg_toast.pg_toast_1262_index; -- no catalog toast index
REINDEX SYSTEM CONCURRENTLY postgres; -- not allowed for SYSTEM
REINDEX (CONCURRENTLY) SYSTEM postgres; -- ditto
REINDEX (CONCURRENTLY) SYSTEM; -- ditto
@@ -1305,8 +1305,8 @@ REINDEX SCHEMA schema_to_reindex;
RESET ROLE;
GRANT USAGE ON SCHEMA pg_toast TO regress_reindexuser;
SET SESSION ROLE regress_reindexuser;
-REINDEX TABLE pg_toast.pg_toast_1260;
-REINDEX INDEX pg_toast.pg_toast_1260_index;
+REINDEX TABLE pg_toast.pg_toast_1262;
+REINDEX INDEX pg_toast.pg_toast_1262_index;
-- Clean up
RESET ROLE;
diff --git a/src/test/regress/sql/misc_sanity.sql b/src/test/regress/sql/misc_sanity.sql
index 16f3a7c0c1..135793871b 100644
--- a/src/test/regress/sql/misc_sanity.sql
+++ b/src/test/regress/sql/misc_sanity.sql
@@ -43,6 +43,8 @@ WHERE refclassid = 0 OR refobjid = 0 OR
-- 2. pg_largeobject and pg_largeobject_metadata. Large object catalogs
-- and toast tables are mutually exclusive and large object data is handled
-- as user data by pg_upgrade, which would cause failures.
+-- 3. pg_authid, since its toast table cannot be accessed when it would be
+-- needed, i.e., during authentication before we've selected a database.
SELECT relname, attname, atttypid::regtype
FROM pg_class c JOIN pg_attribute a ON c.oid = attrelid
diff --git a/src/test/regress/sql/tablespace.sql b/src/test/regress/sql/tablespace.sql
index d274d9615e..c8b83788f0 100644
--- a/src/test/regress/sql/tablespace.sql
+++ b/src/test/regress/sql/tablespace.sql
@@ -40,10 +40,10 @@ REINDEX (TABLESPACE regress_tblspace) TABLE CONCURRENTLY pg_am;
REINDEX (TABLESPACE regress_tblspace) TABLE pg_authid;
REINDEX (TABLESPACE regress_tblspace) TABLE CONCURRENTLY pg_authid;
-- toast relations, fail
-REINDEX (TABLESPACE regress_tblspace) INDEX pg_toast.pg_toast_1260_index;
-REINDEX (TABLESPACE regress_tblspace) INDEX CONCURRENTLY pg_toast.pg_toast_1260_index;
-REINDEX (TABLESPACE regress_tblspace) TABLE pg_toast.pg_toast_1260;
-REINDEX (TABLESPACE regress_tblspace) TABLE CONCURRENTLY pg_toast.pg_toast_1260;
+REINDEX (TABLESPACE regress_tblspace) INDEX pg_toast.pg_toast_1262_index;
+REINDEX (TABLESPACE regress_tblspace) INDEX CONCURRENTLY pg_toast.pg_toast_1262_index;
+REINDEX (TABLESPACE regress_tblspace) TABLE pg_toast.pg_toast_1262;
+REINDEX (TABLESPACE regress_tblspace) TABLE CONCURRENTLY pg_toast.pg_toast_1262;
-- system catalog, fail
REINDEX (TABLESPACE pg_global) TABLE pg_authid;
REINDEX (TABLESPACE pg_global) TABLE CONCURRENTLY pg_authid;
--
2.39.5 (Apple Git-154)
^ permalink raw reply [nested|flat] 17+ messages in thread
* Re: Should rolpassword be toastable?
2024-09-20 14:06 Re: Should rolpassword be toastable? Jonathan S. Katz <[email protected]>
2024-09-20 16:09 ` Re: Should rolpassword be toastable? Nathan Bossart <[email protected]>
@ 2024-09-20 16:27 ` Tom Lane <[email protected]>
2024-09-21 20:25 ` Re: Should rolpassword be toastable? Nathan Bossart <[email protected]>
0 siblings, 1 reply; 17+ messages in thread
From: Tom Lane @ 2024-09-20 16:27 UTC (permalink / raw)
To: Nathan Bossart <[email protected]>; +Cc: Jonathan S. Katz <[email protected]>; Michael Paquier <[email protected]>; Alexander Lakhin <[email protected]>; pgsql-hackers
Nathan Bossart <[email protected]> writes:
> Here is a v3 patch set that fixes the test comment and a compiler warning
> in cfbot.
Nitpick: the message should say "%d bytes" not "%d characters",
because we're counting bytes. Passes an eyeball check otherwise.
regards, tom lane
^ permalink raw reply [nested|flat] 17+ messages in thread
* Re: Should rolpassword be toastable?
2024-09-20 14:06 Re: Should rolpassword be toastable? Jonathan S. Katz <[email protected]>
2024-09-20 16:09 ` Re: Should rolpassword be toastable? Nathan Bossart <[email protected]>
2024-09-20 16:27 ` Re: Should rolpassword be toastable? Tom Lane <[email protected]>
@ 2024-09-21 20:25 ` Nathan Bossart <[email protected]>
2024-10-03 21:27 ` Re: Should rolpassword be toastable? Nathan Bossart <[email protected]>
0 siblings, 1 reply; 17+ messages in thread
From: Nathan Bossart @ 2024-09-21 20:25 UTC (permalink / raw)
To: Tom Lane <[email protected]>; +Cc: Jonathan S. Katz <[email protected]>; Michael Paquier <[email protected]>; Alexander Lakhin <[email protected]>; pgsql-hackers
On Fri, Sep 20, 2024 at 12:27:41PM -0400, Tom Lane wrote:
> Nitpick: the message should say "%d bytes" not "%d characters",
> because we're counting bytes. Passes an eyeball check otherwise.
Thanks for reviewing. I went ahead and committed 0002 since it seems like
there's consensus on that one. I've attached a rebased version of 0001
with s/characters/bytes.
--
nathan
From f039f02cc35d57862af64648d4152693e52fbee2 Mon Sep 17 00:00:00 2001
From: Nathan Bossart <[email protected]>
Date: Thu, 19 Sep 2024 20:59:10 -0500
Subject: [PATCH v4 1/1] place limit on password hash length
---
src/backend/libpq/crypt.c | 60 ++++++++++++++++++--------
src/include/libpq/crypt.h | 10 +++++
src/test/regress/expected/password.out | 7 +++
src/test/regress/sql/password.sql | 4 ++
4 files changed, 63 insertions(+), 18 deletions(-)
diff --git a/src/backend/libpq/crypt.c b/src/backend/libpq/crypt.c
index 629e51e00b..05d289977f 100644
--- a/src/backend/libpq/crypt.c
+++ b/src/backend/libpq/crypt.c
@@ -116,7 +116,7 @@ encrypt_password(PasswordType target_type, const char *role,
const char *password)
{
PasswordType guessed_type = get_password_type(password);
- char *encrypted_password;
+ char *encrypted_password = NULL;
const char *errstr = NULL;
if (guessed_type != PASSWORD_TYPE_PLAINTEXT)
@@ -125,32 +125,56 @@ encrypt_password(PasswordType target_type, const char *role,
* Cannot convert an already-encrypted password from one format to
* another, so return it as it is.
*/
- return pstrdup(password);
+ encrypted_password = pstrdup(password);
}
-
- switch (target_type)
+ else
{
- case PASSWORD_TYPE_MD5:
- encrypted_password = palloc(MD5_PASSWD_LEN + 1);
+ switch (target_type)
+ {
+ case PASSWORD_TYPE_MD5:
+ encrypted_password = palloc(MD5_PASSWD_LEN + 1);
- if (!pg_md5_encrypt(password, role, strlen(role),
- encrypted_password, &errstr))
- elog(ERROR, "password encryption failed: %s", errstr);
- return encrypted_password;
+ if (!pg_md5_encrypt(password, role, strlen(role),
+ encrypted_password, &errstr))
+ elog(ERROR, "password encryption failed: %s", errstr);
+ break;
- case PASSWORD_TYPE_SCRAM_SHA_256:
- return pg_be_scram_build_secret(password);
+ case PASSWORD_TYPE_SCRAM_SHA_256:
+ encrypted_password = pg_be_scram_build_secret(password);
+ break;
- case PASSWORD_TYPE_PLAINTEXT:
- elog(ERROR, "cannot encrypt password with 'plaintext'");
+ case PASSWORD_TYPE_PLAINTEXT:
+ elog(ERROR, "cannot encrypt password with 'plaintext'");
+ break;
+ }
}
+ Assert(encrypted_password);
+
/*
- * This shouldn't happen, because the above switch statements should
- * handle every combination of source and target password types.
+ * Valid password hashes may be very long, but we don't want to store
+ * anything that might need out-of-line storage, since de-TOASTing won't
+ * work during authentication because we haven't selected a database yet
+ * and cannot read pg_class. 256 bytes should be more than enough for all
+ * practical use, so fail for anything longer.
*/
- elog(ERROR, "cannot encrypt password to requested type");
- return NULL; /* keep compiler quiet */
+ if (encrypted_password && /* keep compiler quiet */
+ strlen(encrypted_password) > MAX_ENCRYPTED_PASSWORD_LEN)
+ {
+ /*
+ * We don't expect any of our own hashing routines to produce hashes
+ * that are too long.
+ */
+ Assert(guessed_type != PASSWORD_TYPE_PLAINTEXT);
+
+ ereport(ERROR,
+ (errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
+ errmsg("encrypted password is too long"),
+ errdetail("Encrypted passwords must be no longer than %d bytes.",
+ MAX_ENCRYPTED_PASSWORD_LEN)));
+ }
+
+ return encrypted_password;
}
/*
diff --git a/src/include/libpq/crypt.h b/src/include/libpq/crypt.h
index f744de4d20..a2c99cd16a 100644
--- a/src/include/libpq/crypt.h
+++ b/src/include/libpq/crypt.h
@@ -15,6 +15,16 @@
#include "datatype/timestamp.h"
+/*
+ * Valid password hashes may be very long, but we don't want to store anything
+ * that might need out-of-line storage, since de-TOASTing won't work during
+ * authentication because we haven't selected a database yet and cannot read
+ * pg_class. 256 bytes should be more than enough for all practical use, and
+ * our own password encryption routines should never produce hashes longer than
+ * this.
+ */
+#define MAX_ENCRYPTED_PASSWORD_LEN (256)
+
/*
* Types of password hashes or secrets.
*
diff --git a/src/test/regress/expected/password.out b/src/test/regress/expected/password.out
index 924d6e001d..dbfe92a27d 100644
--- a/src/test/regress/expected/password.out
+++ b/src/test/regress/expected/password.out
@@ -127,6 +127,13 @@ SELECT rolname, rolpassword not like '%A6xHKoH/494E941doaPOYg==%' as is_rolpassw
regress_passwd_sha_len2 | t
(3 rows)
+-- Test that valid hashes that are too long are rejected
+CREATE ROLE regress_passwd10 PASSWORD 'SCRAM-SHA-256$00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000004096:NuDacwYSUxeOeFUEf3ivTQ==$Wgvq3OCYrJI6eUfvKlAzn4p/j3mzgWzXbVnWeFK1qhY=:r1qSP0j2QojCjLpFUjI0i6ckInvxJDKoyWnN3zF8WCM=';
+ERROR: encrypted password is too long
+DETAIL: Encrypted passwords must be no longer than 256 bytes.
+ALTER ROLE regress_passwd9 PASSWORD 'SCRAM-SHA-256$00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000004096:NuDacwYSUxeOeFUEf3ivTQ==$Wgvq3OCYrJI6eUfvKlAzn4p/j3mzgWzXbVnWeFK1qhY=:r1qSP0j2QojCjLpFUjI0i6ckInvxJDKoyWnN3zF8WCM=';
+ERROR: encrypted password is too long
+DETAIL: Encrypted passwords must be no longer than 256 bytes.
DROP ROLE regress_passwd1;
DROP ROLE regress_passwd2;
DROP ROLE regress_passwd3;
diff --git a/src/test/regress/sql/password.sql b/src/test/regress/sql/password.sql
index bb82aa4aa2..442c903c00 100644
--- a/src/test/regress/sql/password.sql
+++ b/src/test/regress/sql/password.sql
@@ -95,6 +95,10 @@ SELECT rolname, rolpassword not like '%A6xHKoH/494E941doaPOYg==%' as is_rolpassw
WHERE rolname LIKE 'regress_passwd_sha_len%'
ORDER BY rolname;
+-- Test that valid hashes that are too long are rejected
+CREATE ROLE regress_passwd10 PASSWORD 'SCRAM-SHA-256$00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000004096:NuDacwYSUxeOeFUEf3ivTQ==$Wgvq3OCYrJI6eUfvKlAzn4p/j3mzgWzXbVnWeFK1qhY=:r1qSP0j2QojCjLpFUjI0i6ckInvxJDKoyWnN3zF8WCM=';
+ALTER ROLE regress_passwd9 PASSWORD 'SCRAM-SHA-256$00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000004096:NuDacwYSUxeOeFUEf3ivTQ==$Wgvq3OCYrJI6eUfvKlAzn4p/j3mzgWzXbVnWeFK1qhY=:r1qSP0j2QojCjLpFUjI0i6ckInvxJDKoyWnN3zF8WCM=';
+
DROP ROLE regress_passwd1;
DROP ROLE regress_passwd2;
DROP ROLE regress_passwd3;
--
2.39.5 (Apple Git-154)
Attachments:
[text/plain] v4-0001-place-limit-on-password-hash-length.patch (7.0K, ../../Zu8r0oO0Vi0FxdpB@nathan/2-v4-0001-place-limit-on-password-hash-length.patch)
download | inline diff:
From f039f02cc35d57862af64648d4152693e52fbee2 Mon Sep 17 00:00:00 2001
From: Nathan Bossart <[email protected]>
Date: Thu, 19 Sep 2024 20:59:10 -0500
Subject: [PATCH v4 1/1] place limit on password hash length
---
src/backend/libpq/crypt.c | 60 ++++++++++++++++++--------
src/include/libpq/crypt.h | 10 +++++
src/test/regress/expected/password.out | 7 +++
src/test/regress/sql/password.sql | 4 ++
4 files changed, 63 insertions(+), 18 deletions(-)
diff --git a/src/backend/libpq/crypt.c b/src/backend/libpq/crypt.c
index 629e51e00b..05d289977f 100644
--- a/src/backend/libpq/crypt.c
+++ b/src/backend/libpq/crypt.c
@@ -116,7 +116,7 @@ encrypt_password(PasswordType target_type, const char *role,
const char *password)
{
PasswordType guessed_type = get_password_type(password);
- char *encrypted_password;
+ char *encrypted_password = NULL;
const char *errstr = NULL;
if (guessed_type != PASSWORD_TYPE_PLAINTEXT)
@@ -125,32 +125,56 @@ encrypt_password(PasswordType target_type, const char *role,
* Cannot convert an already-encrypted password from one format to
* another, so return it as it is.
*/
- return pstrdup(password);
+ encrypted_password = pstrdup(password);
}
-
- switch (target_type)
+ else
{
- case PASSWORD_TYPE_MD5:
- encrypted_password = palloc(MD5_PASSWD_LEN + 1);
+ switch (target_type)
+ {
+ case PASSWORD_TYPE_MD5:
+ encrypted_password = palloc(MD5_PASSWD_LEN + 1);
- if (!pg_md5_encrypt(password, role, strlen(role),
- encrypted_password, &errstr))
- elog(ERROR, "password encryption failed: %s", errstr);
- return encrypted_password;
+ if (!pg_md5_encrypt(password, role, strlen(role),
+ encrypted_password, &errstr))
+ elog(ERROR, "password encryption failed: %s", errstr);
+ break;
- case PASSWORD_TYPE_SCRAM_SHA_256:
- return pg_be_scram_build_secret(password);
+ case PASSWORD_TYPE_SCRAM_SHA_256:
+ encrypted_password = pg_be_scram_build_secret(password);
+ break;
- case PASSWORD_TYPE_PLAINTEXT:
- elog(ERROR, "cannot encrypt password with 'plaintext'");
+ case PASSWORD_TYPE_PLAINTEXT:
+ elog(ERROR, "cannot encrypt password with 'plaintext'");
+ break;
+ }
}
+ Assert(encrypted_password);
+
/*
- * This shouldn't happen, because the above switch statements should
- * handle every combination of source and target password types.
+ * Valid password hashes may be very long, but we don't want to store
+ * anything that might need out-of-line storage, since de-TOASTing won't
+ * work during authentication because we haven't selected a database yet
+ * and cannot read pg_class. 256 bytes should be more than enough for all
+ * practical use, so fail for anything longer.
*/
- elog(ERROR, "cannot encrypt password to requested type");
- return NULL; /* keep compiler quiet */
+ if (encrypted_password && /* keep compiler quiet */
+ strlen(encrypted_password) > MAX_ENCRYPTED_PASSWORD_LEN)
+ {
+ /*
+ * We don't expect any of our own hashing routines to produce hashes
+ * that are too long.
+ */
+ Assert(guessed_type != PASSWORD_TYPE_PLAINTEXT);
+
+ ereport(ERROR,
+ (errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
+ errmsg("encrypted password is too long"),
+ errdetail("Encrypted passwords must be no longer than %d bytes.",
+ MAX_ENCRYPTED_PASSWORD_LEN)));
+ }
+
+ return encrypted_password;
}
/*
diff --git a/src/include/libpq/crypt.h b/src/include/libpq/crypt.h
index f744de4d20..a2c99cd16a 100644
--- a/src/include/libpq/crypt.h
+++ b/src/include/libpq/crypt.h
@@ -15,6 +15,16 @@
#include "datatype/timestamp.h"
+/*
+ * Valid password hashes may be very long, but we don't want to store anything
+ * that might need out-of-line storage, since de-TOASTing won't work during
+ * authentication because we haven't selected a database yet and cannot read
+ * pg_class. 256 bytes should be more than enough for all practical use, and
+ * our own password encryption routines should never produce hashes longer than
+ * this.
+ */
+#define MAX_ENCRYPTED_PASSWORD_LEN (256)
+
/*
* Types of password hashes or secrets.
*
diff --git a/src/test/regress/expected/password.out b/src/test/regress/expected/password.out
index 924d6e001d..dbfe92a27d 100644
--- a/src/test/regress/expected/password.out
+++ b/src/test/regress/expected/password.out
@@ -127,6 +127,13 @@ SELECT rolname, rolpassword not like '%A6xHKoH/494E941doaPOYg==%' as is_rolpassw
regress_passwd_sha_len2 | t
(3 rows)
+-- Test that valid hashes that are too long are rejected
+CREATE ROLE regress_passwd10 PASSWORD 'SCRAM-SHA-256$00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000004096:NuDacwYSUxeOeFUEf3ivTQ==$Wgvq3OCYrJI6eUfvKlAzn4p/j3mzgWzXbVnWeFK1qhY=:r1qSP0j2QojCjLpFUjI0i6ckInvxJDKoyWnN3zF8WCM=';
+ERROR: encrypted password is too long
+DETAIL: Encrypted passwords must be no longer than 256 bytes.
+ALTER ROLE regress_passwd9 PASSWORD 'SCRAM-SHA-256$00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000004096:NuDacwYSUxeOeFUEf3ivTQ==$Wgvq3OCYrJI6eUfvKlAzn4p/j3mzgWzXbVnWeFK1qhY=:r1qSP0j2QojCjLpFUjI0i6ckInvxJDKoyWnN3zF8WCM=';
+ERROR: encrypted password is too long
+DETAIL: Encrypted passwords must be no longer than 256 bytes.
DROP ROLE regress_passwd1;
DROP ROLE regress_passwd2;
DROP ROLE regress_passwd3;
diff --git a/src/test/regress/sql/password.sql b/src/test/regress/sql/password.sql
index bb82aa4aa2..442c903c00 100644
--- a/src/test/regress/sql/password.sql
+++ b/src/test/regress/sql/password.sql
@@ -95,6 +95,10 @@ SELECT rolname, rolpassword not like '%A6xHKoH/494E941doaPOYg==%' as is_rolpassw
WHERE rolname LIKE 'regress_passwd_sha_len%'
ORDER BY rolname;
+-- Test that valid hashes that are too long are rejected
+CREATE ROLE regress_passwd10 PASSWORD 'SCRAM-SHA-256$00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000004096:NuDacwYSUxeOeFUEf3ivTQ==$Wgvq3OCYrJI6eUfvKlAzn4p/j3mzgWzXbVnWeFK1qhY=:r1qSP0j2QojCjLpFUjI0i6ckInvxJDKoyWnN3zF8WCM=';
+ALTER ROLE regress_passwd9 PASSWORD 'SCRAM-SHA-256$00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000004096:NuDacwYSUxeOeFUEf3ivTQ==$Wgvq3OCYrJI6eUfvKlAzn4p/j3mzgWzXbVnWeFK1qhY=:r1qSP0j2QojCjLpFUjI0i6ckInvxJDKoyWnN3zF8WCM=';
+
DROP ROLE regress_passwd1;
DROP ROLE regress_passwd2;
DROP ROLE regress_passwd3;
--
2.39.5 (Apple Git-154)
^ permalink raw reply [nested|flat] 17+ messages in thread
* Re: Should rolpassword be toastable?
2024-09-20 14:06 Re: Should rolpassword be toastable? Jonathan S. Katz <[email protected]>
2024-09-20 16:09 ` Re: Should rolpassword be toastable? Nathan Bossart <[email protected]>
2024-09-20 16:27 ` Re: Should rolpassword be toastable? Tom Lane <[email protected]>
2024-09-21 20:25 ` Re: Should rolpassword be toastable? Nathan Bossart <[email protected]>
@ 2024-10-03 21:27 ` Nathan Bossart <[email protected]>
2024-10-03 21:39 ` Re: Should rolpassword be toastable? Tom Lane <[email protected]>
0 siblings, 1 reply; 17+ messages in thread
From: Nathan Bossart @ 2024-10-03 21:27 UTC (permalink / raw)
To: Tom Lane <[email protected]>; +Cc: Jonathan S. Katz <[email protected]>; Michael Paquier <[email protected]>; Alexander Lakhin <[email protected]>; pgsql-hackers
On Sat, Sep 21, 2024 at 03:25:54PM -0500, Nathan Bossart wrote:
> Thanks for reviewing. I went ahead and committed 0002 since it seems like
> there's consensus on that one. I've attached a rebased version of 0001
> with s/characters/bytes.
For the reasons discussed upthread [0], I can't bring myself to add an
arbitrary limit to the password hash length. I am going to leave 0001
uncommitted for now.
[0] https://postgr.es/m/Zu2eT2H8OT3OXauc%40nathan
--
nathan
^ permalink raw reply [nested|flat] 17+ messages in thread
* Re: Should rolpassword be toastable?
2024-09-20 14:06 Re: Should rolpassword be toastable? Jonathan S. Katz <[email protected]>
2024-09-20 16:09 ` Re: Should rolpassword be toastable? Nathan Bossart <[email protected]>
2024-09-20 16:27 ` Re: Should rolpassword be toastable? Tom Lane <[email protected]>
2024-09-21 20:25 ` Re: Should rolpassword be toastable? Nathan Bossart <[email protected]>
2024-10-03 21:27 ` Re: Should rolpassword be toastable? Nathan Bossart <[email protected]>
@ 2024-10-03 21:39 ` Tom Lane <[email protected]>
2024-10-03 22:17 ` Re: Should rolpassword be toastable? Nathan Bossart <[email protected]>
0 siblings, 1 reply; 17+ messages in thread
From: Tom Lane @ 2024-10-03 21:39 UTC (permalink / raw)
To: Nathan Bossart <[email protected]>; +Cc: Jonathan S. Katz <[email protected]>; Michael Paquier <[email protected]>; Alexander Lakhin <[email protected]>; pgsql-hackers
Nathan Bossart <[email protected]> writes:
> For the reasons discussed upthread [0], I can't bring myself to add an
> arbitrary limit to the password hash length. I am going to leave 0001
> uncommitted for now.
> [0] https://postgr.es/m/Zu2eT2H8OT3OXauc%40nathan
I'm confused, as in [0] you said
>> ... But on the off-chance
>> that someone is building a custom driver that generates long hashes for
>> whatever reason, I'd imagine that a clear error would be more helpful than
>> "row is too big."
I agree with the idea that complaining about the password being too
long is far more intelligible than that. Another problem with
leaving it as it stands in HEAD is that the effective limit is now
platform-specific, if not indeed dependent on the phase of the moon
(or at least, the other contents of the pg_authid row). I fear it
would be very easy to construct cases where a password is accepted
on one machine but fails with "row is too big" on another. A
uniform limit seems much less fraught with surprises.
regards, tom lane
^ permalink raw reply [nested|flat] 17+ messages in thread
* Re: Should rolpassword be toastable?
2024-09-20 14:06 Re: Should rolpassword be toastable? Jonathan S. Katz <[email protected]>
2024-09-20 16:09 ` Re: Should rolpassword be toastable? Nathan Bossart <[email protected]>
2024-09-20 16:27 ` Re: Should rolpassword be toastable? Tom Lane <[email protected]>
2024-09-21 20:25 ` Re: Should rolpassword be toastable? Nathan Bossart <[email protected]>
2024-10-03 21:27 ` Re: Should rolpassword be toastable? Nathan Bossart <[email protected]>
2024-10-03 21:39 ` Re: Should rolpassword be toastable? Tom Lane <[email protected]>
@ 2024-10-03 22:17 ` Nathan Bossart <[email protected]>
2024-10-03 22:24 ` Re: Should rolpassword be toastable? Tom Lane <[email protected]>
0 siblings, 1 reply; 17+ messages in thread
From: Nathan Bossart @ 2024-10-03 22:17 UTC (permalink / raw)
To: Tom Lane <[email protected]>; +Cc: Jonathan S. Katz <[email protected]>; Michael Paquier <[email protected]>; Alexander Lakhin <[email protected]>; pgsql-hackers
On Thu, Oct 03, 2024 at 05:39:06PM -0400, Tom Lane wrote:
> I agree with the idea that complaining about the password being too
> long is far more intelligible than that. Another problem with
> leaving it as it stands in HEAD is that the effective limit is now
> platform-specific, if not indeed dependent on the phase of the moon
> (or at least, the other contents of the pg_authid row). I fear it
> would be very easy to construct cases where a password is accepted
> on one machine but fails with "row is too big" on another. A
> uniform limit seems much less fraught with surprises.
I don't mind proceeding with the patch if there is strong support for it.
I wavered only because it's hard to be confident that we are choosing the
right limit. AFAICT 256 bytes ought to be sufficient to avoid "row is too
big" errors independent of BLCKSZ today, but maybe someone will add another
varlena column in the future that breaks it. Or maybe we add a new
password hashing method that produces longer strings. Or maybe someone is
doing something really out there like storing additional information in the
salt. I don't have any reason to believe that any of these things are
happening or are likely to happen anytime soon, but they seem similar in
likelihood to someone building a custom driver that generates ginormous
hashes. But I can also buy the argument that none of this is a strong
enough reason to avoid making the error message nicer...
--
nathan
^ permalink raw reply [nested|flat] 17+ messages in thread
* Re: Should rolpassword be toastable?
2024-09-20 14:06 Re: Should rolpassword be toastable? Jonathan S. Katz <[email protected]>
2024-09-20 16:09 ` Re: Should rolpassword be toastable? Nathan Bossart <[email protected]>
2024-09-20 16:27 ` Re: Should rolpassword be toastable? Tom Lane <[email protected]>
2024-09-21 20:25 ` Re: Should rolpassword be toastable? Nathan Bossart <[email protected]>
2024-10-03 21:27 ` Re: Should rolpassword be toastable? Nathan Bossart <[email protected]>
2024-10-03 21:39 ` Re: Should rolpassword be toastable? Tom Lane <[email protected]>
2024-10-03 22:17 ` Re: Should rolpassword be toastable? Nathan Bossart <[email protected]>
@ 2024-10-03 22:24 ` Tom Lane <[email protected]>
2024-10-03 23:29 ` Re: Should rolpassword be toastable? Jacob Champion <[email protected]>
0 siblings, 1 reply; 17+ messages in thread
From: Tom Lane @ 2024-10-03 22:24 UTC (permalink / raw)
To: Nathan Bossart <[email protected]>; +Cc: Jonathan S. Katz <[email protected]>; Michael Paquier <[email protected]>; Alexander Lakhin <[email protected]>; pgsql-hackers
Nathan Bossart <[email protected]> writes:
> I don't mind proceeding with the patch if there is strong support for it.
> I wavered only because it's hard to be confident that we are choosing the
> right limit.
I'm not that fussed about it; surely 256 is more than anyone is using?
If not, we'll get push-back and then we can have a discussion about the
correct limit that's informed by more than guesswork.
> ... But I can also buy the argument that none of this is a strong
> enough reason to avoid making the error message nicer...
There's that, and there's also the fact that if you assume someone is
using $sufficiently-long-passwords then we might have broken their
use-case already. We can't have much of a conversation here without
a concrete case to look at.
regards, tom lane
^ permalink raw reply [nested|flat] 17+ messages in thread
* Re: Should rolpassword be toastable?
2024-09-20 14:06 Re: Should rolpassword be toastable? Jonathan S. Katz <[email protected]>
2024-09-20 16:09 ` Re: Should rolpassword be toastable? Nathan Bossart <[email protected]>
2024-09-20 16:27 ` Re: Should rolpassword be toastable? Tom Lane <[email protected]>
2024-09-21 20:25 ` Re: Should rolpassword be toastable? Nathan Bossart <[email protected]>
2024-10-03 21:27 ` Re: Should rolpassword be toastable? Nathan Bossart <[email protected]>
2024-10-03 21:39 ` Re: Should rolpassword be toastable? Tom Lane <[email protected]>
2024-10-03 22:17 ` Re: Should rolpassword be toastable? Nathan Bossart <[email protected]>
2024-10-03 22:24 ` Re: Should rolpassword be toastable? Tom Lane <[email protected]>
@ 2024-10-03 23:29 ` Jacob Champion <[email protected]>
2024-10-04 02:17 ` Re: Should rolpassword be toastable? Jonathan S. Katz <[email protected]>
0 siblings, 1 reply; 17+ messages in thread
From: Jacob Champion @ 2024-10-03 23:29 UTC (permalink / raw)
To: Tom Lane <[email protected]>; +Cc: Nathan Bossart <[email protected]>; Jonathan S. Katz <[email protected]>; Michael Paquier <[email protected]>; Alexander Lakhin <[email protected]>; pgsql-hackers
On Thu, Oct 3, 2024 at 3:25 PM Tom Lane <[email protected]> wrote:
>
> Nathan Bossart <[email protected]> writes:
> > I don't mind proceeding with the patch if there is strong support for it.
> > I wavered only because it's hard to be confident that we are choosing the
> > right limit.
>
> I'm not that fussed about it; surely 256 is more than anyone is using?
> If not, we'll get push-back and then we can have a discussion about the
> correct limit that's informed by more than guesswork.
+1.
Next up is probably SCRAM-SHA-512, which should still have smaller
entries than that -- 222 bytes, I think, with 128-bit salts and a
5-digit iteration count?
--Jacob
^ permalink raw reply [nested|flat] 17+ messages in thread
* Re: Should rolpassword be toastable?
2024-09-20 14:06 Re: Should rolpassword be toastable? Jonathan S. Katz <[email protected]>
2024-09-20 16:09 ` Re: Should rolpassword be toastable? Nathan Bossart <[email protected]>
2024-09-20 16:27 ` Re: Should rolpassword be toastable? Tom Lane <[email protected]>
2024-09-21 20:25 ` Re: Should rolpassword be toastable? Nathan Bossart <[email protected]>
2024-10-03 21:27 ` Re: Should rolpassword be toastable? Nathan Bossart <[email protected]>
2024-10-03 21:39 ` Re: Should rolpassword be toastable? Tom Lane <[email protected]>
2024-10-03 22:17 ` Re: Should rolpassword be toastable? Nathan Bossart <[email protected]>
2024-10-03 22:24 ` Re: Should rolpassword be toastable? Tom Lane <[email protected]>
2024-10-03 23:29 ` Re: Should rolpassword be toastable? Jacob Champion <[email protected]>
@ 2024-10-04 02:17 ` Jonathan S. Katz <[email protected]>
2024-10-04 02:33 ` Re: Should rolpassword be toastable? Tom Lane <[email protected]>
0 siblings, 1 reply; 17+ messages in thread
From: Jonathan S. Katz @ 2024-10-04 02:17 UTC (permalink / raw)
To: Jacob Champion <[email protected]>; Tom Lane <[email protected]>; +Cc: Nathan Bossart <[email protected]>; Michael Paquier <[email protected]>; Alexander Lakhin <[email protected]>; pgsql-hackers
On 10/3/24 7:29 PM, Jacob Champion wrote:
> On Thu, Oct 3, 2024 at 3:25 PM Tom Lane <[email protected]> wrote:
>>
>> Nathan Bossart <[email protected]> writes:
>>> I don't mind proceeding with the patch if there is strong support for it.
>>> I wavered only because it's hard to be confident that we are choosing the
>>> right limit.
>>
>> I'm not that fussed about it; surely 256 is more than anyone is using?
>> If not, we'll get push-back and then we can have a discussion about the
>> correct limit that's informed by more than guesswork.
>
> +1.
>
> Next up is probably SCRAM-SHA-512, which should still have smaller
> entries than that -- 222 bytes, I think, with 128-bit salts and a
> 5-digit iteration count?
The challenge is that salts can be an arbitrary length, even today (as
can the iterator value, though IIRC I think we check if it's in int
bounds, and a large iterator becomes pretty impractical for usage).
Probabalistically, it's unlikely there are many very large salts in the
wild (though I don't have data on that) and most folks are using the
default length, but that probability isn't 0.
I think Tom's initial suggestion (BLCKSZ/2) is better than 256, given we
really don't know what' out there in the wild, and this could end up
being a breaking change. Every other type in pg_authid is pretty small.
That said, I'm also imagining other things we may add that could require
TOAST support (remembering previous passwords? storing multiple
passwords options)?
Thanks,
Jonathan
Attachments:
[application/pgp-signature] OpenPGP_signature.asc (840B, ../../[email protected]/2-OpenPGP_signature.asc)
download
^ permalink raw reply [nested|flat] 17+ messages in thread
* Re: Should rolpassword be toastable?
2024-09-20 14:06 Re: Should rolpassword be toastable? Jonathan S. Katz <[email protected]>
2024-09-20 16:09 ` Re: Should rolpassword be toastable? Nathan Bossart <[email protected]>
2024-09-20 16:27 ` Re: Should rolpassword be toastable? Tom Lane <[email protected]>
2024-09-21 20:25 ` Re: Should rolpassword be toastable? Nathan Bossart <[email protected]>
2024-10-03 21:27 ` Re: Should rolpassword be toastable? Nathan Bossart <[email protected]>
2024-10-03 21:39 ` Re: Should rolpassword be toastable? Tom Lane <[email protected]>
2024-10-03 22:17 ` Re: Should rolpassword be toastable? Nathan Bossart <[email protected]>
2024-10-03 22:24 ` Re: Should rolpassword be toastable? Tom Lane <[email protected]>
2024-10-03 23:29 ` Re: Should rolpassword be toastable? Jacob Champion <[email protected]>
2024-10-04 02:17 ` Re: Should rolpassword be toastable? Jonathan S. Katz <[email protected]>
@ 2024-10-04 02:33 ` Tom Lane <[email protected]>
2024-10-04 14:47 ` Re: Should rolpassword be toastable? Nathan Bossart <[email protected]>
0 siblings, 1 reply; 17+ messages in thread
From: Tom Lane @ 2024-10-04 02:33 UTC (permalink / raw)
To: Jonathan S. Katz <[email protected]>; +Cc: Jacob Champion <[email protected]>; Nathan Bossart <[email protected]>; Michael Paquier <[email protected]>; Alexander Lakhin <[email protected]>; pgsql-hackers
"Jonathan S. Katz" <[email protected]> writes:
> I think Tom's initial suggestion (BLCKSZ/2) is better than 256, given we
> really don't know what' out there in the wild, and this could end up
> being a breaking change. Every other type in pg_authid is pretty small.
I'm having second thoughts about that though, based on the argument
that we don't really want a platform-dependent limit here.
Admittedly, nobody changes BLCKSZ on production systems, but it's
still theoretically an issue. I don't have a problem with selecting
a larger limit such as 512 or 1024 though.
> That said, I'm also imagining other things we may add that could require
> TOAST support (remembering previous passwords? storing multiple
> passwords options)?
Things like previous passwords probably don't need to be accessed
during authentication, so there are at least a couple of ways we
could do that:
* put the previous passwords in an auxiliary table;
* put back pg_authid's toast table, but mark rolpassword as
"STORAGE MAIN" so it doesn't go to toast, while letting columns
that don't need to be touched at startup go there.
However, if you wanted to allow multiple passwords I'm not
sure about a good way.
regards, tom lane
^ permalink raw reply [nested|flat] 17+ messages in thread
* Re: Should rolpassword be toastable?
2024-09-20 14:06 Re: Should rolpassword be toastable? Jonathan S. Katz <[email protected]>
2024-09-20 16:09 ` Re: Should rolpassword be toastable? Nathan Bossart <[email protected]>
2024-09-20 16:27 ` Re: Should rolpassword be toastable? Tom Lane <[email protected]>
2024-09-21 20:25 ` Re: Should rolpassword be toastable? Nathan Bossart <[email protected]>
2024-10-03 21:27 ` Re: Should rolpassword be toastable? Nathan Bossart <[email protected]>
2024-10-03 21:39 ` Re: Should rolpassword be toastable? Tom Lane <[email protected]>
2024-10-03 22:17 ` Re: Should rolpassword be toastable? Nathan Bossart <[email protected]>
2024-10-03 22:24 ` Re: Should rolpassword be toastable? Tom Lane <[email protected]>
2024-10-03 23:29 ` Re: Should rolpassword be toastable? Jacob Champion <[email protected]>
2024-10-04 02:17 ` Re: Should rolpassword be toastable? Jonathan S. Katz <[email protected]>
2024-10-04 02:33 ` Re: Should rolpassword be toastable? Tom Lane <[email protected]>
@ 2024-10-04 14:47 ` Nathan Bossart <[email protected]>
2024-10-06 09:42 ` Re: Should rolpassword be toastable? Hannu Krosing <[email protected]>
0 siblings, 1 reply; 17+ messages in thread
From: Nathan Bossart @ 2024-10-04 14:47 UTC (permalink / raw)
To: Tom Lane <[email protected]>; +Cc: Jonathan S. Katz <[email protected]>; Jacob Champion <[email protected]>; Michael Paquier <[email protected]>; Alexander Lakhin <[email protected]>; pgsql-hackers
On Thu, Oct 03, 2024 at 10:33:04PM -0400, Tom Lane wrote:
> "Jonathan S. Katz" <[email protected]> writes:
>> I think Tom's initial suggestion (BLCKSZ/2) is better than 256, given we
>> really don't know what' out there in the wild, and this could end up
>> being a breaking change. Every other type in pg_authid is pretty small.
>
> I'm having second thoughts about that though, based on the argument
> that we don't really want a platform-dependent limit here.
> Admittedly, nobody changes BLCKSZ on production systems, but it's
> still theoretically an issue. I don't have a problem with selecting
> a larger limit such as 512 or 1024 though.
Since BLCKSZ can be as low as 1024, I think 512 would be a good choice.
> However, if you wanted to allow multiple passwords I'm not
> sure about a good way.
The most recent proposal I'm aware of [0] did seem to target that use-case.
One option might be to move rolpassword to a different catalog. In any
case, I don't think it matters much for the patch at hand.
[0] https://postgr.es/m/CAGB%2BVh5SQQorNDEKP%2B0G%3DsmxHRhbhs%2BVkmQWD5Vh98fmn8X4dg%40mail.gmail.com
--
nathan
^ permalink raw reply [nested|flat] 17+ messages in thread
* Re: Should rolpassword be toastable?
2024-09-20 14:06 Re: Should rolpassword be toastable? Jonathan S. Katz <[email protected]>
2024-09-20 16:09 ` Re: Should rolpassword be toastable? Nathan Bossart <[email protected]>
2024-09-20 16:27 ` Re: Should rolpassword be toastable? Tom Lane <[email protected]>
2024-09-21 20:25 ` Re: Should rolpassword be toastable? Nathan Bossart <[email protected]>
2024-10-03 21:27 ` Re: Should rolpassword be toastable? Nathan Bossart <[email protected]>
2024-10-03 21:39 ` Re: Should rolpassword be toastable? Tom Lane <[email protected]>
2024-10-03 22:17 ` Re: Should rolpassword be toastable? Nathan Bossart <[email protected]>
2024-10-03 22:24 ` Re: Should rolpassword be toastable? Tom Lane <[email protected]>
2024-10-03 23:29 ` Re: Should rolpassword be toastable? Jacob Champion <[email protected]>
2024-10-04 02:17 ` Re: Should rolpassword be toastable? Jonathan S. Katz <[email protected]>
2024-10-04 02:33 ` Re: Should rolpassword be toastable? Tom Lane <[email protected]>
2024-10-04 14:47 ` Re: Should rolpassword be toastable? Nathan Bossart <[email protected]>
@ 2024-10-06 09:42 ` Hannu Krosing <[email protected]>
2024-10-06 13:12 ` Re: Should rolpassword be toastable? Nathan Bossart <[email protected]>
0 siblings, 1 reply; 17+ messages in thread
From: Hannu Krosing @ 2024-10-06 09:42 UTC (permalink / raw)
To: Nathan Bossart <[email protected]>; +Cc: Tom Lane <[email protected]>; Jonathan S. Katz <[email protected]>; Jacob Champion <[email protected]>; Michael Paquier <[email protected]>; Alexander Lakhin <[email protected]>; pgsql-hackers
On Fri, Oct 4, 2024 at 4:48 PM Nathan Bossart <[email protected]> wrote:
>
..
> Since BLCKSZ can be as low as 1024, I think 512 would be a good choice.
>
Where did you get the minimal value of 1024 from ?
I vaguely remember someone testing with 256 at some point in the past
---
Hannu
^ permalink raw reply [nested|flat] 17+ messages in thread
* Re: Should rolpassword be toastable?
2024-09-20 14:06 Re: Should rolpassword be toastable? Jonathan S. Katz <[email protected]>
2024-09-20 16:09 ` Re: Should rolpassword be toastable? Nathan Bossart <[email protected]>
2024-09-20 16:27 ` Re: Should rolpassword be toastable? Tom Lane <[email protected]>
2024-09-21 20:25 ` Re: Should rolpassword be toastable? Nathan Bossart <[email protected]>
2024-10-03 21:27 ` Re: Should rolpassword be toastable? Nathan Bossart <[email protected]>
2024-10-03 21:39 ` Re: Should rolpassword be toastable? Tom Lane <[email protected]>
2024-10-03 22:17 ` Re: Should rolpassword be toastable? Nathan Bossart <[email protected]>
2024-10-03 22:24 ` Re: Should rolpassword be toastable? Tom Lane <[email protected]>
2024-10-03 23:29 ` Re: Should rolpassword be toastable? Jacob Champion <[email protected]>
2024-10-04 02:17 ` Re: Should rolpassword be toastable? Jonathan S. Katz <[email protected]>
2024-10-04 02:33 ` Re: Should rolpassword be toastable? Tom Lane <[email protected]>
2024-10-04 14:47 ` Re: Should rolpassword be toastable? Nathan Bossart <[email protected]>
2024-10-06 09:42 ` Re: Should rolpassword be toastable? Hannu Krosing <[email protected]>
@ 2024-10-06 13:12 ` Nathan Bossart <[email protected]>
2024-10-07 15:58 ` Re: Should rolpassword be toastable? Nathan Bossart <[email protected]>
0 siblings, 1 reply; 17+ messages in thread
From: Nathan Bossart @ 2024-10-06 13:12 UTC (permalink / raw)
To: Hannu Krosing <[email protected]>; +Cc: Tom Lane <[email protected]>; Jonathan S. Katz <[email protected]>; Jacob Champion <[email protected]>; Michael Paquier <[email protected]>; Alexander Lakhin <[email protected]>; pgsql-hackers
On Sun, Oct 06, 2024 at 11:42:53AM +0200, Hannu Krosing wrote:
> On Fri, Oct 4, 2024 at 4:48 PM Nathan Bossart <[email protected]> wrote:
>> Since BLCKSZ can be as low as 1024, I think 512 would be a good choice.
>
> Where did you get the minimal value of 1024 from ?
https://www.postgresql.org/docs/devel/install-make.html#CONFIGURE-OPTION-WITH-BLOCKSIZE
--
nathan
^ permalink raw reply [nested|flat] 17+ messages in thread
* Re: Should rolpassword be toastable?
2024-09-20 14:06 Re: Should rolpassword be toastable? Jonathan S. Katz <[email protected]>
2024-09-20 16:09 ` Re: Should rolpassword be toastable? Nathan Bossart <[email protected]>
2024-09-20 16:27 ` Re: Should rolpassword be toastable? Tom Lane <[email protected]>
2024-09-21 20:25 ` Re: Should rolpassword be toastable? Nathan Bossart <[email protected]>
2024-10-03 21:27 ` Re: Should rolpassword be toastable? Nathan Bossart <[email protected]>
2024-10-03 21:39 ` Re: Should rolpassword be toastable? Tom Lane <[email protected]>
2024-10-03 22:17 ` Re: Should rolpassword be toastable? Nathan Bossart <[email protected]>
2024-10-03 22:24 ` Re: Should rolpassword be toastable? Tom Lane <[email protected]>
2024-10-03 23:29 ` Re: Should rolpassword be toastable? Jacob Champion <[email protected]>
2024-10-04 02:17 ` Re: Should rolpassword be toastable? Jonathan S. Katz <[email protected]>
2024-10-04 02:33 ` Re: Should rolpassword be toastable? Tom Lane <[email protected]>
2024-10-04 14:47 ` Re: Should rolpassword be toastable? Nathan Bossart <[email protected]>
2024-10-06 09:42 ` Re: Should rolpassword be toastable? Hannu Krosing <[email protected]>
2024-10-06 13:12 ` Re: Should rolpassword be toastable? Nathan Bossart <[email protected]>
@ 2024-10-07 15:58 ` Nathan Bossart <[email protected]>
0 siblings, 0 replies; 17+ messages in thread
From: Nathan Bossart @ 2024-10-07 15:58 UTC (permalink / raw)
To: Hannu Krosing <[email protected]>; +Cc: Tom Lane <[email protected]>; Jonathan S. Katz <[email protected]>; Jacob Champion <[email protected]>; Michael Paquier <[email protected]>; Alexander Lakhin <[email protected]>; pgsql-hackers
Committed with the limit set to 512 bytes. We have plenty of time to
adjust this limit as needed before it takes effect in v18.
--
nathan
^ permalink raw reply [nested|flat] 17+ messages in thread
end of thread, other threads:[~2024-10-07 15:58 UTC | newest]
Thread overview: 17+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2019-01-02 19:14 [PATCH v5 1/2] Report progress of CREATE INDEX operations Alvaro Herrera <[email protected]>
2023-12-04 11:23 [PATCH v12 2/7] Row pattern recognition patch (parse/analysis). Tatsuo Ishii <[email protected]>
2024-09-20 14:06 Re: Should rolpassword be toastable? Jonathan S. Katz <[email protected]>
2024-09-20 16:09 ` Re: Should rolpassword be toastable? Nathan Bossart <[email protected]>
2024-09-20 16:27 ` Re: Should rolpassword be toastable? Tom Lane <[email protected]>
2024-09-21 20:25 ` Re: Should rolpassword be toastable? Nathan Bossart <[email protected]>
2024-10-03 21:27 ` Re: Should rolpassword be toastable? Nathan Bossart <[email protected]>
2024-10-03 21:39 ` Re: Should rolpassword be toastable? Tom Lane <[email protected]>
2024-10-03 22:17 ` Re: Should rolpassword be toastable? Nathan Bossart <[email protected]>
2024-10-03 22:24 ` Re: Should rolpassword be toastable? Tom Lane <[email protected]>
2024-10-03 23:29 ` Re: Should rolpassword be toastable? Jacob Champion <[email protected]>
2024-10-04 02:17 ` Re: Should rolpassword be toastable? Jonathan S. Katz <[email protected]>
2024-10-04 02:33 ` Re: Should rolpassword be toastable? Tom Lane <[email protected]>
2024-10-04 14:47 ` Re: Should rolpassword be toastable? Nathan Bossart <[email protected]>
2024-10-06 09:42 ` Re: Should rolpassword be toastable? Hannu Krosing <[email protected]>
2024-10-06 13:12 ` Re: Should rolpassword be toastable? Nathan Bossart <[email protected]>
2024-10-07 15:58 ` Re: Should rolpassword be toastable? 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