KaiGai Kohei wrote:
> (1) Who/Where should allocate a string area?
>
> + /* Note that this assumes that the variable is already allocated! */
> + #define HANDLE_STRING_RELOPTION(optname, var, option) \
> + if (HAVE_RELOPTION(optname, option)) \
> + { \
> + strcpy(var, \
> + option.isset ? option.values.string_val : \
> + ((relopt_string *) option.gen)->default_val); \
> + continue; \
> + }
>
> I think HANDLE_STRING_RELOPTION() should allocate a string area via
> pstrdup(). If we have to put individual pstrdup() for each reloptions,
> it will make noisy listing of codes.
>
> How do you think:
>
> #define HANDLE_STRING_RELOPTION(optname, var, option) \
> if (HAVE_RELOPTION(optname, option)) \
> { \
> char *tmp = (option.isset ? option.values.string_val : \
> ((relopt_string *) option.gen)->default_val); \
> var = pstrdup(tmp); \
> continue; \
> }
Well, that's precisely the problem with string options. If we want
memory to be freed properly, we can only allocate a single chunk which
is what's going to be stored under the rd_options bytea pointer.
Allocating separately doesn't work because we need to rebuild the
relcache entry (freeing it and allocating a new one) when it is
invalidated for whatever reason. Since the relcache code cannot follow
a pointer stored in the bytea area, this would result in a permanent
memory leak.
So the rule I came up with is that the caller is responsible for
allocating it -- but it must be inside the bytea area to be returned.
Below is a sample amoptions routine to show how it works. Note that
this is exactly why I said that only a single string option can be
supported.
If you have a better idea, I'm all ears.
> (2) How does it represent NULL in string_option?
>
> It seems to me we cannot represent a NULL string in the default.
> Is it possible to add a mark to indicate NULL, like "bool default_null"
> within struct relopt_string?
Ah, good point. I'll have a look at this.
> (3) heap_reloptions() from RelationParseRelOptions() makes a trouble.
This is the same as (1) actually.
--
Alvaro Herrera http://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.
/* must follow StdRdOptions conventions */
typedef struct BtOptions
{
int32 vl_len_;
int fillfactor;
char teststring[1];
} BtOptions;
Datum
btoptions(PG_FUNCTION_ARGS)
{
Datum reloptions = PG_GETARG_DATUM(0);
bool validate = PG_GETARG_BOOL(1);
bytea *result;
relopt_value *options;
int numoptions;
int i;
static bool initialized = false;
if (!initialized)
{
add_string_reloption(RELOPT_KIND_BTREE, "teststring", NULL,
"helluva string here
and there!");
initialized = true;
}
options = parseRelOptions(reloptions, validate, RELOPT_KIND_BTREE,
&numoptions);
/* if none set, we're done */
if (numoptions == 0)
result = NULL;
else
{
BtOptions *rdopts;
int tstrlen;
for (i = 0; i < numoptions; i++)
{
if (HAVE_RELOPTION("teststring", options[i]))
{
tstrlen = options[i].isset ?
strlen(options[i].values.string_val) :
((relopt_string *)
options[i].gen)->default_len;
break;
}
}
rdopts = palloc0(sizeof(BtOptions) + tstrlen + 1);
for (i = 0; i < numoptions; i++)
{
HANDLE_INT_RELOPTION("fillfactor", rdopts->fillfactor,
options[i]);
HANDLE_STRING_RELOPTION("teststring",
rdopts->teststring, options[i]);
}
pfree(options);
SET_VARSIZE(rdopts, sizeof(BtOptions) + tstrlen + 1);
result = (bytea *) rdopts;
}
if (result)
PG_RETURN_BYTEA_P(result);
PG_RETURN_NULL();
}
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
