public inbox for [email protected]help / color / mirror / Atom feed
[PATCH v52 6/7] Remove the GUC stats_temp_directory 7+ messages / 5 participants [nested] [flat]
* [PATCH v52 6/7] Remove the GUC stats_temp_directory @ 2020-09-29 13:59 Kyotaro Horiguchi <[email protected]> 0 siblings, 0 replies; 7+ messages in thread From: Kyotaro Horiguchi @ 2020-09-29 13:59 UTC (permalink / raw) The new stats collection system doesn't need temporary directory, so just remove it. pg_stat_statements modified to use pg_stat directory to store its temporary files. As the result basebackup copies the pg_stat_statments' temporary file if exists. --- .../pg_stat_statements/pg_stat_statements.c | 13 +++--- doc/src/sgml/backup.sgml | 2 - doc/src/sgml/config.sgml | 19 -------- doc/src/sgml/storage.sgml | 6 --- src/backend/postmaster/pgstat.c | 10 ----- src/backend/replication/basebackup.c | 36 ---------------- src/backend/utils/misc/guc.c | 43 ------------------- src/backend/utils/misc/postgresql.conf.sample | 1 - src/bin/initdb/initdb.c | 1 - src/bin/pg_rewind/filemap.c | 7 --- src/include/pgstat.h | 3 -- src/test/perl/PostgresNode.pm | 4 -- 12 files changed, 6 insertions(+), 139 deletions(-) diff --git a/contrib/pg_stat_statements/pg_stat_statements.c b/contrib/pg_stat_statements/pg_stat_statements.c index 62cccbfa44..28279f97d5 100644 --- a/contrib/pg_stat_statements/pg_stat_statements.c +++ b/contrib/pg_stat_statements/pg_stat_statements.c @@ -89,14 +89,13 @@ PG_MODULE_MAGIC; #define PGSS_DUMP_FILE PGSTAT_STAT_PERMANENT_DIRECTORY "/pg_stat_statements.stat" /* - * Location of external query text file. We don't keep it in the core - * system's stats_temp_directory. The core system can safely use that GUC - * setting, because the statistics collector temp file paths are set only once - * as part of changing the GUC, but pg_stat_statements has no way of avoiding - * race conditions. Besides, we only expect modest, infrequent I/O for query - * strings, so placing the file on a faster filesystem is not compelling. + * Location of external query text file. We don't keep it in the core system's + * pg_stats. pg_stat_statements has no way of avoiding race conditions even if + * the directory were specified by a GUC. Besides, we only expect modest, + * infrequent I/O for query strings, so placing the file on a faster filesystem + * is not compelling. */ -#define PGSS_TEXT_FILE PG_STAT_TMP_DIR "/pgss_query_texts.stat" +#define PGSS_TEXT_FILE PGSTAT_STAT_PERMANENT_DIRECTORY "/pgss_query_texts.stat" /* Magic number identifying the stats file format */ static const uint32 PGSS_FILE_HEADER = 0x20201218; diff --git a/doc/src/sgml/backup.sgml b/doc/src/sgml/backup.sgml index c5557d5444..875769a57e 100644 --- a/doc/src/sgml/backup.sgml +++ b/doc/src/sgml/backup.sgml @@ -1155,8 +1155,6 @@ SELECT pg_stop_backup(); <filename>pg_snapshots/</filename>, <filename>pg_stat_tmp/</filename>, and <filename>pg_subtrans/</filename> (but not the directories themselves) can be omitted from the backup as they will be initialized on postmaster startup. - If <xref linkend="guc-stats-temp-directory"/> is set and is under the data - directory then the contents of that directory can also be omitted. </para> <para> diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml index 99a8664026..c0b41c8457 100644 --- a/doc/src/sgml/config.sgml +++ b/doc/src/sgml/config.sgml @@ -7509,25 +7509,6 @@ COPY postgres_log FROM '/full/path/to/logfile.csv' WITH csv; </listitem> </varlistentry> - <varlistentry id="guc-stats-temp-directory" xreflabel="stats_temp_directory"> - <term><varname>stats_temp_directory</varname> (<type>string</type>) - <indexterm> - <primary><varname>stats_temp_directory</varname> configuration parameter</primary> - </indexterm> - </term> - <listitem> - <para> - Sets the directory to store temporary statistics data in. This can be - a path relative to the data directory or an absolute path. The default - is <filename>pg_stat_tmp</filename>. Pointing this at a RAM-based - file system will decrease physical I/O requirements and can lead to - improved performance. - This parameter can only be set in the <filename>postgresql.conf</filename> - file or on the server command line. - </para> - </listitem> - </varlistentry> - </variablelist> </sect2> diff --git a/doc/src/sgml/storage.sgml b/doc/src/sgml/storage.sgml index 3234adb639..6bac5e075e 100644 --- a/doc/src/sgml/storage.sgml +++ b/doc/src/sgml/storage.sgml @@ -120,12 +120,6 @@ Item subsystem</entry> </row> -<row> - <entry><filename>pg_stat_tmp</filename></entry> - <entry>Subdirectory containing temporary files for the statistics - subsystem</entry> -</row> - <row> <entry><filename>pg_subtrans</filename></entry> <entry>Subdirectory containing subtransaction status data</entry> diff --git a/src/backend/postmaster/pgstat.c b/src/backend/postmaster/pgstat.c index fe726f55f0..6a0cad034e 100644 --- a/src/backend/postmaster/pgstat.c +++ b/src/backend/postmaster/pgstat.c @@ -99,16 +99,6 @@ bool pgstat_track_counts = false; int pgstat_track_functions = TRACK_FUNC_OFF; int pgstat_track_activity_query_size = 1024; -/* ---------- - * Built from GUC parameter - * ---------- - */ -char *pgstat_stat_directory = NULL; - -/* No longer used, but will be removed with GUC */ -char *pgstat_stat_filename = NULL; -char *pgstat_stat_tmpname = NULL; - /* * WAL usage counters saved from pgWALUsage at the previous call to * pgstat_report_wal(). This is used to calculate how much WAL usage diff --git a/src/backend/replication/basebackup.c b/src/backend/replication/basebackup.c index fa7bdbcefa..f0d75f55bd 100644 --- a/src/backend/replication/basebackup.c +++ b/src/backend/replication/basebackup.c @@ -87,9 +87,6 @@ static int basebackup_read_file(int fd, char *buf, size_t nbytes, off_t offset, /* Was the backup currently in-progress initiated in recovery mode? */ static bool backup_started_in_recovery = false; -/* Relative path of temporary statistics directory */ -static char *statrelpath = NULL; - /* * Size of each block sent into the tar stream for larger files. */ @@ -152,13 +149,6 @@ struct exclude_list_item */ static const char *const excludeDirContents[] = { - /* - * Skip temporary statistics files. PG_STAT_TMP_DIR must be skipped even - * when stats_temp_directory is set because PGSS_TEXT_FILE is always - * created there. - */ - PG_STAT_TMP_DIR, - /* * It is generally not useful to backup the contents of this directory * even if the intention is to restore to another primary. See backup.sgml @@ -261,7 +251,6 @@ perform_base_backup(basebackup_options *opt) StringInfo labelfile; StringInfo tblspc_map_file; backup_manifest_info manifest; - int datadirpathlen; List *tablespaces = NIL; backup_total = 0; @@ -284,8 +273,6 @@ perform_base_backup(basebackup_options *opt) Assert(CurrentResourceOwner == NULL); CurrentResourceOwner = ResourceOwnerCreate(NULL, "base backup"); - datadirpathlen = strlen(DataDir); - backup_started_in_recovery = RecoveryInProgress(); labelfile = makeStringInfo(); @@ -314,18 +301,6 @@ perform_base_backup(basebackup_options *opt) tablespaceinfo *ti; int tblspc_streamed = 0; - /* - * Calculate the relative path of temporary statistics directory in - * order to skip the files which are located in that directory later. - */ - if (is_absolute_path(pgstat_stat_directory) && - strncmp(pgstat_stat_directory, DataDir, datadirpathlen) == 0) - statrelpath = psprintf("./%s", pgstat_stat_directory + datadirpathlen + 1); - else if (strncmp(pgstat_stat_directory, "./", 2) != 0) - statrelpath = psprintf("./%s", pgstat_stat_directory); - else - statrelpath = pgstat_stat_directory; - /* Add a node for the base directory at the end */ ti = palloc0(sizeof(tablespaceinfo)); ti->size = -1; @@ -1377,17 +1352,6 @@ sendDir(const char *path, int basepathlen, bool sizeonly, List *tablespaces, if (excludeFound) continue; - /* - * Exclude contents of directory specified by statrelpath if not set - * to the default (pg_stat_tmp) which is caught in the loop above. - */ - if (statrelpath != NULL && strcmp(pathbuf, statrelpath) == 0) - { - elog(DEBUG1, "contents of directory \"%s\" excluded from backup", statrelpath); - size += _tarWriteDir(pathbuf, basepathlen, &statbuf, sizeonly); - continue; - } - /* * We can skip pg_wal, the WAL segments need to be fetched from the * WAL archive anyway. But include it as an empty directory anyway, so diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c index d42aea7324..dd34e032ce 100644 --- a/src/backend/utils/misc/guc.c +++ b/src/backend/utils/misc/guc.c @@ -203,7 +203,6 @@ static bool check_autovacuum_work_mem(int *newval, void **extra, GucSource sourc static bool check_effective_io_concurrency(int *newval, void **extra, GucSource source); static bool check_maintenance_io_concurrency(int *newval, void **extra, GucSource source); static bool check_huge_page_size(int *newval, void **extra, GucSource source); -static void assign_pgstat_temp_directory(const char *newval, void *extra); static bool check_application_name(char **newval, void **extra, GucSource source); static void assign_application_name(const char *newval, void *extra); static bool check_cluster_name(char **newval, void **extra, GucSource source); @@ -560,8 +559,6 @@ char *HbaFileName; char *IdentFileName; char *external_pid_file; -char *pgstat_temp_directory; - char *application_name; int tcp_keepalives_idle; @@ -4375,17 +4372,6 @@ static struct config_string ConfigureNamesString[] = NULL, NULL, NULL }, - { - {"stats_temp_directory", PGC_SIGHUP, STATS_ACTIVITY, - gettext_noop("Writes temporary statistics files to the specified directory."), - NULL, - GUC_SUPERUSER_ONLY - }, - &pgstat_temp_directory, - PG_STAT_TMP_DIR, - check_canonical_path, assign_pgstat_temp_directory, NULL - }, - { {"synchronous_standby_names", PGC_SIGHUP, REPLICATION_PRIMARY, gettext_noop("Number of synchronous standbys and list of names of potential synchronous ones."), @@ -11791,35 +11777,6 @@ check_huge_page_size(int *newval, void **extra, GucSource source) return true; } -static void -assign_pgstat_temp_directory(const char *newval, void *extra) -{ - /* check_canonical_path already canonicalized newval for us */ - char *dname; - char *tname; - char *fname; - - /* directory */ - dname = guc_malloc(ERROR, strlen(newval) + 1); /* runtime dir */ - sprintf(dname, "%s", newval); - - /* global stats */ - tname = guc_malloc(ERROR, strlen(newval) + 12); /* /global.tmp */ - sprintf(tname, "%s/global.tmp", newval); - fname = guc_malloc(ERROR, strlen(newval) + 13); /* /global.stat */ - sprintf(fname, "%s/global.stat", newval); - - if (pgstat_stat_directory) - free(pgstat_stat_directory); - pgstat_stat_directory = dname; - if (pgstat_stat_tmpname) - free(pgstat_stat_tmpname); - pgstat_stat_tmpname = tname; - if (pgstat_stat_filename) - free(pgstat_stat_filename); - pgstat_stat_filename = fname; -} - static bool check_application_name(char **newval, void **extra, GucSource source) { diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample index e543f5560f..e4fb1c0cc1 100644 --- a/src/backend/utils/misc/postgresql.conf.sample +++ b/src/backend/utils/misc/postgresql.conf.sample @@ -589,7 +589,6 @@ #track_wal_io_timing = off #track_functions = none # none, pl, all #track_activity_query_size = 1024 # (change requires restart) -#stats_temp_directory = 'pg_stat_tmp' # - Monitoring - diff --git a/src/bin/initdb/initdb.c b/src/bin/initdb/initdb.c index 3c1cf78b4f..07a00b8d0d 100644 --- a/src/bin/initdb/initdb.c +++ b/src/bin/initdb/initdb.c @@ -219,7 +219,6 @@ static const char *const subdirs[] = { "pg_replslot", "pg_tblspc", "pg_stat", - "pg_stat_tmp", "pg_xact", "pg_logical", "pg_logical/snapshots", diff --git a/src/bin/pg_rewind/filemap.c b/src/bin/pg_rewind/filemap.c index 2618b4c957..ab5cb51de7 100644 --- a/src/bin/pg_rewind/filemap.c +++ b/src/bin/pg_rewind/filemap.c @@ -87,13 +87,6 @@ struct exclude_list_item */ static const char *excludeDirContents[] = { - /* - * Skip temporary statistics files. PG_STAT_TMP_DIR must be skipped even - * when stats_temp_directory is set because PGSS_TEXT_FILE is always - * created there. - */ - "pg_stat_tmp", /* defined as PG_STAT_TMP_DIR */ - /* * It is generally not useful to backup the contents of this directory * even if the intention is to restore to another primary. See backup.sgml diff --git a/src/include/pgstat.h b/src/include/pgstat.h index 44ef3fdc35..4e464c8528 100644 --- a/src/include/pgstat.h +++ b/src/include/pgstat.h @@ -33,9 +33,6 @@ #define PGSTAT_STAT_PERMANENT_FILENAME "pg_stat/saved_stats" #define PGSTAT_STAT_PERMANENT_TMPFILE "pg_stat/saved_stats.tmp" -/* Default directory to store temporary statistics data in */ -#define PG_STAT_TMP_DIR "pg_stat_tmp" - /* Values for track_functions GUC variable --- order is significant! */ typedef enum TrackFunctionsLevel { diff --git a/src/test/perl/PostgresNode.pm b/src/test/perl/PostgresNode.pm index 9667f7667e..dd41a43b4e 100644 --- a/src/test/perl/PostgresNode.pm +++ b/src/test/perl/PostgresNode.pm @@ -455,10 +455,6 @@ sub init print $conf TestLib::slurp_file($ENV{TEMP_CONFIG}) if defined $ENV{TEMP_CONFIG}; - # XXX Neutralize any stats_temp_directory in TEMP_CONFIG. Nodes running - # concurrently must not share a stats_temp_directory. - print $conf "stats_temp_directory = 'pg_stat_tmp'\n"; - if ($params{allows_streaming}) { if ($params{allows_streaming} eq "logical") -- 2.27.0 ----Next_Part(Wed_Mar_10_17_51_37_2021_192)-- Content-Type: Text/X-Patch; charset=us-ascii Content-Transfer-Encoding: 7bit Content-Disposition: inline; filename="v52-0007-Exclude-pg_stat-directory-from-base-backup.patch" ^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: Add proper planner support for ORDER BY / DISTINCT aggregates @ 2023-01-11 02:45 Richard Guo <[email protected]> 0 siblings, 1 reply; 7+ messages in thread From: Richard Guo @ 2023-01-11 02:45 UTC (permalink / raw) To: Dean Rasheed <[email protected]>; +Cc: David Rowley <[email protected]>; Ronan Dunklau <[email protected]>; Justin Pryzby <[email protected]>; Pavel Luzanov <[email protected]>; Tom Lane <[email protected]>; [email protected]; Ranier Vilela <[email protected]> On Tue, Jan 10, 2023 at 6:12 PM Dean Rasheed <[email protected]> wrote: > While doing some random testing, I noticed that the following is broken in > HEAD: > > SELECT COUNT(DISTINCT random()) FROM generate_series(1,10); > > ERROR: ORDER/GROUP BY expression not found in targetlist > > It appears to have been broken by 1349d279, though I haven't looked at > the details. Yeah, this is definitely broken. For this query, we try to sort the scan/join path by random() before performing the Aggregate, which is an optimization implemented in 1349d2790b. However the scan/join plan's tlist does not contain random(), which I think we need to fix. Thanks Richard ^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: Add proper planner support for ORDER BY / DISTINCT aggregates @ 2023-01-11 04:12 David Rowley <[email protected]> parent: Richard Guo <[email protected]> 0 siblings, 2 replies; 7+ messages in thread From: David Rowley @ 2023-01-11 04:12 UTC (permalink / raw) To: Richard Guo <[email protected]>; +Cc: Dean Rasheed <[email protected]>; Ronan Dunklau <[email protected]>; Justin Pryzby <[email protected]>; Pavel Luzanov <[email protected]>; Tom Lane <[email protected]>; [email protected]; Ranier Vilela <[email protected]> On Wed, 11 Jan 2023 at 15:46, Richard Guo <[email protected]> wrote: > However the scan/join plan's > tlist does not contain random(), which I think we need to fix. I was wondering if that's true and considered that we don't want to evaluate random() for the sort then again when doing the aggregate transitions, but I see that does not really work before 1349d279, per: postgres=# set enable_presorted_aggregate=0; SET postgres=# select string_agg(random()::text, ',' order by random()) from generate_series(1,3); string_agg ----------------------------------------------------------- 0.8659110018246505,0.15612649559563474,0.2022878955613403 (1 row) I'd have expected those random numbers to be concatenated in ascending order. Running: select random() from generate_Series(1,3) order by random(); gives me the results in the order I'd have expected. I think whatever the fix is here, we should likely ensure that the results are consistent regardless of which Aggrefs are the presorted ones. Perhaps the easiest way to do that, and to ensure we call the volatile functions are called the same number of times would just be to never choose Aggrefs with volatile functions when doing make_pathkeys_for_groupagg(). David ^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: Add proper planner support for ORDER BY / DISTINCT aggregates @ 2023-01-11 04:32 Tom Lane <[email protected]> parent: David Rowley <[email protected]> 1 sibling, 1 reply; 7+ messages in thread From: Tom Lane @ 2023-01-11 04:32 UTC (permalink / raw) To: David Rowley <[email protected]>; +Cc: Richard Guo <[email protected]>; Dean Rasheed <[email protected]>; Ronan Dunklau <[email protected]>; Justin Pryzby <[email protected]>; Pavel Luzanov <[email protected]>; [email protected]; Ranier Vilela <[email protected]> David Rowley <[email protected]> writes: > I think whatever the fix is here, we should likely ensure that the > results are consistent regardless of which Aggrefs are the presorted > ones. Perhaps the easiest way to do that, and to ensure we call the > volatile functions are called the same number of times would just be > to never choose Aggrefs with volatile functions when doing > make_pathkeys_for_groupagg(). There's existing logic in equivclass.c and other places that tries to draw very tight lines around what we'll assume about volatile sort expressions (pathkeys). It sounds like there's someplace in this recent patch that didn't get that memo. regards, tom lane ^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: Add proper planner support for ORDER BY / DISTINCT aggregates @ 2023-01-11 05:23 David Rowley <[email protected]> parent: Tom Lane <[email protected]> 0 siblings, 1 reply; 7+ messages in thread From: David Rowley @ 2023-01-11 05:23 UTC (permalink / raw) To: Tom Lane <[email protected]>; +Cc: Richard Guo <[email protected]>; Dean Rasheed <[email protected]>; Ronan Dunklau <[email protected]>; Justin Pryzby <[email protected]>; Pavel Luzanov <[email protected]>; [email protected]; Ranier Vilela <[email protected]> On Wed, 11 Jan 2023 at 17:32, Tom Lane <[email protected]> wrote: > > David Rowley <[email protected]> writes: > > I think whatever the fix is here, we should likely ensure that the > > results are consistent regardless of which Aggrefs are the presorted > > ones. Perhaps the easiest way to do that, and to ensure we call the > > volatile functions are called the same number of times would just be > > to never choose Aggrefs with volatile functions when doing > > make_pathkeys_for_groupagg(). > > There's existing logic in equivclass.c and other places that tries > to draw very tight lines around what we'll assume about volatile > sort expressions (pathkeys). It sounds like there's someplace in > this recent patch that didn't get that memo. I'm not sure I did a good job of communicating my thoughts there. What I mean is, having volatile functions in the aggregate's ORDER BY or DISTINCT clause didn't seem very well behaved prior to the presorted aggregates patch. If I go and fix the bug with the missing targetlist items, then a query such as: select string_agg(random()::text, ',' order by random()) from generate_series(1,3); should start putting the random() numbers in order where it didn't prior to 1349d279. Perhaps users might be happy that those are in order, however, if they then go and change the query to: select sum(a order by a),string_agg(random()::text, ',' order by random()) from generate_series(1,3); then they might become unhappy again that their string_agg is not ordered the way they specified because the planner opted to sort by "a" rather than "random()" after the initial scan. I'm wondering if 1349d279 should have just never opted to presort Aggrefs which have volatile functions so that the existing behaviour of unordered output is given always and nobody is fooled into thinking this works correctly only to be disappointed later when they add some other aggregate to their query or if we should fix both. Certainly, it seems much easier to do the former. David ^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: Add proper planner support for ORDER BY / DISTINCT aggregates @ 2023-01-11 08:47 Richard Guo <[email protected]> parent: David Rowley <[email protected]> 1 sibling, 0 replies; 7+ messages in thread From: Richard Guo @ 2023-01-11 08:47 UTC (permalink / raw) To: David Rowley <[email protected]>; +Cc: Dean Rasheed <[email protected]>; Ronan Dunklau <[email protected]>; Justin Pryzby <[email protected]>; Pavel Luzanov <[email protected]>; Tom Lane <[email protected]>; [email protected]; Ranier Vilela <[email protected]> On Wed, Jan 11, 2023 at 12:13 PM David Rowley <[email protected]> wrote: > postgres=# set enable_presorted_aggregate=0; > SET > postgres=# select string_agg(random()::text, ',' order by random()) > from generate_series(1,3); > string_agg > ----------------------------------------------------------- > 0.8659110018246505,0.15612649559563474,0.2022878955613403 > (1 row) > > I'd have expected those random numbers to be concatenated in ascending > order. select string_agg( random()::text, -- position 1 ',' order by random() -- position 2 ) from generate_series(1,3); I traced this query a bit and found that when executing the aggregation the random() function in the aggregate expression (position 1) and in the order by clause (position 2) are calculated separately. And the sorting is performed based on the function results from the order by clause. In the final output, what we see is the function results from the aggregate expression. Thus we'll notice the output is not sorted. I'm not sure if this is expected or broken though. BTW, if we explicitly add ::text for random() in the order by clause, as select string_agg( random()::text, ',' order by random()::text ) from generate_series(1,3); The random() function will be calculated only once for each tuple, and we can get a sorted output. Thanks Richard ^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: Add proper planner support for ORDER BY / DISTINCT aggregates @ 2023-01-17 00:16 Dean Rasheed <[email protected]> parent: David Rowley <[email protected]> 0 siblings, 0 replies; 7+ messages in thread From: Dean Rasheed @ 2023-01-17 00:16 UTC (permalink / raw) To: David Rowley <[email protected]>; +Cc: Tom Lane <[email protected]>; Richard Guo <[email protected]>; Ronan Dunklau <[email protected]>; Justin Pryzby <[email protected]>; Pavel Luzanov <[email protected]>; [email protected]; Ranier Vilela <[email protected]> On Wed, 11 Jan 2023 at 05:24, David Rowley <[email protected]> wrote: > > I'm wondering if 1349d279 should have just never opted to presort > Aggrefs which have volatile functions so that the existing behaviour > of unordered output is given always and nobody is fooled into thinking > this works correctly only to be disappointed later when they add some > other aggregate to their query or if we should fix both. Certainly, > it seems much easier to do the former. > I took a look at this, and I agree that the best solution is probably to have make_pathkeys_for_groupagg() ignore Aggrefs that contain volatile functions. Not only is that the simplest solution, preserving the old behaviour, I think it's required for correctness. Aside from the fact that I don't think such aggregates would benefit from the optimisation introduced by 1349d279, I think it would be incorrect if there was more than one such aggregate having the same sort expression, because I think that volatile sorting should be evaluated separately for each aggregate. For example: SELECT string_agg(a::text, ',' ORDER BY random()), string_agg(a::text, ',' ORDER BY random()) FROM generate_series(1,3) s(a); string_agg | string_agg ------------+------------ 2,1,3 | 3,2,1 (1 row) so pre-sorting wouldn't be right (or at least it would change existing behaviour in a surprising way). Regards, Dean ^ permalink raw reply [nested|flat] 7+ messages in thread
end of thread, other threads:[~2023-01-17 00:16 UTC | newest] Thread overview: 7+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2020-09-29 13:59 [PATCH v52 6/7] Remove the GUC stats_temp_directory Kyotaro Horiguchi <[email protected]> 2023-01-11 02:45 Re: Add proper planner support for ORDER BY / DISTINCT aggregates Richard Guo <[email protected]> 2023-01-11 04:12 ` Re: Add proper planner support for ORDER BY / DISTINCT aggregates David Rowley <[email protected]> 2023-01-11 04:32 ` Re: Add proper planner support for ORDER BY / DISTINCT aggregates Tom Lane <[email protected]> 2023-01-11 05:23 ` Re: Add proper planner support for ORDER BY / DISTINCT aggregates David Rowley <[email protected]> 2023-01-17 00:16 ` Re: Add proper planner support for ORDER BY / DISTINCT aggregates Dean Rasheed <[email protected]> 2023-01-11 08:47 ` Re: Add proper planner support for ORDER BY / DISTINCT aggregates Richard Guo <[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