Re: [HACKERS] Configuration patch

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: pgsql(at)mohawksoft(dot)com
Cc: "Bruce Momjian" <pgman(at)candle(dot)pha(dot)pa(dot)us>, "PostgreSQL-patches" <pgsql-patches(at)postgresql(dot)org>
Subject: Re: [HACKERS] Configuration patch
Date: 2004-06-03 18:19:45
Message-ID: 8810.1086286785@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers pgsql-patches

pgsql(at)mohawksoft(dot)com writes:
>> AFAICS this patch breaks standalone backends, since the smarts involved
>> in dealing with the new flavors of directory layouts were not taught to
>> postgres.c.

> Hmm, I guess its time to get a CVS version of PG. This was originally done
> in 7.3 and migrated to 7.4. 7.5 is substantially different?

The same issue applied in 7.4 and before; but yes, all that code looks
noticeably different in CVS tip.

> As for include not being a variable, no it isn't. It is a new class of GUC
> parameter. Would you like a better syntax?

As I said, I think this "class of GUC parameter" is ill-considered.

> FWIW: This is exactly the same syntax that was discussed, and no one
> brought up that it was a problem. You even said you liked the idea of
> "include."

I like the idea of include. I don't like this implementation.

> I wanted to create a generic facility for "smarter" configuration. Being
> able to create functions and pass parameters should allow smaller more
> focused configuration parsing.

I don't believe "include" is a representative of a class; it seems a
specialized one-of-a-kind thing. If you had one or two more examples
of this class then I might think you are onto something, but as-is
there is no reason to think that you've got a well-defined idea or
have made the right decisions about how it should work.

Basically, include is not a variable because neither "set include" nor
"show include" are sensible operations at all. Implementing it in a way
that makes you have to deal with these possibilities is simply
wrongheaded.

Another reason why it's not a variable is that processing it as a
variable means the sub-file is not read in the order that the user would
naturally expect; with the implementation as you have it, the behavior
would be quite surprising as to which file wins if both outer file and
subfile set the same variable. (Take another look at guc-file.l: it
eats the whole file and only then starts to evaluate variables.)

What I'd think is reasonable is a special case hack in guc-file.l that
checks for opt_name = "include" and does something immediately upon
seeing it. (CVS tip has a special hack for "custom_variable_classes"
here, which has got problems of its own because that *is* a variable,
but that's right around where I'd envision inserting code for include.)

> I'm open to suggestions. Would you prefer stating the function parameters
> with a special character? '#' is taken, how about '!' ? is in:

It's not the syntax I'm objecting to; it's the implementation.

regards, tom lane

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message pgsql 2004-06-03 19:14:32 Re: [HACKERS] Configuration patch
Previous Message pgsql 2004-06-03 17:46:09 Re: [HACKERS] Configuration patch

Browse pgsql-patches by date

  From Date Subject
Next Message pgsql 2004-06-03 19:14:32 Re: [HACKERS] Configuration patch
Previous Message Tom Lane 2004-06-03 17:58:54 Re: Add error-checking to timestamp_recv