public inbox for [email protected]help / color / mirror / Atom feed
[PATCH 2/4] Allow TAP test to excecise tablespace. 7+ messages / 4 participants [nested] [flat]
* [PATCH 2/4] Allow TAP test to excecise tablespace. @ 2019-04-22 11:58 Kyotaro Horiguchi <[email protected]> 0 siblings, 0 replies; 7+ messages in thread From: Kyotaro Horiguchi @ 2019-04-22 11:58 UTC (permalink / raw) To perform tablespace related checks, this patch lets PostgresNode::backup have a new parameter "tablespace_mapping", and make init_from_backup handle capable to handle a backup created using tablespace_mapping. --- src/test/perl/PostgresNode.pm | 45 +++++++++++++++++++++++++++++++++++++----- src/test/perl/RecursiveCopy.pm | 38 +++++++++++++++++++++++++++++++---- 2 files changed, 74 insertions(+), 9 deletions(-) diff --git a/src/test/perl/PostgresNode.pm b/src/test/perl/PostgresNode.pm index 76874141c5..e951acc461 100644 --- a/src/test/perl/PostgresNode.pm +++ b/src/test/perl/PostgresNode.pm @@ -157,6 +157,7 @@ sub new _host => $pghost, _basedir => "$TestLib::tmp_check/t_${testname}_${name}_data", _name => $name, + _tablespaces => [], _logfile_generation => 0, _logfile_base => "$TestLib::log_path/${testname}_${name}", _logfile => "$TestLib::log_path/${testname}_${name}.log" @@ -342,6 +343,26 @@ sub backup_dir =pod +=item $node->make_tablespace_dir() + +make a tablespace directory + +=cut + +sub make_tablespace_dir +{ + my ($self, $name) = @_; + my $basedir = $self->basedir; + + die "tablespace name contains '/'" if ($name =~ m#/#); + my $reldir = "../$name"; + mkdir $self->data_dir() . "/". $reldir; + push($self->{_tablespaces}, $reldir); + return $reldir; +} + +=pod + =item $node->info() Return a string containing human-readable diagnostic information (paths, etc) @@ -540,13 +561,27 @@ target server since it isn't done by default. sub backup { - my ($self, $backup_name) = @_; - my $backup_path = $self->backup_dir . '/' . $backup_name; + my ($self, $backup_name, %params) = @_; + my $backup_path = $self->backup_dir . '/' . $backup_name . '/' . "pgdata"; my $name = $self->name; + my @rest = (); + + if (defined $params{tablespace_mapping}) + { + foreach my $p (split /,/, $params{tablespace_mapping}) + { + push(@rest, "--tablespace-mapping=$p"); + } + } + + foreach my $p ($self->{_tablespaces}) + { + mkdir "$backup_path/$p"; + } print "# Taking pg_basebackup $backup_name from node \"$name\"\n"; TestLib::system_or_bail('pg_basebackup', '-D', $backup_path, '-h', - $self->host, '-p', $self->port, '--no-sync'); + $self->host, '-p', $self->port, '--no-sync', @rest); print "# Backup finished\n"; return; } @@ -592,7 +627,7 @@ sub backup_fs_cold sub _backup_fs { my ($self, $backup_name, $hot) = @_; - my $backup_path = $self->backup_dir . '/' . $backup_name; + my $backup_path = $self->backup_dir . '/' . $backup_name . '/' . "pgdata"; my $port = $self->port; my $name = $self->name; @@ -655,7 +690,7 @@ unconditionally set to enable replication connections. sub init_from_backup { my ($self, $root_node, $backup_name, %params) = @_; - my $backup_path = $root_node->backup_dir . '/' . $backup_name; + my $backup_path = $root_node->backup_dir . '/' . $backup_name . '/' . "pgdata"; my $host = $self->host; my $port = $self->port; my $node_name = $self->name; diff --git a/src/test/perl/RecursiveCopy.pm b/src/test/perl/RecursiveCopy.pm index baf5d0ac63..f165db7348 100644 --- a/src/test/perl/RecursiveCopy.pm +++ b/src/test/perl/RecursiveCopy.pm @@ -22,6 +22,7 @@ use warnings; use Carp; use File::Basename; use File::Copy; +use TestLib; =pod @@ -97,14 +98,43 @@ sub _copypath_recurse # invoke the filter and skip all further operation if it returns false return 1 unless &$filterfn($curr_path); - # Check for symlink -- needed only on source dir - # (note: this will fall through quietly if file is already gone) - croak "Cannot operate on symlink \"$srcpath\"" if -l $srcpath; - # Abort if destination path already exists. Should we allow directories # to exist already? croak "Destination path \"$destpath\" already exists" if -e $destpath; + # Check for symlink -- needed only on source dir + # (note: this will fall through quietly if file is already gone) + if (-l $srcpath) + { + croak "Cannot operate on symlink \"$srcpath\"" + if ($srcpath !~ /\/(pg_tblspc\/[0-9]+)$/); + + # We have mapped tablespaces. Copy them individually + my $linkname = $1; + my $rpath = my $target = readlink($srcpath); + + #convert to pgdata-based if relative + $rpath =~ s#^\.\./##; + + my $srcrealdir = "$base_src_dir/$rpath"; + my $dstrealdir = "$base_dest_dir/$rpath"; + + mkdir $dstrealdir; + opendir(my $dh, $srcrealdir); + while (readdir $dh) + { + next if (/^\.\.?$/); + my $spath = "$srcrealdir/$_"; + my $dpath = "$dstrealdir/$_"; + + copypath($spath, $dpath); + } + closedir $dh; + + symlink $target, $destpath; + return 1; + } + # If this source path is a file, simply copy it to destination with the # same name and we're done. if (-f $srcpath) -- 2.16.3 ----Next_Part(Wed_Apr_24_13_18_45_2019_938)-- Content-Type: Text/X-Patch; charset=us-ascii Content-Transfer-Encoding: 7bit Content-Disposition: inline; filename="0003-Add-check-for-recovery-failure-caused-by-tablespace.patch" ^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: Remove source code display from \df+? @ 2023-01-23 01:23 Isaac Morland <[email protected]> 0 siblings, 2 replies; 7+ messages in thread From: Isaac Morland @ 2023-01-23 01:23 UTC (permalink / raw) To: Justin Pryzby <[email protected]>; +Cc: Tom Lane <[email protected]>; Alvaro Herrera <[email protected]>; Pavel Stehule <[email protected]>; Magnus Hagander <[email protected]>; pgsql-hackers On Sun, 22 Jan 2023 at 17:27, Justin Pryzby <[email protected]> wrote: > On Sun, Jan 22, 2023 at 04:28:21PM -0500, Isaac Morland wrote: > > On Sun, 22 Jan 2023 at 15:04, Tom Lane <[email protected]> wrote: > But now I'm having a problem I don't understand: the CI are still > failling, > > but not in the psql test. Instead, I get this: > > > > [20:11:17.624] +++ tap check in src/bin/pg_upgrade +++ > > You'll find the diff in the "artifacts", but not a separate "diff" file. > > https://api.cirrus-ci.com/v1/artifact/task/6146418377752576/testrun/build/testrun/pg_upgrade/002_pg_... > > CREATE FUNCTION public.regress_psql_df_sql() RETURNS void > LANGUAGE sql > BEGIN ATOMIC > - SELECT NULL::text; > + SELECT NULL::text AS text; > END; > > It's failing because after restoring the function, the column is named > "text" - maybe it's a bug. > OK, thanks. I'd say I've uncovered a bug related to pg_upgrade, unless I’m missing something. However, I've adjusted my patch so that nothing it creates is kept. This seems tidier even without the test failure. Tom's earlier point was that neither the function nor its owner needs to > be preserved (as is done to exercise pg_dump/restore/upgrade - surely > functions are already tested). Dropping it when you're done running \df > will avoid any possible issue with pg_upgrade. > > Were you able to test with your own github account ? > I haven’t had a chance to try this. I must confess to being a bit confused by the distinction between running the CI tests and doing "make check"; ideally I would like to be able to run all the tests on my own machine without any external resources. But at the same time I don’t pretend to understand the full situation so I will try to use this when I get some time. Attachments: [application/octet-stream] 0001-Remove-source-code-display-from-df-v6.patch (6.5K, ../../CAMsGm5f97iYDV9DF8sLjH2Lc+2cjis7OuYVEtGw+c21rGPUy2g@mail.gmail.com/3-0001-Remove-source-code-display-from-df-v6.patch) download | inline diff: From 114c154fbb3a4e75a6abed9dcecf4d19b8a344ad Mon Sep 17 00:00:00 2001 From: Isaac Morland <[email protected]> Date: Tue, 17 Jan 2023 14:17:42 -0500 Subject: [PATCH] Remove source code display from \df+ The column is renamed to "Internal name" and will still show the internal name for C and Internal functions. --- doc/src/sgml/ref/psql-ref.sgml | 3 +- src/bin/psql/describe.c | 11 ++------ src/test/regress/expected/psql.out | 45 ++++++++++++++++++++++++++++++ src/test/regress/sql/psql.sql | 37 ++++++++++++++++++++++++ 4 files changed, 87 insertions(+), 9 deletions(-) diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml index dc6528dc11..afdf8668d6 100644 --- a/doc/src/sgml/ref/psql-ref.sgml +++ b/doc/src/sgml/ref/psql-ref.sgml @@ -1650,7 +1650,8 @@ INSERT INTO tbl1 VALUES ($1, $2) \bind 'first value' 'second value' \g If the form <literal>\df+</literal> is used, additional information about each function is shown, including volatility, parallel safety, owner, security classification, access privileges, - language, source code and description. + language, internal name (for C and Internal language functions only), + and description. Source code can be shown using <literal>\sf</literal>. </para> </listitem> diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c index c8a0bb7b3a..e487fcd480 100644 --- a/src/bin/psql/describe.c +++ b/src/bin/psql/describe.c @@ -410,14 +410,9 @@ describeFunctions(const char *functypes, const char *func_pattern, appendPQExpBuffer(&buf, ",\n l.lanname as \"%s\"", gettext_noop("Language")); - if (pset.sversion >= 140000) - appendPQExpBuffer(&buf, - ",\n COALESCE(pg_catalog.pg_get_function_sqlbody(p.oid), p.prosrc) as \"%s\"", - gettext_noop("Source code")); - else - appendPQExpBuffer(&buf, - ",\n p.prosrc as \"%s\"", - gettext_noop("Source code")); + appendPQExpBuffer(&buf, + ",\n CASE WHEN l.lanname IN ('internal', 'c') THEN p.prosrc END AS \"%s\"", + gettext_noop("Internal name")); appendPQExpBuffer(&buf, ",\n pg_catalog.obj_description(p.oid, 'pg_proc') as \"%s\"", gettext_noop("Description")); diff --git a/src/test/regress/expected/psql.out b/src/test/regress/expected/psql.out index 8fc62cebd2..fffb6baace 100644 --- a/src/test/regress/expected/psql.out +++ b/src/test/regress/expected/psql.out @@ -5247,6 +5247,51 @@ reset work_mem; pg_catalog | && | anyarray | anyarray | boolean | overlaps (1 row) +-- check \df+ +BEGIN; +CREATE ROLE regress_psql_df; +CREATE OR REPLACE FUNCTION regress_psql_df_c (integer, integer, cstring, internal, integer) + RETURNS VOID + LANGUAGE c + IMMUTABLE PARALLEL SAFE STRICT +AS '$libdir/utf8_and_iso8859', 'utf8_to_iso8859'; +CREATE OR REPLACE FUNCTION regress_psql_df_internal (regclass, text) + RETURNS bigint + LANGUAGE internal + PARALLEL SAFE STRICT +AS 'pg_relation_size'; +CREATE OR REPLACE FUNCTION regress_psql_df_sql () + RETURNS VOID +BEGIN ATOMIC + SELECT NULL; +END; +CREATE OR REPLACE FUNCTION regress_psql_df_plpgsql () + RETURNS VOID + LANGUAGE plpgsql +AS $$ +BEGIN + RETURN; +END; +$$; +ALTER FUNCTION regress_psql_df_c (integer, integer, cstring, internal, integer) + OWNER TO regress_psql_df; +ALTER FUNCTION regress_psql_df_internal (regclass, text) + OWNER TO regress_psql_df; +ALTER FUNCTION regress_psql_df_sql () + OWNER TO regress_psql_df; +ALTER FUNCTION regress_psql_df_plpgsql () + OWNER TO regress_psql_df; +\df+ regress_psql_df_* + List of functions + Schema | Name | Result data type | Argument data types | Type | Volatility | Parallel | Owner | Security | Access privileges | Language | Internal name | Description +--------+--------------------------+------------------+----------------------------------------------+------+------------+----------+-----------------+----------+-------------------+----------+------------------+------------- + public | regress_psql_df_c | void | integer, integer, cstring, internal, integer | func | immutable | safe | regress_psql_df | invoker | | c | utf8_to_iso8859 | + public | regress_psql_df_internal | bigint | regclass, text | func | volatile | safe | regress_psql_df | invoker | | internal | pg_relation_size | + public | regress_psql_df_plpgsql | void | | func | volatile | unsafe | regress_psql_df | invoker | | plpgsql | | + public | regress_psql_df_sql | void | | func | volatile | unsafe | regress_psql_df | invoker | | sql | | +(4 rows) + +ROLLBACK; -- check \sf \sf information_schema._pg_expandarray CREATE OR REPLACE FUNCTION information_schema._pg_expandarray(anyarray, OUT x anyelement, OUT n integer) diff --git a/src/test/regress/sql/psql.sql b/src/test/regress/sql/psql.sql index 2da9665a19..9fb2e1e095 100644 --- a/src/test/regress/sql/psql.sql +++ b/src/test/regress/sql/psql.sql @@ -1275,6 +1275,43 @@ reset work_mem; \do - pg_catalog.int4 \do && anyarray * +-- check \df+ +BEGIN; +CREATE ROLE regress_psql_df; +CREATE OR REPLACE FUNCTION regress_psql_df_c (integer, integer, cstring, internal, integer) + RETURNS VOID + LANGUAGE c + IMMUTABLE PARALLEL SAFE STRICT +AS '$libdir/utf8_and_iso8859', 'utf8_to_iso8859'; +CREATE OR REPLACE FUNCTION regress_psql_df_internal (regclass, text) + RETURNS bigint + LANGUAGE internal + PARALLEL SAFE STRICT +AS 'pg_relation_size'; +CREATE OR REPLACE FUNCTION regress_psql_df_sql () + RETURNS VOID +BEGIN ATOMIC + SELECT NULL; +END; +CREATE OR REPLACE FUNCTION regress_psql_df_plpgsql () + RETURNS VOID + LANGUAGE plpgsql +AS $$ +BEGIN + RETURN; +END; +$$; +ALTER FUNCTION regress_psql_df_c (integer, integer, cstring, internal, integer) + OWNER TO regress_psql_df; +ALTER FUNCTION regress_psql_df_internal (regclass, text) + OWNER TO regress_psql_df; +ALTER FUNCTION regress_psql_df_sql () + OWNER TO regress_psql_df; +ALTER FUNCTION regress_psql_df_plpgsql () + OWNER TO regress_psql_df; +\df+ regress_psql_df_* +ROLLBACK; + -- check \sf \sf information_schema._pg_expandarray \sf+ information_schema._pg_expandarray -- 2.32.1 (Apple Git-133) ^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: Remove source code display from \df+? @ 2023-01-23 02:37 Justin Pryzby <[email protected]> parent: Isaac Morland <[email protected]> 1 sibling, 1 reply; 7+ messages in thread From: Justin Pryzby @ 2023-01-23 02:37 UTC (permalink / raw) To: Isaac Morland <[email protected]>; +Cc: Tom Lane <[email protected]>; Alvaro Herrera <[email protected]>; Pavel Stehule <[email protected]>; Magnus Hagander <[email protected]>; pgsql-hackers On Sun, Jan 22, 2023 at 08:23:25PM -0500, Isaac Morland wrote: > > Were you able to test with your own github account ? > > I haven’t had a chance to try this. I must confess to being a bit confused > by the distinction between running the CI tests and doing "make check"; > ideally I would like to be able to run all the tests on my own machine > without any external resources. But at the same time I don’t pretend to > understand the full situation so I will try to use this when I get some > time. First: "make check" only runs the sql tests, and not the perl tests (including pg_upgrade) or isolation tests. check-world runs everything. One difference from running it locally is that cirrus runs tests under four OSes. Another is that it has a bunch of compilation flags and variations to help catch errors (although it's currently missing ENFORCE_REGRESSION_TEST_NAME_RESTRICTIONS, so that wouldn't have been caught). And another reason is that it runs in a "clean" environment, so (for example) it'd probably catch if you have local, uncommited changes, or if you assumed that the username is "postgres" (earlier I said that it didn't, but actually the mac task runs as "admin"). The old way of doing things was for cfbot to "inject" the cirrus.yml file and then push a branch to cirrusci to run tests; it made some sense for people to mail a patch to the list to cause cfbot to run the tests under cirrusci. The current/new way is that .cirrus.yml is in the source tree, so anyone with a github account can do that. IMO it no longer makes sense to send patches to the list "to see" if it passes tests. I encouraging those who haven't to try it. -- Justin ^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: Remove source code display from \df+? @ 2023-01-23 02:50 Isaac Morland <[email protected]> parent: Justin Pryzby <[email protected]> 0 siblings, 1 reply; 7+ messages in thread From: Isaac Morland @ 2023-01-23 02:50 UTC (permalink / raw) To: Justin Pryzby <[email protected]>; +Cc: Tom Lane <[email protected]>; Alvaro Herrera <[email protected]>; Pavel Stehule <[email protected]>; Magnus Hagander <[email protected]>; pgsql-hackers On Sun, 22 Jan 2023 at 21:37, Justin Pryzby <[email protected]> wrote: > On Sun, Jan 22, 2023 at 08:23:25PM -0500, Isaac Morland wrote: > > > Were you able to test with your own github account ? > > > > I haven’t had a chance to try this. I must confess to being a bit > confused > > by the distinction between running the CI tests and doing "make check"; > > ideally I would like to be able to run all the tests on my own machine > > without any external resources. But at the same time I don’t pretend to > > understand the full situation so I will try to use this when I get some > > time. > > First: "make check" only runs the sql tests, and not the perl tests > (including pg_upgrade) or isolation tests. check-world runs everything. > Thanks very much. I should have remembered check-world, and of course the fact that the CI tests multiple platforms. I’ll go and do some reading/re-reading; now that I’ve gone through some parts of the process I’ll probably understand more. The latest submission appears to have passed: http://cfbot.cputube.org/isaac-morland.html However, one of the jobs (Windows - Server 2019, MinGW64 - Meson) is paused and appears never to have run: https://cirrus-ci.com/task/6687014536347648 Other than that, I think this is passing the tests. ^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: Remove source code display from \df+? @ 2023-01-23 04:09 Justin Pryzby <[email protected]> parent: Isaac Morland <[email protected]> 0 siblings, 0 replies; 7+ messages in thread From: Justin Pryzby @ 2023-01-23 04:09 UTC (permalink / raw) To: Isaac Morland <[email protected]>; +Cc: Tom Lane <[email protected]>; Alvaro Herrera <[email protected]>; Pavel Stehule <[email protected]>; Magnus Hagander <[email protected]>; pgsql-hackers On Sun, Jan 22, 2023 at 09:50:29PM -0500, Isaac Morland wrote: > However, one of the jobs (Windows - Server 2019, MinGW64 - Meson) is paused > and appears never to have run: > > https://cirrus-ci.com/task/6687014536347648 Yeah, mingw is currently set to run only when manually "triggered" by the repository owner (because it's slow). There's no mechanism to tell cfbot to trigger the task, but you can do it if you run from your own github. Anyway, there's no reason to think this patch needs extra platform-specific coverage. -- Justin ^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: Remove source code display from \df+? @ 2023-03-02 22:20 Tom Lane <[email protected]> parent: Isaac Morland <[email protected]> 1 sibling, 1 reply; 7+ messages in thread From: Tom Lane @ 2023-03-02 22:20 UTC (permalink / raw) To: Isaac Morland <[email protected]>; +Cc: Justin Pryzby <[email protected]>; Alvaro Herrera <[email protected]>; Pavel Stehule <[email protected]>; Magnus Hagander <[email protected]>; pgsql-hackers Isaac Morland <[email protected]> writes: > [ 0001-Remove-source-code-display-from-df-v6.patch ] Pushed after some editorialization on the test case. One thing I noticed while testing is that if you apply \df+ to an aggregate function, it will show "Internal name" of "aggregate_dummy". While that's an accurate description of what's in prosrc, it seems not especially useful and perhaps indeed confusing to novices. So I thought about suppressing it. However, that would require a server-version-dependent test and I wasn't quite convinced it'd be worth the trouble. Any thoughts on that? regards, tom lane ^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: Remove source code display from \df+? @ 2023-03-03 04:49 Isaac Morland <[email protected]> parent: Tom Lane <[email protected]> 0 siblings, 0 replies; 7+ messages in thread From: Isaac Morland @ 2023-03-03 04:49 UTC (permalink / raw) To: Tom Lane <[email protected]>; +Cc: Justin Pryzby <[email protected]>; Alvaro Herrera <[email protected]>; Pavel Stehule <[email protected]>; Magnus Hagander <[email protected]>; pgsql-hackers On Thu, 2 Mar 2023 at 17:20, Tom Lane <[email protected]> wrote: > Isaac Morland <[email protected]> writes: > > [ 0001-Remove-source-code-display-from-df-v6.patch ] > > Pushed after some editorialization on the test case. > Thanks! One thing I noticed while testing is that if you apply \df+ to an > aggregate function, it will show "Internal name" of "aggregate_dummy". > While that's an accurate description of what's in prosrc, it seems > not especially useful and perhaps indeed confusing to novices. > So I thought about suppressing it. However, that would require > a server-version-dependent test and I wasn't quite convinced it'd > be worth the trouble. Any thoughts on that? > I think it’s OK. Right now \df+ claims that the source code for an aggregate function is “aggregate_dummy”; that’s probably more untrue than saying that its internal name is “aggregate_dummy”. There are several features of aggregate functions that are always defined the same way by the creation process; who’s to say they don’t all have a shared dummy internal name? ^ permalink raw reply [nested|flat] 7+ messages in thread
end of thread, other threads:[~2023-03-03 04:49 UTC | newest] Thread overview: 7+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2019-04-22 11:58 [PATCH 2/4] Allow TAP test to excecise tablespace. Kyotaro Horiguchi <[email protected]> 2023-01-23 01:23 Re: Remove source code display from \df+? Isaac Morland <[email protected]> 2023-01-23 02:37 ` Re: Remove source code display from \df+? Justin Pryzby <[email protected]> 2023-01-23 02:50 ` Re: Remove source code display from \df+? Isaac Morland <[email protected]> 2023-01-23 04:09 ` Re: Remove source code display from \df+? Justin Pryzby <[email protected]> 2023-03-02 22:20 ` Re: Remove source code display from \df+? Tom Lane <[email protected]> 2023-03-03 04:49 ` Re: Remove source code display from \df+? Isaac Morland <[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