Hi,

I'm getting this error when I set lot_statement_stats to on :

FATAL: invalid value for parameter "log_statement_stats": 1

and server does not start. This is "almost" default configuration file
(8.2.5, Fedora 8, running on my laptop):

http://www.gunduz.org/tmp/postgresql.conf

Per Alexey (and Alvaro), both log_planner_stats and log_statement_stats
cannot be turned on at the same time. Is that documented somewhere, or
can we please improve the error message here -- or is it a bug?

Thoughts?

Regards,
--
Devrim GÜNDÜZ , RHCE
PostgreSQL Replication, Consulting, Custom Development, 24x7 support
Managed Services, Shared and Dedicated Hosting
Co-Authors: plPHP, ODBCng - http://www.commandprompt.com/

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.7 (GNU/Linux)

iD8DBQBHco9bpme12CBXnxERAnxPAJ0aekQiEbeY65OYkvQ4Io AhXY83JQCfX5AV
OTASXYIWbmj4tSicqghtNNY=
=NEQt
-----END PGP SIGNATURE-----

Devrim =?ISO-8859-1?Q?G=DCND=DCZ?= <devrim@CommandPrompt.com> writes:
> I'm getting this error when I set lot_statement_stats to on :
> FATAL: invalid value for parameter "log_statement_stats": 1

> Per Alexey (and Alvaro), both log_planner_stats and log_statement_stats
> cannot be turned on at the same time.

Yup:

regression=# set log_planner_stats TO 1;
SET
regression=# set log_statement_stats TO 1;
ERROR: cannot enable "log_statement_stats" when "log_parser_stats", "log_planner_stats", or "log_executor_stats" is true

> Is that documented somewhere,

Yes.

> or can we please improve the error message here -- or is it a bug?

It's not a bug. However, the specific error message only comes out in
interactive-SET cases:

if (source >= PGC_S_INTERACTIVE)
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
errmsg("cannot enable \"log_statement_stats\" when "
"\"log_parser_stats\", \"log_planner_stats\", "
"or \"log_executor_stats\" is true")));

There are a bunch of other GUC assign hooks that behave similarly.
Perhaps it'd be sensible to emit these complaints as LOG messages
when we're dealing with a noninteractive source (ie, the config file)?

regards, tom lane

---------------------------(end of broadcast)---------------------------
TIP 7: You can help support the PostgreSQL project by donating at

http://www.postgresql.org/about/donate

Tom Lane wrote:
> Devrim =?ISO-8859-1?Q?G=DCND=DCZ?= <devrim@CommandPrompt.com> writes:

> > or can we please improve the error message here -- or is it a bug?
>
> It's not a bug. However, the specific error message only comes out in
> interactive-SET cases:
>
> if (source >= PGC_S_INTERACTIVE)
> ereport(ERROR,
> (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
> errmsg("cannot enable \"log_statement_stats\" when "
> "\"log_parser_stats\", \"log_planner_stats\", "
> "or \"log_executor_stats\" is true")));
>
> There are a bunch of other GUC assign hooks that behave similarly.
> Perhaps it'd be sensible to emit these complaints as LOG messages
> when we're dealing with a noninteractive source (ie, the config file)?

I was wondering whether we could make this error message (and
equivalent ones) be the FATAL one that prevents the server from
starting.

--
Alvaro Herrera http://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

---------------------------(end of broadcast)---------------------------
TIP 7: You can help support the PostgreSQL project by donating at

http://www.postgresql.org/about/donate

Alvaro Herrera <alvherre@CommandPrompt.com> writes:
> Tom Lane wrote:
>> There are a bunch of other GUC assign hooks that behave similarly.
>> Perhaps it'd be sensible to emit these complaints as LOG messages
>> when we're dealing with a noninteractive source (ie, the config file)?

> I was wondering whether we could make this error message (and
> equivalent ones) be the FATAL one that prevents the server from
> starting.

There is some good reason, which I don't recall at the moment, why
assign-hooks (and most of the rest of GUC) shouldn't FATAL in the
middle of processing a config file. However a LOG seems relatively
harmless, and better than not emitting anything.

regards, tom lane

---------------------------(end of broadcast)---------------------------
TIP 9: In versions below 8.0, the planner will ignore your desire to
choose an index scan if your joining column's datatypes do not
match

Tom Lane wrote:
> Alvaro Herrera <alvherre@CommandPrompt.com> writes:
> > Tom Lane wrote:
> >> There are a bunch of other GUC assign hooks that behave similarly.
> >> Perhaps it'd be sensible to emit these complaints as LOG messages
> >> when we're dealing with a noninteractive source (ie, the config file)?
>
> > I was wondering whether we could make this error message (and
> > equivalent ones) be the FATAL one that prevents the server from
> > starting.
>
> There is some good reason, which I don't recall at the moment, why
> assign-hooks (and most of the rest of GUC) shouldn't FATAL in the
> middle of processing a config file. However a LOG seems relatively
> harmless, and better than not emitting anything.

I'm thinking of keeping the ERROR, which (I think) would cause
postmaster to treat it as FATAL because there is no error handler set up
yet (no, I haven't tried it).

Perhaps you are correct in that LOG is the path of least resistance ...

--
Alvaro Herrera http://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

---------------------------(end of broadcast)---------------------------
TIP 2: Don't 'kill -9' the postmaster

Alvaro Herrera <alvherre@CommandPrompt.com> writes:
> Tom Lane wrote:
>> There is some good reason, which I don't recall at the moment, why
>> assign-hooks (and most of the rest of GUC) shouldn't FATAL in the
>> middle of processing a config file.

> I'm thinking of keeping the ERROR, which (I think) would cause
> postmaster to treat it as FATAL because there is no error handler set up
> yet (no, I haven't tried it).

Yeah, it would, and that is the wrong thing.

It is definitely not acceptable to throw an ERROR unconditionally,
as that would cause postmaster abort during SIGHUP if an error is
introduced into the config file after startup. It's possible that
we could add some state by which assign-hook routines could check
whether this is initial startup or not, but then we'd still have
the issue of whether there are any other reasons not to throw ERROR
just there (I have a feeling there are some internal-to-GUC reasons
why not, as well as the obvious one that SIGHUP shouldn't be able
to crash the postmaster).

The whole idea sounds pretty shaky to me, and definitely not
something to change in late beta. LOG we could do now without
risking breaking anything.

regards, tom lane

---------------------------(end of broadcast)---------------------------
TIP 9: In versions below 8.0, the planner will ignore your desire to
choose an index scan if your joining column's datatypes do not
match

I wrote:
> The whole idea sounds pretty shaky to me, and definitely not
> something to change in late beta. LOG we could do now without
> risking breaking anything.

Poking around a bit more, I notice that there are already some places
that do it the way I was thinking of, eg in commands/variable.c:

if (!new_tz)
{
ereport((source >= PGC_S_INTERACTIVE) ? ERROR : LOG,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
errmsg("unrecognized time zone name: \"%s\"",
value)));
return NULL;
}

However, even this is not really good enough: in the event of a wrong
entry inserted into postgresql.conf, this coding will result in every
extant backend emitting the same LOG message at SIGHUP. That's
annoying. The right way to do it is as illustrated in
set_config_option(): only the postmaster should log at LOG level,
everyone else should be down around DEBUG2 (if not lower).

That's getting to be a bit complicated to replicate in N places, though.
Plus if we ever want to make it work like Alvaro is thinking of, we'd
have to go back and change all those places again. So I propose
inventing a function

int guc_complaint_level(GucSource source)

that encapsulates this logic. The correct coding for specialized
error messages in assign-hook routines would then be like

if (!new_tz)
{
ereport(guc_complaint_level(source),
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
errmsg("unrecognized time zone name: \"%s\"",
value)));
return NULL;
}

giving us only one place to change to alter this logic.

Comments, objections?

regards, tom lane

---------------------------(end of broadcast)---------------------------
TIP 9: In versions below 8.0, the planner will ignore your desire to
choose an index scan if your joining column's datatypes do not
match

Tom Lane wrote:

> That's getting to be a bit complicated to replicate in N places, though.
> Plus if we ever want to make it work like Alvaro is thinking of, we'd
> have to go back and change all those places again. So I propose
> inventing a function
>
> int guc_complaint_level(GucSource source)
>
> that encapsulates this logic.

I think this makes plenty of sense. However, something that occured to
me just now is that perhaps the right thing to do in the long term is to
put this message in errcontext and leave the "invalid value for XXX" as
the main error message. That would probably involve attaching a
errcontext callback and removing the complaint_level from this message
altogether, letting the outer caller deal with it. I'm not sure how
would GUC work if the assign hook did not raise an ERROR in the
interactive case though.

--
Alvaro Herrera http://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

---------------------------(end of broadcast)---------------------------
TIP 2: Don't 'kill -9' the postmaster

Alvaro Herrera <alvherre@CommandPrompt.com> writes:
> I think this makes plenty of sense. However, something that occured to
> me just now is that perhaps the right thing to do in the long term is to
> put this message in errcontext and leave the "invalid value for XXX" as
> the main error message. That would probably involve attaching a
> errcontext callback and removing the complaint_level from this message
> altogether, letting the outer caller deal with it.

errcontext wouldn't help --- by the time guc.c is ready to throw the
error, the assign hook isn't in the call stack anymore. We'd have to
invent some other special-purpose gizmo to make this work.

Might be worth doing, but again, seems too big a change for 8.3.

regards, tom lane

---------------------------(end of broadcast)---------------------------
TIP 5: don't forget to increase your free space map settings

Tom Lane wrote:
> Alvaro Herrera <alvherre@CommandPrompt.com> writes:
> > I think this makes plenty of sense. However, something that occured to
> > me just now is that perhaps the right thing to do in the long term is to
> > put this message in errcontext and leave the "invalid value for XXX" as
> > the main error message. That would probably involve attaching a
> > errcontext callback and removing the complaint_level from this message
> > altogether, letting the outer caller deal with it.
>
> errcontext wouldn't help --- by the time guc.c is ready to throw the
> error, the assign hook isn't in the call stack anymore. We'd have to
> invent some other special-purpose gizmo to make this work.
>
> Might be worth doing, but again, seems too big a change for 8.3.

Right -- your proposed fix seems fine for 8.3.

--
Alvaro Herrera http://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

---------------------------(end of broadcast)---------------------------
TIP 3: Have you checked our extensive FAQ?

http://www.postgresql.org/docs/faq