Re: Include extension path on pg_available_extensions

Chao Li <li.evan.chao@gmail.com>

From: Chao Li <li.evan.chao@gmail.com>
To: Matheus Alcantara <matheusssilv97@gmail.com>
Cc: Pg Hackers <pgsql-hackers@postgresql.org>
Date: 2025-10-23T01:28:13Z
Lists: pgsql-hackers

> On 9/16/25 8:18 AM, Matheus Alcantara wrote:
> 
>> Any opinions on this?
>> [1] https://www.postgresql.org/message-id/CAKFQuwbR1Fzr8yRuMW%3DN1UMA1cTpFcqZe9bW_-ZF8%3DBa2Ud2%3Dw%40mail.gmail.com
> Just as the discussion here. Adding extension location is a good idea.


+1. I like the ideal.

> --
> Matheus Alcantara
> <v1-0001-Add-path-of-extension-on-pg_available_extensions.patch>

Got a few comments:

1 - extension.c
```
+/*
+ * A location configured on extension_control_path GUC.
+ *
+ * The macro is the macro plaeholder that the extension_control_path support
+ * and which is replaced by a system value that is stored on loc. For custom
+ * paths that don't have a macro the macro field is NULL.
+ */
```

Some problems in the comment:

* typo: plaebholder -> placeholder
* "the extension_control_path support” where “support” should be “supports”
* “stored on loc” should be “stored in loc”

Also, “The macro is the macro placeholder …” sounds redundant, suggested revision: “The macro field stores the name of a macro (for example “$system”) that  extension_control_path supports, which is replaced by …"

2 - extension.c
```
+		Location   *location = palloc0_object(Location);
+
+		location->macro = NULL;
+		location->loc = system_dir;
+		paths = lappend(paths, location);
```

As you immediately assign values to all fields, palloc0_object() is not needed, palloc_object() is good enough.

3 - extension.c
```
@@ -366,6 +384,7 @@ get_extension_control_directories(void)
 			int			len;
 			char	   *mangled;
 			char	   *piece = first_path_var_separator(ecp);
+			Location   *location = palloc0_object(Location);
```

In all execution paths, location will be initiated, thus palloc_object() is good enough.

4 - extension.c
```
+				/* location */
+				if (location->macro == NULL)
+					values[3] = CStringGetTextDatum(location->loc);
+				else
+					values[3] = CStringGetTextDatum(location->macro);
```

There are multiple places of this “if-else”. So, “macro” basically is for display, and loc is the real location. I am thinking, maybe we can change the definition of Location to:

```
structure Location {
  Char *display;
  Char *real;
```

When it is not a macro, just assign real to display, so that we can avoid all these “if-else”.

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/