> Subject: [PATCH v1 1/2] Expose f_smgr to extensions for manual implementation
From what I can see, all the md* APIs that were exposed in md.h can now
be made static in md.c. The only other references to those APIs were in
smgr.c.
> Subject: [PATCH v1 2/2] Prototype: Allow tablespaces to specify which SMGR
> they use
> -typedef uint8 SMgrId;
> +/*
> + * volatile ID of the smgr. Across various configurations IDs may vary,
> + * true identity is the name of each smgr.
> + */
> +typedef int SMgrId;
>
> -#define MaxSMgrId UINT8_MAX
> +#define MaxSMgrId INT_MAX
In a future revision of this patch, seems worthwhile to just start as
int instead of a uint8 to avoid this song and dance. Maybe int8 instead
of int?
> +static SMgrId recent_smgrid = -1;
You could use InvalidSmgrId here.
> +void smgrvalidatetspopts(const char *smgrname, List *opts)
> +{
> + SMgrId smgrid = get_smgr_by_name(smgrname, false);
> +
> + smgrsw[smgrid].smgr_validate_tspopts(opts);
> +}
> +
> +void smgrcreatetsp(const char *smgrname, Oid tsp, List *opts, bool isredo)
> +{
> + SMgrId smgrid = get_smgr_by_name(smgrname, false);
> +
> + smgrsw[smgrid].smgr_create_tsp(tsp, opts, isredo);
> +}
> +
> +void smgrdroptsp(const char *smgrname, Oid tsp, bool isredo)
> +{
> + SMgrId smgrid = get_smgr_by_name(smgrname, false);
> +
> + smgrsw[smgrid].smgr_drop_tsp(tsp, isredo);
> +}
Do you not need to check if smgrid is the InvalidSmgrId? I didn't see
any other validation anywhere.
> + char *smgr;
> + List *smgropts; /* list of DefElem nodes */
smgrname would probably work better alongside tablespacename in that
struct.
> @@ -221,7 +229,7 @@ mdexists(SMgrRelation reln, ForkNumber forknum)
> if (!InRecovery)
> mdclose(reln, forknum);
>
> - return (mdopenfork(reln, forknum, EXTENSION_RETURN_NULL) != NULL);
> + return (mdopenfork(mdreln, forknum, EXTENSION_RETURN_NULL) != NULL);
> }
Was this a victim of a bad rebase? Seems like it belongs in the previous
patch.
> +void mddroptsp(Oid tsp, bool isredo)
> +{
> +
> +}
Some functions in this file have the return type on the previous line.
This is a pretty slick patchset. Excited to read more dicussion and how
it evolves.
--
Tristan Partin
Neon (https://neon.tech)