public inbox for [email protected]  

help / color / mirror / Atom feed

From: Chao Li <[email protected]>
To: Matheus Alcantara <[email protected]>
Cc: Pg Hackers <[email protected]>
Subject: Re: Include extension path on pg_available_extensions
Date: Thu, 23 Oct 2025 09:28:13 +0800
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>



> 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%40ma...
> 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/

view thread (23+ 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]
  Subject: Re: Include extension path on pg_available_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