Re: [HACKERS] [COMMITTERS] pgsql: Add regression tests for MONEY type.
On Sat, 24 Nov 2007 17:22:35 -0300 Alvaro Herrera [EMAIL PROTECTED] wrote: D'Arcy J.M. Cain wrote: Log Message: --- Add regression tests for MONEY type. This has broken the buildfarm ... It worked here. Can you send me the error output. -- D'Arcy J.M. Cain [EMAIL PROTECTED] | Democracy is three wolves http://www.druid.net/darcy/| and a sheep voting on +1 416 425 1212 (DoD#0082)(eNTP) | what's for dinner. ---(end of broadcast)--- TIP 7: You can help support the PostgreSQL project by donating at http://www.postgresql.org/about/donate
Re: [HACKERS] quote_literal(integer) does not exist
On Sat, 24 Nov 2007 21:17:39 -0500 Tom Lane wrote: Andreas 'ads' Scherbaum [EMAIL PROTECTED] writes: we have some plpgsql functions which use quote_literal() regardless of the data type. With Beta 3 this does not work anymore[1]. If you're unwilling to fix your application, you can hack around that for yourself. regression=# select quote_literal(42); ERROR: function quote_literal(integer) does not exist LINE 1: select quote_literal(42); ^ HINT: No function matches the given name and argument types. You might need to add explicit type casts. regression=# create function quote_literal(anyelement) returns text as $$ regression$# select pg_catalog.quote_literal($1 :: pg_catalog.text) regression$# $$ language sql; CREATE FUNCTION regression=# select quote_literal(42); quote_literal --- '42' (1 row) Already had a similar function in my test case, but yours is more elegant. I also think, that we will fix our applications or at least most of them. But that's not the point: more people will run into this problem and this looks like a showstopper for updating to 8.3. By the way, the function is named quote_literal(), not quote_text(). From my point of view i expect to get everything correctly quoted, what's feeded as input into this function. Given the fact, that previous versions accepted every input without notice about the implicit cast, i don't see not so much blame in the application. Kind regards -- Andreas 'ads' Scherbaum PostgreSQL User Group Germany ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [HACKERS] Open 8.3 issues
On Sat, 2007-11-24 at 17:06 -0500, Greg Smith wrote: On Sat, 24 Nov 2007, Tom Lane wrote: [PATCHES] [Fwd: Re: [HACKERS] Postgres 8.3 archive_command], Simon Riggs Applied. Getting positive feedback that your archive command has triggered is helpful for new users of this feature, and making it so that doesn't happen anymore is a step backwards for them as far as I'm concerned. Simon suggested he had a documentation update that was going to cover this. I'd like to see that and a mention of this change in the release notes before this is closed. OK, I'll patch the docs, you patch the release notes, how's that? My notes say I have these doc changes to submit for 8.3 - SQL Standard changes - array references clean-up - locking clarification - PITR clarifications I already thought that the way the archive_command examples do everything as a command line instead of calling a script sets a bad example practice, and unless you call something you don't have options like tweak your archive_command script to do some logging of its own. This change makes a much stronger case for saying outright the archive_command should call a separate script, so you can adjust things including logging there. That concept isn't even introduced by the current documentation. I know I was surprised the first time I echo'd something from the script and discovered it showed up in the server logs. Being able to see what the command is seems like good practice to me, so I don't agree that we should encourage people to use scripts in all cases. But if you say that it isn't clear you can use scripts then that should change, especially the additional messages aspect. -- Simon Riggs 2ndQuadrant http://www.2ndQuadrant.com ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
Re: [HACKERS] 8.3devel slower than 8.2 under read-only load
On Nov 25, 2007 8:06 AM, Tom Lane [EMAIL PROTECTED] wrote: The TPS numbers bounce around by 1% or so on repeated trials, so I wouldn't put too much faith in small differences. What it looks like to me is that it's all about the stats collection overhead. The drop on 01-17 corresponds to autovac and stats_row_level being turned on by default. The improvement on 03-02 is the fix for the problem that the stats collector process wanted to write the stats file way too often, and the improvement on 04-30 comes from rate-limiting stats messages from individual backends to the stats collector. Using pgbench -n -S -c 10 -t 10, I also have CVS tip as fast as CVS from january. But using my set of queries, it's not. It's hard to tell what is exactly causing this but the only difference between both is that mine is based on varchars and yours is based on integers so as Greg suggested it, maybe the culprit is the packed varlenas patch. I'll try to measure the overhead of this patch alone. -- Guillaume ---(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
Re: [HACKERS] quote_literal(integer) does not exist
On Nov 25, 2007 11:51 PM, Andreas 'ads' Scherbaum [EMAIL PROTECTED] wrote: But that's not the point: more people will run into this problem and this looks like a showstopper for updating to 8.3. By the way, the function is named quote_literal(), not quote_text(). From my point of view i expect to get everything correctly quoted, what's feeded as input into this function. Given the fact, that previous versions accepted every input without notice about the implicit cast, i don't see not so much blame in the application. I had a similar experience to Andreas when I first migrated my app to 8.3b1 for testing. It wasn't hard to fix, but did seem like an unnecessary barrier to upgrading. I agree that the name of the function (and its behavior up till now) encourage users to think of it as a quote whatever I throw at you function, which is indeed what you would want/expect in the context of building a dynamic query. Having quote_literal take its argument as text was fine whilst we were automatically casting arguments, but in 8.3 it seems that quote_literal(anyelement) makes a lot more sense than quote_literal(text). So, I wonder why we don't just adapt the internal function to take anyelement? It would save a lot of apps from being broken by the move to 8.3, and make the function more convenient. Regards, BJ ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
Re: [HACKERS] 8.3devel slower than 8.2 under read-only load
On Nov 24, 2007 5:50 PM, Gregory Stark [EMAIL PROTECTED] wrote: It would be nice to have infrastructure similar to the buldfarm running a standard set of benchmarks every day. It would be fascinating to see the graphs day-by-day of performance. Hopefully we wouldn't see too many dips and just a steady increase over time. FYI, in addition to the 4 boxes donated to PostgreSQLFr (which will be used for hosting purposes AFAIK), Continuent Inc. donated 7 servers to Open Wide for PostgreSQL community usage. They will be hosted here at Open Wide (Lyon, France) as soon as our new datacenter will be available (we're a little short at rack space ATM; my target is to have them setup at end of January). My main goal for these servers is to set up a PostgreSQL benchmark lab with automatic daily benchmarks, probably simple ones at the beginning (various pgbench settings using one, two, three clients on one server to get concurrency) but I hope we will be able to set up more representative ones little by little. I can provide simple results by myself but I'm sure Mark Wong's and others' experience will be highly valuable to get more detailed results on a daily basis. I won't have the time to setup something similar to the build farm but really want us to have results soon so I'll set up something simple to begin with and if someone has the time to build a bench farm thing, I'll be happy to test it on these boxes and contribute. By the way, we will grant access to these boxes to community people without any problem. I'll give more news on this ASAP. -- Guillaume ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [HACKERS] quote_literal(integer) does not exist
Hello Having quote_literal take its argument as text was fine whilst we were automatically casting arguments, but in 8.3 it seems that quote_literal(anyelement) makes a lot more sense than quote_literal(text). So, I wonder why we don't just adapt the internal function to take anyelement? It would save a lot of apps from being broken by the move to 8.3, and make the function more convenient. It's good idea. I'll look on it. It needs change of pgproc :( Pavel ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
[HACKERS] polymorphic functions and unknown type problem
Hello I tryed implement quote_literal function with anyelement parametr. It works for all specified types, but I have problem with unknown type. Is it cstring? I can understand prohibition in SQL or PLPGSQL, but why I cannot use it in C language. Regards Pavel Stehule ---(end of broadcast)--- TIP 7: You can help support the PostgreSQL project by donating at http://www.postgresql.org/about/donate
Re: [HACKERS] 8.3devel slower than 8.2 under read-only load
Guillaume Smet [EMAIL PROTECTED] writes: Using pgbench -n -S -c 10 -t 10, I also have CVS tip as fast as CVS from january. But using my set of queries, it's not. Were you ever able to send your queries? -- Gregory Stark EnterpriseDB http://www.enterprisedb.com Ask me about EnterpriseDB's Slony Replication support! ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
Re: [HACKERS] quote_literal(integer) does not exist
Hello I sent patch to pg_patches. Regards Pavel Stehule ---(end of broadcast)--- TIP 1: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
Re: [HACKERS] 8.3devel slower than 8.2 under read-only load
Guillaume Smet [EMAIL PROTECTED] writes: Using pgbench -n -S -c 10 -t 10, I also have CVS tip as fast as CVS from january. But using my set of queries, it's not. Would you show us the test case you're using? regards, tom lane ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [HACKERS] [COMMITTERS] pgsql: Add regression tests for MONEY type.
D'Arcy J.M. Cain [EMAIL PROTECTED] writes: Alvaro Herrera [EMAIL PROTECTED] wrote: This has broken the buildfarm ... It worked here. Can you send me the error output. The problem was you forgot to update serial_schedule. regards, tom lane ---(end of broadcast)--- TIP 1: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
Re: [HACKERS] quote_literal(integer) does not exist
Brendan Jurd [EMAIL PROTECTED] writes: So, I wonder why we don't just adapt the internal function to take anyelement? The main argument against is the slippery slope one: once we accept this, what else? The entire point of that change was to make people be aware of where they are inducing text coercions, and deciding to hide that again on the basis of individual complaints is no way to design a system. As a not-too-far-away example, I see that the proposed patch Pavel sent in arbitrarily decides to change quote_ident() too, which was not asked for and has got much less justification than changing quote_literal(). That sort of cowboy approach to semantics is not the way to proceed. Another issue is that changing pg_proc.h without forcing initdb is not good practice. We are far enough along in the beta cycle that we shouldn't force initdb lightly, and I definitely *don't* want to do it again next week when someone else comes up with some other must have auto-coercing function. If anyone wants to make a serious argument for this, look through the whole of pg_proc.h, see what else needs to be changed at the same time, and make a coherent proposal. regards, tom lane ---(end of broadcast)--- TIP 1: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
Re: [HACKERS] 8.3devel slower than 8.2 under read-only load
On Nov 25, 2007 6:41 PM, Tom Lane [EMAIL PROTECTED] wrote: Would you show us the test case you're using? Sure, it's the same queries I posted earlier. My pgbench script is the following: BEGIN select libvil from vilsitelang where codelang='FRA' and codevil='LYO' select TL.motsclesmetatags, TL.descriptifmeta, TL.motcleoverture_l, TL.motcleoverture_c, TL.baselinetheme from themelang TL where TL.codeth = 'ASS' and TL.codelang = 'FRA' SELECT libvilpubwoo, codelang, codepays, petiteville FROM vilsite WHERE codevil = 'LYO' select libvil from vilsitelang where codelang='FRA' and codevil='LYO' END I send a link to the data to both of you in private. -- Guillaume ---(end of broadcast)--- TIP 1: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
Re: [HACKERS] 8.3devel slower than 8.2 under read-only load
Guillaume Smet [EMAIL PROTECTED] writes: On Nov 25, 2007 6:41 PM, Tom Lane [EMAIL PROTECTED] wrote: Would you show us the test case you're using? Sure, it's the same queries I posted earlier. What about the table schemas? I send a link to the data to both of you in private. I doubt that the specific data is important. regards, tom lane ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [HACKERS] 8.3devel slower than 8.2 under read-only load
On Nov 25, 2007 7:35 PM, Tom Lane [EMAIL PROTECTED] wrote: What about the table schemas? The schema is in the dump. That's the main reason why I've sent the dump to both of you. Anyway, here it is: db=# \d themelang Table public.themelang Column | Type | Modifiers --++--- codeth | character varying(3) | not null codelang | character varying(3) | not null libtheme | character varying(70) | not null motsclesmetatags | character varying(500) | descriptifmeta | character varying(500) | motcleoverture_l | character varying(100) | motcleoverture_c | character varying(100) | liencourt| character varying(30) | baselinetheme| character varying(300) | ordrenewsletter | integer| Indexes: pk_themelang PRIMARY KEY, btree (codeth, codelang) db=# \d vilsite Table public.vilsite Column| Type | Modifiers -+---+ codevil | character varying(3) | not null codelang| character varying(3) | not null codepays| character varying(3) | not null regionwap | character varying(3) | codedep | character varying(3) | codetypevil | character varying(1) | not null ouverte | integer | not null codeinteret | character varying(1) | not null familleville| character varying(1) | not null codevilpostale | character varying(8) | capitale| integer | not null codemeteodirect | character varying(5) | libvilpubwoo| character varying(10) | population | integer | codeinsee | character varying(5) | petiteville | integer | not null default 0 logincommercial | character varying(20) | logincommercialprec | character varying(20) | Indexes: pk_vilsite PRIMARY KEY, btree (codevil) db=# \d vilsitelang Table public.vilsitelang Column | Type | Modifiers --+---+--- codevil | character varying(3) | not null codelang | character varying(3) | not null libvil | character varying(50) | not null trafic | integer | Indexes: pk_vilsitelang PRIMARY KEY, btree (codevil, codelang) Foreign-key constraints: fk_vilsitel_vilsitela_vilsite FOREIGN KEY (codevil) REFERENCES vilsite(codevil) ON DELETE CASCADE -- Guillaume ---(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
Re: [HACKERS] quote_literal(integer) does not exist
On 25/11/2007, Tom Lane [EMAIL PROTECTED] wrote:
Brendan Jurd [EMAIL PROTECTED] writes:
So, I wonder why we don't just adapt the internal function to take
anyelement?
The main argument against is the slippery slope one: once we accept
this, what else? The entire point of that change was to make people
be aware of where they are inducing text coercions, and deciding to
hide that again on the basis of individual complaints is no way to
design a system.
I know so is too late now. This patch can be saved for 8.4.
There is one reason for patching. It is consistence with || operator.
That's all. This patch doesn't change casting rules, and these rules
are strict still (what is good). But quote_literal is more natural
with anyelement parameter, than only text element. There isn't any
reason for some limit. This discus is not about change rules, but
about change of definition of some functions.
As a not-too-far-away example, I see that the proposed patch Pavel
sent in arbitrarily decides to change quote_ident() too, which was
not asked for and has got much less justification than changing
quote_literal(). That sort of cowboy approach to semantics is not
the way to proceed.
Reason is same. Consistence with || operator. But, equivalent in SQL is:
CREATE OR REPLACE FUNCTION quote_ident(anyelement)
RETURNS text AS
SELECT CASE when $1::text LIKE '% %' - first problem
THEN '' || $1 || ''
ELSE $1::text END - second problem
$$ LANGUAGE SQL;
so, I see, with quote_ident I was wrong. I belive so with anyelement
can be more usable, but it is different than quote_literal.
Another issue is that changing pg_proc.h without forcing initdb
is not good practice. We are far enough along in the beta cycle
that we shouldn't force initdb lightly, and I definitely *don't*
want to do it again next week when someone else comes up with
some other must have auto-coercing function. If anyone wants
to make a serious argument for this, look through the whole of
pg_proc.h, see what else needs to be changed at the same time,
and make a coherent proposal.
probably quote_literal is more important than others. It is wide used
with dynamic selects. On other side, any similar problem can be simple
solved with custom wrapper (with some note in release notes).
I found more important problem. I cannot simply use literal in
polymorphic function. I cannot call anyfce('literal') what is
acceptable in SQL or plpgsql languages, but not in C language. Sure,
this topic is for 8.4.
nice a day
Pavel Stehule
regards, tom lane
---(end of broadcast)---
TIP 1: if posting/reading through Usenet, please send an appropriate
subscribe-nomail command to [EMAIL PROTECTED] so that your
message can get through to the mailing list cleanly
---(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
Re: [HACKERS] quote_literal(integer) does not exist
On Nov 26, 2007 5:23 AM, Tom Lane [EMAIL PROTECTED] wrote: Brendan Jurd [EMAIL PROTECTED] writes: So, I wonder why we don't just adapt the internal function to take anyelement? The main argument against is the slippery slope one: once we accept this, what else? The entire point of that change was to make people be aware of where they are inducing text coercions, and deciding to hide that again on the basis of individual complaints is no way to design a system. I'm all for the idea of making people conscious of text coercions in general, but in the *particular* case of quote_literal, having it only accept text is undesirable, unintuitive and most importantly, it will break apps which otherwise may have been able to enjoy a smooth transition to 8.3. I would argue that quote_literal should have been set up to accept anyelement in the very first place, and I'd guess that the original choice of text as an argument type was partially driven by the understanding that everything gets coerced to text, making it a de facto anyelement substitute. Or maybe anyelement wasn't available when it was introduced. Either way, if quote_literal() is all about safely stuffing variables into dynamic queries, the new behaviour is a regression. In context, it makes perfect sense to throw integers, numerics and whatever else at quote_literal and expect it to Just Work. My feeling is that the change in text coercion behaviour has well illuminated that the text argument type for quote_literal isn't ideal. Great! Let's fix it. As a not-too-far-away example, I see that the proposed patch Pavel sent in arbitrarily decides to change quote_ident() too, which was not asked for and has got much less justification than changing quote_literal(). That sort of cowboy approach to semantics is not the way to proceed. I'd pass on changing quote_ident. It seems natural for it to take a text argument. I can imagine a lot of people using, say, quote_literal(int) in the field; I can't imagine the same for quote_ident. Another issue is that changing pg_proc.h without forcing initdb is not good practice. We are far enough along in the beta cycle that we shouldn't force initdb lightly, and I definitely *don't* want to do it again next week when someone else comes up with some other must have auto-coercing function. If anyone wants to make a serious argument for this, look through the whole of pg_proc.h, see what else needs to be changed at the same time, and make a coherent proposal. I took your suggestion and looked through all the procs that take a text argument. I honestly didn't see anything else I thought needed to change. So my proposal is to add your quote_literal(anyelement) SQL function to pg_proc and be done with it. I can see your reluctance to force an initdb, but what's the greater mischief; forcing initdb in beta, or breaking applications on release? My personal perspective is that it's an easy choice ... avoid breaking the apps, that's what betas are for. Thanks for your time, BJ ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [HACKERS] [GENERAL] Empty arrays with ARRAY[]
On Nov 26, 2007 3:58 AM, Martijn van Oosterhout [EMAIL PROTECTED] wrote: On Mon, Nov 26, 2007 at 03:51:37AM +1100, Brendan Jurd wrote: I noticed in the 8.3 release notes that ARRAY(SELECT ...) now returns an empty array if there are no rows returned by the subquery. This has come up before, Tom had an idea about how to fix it: http://groups.google.com/group/pgsql.general/browse_thread/thread/911791e145a17daa/6b035035aeaac399 http://www.mail-archive.com/[EMAIL PROTECTED]/msg90681.html [moving thread to -hackers] Thanks for the link Martijn. I'd be interested in taking a swing at this if nobody else has laid claim. Since that thread died back in January, I'm guessing it's wide open. Regards, BJ ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
Re: [HACKERS] quote_literal(integer) does not exist
On Mon, 26 Nov 2007 06:35:20 +1100 Brendan Jurd wrote: On Nov 26, 2007 5:23 AM, Tom Lane [EMAIL PROTECTED] wrote: I'm all for the idea of making people conscious of text coercions in general, but in the *particular* case of quote_literal, having it only accept text is undesirable, unintuitive and most importantly, it will break apps which otherwise may have been able to enjoy a smooth transition to 8.3. I would argue that quote_literal should have been set up to accept anyelement in the very first place, and I'd guess that the original choice of text as an argument type was partially driven by the understanding that everything gets coerced to text, making it a de facto anyelement substitute. Or maybe anyelement wasn't available when it was introduced. Either way, if quote_literal() is all about safely stuffing variables into dynamic queries, the new behaviour is a regression. In context, it makes perfect sense to throw integers, numerics and whatever else at quote_literal and expect it to Just Work. The problem for me is: we expect and encourage people to do safe programming and now they have to debug their programs and remove some of the safe parts just to make PostgreSQL happy. As you said, that is not, what the average programmer expect. My feeling is that the change in text coercion behaviour has well illuminated that the text argument type for quote_literal isn't ideal. Great! Let's fix it. Yes, Tom Lane is right that the current behavior is broken. But the solution cannot be to exclude anything beside text but instead we should move forward to accept anything (at least, if it's possible). As a not-too-far-away example, I see that the proposed patch Pavel sent in arbitrarily decides to change quote_ident() too, which was not asked for and has got much less justification than changing quote_literal(). That sort of cowboy approach to semantics is not the way to proceed. I'd pass on changing quote_ident. It seems natural for it to take a text argument. I can imagine a lot of people using, say, quote_literal(int) in the field; I can't imagine the same for quote_ident. True. You can't even create a table who's name is just an integer or where the name starts with an integer, so in any way you already have to use quotes and you are aware of the problem. I can see your reluctance to force an initdb, but what's the greater mischief; forcing initdb in beta, or breaking applications on release? My personal perspective is that it's an easy choice ... avoid breaking the apps, that's what betas are for. Yeah, that's what a beta is for. We don't expect to have people running production systems with beta software so it needs an reinstall anyway after the release. Kind regards -- Andreas 'ads' Scherbaum PostgreSQL User Group Germany ---(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
Re: [HACKERS] 8.3devel slower than 8.2 under read-only load
Guillaume Smet [EMAIL PROTECTED] writes: Sure, it's the same queries I posted earlier. My pgbench script is the following: BEGIN select libvil from vilsitelang where codelang='FRA' and codevil='LYO' select TL.motsclesmetatags, TL.descriptifmeta, TL.motcleoverture_l, TL.motcleoverture_c, TL.baselinetheme from themelang TL where TL.codeth = 'ASS' and TL.codelang = 'FRA' SELECT libvilpubwoo, codelang, codepays, petiteville FROM vilsite WHERE codevil = 'LYO' select libvil from vilsitelang where codelang='FRA' and codevil='LYO' END I poked into this a bit, and it seems the extra overhead is all coming from resolving the ambiguous = operators. That didn't show up in my test because my query had int4_column = int4_const which is an exact match to a pg_operator entry. But since your columns are varchar, which doesn't have any operators of its own, we have to go through oper_select_candidate(), which is noticeably slower than before. The slowdown seems to have two causes: 1. Datatype bloat: there are 58 = operators in pg_operator today, versus 54 at the beginning of the year. That's 7% more work right there to sort through the additional operators. 2. Removal of pg_cast entries associated with explicit varchar coercions: when there's not a pg_cast entry for the desired coercion, find_coercion_pathway does a second catalog lookup to see if it might be an array case. That happens more often in this test case than it did at the start of the year, because I got rid of pg_cast entries that could be replaced by the generic CoerceViaIO mechanism. I'm not sure how big a hit #2 really is. Presumably the removal of the redundant entries has some distributed savings associated with it, which would partially counteract the extra lookup; but I don't have any tools that can isolate the cost of those particular SearchSysCache calls out of all the rest. In any case, #2 is specific to varchar and text while effect #1 is an issue for just about everything. The cost of resolving ambiguous operators has been an issue for a long time, of course, but it seems particularly bad in this case --- gprof blames 37% of the runtime on oper_select_candidate(). It might be time to think about caching the results of operator searches somehow. Too late for 8.3 though. regards, tom lane ---(end of broadcast)--- TIP 7: You can help support the PostgreSQL project by donating at http://www.postgresql.org/about/donate
Re: [HACKERS] 8.3devel slower than 8.2 under read-only load
On Nov 26, 2007 1:35 AM, Tom Lane [EMAIL PROTECTED] wrote: The cost of resolving ambiguous operators has been an issue for a long time, of course, but it seems particularly bad in this case --- gprof blames 37% of the runtime on oper_select_candidate(). It might be time to think about caching the results of operator searches somehow. Too late for 8.3 though. From what you say, I understand we can't even find a workaround for 8.3 to improve the situation while waiting for a cleaner solution in 8.4+? At least, I'm glad we finally found an explanation for this problem. Thanks. -- Guillaume ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
Re: [HACKERS] 8.3devel slower than 8.2 under read-only load
On Nov 26, 2007 1:54 AM, Guillaume Smet [EMAIL PROTECTED] wrote: From what you say, I understand we can't even find a workaround for 8.3 to improve the situation while waiting for a cleaner solution in 8.4+? To explain the reasons why I'm so worried, I should have explained that all the primary keys of this particular database are varchar... So it's really the worst case for this problem. -- Guillaume ---(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
Re: [HACKERS] quote_literal(integer) does not exist
Brendan Jurd [EMAIL PROTECTED] writes:
On Nov 26, 2007 5:23 AM, Tom Lane [EMAIL PROTECTED] wrote:
... If anyone wants
to make a serious argument for this, look through the whole of
pg_proc.h, see what else needs to be changed at the same time,
and make a coherent proposal.
I took your suggestion and looked through all the procs that take a
text argument. I honestly didn't see anything else I thought needed
to change.
So my proposal is to add your quote_literal(anyelement) SQL function
to pg_proc and be done with it.
I did the same search. Ignoring cases where it's fairly obvious that
you're trying to apply a textual operation to something non-textual
(eg, LIKE and btrim(), though both of these have been complained of
since beta started...), it seems that quote_literal() has a good case,
and you could also make an argument for allowing a non-text second
argument for set_config(), since these things used to work:
regression=# select set_config('work_mem', 1000, false);
set_config
1000kB
(1 row)
regression=# select set_config('random_page_cost', 2.5, false);
set_config
2.5
(1 row)
I don't find that amazingly compelling, but it's not silly either.
If we were to do this I think I'd introduce set_config(text,float8,bool)
rather than going all the way with anyelement, though. That would be
enough to cover both the int and float cases, as well as anyone who
likes to spell booleans as 1 and 0.
I don't offhand see anything else I'd consider weakening the casting
rules for. If anyone else is interested, I took
select p.oid::regprocedure as regprocedure, oprname from pg_proc p left
join pg_operator o on (oprcode = p.oid) where 25 = any (proargtypes)
and removed duplicates and obviously-internal functions such as textout,
leaving me with this list:
regprocedure| oprname
---+-
array_to_string(anyarray,text)|
btrim(text) |
btrim(text,text) |
character_length(text)|
convert_to(text,name) |
initcap(text) |
length(text) |
lower(text) |
lpad(text,integer)|
lpad(text,integer,text) |
ltrim(text) |
ltrim(text,text) |
md5(text) |
octet_length(text)|
overlay(text,text,integer)|
overlay(text,text,integer,integer)|
position(text,text) |
quote_ident(text) |
quote_literal(text) |
regexp_matches(text,text) |
regexp_matches(text,text,text)|
regexp_replace(text,text,text)|
regexp_replace(text,text,text,text) |
regexp_split_to_array(text,text) |
regexp_split_to_array(text,text,text) |
regexp_split_to_table(text,text) |
regexp_split_to_table(text,text,text) |
repeat(text,integer) |
replace(text,text,text) |
rpad(text,integer)|
rpad(text,integer,text) |
rtrim(text) |
rtrim(text,text) |
set_config(text,text,boolean) |
split_part(text,text,integer) |
string_to_array(text,text)|
strpos(text,text) |
substr(text,integer) |
substr(text,integer,integer) |
substring(text,integer) |
substring(text,integer,integer) |
substring(text,text) |
substring(text,text,text) |
texticlike(text,text) | ~~*
texticnlike(text,text)| !~~*
texticregexeq(text,text) | ~*
texticregexne(text,text) | !~*
textlike(text,text) | ~~
textnlike(text,text)
Re: [HACKERS] 8.3devel slower than 8.2 under read-only load
Tom Lane [EMAIL PROTECTED] writes: But since your columns are varchar, which doesn't have any operators of its own, we have to go through oper_select_candidate() I wonder whether at some point we shouldn't just eliminate this distinction entirely. Just make text and varchar the same type and spell it text when there's no typmod length restriction and varchar(x) when there is. 1. Datatype bloat: there are 58 = operators in pg_operator today, versus 54 at the beginning of the year. That's 7% more work right there to sort through the additional operators. That's particularly scary because it means that databases which load piles of contrib modules have that much more of an effect here. Some contrib modules create a *lot* of operators. -- Gregory Stark EnterpriseDB http://www.enterprisedb.com Ask me about EnterpriseDB's RemoteDBA services! ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
Re: [HACKERS] 8.3devel slower than 8.2 under read-only load
Gregory Stark [EMAIL PROTECTED] writes: Tom Lane [EMAIL PROTECTED] writes: But since your columns are varchar, which doesn't have any operators of its own, we have to go through oper_select_candidate() I wonder whether at some point we shouldn't just eliminate this distinction entirely. Just make text and varchar the same type and spell it text when there's no typmod length restriction and varchar(x) when there is. I've thought about that more than once, but I'm worried that it would eliminate one of the few heavily-used cases we have for binary-compatible operations, thereby making it even harder to find performance issues for those situations. In any case, it wouldn't do anything to fix the basic problem that ambiguous-operator resolution is expensive when there are lots of similarly-named operators. We've chipped away at that with various hacks over the years, but I don't think it's ever occurred to us (or at least to me) before to try short-circuiting the entire process through a lookaside cache. We'd probably need to flush the cache on changes in pg_operator or pg_cast, but neither of those change often ... regards, tom lane ---(end of broadcast)--- TIP 7: You can help support the PostgreSQL project by donating at http://www.postgresql.org/about/donate
Re: [HACKERS] quote_literal(integer) does not exist
Tom Lane [EMAIL PROTECTED] writes: Thoughts? I wouldn't have thought set_config was frequently enough used to warrant a catalog change but quote_literal does seem like a common enough use case to warrant it and once we're doing it there... I started to think md5(bytea) would be reasonable but then I checked and we do in fact already have that. Perhaps it would be interesting to have some of the other functions like overlay or replace for bytea as well in the future but I hardly think anyone is going to complain urgently given that using them via free casts to text would never have really worked that well previously. -- Gregory Stark EnterpriseDB http://www.enterprisedb.com Ask me about EnterpriseDB's RemoteDBA services! ---(end of broadcast)--- TIP 7: You can help support the PostgreSQL project by donating at http://www.postgresql.org/about/donate
Re: [HACKERS] quote_literal(integer) does not exist
Tom Lane wrote: [...] it seems that quote_literal() has a good case, and you could also make an argument for allowing a non-text second argument for set_config() [...] Thoughts? I think there is just enough of a case for quote_literal(), although in my experience the vast majority of places where it is used in fact should be replaced by using placeholders. I find that I need to use it very rarely indeed. The case for set_config() is less compelling. cheers andrew ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
