> 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/
