Re: [PATCHES] Proposed patch to make mergejoin cost estimationmore symmetric

2007-12-07 Thread Gregory Stark
"Simon Riggs" <[EMAIL PROTECTED]> writes:

> I think we all accept that you're the master here. The question is how
> will we know to report problems to you? 

Hm, I think I agree. The problem is where to draw the line. Ultimately
everything in the statistics tables and more could potentially be relevant.

The other problem is that currently our explain output is a bit of a hack.
It's just text we print out and we're limited in how much data we can put in
that without it being unwieldy.

When we get the table-based or xml-based or some other machine-readable
explain plan we could stuff a lot more data in there and have it be the UI's
responsibility to decide what data to display.

When that happens it would be nice to have the raw data used to generate the
cost estimations. At least the most important factors.

-- 
  Gregory Stark
  EnterpriseDB  http://www.enterprisedb.com
  Ask me about EnterpriseDB's RemoteDBA services!

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


[PATCHES] [PATCH] automatic integer conversion

2007-12-07 Thread xeb
Hello!
Here is patch which makes libpq more confortable when working with binary 
integers. 
Automatic conversion intialized by PQsetconvertint(conn,1), so old applications 
continues to work proper.


--
diff -ur postgresql-8.2.4.orig/src/interfaces/libpq/exports.txt 
postgresql-8.2.4/src/interfaces/libpq/exports.txt
--- postgresql-8.2.4.orig/src/interfaces/libpq/exports.txt  2006-08-18 
23:52:39.0 +0400
+++ postgresql-8.2.4/src/interfaces/libpq/exports.txt   2007-11-26 
16:23:25.0 +0300
@@ -136,3 +136,6 @@
 PQdescribePortal  134
 PQsendDescribePrepared135
 PQsendDescribePortal  136
+PQsetconvertint   137
+PQisconverrtint   138
+PQexecPreparedParams  139
diff -ur postgresql-8.2.4.orig/src/interfaces/libpq/fe-exec.c 
postgresql-8.2.4/src/interfaces/libpq/fe-exec.c
--- postgresql-8.2.4.orig/src/interfaces/libpq/fe-exec.c2006-10-04 
04:30:13.0 +0400
+++ postgresql-8.2.4/src/interfaces/libpq/fe-exec.c 2007-11-26 
16:20:28.0 +0300
@@ -12,16 +12,21 @@
  *
  *-
  */
+#include "postgres.h"
 #include "postgres_fe.h"

 #include 
 #include 
+#include 
+#include 

 #include "libpq-fe.h"
 #include "libpq-int.h"

 #include "mb/pg_wchar.h"

+#include "catalog/pg_type.h"
+
 #ifdef WIN32
 #include "win32.h"
 #else
@@ -879,6 +884,43 @@
   resultFormat);
 }

+
+/*
+ * PQsendQueryPreparedParams
+ * Like PQsendQuery, but execute a previously prepared statement,
+ * parameters passed as helpers for integer converting
+ */
+int
+PQsendQueryPreparedParams(PGconn *conn,
+   const char *stmtName,
+   int nParams,
+   const Oid *paramTypes,
+   const char *const * paramValues,
+   const int *paramLengths,
+   const int *paramFormats,
+   int resultFormat)
+{
+   if (!PQsendQueryStart(conn))
+   return 0;
+
+   if (!stmtName)
+   {
+   printfPQExpBuffer(&conn->errorMessage,
+   libpq_gettext("statement name is a null 
pointer\n"));
+   return 0;
+   }
+
+   return PQsendQueryGuts(conn,
+  NULL,/* no command 
to parse */
+  stmtName,
+  nParams,
+  paramTypes,
+  paramValues,
+  paramLengths,
+  paramFormats,
+  resultFormat);
+}
+
 /*
  * Common startup code for PQsendQuery and sibling routines
  */
@@ -1003,6 +1045,13 @@
if (paramValues && paramValues[i])
{
int nbytes;
+   char const *val;
+   union
+   {
+   uint64 i64;
+   uint32 i32;
+   uint16 i16;
+   } tmpint;

if (paramFormats && paramFormats[i] != 0)
{
@@ -1021,8 +1070,33 @@
/* text parameter, do not use paramLengths */
nbytes = strlen(paramValues[i]);
}
+   #if __BYTE_ORDER == __LITTLE_ENDIAN
+   if (conn->convert_int && paramTypes &&
+   (paramTypes[i]==INT2OID || paramTypes[i]==INT4OID ||
+paramTypes[i]==INT8OID))
+   {
+   switch(nbytes)
+   {
+   case 2:
+   
tmpint.i16=bswap_16(*(uint16*)paramValues[i]);
+   val=(char*)&tmpint;
+   break;
+   case 4:
+   
tmpint.i32=bswap_32(*(uint32*)paramValues[i]);
+   val=(char*)&tmpint;
+   break;
+   case 8:
+   
tmpint.i64=bswap_64(*(uint64*)paramValues[i]);
+   val=(char*)&tmpint;
+   break;
+   default:
+   val=paramValues[i];
+   }
+   }els

Re: [PATCHES] Proposed patch to make mergejoin cost estimation more symmetric

2007-12-07 Thread Tom Lane
Alvaro Herrera <[EMAIL PROTECTED]> writes:
> Yes, but how can you tell by looking at the explain?  I think the point
> is that the "fraction that would be skipped" should be reported somehow.

It is: it's directly reflected in the startup cost.  Previously, a
mergejoin would always have startup cost equal to the sum of its
input startup costs (hence, zero in the cases of interest here).

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: [PATCHES] Proposed patch to make mergejoin cost estimation more symmetric

2007-12-07 Thread Simon Riggs
On Fri, 2007-12-07 at 09:48 -0500, Tom Lane wrote:
> Simon Riggs <[EMAIL PROTECTED]> writes:
> > I do have a longer term issue that there is no information provided by
> > EXPLAIN to allow us to differentiate these two conditions.
> 
> Sure there is: the new startup condition affects the estimated plan
> startup cost (only) and the existing termination condition affects the
> estimated total cost (only).

I think we all accept that you're the master here. The question is how
will we know to report problems to you? If we all get used to the idea
that sometimes the numbers vary, then we're in the situation where we
have to say to people "its magic". That destroys any feedback loop you
have for problem reporting and then you get out of touch with what is
happening on the ground. 

It's a long term issue, so I have no complaints about what you've done,
its just something to think about. I've been exactly here before in one
of my past lives and I don't want to reinvent that particular wheel.

-- 
  Simon Riggs
  2ndQuadrant  http://www.2ndQuadrant.com


---(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: [PATCHES] Proposed patch to make mergejoin cost estimation more symmetric

2007-12-07 Thread Alvaro Herrera
Tom Lane wrote:
> Simon Riggs <[EMAIL PROTECTED]> writes:
> > I do have a longer term issue that there is no information provided by
> > EXPLAIN to allow us to differentiate these two conditions.
> 
> Sure there is: the new startup condition affects the estimated plan
> startup cost (only) and the existing termination condition affects the
> estimated total cost (only).

Yes, but how can you tell by looking at the explain?  I think the point
is that the "fraction that would be skipped" should be reported somehow.

-- 
Alvaro Herrera  http://www.amazon.com/gp/registry/5ZYLFMCVHXC
Criptografía: Poderosa técnica algorítmica de codificación que es
empleada en la creación de manuales de computadores.

---(end of broadcast)---
TIP 4: Have you searched our list archives?

   http://archives.postgresql.org


Re: [PATCHES] Proposed patch to make mergejoin cost estimation more symmetric

2007-12-07 Thread Tom Lane
Gregory Stark <[EMAIL PROTECTED]> writes:
> What about a merge join against an empty table? I suppose there would just be
> no statistics?

Yeah.  The defenses against silly results in mergejoinscansel should be
enough to prevent it from doing anything really insane.

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: [PATCHES] Proposed patch to make mergejoin cost estimation more symmetric

2007-12-07 Thread Tom Lane
Simon Riggs <[EMAIL PROTECTED]> writes:
> I do have a longer term issue that there is no information provided by
> EXPLAIN to allow us to differentiate these two conditions.

Sure there is: the new startup condition affects the estimated plan
startup cost (only) and the existing termination condition affects the
estimated total cost (only).

regards, tom lane

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


Re: [PATCHES] Proposed patch to make mergejoin cost estimation more symmetric

2007-12-07 Thread Gregory Stark
"Tom Lane" <[EMAIL PROTECTED]> writes:

> Any objections to applying the patch?

I like applying it.

You don't include any negative tests and corner cases as well; cases where the
new code shouldn't be disturbing the results such as a symmetric join or a
join against a single-row table. The comments make me think you ran them but
just didn't show them though.

What about a merge join against an empty table? I suppose there would just be
no statistics?


-- 
  Gregory Stark
  EnterpriseDB  http://www.enterprisedb.com
  Ask me about EnterpriseDB's 24x7 Postgres support!

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


Re: [PATCHES] Proposed patch to make mergejoin cost estimation more symmetric

2007-12-07 Thread Simon Riggs
On Thu, 2007-12-06 at 19:19 -0500, Tom Lane wrote:

> Since something needs to be done to that code anyway, I'm considering
> applying the attached patch.  It's a bit larger than I would normally
> consider to be a good idea for an optional patch in late beta.  But then
> again I wouldn't have the slightest hesitation about back-patching a
> change of this size after final release, so it seems attractive to put
> it in now.
> 
> Aside from the patch, I have attached a test script that exercises merge
> join planning for some simple cases, and the plans output by the script
> in CVS HEAD with/without the patch.  The cost estimates with the patch
> are in line with expectation, the estimates without, not so much.
> In particular, the existing bug can be seen at work here in that the
> sixth and eighth test cases ("big join highm on b=h order by b desc" and
> "big join high on b=h order by b desc") are given unreasonably small
> cost estimates by the unpatched code.  (Note: the two sets of numbers
> vary a bit because they were working with nonidentical ANALYZE
> statistics.)

Looks good.

> Any objections to applying the patch?

None.



I do have a longer term issue that there is no information provided by
EXPLAIN to allow us to differentiate these two conditions. That makes it
harder to understand the basis of the plans and also gets everybody used
to seeing EXPLAINs that can't easily be explained, which leads to people
not reporting problems that exist. I doubt we can fix anything now, but
increased debugging/logging output is definitely required in some form.

> explain select * from big join highm on b=h order by b desc;
>   QUERY PLAN  
>  
> ---
>  Merge Join  (cost=298.67..387.53 rows=1000 width=8)
>Merge Cond: (big.b = highm.h)
>->  Index Scan Backward using bigi on big  (cost=0.00..3050.26 rows=10 
> width=4)
>->  Index Scan Backward using highmi on highm  (cost=0.00..43.25 rows=1000 
> width=4)
> (4 rows)

> explain select * from big join high on b=h order by b desc;
>   QUERY PLAN  
>  
> ---
>  Merge Join  (cost=0.05..88.19 rows=1000 width=8)
>Merge Cond: (big.b = high.h)
>->  Index Scan Backward using bigi on big  (cost=0.00..3050.26 rows=10 
> width=4)
>->  Index Scan Backward using highi on high  (cost=0.00..43.25 rows=1000 
> width=4)
> (4 rows)

-- 
  Simon Riggs
  2ndQuadrant  http://www.2ndQuadrant.com


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

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


Re: [PATCHES] A minor typo fix on pg_standby docs

2007-12-07 Thread Neil Conway
Applied, thanks.

-Neil

On Fri, 2007-12-07 at 18:48 +1100, FAST PostgreSQL wrote:
> Rgds,
> Arul Shaji
> ---(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


---(end of broadcast)---
TIP 6: explain analyze is your friend


Re: [PATCHES] A minor typo fix on pg_standby docs

2007-12-07 Thread Simon Riggs
On Fri, 2007-12-07 at 18:48 +1100, FAST PostgreSQL wrote:

Yep, thats an error. Thanks for patching.

-- 
  Simon Riggs
  2ndQuadrant  http://www.2ndQuadrant.com


---(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