Re: [HACKERS] Re: [Pljava-dev] Should creating a new base type require superuser status?

2008-08-03 Thread Thomas Hallgren

Tom Lane wrote:


This is a non-issue in PL/Java. An integer parameter is never passed by 
reference and there's no way the PL/Java user can get direct access to 
backend memory.



So what exactly does happen when the user deliberately specifies wrong
typlen/typbyval/typalign info when creating a type based on PL/Java
functions?

  
Everything is converted into instances of Java classes such as String, 
byte[], etc.


I think that assumption is without ground. Java doesn't permit you to 
access memory unless you use Java classes (java.nio stuff) that is 
explicitly designed to do that and you need native code to set such 
things up. A PL/Java user can not do that unless he is able to link in 
other shared objects or dll's to the backend process.



PL/Java itself must be doing unsafe things in order to interface with
PG at all.  So what your argument really is is that you have managed to
securely sandbox the user-written code you are calling.  That might or
might not be true, but I don't think that worrying about it is without
foundation.

  
I would be presumptuous to claim that I provide the sandbox. All PL/Java 
does is to provide the type mapping. The sandbox as such is implicit in 
Java, much in the same way that it does it for web-browsers etc.


Regardless of that, I think there's some difference in expressing a 
worry that might or might not have a foundation versus claiming that 
there indeed must be a security hole a mile wide ;-)


- thomas


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Mini improvement: statement_cost_limit

2008-08-03 Thread daveg
On Sat, Aug 02, 2008 at 09:30:08PM +0200, Hans-Jürgen Schönig wrote:
 On Aug 2, 2008, at 8:38 PM, Tom Lane wrote:
 
 Andrew Dunstan [EMAIL PROTECTED] writes:
 Hans-Jürgen Schönig wrote:
 i introduced a GUC called statement_cost_limit which can be used to
 error out if a statement is expected to be too expensive.
 
 You clearly have far more faith in the cost estimates than I do.
 
 Wasn't this exact proposal discussed and rejected awhile back?
 
  regards, tom lane
 
 
 
 i don't remember precisely.
 i have seen it on simon's wiki page and it is something which would  
 have been useful in some cases in the past.

I think a variation on this could be very useful in development and test
environments. Suppose it raised a warning or notice if the cost was over
the limit. Then one could set a limit of a few million on the development
and test servers and developers would at least have a clue that they needed
to look at explain for that query. As it is now, one can exhort them to
run explain, but it has no effect.  Instead we later see queries killed
by a 24 hour timeout with estimated costs ranging from until they unplug
the machine and dump it to until the sun turns into a red giant.

-dg

-- 
David Gould   [EMAIL PROTECTED]  510 536 1443510 282 0869
If simplicity worked, the world would be overrun with insects.

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Parsing of pg_hba.conf and authentication inconsistencies

2008-08-03 Thread Magnus Hagander
Tom Lane wrote:
 Robert Treat [EMAIL PROTECTED] writes:
 Certainly there isn't any reason to allow a reload of a file that is just 
 going to break things when the first connection happens. For that matter,  
 why would we ever not want to parse it at HUP time rather than connect time? 
 
 Two or three reasons why not were already mentioned upthread, but for
 the stubborn, here's another one: are you volunteering to write the code
 that backs out the config-file reload after the checks have determined
 it was bad?  Given the amount of pain we suffered trying to make GUC do
 something similar, any sane person would run screaming from the
 prospect.

For pg_hba.conf, I don't see that as a very big problem, really. It
doesn't (and shouldn't) modify any external variables, so it should be
as simple as parsing the new file into a completely separate
list-of-structs and only if it's all correct switch the main pointer
(and free the old struct).

Yes, I still think we should do the simple parsing step at HUP time.
That doesn't mean that it wouldn't be a good idea to have one of these
check-config options that can look for conflicting options *as well*, of
course. But I'm getting the feeling I'm on the losing side of the debate
here...

//Magnus


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Parsing of pg_hba.conf and authentication inconsistencies

2008-08-03 Thread Magnus Hagander
Tom Lane wrote:
 Magnus Hagander [EMAIL PROTECTED] writes:
 The good way to solve this would be to have independant command line
 utilities which check pg_hba.conf, pg_ident.conf and postgresql.conf for
 errors.  Then DBAs could run a check *before* restarting the server.
 
 While clearly useful, it'd still leave the fairly large foot-gun that is
 editing the hba file and HUPing things which can leave you with a
 completely un-connectable database because of a small typo.
 
 That will *always* be possible, just because software is finite and
 human foolishness is not ;-).

Certainly - been bitten by that more than once. But we can make it
harder or easier to make the mistakes..


 Now, we could ameliorate it a bit given a postgres --check-config
 mode by having pg_ctl automatically run that mode before any start,
 restart, or reload command, and then refusing to proceed if the check
 detects any indubitable errors.  On the other hand, that would leave
 us with the scenario where the checking code warns about stuff that it
 can't be sure is wrong, but then we go ahead and install the borked
 config anyway.   (Nobody is going to put up with code that refuses
 to install config settings that aren't 100% clean, unless the checks
 are so weak that they miss a lot of possibly-useful warnings.)
 
 Seems a lot better to me to just train people to run the check-config
 code by hand before pulling the trigger to load the settings for real.

It's certainly easier for us, but that means a whole lot of people are
never going to do it. And initscripts might end up using it anyway,
forcing the issue.

I think it'd be reasonable to refuse starting if the config is *known
broken* (such as containing lines that are unparseable, or that contain
completely invalid tokens), whereas you'd start if they just contain
things that are probably wrong. But picking from your previous
examples of more advanced checks,  there are lots of cases where
things like overlapping CIDR address ranges are perfectly valid, so I
don't think we could even throw a warning for that - unless there's a
separate flag to enable/disable warnings for such a thing.

//Magnus

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Auto-explain patch

2008-08-03 Thread Dean Rasheed

Thanks for the feedback, and sorry for my delay in replying (I was on
holiday).

 Tom Lane wrote:

 Comments:

 I do not think that you should invent a new elog level for this, and
 especially not one that is designed to send unexpected messages to the
 client. Having to kluge tab completion like that is just a signal that
 you're going to break a lot of other clients too. It seems to me that
 the right behavior for auto-explain messages is to go only to the log by
 default, which means that LOG is already a perfectly good elog level for
 auto-explain messages.

The more I thought about this, the more I thought that it was OTT to
add a new elog level just for this, so I agree it should probably just
go to the LOG elog level.

I don't agree with your reasoning on tab-completion though. I actually
think that this is a signal of a broken client. If the user sets
client_min_messages to LOG or lower, and has any of the other logging
or debugging parameters enabled, the output tramples all over the
suggested completions. I don't think the average user is interested in
log/debug output from the queries psql does internally during
tab-completion. So IMHO the tab-completion 'kludge', is still
worthwhile, regardless of the rest of the patch.


 Drop the query_string addition to PlannedStmt --- there are other ways
 you can get that string in CVS HEAD.

OK. What is the best way to get this string now? Are you referring to
debug_query_string, or is there another way?


 I don't think that planner_time
 belongs there either. It would be impossible to define a principled way
 to compare two PlannedStmts for equality with that in there. Nor do I
 see the point of doing it the way you're doing it. Why don't you just
 log the slow planning cycle immediately upon detecting it in planner()?
 I don't see that a slow planning cycle has anything necessarily to do
 with a slow execution cycle, so IMHO they ought to just get logged
 independently.

Makes sense.


 Please do not export ExplainState --- that's an internal matter for
 explain.c. Export some wrapper function with a cleaner API than
 explain_outNode, instead.

 regards, tom lane

OK, that's much neater.

I'll try to rework this ASAP but I understand if it's too late for
this commitfest.

Cheers, Dean.

_
Win a voice over part with Kung Fu Panda  Live Search   and   100’s of Kung Fu 
Panda prizes to win with Live Search
http://clk.atdmt.com/UKM/go/107571439/direct/01/
-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Parsing of pg_hba.conf and authentication inconsistencies

2008-08-03 Thread Gregory Stark
Tom Lane [EMAIL PROTECTED] writes:

 Robert Treat [EMAIL PROTECTED] writes:
 Certainly there isn't any reason to allow a reload of a file that is just 
 going to break things when the first connection happens. For that matter,  
 why would we ever not want to parse it at HUP time rather than connect time? 

 Two or three reasons why not were already mentioned upthread, but for
 the stubborn, here's another one: are you volunteering to write the code
 that backs out the config-file reload after the checks have determined
 it was bad?  Given the amount of pain we suffered trying to make GUC do
 something similar, any sane person would run screaming from the
 prospect.

Wouldn't that be *easier* if we do more parsing in the postmaster instead of
in the backends as Magnus suggested? Then it could build a new set of
structures and if there are any errors just throw them out before replacing
the old ones.

-- 
  Gregory Stark
  EnterpriseDB  http://www.enterprisedb.com
  Ask me about EnterpriseDB's Slony Replication support!

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Parsing of pg_hba.conf and authentication inconsistencies

2008-08-03 Thread Stephen Frost
* Magnus Hagander ([EMAIL PROTECTED]) wrote:
 For pg_hba.conf, I don't see that as a very big problem, really. It
 doesn't (and shouldn't) modify any external variables, so it should be
 as simple as parsing the new file into a completely separate
 list-of-structs and only if it's all correct switch the main pointer
 (and free the old struct).

I'm in agreement with this approach.  Allowing a config which won't
parse properly to completely break access to a running database is
terrible.  It just doesn't come across to me as being all that difficult
or complex code for pg_hba.conf.

 Yes, I still think we should do the simple parsing step at HUP time.
 That doesn't mean that it wouldn't be a good idea to have one of these
 check-config options that can look for conflicting options *as well*, of
 course. But I'm getting the feeling I'm on the losing side of the debate
 here...

A little extra code in the backend is well worth fixing this foot-gun.
The concerns raised so far have been who will write it? and what if
it has bugs?.  Neither of these are particularly compelling arguments
when you've already offered to write and bug-test it (right, Magnus? :).

Thanks,

Stephen


signature.asc
Description: Digital signature


Re: [HACKERS] Parsing of pg_hba.conf and authentication inconsistencies

2008-08-03 Thread Magnus Hagander
Stephen Frost wrote:
 * Magnus Hagander ([EMAIL PROTECTED]) wrote:
 For pg_hba.conf, I don't see that as a very big problem, really. It
 doesn't (and shouldn't) modify any external variables, so it should be
 as simple as parsing the new file into a completely separate
 list-of-structs and only if it's all correct switch the main pointer
 (and free the old struct).
 
 I'm in agreement with this approach.  Allowing a config which won't
 parse properly to completely break access to a running database is
 terrible.  It just doesn't come across to me as being all that difficult
 or complex code for pg_hba.conf.

That's my thoughts as well, which may be off of course ;-)


 Yes, I still think we should do the simple parsing step at HUP time.
 That doesn't mean that it wouldn't be a good idea to have one of these
 check-config options that can look for conflicting options *as well*, of
 course. But I'm getting the feeling I'm on the losing side of the debate
 here...
 
 A little extra code in the backend is well worth fixing this foot-gun.
 The concerns raised so far have been who will write it? and what if
 it has bugs?.  Neither of these are particularly compelling arguments
 when you've already offered to write and bug-test it (right, Magnus? :).

Toms main argument has been that it would move the code *from* the
backend and into the *postmaster*, which is much more sensitive.

And yes, I've offered to write the code. I take this as an offer from
you to bug-test it :-)

//Magnus

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Parsing of pg_hba.conf and authentication inconsistencies

2008-08-03 Thread Stephen Frost
* Magnus Hagander ([EMAIL PROTECTED]) wrote:
 I think it'd be reasonable to refuse starting if the config is *known
 broken* (such as containing lines that are unparseable, or that contain
 completely invalid tokens), whereas you'd start if they just contain
 things that are probably wrong. But picking from your previous
 examples of more advanced checks,  there are lots of cases where
 things like overlapping CIDR address ranges are perfectly valid, so I
 don't think we could even throw a warning for that - unless there's a
 separate flag to enable/disable warnings for such a thing.

Agreed.  Making sure the config can parse is different from parsable but
non-sensible.  It's ridiculously easy to mistakenly add a line w/ a
single character on it or something equally bad when saving a file
that's being modified by hand.  That's a simple check that should be
done on re-hup and the broken config shouldn't be put in place.

I certainly agree that we should *also* have a way to just check the
config, so that can be built into init scripts and whatnot.  I don't
think having one precludes having the other, and I'm pretty confident we
could find a way to not duplicate the code and have things be clean.

Thanks,

Stephen


signature.asc
Description: Digital signature


Re: [HACKERS] Parsing of pg_hba.conf and authentication inconsistencies

2008-08-03 Thread Stephen Frost
* Magnus Hagander ([EMAIL PROTECTED]) wrote:
 Stephen Frost wrote:
  A little extra code in the backend is well worth fixing this foot-gun.
  The concerns raised so far have been who will write it? and what if
  it has bugs?.  Neither of these are particularly compelling arguments
  when you've already offered to write and bug-test it (right, Magnus? :).
 
 Toms main argument has been that it would move the code *from* the
 backend and into the *postmaster*, which is much more sensitive.

erm, yes, sorry I wasn't being clear.  Brain moving faster than fingers
sometimes. :)

 And yes, I've offered to write the code. I take this as an offer from
 you to bug-test it :-)

Indeed, as long as I'm bug-testing the Kerberos mappings at the same
time... ;)  Seriously though, I'd be happy to review and test the code.

Thanks,

Stephen


signature.asc
Description: Digital signature


Re: [HACKERS] Mini improvement: statement_cost_limit

2008-08-03 Thread Simon Riggs

On Sun, 2008-08-03 at 00:44 -0700, daveg wrote:
 On Sat, Aug 02, 2008 at 09:30:08PM +0200, Hans-Jürgen Schönig wrote:
  On Aug 2, 2008, at 8:38 PM, Tom Lane wrote:
  
  Andrew Dunstan [EMAIL PROTECTED] writes:
  Hans-Jürgen Schönig wrote:
  i introduced a GUC called statement_cost_limit which can be used to
  error out if a statement is expected to be too expensive.
  
  You clearly have far more faith in the cost estimates than I do.
  
  Wasn't this exact proposal discussed and rejected awhile back?
  
  i don't remember precisely.
  i have seen it on simon's wiki page and it is something which would  
  have been useful in some cases in the past.

I still support it. Regrettably, many SQL developers introduce product
joins and other unintentional errors. Why let problem queries through?
Security-wise they're great Denial of Service attacks, bringing the
server to its knees better than most ways I know, in conjunction with a
nice hefty work_mem setting. 27 table product joins: memory, CPU, I/O
and diskspace resources used all in a simple killer query.

If anybody thinks costs are inaccurate, don't use it. Or better still
improve the cost models. It isn't any harder or easier to find a useful
value than it is to use statement_timeout. What's the difference between
picking an arbitrary time and an arbitrary cost? You need to alter the
value according to people's complaints in both cases.

 I think a variation on this could be very useful in development and test
 environments. Suppose it raised a warning or notice if the cost was over
 the limit. Then one could set a limit of a few million on the development
 and test servers and developers would at least have a clue that they needed
 to look at explain for that query. As it is now, one can exhort them to
 run explain, but it has no effect.  Instead we later see queries killed
 by a 24 hour timeout with estimated costs ranging from until they unplug
 the machine and dump it to until the sun turns into a red giant.

Great argument. So that's 4 in favour at least.

A compromise would be to have log_min_statement_cost (or
warn_min_statement_cost) which will at least help find these problems in
testing before we put things live, but that still won't help with
production issues.

Another alternative would be to have a plugin that can examine the plan
immediately after planner executes, so you can implement this yourself,
plus some other possibilities.

-- 
 Simon Riggs   www.2ndQuadrant.com
 PostgreSQL Training, Services and Support


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] small bug in hlCover

2008-08-03 Thread Sushant Sinha
Has any one noticed this?

-Sushant.

On Wed, 2008-07-16 at 23:01 -0400, Sushant Sinha wrote:
 I think there is a slight bug in hlCover function in wparser_def.c
 
 If there is only one query item and that is the first word in the text,
 then hlCover does not returns any cover. This is evident in this example
 when ts_headline only generates the min_words:
 
 testdb=# select ts_headline('1 2 3 4 5 6 7 8 9 10','1'::tsquery,
 'MinWords=5');
ts_headline
 --
  b1/b 2 3 4 5
 (1 row)
 
 The problem is that *q is initialized to 0 which is a legitimate value
 for a cover. So I have attached a patch that fixes it and after applying
 the patch here is the result.
 
 testdb=# select ts_headline('1 2 3 4 5 6 7 8 9 10','1'::tsquery,
 'MinWords=5');
  ts_headline 
 -
  b1/b 2 3 4 5 6 7 8 9 10
 (1 row)
 
 -Sushant.


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Mini improvement: statement_cost_limit

2008-08-03 Thread Hans-Jürgen Schönig

hello ...




I still support it. Regrettably, many SQL developers introduce product
joins and other unintentional errors. Why let problem queries through?



i think the killer is that we don't have to wait until the query dies  
with a statement_timeout.
it is ways more elegant to kill things before they have already eaten  
too many cycles.
one thing which is important as well: statement_cost_limit  does not  
kill queries which have just been waiting for a lock.

this makes things slightly more predictable.



Security-wise they're great Denial of Service attacks, bringing the
server to its knees better than most ways I know, in conjunction  
with a

nice hefty work_mem setting. 27 table product joins: memory, CPU, I/O
and diskspace resources used all in a simple killer query.




i am not too concerned about DNS, i have to admit.
i would rather see it as a way to make developers do better things.



If anybody thinks costs are inaccurate, don't use it. Or better still
improve the cost models. It isn't any harder or easier to find a  
useful
value than it is to use statement_timeout. What's the difference  
between

picking an arbitrary time and an arbitrary cost? You need to alter the
value according to people's complaints in both cases.



the cost model is good enough to see if something is good or bad.
this is basically all we want to do here --- killing all evil.



*snip*






A compromise would be to have log_min_statement_cost (or
warn_min_statement_cost) which will at least help find these  
problems in

testing before we put things live, but that still won't help with
production issues.




definitely. a good idea as well - but people will hardly read it, i  
guess :(.



Another alternative would be to have a plugin that can examine the  
plan
immediately after planner executes, so you can implement this  
yourself,

plus some other possibilities.




this would be really fancy.
how could a plugin like that look like?

hans



--
Cybertec Schönig  Schönig GmbH
Gröhrmühlgasse 26
A-2700 Wiener Neustadt
Web: www.postgresql-support.de


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] patch: Add a separate TRUNCATE permission

2008-08-03 Thread Simon Riggs

On Tue, 2008-07-29 at 09:03 -0400, Stephen Frost wrote:
 * Peter Eisentraut ([EMAIL PROTECTED]) wrote:
  Am Tuesday, 29. July 2008 schrieb Stephen Frost:
   I'd certainly like to see a truncate permission, I wrote a patch for it
   myself back in January of 2006.  That thread starts here:
  
   http://archives.postgresql.org/pgsql-patches/2006-01/msg00028.php
  
   I think someone else submitted a patch for it last year too, actually.

It was raised in January and rejected again then. Glad to see it raised
again here. I believe Tom's previous concerns about allowing truncate
permissions were related to the potentially increased number of truncate
commands this would cause and the need to tune invalidation messages.
That's done now.
 
  Well, that certainly indicates some demand.
  
  I think we should worry about adding more bits when we need them.  It's 
  certainly possible to add more bits, so it is not like we need to save the 
  ones we have forever.
 
 I would agree with this.  Others?

I've no problem with running out of bits. At this rate, we have enough
some for some years yet. But I don't see too many additional permissions
coming along anyhow.

-- 
 Simon Riggs   www.2ndQuadrant.com
 PostgreSQL Training, Services and Support


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Mini improvement: statement_cost_limit

2008-08-03 Thread Simon Riggs

On Sun, 2008-08-03 at 22:09 +0200, Hans-Jürgen Schönig wrote:

  Another alternative would be to have a plugin that can examine the  
  plan
  immediately after planner executes, so you can implement this  
  yourself,
  plus some other possibilities.
 

 this would be really fancy.
 how could a plugin like that look like?

Hmm...thinks: exactly like the existing planner_hook().

So, rewrite this as a planner hook and submit as a contrib module.

-- 
 Simon Riggs   www.2ndQuadrant.com
 PostgreSQL Training, Services and Support


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


[HACKERS] unnecessary code in_bt_split

2008-08-03 Thread Zdenek Kotala
I found that _bt_split function calls PageGetTempPage, but next call is 
_bt_page_init which clear all contents anyway. Is there any reason to call 
PageGetTempPage instead of palloc?


Original code:

00796 leftpage = PageGetTempPage(origpage, sizeof(BTPageOpaqueData));
00797 rightpage = BufferGetPage(rbuf);
00798
00799 _bt_pageinit(leftpage, BufferGetPageSize(buf));

Suggested code:

00796 leftpage = palloc(PageGetSize(origpage));
00797 rightpage = BufferGetPage(rbuf);
00798
00799 _bt_pageinit(leftpage, BufferGetPageSize(buf));


Any idea?

thanks Zdenek



--
Zdenek Kotala  Sun Microsystems
Prague, Czech Republic http://sun.com/postgresql


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] unnecessary code in_bt_split

2008-08-03 Thread Tom Lane
Zdenek Kotala [EMAIL PROTECTED] writes:
 I found that _bt_split function calls PageGetTempPage, but next call is 
 _bt_page_init which clear all contents anyway. Is there any reason to call 
 PageGetTempPage instead of palloc?

Not violating a perfectly good abstraction?

I agree that PageGetTempPage isn't amazingly efficient, but internal
refactoring would halve its cost; and if you have some evidence that
there's a real performance issue then we could think about adjusting
the temp-page API to allow _bt_pageinit to be combined with it.  But
I have a real problem with hacking up _bt_split so that it will call
PageRestoreTempPage on something it didn't get from PageGetTempPage.

Considering the WAL and regular I/O that will be induced by a split,
I kinda doubt this is even worth worrying about anyway...

regards, tom lane

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Mini improvement: statement_cost_limit

2008-08-03 Thread Josh Berkus
Tom,

 Wasn't this exact proposal discussed and rejected awhile back?

We rejected Greenplum's much more invasive resource manager, because it 
created a large performance penalty on small queries whether or not it was 
turned on.  However, I don't remember any rejection of an idea as simple 
as a cost limit rejection.

This would, IMHO, be very useful for production instances of PostgreSQL.  
The penalty for mis-rejection of a poorly costed query is much lower than 
the penalty for having a bad query eat all your CPU.

-- 
--Josh

Josh Berkus
PostgreSQL
San Francisco

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Instructions for adding new catalog

2008-08-03 Thread Gustavo Tonini
Thanks for reply, Tom.
Well, just for documenting the process...
Adding new postgres catalog in 2 little steps:

1)Write catalog header file and save it into src/include/catalog
directory. Hint: copy format from other catalog headers.
2)Add your header file name to variable POSTGRES_BKI_SRCS in file
src/backend/catalog/Makefile

Make, make install, then initdb will create new system catalogs for you.

Gustavo.

On Fri, Aug 1, 2008 at 11:28 AM, Tom Lane [EMAIL PROTECTED] wrote:
 Gustavo Tonini [EMAIL PROTECTED] writes:
 I read the archives, but I can't find the instructions for adding new
 catalogs to the system.

 That's probably because there aren't any.  Look at recent patches
 that added a new catalog to get an idea of what you need to do.
 The enum patch might be a good choice.

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Mini improvement: statement_cost_limit

2008-08-03 Thread Robert Treat
On Sunday 03 August 2008 15:12:22 Simon Riggs wrote:
 On Sun, 2008-08-03 at 00:44 -0700, daveg wrote:
  On Sat, Aug 02, 2008 at 09:30:08PM +0200, Hans-Jürgen Schönig wrote:
   On Aug 2, 2008, at 8:38 PM, Tom Lane wrote:
   Andrew Dunstan [EMAIL PROTECTED] writes:
   Hans-Jürgen Schönig wrote:
   i introduced a GUC called statement_cost_limit which can be used to
   error out if a statement is expected to be too expensive.
   
   You clearly have far more faith in the cost estimates than I do.
   
   Wasn't this exact proposal discussed and rejected awhile back?
  
   i don't remember precisely.
   i have seen it on simon's wiki page and it is something which would
   have been useful in some cases in the past.

 I still support it. Regrettably, many SQL developers introduce product
 joins and other unintentional errors. Why let problem queries through?
 Security-wise they're great Denial of Service attacks, bringing the
 server to its knees better than most ways I know, in conjunction with a
 nice hefty work_mem setting. 27 table product joins: memory, CPU, I/O
 and diskspace resources used all in a simple killer query.


ISTR that what ended up killing the enthusiasm for this was that most people 
realized that this GUC was just a poor tool to take a stab at solving other 
problems (ie. rate limiting cpu for queries). 

 If anybody thinks costs are inaccurate, don't use it. Or better still
 improve the cost models. It isn't any harder or easier to find a useful
 value than it is to use statement_timeout. What's the difference between
 picking an arbitrary time and an arbitrary cost? You need to alter the
 value according to people's complaints in both cases.


I think the original argument for statement_timeout was that long running 
queries were known to cause have wrt vacuum strategies (remember, that one 
has been in the back end a long time). ISTR some recent threds on -hackers 
questioning whether statement_timeout should be eliminated itself.  

  I think a variation on this could be very useful in development and test
  environments. Suppose it raised a warning or notice if the cost was over
  the limit. Then one could set a limit of a few million on the development
  and test servers and developers would at least have a clue that they
  needed to look at explain for that query. As it is now, one can exhort
  them to run explain, but it has no effect.  Instead we later see queries
  killed by a 24 hour timeout with estimated costs ranging from until they
  unplug the machine and dump it to until the sun turns into a red
  giant.

 Great argument. So that's 4 in favour at least.


Not such a great argument. Cost models on development servers can and often 
are quite different from those on production, so you might be putting an 
artifical limit on top of your developers. 

 A compromise would be to have log_min_statement_cost (or
 warn_min_statement_cost) which will at least help find these problems in
 testing before we put things live, but that still won't help with
 production issues.

 Another alternative would be to have a plugin that can examine the plan
 immediately after planner executes, so you can implement this yourself,
 plus some other possibilities.


I still think it is worth revisiting what problems people are trying to solve, 
and see if there are better tools they can be given to solve them.  Barring 
that, I suppose a crude solution is better than nothing, though I fear people 
might point at the crude solution as a good enough solution to justify not 
working on better solutions. 

-- 
Robert Treat
Build A Brighter LAMP :: Linux Apache {middleware} PostgreSQL

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Instructions for adding new catalog

2008-08-03 Thread Tom Lane
Gustavo Tonini [EMAIL PROTECTED] writes:
 Well, just for documenting the process...
 Adding new postgres catalog in 2 little steps:

 1)Write catalog header file and save it into src/include/catalog
 directory. Hint: copy format from other catalog headers.
 2)Add your header file name to variable POSTGRES_BKI_SRCS in file
 src/backend/catalog/Makefile

As long as we're documenting ...

* Documenting a new catalog in catalogs.sgml is not considered optional.
* Most likely, your catalog needs some indexes, which should be declared
  in catalog/indexing.h.
* Of course, the reason you are defining a new system catalog is that
  the C code is going to know about it.  So there is a whole lot of
  other stuff you probably need to write: object insertion code,
  object deletion code, lookup code (maybe a new syscache or two).
  Traditionally the lowest-level insertion/deletion code goes into
  catalog/pg_yourcatalog.c, while slightly higher-level code such as
  the implementation of new utility commands to manage this new kind
  of object will go elsewhere.

That's before you get to any actual new functionality...

regards, tom lane

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] small bug in hlCover

2008-08-03 Thread Sushant Sinha
On Mon, 2008-08-04 at 00:36 -0300, Euler Taveira de Oliveira wrote:
 Sushant Sinha escreveu:
  I think there is a slight bug in hlCover function in wparser_def.c
  
 The bug is not in the hlCover. In prsd_headline, if we didn't find a 
 suitable bestlen (i.e. = 0), than it includes up to document length or 
 *maxWords* (here is the bug). I'm attaching a small patch that fixes it 
 and some comment typos. Please apply it to 8_3_STABLE too.

Well hlCover purpose is to find a cover and for the document  '1 2 3 4 5
6 7 8 9 10' and the query '1'::tsquery, a cover exists. So it should
point it out.

On my source I see that prsd_headline marks only min_words which seems
like the right thing to do.

-Sushant.

 
 euler=# select ts_headline('1 2 3 4 5 6 7 8 9 10','1'::tsquery, 
 'MinWords=5');
   ts_headline
 -
   b1/b 2 3 4 5 6 7 8 9 10
 (1 registro)
 
 euler=# select ts_headline('1 2 3 4 5 6 7 8 9 10','1'::tsquery);
   ts_headline
 -
   b1/b 2 3 4 5 6 7 8 9 10
 (1 registro)
 
 


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Mini improvement: statement_cost_limit

2008-08-03 Thread Mark Kirkwood

Josh Berkus wrote:

Tom,

  

Wasn't this exact proposal discussed and rejected awhile back?



We rejected Greenplum's much more invasive resource manager, because it 
created a large performance penalty on small queries whether or not it was 
turned on.  However, I don't remember any rejection of an idea as simple 
as a cost limit rejection.


This would, IMHO, be very useful for production instances of PostgreSQL.  
The penalty for mis-rejection of a poorly costed query is much lower than 
the penalty for having a bad query eat all your CPU.


  
Greenplum's introduced a way to creating a cost threshold a bit like 
the way Simon was going to do shared work_mem. It did 2 things:


1/ Counted the cost of an about-to-be run query against the threshold, 
and made the query wait if it would exhaust it

2/ Aborted the query if its  cost was greater than the threshold

Initially there was quite a noticeable performance penalty with it 
enabled - but as the guy working on it (me) redid bits and pieces then 
penalty decreased massively. Note that in all cases, disabling the 
feature meant there was no penalty.


The latest variant of the code is around in the Bizgres repository 
(src/backend/utils/resscheduler I think) - some bits might be worth 
looking at!


Best wishes

Mark

P.s : I'm not working for Greenplum now.

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers