agora inbox for [email protected]
help / color / mirror / Atom feed[PATCH 5/5] Remove the GUC stats_temp_directory
7+ messages / 5 participants
[nested] [flat]
* [PATCH 5/5] Remove the GUC stats_temp_directory
@ 2018-11-27 05:42 Kyotaro Horiguchi <[email protected]>
0 siblings, 0 replies; 7+ messages in thread
From: Kyotaro Horiguchi @ 2018-11-27 05:42 UTC (permalink / raw)
The guc used to specifie the directory to store temporary statistics
files. It is no longer needed by the stats collector but still used by
the programs in bin and contirb, and maybe other extensions. Thus this
patch removes the GUC but some backing variables and macro definitions
are left alone for backward comptibility.
---
doc/src/sgml/backup.sgml | 2 --
doc/src/sgml/config.sgml | 19 -------------
doc/src/sgml/monitoring.sgml | 7 +----
doc/src/sgml/storage.sgml | 3 +-
src/backend/postmaster/pgstat.c | 13 ++++-----
src/backend/replication/basebackup.c | 13 ++-------
src/backend/utils/misc/guc.c | 41 ---------------------------
src/backend/utils/misc/postgresql.conf.sample | 1 -
src/include/pgstat.h | 5 +++-
src/test/perl/PostgresNode.pm | 4 ---
10 files changed, 14 insertions(+), 94 deletions(-)
diff --git a/doc/src/sgml/backup.sgml b/doc/src/sgml/backup.sgml
index 9d4c000df0..bbae70221e 100644
--- a/doc/src/sgml/backup.sgml
+++ b/doc/src/sgml/backup.sgml
@@ -1145,8 +1145,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 c91e3e1550..b85f6f421a 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -6827,25 +6827,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/monitoring.sgml b/doc/src/sgml/monitoring.sgml
index 990995c17b..2a2adaa0f7 100644
--- a/doc/src/sgml/monitoring.sgml
+++ b/doc/src/sgml/monitoring.sgml
@@ -195,12 +195,7 @@ postgres 27093 0.0 0.0 30096 2752 ? Ss 11:34 0:00 postgres: ser
<para>
The statistics collector transmits the collected information to other
- <productname>PostgreSQL</productname> processes through temporary files.
- These files are stored in the directory named by the
- <xref linkend="guc-stats-temp-directory"/> parameter,
- <filename>pg_stat_tmp</filename> by default.
- For better performance, <varname>stats_temp_directory</varname> can be
- pointed at a RAM-based file system, decreasing physical I/O requirements.
+ <productname>PostgreSQL</productname> processes through shared memory.
When the server shuts down cleanly, a permanent copy of the statistics
data is stored in the <filename>pg_stat</filename> subdirectory, so that
statistics can be retained across server restarts. When recovery is
diff --git a/doc/src/sgml/storage.sgml b/doc/src/sgml/storage.sgml
index 1047c77a63..b47f8d084e 100644
--- a/doc/src/sgml/storage.sgml
+++ b/doc/src/sgml/storage.sgml
@@ -122,8 +122,7 @@ Item
<row>
<entry><filename>pg_stat_tmp</filename></entry>
- <entry>Subdirectory containing temporary files for the statistics
- subsystem</entry>
+ <entry>Subdirectory containing ephemeral files for extensions</entry>
</row>
<row>
diff --git a/src/backend/postmaster/pgstat.c b/src/backend/postmaster/pgstat.c
index 514ea78a68..64e1374288 100644
--- a/src/backend/postmaster/pgstat.c
+++ b/src/backend/postmaster/pgstat.c
@@ -108,15 +108,12 @@ bool pgstat_track_counts = false;
int pgstat_track_functions = TRACK_FUNC_OFF;
int pgstat_track_activity_query_size = 1024;
-/* ----------
- * Built from GUC parameter
- * ----------
+/*
+ * This used to be a GUC variable and is no longer used in this file, but left
+ * alone just for backward comptibility for extensions, having the default
+ * value.
*/
-char *pgstat_stat_directory = NULL;
-
-/* No longer used, but will be removed with GUC */
-char *pgstat_stat_filename = NULL;
-char *pgstat_stat_tmpname = NULL;
+char *pgstat_stat_directory = PG_STAT_TMP_DIR;
#define StatsLock (&StatsShmem->StatsMainLock)
diff --git a/src/backend/replication/basebackup.c b/src/backend/replication/basebackup.c
index 57f17e1418..c854d7e193 100644
--- a/src/backend/replication/basebackup.c
+++ b/src/backend/replication/basebackup.c
@@ -230,11 +230,8 @@ perform_base_backup(basebackup_options *opt)
TimeLineID endtli;
StringInfo labelfile;
StringInfo tblspc_map_file = NULL;
- int datadirpathlen;
List *tablespaces = NIL;
- datadirpathlen = strlen(DataDir);
-
backup_started_in_recovery = RecoveryInProgress();
labelfile = makeStringInfo();
@@ -265,13 +262,9 @@ perform_base_backup(basebackup_options *opt)
* 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;
+
+ Assert(strchr(PG_STAT_TMP_DIR, '/') == NULL);
+ statrelpath = psprintf("./%s", PG_STAT_TMP_DIR);
/* Add a node for the base directory at the end */
ti = palloc0(sizeof(tablespaceinfo));
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index fc463601ff..05af81ac47 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -194,7 +194,6 @@ static bool check_max_wal_senders(int *newval, void **extra, GucSource source);
static bool check_autovacuum_work_mem(int *newval, void **extra, GucSource source);
static bool check_effective_io_concurrency(int *newval, void **extra, GucSource source);
static void assign_effective_io_concurrency(int newval, void *extra);
-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);
@@ -4085,17 +4084,6 @@ static struct config_string ConfigureNamesString[] =
NULL, NULL, NULL
},
- {
- {"stats_temp_directory", PGC_SIGHUP, STATS_COLLECTOR,
- 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_MASTER,
gettext_noop("Number of synchronous standbys and list of names of potential synchronous ones."),
@@ -11368,35 +11356,6 @@ assign_effective_io_concurrency(int newval, void *extra)
#endif /* USE_PREFETCH */
}
-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 cfad86c02a..29b7ebca46 100644
--- a/src/backend/utils/misc/postgresql.conf.sample
+++ b/src/backend/utils/misc/postgresql.conf.sample
@@ -562,7 +562,6 @@
#track_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/include/pgstat.h b/src/include/pgstat.h
index c0bbf8a7d5..2c7c799ca9 100644
--- a/src/include/pgstat.h
+++ b/src/include/pgstat.h
@@ -31,7 +31,10 @@
#define PGSTAT_STAT_PERMANENT_FILENAME "pg_stat/global.stat"
#define PGSTAT_STAT_PERMANENT_TMPFILE "pg_stat/global.tmp"
-/* Default directory to store temporary statistics data in */
+/*
+ * This used to be the directory to store temporary statistics data in but is
+ * no longer used. Defined here for backward compatibility.
+ */
#define PG_STAT_TMP_DIR "pg_stat_tmp"
/* Values for track_functions GUC variable --- order is significant! */
diff --git a/src/test/perl/PostgresNode.pm b/src/test/perl/PostgresNode.pm
index 6019f37f91..a107683c0d 100644
--- a/src/test/perl/PostgresNode.pm
+++ b/src/test/perl/PostgresNode.pm
@@ -451,10 +451,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.16.3
----Next_Part(Thu_Jul_11_16_40_59_2019_397)--
Content-Type: Text/Plain; charset=us-ascii
Content-Transfer-Encoding: 7bit
Content-Disposition: inline; filename="statbench.pl"
#! /usr/bin/perl
use strict;
use IPC::Open2;
use Time::HiRes qw( usleep ualarm gettimeofday tv_interval );
use Errno qw/ECHILD/;
my $totaldbs = 100;
my $totaltbls = 100;
my $ndbs = 50;
my $ntbls = 100;
my $loops = 20;
my $chiter = 20000;
my $chtrlen = 1000;
my $nprocs = 90;
my $trg_file = '/tmp/dbrun_pl.trg';
my $refresult_file = '/tmp/refresult.txt';
# title loops,ndbs,clnts,ntbls,childiter , xactlen,refprocs
# testrun("A1", $loops, 1, 1, 1, $chiter*10, $chtrlen, 0);
# testrun("A2", $loops, 1, 1, 1, $chiter*10, $chtrlen, 1);
# testrun("B1", $loops, 1, 90, 1, $chiter , $chtrlen, 0);
# testrun("B2", $loops, 1, 90, 1, $chiter , $chtrlen, 1);
# testrun("C1", $loops, 1, 90, 50, $chiter , $chtrlen, 0);
# testrun("C2", $loops, 1, 90, 50, $chiter , $chtrlen, 10);
# testrun("D1", $loops, 50, 90, 1, $chiter , $chtrlen, 0);
# testrun("D2", $loops, 50, 90, 1, $chiter , $chtrlen, 1);
# testrun("E1", $loops, 50, 90, 1, $chiter , $chtrlen, 10);
# testrun("F1", $loops , 1, 10, 1, $chiter , $chtrlen, 90);
testrun("G1", 10, 1, 400, 1000, 20000, 1000, 0);
testrun("G2", 10, 1, 400, 1000, 20000, 1000, 1);
exit;
sub testrun
{
my ($test_name, $loops, $ndbs, $nprocs, $ntbls,
$childiter, $childtrlen, $nreferers)
= @_;
my @results = ();
my @refresults = ();
# run each iteration
for (my $l = 0 ; $l < $loops ; $l++)
{
my %starttime = ();
my %endtime = ();
# This file is used for stopping free-running processes.
open(OUT, '>', $trg_file) || die "failed to open file:$!\n";
print OUT "$$\n";
close(OUT);
# It's very mysterious that one dummy subprocess makes things
# stable. Perl runs slowly if there's only one psql process
# without this...
{
my $pid = fork;
if ($pid == 0)
{
while (-f $trg_file)
{
usleep (500000);
}
exit;
}
}
# start referer processes, collecting how many times the query ran.
pipe(my $refresrd, my $refreswr);
for(my $i = 0 ; $i < $nreferers ; $i++)
{
my $pid = fork;
if ($pid < 0) { die "fork referer process failed : $!\n"; }
if ($pid == 0)
{
my $pid = open2(my $psqlrd, my $psqlwr, "psql postgres");
my $count = 0;
close($refresrd);
if ($pid < 0) { die "fork psql failed : $!\n"; }
while (-f $trg_file)
{
print $psqlwr "select * from pg_stat_user_tables;\n";
while (<$psqlrd>)
{
last if ($_ =~ /rows\)/);
}
$count++;
}
print $refreswr "$count\n";
exit;
}
}
close($refreswr);
# launch updator processes
for (my $i = 0 ; $i < $nprocs ; $i++)
{
#my $dbn = rand($ndbs); # equal dist.
my $dbn = $i % $totaldbs; # round robbin
my $dbname = sprintf("db%03d", $dbn);
my $pid = fork;
if ($pid < 0)
{
die "fork failed: $!\n";
}
elsif ($pid == 0)
{
my $pid = open2(my $rd, my $wr, "psql $dbname > /dev/null");
if ($pid < 0) { die "sub fork failed: $!\n"; }
my $ncmd = $childtrlen;
#print $wr "set log_min_duration_statement to 0;\n";
print $wr "begin;\n";
for (my $i = 0 ; $i < $childiter ; $i++)
{
#my $tbn = rand($ntbls); # equal dist.
my $tbn = $i % $totaltbls; # round robbin
printf $wr "select /* $dbname\[$i\]*/ count(*) from t%03d;\n", $tbn;
if (--$ncmd == 0)
{
print $wr "commit;begin;\n";
$ncmd = $childtrlen;
}
}
print $wr "commit;\n";
print $wr "\\q\n";
my $res = <$rd>;
exit;
}
(my $sec, my $usec) = &gettimeofday();
$starttime{$pid} = $sec * 1000 + $usec / 1000;
# print "start\[$pid\] = $starttime{$pid}\n";
}
# wait for updateors to finish
for (my $i = 0 ; $i < $nprocs ; $i++)
{
my $pid = wait();
if ($pid < 0) { if ($! != ECHILD) {die "???: $!\n"; }}
redo if (!defined $starttime{$pid});
(my $sec, my $usec) = &gettimeofday();
$endtime{$pid} = $sec * 1000 + $usec / 1000;
# printf "%d[%d]: %d - %d = %d ms\n", $i, $pid, $endtime{$pid}, $starttime{$pid}, $endtime{$pid} - $starttime{$pid};
}
my $sum = 0;
foreach my $pid (keys %starttime)
{
$sum += $endtime{$pid} - $starttime{$pid};
# printf "[%d]: %d ms (%d, %d)\n", $pid, $endtime{$pid} - $starttime{$pid}, $starttime{$pid}, $endtime{$pid};
}
push(@results, $sum / $nprocs);
# kill referers if any (and dummy process)
unlink($trg_file);
while (wait() == 0) {}
my $nrefs = 0;
my $refcount = 0;
while(<$refresrd>)
{
chomp;
$refcount += $_;
$nrefs++;
}
close($refresrd);
push (@refresults, $refcount / $nrefs) if ($nrefs > 0);
}
# calculate stdev
(my $updmean, my $updstdev) = stdev(@results);
(my $refmean, my $refstdev) = stdev(@refresults);
printf "$test_name (l:%d, d:%d, t:%d, i:%d, tr:%d): %.2f ms (stdev %.2f) / %d updprocs, %.2f refs (stdev %.2f) / %d refprocs\n", $loops, $ndbs, $ntbls, $childiter, $childtrlen, $updmean, $updstdev, $nprocs, $refmean, $refstdev, $nreferers;
}
sub stdev
{
my $sum = 0;
my $sqsum = 0;
my $count = $#_ + 1;
return (0, 0) if $count == 0;
foreach my $el (@_)
{
$sum += 1.0 * $el;
$sqsum += 1.0 * $el * $el;
}
my $mean = $sum / $count;
my $stdev = sqrt($sqsum / $count - $mean * $mean);
return ($mean, $stdev);
}
----Next_Part(Thu_Jul_11_16_40_59_2019_397)----
^ permalink raw reply [nested|flat] 7+ messages in thread
* [PATCH] JsonLexContext allocation/free
@ 2023-08-03 09:44 Alvaro Herrera <[email protected]>
0 siblings, 0 replies; 7+ messages in thread
From: Alvaro Herrera @ 2023-08-03 09:44 UTC (permalink / raw)
---
src/backend/utils/adt/json.c | 38 ++++----
src/backend/utils/adt/jsonb.c | 13 +--
src/backend/utils/adt/jsonfuncs.c | 106 +++++++++++++----------
src/bin/pg_verifybackup/parse_manifest.c | 4 +-
src/common/jsonapi.c | 43 +++++++--
src/include/common/jsonapi.h | 23 +++--
src/include/utils/jsonfuncs.h | 2 +-
7 files changed, 146 insertions(+), 83 deletions(-)
diff --git a/src/backend/utils/adt/json.c b/src/backend/utils/adt/json.c
index e405791f5d..27f9a51228 100644
--- a/src/backend/utils/adt/json.c
+++ b/src/backend/utils/adt/json.c
@@ -106,11 +106,11 @@ json_in(PG_FUNCTION_ARGS)
{
char *json = PG_GETARG_CSTRING(0);
text *result = cstring_to_text(json);
- JsonLexContext *lex;
+ JsonLexContext lex;
/* validate it */
- lex = makeJsonLexContext(result, false);
- if (!pg_parse_json_or_errsave(lex, &nullSemAction, fcinfo->context))
+ makeJsonLexContext(&lex, result, false);
+ if (!pg_parse_json_or_errsave(&lex, &nullSemAction, fcinfo->context))
PG_RETURN_NULL();
/* Internal representation is the same as text */
@@ -152,13 +152,13 @@ json_recv(PG_FUNCTION_ARGS)
StringInfo buf = (StringInfo) PG_GETARG_POINTER(0);
char *str;
int nbytes;
- JsonLexContext *lex;
+ JsonLexContext lex;
str = pq_getmsgtext(buf, buf->len - buf->cursor, &nbytes);
/* Validate it. */
- lex = makeJsonLexContextCstringLen(str, nbytes, GetDatabaseEncoding(), false);
- pg_parse_json_or_ereport(lex, &nullSemAction);
+ makeJsonLexContextCstringLen(&lex, str, nbytes, GetDatabaseEncoding(), false);
+ pg_parse_json_or_ereport(&lex, &nullSemAction);
PG_RETURN_TEXT_P(cstring_to_text_with_len(str, nbytes));
}
@@ -1625,14 +1625,16 @@ json_unique_object_field_start(void *_state, char *field, bool isnull)
bool
json_validate(text *json, bool check_unique_keys, bool throw_error)
{
- JsonLexContext *lex = makeJsonLexContext(json, check_unique_keys);
+ JsonLexContext lex;
JsonSemAction uniqueSemAction = {0};
JsonUniqueParsingState state;
JsonParseErrorType result;
+ makeJsonLexContext(&lex, json, check_unique_keys);
+
if (check_unique_keys)
{
- state.lex = lex;
+ state.lex = &lex;
state.stack = NULL;
state.id_counter = 0;
state.unique = true;
@@ -1644,12 +1646,12 @@ json_validate(text *json, bool check_unique_keys, bool throw_error)
uniqueSemAction.object_end = json_unique_object_end;
}
- result = pg_parse_json(lex, check_unique_keys ? &uniqueSemAction : &nullSemAction);
+ result = pg_parse_json(&lex, check_unique_keys ? &uniqueSemAction : &nullSemAction);
if (result != JSON_SUCCESS)
{
if (throw_error)
- json_errsave_error(result, lex, NULL);
+ json_errsave_error(result, &lex, NULL);
return false; /* invalid json */
}
@@ -1664,6 +1666,9 @@ json_validate(text *json, bool check_unique_keys, bool throw_error)
return false; /* not unique keys */
}
+ if (check_unique_keys)
+ freeJsonLexContext(&lex);
+
return true; /* ok */
}
@@ -1683,18 +1688,17 @@ Datum
json_typeof(PG_FUNCTION_ARGS)
{
text *json = PG_GETARG_TEXT_PP(0);
- JsonLexContext *lex = makeJsonLexContext(json, false);
+ JsonLexContext lex;
char *type;
- JsonTokenType tok;
JsonParseErrorType result;
/* Lex exactly one token from the input and check its type. */
- result = json_lex(lex);
+ makeJsonLexContext(&lex, json, false);
+ result = json_lex(&lex);
if (result != JSON_SUCCESS)
- json_errsave_error(result, lex, NULL);
- tok = lex->token_type;
+ json_errsave_error(result, &lex, NULL);
- switch (tok)
+ switch (lex.token_type)
{
case JSON_TOKEN_OBJECT_START:
type = "object";
@@ -1716,7 +1720,7 @@ json_typeof(PG_FUNCTION_ARGS)
type = "null";
break;
default:
- elog(ERROR, "unexpected json token: %d", tok);
+ elog(ERROR, "unexpected json token: %d", lex.token_type);
}
PG_RETURN_TEXT_P(cstring_to_text(type));
diff --git a/src/backend/utils/adt/jsonb.c b/src/backend/utils/adt/jsonb.c
index 9781852b0c..b10a60ac66 100644
--- a/src/backend/utils/adt/jsonb.c
+++ b/src/backend/utils/adt/jsonb.c
@@ -252,13 +252,13 @@ jsonb_typeof(PG_FUNCTION_ARGS)
static inline Datum
jsonb_from_cstring(char *json, int len, bool unique_keys, Node *escontext)
{
- JsonLexContext *lex;
+ JsonLexContext lex;
JsonbInState state;
JsonSemAction sem;
memset(&state, 0, sizeof(state));
memset(&sem, 0, sizeof(sem));
- lex = makeJsonLexContextCstringLen(json, len, GetDatabaseEncoding(), true);
+ makeJsonLexContextCstringLen(&lex, json, len, GetDatabaseEncoding(), true);
state.unique_keys = unique_keys;
state.escontext = escontext;
@@ -271,7 +271,7 @@ jsonb_from_cstring(char *json, int len, bool unique_keys, Node *escontext)
sem.scalar = jsonb_in_scalar;
sem.object_field_start = jsonb_in_object_field_start;
- if (!pg_parse_json_or_errsave(lex, &sem, escontext))
+ if (!pg_parse_json_or_errsave(&lex, &sem, escontext))
return (Datum) 0;
/* after parsing, the item member has the composed jsonb structure */
@@ -755,11 +755,11 @@ datum_to_jsonb_internal(Datum val, bool is_null, JsonbInState *result,
case JSONTYPE_JSON:
{
/* parse the json right into the existing result object */
- JsonLexContext *lex;
+ JsonLexContext lex;
JsonSemAction sem;
text *json = DatumGetTextPP(val);
- lex = makeJsonLexContext(json, true);
+ makeJsonLexContext(&lex, json, true);
memset(&sem, 0, sizeof(sem));
@@ -772,7 +772,8 @@ datum_to_jsonb_internal(Datum val, bool is_null, JsonbInState *result,
sem.scalar = jsonb_in_scalar;
sem.object_field_start = jsonb_in_object_field_start;
- pg_parse_json_or_ereport(lex, &sem);
+ pg_parse_json_or_ereport(&lex, &sem);
+ freeJsonLexContext(&lex);
}
break;
case JSONTYPE_JSONB:
diff --git a/src/backend/utils/adt/jsonfuncs.c b/src/backend/utils/adt/jsonfuncs.c
index a4bfa5e404..3f855d8f2b 100644
--- a/src/backend/utils/adt/jsonfuncs.c
+++ b/src/backend/utils/adt/jsonfuncs.c
@@ -526,7 +526,7 @@ pg_parse_json_or_errsave(JsonLexContext *lex, JsonSemAction *sem,
* directly.
*/
JsonLexContext *
-makeJsonLexContext(text *json, bool need_escapes)
+makeJsonLexContext(JsonLexContext *lex, text *json, bool need_escapes)
{
/*
* Most callers pass a detoasted datum, but it's not clear that they all
@@ -534,7 +534,8 @@ makeJsonLexContext(text *json, bool need_escapes)
*/
json = pg_detoast_datum_packed(json);
- return makeJsonLexContextCstringLen(VARDATA_ANY(json),
+ return makeJsonLexContextCstringLen(lex,
+ VARDATA_ANY(json),
VARSIZE_ANY_EXHDR(json),
GetDatabaseEncoding(),
need_escapes);
@@ -725,17 +726,19 @@ json_object_keys(PG_FUNCTION_ARGS)
if (SRF_IS_FIRSTCALL())
{
text *json = PG_GETARG_TEXT_PP(0);
- JsonLexContext *lex = makeJsonLexContext(json, true);
+ JsonLexContext lex;
JsonSemAction *sem;
MemoryContext oldcontext;
+ makeJsonLexContext(&lex, json, true);
+
funcctx = SRF_FIRSTCALL_INIT();
oldcontext = MemoryContextSwitchTo(funcctx->multi_call_memory_ctx);
state = palloc(sizeof(OkeysState));
sem = palloc0(sizeof(JsonSemAction));
- state->lex = lex;
+ state->lex = &lex;
state->result_size = 256;
state->result_count = 0;
state->sent_count = 0;
@@ -747,12 +750,10 @@ json_object_keys(PG_FUNCTION_ARGS)
sem->object_field_start = okeys_object_field_start;
/* remainder are all NULL, courtesy of palloc0 above */
- pg_parse_json_or_ereport(lex, sem);
+ pg_parse_json_or_ereport(&lex, sem);
/* keys are now in state->result */
- pfree(lex->strval->data);
- pfree(lex->strval);
- pfree(lex);
+ freeJsonLexContext(&lex);
pfree(sem);
MemoryContextSwitchTo(oldcontext);
@@ -1096,13 +1097,13 @@ get_worker(text *json,
int npath,
bool normalize_results)
{
- JsonLexContext *lex = makeJsonLexContext(json, true);
JsonSemAction *sem = palloc0(sizeof(JsonSemAction));
GetState *state = palloc0(sizeof(GetState));
Assert(npath >= 0);
- state->lex = lex;
+ state->lex = makeJsonLexContext(NULL, json, true);
+
/* is it "_as_text" variant? */
state->normalize_results = normalize_results;
state->npath = npath;
@@ -1140,7 +1141,7 @@ get_worker(text *json,
sem->array_element_end = get_array_element_end;
}
- pg_parse_json_or_ereport(lex, sem);
+ pg_parse_json_or_ereport(state->lex, sem);
return state->tresult;
}
@@ -1842,25 +1843,24 @@ json_array_length(PG_FUNCTION_ARGS)
{
text *json = PG_GETARG_TEXT_PP(0);
AlenState *state;
- JsonLexContext *lex;
JsonSemAction *sem;
+ JsonLexContext lex;
- lex = makeJsonLexContext(json, false);
state = palloc0(sizeof(AlenState));
sem = palloc0(sizeof(JsonSemAction));
+ state->lex = makeJsonLexContext(&lex, json, false);
/* palloc0 does this for us */
#if 0
state->count = 0;
#endif
- state->lex = lex;
sem->semstate = (void *) state;
sem->object_start = alen_object_start;
sem->scalar = alen_scalar;
sem->array_element_start = alen_array_element_start;
- pg_parse_json_or_ereport(lex, sem);
+ pg_parse_json_or_ereport(state->lex, sem);
PG_RETURN_INT32(state->count);
}
@@ -2049,12 +2049,12 @@ static Datum
each_worker(FunctionCallInfo fcinfo, bool as_text)
{
text *json = PG_GETARG_TEXT_PP(0);
- JsonLexContext *lex;
+ JsonLexContext lex;
JsonSemAction *sem;
ReturnSetInfo *rsi;
EachState *state;
- lex = makeJsonLexContext(json, true);
+ makeJsonLexContext(&lex, json, true);
state = palloc0(sizeof(EachState));
sem = palloc0(sizeof(JsonSemAction));
@@ -2072,12 +2072,12 @@ each_worker(FunctionCallInfo fcinfo, bool as_text)
state->normalize_results = as_text;
state->next_scalar = false;
- state->lex = lex;
+ state->lex = &lex;
state->tmp_cxt = AllocSetContextCreate(CurrentMemoryContext,
"json_each temporary cxt",
ALLOCSET_DEFAULT_SIZES);
- pg_parse_json_or_ereport(lex, sem);
+ pg_parse_json_or_ereport(&lex, sem);
MemoryContextDelete(state->tmp_cxt);
@@ -2299,13 +2299,14 @@ static Datum
elements_worker(FunctionCallInfo fcinfo, const char *funcname, bool as_text)
{
text *json = PG_GETARG_TEXT_PP(0);
-
- /* elements only needs escaped strings when as_text */
- JsonLexContext *lex = makeJsonLexContext(json, as_text);
+ JsonLexContext lex;
JsonSemAction *sem;
ReturnSetInfo *rsi;
ElementsState *state;
+ /* elements only needs escaped strings when as_text */
+ makeJsonLexContext(&lex, json, as_text);
+
state = palloc0(sizeof(ElementsState));
sem = palloc0(sizeof(JsonSemAction));
@@ -2323,12 +2324,12 @@ elements_worker(FunctionCallInfo fcinfo, const char *funcname, bool as_text)
state->function_name = funcname;
state->normalize_results = as_text;
state->next_scalar = false;
- state->lex = lex;
+ state->lex = &lex;
state->tmp_cxt = AllocSetContextCreate(CurrentMemoryContext,
"json_array_elements temporary cxt",
ALLOCSET_DEFAULT_SIZES);
- pg_parse_json_or_ereport(lex, sem);
+ pg_parse_json_or_ereport(&lex, sem);
MemoryContextDelete(state->tmp_cxt);
@@ -2704,7 +2705,8 @@ populate_array_json(PopulateArrayContext *ctx, char *json, int len)
PopulateArrayState state;
JsonSemAction sem;
- state.lex = makeJsonLexContextCstringLen(json, len, GetDatabaseEncoding(), true);
+ state.lex = makeJsonLexContextCstringLen(NULL, json, len,
+ GetDatabaseEncoding(), true);
state.ctx = ctx;
memset(&sem, 0, sizeof(sem));
@@ -2720,7 +2722,7 @@ populate_array_json(PopulateArrayContext *ctx, char *json, int len)
/* number of dimensions should be already known */
Assert(ctx->ndims > 0 && ctx->dims);
- pfree(state.lex);
+ freeJsonLexContext(state.lex);
}
/*
@@ -3547,7 +3549,6 @@ get_json_object_as_hash(char *json, int len, const char *funcname)
HASHCTL ctl;
HTAB *tab;
JHashState *state;
- JsonLexContext *lex = makeJsonLexContextCstringLen(json, len, GetDatabaseEncoding(), true);
JsonSemAction *sem;
ctl.keysize = NAMEDATALEN;
@@ -3563,7 +3564,8 @@ get_json_object_as_hash(char *json, int len, const char *funcname)
state->function_name = funcname;
state->hash = tab;
- state->lex = lex;
+ state->lex = makeJsonLexContextCstringLen(NULL, json, len,
+ GetDatabaseEncoding(), true);
sem->semstate = (void *) state;
sem->array_start = hash_array_start;
@@ -3571,7 +3573,9 @@ get_json_object_as_hash(char *json, int len, const char *funcname)
sem->object_field_start = hash_object_field_start;
sem->object_field_end = hash_object_field_end;
- pg_parse_json_or_ereport(lex, sem);
+ pg_parse_json_or_ereport(state->lex, sem);
+
+ freeJsonLexContext(state->lex);
return tab;
}
@@ -3863,12 +3867,12 @@ populate_recordset_worker(FunctionCallInfo fcinfo, const char *funcname,
if (is_json)
{
text *json = PG_GETARG_TEXT_PP(json_arg_num);
- JsonLexContext *lex;
+ JsonLexContext lex;
JsonSemAction *sem;
sem = palloc0(sizeof(JsonSemAction));
- lex = makeJsonLexContext(json, true);
+ makeJsonLexContext(&lex, json, true);
sem->semstate = (void *) state;
sem->array_start = populate_recordset_array_start;
@@ -3879,9 +3883,12 @@ populate_recordset_worker(FunctionCallInfo fcinfo, const char *funcname,
sem->object_start = populate_recordset_object_start;
sem->object_end = populate_recordset_object_end;
- state->lex = lex;
+ state->lex = &lex;
- pg_parse_json_or_ereport(lex, sem);
+ pg_parse_json_or_ereport(&lex, sem);
+
+ freeJsonLexContext(&lex);
+ state->lex = NULL;
}
else
{
@@ -4217,16 +4224,16 @@ json_strip_nulls(PG_FUNCTION_ARGS)
{
text *json = PG_GETARG_TEXT_PP(0);
StripnullState *state;
- JsonLexContext *lex;
+ JsonLexContext lex;
JsonSemAction *sem;
- lex = makeJsonLexContext(json, true);
+ makeJsonLexContext(&lex, json, true);
state = palloc0(sizeof(StripnullState));
sem = palloc0(sizeof(JsonSemAction));
state->strval = makeStringInfo();
state->skip_next_null = false;
- state->lex = lex;
+ state->lex = &lex;
sem->semstate = (void *) state;
sem->object_start = sn_object_start;
@@ -4237,7 +4244,7 @@ json_strip_nulls(PG_FUNCTION_ARGS)
sem->array_element_start = sn_array_element_start;
sem->object_field_start = sn_object_field_start;
- pg_parse_json_or_ereport(lex, sem);
+ pg_parse_json_or_ereport(&lex, sem);
PG_RETURN_TEXT_P(cstring_to_text_with_len(state->strval->data,
state->strval->len));
@@ -5433,11 +5440,13 @@ void
iterate_json_values(text *json, uint32 flags, void *action_state,
JsonIterateStringValuesAction action)
{
- JsonLexContext *lex = makeJsonLexContext(json, true);
+ JsonLexContext lex;
JsonSemAction *sem = palloc0(sizeof(JsonSemAction));
IterateJsonStringValuesState *state = palloc0(sizeof(IterateJsonStringValuesState));
- state->lex = lex;
+ makeJsonLexContext(&lex, json, true);
+
+ state->lex = &lex;
state->action = action;
state->action_state = action_state;
state->flags = flags;
@@ -5446,7 +5455,7 @@ iterate_json_values(text *json, uint32 flags, void *action_state,
sem->scalar = iterate_values_scalar;
sem->object_field_start = iterate_values_object_field_start;
- pg_parse_json_or_ereport(lex, sem);
+ pg_parse_json_or_ereport(&lex, sem);
}
/*
@@ -5553,11 +5562,12 @@ text *
transform_json_string_values(text *json, void *action_state,
JsonTransformStringValuesAction transform_action)
{
- JsonLexContext *lex = makeJsonLexContext(json, true);
+ JsonLexContext lex;
JsonSemAction *sem = palloc0(sizeof(JsonSemAction));
TransformJsonStringValuesState *state = palloc0(sizeof(TransformJsonStringValuesState));
- state->lex = lex;
+ makeJsonLexContext(&lex, json, true);
+ state->lex = &lex;
state->strval = makeStringInfo();
state->action = transform_action;
state->action_state = action_state;
@@ -5571,7 +5581,7 @@ transform_json_string_values(text *json, void *action_state,
sem->array_element_start = transform_string_values_array_element_start;
sem->object_field_start = transform_string_values_object_field_start;
- pg_parse_json_or_ereport(lex, sem);
+ pg_parse_json_or_ereport(&lex, sem);
return cstring_to_text_with_len(state->strval->data, state->strval->len);
}
@@ -5670,19 +5680,19 @@ transform_string_values_scalar(void *state, char *token, JsonTokenType tokentype
JsonTokenType
json_get_first_token(text *json, bool throw_error)
{
- JsonLexContext *lex;
+ JsonLexContext lex;
JsonParseErrorType result;
- lex = makeJsonLexContext(json, false);
+ makeJsonLexContext(&lex, json, false);
/* Lex exactly one token from the input and check its type. */
- result = json_lex(lex);
+ result = json_lex(&lex);
if (result == JSON_SUCCESS)
- return lex->token_type;
+ return lex.token_type;
if (throw_error)
- json_errsave_error(result, lex, NULL);
+ json_errsave_error(result, &lex, NULL);
return JSON_TOKEN_INVALID; /* invalid json */
}
diff --git a/src/bin/pg_verifybackup/parse_manifest.c b/src/bin/pg_verifybackup/parse_manifest.c
index 2379f7be7b..f0acd9f1e7 100644
--- a/src/bin/pg_verifybackup/parse_manifest.c
+++ b/src/bin/pg_verifybackup/parse_manifest.c
@@ -130,7 +130,7 @@ json_parse_manifest(JsonManifestParseContext *context, char *buffer,
parse.saw_version_field = false;
/* Create a JSON lexing context. */
- lex = makeJsonLexContextCstringLen(buffer, size, PG_UTF8, true);
+ lex = makeJsonLexContextCstringLen(NULL, buffer, size, PG_UTF8, true);
/* Set up semantic actions. */
sem.semstate = &parse;
@@ -153,6 +153,8 @@ json_parse_manifest(JsonManifestParseContext *context, char *buffer,
/* Verify the manifest checksum. */
verify_manifest_checksum(&parse, buffer, size);
+
+ freeJsonLexContext(lex);
}
/*
diff --git a/src/common/jsonapi.c b/src/common/jsonapi.c
index 2e86589cfd..e30d8491c9 100644
--- a/src/common/jsonapi.c
+++ b/src/common/jsonapi.c
@@ -135,26 +135,59 @@ IsValidJsonNumber(const char *str, int len)
/*
* makeJsonLexContextCstringLen
+ * Initialize the given JsonLexContext object, or create one
*
- * lex constructor, with or without StringInfo object for de-escaped lexemes.
+ * If a valid 'lex' pointer is given, it is initialized. This can
+ * be used for stack-allocated structs, saving overhead. Otherwise,
+ * one is allocated.
*
- * Without is better as it makes the processing faster, so only make one
- * if really required.
+ * If need_escapes is true, ->strval stores the unescaped lexemes.
+ * Unescaping is expensive, so only request it when necessary.
+ *
+ * If either need_escapes or lex was given as NULL, then caller
+ * is responsible for freeing the object, either by calling
+ * freeJsonLexContext() or via memory context cleanup.
*/
JsonLexContext *
-makeJsonLexContextCstringLen(char *json, int len, int encoding, bool need_escapes)
+makeJsonLexContextCstringLen(JsonLexContext *lex, char *json,
+ int len, int encoding, bool need_escapes)
{
- JsonLexContext *lex = palloc0(sizeof(JsonLexContext));
+ if (lex == NULL)
+ {
+ lex = palloc0(sizeof(JsonLexContext));
+ lex->flags |= JSONLEX_FREE_STRUCT;
+ }
+ else
+ memset(lex, 0, sizeof(JsonLexContext));
lex->input = lex->token_terminator = lex->line_start = json;
lex->line_number = 1;
lex->input_length = len;
lex->input_encoding = encoding;
if (need_escapes)
+ {
lex->strval = makeStringInfo();
+ lex->flags |= JSONLEX_FREE_STRVAL;
+ }
+
return lex;
}
+/*
+ * Free memory in a JsonLexContext
+ */
+void
+freeJsonLexContext(JsonLexContext *lex)
+{
+ if (lex->flags & JSONLEX_FREE_STRVAL)
+ {
+ pfree(lex->strval->data);
+ pfree(lex->strval);
+ }
+ if (lex->flags & JSONLEX_FREE_STRUCT)
+ pfree(lex);
+}
+
/*
* pg_parse_json
*
diff --git a/src/include/common/jsonapi.h b/src/include/common/jsonapi.h
index 4310084b2b..a03d8310d4 100644
--- a/src/include/common/jsonapi.h
+++ b/src/include/common/jsonapi.h
@@ -71,6 +71,8 @@ typedef enum JsonParseErrorType
* AFTER the end of the token, i.e. where there would be a nul byte
* if we were using nul-terminated strings.
*/
+#define JSONLEX_FREE_STRUCT (1 << 0)
+#define JSONLEX_FREE_STRVAL (1 << 1)
typedef struct JsonLexContext
{
char *input;
@@ -84,6 +86,7 @@ typedef struct JsonLexContext
int line_number; /* line number, starting from 1 */
char *line_start; /* where that line starts within input */
StringInfo strval;
+ bits32 flags;
} JsonLexContext;
typedef JsonParseErrorType (*json_struct_action) (void *state);
@@ -151,15 +154,25 @@ extern JsonParseErrorType json_count_array_elements(JsonLexContext *lex,
int *elements);
/*
- * constructor for JsonLexContext, with or without strval element.
- * If supplied, the strval element will contain a de-escaped version of
- * the lexeme. However, doing this imposes a performance penalty, so
- * it should be avoided if the de-escaped lexeme is not required.
+ * initializer for JsonLexContext.
+ *
+ * If a valid 'lex' pointer is given, it is initialized. This can be used
+ * for stack-allocated structs, saving overhead. If NULL is given, a new
+ * struct is allocated.
+ *
+ * If need_escapes is true, ->strval stores the unescaped lexemes.
+ * Unescaping is expensive, so only request it when necessary.
+ *
+ * If either need_escapes or lex was given as NULL, then the caller is
+ * responsible for freeing the returned struct, either by calling
+ * freeJsonLexContext() or via memory context cleanup.
*/
-extern JsonLexContext *makeJsonLexContextCstringLen(char *json,
+extern JsonLexContext *makeJsonLexContextCstringLen(JsonLexContext *lex,
+ char *json,
int len,
int encoding,
bool need_escapes);
+extern void freeJsonLexContext(JsonLexContext *lex);
/* lex one token */
extern JsonParseErrorType json_lex(JsonLexContext *lex);
diff --git a/src/include/utils/jsonfuncs.h b/src/include/utils/jsonfuncs.h
index c677ac8ff7..8d77aa9de0 100644
--- a/src/include/utils/jsonfuncs.h
+++ b/src/include/utils/jsonfuncs.h
@@ -37,7 +37,7 @@ typedef void (*JsonIterateStringValuesAction) (void *state, char *elem_value, in
typedef text *(*JsonTransformStringValuesAction) (void *state, char *elem_value, int elem_len);
/* build a JsonLexContext from a text datum */
-extern JsonLexContext *makeJsonLexContext(text *json, bool need_escapes);
+extern JsonLexContext *makeJsonLexContext(JsonLexContext *lex, text *json, bool need_escapes);
/* try to parse json, and errsave(escontext) on failure */
extern bool pg_parse_json_or_errsave(JsonLexContext *lex, JsonSemAction *sem,
--
2.39.2
--lzw6hdh7nrlzilxo--
^ permalink raw reply [nested|flat] 7+ messages in thread
* [PATCH 3/6] Move conversion of a "historic" to MVCC snapshot to a separate function.
@ 2026-01-27 10:48 Antonin Houska <[email protected]>
0 siblings, 0 replies; 7+ messages in thread
From: Antonin Houska @ 2026-01-27 10:48 UTC (permalink / raw)
The conversion is now handled by SnapBuildMVCCFromHistoric(). REPACK
CONCURRENTLY will also need it.
---
src/backend/replication/logical/snapbuild.c | 59 +++++++++++++++++----
src/backend/utils/time/snapmgr.c | 3 +-
src/include/replication/snapbuild.h | 1 +
src/include/utils/snapmgr.h | 1 +
4 files changed, 52 insertions(+), 12 deletions(-)
diff --git a/src/backend/replication/logical/snapbuild.c b/src/backend/replication/logical/snapbuild.c
index 7f79621b57e..a738ad8a864 100644
--- a/src/backend/replication/logical/snapbuild.c
+++ b/src/backend/replication/logical/snapbuild.c
@@ -440,10 +440,7 @@ Snapshot
SnapBuildInitialSnapshot(SnapBuild *builder)
{
Snapshot snap;
- TransactionId xid;
TransactionId safeXid;
- TransactionId *newxip;
- int newxcnt = 0;
Assert(XactIsoLevel == XACT_REPEATABLE_READ);
Assert(builder->building_full_snapshot);
@@ -485,7 +482,35 @@ SnapBuildInitialSnapshot(SnapBuild *builder)
MyProc->xmin = snap->xmin;
- /* allocate in transaction context */
+ /* Convert the historic snapshot to MVCC snapshot. */
+ return SnapBuildMVCCFromHistoric(snap, true);
+}
+
+/*
+ * Turn a historic MVCC snapshot into an ordinary MVCC snapshot.
+ *
+ * Unlike a regular (non-historic) MVCC snapshot, the 'xip' array of this
+ * snapshot contains not only running main transactions, but also their
+ * subtransactions. On the other hand, 'subxip' will usually be empty. This
+ * difference does not affect the result of XidInMVCCSnapshot() because it
+ * searches both in 'xip' and 'subxip'.
+ *
+ * Pass true for 'in_place' if you don't care about modifying the source
+ * snapshot. If you need a new instance, and one that was allocated as a
+ * single chunk of memory, pass false.
+ */
+Snapshot
+SnapBuildMVCCFromHistoric(Snapshot snapshot, bool in_place)
+{
+ TransactionId xid;
+ TransactionId *oldxip = snapshot->xip;
+ uint32 oldxcnt = snapshot->xcnt;
+ TransactionId *newxip;
+ int newxcnt = 0;
+ Snapshot result;
+
+ Assert(snapshot->snapshot_type == SNAPSHOT_HISTORIC_MVCC);
+
newxip = palloc_array(TransactionId, GetMaxSnapshotXidCount());
/*
@@ -494,7 +519,7 @@ SnapBuildInitialSnapshot(SnapBuild *builder)
* classical snapshot by marking all non-committed transactions as
* in-progress. This can be expensive.
*/
- for (xid = snap->xmin; NormalTransactionIdPrecedes(xid, snap->xmax);)
+ for (xid = snapshot->xmin; NormalTransactionIdPrecedes(xid, snapshot->xmax);)
{
void *test;
@@ -502,7 +527,7 @@ SnapBuildInitialSnapshot(SnapBuild *builder)
* Check whether transaction committed using the decoding snapshot
* meaning of ->xip.
*/
- test = bsearch(&xid, snap->xip, snap->xcnt,
+ test = bsearch(&xid, snapshot->xip, snapshot->xcnt,
sizeof(TransactionId), xidComparator);
if (test == NULL)
@@ -519,11 +544,25 @@ SnapBuildInitialSnapshot(SnapBuild *builder)
}
/* adjust remaining snapshot fields as needed */
- snap->snapshot_type = SNAPSHOT_MVCC;
- snap->xcnt = newxcnt;
- snap->xip = newxip;
+ snapshot->xcnt = newxcnt;
+ snapshot->xip = newxip;
+
+ if (in_place)
+ result = snapshot;
+ else
+ {
+ result = CopySnapshot(snapshot);
+
+ /* Restore the original values so the source is intact. */
+ snapshot->xip = oldxip;
+ snapshot->xcnt = oldxcnt;
+
+ /* newxip has been copied */
+ pfree(newxip);
+ }
+ result->snapshot_type = SNAPSHOT_MVCC;
- return snap;
+ return result;
}
/*
diff --git a/src/backend/utils/time/snapmgr.c b/src/backend/utils/time/snapmgr.c
index 2e6197f5f35..3af1b366adf 100644
--- a/src/backend/utils/time/snapmgr.c
+++ b/src/backend/utils/time/snapmgr.c
@@ -213,7 +213,6 @@ typedef struct ExportedSnapshot
static List *exportedSnapshots = NIL;
/* Prototypes for local functions */
-static Snapshot CopySnapshot(Snapshot snapshot);
static void UnregisterSnapshotNoOwner(Snapshot snapshot);
static void FreeSnapshot(Snapshot snapshot);
static void SnapshotResetXmin(void);
@@ -604,7 +603,7 @@ SetTransactionSnapshot(Snapshot sourcesnap, VirtualTransactionId *sourcevxid,
* The copy is palloc'd in TopTransactionContext and has initial refcounts set
* to 0. The returned snapshot has the copied flag set.
*/
-static Snapshot
+Snapshot
CopySnapshot(Snapshot snapshot)
{
Snapshot newsnap;
diff --git a/src/include/replication/snapbuild.h b/src/include/replication/snapbuild.h
index ccded021433..34383dea776 100644
--- a/src/include/replication/snapbuild.h
+++ b/src/include/replication/snapbuild.h
@@ -73,6 +73,7 @@ extern void FreeSnapshotBuilder(SnapBuild *builder);
extern void SnapBuildSnapDecRefcount(Snapshot snap);
extern Snapshot SnapBuildInitialSnapshot(SnapBuild *builder);
+extern Snapshot SnapBuildMVCCFromHistoric(Snapshot snapshot, bool in_place);
extern const char *SnapBuildExportSnapshot(SnapBuild *builder);
extern void SnapBuildClearExportedSnapshot(void);
extern void SnapBuildResetExportedSnapshotState(void);
diff --git a/src/include/utils/snapmgr.h b/src/include/utils/snapmgr.h
index b8c01a291a1..de824945f0b 100644
--- a/src/include/utils/snapmgr.h
+++ b/src/include/utils/snapmgr.h
@@ -63,6 +63,7 @@ extern Snapshot GetTransactionSnapshot(void);
extern Snapshot GetLatestSnapshot(void);
extern void SnapshotSetCommandId(CommandId curcid);
+extern Snapshot CopySnapshot(Snapshot snapshot);
extern Snapshot GetCatalogSnapshot(Oid relid);
extern Snapshot GetNonHistoricCatalogSnapshot(Oid relid);
extern void InvalidateCatalogSnapshot(void);
--
2.47.3
--=-=-=
Content-Type: text/plain
Content-Disposition: attachment;
filename=v32-0004-Add-CONCURRENTLY-option-to-REPACK-command.patch
^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: [PATCH] Resolve unknown-type literals in GRAPH_TABLE COLUMNS
@ 2026-05-04 15:12 Peter Eisentraut <[email protected]>
2026-05-12 06:20 ` Re: [PATCH] Resolve unknown-type literals in GRAPH_TABLE COLUMNS Ashutosh Bapat <[email protected]>
0 siblings, 1 reply; 7+ messages in thread
From: Peter Eisentraut @ 2026-05-04 15:12 UTC (permalink / raw)
To: Ashutosh Bapat <[email protected]>; SATYANARAYANA NARLAPURAM <[email protected]>; +Cc: Junwang Zhao <[email protected]>; PostgreSQL Hackers <[email protected]>
On 29.04.26 16:09, Ashutosh Bapat wrote:
> On Mon, Apr 27, 2026 at 11:34 AM SATYANARAYANA NARLAPURAM
> <[email protected]> wrote:
>>
>> Hi,
>>
>> On Sun, Apr 26, 2026 at 8:10 AM Junwang Zhao <[email protected]> wrote:
>>>
>>> Hi SATYANARAYANA,
>>>
>>> On Sun, Apr 26, 2026 at 3:53 AM SATYANARAYANA NARLAPURAM
>>> <[email protected]> wrote:
>>>>
>>>> Hi hackers,
>>>>
>>>> transformRangeGraphTable() calls transformExpr() and
>>>> assign_list_collations() for COLUMNS expressions but missed calling
>>>> resolveTargetListUnknowns(). As a result, literals such as 'val1'
>>>> in a COLUMNS clause retained type "unknown", causing failures with
>>>> ORDER BY, UNION, and output conversions.
>>>>
>>>> Fix by calling resolveTargetListUnknowns() on the columns target
>>>> list right after assign_list_collations(), similar to SELECT target lists in
>>>> transformSelectStmt().
>>>>
>>>> Attached a patch to fix this, which also includes test cases to reproduce.
>>>
>>> I can reproduce this and the patch fixes it.
>>>
>>> One question: why is resolveTargetListUnknowns called after
>>> assign_list_collations?
>>>
>>> I'm asking because in transformSelectStmt, resolveTargetListUnknowns
>>> is invoked before assign_query_collations. It might not matter, but keeping
>>> the order consistent would be good for readers.
>>
>>
>> Updated the patch. It should be before.
>
> Do we really need a test for ORDER BY on a literal column? I replaced
> all the test queries with a single one which covers all the scenarios
> covered by those queries.
>
> The patch needed to consider pstate->p_resolve_unknowns. Unknown
> literals are not resolved as text always. See the test query.
I couldn't find a commit to apply this patch cleanly (the subject says
patch 5/5, so maybe you had some unpublished local changes?). After
applying the test case manually, it looks like the test output is
already correct without the code change. So if this patch is still
required, we need a better test case.
^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: [PATCH] Resolve unknown-type literals in GRAPH_TABLE COLUMNS
2026-05-04 15:12 Re: [PATCH] Resolve unknown-type literals in GRAPH_TABLE COLUMNS Peter Eisentraut <[email protected]>
@ 2026-05-12 06:20 ` Ashutosh Bapat <[email protected]>
2026-07-03 15:17 ` Re: [PATCH] Resolve unknown-type literals in GRAPH_TABLE COLUMNS Peter Eisentraut <[email protected]>
0 siblings, 1 reply; 7+ messages in thread
From: Ashutosh Bapat @ 2026-05-12 06:20 UTC (permalink / raw)
To: Peter Eisentraut <[email protected]>; +Cc: SATYANARAYANA NARLAPURAM <[email protected]>; Junwang Zhao <[email protected]>; PostgreSQL Hackers <[email protected]>
On Mon, May 4, 2026 at 8:42 PM Peter Eisentraut <[email protected]> wrote:
>
> On 29.04.26 16:09, Ashutosh Bapat wrote:
> > On Mon, Apr 27, 2026 at 11:34 AM SATYANARAYANA NARLAPURAM
> > <[email protected]> wrote:
> >>
> >> Hi,
> >>
> >> On Sun, Apr 26, 2026 at 8:10 AM Junwang Zhao <[email protected]> wrote:
> >>>
> >>> Hi SATYANARAYANA,
> >>>
> >>> On Sun, Apr 26, 2026 at 3:53 AM SATYANARAYANA NARLAPURAM
> >>> <[email protected]> wrote:
> >>>>
> >>>> Hi hackers,
> >>>>
> >>>> transformRangeGraphTable() calls transformExpr() and
> >>>> assign_list_collations() for COLUMNS expressions but missed calling
> >>>> resolveTargetListUnknowns(). As a result, literals such as 'val1'
> >>>> in a COLUMNS clause retained type "unknown", causing failures with
> >>>> ORDER BY, UNION, and output conversions.
> >>>>
> >>>> Fix by calling resolveTargetListUnknowns() on the columns target
> >>>> list right after assign_list_collations(), similar to SELECT target lists in
> >>>> transformSelectStmt().
> >>>>
> >>>> Attached a patch to fix this, which also includes test cases to reproduce.
> >>>
> >>> I can reproduce this and the patch fixes it.
> >>>
> >>> One question: why is resolveTargetListUnknowns called after
> >>> assign_list_collations?
> >>>
> >>> I'm asking because in transformSelectStmt, resolveTargetListUnknowns
> >>> is invoked before assign_query_collations. It might not matter, but keeping
> >>> the order consistent would be good for readers.
> >>
> >>
> >> Updated the patch. It should be before.
> >
> > Do we really need a test for ORDER BY on a literal column? I replaced
> > all the test queries with a single one which covers all the scenarios
> > covered by those queries.
> >
> > The patch needed to consider pstate->p_resolve_unknowns. Unknown
> > literals are not resolved as text always. See the test query.
>
> I couldn't find a commit to apply this patch cleanly (the subject says
> patch 5/5, so maybe you had some unpublished local changes?).
I am maintaining all the SQL/PGQ fixes in the same branch and creating
patches from that branch. Hence 5/5. But still it shouldn't have
applied cleanly. Both git cherry-pick and git am <attached patch
file>, on a fresh branch succeeded. Please let me know if you still
face the issue.
> After
> applying the test case manually, it looks like the test output is
> already correct without the code change. So if this patch is still
> required, we need a better test case.
>
Yes, the test query doesn't reproduce the bug. Actually all but only
two queries from the Satya's patch show the bug. I have included a
test query which fails, with expected error message, without code
changes now. Also I don't think we need a separate section for this
test query. Included the query in one of the earliest sections in the
file.
--
Best Wishes,
Ashutosh Bapat
Attachments:
[text/x-patch] v20260512-0001-Resolve-unknown-type-literals-in-GRAPH_TAB.patch (3.8K, ../../CAExHW5sG6f5m22viW99wgsOmAXXK9312obZJMfZjCUevM7K9rg@mail.gmail.com/2-v20260512-0001-Resolve-unknown-type-literals-in-GRAPH_TAB.patch)
download | inline diff:
From 69fcbf2b907ff83aaa17593c5596e495cc0720e7 Mon Sep 17 00:00:00 2001
From: Ashutosh Bapat <[email protected]>
Date: Wed, 29 Apr 2026 19:07:13 +0530
Subject: [PATCH v20260512 4/4] Resolve unknown-type literals in GRAPH_TABLE
COLUMNS
The unknown-type literals in the COLUMNS clause of a GRAPH_TABLE are now
resolved to the appropriate types, causing various failures. Call
resolveTargetListUnknowns() on the columns targetlist to resolve unknonw type
literals. Do this before assigning collations so that the resolved types are
used for collations assignment.
Reported by: Satya Narlapuram <[email protected]>
Author: Satya Narlapuram <[email protected]>
Author: Ashutosh Bapat <[email protected]>
Reviewed by: Junwang Zhao <[email protected]>
Discussion: https://www.postgresql.org/message-id/CAHg+QDcyKNWyzDoKMxiZNjv7C-wAxs8y0ZoNkOV137Y+nk3UXg@mail.gmail.com
---
src/backend/parser/parse_clause.c | 4 ++++
src/test/regress/expected/graph_table.out | 9 +++++++++
src/test/regress/sql/graph_table.sql | 2 ++
3 files changed, 15 insertions(+)
diff --git a/src/backend/parser/parse_clause.c b/src/backend/parser/parse_clause.c
index 4270c2382c4..3d0585b8efb 100644
--- a/src/backend/parser/parse_clause.c
+++ b/src/backend/parser/parse_clause.c
@@ -1003,6 +1003,10 @@ transformRangeGraphTable(ParseState *pstate, RangeGraphTable *rgt)
columns = lappend(columns, te);
}
+ /* resolve any still-unresolved output columns as being type text */
+ if (pstate->p_resolve_unknowns)
+ resolveTargetListUnknowns(pstate, columns);
+
/*
* Assign collations to column expressions now since
* assign_query_collations() does not process rangetable entries.
diff --git a/src/test/regress/expected/graph_table.out b/src/test/regress/expected/graph_table.out
index cc6d80afd82..e8d49fd5cd4 100644
--- a/src/test/regress/expected/graph_table.out
+++ b/src/test/regress/expected/graph_table.out
@@ -160,6 +160,15 @@ SELECT * FROM GRAPH_TABLE (myshop MATCH (c IS customers) COLUMNS (c.name));
customer3
(3 rows)
+-- Unknown type resolution
+SELECT *, pg_typeof(unknown_col) AS unknown_col_type, pg_typeof(null_col) AS null_col_type FROM GRAPH_TABLE (myshop MATCH (c IS customers) COLUMNS (c.name, 'unknown-literal' AS unknown_col, NULL AS null_col));
+ name | unknown_col | null_col | unknown_col_type | null_col_type
+-----------+-----------------+----------+------------------+---------------
+ customer1 | unknown-literal | | text | text
+ customer2 | unknown-literal | | text | text
+ customer3 | unknown-literal | | text | text
+(3 rows)
+
SELECT * FROM GRAPH_TABLE (myshop MATCH (c IS customers WHERE c.address = 'US')-[IS customer_orders]->(o IS orders) COLUMNS (c.name));
name
-----------
diff --git a/src/test/regress/sql/graph_table.sql b/src/test/regress/sql/graph_table.sql
index 0e381ec72bc..e761f09e057 100644
--- a/src/test/regress/sql/graph_table.sql
+++ b/src/test/regress/sql/graph_table.sql
@@ -134,6 +134,8 @@ INSERT INTO wishlist_items (wishlist_items_id, wishlist_id, product_no) VALUES
-- single element path pattern
SELECT * FROM GRAPH_TABLE (myshop MATCH (c IS customers) COLUMNS (c.name));
+-- Unknown type resolution
+SELECT *, pg_typeof(unknown_col) AS unknown_col_type, pg_typeof(null_col) AS null_col_type FROM GRAPH_TABLE (myshop MATCH (c IS customers) COLUMNS (c.name, 'unknown-literal' AS unknown_col, NULL AS null_col));
SELECT * FROM GRAPH_TABLE (myshop MATCH (c IS customers WHERE c.address = 'US')-[IS customer_orders]->(o IS orders) COLUMNS (c.name));
-- graph element specification without label or variable
SELECT * FROM GRAPH_TABLE (myshop MATCH (c IS customers WHERE c.address = 'US')-[]->(o IS orders) COLUMNS (c.name AS customer_name));
--
2.34.1
^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: [PATCH] Resolve unknown-type literals in GRAPH_TABLE COLUMNS
2026-05-04 15:12 Re: [PATCH] Resolve unknown-type literals in GRAPH_TABLE COLUMNS Peter Eisentraut <[email protected]>
2026-05-12 06:20 ` Re: [PATCH] Resolve unknown-type literals in GRAPH_TABLE COLUMNS Ashutosh Bapat <[email protected]>
@ 2026-07-03 15:17 ` Peter Eisentraut <[email protected]>
2026-07-06 11:38 ` Re: [PATCH] Resolve unknown-type literals in GRAPH_TABLE COLUMNS Ashutosh Bapat <[email protected]>
0 siblings, 1 reply; 7+ messages in thread
From: Peter Eisentraut @ 2026-07-03 15:17 UTC (permalink / raw)
To: Ashutosh Bapat <[email protected]>; +Cc: SATYANARAYANA NARLAPURAM <[email protected]>; Junwang Zhao <[email protected]>; PostgreSQL Hackers <[email protected]>
On 12.05.26 08:20, Ashutosh Bapat wrote:
> Yes, the test query doesn't reproduce the bug. Actually all but only
> two queries from the Satya's patch show the bug. I have included a
> test query which fails, with expected error message, without code
> changes now. Also I don't think we need a separate section for this
> test query. Included the query in one of the earliest sections in the
> file.
committed
^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: [PATCH] Resolve unknown-type literals in GRAPH_TABLE COLUMNS
2026-05-04 15:12 Re: [PATCH] Resolve unknown-type literals in GRAPH_TABLE COLUMNS Peter Eisentraut <[email protected]>
2026-05-12 06:20 ` Re: [PATCH] Resolve unknown-type literals in GRAPH_TABLE COLUMNS Ashutosh Bapat <[email protected]>
2026-07-03 15:17 ` Re: [PATCH] Resolve unknown-type literals in GRAPH_TABLE COLUMNS Peter Eisentraut <[email protected]>
@ 2026-07-06 11:38 ` Ashutosh Bapat <[email protected]>
0 siblings, 0 replies; 7+ messages in thread
From: Ashutosh Bapat @ 2026-07-06 11:38 UTC (permalink / raw)
To: Peter Eisentraut <[email protected]>; +Cc: SATYANARAYANA NARLAPURAM <[email protected]>; Junwang Zhao <[email protected]>; PostgreSQL Hackers <[email protected]>
On Fri, Jul 3, 2026 at 8:47 PM Peter Eisentraut <[email protected]> wrote:
>
> On 12.05.26 08:20, Ashutosh Bapat wrote:
> > Yes, the test query doesn't reproduce the bug. Actually all but only
> > two queries from the Satya's patch show the bug. I have included a
> > test query which fails, with expected error message, without code
> > changes now. Also I don't think we need a separate section for this
> > test query. Included the query in one of the earliest sections in the
> > file.
>
> committed
>
Thanks.
I actually had another fix in the related area for an issue reported
in [1]. I shared the patch on that thread, but missed to do it before.
Sorry. Will share rebased patch on that thread.
[1] Discussion:
https://www.postgresql.org/message-id/[email protected]
--
Best Wishes,
Ashutosh Bapat
^ permalink raw reply [nested|flat] 7+ messages in thread
end of thread, other threads:[~2026-07-06 11:38 UTC | newest]
Thread overview: 7+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2018-11-27 05:42 [PATCH 5/5] Remove the GUC stats_temp_directory Kyotaro Horiguchi <[email protected]>
2023-08-03 09:44 [PATCH] JsonLexContext allocation/free Alvaro Herrera <[email protected]>
2026-01-27 10:48 [PATCH 3/6] Move conversion of a "historic" to MVCC snapshot to a separate function. Antonin Houska <[email protected]>
2026-05-04 15:12 Re: [PATCH] Resolve unknown-type literals in GRAPH_TABLE COLUMNS Peter Eisentraut <[email protected]>
2026-05-12 06:20 ` Re: [PATCH] Resolve unknown-type literals in GRAPH_TABLE COLUMNS Ashutosh Bapat <[email protected]>
2026-07-03 15:17 ` Re: [PATCH] Resolve unknown-type literals in GRAPH_TABLE COLUMNS Peter Eisentraut <[email protected]>
2026-07-06 11:38 ` Re: [PATCH] Resolve unknown-type literals in GRAPH_TABLE COLUMNS Ashutosh Bapat <[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