public inbox for [email protected]
help / color / mirror / Atom feedRe: pg_upgrade verbosity when redirecting output to log file
7+ messages / 5 participants
[nested] [flat]
* Re: pg_upgrade verbosity when redirecting output to log file
@ 2022-02-16 04:09 Thomas Munro <[email protected]>
0 siblings, 1 reply; 7+ messages in thread
From: Thomas Munro @ 2022-02-16 04:09 UTC (permalink / raw)
To: Bruce Momjian <[email protected]>; +Cc: Andres Freund <[email protected]>; Tom Lane <[email protected]>; pgsql-hackers
On Tue, Jan 11, 2022 at 4:42 AM Bruce Momjian <[email protected]> wrote:
> On Sun, Jan 9, 2022 at 10:39:58PM -0800, Andres Freund wrote:
> > On 2022-01-10 01:14:32 -0500, Tom Lane wrote:
> > > I think I'd vote for just nuking that output altogether.
> > > It seems of very dubious value.
> >
> > It seems worthwhile in some form - on large cluster in copy mode, the "Copying
> > user relation files" step can take *quite* a while, and even link/clone mode
> > aren't fast. But perhaps what'd be really needed is something counting up
> > actual progress in percentage of files and/or space...
> >
> > I think just coupling it to verbose mode makes the most sense, for now?
>
> All of this logging is from the stage where I was excited pg_upgrade
> worked, and I wanted to give clear output if it failed in some way ---
> printing the file names seems like an easy solution. I agree at this
> point that logging should be reduced, and if they want more logging, the
> verbose option is the right way to get it.
+1
^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: pg_upgrade verbosity when redirecting output to log file
@ 2022-02-19 01:20 Andres Freund <[email protected]>
parent: Thomas Munro <[email protected]>
0 siblings, 1 reply; 7+ messages in thread
From: Andres Freund @ 2022-02-19 01:20 UTC (permalink / raw)
To: Thomas Munro <[email protected]>; +Cc: Bruce Momjian <[email protected]>; Tom Lane <[email protected]>; pgsql-hackers
Hi,
On 2022-02-16 17:09:34 +1300, Thomas Munro wrote:
> On Tue, Jan 11, 2022 at 4:42 AM Bruce Momjian <[email protected]> wrote:
> > On Sun, Jan 9, 2022 at 10:39:58PM -0800, Andres Freund wrote:
> > > On 2022-01-10 01:14:32 -0500, Tom Lane wrote:
> > > > I think I'd vote for just nuking that output altogether.
> > > > It seems of very dubious value.
> > >
> > > It seems worthwhile in some form - on large cluster in copy mode, the "Copying
> > > user relation files" step can take *quite* a while, and even link/clone mode
> > > aren't fast. But perhaps what'd be really needed is something counting up
> > > actual progress in percentage of files and/or space...
> > >
> > > I think just coupling it to verbose mode makes the most sense, for now?
> >
> > All of this logging is from the stage where I was excited pg_upgrade
> > worked, and I wanted to give clear output if it failed in some way ---
> > printing the file names seems like an easy solution. I agree at this
> > point that logging should be reduced, and if they want more logging, the
> > verbose option is the right way to get it.
>
> +1
I got a bit stuck on how to best resolve this. I felt bad about removing all
interactive progress, because a pg_upgrade can take a while after all. But
it's also not easy to come up with some good, without a substantially bigger
effort than I want to invest.
After all, I just want to be able to read check-world output. Nearly half of
which is pg_upgrade test output right now.
The attached is my attempt at coming up with something halfway sane without
rewriting pg_upgrade logging entirely. I think it mostly ends up with at least
as sane output as the current code. I needed to add a separate
prep_status_progress() function to make that work.
Greetings,
Andres Freund
Attachments:
[text/x-diff] v2-0001-pg_upgrade-Don-t-print-progress-status-when-outpu.patch (6.9K, ../../[email protected]/2-v2-0001-pg_upgrade-Don-t-print-progress-status-when-outpu.patch)
download | inline diff:
From a04d989a64e6c6bc273b0c8c0d778d5f66f00158 Mon Sep 17 00:00:00 2001
From: Andres Freund <[email protected]>
Date: Fri, 18 Feb 2022 17:16:37 -0800
Subject: [PATCH v2] pg_upgrade: Don't print progress status when output is not
a tty.
Until this change pg_upgrade with output redirected to a file / pipe would end
up printing all files in the cluster. This has made check-world output
exceedingly verbose.
Author: Andres Freund
Discussion: https://postgr.es/m/CA+hUKGKjrV61ZVJ8OSag+3rKRmCZXPc03bDyWMqhXg3rdZ=fOw@mail.gmail.com
---
src/bin/pg_upgrade/dump.c | 2 +-
src/bin/pg_upgrade/option.c | 2 +
src/bin/pg_upgrade/pg_upgrade.c | 2 +-
src/bin/pg_upgrade/pg_upgrade.h | 2 +
src/bin/pg_upgrade/relfilenode.c | 6 +--
src/bin/pg_upgrade/util.c | 63 ++++++++++++++++++++++++++------
6 files changed, 60 insertions(+), 17 deletions(-)
diff --git a/src/bin/pg_upgrade/dump.c b/src/bin/pg_upgrade/dump.c
index b69b4f95695..29b9e44f782 100644
--- a/src/bin/pg_upgrade/dump.c
+++ b/src/bin/pg_upgrade/dump.c
@@ -29,7 +29,7 @@ generate_old_dump(void)
GLOBALS_DUMP_FILE);
check_ok();
- prep_status("Creating dump of database schemas\n");
+ prep_status_progress("Creating dump of database schemas");
/* create per-db dump files */
for (dbnum = 0; dbnum < old_cluster.dbarr.ndbs; dbnum++)
diff --git a/src/bin/pg_upgrade/option.c b/src/bin/pg_upgrade/option.c
index d2c82cc2bbb..e75be2c423e 100644
--- a/src/bin/pg_upgrade/option.c
+++ b/src/bin/pg_upgrade/option.c
@@ -207,6 +207,8 @@ parseCommandLine(int argc, char *argv[])
if (log_opts.verbose)
pg_log(PG_REPORT, "Running in verbose mode\n");
+ log_opts.isatty = isatty(fileno(stdout));
+
/* Turn off read-only mode; add prefix to PGOPTIONS? */
if (getenv("PGOPTIONS"))
{
diff --git a/src/bin/pg_upgrade/pg_upgrade.c b/src/bin/pg_upgrade/pg_upgrade.c
index f66bbd53079..ecb3e1f6474 100644
--- a/src/bin/pg_upgrade/pg_upgrade.c
+++ b/src/bin/pg_upgrade/pg_upgrade.c
@@ -381,7 +381,7 @@ create_new_objects(void)
{
int dbnum;
- prep_status("Restoring database schemas in the new cluster\n");
+ prep_status_progress("Restoring database schemas in the new cluster");
/*
* We cannot process the template1 database concurrently with others,
diff --git a/src/bin/pg_upgrade/pg_upgrade.h b/src/bin/pg_upgrade/pg_upgrade.h
index 0aca0a77aae..ca86c112924 100644
--- a/src/bin/pg_upgrade/pg_upgrade.h
+++ b/src/bin/pg_upgrade/pg_upgrade.h
@@ -274,6 +274,7 @@ typedef struct
char *basedir; /* Base output directory */
char *dumpdir; /* Dumps */
char *logdir; /* Log files */
+ bool isatty; /* is stdout a tty */
} LogOpts;
@@ -427,6 +428,7 @@ void pg_log(eLogType type, const char *fmt,...) pg_attribute_printf(2, 3);
void pg_fatal(const char *fmt,...) pg_attribute_printf(1, 2) pg_attribute_noreturn();
void end_progress_output(void);
void prep_status(const char *fmt,...) pg_attribute_printf(1, 2);
+void prep_status_progress(const char *fmt,...) pg_attribute_printf(1, 2);
void check_ok(void);
unsigned int str2uint(const char *str);
diff --git a/src/bin/pg_upgrade/relfilenode.c b/src/bin/pg_upgrade/relfilenode.c
index 2f4deb34163..d23ac884bd1 100644
--- a/src/bin/pg_upgrade/relfilenode.c
+++ b/src/bin/pg_upgrade/relfilenode.c
@@ -32,13 +32,13 @@ transfer_all_new_tablespaces(DbInfoArr *old_db_arr, DbInfoArr *new_db_arr,
switch (user_opts.transfer_mode)
{
case TRANSFER_MODE_CLONE:
- pg_log(PG_REPORT, "Cloning user relation files\n");
+ prep_status_progress("Cloning user relation files");
break;
case TRANSFER_MODE_COPY:
- pg_log(PG_REPORT, "Copying user relation files\n");
+ prep_status_progress("Copying user relation files");
break;
case TRANSFER_MODE_LINK:
- pg_log(PG_REPORT, "Linking user relation files\n");
+ prep_status_progress("Linking user relation files");
break;
}
diff --git a/src/bin/pg_upgrade/util.c b/src/bin/pg_upgrade/util.c
index d98deb7f24e..6cf02a8ad64 100644
--- a/src/bin/pg_upgrade/util.c
+++ b/src/bin/pg_upgrade/util.c
@@ -38,15 +38,18 @@ report_status(eLogType type, const char *fmt,...)
}
-/* force blank output for progress display */
void
end_progress_output(void)
{
/*
- * In case nothing printed; pass a space so gcc doesn't complain about
- * empty format string.
+ * For output to a tty, erase prior contents of progress line. When either
+ * tty or verbose, indent so that report_status() output will align
+ * nicely.
*/
- prep_status(" ");
+ if (log_opts.isatty)
+ pg_log(PG_REPORT, "\r%-*s", MESSAGE_WIDTH, "");
+ else if (log_opts.verbose)
+ pg_log(PG_REPORT, "%-*s", MESSAGE_WIDTH, "");
}
@@ -75,13 +78,42 @@ prep_status(const char *fmt,...)
vsnprintf(message, sizeof(message), fmt, args);
va_end(args);
- if (strlen(message) > 0 && message[strlen(message) - 1] == '\n')
- pg_log(PG_REPORT, "%s", message);
- else
- /* trim strings that don't end in a newline */
- pg_log(PG_REPORT, "%-*s", MESSAGE_WIDTH, message);
+ /* trim strings */
+ pg_log(PG_REPORT, "%-*s", MESSAGE_WIDTH, message);
}
+/*
+ * prep_status_progress
+ *
+ * Like prep_status(), but for potentially longer running operations.
+ * Details about what item is currently being processed can be displayed
+ * with pg_log(PG_STATUS, ...). A typical sequence would look like this:
+ *
+ * prep_status_progress("copying files");
+ * for (...)
+ * pg_log(PG_STATUS, "%s", filename);
+ * end_progress_output();
+ * report_status(PG_REPORT, "ok");
+ */
+void
+prep_status_progress(const char *fmt,...)
+{
+ va_list args;
+ char message[MAX_STRING];
+
+ va_start(args, fmt);
+ vsnprintf(message, sizeof(message), fmt, args);
+ va_end(args);
+
+ /*
+ * If outputting to a tty / or , append newline. pg_log_v() will put the
+ * individual progress items onto the next line.
+ */
+ if (log_opts.isatty || log_opts.verbose)
+ pg_log(PG_REPORT, "%-*s\n", MESSAGE_WIDTH, message);
+ else
+ pg_log(PG_REPORT, "%-*s", MESSAGE_WIDTH, message);
+}
static void
pg_log_v(eLogType type, const char *fmt, va_list ap)
@@ -111,8 +143,15 @@ pg_log_v(eLogType type, const char *fmt, va_list ap)
break;
case PG_STATUS:
- /* for output to a display, do leading truncation and append \r */
- if (isatty(fileno(stdout)))
+ /*
+ * For output to a display, do leading truncation. Append \r so
+ * that the next message is output at the start of the line.
+ *
+ * If going to non-interactive output, only display progress if
+ * verbose is enabled. Otherwise the output gets unreasonably
+ * large by default.
+ */
+ if (log_opts.isatty)
/* -2 because we use a 2-space indent */
printf(" %s%-*.*s\r",
/* prefix with "..." if we do leading truncation */
@@ -121,7 +160,7 @@ pg_log_v(eLogType type, const char *fmt, va_list ap)
/* optional leading truncation */
strlen(message) <= MESSAGE_WIDTH - 2 ? message :
message + strlen(message) - MESSAGE_WIDTH + 3 + 2);
- else
+ else if (log_opts.verbose)
printf(" %s\n", message);
break;
--
2.34.0
^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: pg_upgrade verbosity when redirecting output to log file
@ 2022-02-19 01:46 Justin Pryzby <[email protected]>
parent: Andres Freund <[email protected]>
0 siblings, 1 reply; 7+ messages in thread
From: Justin Pryzby @ 2022-02-19 01:46 UTC (permalink / raw)
To: Andres Freund <[email protected]>; +Cc: Thomas Munro <[email protected]>; Bruce Momjian <[email protected]>; Tom Lane <[email protected]>; pgsql-hackers
+ * If outputting to a tty / or , append newline. pg_log_v() will put the
+ * individual progress items onto the next line.
+ */
+ if (log_opts.isatty || log_opts.verbose)
I guess the comment should say "or in verbose mode".
--
Justin
^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: pg_upgrade verbosity when redirecting output to log file
@ 2022-02-21 01:07 Andres Freund <[email protected]>
parent: Justin Pryzby <[email protected]>
0 siblings, 1 reply; 7+ messages in thread
From: Andres Freund @ 2022-02-21 01:07 UTC (permalink / raw)
To: Justin Pryzby <[email protected]>; +Cc: Thomas Munro <[email protected]>; Bruce Momjian <[email protected]>; Tom Lane <[email protected]>; pgsql-hackers
Hi,
On 2022-02-18 19:46:26 -0600, Justin Pryzby wrote:
> + * If outputting to a tty / or , append newline. pg_log_v() will put the
> + * individual progress items onto the next line.
> + */
> + if (log_opts.isatty || log_opts.verbose)
>
> I guess the comment should say "or in verbose mode".
Indeed. I think I got caught in a back-and-forth between different
formulations.
Baring that, anybody against committing this?
Greetings,
Andres Freund
^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: pg_upgrade verbosity when redirecting output to log file
@ 2022-02-21 14:29 Daniel Gustafsson <[email protected]>
parent: Andres Freund <[email protected]>
0 siblings, 1 reply; 7+ messages in thread
From: Daniel Gustafsson @ 2022-02-21 14:29 UTC (permalink / raw)
To: Andres Freund <[email protected]>; +Cc: Justin Pryzby <[email protected]>; Thomas Munro <[email protected]>; Bruce Momjian <[email protected]>; Tom Lane <[email protected]>; pgsql-hackers
> On 21 Feb 2022, at 02:07, Andres Freund <[email protected]> wrote:
> Baring that, anybody against committing this?
LGTM. The above mentioned comment was the only thing I found as well.
--
Daniel Gustafsson https://vmware.com/
^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: pg_upgrade verbosity when redirecting output to log file
@ 2022-02-21 16:35 Andres Freund <[email protected]>
parent: Daniel Gustafsson <[email protected]>
0 siblings, 0 replies; 7+ messages in thread
From: Andres Freund @ 2022-02-21 16:35 UTC (permalink / raw)
To: Daniel Gustafsson <[email protected]>; +Cc: Justin Pryzby <[email protected]>; Thomas Munro <[email protected]>; Bruce Momjian <[email protected]>; Tom Lane <[email protected]>; pgsql-hackers
On 2022-02-21 15:29:09 +0100, Daniel Gustafsson wrote:
> > On 21 Feb 2022, at 02:07, Andres Freund <[email protected]> wrote:
>
> > Baring that, anybody against committing this?
>
> LGTM. The above mentioned comment was the only thing I found as well.
Thanks for the review Justin and Daniel. Pushed.
^ permalink raw reply [nested|flat] 7+ messages in thread
* [PATCH 2/4] Adjust tuplesort API to have bitwise flags instead of bool
@ 2022-04-01 07:04 David Rowley <[email protected]>
0 siblings, 0 replies; 7+ messages in thread
From: David Rowley @ 2022-04-01 07:04 UTC (permalink / raw)
For now we only had a single bool option for randomAccess, however an
upcoming patch requires adding another option, so instead of breaking the
API for that, then breaking it again one day if we add more options, let's
just break it once. Any options we add in the future will just make use
of an unused bit in the flags.
Author: David Rowley
Discussion: https://postgr.es/m/CAApHDvoH4ASzsAOyHcxkuY01Qf%2B%2B8JJ0paw%2B03dk%2BW25tQEcNQ%40mail.gmail.com
---
src/backend/access/gist/gistbuild.c | 2 +-
src/backend/access/hash/hashsort.c | 2 +-
src/backend/access/heap/heapam_handler.c | 2 +-
src/backend/access/nbtree/nbtsort.c | 6 +-
src/backend/catalog/index.c | 2 +-
src/backend/executor/nodeAgg.c | 6 +-
src/backend/executor/nodeIncrementalSort.c | 4 +-
src/backend/executor/nodeSort.c | 8 +-
src/backend/utils/adt/orderedsetaggs.c | 10 +-
src/backend/utils/sort/tuplesort.c | 101 +++++++++++----------
src/include/utils/tuplesort.h | 19 ++--
11 files changed, 90 insertions(+), 72 deletions(-)
diff --git a/src/backend/access/gist/gistbuild.c b/src/backend/access/gist/gistbuild.c
index e081e6571a4..f5a5caff8ec 100644
--- a/src/backend/access/gist/gistbuild.c
+++ b/src/backend/access/gist/gistbuild.c
@@ -271,7 +271,7 @@ gistbuild(Relation heap, Relation index, IndexInfo *indexInfo)
index,
maintenance_work_mem,
NULL,
- false);
+ TUPLESORT_NONE);
/* Scan the table, adding all tuples to the tuplesort */
reltuples = table_index_build_scan(heap, index, indexInfo, true, true,
diff --git a/src/backend/access/hash/hashsort.c b/src/backend/access/hash/hashsort.c
index 6d8512283a8..aa61e39f26a 100644
--- a/src/backend/access/hash/hashsort.c
+++ b/src/backend/access/hash/hashsort.c
@@ -86,7 +86,7 @@ _h_spoolinit(Relation heap, Relation index, uint32 num_buckets)
hspool->max_buckets,
maintenance_work_mem,
NULL,
- false);
+ TUPLESORT_NONE);
return hspool;
}
diff --git a/src/backend/access/heap/heapam_handler.c b/src/backend/access/heap/heapam_handler.c
index dee264e8596..3a9532cb4f7 100644
--- a/src/backend/access/heap/heapam_handler.c
+++ b/src/backend/access/heap/heapam_handler.c
@@ -726,7 +726,7 @@ heapam_relation_copy_for_cluster(Relation OldHeap, Relation NewHeap,
if (use_sort)
tuplesort = tuplesort_begin_cluster(oldTupDesc, OldIndex,
maintenance_work_mem,
- NULL, false);
+ NULL, TUPLESORT_NONE);
else
tuplesort = NULL;
diff --git a/src/backend/access/nbtree/nbtsort.c b/src/backend/access/nbtree/nbtsort.c
index 8a19de2f66c..15605dac6bb 100644
--- a/src/backend/access/nbtree/nbtsort.c
+++ b/src/backend/access/nbtree/nbtsort.c
@@ -436,7 +436,7 @@ _bt_spools_heapscan(Relation heap, Relation index, BTBuildState *buildstate,
tuplesort_begin_index_btree(heap, index, buildstate->isunique,
buildstate->nulls_not_distinct,
maintenance_work_mem, coordinate,
- false);
+ TUPLESORT_NONE);
/*
* If building a unique index, put dead tuples in a second spool to keep
@@ -475,7 +475,7 @@ _bt_spools_heapscan(Relation heap, Relation index, BTBuildState *buildstate,
*/
buildstate->spool2->sortstate =
tuplesort_begin_index_btree(heap, index, false, false, work_mem,
- coordinate2, false);
+ coordinate2, TUPLESORT_NONE);
}
/* Fill spool using either serial or parallel heap scan */
@@ -1939,7 +1939,7 @@ _bt_parallel_scan_and_sort(BTSpool *btspool, BTSpool *btspool2,
btspool->isunique,
btspool->nulls_not_distinct,
sortmem, coordinate,
- false);
+ TUPLESORT_NONE);
/*
* Just as with serial case, there may be a second spool. If so, a
diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c
index dd715ca0609..55800c94786 100644
--- a/src/backend/catalog/index.c
+++ b/src/backend/catalog/index.c
@@ -3364,7 +3364,7 @@ validate_index(Oid heapId, Oid indexId, Snapshot snapshot)
state.tuplesort = tuplesort_begin_datum(INT8OID, Int8LessOperator,
InvalidOid, false,
maintenance_work_mem,
- NULL, false);
+ NULL, TUPLESORT_NONE);
state.htups = state.itups = state.tups_inserted = 0;
/* ambulkdelete updates progress metrics */
diff --git a/src/backend/executor/nodeAgg.c b/src/backend/executor/nodeAgg.c
index 08cf569d8fa..23030a32a59 100644
--- a/src/backend/executor/nodeAgg.c
+++ b/src/backend/executor/nodeAgg.c
@@ -530,7 +530,7 @@ initialize_phase(AggState *aggstate, int newphase)
sortnode->collations,
sortnode->nullsFirst,
work_mem,
- NULL, false);
+ NULL, TUPLESORT_NONE);
}
aggstate->current_phase = newphase;
@@ -607,7 +607,7 @@ initialize_aggregate(AggState *aggstate, AggStatePerTrans pertrans,
pertrans->sortOperators[0],
pertrans->sortCollations[0],
pertrans->sortNullsFirst[0],
- work_mem, NULL, false);
+ work_mem, NULL, TUPLESORT_NONE);
}
else
pertrans->sortstates[aggstate->current_set] =
@@ -617,7 +617,7 @@ initialize_aggregate(AggState *aggstate, AggStatePerTrans pertrans,
pertrans->sortOperators,
pertrans->sortCollations,
pertrans->sortNullsFirst,
- work_mem, NULL, false);
+ work_mem, NULL, TUPLESORT_NONE);
}
/*
diff --git a/src/backend/executor/nodeIncrementalSort.c b/src/backend/executor/nodeIncrementalSort.c
index d6fb56dec73..4f50bc845da 100644
--- a/src/backend/executor/nodeIncrementalSort.c
+++ b/src/backend/executor/nodeIncrementalSort.c
@@ -315,7 +315,7 @@ switchToPresortedPrefixMode(PlanState *pstate)
&(plannode->sort.nullsFirst[nPresortedCols]),
work_mem,
NULL,
- false);
+ TUPLESORT_NONE);
node->prefixsort_state = prefixsort_state;
}
else
@@ -616,7 +616,7 @@ ExecIncrementalSort(PlanState *pstate)
plannode->sort.nullsFirst,
work_mem,
NULL,
- false);
+ TUPLESORT_NONE);
node->fullsort_state = fullsort_state;
}
else
diff --git a/src/backend/executor/nodeSort.c b/src/backend/executor/nodeSort.c
index 9481a622bf5..a113d737955 100644
--- a/src/backend/executor/nodeSort.c
+++ b/src/backend/executor/nodeSort.c
@@ -77,6 +77,7 @@ ExecSort(PlanState *pstate)
Sort *plannode = (Sort *) node->ss.ps.plan;
PlanState *outerNode;
TupleDesc tupDesc;
+ int tuplesortopts = TUPLESORT_NONE;
SO1_printf("ExecSort: %s\n",
"sorting subplan");
@@ -96,6 +97,9 @@ ExecSort(PlanState *pstate)
outerNode = outerPlanState(node);
tupDesc = ExecGetResultType(outerNode);
+ if (node->randomAccess)
+ tuplesortopts |= TUPLESORT_RANDOMACCESS;
+
if (node->datumSort)
tuplesortstate = tuplesort_begin_datum(TupleDescAttr(tupDesc, 0)->atttypid,
plannode->sortOperators[0],
@@ -103,7 +107,7 @@ ExecSort(PlanState *pstate)
plannode->nullsFirst[0],
work_mem,
NULL,
- node->randomAccess);
+ tuplesortopts);
else
tuplesortstate = tuplesort_begin_heap(tupDesc,
plannode->numCols,
@@ -113,7 +117,7 @@ ExecSort(PlanState *pstate)
plannode->nullsFirst,
work_mem,
NULL,
- node->randomAccess);
+ tuplesortopts);
if (node->bounded)
tuplesort_set_bound(tuplesortstate, node->bound);
node->tuplesortstate = (void *) tuplesortstate;
diff --git a/src/backend/utils/adt/orderedsetaggs.c b/src/backend/utils/adt/orderedsetaggs.c
index 96dae6ec4a8..6d4f6b7dca2 100644
--- a/src/backend/utils/adt/orderedsetaggs.c
+++ b/src/backend/utils/adt/orderedsetaggs.c
@@ -118,6 +118,7 @@ ordered_set_startup(FunctionCallInfo fcinfo, bool use_tuples)
OSAPerQueryState *qstate;
MemoryContext gcontext;
MemoryContext oldcontext;
+ int tuplesortopt;
/*
* Check we're called as aggregate (and not a window function), and get
@@ -283,6 +284,11 @@ ordered_set_startup(FunctionCallInfo fcinfo, bool use_tuples)
osastate->qstate = qstate;
osastate->gcontext = gcontext;
+ tuplesortopt = TUPLESORT_NONE;
+
+ if (qstate->rescan_needed)
+ tuplesortopt |= TUPLESORT_RANDOMACCESS;
+
/*
* Initialize tuplesort object.
*/
@@ -295,7 +301,7 @@ ordered_set_startup(FunctionCallInfo fcinfo, bool use_tuples)
qstate->sortNullsFirsts,
work_mem,
NULL,
- qstate->rescan_needed);
+ tuplesortopt);
else
osastate->sortstate = tuplesort_begin_datum(qstate->sortColType,
qstate->sortOperator,
@@ -303,7 +309,7 @@ ordered_set_startup(FunctionCallInfo fcinfo, bool use_tuples)
qstate->sortNullsFirst,
work_mem,
NULL,
- qstate->rescan_needed);
+ tuplesortopt);
osastate->number_of_rows = 0;
osastate->sort_done = false;
diff --git a/src/backend/utils/sort/tuplesort.c b/src/backend/utils/sort/tuplesort.c
index 086e948fca6..5cc1328febf 100644
--- a/src/backend/utils/sort/tuplesort.c
+++ b/src/backend/utils/sort/tuplesort.c
@@ -246,7 +246,7 @@ struct Tuplesortstate
{
TupSortStatus status; /* enumerated value as shown above */
int nKeys; /* number of columns in sort key */
- bool randomAccess; /* did caller request random access? */
+ int sortopt; /* Bitmask of flags used to setup sort */
bool bounded; /* did caller specify a maximum number of
* tuples to return? */
bool boundUsed; /* true if we made use of a bounded heap */
@@ -558,12 +558,12 @@ struct Sharedsort
* may or may not match the in-memory representation of the tuple ---
* any conversion needed is the job of the writetup and readtup routines.
*
- * If state->randomAccess is true, then the stored representation of the
- * tuple must be followed by another "unsigned int" that is a copy of the
- * length --- so the total tape space used is actually sizeof(unsigned int)
- * more than the stored length value. This allows read-backwards. When
- * randomAccess is not true, the write/read routines may omit the extra
- * length word.
+ * If state->sortopt contains TUPLESORT_RANDOMACCESS, then the stored
+ * representation of the tuple must be followed by another "unsigned int" that
+ * is a copy of the length --- so the total tape space used is actually
+ * sizeof(unsigned int) more than the stored length value. This allows
+ * read-backwards. When the random access flag was not specified, the
+ * write/read routines may omit the extra length word.
*
* writetup is expected to write both length words as well as the tuple
* data. When readtup is called, the tape is positioned just after the
@@ -608,7 +608,7 @@ struct Sharedsort
static Tuplesortstate *tuplesort_begin_common(int workMem,
SortCoordinate coordinate,
- bool randomAccess);
+ int sortopt);
static void tuplesort_begin_batch(Tuplesortstate *state);
static void puttuple_common(Tuplesortstate *state, SortTuple *tuple);
static bool consider_abort_common(Tuplesortstate *state);
@@ -713,21 +713,20 @@ static void tuplesort_updatemax(Tuplesortstate *state);
* Each variant of tuplesort_begin has a workMem parameter specifying the
* maximum number of kilobytes of RAM to use before spilling data to disk.
* (The normal value of this parameter is work_mem, but some callers use
- * other values.) Each variant also has a randomAccess parameter specifying
- * whether the caller needs non-sequential access to the sort result.
+ * other values.) Each variant also has a sortopt which is a bitmask of
+ * sort options. See TUPLESORT_* definitions in tuplesort.h
*/
static Tuplesortstate *
-tuplesort_begin_common(int workMem, SortCoordinate coordinate,
- bool randomAccess)
+tuplesort_begin_common(int workMem, SortCoordinate coordinate, int sortopt)
{
Tuplesortstate *state;
MemoryContext maincontext;
MemoryContext sortcontext;
MemoryContext oldcontext;
- /* See leader_takeover_tapes() remarks on randomAccess support */
- if (coordinate && randomAccess)
+ /* See leader_takeover_tapes() remarks on random access support */
+ if (coordinate && (sortopt & TUPLESORT_RANDOMACCESS))
elog(ERROR, "random access disallowed under parallel sort");
/*
@@ -764,7 +763,7 @@ tuplesort_begin_common(int workMem, SortCoordinate coordinate,
pg_rusage_init(&state->ru_start);
#endif
- state->randomAccess = randomAccess;
+ state->sortopt = sortopt;
state->tuples = true;
/*
@@ -898,10 +897,10 @@ tuplesort_begin_heap(TupleDesc tupDesc,
int nkeys, AttrNumber *attNums,
Oid *sortOperators, Oid *sortCollations,
bool *nullsFirstFlags,
- int workMem, SortCoordinate coordinate, bool randomAccess)
+ int workMem, SortCoordinate coordinate, int sortopt)
{
Tuplesortstate *state = tuplesort_begin_common(workMem, coordinate,
- randomAccess);
+ sortopt);
MemoryContext oldcontext;
int i;
@@ -913,7 +912,7 @@ tuplesort_begin_heap(TupleDesc tupDesc,
if (trace_sort)
elog(LOG,
"begin tuple sort: nkeys = %d, workMem = %d, randomAccess = %c",
- nkeys, workMem, randomAccess ? 't' : 'f');
+ nkeys, workMem, sortopt & TUPLESORT_RANDOMACCESS ? 't' : 'f');
#endif
state->nKeys = nkeys;
@@ -922,7 +921,7 @@ tuplesort_begin_heap(TupleDesc tupDesc,
false, /* no unique check */
nkeys,
workMem,
- randomAccess,
+ sortopt & TUPLESORT_RANDOMACCESS,
PARALLEL_SORT(state));
state->comparetup = comparetup_heap;
@@ -971,10 +970,10 @@ Tuplesortstate *
tuplesort_begin_cluster(TupleDesc tupDesc,
Relation indexRel,
int workMem,
- SortCoordinate coordinate, bool randomAccess)
+ SortCoordinate coordinate, int sortopt)
{
Tuplesortstate *state = tuplesort_begin_common(workMem, coordinate,
- randomAccess);
+ sortopt);
BTScanInsert indexScanKey;
MemoryContext oldcontext;
int i;
@@ -988,7 +987,7 @@ tuplesort_begin_cluster(TupleDesc tupDesc,
elog(LOG,
"begin tuple sort: nkeys = %d, workMem = %d, randomAccess = %c",
RelationGetNumberOfAttributes(indexRel),
- workMem, randomAccess ? 't' : 'f');
+ workMem, sortopt & TUPLESORT_RANDOMACCESS ? 't' : 'f');
#endif
state->nKeys = IndexRelationGetNumberOfKeyAttributes(indexRel);
@@ -997,7 +996,7 @@ tuplesort_begin_cluster(TupleDesc tupDesc,
false, /* no unique check */
state->nKeys,
workMem,
- randomAccess,
+ sortopt & TUPLESORT_RANDOMACCESS,
PARALLEL_SORT(state));
state->comparetup = comparetup_cluster;
@@ -1069,10 +1068,10 @@ tuplesort_begin_index_btree(Relation heapRel,
bool uniqueNullsNotDistinct,
int workMem,
SortCoordinate coordinate,
- bool randomAccess)
+ int sortopt)
{
Tuplesortstate *state = tuplesort_begin_common(workMem, coordinate,
- randomAccess);
+ sortopt);
BTScanInsert indexScanKey;
MemoryContext oldcontext;
int i;
@@ -1084,7 +1083,7 @@ tuplesort_begin_index_btree(Relation heapRel,
elog(LOG,
"begin index sort: unique = %c, workMem = %d, randomAccess = %c",
enforceUnique ? 't' : 'f',
- workMem, randomAccess ? 't' : 'f');
+ workMem, sortopt & TUPLESORT_RANDOMACCESS ? 't' : 'f');
#endif
state->nKeys = IndexRelationGetNumberOfKeyAttributes(indexRel);
@@ -1093,7 +1092,7 @@ tuplesort_begin_index_btree(Relation heapRel,
enforceUnique,
state->nKeys,
workMem,
- randomAccess,
+ sortopt & TUPLESORT_RANDOMACCESS,
PARALLEL_SORT(state));
state->comparetup = comparetup_index_btree;
@@ -1150,10 +1149,10 @@ tuplesort_begin_index_hash(Relation heapRel,
uint32 max_buckets,
int workMem,
SortCoordinate coordinate,
- bool randomAccess)
+ int sortopt)
{
Tuplesortstate *state = tuplesort_begin_common(workMem, coordinate,
- randomAccess);
+ sortopt);
MemoryContext oldcontext;
oldcontext = MemoryContextSwitchTo(state->maincontext);
@@ -1166,7 +1165,8 @@ tuplesort_begin_index_hash(Relation heapRel,
high_mask,
low_mask,
max_buckets,
- workMem, randomAccess ? 't' : 'f');
+ workMem,
+ sortopt & TUPLESORT_RANDOMACCESS ? 't' : 'f');
#endif
state->nKeys = 1; /* Only one sort column, the hash code */
@@ -1193,10 +1193,10 @@ tuplesort_begin_index_gist(Relation heapRel,
Relation indexRel,
int workMem,
SortCoordinate coordinate,
- bool randomAccess)
+ int sortopt)
{
Tuplesortstate *state = tuplesort_begin_common(workMem, coordinate,
- randomAccess);
+ sortopt);
MemoryContext oldcontext;
int i;
@@ -1206,7 +1206,7 @@ tuplesort_begin_index_gist(Relation heapRel,
if (trace_sort)
elog(LOG,
"begin index sort: workMem = %d, randomAccess = %c",
- workMem, randomAccess ? 't' : 'f');
+ workMem, sortopt & TUPLESORT_RANDOMACCESS ? 't' : 'f');
#endif
state->nKeys = IndexRelationGetNumberOfKeyAttributes(indexRel);
@@ -1248,10 +1248,10 @@ tuplesort_begin_index_gist(Relation heapRel,
Tuplesortstate *
tuplesort_begin_datum(Oid datumType, Oid sortOperator, Oid sortCollation,
bool nullsFirstFlag, int workMem,
- SortCoordinate coordinate, bool randomAccess)
+ SortCoordinate coordinate, int sortopt)
{
Tuplesortstate *state = tuplesort_begin_common(workMem, coordinate,
- randomAccess);
+ sortopt);
MemoryContext oldcontext;
int16 typlen;
bool typbyval;
@@ -1262,7 +1262,7 @@ tuplesort_begin_datum(Oid datumType, Oid sortOperator, Oid sortCollation,
if (trace_sort)
elog(LOG,
"begin datum sort: workMem = %d, randomAccess = %c",
- workMem, randomAccess ? 't' : 'f');
+ workMem, sortopt & TUPLESORT_RANDOMACCESS ? 't' : 'f');
#endif
state->nKeys = 1; /* always a one-column sort */
@@ -2165,7 +2165,7 @@ tuplesort_gettuple_common(Tuplesortstate *state, bool forward,
switch (state->status)
{
case TSS_SORTEDINMEM:
- Assert(forward || state->randomAccess);
+ Assert(forward || state->sortopt & TUPLESORT_RANDOMACCESS);
Assert(!state->slabAllocatorUsed);
if (forward)
{
@@ -2209,7 +2209,7 @@ tuplesort_gettuple_common(Tuplesortstate *state, bool forward,
break;
case TSS_SORTEDONTAPE:
- Assert(forward || state->randomAccess);
+ Assert(forward || state->sortopt & TUPLESORT_RANDOMACCESS);
Assert(state->slabAllocatorUsed);
/*
@@ -2984,7 +2984,8 @@ mergeruns(Tuplesortstate *state)
* sorted tape, we can stop at this point and do the final merge
* on-the-fly.
*/
- if (!state->randomAccess && state->nInputRuns <= state->nInputTapes
+ if ((state->sortopt & TUPLESORT_RANDOMACCESS) == 0
+ && state->nInputRuns <= state->nInputTapes
&& !WORKER(state))
{
/* Tell logtape.c we won't be writing anymore */
@@ -3230,7 +3231,7 @@ tuplesort_rescan(Tuplesortstate *state)
{
MemoryContext oldcontext = MemoryContextSwitchTo(state->sortcontext);
- Assert(state->randomAccess);
+ Assert(state->sortopt & TUPLESORT_RANDOMACCESS);
switch (state->status)
{
@@ -3263,7 +3264,7 @@ tuplesort_markpos(Tuplesortstate *state)
{
MemoryContext oldcontext = MemoryContextSwitchTo(state->sortcontext);
- Assert(state->randomAccess);
+ Assert(state->sortopt & TUPLESORT_RANDOMACCESS);
switch (state->status)
{
@@ -3294,7 +3295,7 @@ tuplesort_restorepos(Tuplesortstate *state)
{
MemoryContext oldcontext = MemoryContextSwitchTo(state->sortcontext);
- Assert(state->randomAccess);
+ Assert(state->sortopt & TUPLESORT_RANDOMACCESS);
switch (state->status)
{
@@ -3853,7 +3854,7 @@ writetup_heap(Tuplesortstate *state, LogicalTape *tape, SortTuple *stup)
LogicalTapeWrite(tape, (void *) &tuplen, sizeof(tuplen));
LogicalTapeWrite(tape, (void *) tupbody, tupbodylen);
- if (state->randomAccess) /* need trailing length word? */
+ if (state->sortopt & TUPLESORT_RANDOMACCESS) /* need trailing length word? */
LogicalTapeWrite(tape, (void *) &tuplen, sizeof(tuplen));
if (!state->slabAllocatorUsed)
@@ -3876,7 +3877,7 @@ readtup_heap(Tuplesortstate *state, SortTuple *stup,
/* read in the tuple proper */
tuple->t_len = tuplen;
LogicalTapeReadExact(tape, tupbody, tupbodylen);
- if (state->randomAccess) /* need trailing length word? */
+ if (state->sortopt & TUPLESORT_RANDOMACCESS) /* need trailing length word? */
LogicalTapeReadExact(tape, &tuplen, sizeof(tuplen));
stup->tuple = (void *) tuple;
/* set up first-column key value */
@@ -4087,7 +4088,7 @@ writetup_cluster(Tuplesortstate *state, LogicalTape *tape, SortTuple *stup)
LogicalTapeWrite(tape, &tuplen, sizeof(tuplen));
LogicalTapeWrite(tape, &tuple->t_self, sizeof(ItemPointerData));
LogicalTapeWrite(tape, tuple->t_data, tuple->t_len);
- if (state->randomAccess) /* need trailing length word? */
+ if (state->sortopt & TUPLESORT_RANDOMACCESS) /* need trailing length word? */
LogicalTapeWrite(tape, &tuplen, sizeof(tuplen));
if (!state->slabAllocatorUsed)
@@ -4113,7 +4114,7 @@ readtup_cluster(Tuplesortstate *state, SortTuple *stup,
tuple->t_tableOid = InvalidOid;
/* Read in the tuple body */
LogicalTapeReadExact(tape, tuple->t_data, tuple->t_len);
- if (state->randomAccess) /* need trailing length word? */
+ if (state->sortopt & TUPLESORT_RANDOMACCESS) /* need trailing length word? */
LogicalTapeReadExact(tape, &tuplen, sizeof(tuplen));
stup->tuple = (void *) tuple;
/* set up first-column key value, if it's a simple column */
@@ -4337,7 +4338,7 @@ writetup_index(Tuplesortstate *state, LogicalTape *tape, SortTuple *stup)
tuplen = IndexTupleSize(tuple) + sizeof(tuplen);
LogicalTapeWrite(tape, (void *) &tuplen, sizeof(tuplen));
LogicalTapeWrite(tape, (void *) tuple, IndexTupleSize(tuple));
- if (state->randomAccess) /* need trailing length word? */
+ if (state->sortopt & TUPLESORT_RANDOMACCESS) /* need trailing length word? */
LogicalTapeWrite(tape, (void *) &tuplen, sizeof(tuplen));
if (!state->slabAllocatorUsed)
@@ -4355,7 +4356,7 @@ readtup_index(Tuplesortstate *state, SortTuple *stup,
IndexTuple tuple = (IndexTuple) readtup_alloc(state, tuplen);
LogicalTapeReadExact(tape, tuple, tuplen);
- if (state->randomAccess) /* need trailing length word? */
+ if (state->sortopt & TUPLESORT_RANDOMACCESS) /* need trailing length word? */
LogicalTapeReadExact(tape, &tuplen, sizeof(tuplen));
stup->tuple = (void *) tuple;
/* set up first-column key value */
@@ -4425,7 +4426,7 @@ writetup_datum(Tuplesortstate *state, LogicalTape *tape, SortTuple *stup)
LogicalTapeWrite(tape, (void *) &writtenlen, sizeof(writtenlen));
LogicalTapeWrite(tape, waddr, tuplen);
- if (state->randomAccess) /* need trailing length word? */
+ if (state->sortopt & TUPLESORT_RANDOMACCESS) /* need trailing length word? */
LogicalTapeWrite(tape, (void *) &writtenlen, sizeof(writtenlen));
if (!state->slabAllocatorUsed && stup->tuple)
@@ -4465,7 +4466,7 @@ readtup_datum(Tuplesortstate *state, SortTuple *stup,
stup->tuple = raddr;
}
- if (state->randomAccess) /* need trailing length word? */
+ if (state->sortopt & TUPLESORT_RANDOMACCESS) /* need trailing length word? */
LogicalTapeReadExact(tape, &tuplen, sizeof(tuplen));
}
diff --git a/src/include/utils/tuplesort.h b/src/include/utils/tuplesort.h
index da5ba591989..345f4ce8024 100644
--- a/src/include/utils/tuplesort.h
+++ b/src/include/utils/tuplesort.h
@@ -86,6 +86,12 @@ typedef enum
SORT_SPACE_TYPE_MEMORY
} TuplesortSpaceType;
+/* Bitwise option flags for tuple sorts */
+#define TUPLESORT_NONE 0
+
+/* specifies whether non-sequential access to the sort result is required */
+#define TUPLESORT_RANDOMACCESS (1 << 0)
+
typedef struct TuplesortInstrumentation
{
TuplesortMethod sortMethod; /* sort algorithm used */
@@ -201,32 +207,33 @@ extern Tuplesortstate *tuplesort_begin_heap(TupleDesc tupDesc,
Oid *sortOperators, Oid *sortCollations,
bool *nullsFirstFlags,
int workMem, SortCoordinate coordinate,
- bool randomAccess);
+ int sortopt);
extern Tuplesortstate *tuplesort_begin_cluster(TupleDesc tupDesc,
Relation indexRel, int workMem,
- SortCoordinate coordinate, bool randomAccess);
+ SortCoordinate coordinate,
+ int sortopt);
extern Tuplesortstate *tuplesort_begin_index_btree(Relation heapRel,
Relation indexRel,
bool enforceUnique,
bool uniqueNullsNotDistinct,
int workMem, SortCoordinate coordinate,
- bool randomAccess);
+ int sortopt);
extern Tuplesortstate *tuplesort_begin_index_hash(Relation heapRel,
Relation indexRel,
uint32 high_mask,
uint32 low_mask,
uint32 max_buckets,
int workMem, SortCoordinate coordinate,
- bool randomAccess);
+ int sortopt);
extern Tuplesortstate *tuplesort_begin_index_gist(Relation heapRel,
Relation indexRel,
int workMem, SortCoordinate coordinate,
- bool randomAccess);
+ int sortopt);
extern Tuplesortstate *tuplesort_begin_datum(Oid datumType,
Oid sortOperator, Oid sortCollation,
bool nullsFirstFlag,
int workMem, SortCoordinate coordinate,
- bool randomAccess);
+ int sortopt);
extern void tuplesort_set_bound(Tuplesortstate *state, int64 bound);
extern bool tuplesort_used_bound(Tuplesortstate *state);
--
2.17.1
--RHnfto7yGd+QdmRl
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment; filename="0003-f.patch"
^ permalink raw reply [nested|flat] 7+ messages in thread
end of thread, other threads:[~2022-04-01 07:04 UTC | newest]
Thread overview: 7+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2022-02-16 04:09 Re: pg_upgrade verbosity when redirecting output to log file Thomas Munro <[email protected]>
2022-02-19 01:20 ` Andres Freund <[email protected]>
2022-02-19 01:46 ` Justin Pryzby <[email protected]>
2022-02-21 01:07 ` Andres Freund <[email protected]>
2022-02-21 14:29 ` Daniel Gustafsson <[email protected]>
2022-02-21 16:35 ` Andres Freund <[email protected]>
2022-04-01 07:04 [PATCH 2/4] Adjust tuplesort API to have bitwise flags instead of bool David Rowley <[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