Re: Include extension path on pg_available_extensions

From: Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com>
To: Matheus Alcantara <matheusssilv97(at)gmail(dot)com>
Cc: Pg Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Include extension path on pg_available_extensions
Date: 2025-10-23 01:28:13
Message-ID: 0DDF33A5-4F8C-4725-A124-B301147FA1FA@gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
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/

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2025-10-23 01:49:19 Memory leak due to thinko in PL/Python error handling
Previous Message Michael Paquier 2025-10-23 00:35:04 Re: Extended Statistics set/restore/clear functions.