| From: | Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com> |
|---|---|
| To: | Álvaro Herrera <alvherre(at)kurilemu(dot)de> |
| Cc: | Manni Wood <manni(dot)wood(at)enterprisedb(dot)com>, Nishant Sharma <nishant(dot)sharma(at)enterprisedb(dot)com>, Jim Jones <jim(dot)jones(at)uni-muenster(dot)de>, pgsql-hackers(at)postgresql(dot)org |
| Subject: | Re: [PATCH] Add pg_get_tablespace_ddl() function to reconstruct CREATE TABLESPACE statement |
| Date: | 2025-11-13 06:06:44 |
| Message-ID: | 7DF2334B-1451-4C65-A04C-3C306719BB46@gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
Hi Manni,
I just reviewed and tested the patch, just got a few small comments:
> On Nov 13, 2025, at 00:15, Álvaro Herrera <alvherre(at)kurilemu(dot)de> wrote:
>
> On 2025-Nov-12, Manni Wood wrote:
>
>> Thanks, Álvaro, for your continued help with this.
>>
>> I have attached v11 patches that use all of the fixes from your
>> review.patch.txt.
>
> OK, thanks, I pushed 0001 now.
>
> I think you could claim that some routines currently in
> src/backend/commands/tablespace.c logically belong in the new file, but
> unless you want to take on the task of moving a lot of other routines
> under commands/ to their respective catalog/ file, then I think it's
> more or less fine as is.
>
> To be clear, I do not intend to do anything with your 0002 patch [for
> now]. I'm going to let Andrew take these DDL-producing functions in his
> hands. Here I'm just posting your 0002 again, to make the cfbot happy.
>
> Thanks
>
> --
> Álvaro Herrera 48°01'N 7°57'E — https://www.EnterpriseDB.com/
> "Nunca se desea ardientemente lo que solo se desea por razón" (F. Alexandre)
> <v12-0002-Adds-pg_get_tablespace_ddl-function.patch>
1.
```
+ errmsg("tablespace with oid %d does not exist",
```
Existing code all use “%u” to format oid. You may search for “oid %” to see that.
2.
```
+ /* Add the options in WITH clause */
+ appendStringInfoString(&buf, TextDatumGetCString(optdatums[i]));
+ appendStringInfoString(&buf, ", “);
```
This two statements can be combined into one:
appendStringInfoString(&buf, “%s, “, TextDatumGetCString(optdatums[i]));
3
```
+ if (strncmp(PG_TBLSPC_DIR_SLASH, path, strlen(PG_TBLSPC_DIR_SLASH)) == 0)
+ appendStringInfoString(&buf, " LOCATION ''");
+ else
+ appendStringInfo(&buf, " LOCATION '%s'", path);
+ }
```
Instead of hardcoding single-quotes, we can use quote_literal_cstr().
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Shlok Kyal | 2025-11-13 06:08:17 | Re: Skipping schema changes in publication |
| Previous Message | Zhijie Hou (Fujitsu) | 2025-11-13 06:01:29 | Few untranslated error messages in OAuth |