Skip site navigation (1) Skip section navigation (2)

Re: generic reloptions improvement

From: Alvaro Herrera <alvherre(at)commandprompt(dot)com>
To: KaiGai Kohei <kaigai(at)ak(dot)jp(dot)nec(dot)com>
Cc: KaiGai Kohei <kaigai(at)kaigai(dot)gr(dot)jp>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Peter Eisentraut <peter_e(at)gmx(dot)net>, pgsql-hackers(at)postgresql(dot)org, ITAGAKI Takahiro <itagaki(dot)takahiro(at)oss(dot)ntt(dot)co(dot)jp>
Subject: Re: generic reloptions improvement
Date: 2009-01-05 01:40:59
Message-ID: 20090105014059.GH26552@alvh.no-ip.org (view raw or flat)
Thread:
Lists: pgsql-hackers
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.

In response to

Responses

pgsql-hackers by date

Next:From: Alvaro HerreraDate: 2009-01-05 01:46:54
Subject: Re: generic reloptions improvement
Previous:From: Stephen R. van den BergDate: 2009-01-05 01:34:07
Subject: Re: Significantly larger toast tables on 8.4?

Privacy Policy | About PostgreSQL
Copyright © 1996-2014 The PostgreSQL Global Development Group