public inbox for [email protected]  
help / color / mirror / Atom feed
[PATCH 1/2] Move tablespace cleanup out of pg_regress.
6+ messages / 4 participants
[nested] [flat]

* [PATCH 1/2] Move tablespace cleanup out of pg_regress.
@ 2020-04-30 05:06 Kyotaro Horiguchi <[email protected]>
  0 siblings, 0 replies; 6+ messages in thread

From: Kyotaro Horiguchi @ 2020-04-30 05:06 UTC (permalink / raw)

Tablespace directory is cleaned-up in regress/GNUmakefile for all
platforms other than Windows. For Windoiws pg_regress does that on
behalf of make, which is ugly. In addition to that, pg_regress does
the clean up twice at once. This patch moves the cleanup code out of
pg_regress into vcregress.pl, which is more sutable place.
---
 src/test/regress/pg_regress.c | 22 ----------------------
 src/tools/msvc/vcregress.pl   | 14 ++++++++++++++
 2 files changed, 14 insertions(+), 22 deletions(-)

diff --git a/src/test/regress/pg_regress.c b/src/test/regress/pg_regress.c
index 38b2b1e8e1..c56bfaf7f5 100644
--- a/src/test/regress/pg_regress.c
+++ b/src/test/regress/pg_regress.c
@@ -491,28 +491,6 @@ convert_sourcefiles_in(const char *source_subdir, const char *dest_dir, const ch
 
 	snprintf(testtablespace, MAXPGPATH, "%s/testtablespace", outputdir);
 
-#ifdef WIN32
-
-	/*
-	 * On Windows only, clean out the test tablespace dir, or create it if it
-	 * doesn't exist.  On other platforms we expect the Makefile to take care
-	 * of that.  (We don't migrate that functionality in here because it'd be
-	 * harder to cope with platform-specific issues such as SELinux.)
-	 *
-	 * XXX it would be better if pg_regress.c had nothing at all to do with
-	 * testtablespace, and this were handled by a .BAT file or similar on
-	 * Windows.  See pgsql-hackers discussion of 2008-01-18.
-	 */
-	if (directory_exists(testtablespace))
-		if (!rmtree(testtablespace, true))
-		{
-			fprintf(stderr, _("\n%s: could not remove test tablespace \"%s\"\n"),
-					progname, testtablespace);
-			exit(2);
-		}
-	make_directory(testtablespace);
-#endif
-
 	/* finally loop on each file and do the replacement */
 	for (name = names; *name; name++)
 	{
diff --git a/src/tools/msvc/vcregress.pl b/src/tools/msvc/vcregress.pl
index f95f7a5c7a..74d37a31de 100644
--- a/src/tools/msvc/vcregress.pl
+++ b/src/tools/msvc/vcregress.pl
@@ -104,6 +104,7 @@ exit 0;
 sub installcheck_internal
 {
 	my ($schedule, @EXTRA_REGRESS_OPTS) = @_;
+
 	my @args = (
 		"../../../$Config/pg_regress/pg_regress",
 		"--dlpath=.",
@@ -114,6 +115,7 @@ sub installcheck_internal
 		"--no-locale");
 	push(@args, $maxconn) if $maxconn;
 	push(@args, @EXTRA_REGRESS_OPTS);
+	CleanupTablespaceDirectory();
 	system(@args);
 	my $status = $? >> 8;
 	exit $status if $status;
@@ -143,6 +145,7 @@ sub check
 		"--temp-instance=./tmp_check");
 	push(@args, $maxconn)     if $maxconn;
 	push(@args, $temp_config) if $temp_config;
+	CleanupTablespaceDirectory();
 	system(@args);
 	my $status = $? >> 8;
 	exit $status if $status;
@@ -178,6 +181,7 @@ sub ecpgcheck
 sub isolationcheck
 {
 	chdir "../isolation";
+	CleanupTablespaceDirectory();
 	copy("../../../$Config/isolationtester/isolationtester.exe",
 		"../../../$Config/pg_isolation_regress");
 	my @args = (
@@ -382,6 +386,7 @@ sub plcheck
 			"$topdir/$Config/pg_regress/pg_regress",
 			"--bindir=$topdir/$Config/psql",
 			"--dbname=pl_regression", @lang_args, @tests);
+		CleanupTablespaceDirectory();
 		system(@args);
 		my $status = $? >> 8;
 		exit $status if $status;
@@ -439,6 +444,7 @@ sub subdircheck
 
 	print "============================================================\n";
 	print "Checking $module\n";
+	CleanupTablespaceDirectory();
 	my @args = (
 		"$topdir/$Config/pg_regress/pg_regress",
 		"--bindir=${topdir}/${Config}/psql",
@@ -741,6 +747,14 @@ sub InstallTemp
 	return;
 }
 
+sub CleanupTablespaceDirectory
+{
+	my $tablespace = 'testtablespace';
+
+	rmtree($tablespace) if (-e $tablespace);
+	mkdir($tablespace);
+}
+
 sub usage
 {
 	print STDERR
-- 
2.18.2


----Next_Part(Mon_May_11_17_13_54_2020_626)--
Content-Type: Text/X-Patch; charset=us-ascii
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
 filename="0002-Don-t-setup-tablespace-directory-while-testing-pg_up.patch"



^ permalink  raw  reply  [nested|flat] 6+ messages in thread

* Fix CheckIndexCompatible comment
@ 2022-02-03 16:46 Yugo NAGATA <[email protected]>
  2022-02-04 00:08 ` Re: Fix CheckIndexCompatible comment Fujii Masao <[email protected]>
  0 siblings, 1 reply; 6+ messages in thread

From: Yugo NAGATA @ 2022-02-03 16:46 UTC (permalink / raw)
  To: pgsql-hackers

Hello,

I found a old parameter name 'heapRelation' in the comment
of CheckIndexCompatible. This parameter was removed by 5f173040.

Attached is a patch to remove it from the comment.

Regards,
Yugo Nagata

-- 
Yugo NAGATA <[email protected]>


Attachments:

  [text/x-diff] fix_CheckIndexCompatible_comment.patch (591B, ../../[email protected]/2-fix_CheckIndexCompatible_comment.patch)
  download | inline diff:
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index 560dcc87a2..95cb15cbb9 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -129,7 +129,6 @@ typedef struct ReindexErrorInfo
  *		prospective index definition, such that the existing index storage
  *		could become the storage of the new index, avoiding a rebuild.
  *
- * 'heapRelation': the relation the index would apply to.
  * 'accessMethodName': name of the AM to use.
  * 'attributeList': a list of IndexElem specifying columns and expressions
  *		to index on.


^ permalink  raw  reply  [nested|flat] 6+ messages in thread

* Re: Fix CheckIndexCompatible comment
  2022-02-03 16:46 Fix CheckIndexCompatible comment Yugo NAGATA <[email protected]>
@ 2022-02-04 00:08 ` Fujii Masao <[email protected]>
  2022-02-04 00:14   ` Re: Fix CheckIndexCompatible comment Nathan Bossart <[email protected]>
  2022-02-07 10:14   ` Re: Fix CheckIndexCompatible comment Yugo NAGATA <[email protected]>
  0 siblings, 2 replies; 6+ messages in thread

From: Fujii Masao @ 2022-02-04 00:08 UTC (permalink / raw)
  To: Yugo NAGATA <[email protected]>; pgsql-hackers



On 2022/02/04 1:46, Yugo NAGATA wrote:
> Hello,
> 
> I found a old parameter name 'heapRelation' in the comment
> of CheckIndexCompatible. This parameter was removed by 5f173040.
> 
> Attached is a patch to remove it from the comment.

Thanks for the report! I agree to remove the mention of parameter already dropped, from the comment. OTOH, I found CheckIndexCompatible() now has "oldId" parameter but there is no comment about it though there are comments about other parameters. Isn't it better to add the comment about "oldId"?

Regards,

-- 
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION






^ permalink  raw  reply  [nested|flat] 6+ messages in thread

* Re: Fix CheckIndexCompatible comment
  2022-02-03 16:46 Fix CheckIndexCompatible comment Yugo NAGATA <[email protected]>
  2022-02-04 00:08 ` Re: Fix CheckIndexCompatible comment Fujii Masao <[email protected]>
@ 2022-02-04 00:14   ` Nathan Bossart <[email protected]>
  1 sibling, 0 replies; 6+ messages in thread

From: Nathan Bossart @ 2022-02-04 00:14 UTC (permalink / raw)
  To: Fujii Masao <[email protected]>; +Cc: Yugo NAGATA <[email protected]>; pgsql-hackers

On Fri, Feb 04, 2022 at 09:08:22AM +0900, Fujii Masao wrote:
> On 2022/02/04 1:46, Yugo NAGATA wrote:
>> I found a old parameter name 'heapRelation' in the comment
>> of CheckIndexCompatible. This parameter was removed by 5f173040.
>> 
>> Attached is a patch to remove it from the comment.

It looks like this parameter was removed in 5f17304.
 
> Thanks for the report! I agree to remove the mention of parameter already dropped, from the comment. OTOH, I found CheckIndexCompatible() now has "oldId" parameter but there is no comment about it though there are comments about other parameters. Isn't it better to add the comment about "oldId"?

+1

-- 
Nathan Bossart
Amazon Web Services: https://aws.amazon.com






^ permalink  raw  reply  [nested|flat] 6+ messages in thread

* Re: Fix CheckIndexCompatible comment
  2022-02-03 16:46 Fix CheckIndexCompatible comment Yugo NAGATA <[email protected]>
  2022-02-04 00:08 ` Re: Fix CheckIndexCompatible comment Fujii Masao <[email protected]>
@ 2022-02-07 10:14   ` Yugo NAGATA <[email protected]>
  2022-02-18 03:22     ` Re: Fix CheckIndexCompatible comment Fujii Masao <[email protected]>
  1 sibling, 1 reply; 6+ messages in thread

From: Yugo NAGATA @ 2022-02-07 10:14 UTC (permalink / raw)
  To: Fujii Masao <[email protected]>; +Cc: pgsql-hackers

Hello, Fujii-san,

On Fri, 4 Feb 2022 09:08:22 +0900
Fujii Masao <[email protected]> wrote:

> 
> 
> On 2022/02/04 1:46, Yugo NAGATA wrote:
> > Hello,
> > 
> > I found a old parameter name 'heapRelation' in the comment
> > of CheckIndexCompatible. This parameter was removed by 5f173040.
> > 
> > Attached is a patch to remove it from the comment.
> 
> Thanks for the report! I agree to remove the mention of parameter already dropped, from the comment. OTOH, I found CheckIndexCompatible() now has "oldId" parameter but there is no comment about it though there are comments about other parameters. Isn't it better to add the comment about "oldId"?

Agreed. I updated the patch to add a comment about 'oldId'.

Regards,
Yugo Nagata

-- 
Yugo NAGATA <[email protected]>


Attachments:

  [text/x-diff] v2_fix_CheckIndexCompatible_comment.patch (638B, ../../[email protected]/2-v2_fix_CheckIndexCompatible_comment.patch)
  download | inline diff:
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index 560dcc87a2..408ffc70f6 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -129,7 +129,7 @@ typedef struct ReindexErrorInfo
  *		prospective index definition, such that the existing index storage
  *		could become the storage of the new index, avoiding a rebuild.
  *
- * 'heapRelation': the relation the index would apply to.
+ * 'oldId': the existing index's relation OID
  * 'accessMethodName': name of the AM to use.
  * 'attributeList': a list of IndexElem specifying columns and expressions
  *		to index on.


^ permalink  raw  reply  [nested|flat] 6+ messages in thread

* Re: Fix CheckIndexCompatible comment
  2022-02-03 16:46 Fix CheckIndexCompatible comment Yugo NAGATA <[email protected]>
  2022-02-04 00:08 ` Re: Fix CheckIndexCompatible comment Fujii Masao <[email protected]>
  2022-02-07 10:14   ` Re: Fix CheckIndexCompatible comment Yugo NAGATA <[email protected]>
@ 2022-02-18 03:22     ` Fujii Masao <[email protected]>
  0 siblings, 0 replies; 6+ messages in thread

From: Fujii Masao @ 2022-02-18 03:22 UTC (permalink / raw)
  To: Yugo NAGATA <[email protected]>; +Cc: pgsql-hackers



On 2022/02/07 19:14, Yugo NAGATA wrote:
> Agreed. I updated the patch to add a comment about 'oldId'.

Thanks for updating the patch! I slightly modified the patch and pushed it.

Regards,

-- 
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION






^ permalink  raw  reply  [nested|flat] 6+ messages in thread


end of thread, other threads:[~2022-02-18 03:22 UTC | newest]

Thread overview: 6+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2020-04-30 05:06 [PATCH 1/2] Move tablespace cleanup out of pg_regress. Kyotaro Horiguchi <[email protected]>
2022-02-03 16:46 Fix CheckIndexCompatible comment Yugo NAGATA <[email protected]>
2022-02-04 00:08 ` Re: Fix CheckIndexCompatible comment Fujii Masao <[email protected]>
2022-02-04 00:14   ` Re: Fix CheckIndexCompatible comment Nathan Bossart <[email protected]>
2022-02-07 10:14   ` Re: Fix CheckIndexCompatible comment Yugo NAGATA <[email protected]>
2022-02-18 03:22     ` Re: Fix CheckIndexCompatible comment Fujii Masao <[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