pgsql/src/backend commands/variable.c parser/g ...

thomas@postgresql.org (Thomas Lockhart) writes:

Log message:
Remove the definition for set_name_needs_quotes() on the assumption that
it is now obsolete. Need some regression test cases to prove otherwise...

I agree that we don't want to reinstate that hack on the gram.y side.
However, it seems to me way past time that we did what needs to be done
with variable.c --- ie, get rid of it. All these special-cased
variables should be folded into GUC.

The code as committed has some problems beyond having broken support
for search_path with a list:

regression=# set seed to 1,2;
server closed the connection unexpectedly

(crash is due to assert failure)

but really there's no point in worrying about that one case. What we
need to do is figure out what needs to be done to GUC to let it support
these variables, and then merge the variable.c code into that structure.

Should we allow GUC stuff to take a list of A_Const as being the most
general case, or is that overkill?

regards, tom lane

I agree that we don't want to reinstate that hack on the gram.y side.
However, it seems to me way past time that we did what needs to be done
with variable.c --- ie, get rid of it. All these special-cased
variables should be folded into GUC.

Or in some cases into pg_database? We might want some of this to travel
as database-specific properties adjustable using SQL or SET syntax.

The code as committed has some problems beyond having broken support
for search_path with a list:
regression=# set seed to 1,2;
server closed the connection unexpectedly

OK. Would be nice to have a regression test covering this. And this is
quite easy to fix of course.

but really there's no point in worrying about that one case. What we
need to do is figure out what needs to be done to GUC to let it support
these variables, and then merge the variable.c code into that structure.
Should we allow GUC stuff to take a list of A_Const as being the most
general case, or is that overkill?

Not sure. Peter would like to change the SET DATESTYLE support if I
remember correctly. But I've gotten the impression, perhaps wrongly,
that this extends to changing features in dates and times beyond style
settings. If it is just the two-dimensional nature of the datestyle
parameters (euro vs non-euro, and output format) then I'm sure that some
other reasonable syntax could be arranged. I'm not sure what he would
recommend wrt GUC in just the context of general capabilities for
specifying parameters.

- Thomas

Thomas Lockhart <thomas@fourpalms.org> writes:

However, it seems to me way past time that we did what needs to be done
with variable.c --- ie, get rid of it. All these special-cased
variables should be folded into GUC.

Or in some cases into pg_database? We might want some of this to travel
as database-specific properties adjustable using SQL or SET syntax.

Ah, but we *have* that ability right now; see Peter's recent changes
to support per-database and per-user GUC settings. The functionality
available for handling GUC-ified variables is now so far superior to
plain SET that it's really foolish to consider having any parameters
that are outside GUC control.

regards, tom lane

...

Ah, but we *have* that ability right now; see Peter's recent changes
to support per-database and per-user GUC settings. The functionality
available for handling GUC-ified variables is now so far superior to
plain SET that it's really foolish to consider having any parameters
that are outside GUC control.

istm that with the recent discussion of transaction-fying SET variables
that table-fying some settable parameters may be appropriate. Leave out
the "foolish" from the discussion please ;)

- Thomas

...

regression=# set seed to 1,2;
server closed the connection unexpectedly
(crash is due to assert failure)

Now that I look, the assert is one I put in explicitly to catch multiple
arguments! I wasn't sure what the behavior *should* be, though I could
have done worse than simply checking for multiple arguments and throwing
a more graceful elog(ERROR) with a message about having too many
arguments to SET SEED...

- Thomas

Hmm. In looking at SET, why couldn't we develop this as an extendable
capability a la pg_proc? If PostgreSQL knew how to link up the set
keyword with a call to a subroutine, then we could go ahead and call
that routine generically, right? Do the proposals on the table call for
this kind of implementation, or are they all "extra-tabular"?

We could make this extensible by defining a separate table, or by
defining a convention for pg_proc as we do under different circumstances
with type coersion.

The side effects of the calls would still need some protection to be
rolled back on transaction abort.

Comments?

- Thomas

Thomas Lockhart <thomas@fourpalms.org> writes:

Hmm. In looking at SET, why couldn't we develop this as an extendable
capability a la pg_proc?

Well, my thoughts were along the line of providing specialized parsing
subroutines tied to specific GUC variables. There already are
parse_hook and assign_hook concepts in GUC, but possibly they need a
little more generalization to cover what these variables need to do.

If you're suggesting setting up an actual database table, I'm not
sure I see the point. Any system parameter is going to have to be
tied to backend code that knows what to do with the parameter, so
it's not like you can expect to do anything useful purely by adding
table entries. The C-code tables existing inside guc.c seem like
enough flexibility to me.

regards, tom lane

Peter Eisentraut <peter_e@gmx.net> writes:

The only thing that I had suggested on occasion was that if nontrivial
work were to be put into SET DATESTYLE, we might want to consider if a
certain amount of "cleanup" could be done at the same time. For example,
the particular date styles have somewhat unfortunate names, as does the
"european" option. And the parameter could be separated into two. One
doesn't have to agree with these suggestions, but without them the work is
sufficiently complicated that no one has gotten around to it yet.

I think you were mainly concerned that we not define two interacting
GUC variables (ie, setting one could have side-effects on the other)?

I don't see any inherent reason that DATESTYLE couldn't be imported into
GUC as-is. The semantics might be uglier than you'd like, but why would
they be any worse than they are now?

regards, tom lane

Thomas Lockhart writes:

Not sure. Peter would like to change the SET DATESTYLE support if I
remember correctly. But I've gotten the impression, perhaps wrongly,
that this extends to changing features in dates and times beyond style
settings. If it is just the two-dimensional nature of the datestyle
parameters (euro vs non-euro, and output format) then I'm sure that some
other reasonable syntax could be arranged. I'm not sure what he would
recommend wrt GUC in just the context of general capabilities for
specifying parameters.

The only thing that I had suggested on occasion was that if nontrivial
work were to be put into SET DATESTYLE, we might want to consider if a
certain amount of "cleanup" could be done at the same time. For example,
the particular date styles have somewhat unfortunate names, as does the
"european" option. And the parameter could be separated into two. One
doesn't have to agree with these suggestions, but without them the work is
sufficiently complicated that no one has gotten around to it yet.

--
Peter Eisentraut peter_e@gmx.net

...

If you're suggesting setting up an actual database table, I'm not
sure I see the point. Any system parameter is going to have to be
tied to backend code that knows what to do with the parameter, so
it's not like you can expect to do anything useful purely by adding
table entries. The C-code tables existing inside guc.c seem like
enough flexibility to me.

Ah, certainly not! (which is as close as I'll come to saying "how
foolish!" :)

You've done work on generalizing the extensibility features of
PostgreSQL. A next step to take with that is to allow for a more generic
"package" capability, where packages can be defined, and can have some
initialization code run at the time the database starts up. This would
allow packages to have their own internal state as extensions to the
internal state of the core package.

Having SET be extensible is another piece to the puzzle, allowing these
kinds of parameters to also be extensible. I'm not sure that this should
be considered a part of the GUC design (the parameters are designed to
be available *outside* the database itself, to allow startup issues to
be avoided, right?) but perhaps GUC should be considered a subset of the
actual SET feature set.

I got the strong feeling that Hiroshi was concerned that we were
intending to lump all SET features into a single one-size-fits-all
framework. This may be the flip side of it; just because we like SET to
be used in lots of places doesn't mean we should always limit it to
things built in to the core. And we should be wary of forcing all things
"SET" to behave with transactional properties if that doesn't make
sense. I've always been comfortable with the concept of "out of band"
behavior, which I think is reflected, for example, with DDL vs DML
aspects of the SQL language. Current SET behavior aside (where the
parser is rejecting SET commands out of hand after errors within a
transaction) we should put as few *designed in* restrictions on SET as
possible, at least until we are willing to introduce a richer set of
commands (that is, something in addition to SET) as alternatives.

all imho of course :)

- Thomas