public inbox for [email protected]  
help / color / mirror / Atom feed
From: Peter Eisentraut <[email protected]>
To: Matheus Alcantara <[email protected]>
To: Andrew Dunstan <[email protected]>
Cc: Gabriele Bartolini <[email protected]>
Cc: Craig Ringer <[email protected]>
Cc: David E. Wheeler <[email protected]>
Cc: Robert Haas <[email protected]>
Cc: Alvaro Herrera <[email protected]>
Cc: PostgreSQL-development <[email protected]>
Subject: Re: RFC: Additional Directory for Extensions
Date: Thu, 6 Mar 2025 14:46:08 +0100
Message-ID: <[email protected]> (raw)
In-Reply-To: <CAFY6G8fV5FTqn7p8pxbvPYUD5+ffik-J50GChjKH2Gg7d=fRPg@mail.gmail.com>
References: <CA+TgmoaMk==vOUmWwJ5ZA-EueNqa9RQaWMEiHCF1ZKW_YCcj8g@mail.gmail.com>
	<CA+VUV5oWkMVvtNB0BG2CM8SrmOkeW6fEtXGehbniHqR9MhMzVw@mail.gmail.com>
	<CAGRY4nxP6A5Dz23g+aGD-agdVwUj_qrG6szd-mWc0E5OFMBg4w@mail.gmail.com>
	<[email protected]>
	<CAGRY4ny=Oy-F0qCRFQa=XtEqyUbMygvbqqKahAvbae7v4LTOrQ@mail.gmail.com>
	<[email protected]>
	<CAGRY4nx8Fmea4udk--PKdvfgpNdqC+rcueAmGwuw2PD9dv+zqQ@mail.gmail.com>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<CAFY6G8dzuLXFjONX5E-TvyD5E9r21b5vp1My_nMcqdGD0VYfYw@mail.gmail.com>
	<[email protected]>
	<CAFY6G8dGcv1D8tCkjrz+9DvAYG9h4U=hBm0VXjJEXExy44hauQ@mail.gmail.com>
	<CAFY6G8eOMkGXLqzUz=qXrwt4X_HvmVZ13570DAbgAiw+3q03DA@mail.gmail.com>
	<CAFY6G8fV5FTqn7p8pxbvPYUD5+ffik-J50GChjKH2Gg7d=fRPg@mail.gmail.com>

On 03.03.25 19:45, Matheus Alcantara wrote:
> Hi, attached a new v5 with some minor improvements on TAP tests:
> 
> - Add a proper test name for all test cases
> - Add CREATE EXTENSION command execution
> - Change the assert on pg_available_extensions and
>    pg_available_extension_versions to validate the row content
> 
> Also rebased with master

This looks very good to me.  I have one issue to point out:  The logic 
in get_extension_control_directories() needs to be a little bit more 
careful to align with the rules in find_in_path().  For example, it 
should use first_path_var_separator() to get the platform-specific path 
separator, and probably also substitute_path_macro() and 
canonicalize_path() etc., to keep everything consistent.  (Maybe it 
would be ok to move the function to dfmgr.c to avoid having to export 
too many things from there.)

Independent of that, attached is a small patch that suggests to use the 
newer foreach_ptr() macro in some places.
From 35c2106558095e74359cec58b9631fa19ed937b3 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <[email protected]>
Date: Thu, 6 Mar 2025 14:28:08 +0100
Subject: [PATCH] Use foreach_ptr

---
 src/backend/commands/extension.c | 20 ++++++--------------
 1 file changed, 6 insertions(+), 14 deletions(-)

diff --git a/src/backend/commands/extension.c b/src/backend/commands/extension.c
index 7e8a28e4064..4bdb20aaf54 100644
--- a/src/backend/commands/extension.c
+++ b/src/backend/commands/extension.c
@@ -51,10 +51,10 @@
 #include "commands/defrem.h"
 #include "commands/extension.h"
 #include "commands/schemacmds.h"
-#include "nodes/pg_list.h"
 #include "funcapi.h"
 #include "mb/pg_wchar.h"
 #include "miscadmin.h"
+#include "nodes/pg_list.h"
 #include "nodes/queryjumble.h"
 #include "storage/fd.h"
 #include "tcop/utility.h"
@@ -2183,20 +2183,17 @@ Datum
 pg_available_extensions(PG_FUNCTION_ARGS)
 {
 	ReturnSetInfo *rsinfo = (ReturnSetInfo *) fcinfo->resultinfo;
+	List	   *locations;
 	DIR		   *dir;
 	struct dirent *de;
-	List	   *locations;
-	ListCell   *cell;
 
 	/* Build tuplestore to hold the result rows */
 	InitMaterializedSRF(fcinfo, 0);
 
 	locations = get_extension_control_directories();
 
-	foreach(cell, locations)
+	foreach_ptr(char, location, locations)
 	{
-		char	   *location = (char *) lfirst(cell);
-
 		dir = AllocateDir(location);
 
 		/*
@@ -2271,7 +2268,6 @@ pg_available_extension_versions(PG_FUNCTION_ARGS)
 {
 	ReturnSetInfo *rsinfo = (ReturnSetInfo *) fcinfo->resultinfo;
 	List	   *locations;
-	ListCell   *cell;
 	DIR		   *dir;
 	struct dirent *de;
 
@@ -2279,10 +2275,9 @@ pg_available_extension_versions(PG_FUNCTION_ARGS)
 	InitMaterializedSRF(fcinfo, 0);
 
 	locations = get_extension_control_directories();
-	foreach(cell, locations)
-	{
-		char	   *location = (char *) lfirst(cell);
 
+	foreach_ptr(char, location, locations)
+	{
 		dir = AllocateDir(location);
 
 		/*
@@ -2449,16 +2444,13 @@ extension_file_exists(const char *extensionName)
 {
 	bool		result = false;
 	List	   *locations;
-	ListCell   *cell;
 	DIR		   *dir;
 	struct dirent *de;
 
 	locations = get_extension_control_directories();
 
-	foreach(cell, locations)
+	foreach_ptr(char, location, locations)
 	{
-		char	   *location = (char *) lfirst(cell);
-
 		dir = AllocateDir(location);
 
 		/*
-- 
2.48.1



Attachments:

  [text/plain] 0001-Use-foreach_ptr.patch.nocfbot (2.3K, ../[email protected]/2-0001-Use-foreach_ptr.patch.nocfbot)
  download | inline diff:
From 35c2106558095e74359cec58b9631fa19ed937b3 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <[email protected]>
Date: Thu, 6 Mar 2025 14:28:08 +0100
Subject: [PATCH] Use foreach_ptr

---
 src/backend/commands/extension.c | 20 ++++++--------------
 1 file changed, 6 insertions(+), 14 deletions(-)

diff --git a/src/backend/commands/extension.c b/src/backend/commands/extension.c
index 7e8a28e4064..4bdb20aaf54 100644
--- a/src/backend/commands/extension.c
+++ b/src/backend/commands/extension.c
@@ -51,10 +51,10 @@
 #include "commands/defrem.h"
 #include "commands/extension.h"
 #include "commands/schemacmds.h"
-#include "nodes/pg_list.h"
 #include "funcapi.h"
 #include "mb/pg_wchar.h"
 #include "miscadmin.h"
+#include "nodes/pg_list.h"
 #include "nodes/queryjumble.h"
 #include "storage/fd.h"
 #include "tcop/utility.h"
@@ -2183,20 +2183,17 @@ Datum
 pg_available_extensions(PG_FUNCTION_ARGS)
 {
 	ReturnSetInfo *rsinfo = (ReturnSetInfo *) fcinfo->resultinfo;
+	List	   *locations;
 	DIR		   *dir;
 	struct dirent *de;
-	List	   *locations;
-	ListCell   *cell;
 
 	/* Build tuplestore to hold the result rows */
 	InitMaterializedSRF(fcinfo, 0);
 
 	locations = get_extension_control_directories();
 
-	foreach(cell, locations)
+	foreach_ptr(char, location, locations)
 	{
-		char	   *location = (char *) lfirst(cell);
-
 		dir = AllocateDir(location);
 
 		/*
@@ -2271,7 +2268,6 @@ pg_available_extension_versions(PG_FUNCTION_ARGS)
 {
 	ReturnSetInfo *rsinfo = (ReturnSetInfo *) fcinfo->resultinfo;
 	List	   *locations;
-	ListCell   *cell;
 	DIR		   *dir;
 	struct dirent *de;
 
@@ -2279,10 +2275,9 @@ pg_available_extension_versions(PG_FUNCTION_ARGS)
 	InitMaterializedSRF(fcinfo, 0);
 
 	locations = get_extension_control_directories();
-	foreach(cell, locations)
-	{
-		char	   *location = (char *) lfirst(cell);
 
+	foreach_ptr(char, location, locations)
+	{
 		dir = AllocateDir(location);
 
 		/*
@@ -2449,16 +2444,13 @@ extension_file_exists(const char *extensionName)
 {
 	bool		result = false;
 	List	   *locations;
-	ListCell   *cell;
 	DIR		   *dir;
 	struct dirent *de;
 
 	locations = get_extension_control_directories();
 
-	foreach(cell, locations)
+	foreach_ptr(char, location, locations)
 	{
-		char	   *location = (char *) lfirst(cell);
-
 		dir = AllocateDir(location);
 
 		/*
-- 
2.48.1



view thread (44+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
  Subject: Re: RFC: Additional Directory for Extensions
  In-Reply-To: <[email protected]>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox