Re: [PATCHES] [HACKERS] Interval aggregate regression failure (expected seems
On Aug 26, 2006, at 11:40 , Bruce Momjian wrote:
I used your ideas to make a patch to fix your example:
test= select '41 months'::interval / 10;
?column?
---
4 mons 3 days
(1 row)
and
test= select '41 months'::interval * 0.3;
?column?
---
1 year 9 days
(1 row)
The trick was not to play with the division, but to check if the
number
of seconds cascaded into full days and/or months.
While this does provide a fix for the example, I don't believe it's a
complete solution. For example, with your patch, you also get the
following results:
select '41 mon 360:00'::interval / 10 as pos
, '-41 mon -360:00'::interval / 10 as neg;
pos | neg
+--
4 mons 2 days 60:00:00 | -4 mons -2 days -59:59:60.00
(1 row)
If I've done the math right, this should be:
4 mons 3 days 36:00:00 | -4 mons -3 days -36:00:00
select '41 mon -360:00'::interval / 10 as pos
, '-41 mon 360:00'::interval / 10 as neg;
pos |neg
-+---
4 mons 2 days -12:00:00 | -4 mons -2 days +12:00:00
(1 row)
Should be:
4 mons 3 days -36:00:00 | -4 mons -3 days +36:00:00
What we want to do is check just the month contribution to the day
component to see if it is greater than 24 hours. Perhaps the simplest
way to accomplish this is something like (psuedo code):
if (abs(tsround(month_remainder * SECS_PER_DAY)) == SECS_PER_DAY)
{
if (month_remainder 0)
{
result-month++;
}
else
{
result-month--;
}
}
I'm going to try something along these lines this evening.
FWIW, I've included the patch of for what I'm working on. It's pretty
heavily commented right now with expected results as I think through
what the code's doing. (It also includes the DecodeInterval patch I
sent to -patches earlier today.) I'm still getting overflow warnings
in the lines including USECS_PER_DAY and USECS_PER_MONTH, and my
inexperience with C and gdb is getting the best of me right now
(though I'm still plugging away: ).
Michael Glaesemann
grzm seespotcode net
8---
? CONFIGURE_ARGS
? datetime.patch
? timestamp.patch
? src/backend/.DS_Store
? src/include/.DS_Store
? src/test/.DS_Store
Index: src/backend/utils/adt/datetime.c
===
RCS file: /projects/cvsroot/pgsql/src/backend/utils/adt/datetime.c,v
retrieving revision 1.169
diff -c -r1.169 datetime.c
*** src/backend/utils/adt/datetime.c25 Jul 2006 03:51:21 - 1.169
--- src/backend/utils/adt/datetime.c28 Aug 2006 07:08:46 -
***
*** 2920,2935
tm-tm_mday += val * 7;
if (fval != 0)
{
! int
sec;
!
! fval *= 7 *
SECS_PER_DAY;
! sec = fval;
! tm-tm_sec += sec;
#ifdef HAVE_INT64_TIMESTAMP
! *fsec += (fval - sec) *
100;
#else
! *fsec += fval - sec;
#endif
}
tmask = (fmask DTK_M(DAY)) ?
0 : DTK_M(DAY);
break;
--- 2920,2942
tm-tm_mday += val * 7;
if (fval != 0)
{
! int extra_days;
! fval *= 7;
! extra_days = (int32)
fval;
! tm-tm_mday +=
extra_days;
! fval -= extra_days;
! if (fval != 0)
! {
! int
sec;
! fval *=
SECS_PER_DAY;
! sec = fval;
! tm-tm_sec +=
sec;
#ifdef HAVE_INT64_TIMESTAMP
! *fsec += (fval
- sec) * 100;
#else
!
Re: [PATCHES] [HACKERS] Performance testing of COPY (SELECT) TO
Hi, what's the problem with COPY view TO, other than you don't like it? :-) That was the beginning and is used in production according to the original authors. I also broke the check for a FOR UPDATE clause. Not sure where but it must be easy to fix :-) I'd do it myself but I'm heading to bed right now. Fixed. I also wanted to check these hunks in your patch, which I didn't like very much: -ERROR: column a of relation test does not exist +ERROR: column a does not exist It was because of too much code sharing. I fixed it by passing the relation name to CopyGetAttnums() in the relation case, so the other regression tests aren't bothered now. The docs and the regression test is modified according to your version. Best regards, Zoltán Böszörményi pgsql-copyselect-10.patch.gz Description: GNU Zip compressed data ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
Re: [HACKERS] [PATCHES] Trivial patch to double vacuum speed on tables with no indexes
Tom Lane [EMAIL PROTECTED] writes:
The reason the patch is so short is that it's a kluge. If we really
cared about supporting this case, more wide-ranging changes would be
needed (eg, there's no need to eat maintenance_work_mem worth of RAM
for the dead-TIDs array); and a decent respect to the opinions of
mankind would require some attention to updating the header comments
and function descriptions, too.
The only part that seems klugy to me is how it releases the lock and
reacquires it rather than wait in the first place until it can acquire the
lock. Fixed that and changed lazy_space_alloc to allocate only as much space
as is really necessary.
Gosh, I've never been accused of offending all mankind before.
--- vacuumlazy.c31 Jul 2006 21:09:00 +0100 1.76
+++ vacuumlazy.c28 Aug 2006 09:58:41 +0100
@@ -16,6 +16,10 @@
* perform a pass of index cleanup and page compaction, then resume the heap
* scan with an empty TID array.
*
+ * As a special exception if we're processing a table with no indexes we can
+ * vacuum each page as we go so we don't need to allocate more space than
+ * enough to hold as many heap tuples fit on one page.
+ *
* We can limit the storage for page free space to MaxFSMPages entries,
* since that's the most the free space map will be willing to remember
* anyway. If the relation has fewer than that many pages with free space,
@@ -106,7 +110,7 @@
TransactionId
OldestXmin);
static BlockNumber count_nondeletable_pages(Relation onerel,
LVRelStats *vacrelstats,
TransactionId OldestXmin);
-static void lazy_space_alloc(LVRelStats *vacrelstats, BlockNumber relblocks);
+static void lazy_space_alloc(LVRelStats *vacrelstats, BlockNumber relblocks,
unsigned nindexes);
static void lazy_record_dead_tuple(LVRelStats *vacrelstats,
ItemPointer itemptr);
static void lazy_record_free_space(LVRelStats *vacrelstats,
@@ -206,7 +210,8 @@
* This routine sets commit status bits, builds lists of dead
tuples
* and pages with free space, and calculates statistics on the
number
* of live tuples in the heap. When done, or when we run low on
space
- * for dead-tuple TIDs, invoke vacuuming of indexes and heap.
+ * for dead-tuple TIDs, or after every page if the table has no
indexes
+ * invoke vacuuming of indexes and heap.
*
* It also updates the minimum Xid found anywhere on the table in
* vacrelstats-minxid, for later storing it in pg_class.relminxid.
@@ -247,7 +252,7 @@
vacrelstats-rel_pages = nblocks;
vacrelstats-nonempty_pages = 0;
- lazy_space_alloc(vacrelstats, nblocks);
+ lazy_space_alloc(vacrelstats, nblocks, nindexes);
for (blkno = 0; blkno nblocks; blkno++)
{
@@ -282,8 +287,14 @@
buf = ReadBuffer(onerel, blkno);
- /* In this phase we only need shared access to the buffer */
- LockBuffer(buf, BUFFER_LOCK_SHARE);
+ /* In this phase we only need shared access to the buffer
unless we're
+* going to do the vacuuming now which we do if there are no
indexes
+*/
+
+ if (nindexes)
+ LockBuffer(buf, BUFFER_LOCK_SHARE);
+ else
+ LockBufferForCleanup(buf);
page = BufferGetPage(buf);
@@ -450,6 +461,12 @@
{
lazy_record_free_space(vacrelstats, blkno,
PageGetFreeSpace(page));
+ } else if (!nindexes) {
+ /* If there are no indexes we can vacuum the page right
now instead
+* of doing a second scan */
+ lazy_vacuum_page(onerel, blkno, buf, 0, vacrelstats);
+ lazy_record_free_space(vacrelstats, blkno,
PageGetFreeSpace(BufferGetPage(buf)));
+ vacrelstats-num_dead_tuples = 0;
}
/* Remember the location of the last page with nonremovable
tuples */
@@ -891,16 +908,20 @@
* See the comments at the head of this file for rationale.
*/
static void
-lazy_space_alloc(LVRelStats *vacrelstats, BlockNumber relblocks)
+lazy_space_alloc(LVRelStats *vacrelstats, BlockNumber relblocks, unsigned
nindexes)
{
longmaxtuples;
int maxpages;
- maxtuples = (maintenance_work_mem * 1024L) / sizeof(ItemPointerData);
- maxtuples = Min(maxtuples, INT_MAX);
- maxtuples = Min(maxtuples, MaxAllocSize / sizeof(ItemPointerData));
- /* stay sane if small maintenance_work_mem */
- maxtuples = Max(maxtuples, MaxHeapTuplesPerPage);
+ if (nindexes) {
+
Re: [PATCHES] [HACKERS] CSStorm occurred again by postgreSQL8.2
Bruce Momjian [EMAIL PROTECTED] wrote: Is there anything to do for 8.2 here? I'm working on Tom's idea. It is not a feature and does not change the on-disk-structures, so I hope it meet the 8.2 deadline... Tom Lane [EMAIL PROTECTED] wrote: I'm wondering about doing something similar to what TransactionIdIsInProgress does, ie, make use of the PGPROC lists of live subtransactions. Suppose that GetSnapshotData copies not only top xids but live subxids into the snapshot, and adds a flag indicating whether the subxids are complete (ie, none of the subxid lists have overflowed). Then if the flag is set, tqual.c doesn't need to do SubTransGetTopmostTransaction() before searching the list. I added a subxid array to Snapshot and running subxids are gathered from PGPROC-subxids cache. There are two overflowed case; any of PGPROC-subxids are overflowed or the number of total subxids exceeds pre-allocated buffers. If overflowed, we cannot avoid to call SubTransGetTopmostTransaction. I tested the patch and did not see any context switch storm which comes from pg_subtrans, but there may be some bugs in the visibility checking. It would be very nice if you could review or test the patch. Regards, --- ITAGAKI Takahiro NTT Open Source Software Center snapshot_subtrans.patch Description: Binary data ---(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
[PATCHES] updated patch for selecting large results sets in psql using cursors
Hi there,
here comes the latest version (version 7) of the patch to handle large
result sets with psql. As previously discussed, a cursor is used
for SELECT queries when \set FETCH_COUNT some_value 0
(defaults to 100 if FETCH_COUNT is set with no value).
Comparing to the previous version, the patch actually got smaller and is
less invasive, because I doesn't have to deal with a new command and
can share some more code with SendQuery() in common.c.
Bye,
Chris.
--
Chris Mair
http://www.1006.org
diff -rc pgsql.original/doc/src/sgml/ref/psql-ref.sgml pgsql/doc/src/sgml/ref/psql-ref.sgml
*** pgsql.original/doc/src/sgml/ref/psql-ref.sgml 2006-08-28 16:01:15.0 +0200
--- pgsql/doc/src/sgml/ref/psql-ref.sgml 2006-08-28 16:04:46.0 +0200
***
*** 2008,2013
--- 2008,2030
/varlistentry
varlistentry
+ termvarnameFETCH_COUNT/varname/term
+ listitem
+ para
+ If this variable is set to an integer value gt; 0,
+ all commandSELECT/command or commandVALUES/command
+ queries are performed using a cursor. Therefore only a
+ limited amount of memory is used, regardless the size of
+ the result set. The integer value defines the fetch count.
+ It defaults to literal100/literal. This variable can be
+ used whenever a result set needs to be retrieved that exceeds
+ the client's memory resources. Output is always unaligned
+ and uses the current field separator (see command\pset/command).
+ /para
+ /listitem
+ /varlistentry
+
+ varlistentry
termvarnameHISTCONTROL/varname/term
listitem
para
diff -rc pgsql.original/src/bin/psql/common.c pgsql/src/bin/psql/common.c
*** pgsql.original/src/bin/psql/common.c 2006-08-28 16:01:18.0 +0200
--- pgsql/src/bin/psql/common.c 2006-08-28 16:04:07.0 +0200
***
*** 28,33
--- 28,34
#include command.h
#include copy.h
#include mb/pg_wchar.h
+ #include mbprint.h
/* Workarounds for Windows */
***
*** 52,59
((T)-millitm - (U)-millitm))
#endif
!
static bool command_no_begin(const char *query);
/*
* Safe wrapper around strdup()
--- 53,61
((T)-millitm - (U)-millitm))
#endif
! static bool is_select_command(const char *query);
static bool command_no_begin(const char *query);
+ static bool SendQueryUsingCursor(const char *query);
/*
* Safe wrapper around strdup()
***
*** 827,832
--- 829,841
SetCancelConn();
+ /* if FETCH_COUNT is set 0 and this is a select query, use
+ * alternative query/output code for large result sets
+ */
+ if (GetVariableNum(pset.vars, FETCH_COUNT, -1, DEFAULT_FETCH_COUNT, false) 0 is_select_command(query)) {
+ return SendQueryUsingCursor(query);
+ }
+
transaction_status = PQtransactionStatus(pset.db);
if (transaction_status == PQTRANS_IDLE
***
*** 952,957
--- 961,1131
/*
+ * SendQueryUsingCursor:
+ * This is called by SendQuery for SELECT queries when FETCH_COUNT is set 0.
+ * The query is performed using a cursor, so that large result sets exceeding
+ * the client's RAM size can be dealt with.
+ *
+ * Unlike with SendQuery(), timing and format settings (except delimiters)
+ * are NOT honoured.
+ *
+ * Returns true if the query executed successfully, false otherwise.
+ */
+ static bool
+ SendQueryUsingCursor(const char *query)
+ {
+ PGresult *results;
+ PQExpBufferData buf;
+ FILE*queryFout_copy = NULL;
+ boolqueryFoutPipe_copy = false;
+ bool started_txn = false;
+ intntuples, nfields = -1;
+ inti, j;
+ charfetch_str[64];
+
+ /* prepare to write output to \g argument, if any */
+ if (pset.gfname)
+ {
+ queryFout_copy = pset.queryFout;
+ queryFoutPipe_copy = pset.queryFoutPipe;
+
+ pset.queryFout = stdout;/* so it doesn't get closed */
+
+ /* open file/pipe */
+ if (!setQFout(pset.gfname))
+ {
+ pset.queryFout = queryFout_copy;
+ pset.queryFoutPipe = queryFoutPipe_copy;
+ ResetCancelConn();
+ return false;
+ }
+ }
+
+ /* if we're not in a transaction, start one */
+ if (PQtransactionStatus(pset.db) == PQTRANS_IDLE)
+ {
+ results = PQexec(pset.db, BEGIN);
+ if (PQresultStatus(results) != PGRES_COMMAND_OK)
+ goto error;
+
+ PQclear(results);
+ started_txn = true;
+ }
+
+ initPQExpBuffer(buf);
+ appendPQExpBuffer(buf,
+ DECLARE _psql_cursor NO SCROLL CURSOR FOR %s,
+ query);
+
+ results = PQexec(pset.db, buf.data);
+ if (PQresultStatus(results) != PGRES_COMMAND_OK)
+ goto error;
+
+ PQclear(results);
+ termPQExpBuffer(buf);
+
+ snprintf(fetch_str, sizeof(fetch_str), FETCH FORWARD %d FROM _psql_cursor,
+ GetVariableNum(pset.vars, FETCH_COUNT, -1, DEFAULT_FETCH_COUNT, false));
+
+ for (;;)
+ {
+ /* get FETCH_COUNT tuples at a time */
+ results = PQexec(pset.db, fetch_str);
Re: [PATCHES] [HACKERS] Performance testing of COPY (SELECT) TO
Böszörményi Zoltán wrote: Hi, what's the problem with COPY view TO, other than you don't like it? :-) The problem is that it required a ugly piece of code. Not supporting it means we can keep the code nice. The previous discussion led to this conclusion anyway so I don't know why we are debating it again. -- Alvaro Herrerahttp://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc. ---(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] [PATCHES] Trivial patch to double vacuum speed on tables with no indexes
Gregory Stark wrote: Tom Lane [EMAIL PROTECTED] writes: The reason the patch is so short is that it's a kluge. If we really cared about supporting this case, more wide-ranging changes would be needed (eg, there's no need to eat maintenance_work_mem worth of RAM for the dead-TIDs array); and a decent respect to the opinions of mankind would require some attention to updating the header comments and function descriptions, too. The only part that seems klugy to me is how it releases the lock and reacquires it rather than wait in the first place until it can acquire the lock. Fixed that and changed lazy_space_alloc to allocate only as much space as is really necessary. Gosh, I've never been accused of offending all mankind before. Does that feel good or bad? I won't comment on the spirit of the patch but I'll observe that you should respect mankind a little more by observing brace position in if/else ;-) -- Alvaro Herrerahttp://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc. ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [PATCHES] [HACKERS] Performance testing of COPY (SELECT) TO
Alvaro Herrera wrote: Böszörményi Zoltán wrote: Hi, what's the problem with COPY view TO, other than you don't like it? :-) The problem is that it required a ugly piece of code. Not supporting it means we can keep the code nice. The previous discussion led to this conclusion anyway so I don't know why we are debating it again. What is so ugly about it? I haven't looked at the code, but I am curious to know. I also don't recall the consensus being quite so clear cut. I guess there is a case for saying that if it's not allowed then you know that COPY relname TO is going to be fast. But, code aesthetics aside, the reasons for disallowing it seem a bit thin, to me. cheers andrew ---(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] [HACKERS] Performance testing of COPY (SELECT) TO
Andrew Dunstan írta: Alvaro Herrera wrote: Böszörményi Zoltán wrote: Hi, what's the problem with COPY view TO, other than you don't like it? :-) The problem is that it required a ugly piece of code. Not supporting it means we can keep the code nice. The previous discussion led to this conclusion anyway so I don't know why we are debating it again. What is so ugly about it? I haven't looked at the code, but I am curious to know. I also don't recall the consensus being quite so clear cut. I guess there is a case for saying that if it's not allowed then you know that COPY relname TO is going to be fast. But, code aesthetics aside, the reasons for disallowing it seem a bit thin, to me. cheers andrew I would say the timing difference between COPY table TO and COPY (SELECT * FROM table) TO was noise, so it's not even faster. And an updatable VIEW *may* allow COPY view FROM... Best regards, Zoltán Böszörményi ---(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: [PATCHES] [HACKERS] Performance testing of COPY (SELECT) TO
Zoltan Boszormenyi wrote: Andrew Dunstan írta: Alvaro Herrera wrote: Böszörményi Zoltán wrote: what's the problem with COPY view TO, other than you don't like it? :-) The problem is that it required a ugly piece of code. Not supporting it means we can keep the code nice. The previous discussion led to this conclusion anyway so I don't know why we are debating it again. What is so ugly about it? I haven't looked at the code, but I am curious to know. It used a SELECT * FROM %s string that was passed back to the parser. I also don't recall the consensus being quite so clear cut. I guess there is a case for saying that if it's not allowed then you know that COPY relname TO is going to be fast. But, code aesthetics aside, the reasons for disallowing it seem a bit thin, to me. I would say the timing difference between COPY table TO and COPY (SELECT * FROM table) TO was noise, so it's not even faster. Remember that we were talking about supporting views, not tables. And if a view uses a slow query then you are in immediate danger of having a slow COPY. This may not be a problem but it needs to be discussed and an agreement must be reached before introducing such a change (and not during feature freeze). And an updatable VIEW *may* allow COPY view FROM... May I remind you that we've been in feature freeze for four weeks already? Now it's *not* the time to be drooling over cool features that would be nice to have. -- Alvaro Herrerahttp://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc. ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
Re: [PATCHES] [HACKERS] Performance testing of COPY (SELECT) TO
Remember that we were talking about supporting views, not tables. And if a view uses a slow query then you are in immediate danger of having a slow COPY. This may not be a problem but it needs to be discussed and an agreement must be reached before introducing such a change (and not during feature freeze). this will definitely be the case - however, this is not what it was made for. it has not been made to be fast but it has been made to fulfill some other task. the reason why this has been implemented is: consider a large scale database containing hundreds of gigs of data. in our special case we have to export in a flexible way. the data which has to be exported comes from multiple tables (between 3 and 7 depending on the data we are looking at in this project. the export has to be performed in a flexible way and it needs certain parameters. defining tmp tables and store the data in there is simply not nice at all. in most cases exports want to transform data on the fly - speed is not as important as flexibility here. so in my view the speed argument does not matter. if somebody passes a stupid query to copy he will get stupid runtimes - just like on ordinary sql. however, we can use COPY's capabilities to format / escape data to make exports more flexible. so basically it is a win. best regards, hans -- Cybertec Geschwinde Schönig GmbH Schöngrabern 134; A-2020 Hollabrunn Tel: +43/1/205 10 35 / 340 www.postgresql.at, www.cybertec.at ---(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: [PATCHES] [HACKERS] Performance testing of COPY (SELECT) TO
Hans-Juergen Schoenig wrote: Remember that we were talking about supporting views, not tables. And if a view uses a slow query then you are in immediate danger of having a slow COPY. This may not be a problem but it needs to be discussed and an agreement must be reached before introducing such a change (and not during feature freeze). this will definitely be the case - however, this is not what it was made for. it has not been made to be fast but it has been made to fulfill some other task. the reason why this has been implemented is: consider a large scale database containing hundreds of gigs of data. in our special case we have to export in a flexible way. the data which has to be exported comes from multiple tables (between 3 and 7 depending on the data we are looking at in this project. the export has to be performed in a flexible way and it needs certain parameters. defining tmp tables and store the data in there is simply not nice at all. in most cases exports want to transform data on the fly - speed is not as important as flexibility here. My question is, if we allow this: copy (select * from view) to stdout; (or to a file, whatever), is it enough for you? Or would you insist on also having copy view to stdout; ? We can, and the posted patch does, support the first form, but not the second. In fact I deliberately removed support for the second form for Zoltán's patch because it uglifies the surrounding code. -- Alvaro Herrerahttp://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc. ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [PATCHES] [HACKERS] Performance testing of COPY (SELECT) TO
Alvaro Herrera írta: Zoltan Boszormenyi wrote: Andrew Dunstan írta: Alvaro Herrera wrote: Böszörményi Zoltán wrote: what's the problem with COPY view TO, other than you don't like it? :-) The problem is that it required a ugly piece of code. Not supporting it means we can keep the code nice. The previous discussion led to this conclusion anyway so I don't know why we are debating it again. What is so ugly about it? I haven't looked at the code, but I am curious to know. It used a SELECT * FROM %s string that was passed back to the parser. I also don't recall the consensus being quite so clear cut. I guess there is a case for saying that if it's not allowed then you know that COPY relname TO is going to be fast. But, code aesthetics aside, the reasons for disallowing it seem a bit thin, to me. I would say the timing difference between COPY table TO and COPY (SELECT * FROM table) TO was noise, so it's not even faster. Remember that we were talking about supporting views, not tables. And if a view uses a slow query then you are in immediate danger of having a slow COPY. This may not be a problem but it needs to be discussed and an agreement must be reached before introducing such a change (and not during feature freeze). COPY relname TO meant tables _and_ views to me. My previous tsting showed no difference between COPY table TO and COPY (SELECT * FROM table) TO. Similarly a slow query defined in the view should show no difference between COPY view TO and COPY (SELECT * FROM view) TO. And remember, Bruce put the original COPY view TO patch into the unapplied queue, without the SELECT feature. Rewriting COPY view TO internally to COPY (SELECT * FROM view) TO is very straightforward, even if you think it's ugly. BTW, why is it ugly if I call raw_parser() from under src/backend/parser/*.c ? It is on a query distinct to the query the parser is currently running. Or is it the recursion that bothers you? It's not a possible infinite recursion. And an updatable VIEW *may* allow COPY view FROM... May I remind you that we've been in feature freeze for four weeks already? Now it's *not* the time to be drooling over cool features that would be nice to have Noted. However, as the COPY view TO is a straight internal rewrite, a COPY view FROM could also be. Even if it's a long term development. I wasn't proposing delaying beta. Best regards, Zoltán Böszörményi ---(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] [HACKERS] Performance testing of COPY (SELECT) TO
Alvaro Herrera [EMAIL PROTECTED] writes: My question is, if we allow this: copy (select * from view) to stdout; (or to a file, whatever), is it enough for you? Or would you insist on also having copy view to stdout; ? We can, and the posted patch does, support the first form, but not the second. In fact I deliberately removed support for the second form for Zoltán's patch because it uglifies the surrounding code. Personally, I have no moral objection to supporting the second form as a special case of the general COPY-from-select feature, but if it can't be done without uglifying the code then I'd agree with dropping it. I guess the question is whether the uglification is intrinsic or just a result of being descended from a poor original implementation. The feature-freeze argument seems not relevant, given that the code we had on the feature-freeze date did both things. Has this patch settled to the point where I can review it, or is it still in motion? regards, tom lane ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [PATCHES] [HACKERS] Performance testing of COPY (SELECT) TO
Alvaro Herrera wrote: Hans-Juergen Schoenig wrote: Remember that we were talking about supporting views, not tables. And if a view uses a slow query then you are in immediate danger of having a slow COPY. This may not be a problem but it needs to be discussed and an agreement must be reached before introducing such a change (and not during feature freeze). this will definitely be the case - however, this is not what it was made for. it has not been made to be fast but it has been made to fulfill some other task. the reason why this has been implemented is: consider a large scale database containing hundreds of gigs of data. in our special case we have to export in a flexible way. the data which has to be exported comes from multiple tables (between 3 and 7 depending on the data we are looking at in this project. the export has to be performed in a flexible way and it needs certain parameters. defining tmp tables and store the data in there is simply not nice at all. in most cases exports want to transform data on the fly - speed is not as important as flexibility here. My question is, if we allow this: copy (select * from view) to stdout; (or to a file, whatever), is it enough for you? Or would you insist on also having copy view to stdout; ? i would say that copy view to stdout is just some syntactic sugar (to me at least). the important thing is that we add the flexibility of SELECT to it. a view is nothing else than a rule on SELECT anyway. to be honest i never thought about views when creating this copy idea. however, i think it is not bad to have it because i have seen a couple of times already that tables turn into views when new features are added to an existing data structure . if we support copy on views this means that exports can stay as they are even if the data structure is changed in that way. however, if people think that views are not needed that way it is still a good solution as views are not the basic reason why this new functionality is a good thing to have. many thanks, hans -- Cybertec Geschwinde Schönig GmbH Schöngrabern 134; A-2020 Hollabrunn Tel: +43/1/205 10 35 / 340 www.postgresql.at, www.cybertec.at ---(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] [HACKERS] Performance testing of COPY (SELECT) TO
Zoltan Boszormenyi wrote: Alvaro Herrera írta: Remember that we were talking about supporting views, not tables. And if a view uses a slow query then you are in immediate danger of having a slow COPY. This may not be a problem but it needs to be discussed and an agreement must be reached before introducing such a change (and not during feature freeze). COPY relname TO meant tables _and_ views to me. My previous tsting showed no difference between COPY table TO and COPY (SELECT * FROM table) TO. Similarly a slow query defined in the view should show no difference between COPY view TO and COPY (SELECT * FROM view) TO. The difference is that we are giving a very clear distinction between a table and a view. If we don't support the view in the direct COPY, but instead insist that it be passed via a SELECT query, then the user will be aware that it may be slow. relname at this point may mean anything -- are you supporting sequences and toast tables as well? And remember, Bruce put the original COPY view TO patch into the unapplied queue, without the SELECT feature. All sort of junk enters that queue so that's not an argument. (Not meant to insult Bruce -- I'm just saying that he doesn't filter stuff. We've had patches rejected from the queue before plenty of times.) Rewriting COPY view TO internally to COPY (SELECT * FROM view) TO is very straightforward, even if you think it's ugly. BTW, why is it ugly if I call raw_parser() from under src/backend/parser/*.c ? It is on a query distinct to the query the parser is currently running. Or is it the recursion that bothers you? It's not a possible infinite recursion. It's ugly because you are forcing the system to parse something that was already parsed. On the other hand I don't see why you are arguing in favor of a useless feature whose coding is dubious; you can have _the same thing_ with nice code and no discussion. -- Alvaro Herrerahttp://www.CommandPrompt.com/ PostgreSQL Replication, Consulting, Custom Development, 24x7 support ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
Re: [PATCHES] [HACKERS] Performance testing of COPY (SELECT) TO
Tom Lane wrote: Alvaro Herrera [EMAIL PROTECTED] writes: My question is, if we allow this: copy (select * from view) to stdout; (or to a file, whatever), is it enough for you? Or would you insist on also having copy view to stdout; ? We can, and the posted patch does, support the first form, but not the second. In fact I deliberately removed support for the second form for Zoltán's patch because it uglifies the surrounding code. Personally, I have no moral objection to supporting the second form as a special case of the general COPY-from-select feature, but if it can't be done without uglifying the code then I'd agree with dropping it. I guess the question is whether the uglification is intrinsic or just a result of being descended from a poor original implementation. I'm quite sure you could refactor things as needed to support the COPY view case reasonably. It's just beyond what I'd do during the current freeze. It seems I'm alone on the view may be slow camp. If I lost that argument I have no problem accepting that. The feature-freeze argument seems not relevant, given that the code we had on the feature-freeze date did both things. Actually IIRC the patch on the queue only did the COPY view stuff, not the COPY select. (Thanks go to Zoltan for properly morphing the patch). Has this patch settled to the point where I can review it, or is it still in motion? Personally I'm finished doing the cleanup I wanted to do. -- Alvaro Herrerahttp://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc. ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [PATCHES] [HACKERS] Performance testing of COPY (SELECT) TO
Remember that we were talking about supporting views, not tables. And if a view uses a slow query then you are in immediate danger of having a slow COPY. This may not be a problem but it needs to be discussed and an agreement must be reached before introducing such a change (and not during feature freeze). COPY relname TO meant tables _and_ views to me. My previous tsting showed no difference between COPY table TO and COPY (SELECT * FROM table) TO. Similarly a slow query defined in the view should show no difference between COPY view TO and COPY (SELECT * FROM view) TO. The difference is that we are giving a very clear distinction between a table and a view. If we don't support the view in the direct COPY, but instead insist that it be passed via a SELECT query, then the user will be aware that it may be slow. what kind of clever customers do you have in the US? ;) i would never say something like that here :). i see your point and i think it is not a too bad idea. at least some folks might see that there is no voodoo going on ... relname at this point may mean anything -- are you supporting sequences and toast tables as well? good point ... It's ugly because you are forcing the system to parse something that was already parsed. definitely an argument for dropping the view stuff ... On the other hand I don't see why you are arguing in favor of a useless feature whose coding is dubious; you can have _the same thing_ with nice code and no discussion. what are you referring to? hans -- Cybertec Geschwinde Schönig GmbH Schöngrabern 134; A-2020 Hollabrunn Tel: +43/1/205 10 35 / 340 www.postgresql.at, www.cybertec.at ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
Re: [PATCHES] [HACKERS] Performance testing of COPY (SELECT) TO
Alvaro Herrera írta: Zoltan Boszormenyi wrote: Alvaro Herrera írta: Remember that we were talking about supporting views, not tables. And if a view uses a slow query then you are in immediate danger of having a slow COPY. This may not be a problem but it needs to be discussed and an agreement must be reached before introducing such a change (and not during feature freeze). COPY relname TO meant tables _and_ views to me. My previous tsting showed no difference between COPY table TO and COPY (SELECT * FROM table) TO. Similarly a slow query defined in the view should show no difference between COPY view TO and COPY (SELECT * FROM view) TO. The difference is that we are giving a very clear distinction between a table and a view. If we don't support the view in the direct COPY, but instead insist that it be passed via a SELECT query, then the user will be aware that it may be slow. It still can be documented with supporting the COPY view TO syntax. But COPY view (col1, col2, ...) TO may still be useful even if the COPY (SELECT ...) (col1, col2, ...) TO is pointless. [1] relname at this point may mean anything -- are you supporting sequences and toast tables as well? Well, not really. :-) And remember, Bruce put the original COPY view TO patch into the unapplied queue, without the SELECT feature. All sort of junk enters that queue so that's not an argument. (Not meant to insult Bruce -- I'm just saying that he doesn't filter stuff. We've had patches rejected from the queue before plenty of times.) OK. :-) Rewriting COPY view TO internally to COPY (SELECT * FROM view) TO is very straightforward, even if you think it's ugly. BTW, why is it ugly if I call raw_parser() from under src/backend/parser/*.c ? It is on a query distinct to the query the parser is currently running. Or is it the recursion that bothers you? It's not a possible infinite recursion. It's ugly because you are forcing the system to parse something that was already parsed. Well, to be true to the word, during parsing COPY view TO the parser never saw SELECT * FROM view. On the other hand I don't see why you are arguing in favor of a useless feature whose coding is dubious; you can have _the same thing_ with nice code and no discussion. Because of [1] and because Mr. Schoenig's arguments about changing schemas. Best regards, Zoltán Böszörményi ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [PATCHES] [HACKERS] Performance testing of COPY (SELECT) TO
Hans-Juergen Schoenig wrote: It's ugly because you are forcing the system to parse something that was already parsed. definitely an argument for dropping the view stuff ... On the other hand, it's quite possible that this could be made to work _without_ doing black magic (which would be OK by me). On the other hand I don't see why you are arguing in favor of a useless feature whose coding is dubious; you can have _the same thing_ with nice code and no discussion. what are you referring to? The fact that the direct copy view feature is just syntactic sugar over copy (select * from view). The latter we can have without discussion -- from me, that is :-) -- Alvaro Herrerahttp://www.CommandPrompt.com/ PostgreSQL Replication, Consulting, Custom Development, 24x7 support ---(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: [PATCHES] [HACKERS] Performance testing of COPY (SELECT) TO
Zoltan Boszormenyi wrote: Alvaro Herrera írta: But COPY view (col1, col2, ...) TO may still be useful even if the COPY (SELECT ...) (col1, col2, ...) TO is pointless. [1] Hum, I don't understand what you're saying here -- are you saying that you can't do something with the first form, that you cannot do with the second? It's ugly because you are forcing the system to parse something that was already parsed. Well, to be true to the word, during parsing COPY view TO the parser never saw SELECT * FROM view. Hmm! The COPY view stuff stopped working when I changed back the relation case. Your patch changed it so that instead of flowing as RangeVar all the way to the copy.c code, the parser changed it into a select * from %s query, and then stashed the resulting Query node into the query %case. (So what was happening was that the Relation case was never %used). I reverted this. On the other hand I don't see why you are arguing in favor of a useless feature whose coding is dubious; you can have _the same thing_ with nice code and no discussion. Because of [1] and because Mr. Schoenig's arguments about changing schemas. Yeah, that argument makes sense to me as well. -- Alvaro Herrerahttp://www.CommandPrompt.com/ PostgreSQL Replication, Consulting, Custom Development, 24x7 support ---(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] [HACKERS] Performance testing of COPY (SELECT) TO
On the other hand I don't see why you are arguing in favor of a useless feature whose coding is dubious; you can have _the same thing_ with nice code and no discussion. Because of [1] and because Mr. Schoenig's arguments about changing schemas. first of all; hans is enough - skip the mr ;) i think changing schema is a good argument but we could sacrifice that for the sake of clarity and clean code. i am not against keeping it but i can understand the argument against views. i always preferred select. mr hans ;) -- Cybertec Geschwinde Schönig GmbH Schöngrabern 134; A-2020 Hollabrunn Tel: +43/1/205 10 35 / 340 www.postgresql.at, www.cybertec.at ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [PATCHES] updated patch for selecting large results sets in psql using cursors
[EMAIL PROTECTED] writes: here comes the latest version (version 7) of the patch to handle large result sets with psql. As previously discussed, a cursor is used for SELECT queries when \set FETCH_COUNT some_value 0 Wait a minute. What I thought we had agreed to was a patch to make commands sent with \g use a cursor. This patch changes SendQuery so that *every* command executed via psql is treated this way. That's a whole lot bigger behavioral change than I think is warranted. 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] [HACKERS] Performance testing of COPY (SELECT) TO
Alvaro Herrera írta: Zoltan Boszormenyi wrote: Alvaro Herrera írta: But COPY view (col1, col2, ...) TO may still be useful even if the COPY (SELECT ...) (col1, col2, ...) TO is pointless. [1] Hum, I don't understand what you're saying here -- are you saying that you can't do something with the first form, that you cannot do with the second? Say you have a large often used query. Would you like to retype it every time or just create a view? Later you may want to export only a subset of the fields... My v8 had the syntax support for COPY (SELECT ...) (col1, col2, ...) TO and it was actually working. In your v9 you rewrote the syntax parsing so that feature was lost in translation. It's ugly because you are forcing the system to parse something that was already parsed. Well, to be true to the word, during parsing COPY view TO the parser never saw SELECT * FROM view. Hmm! The COPY view stuff stopped working when I changed back the relation case. Your patch changed it so that instead of flowing as RangeVar all the way to the copy.c code, the parser changed it into a select * from %s query, and then stashed the resulting Query node into the query %case. (So what was happening was that the Relation case was never %used). I reverted this. Well, the VIEW case wasn't supported before so I took the opportunity to transform it in analyze.c which you deleted as being ugly. On the other hand I don't see why you are arguing in favor of a useless feature whose coding is dubious; you can have _the same thing_ with nice code and no discussion. Because of [1] and because Mr. Schoenig's arguments about changing schemas. Yeah, that argument makes sense to me as well. So, may I put it back? :-) Also, can you suggest anything cleaner than calling raw_parser(SELECT * FROM view)? ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
Re: [PATCHES] [HACKERS] Performance testing of COPY (SELECT) TO
Zoltan Boszormenyi wrote: Alvaro Herrera írta: Zoltan Boszormenyi wrote: Alvaro Herrera írta: But COPY view (col1, col2, ...) TO may still be useful even if the COPY (SELECT ...) (col1, col2, ...) TO is pointless. [1] Hum, I don't understand what you're saying here -- are you saying that you can't do something with the first form, that you cannot do with the second? Say you have a large often used query. Would you like to retype it every time or just create a view? Later you may want to export only a subset of the fields... My v8 had the syntax support for COPY (SELECT ...) (col1, col2, ...) TO and it was actually working. In your v9 you rewrote the syntax parsing so that feature was lost in translation. Interesting. I didn't realize this was possible -- obviously I didn't test it (did you have a test for it in the regression tests? I may have missed it). In fact, I deliberately removed the column list from the grammar, because it can certainly be controlled inside the SELECT, so I thought there was no reason the support controlling it in the COPY column list. I don't think it's difficult to put it back. But this has nothing to do with COPY view, does it? On the other hand I don't see why you are arguing in favor of a useless feature whose coding is dubious; you can have _the same thing_ with nice code and no discussion. Because of [1] and because Mr. Schoenig's arguments about changing schemas. Yeah, that argument makes sense to me as well. So, may I put it back? :-) Also, can you suggest anything cleaner than calling raw_parser(SELECT * FROM view)? I think at this point is someone else's judgement whether you can put it back or not. Tom already said that he doesn't object to the feature per se; no one else seems opposed to the feature per se, in fact. Now, I don't really see _how_ to do it in nice code, so no, I don't have any suggestion for you. You may want to give the pumpkin to Tom so that he gives the patch the finishing touches (hopefully making it support the COPY view feature as well). If it were up to me, I'd just commit it as is (feature-wise -- more thorough review is still needed) and revisit the COPY view stuff in 8.3 if there is demand. -- Alvaro Herrerahttp://www.CommandPrompt.com/ PostgreSQL Replication, Consulting, Custom Development, 24x7 support ---(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: [PATCHES] updated patch for selecting large results sets in psql using cursors
Tom Lane wrote: Wait a minute. What I thought we had agreed to was a patch to make commands sent with \g use a cursor. This patch changes SendQuery so that *every* command executed via psql is treated this way. That's what I remembered. I don't think we want to introduce a difference between ; and \g. -- Peter Eisentraut http://developer.postgresql.org/~petere/ ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [PATCHES] [HACKERS] Performance testing of COPY (SELECT) TO
Alvaro Herrera írta: Zoltan Boszormenyi wrote: Alvaro Herrera írta: Zoltan Boszormenyi wrote: Alvaro Herrera írta: But COPY view (col1, col2, ...) TO may still be useful even if the COPY (SELECT ...) (col1, col2, ...) TO is pointless. [1] Hum, I don't understand what you're saying here -- are you saying that you can't do something with the first form, that you cannot do with the second? Say you have a large often used query. Would you like to retype it every time or just create a view? Later you may want to export only a subset of the fields... My v8 had the syntax support for COPY (SELECT ...) (col1, col2, ...) TO and it was actually working. In your v9 you rewrote the syntax parsing so that feature was lost in translation. Interesting. I didn't realize this was possible -- obviously I didn't test it (did you have a test for it in the regression tests? I may have missed it). In fact, I deliberately removed the column list from the grammar, because it can certainly be controlled inside the SELECT, so I thought there was no reason the support controlling it in the COPY column list. Yes, it was even documented. I thought about having queries stored statically somewhere (not in views) and being able to use only part of the result. I don't think it's difficult to put it back. But this has nothing to do with COPY view, does it? No, but it may be confusing seeing COPY (SELECT ) (col1, col2, ...) TO instead of COPY (SELECT col1, col2, ...) TO. With the COPY VIEW (col1, col2, ...) TO syntax it may be cleaner from the user's point of view. Together with the changing schemas argument it gets more and more tempting. On the other hand I don't see why you are arguing in favor of a useless feature whose coding is dubious; you can have _the same thing_ with nice code and no discussion. Because of [1] and because Mr. Schoenig's arguments about changing schemas. Yeah, that argument makes sense to me as well. So, may I put it back? :-) Also, can you suggest anything cleaner than calling raw_parser(SELECT * FROM view)? I think at this point is someone else's judgement whether you can put it back or not. Tom already said that he doesn't object to the feature per se; no one else seems opposed to the feature per se, in fact. Now, I don't really see _how_ to do it in nice code, so no, I don't have any suggestion for you. You may want to give the pumpkin to Tom so that he gives the patch the finishing touches (hopefully making it support the COPY view feature as well). If it were up to me, I'd just commit it as is (feature-wise -- more thorough review is still needed) and revisit the COPY view stuff in 8.3 if there is demand. OK, I will put it back as it was in v8 keeping all your other cleanup and let Bruce and Tom decide. (BTW, is there anyone as high-ranking as them, or the committee is a duumvirate? :-) ) ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
Re: [PATCHES] [HACKERS] Performance testing of COPY (SELECT) TO
Zoltan Boszormenyi wrote: I think at this point is someone else's judgement whether you can put it back or not. Tom already said that he doesn't object to the feature per se; no one else seems opposed to the feature per se, in fact. Now, I don't really see _how_ to do it in nice code, so no, I don't have any suggestion for you. You may want to give the pumpkin to Tom so that he gives the patch the finishing touches (hopefully making it support the COPY view feature as well). If it were up to me, I'd just commit it as is (feature-wise -- more thorough review is still needed) and revisit the COPY view stuff in 8.3 if there is demand. OK, I will put it back as it was in v8 keeping all your other cleanup and let Bruce and Tom decide. Hum, are you going to put back the original cruft to support copy view? I suggest you don't do that. (BTW, is there anyone as high-ranking as them, or the committee is a duumvirate? :-) ) There is a core, there are committers, there are major developers, and there are contributors. This is documented in the developer's page on the website, though the committers group is not documented anywhere. (Most, but not all, of Core are also committers. Some Major Developers are committers as well). There is no committee. The closer you get to that, is people vocal enough on pgsql-hackers. -- Alvaro Herrerahttp://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc. ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
Re: [PATCHES] [HACKERS] Performance testing of COPY (SELECT) TO
Alvaro Herrera írta: Zoltan Boszormenyi wrote: I think at this point is someone else's judgement whether you can put it back or not. Tom already said that he doesn't object to the feature per se; no one else seems opposed to the feature per se, in fact. Now, I don't really see _how_ to do it in nice code, so no, I don't have any suggestion for you. You may want to give the pumpkin to Tom so that he gives the patch the finishing touches (hopefully making it support the COPY view feature as well). If it were up to me, I'd just commit it as is (feature-wise -- more thorough review is still needed) and revisit the COPY view stuff in 8.3 if there is demand. OK, I will put it back as it was in v8 keeping all your other cleanup and let Bruce and Tom decide. Hum, are you going to put back the original cruft to support copy view? I suggest you don't do that. Well, the other way around is to teach heap_open() to use views. Brrr. Would it be any cleaner? ---(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] [HACKERS] Performance testing of COPY (SELECT) TO
Zoltan Boszormenyi wrote: Alvaro Herrera írta: Zoltan Boszormenyi wrote: I think at this point is someone else's judgement whether you can put it back or not. Tom already said that he doesn't object to the feature per se; no one else seems opposed to the feature per se, in fact. Now, I don't really see _how_ to do it in nice code, so no, I don't have any suggestion for you. You may want to give the pumpkin to Tom so that he gives the patch the finishing touches (hopefully making it support the COPY view feature as well). If it were up to me, I'd just commit it as is (feature-wise -- more thorough review is still needed) and revisit the COPY view stuff in 8.3 if there is demand. OK, I will put it back as it was in v8 keeping all your other cleanup and let Bruce and Tom decide. Hum, are you going to put back the original cruft to support copy view? I suggest you don't do that. Well, the other way around is to teach heap_open() to use views. Brrr. Would it be any cleaner? Certainly not. -- Alvaro Herrerahttp://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc. ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [PATCHES] [HACKERS] Performance testing of COPY (SELECT) TO
Alvaro Herrera [EMAIL PROTECTED] writes: Zoltan Boszormenyi wrote: My v8 had the syntax support for COPY (SELECT ...) (col1, col2, ...) TO and it was actually working. In your v9 you rewrote the syntax parsing so that feature was lost in translation. Interesting. I didn't realize this was possible -- obviously I didn't test it (did you have a test for it in the regression tests? I may have missed it). In fact, I deliberately removed the column list from the grammar, because it can certainly be controlled inside the SELECT, so I thought there was no reason the support controlling it in the COPY column list. I would vote against allowing a column list here, because it's useless and it strikes me as likely to result in strange syntax error messages if the user makes any little mistake. What worries me is that the above looks way too nearly like a function call, which means that for instance if you omit a right paren somewhere in the SELECT part, you're likely to get a syntax error that points far to the right of the actual mistake. The parser could also mistake the column list for a table-alias column list. Specifying a column list with a view name is useful, of course, but what is the point when you are writing out a SELECT anyway? regards, tom lane ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
Re: [PATCHES] [HACKERS] Performance testing of COPY (SELECT) TO
Zoltan Boszormenyi [EMAIL PROTECTED] writes: Alvaro Herrera írta: Hum, are you going to put back the original cruft to support copy view? I suggest you don't do that. Well, the other way around is to teach heap_open() to use views. Brrr. Would it be any cleaner? Don't even think of going there ;-) 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
Re: [PATCHES] [HACKERS] Performance testing of COPY (SELECT) TO
Tom Lane írta: Zoltan Boszormenyi [EMAIL PROTECTED] writes: Alvaro Herrera írta: Hum, are you going to put back the original cruft to support copy view? I suggest you don't do that. Well, the other way around is to teach heap_open() to use views. Brrr. Would it be any cleaner? Don't even think of going there ;-) regards, tom lane I didn't. :-) Here's my last, the cruft (i.e. COPY view TO support by rewriting to a SELECT) put back. Tested and docs modified accordingly. You can find the previous one (v10) on the list without it if you need it. Best regards, Zoltán Böszörményi pgsql-copyselect-11.patch.gz Description: Unix tar archive ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
Re: [PATCHES] [HACKERS] Performance testing of COPY (SELECT)
Tom Lane wrote: Alvaro Herrera [EMAIL PROTECTED] writes: Zoltan Boszormenyi wrote: My v8 had the syntax support for COPY (SELECT ...) (col1, col2, ...) TO and it was actually working. In your v9 you rewrote the syntax parsing so that feature was lost in translation. Interesting. I didn't realize this was possible -- obviously I didn't test it (did you have a test for it in the regression tests? I may have missed it). In fact, I deliberately removed the column list from the grammar, because it can certainly be controlled inside the SELECT, so I thought there was no reason the support controlling it in the COPY column list. I would vote against allowing a column list here, because it's useless and it strikes me as likely to result in strange syntax error messages if the user makes any little mistake. What worries me is that the above looks way too nearly like a function call, which means that for instance if you omit a right paren somewhere in the SELECT part, you're likely to get a syntax error that points far to the right of the actual mistake. The parser could also mistake the column list for a table-alias column list. Specifying a column list with a view name is useful, of course, but what is the point when you are writing out a SELECT anyway? If you don't support COPY view TO, at least return an error messsage that suggests using COPY (SELECT * FROM view). And if you support COPY VIEW, you are going to have to support a column list for that. Is that additional complexity in COPY? If so, it might be a reason to just throw an error on views and do use COPY SELECT. Seeing that COPY VIEW only supports TO, not FROM, and COPY SELECT support only TO, not FROM, it seems logical for COPY to just support relations, and COPY SELECT to be used for views, if we can throw an error on COPY VIEW to tell people to use COPY SELECT. -- Bruce Momjian [EMAIL PROTECTED] EnterpriseDBhttp://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. + ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
Re: [PATCHES] updated patch for selecting large results sets
Peter Eisentraut wrote: Tom Lane wrote: Wait a minute. What I thought we had agreed to was a patch to make commands sent with \g use a cursor. This patch changes SendQuery so that *every* command executed via psql is treated this way. That's what I remembered. I don't think we want to introduce a difference between ; and \g. I am confused. I assume \g and ; should be affected, like Peter says. Tom, what *every* command are you talking about? You mean \d? -- Bruce Momjian [EMAIL PROTECTED] EnterpriseDBhttp://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. + ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
Re: [PATCHES] updated patch for selecting large results sets in psql using cursors
Bruce Momjian [EMAIL PROTECTED] writes: Peter Eisentraut wrote: Tom Lane wrote: Wait a minute. What I thought we had agreed to was a patch to make commands sent with \g use a cursor. I am confused. I assume \g and ; should be affected, like Peter says. Tom, what *every* command are you talking about? You mean \d? Like I said, I thought we were intending to modify \g's behavior only; that was certainly the implication of the discussion of \gc. regards, tom lane ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [PATCHES] updated patch for selecting large results sets
Tom Lane wrote: Bruce Momjian [EMAIL PROTECTED] writes: Peter Eisentraut wrote: Tom Lane wrote: Wait a minute. What I thought we had agreed to was a patch to make commands sent with \g use a cursor. I am confused. I assume \g and ; should be affected, like Peter says. Tom, what *every* command are you talking about? You mean \d? Like I said, I thought we were intending to modify \g's behavior only; that was certainly the implication of the discussion of \gc. OK, got it. I just don't see the value to doing \g and not ;. I think the \gc case was a hack when he didn't have \set. Now that we have \set, we should be consistent. -- Bruce Momjian [EMAIL PROTECTED] EnterpriseDBhttp://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. + ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
Re: [PATCHES] updated patch for selecting large results sets
Bruce Momjian [EMAIL PROTECTED] writes: OK, got it. I just don't see the value to doing \g and not ;. I think the \gc case was a hack when he didn't have \set. Now that we have \set, we should be consistent. I'm willing to accept this if we can make sure we aren't adding any overhead --- see my proposal elsewhere in the thread for fixing that. regards, tom lane ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
Re: [PATCHES] updated patch for selecting large results sets in
I am confused. I assume \g and ; should be affected, like Peter says. Tom, what *every* command are you talking about? You mean \d? Like I said, I thought we were intending to modify \g's behavior only; that was certainly the implication of the discussion of \gc. At some point you OKed the \g and ; proposal. I admit I was quick adding the and ; part, but it seemed so natural once we agreed on using a variable. OK, got it. I just don't see the value to doing \g and not ;. I think the \gc case was a hack when he didn't have \set. Now that we have \set, we should be consistent. I agree with this statement. If we have a variable that switches just between two versions of \g, we could have gone with using \u (or whatever) in the first place. In the mean time I have been converted by the variable camp, and I think the variable should change \g and ; together, consistently. If we find we can't live with the performance overhead of that if(FETCH_COUNT), it is still not clear why we would be better off moving it into the \g code path only. Is it because presumably \g is used less often in existing psql scripts? Bye, Chris. -- Chris Mair http://www.1006.org ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
Re: [PATCHES] updated patch for selecting large results sets
Tom Lane wrote: Bruce Momjian [EMAIL PROTECTED] writes: OK, got it. I just don't see the value to doing \g and not ;. I think the \gc case was a hack when he didn't have \set. Now that we have \set, we should be consistent. I'm willing to accept this if we can make sure we aren't adding any overhead --- see my proposal elsewhere in the thread for fixing that. Right, if \g has overhead, I don't want people to start using ; because it is faster. That is the kind of behavior that makes us look sloppy. -- Bruce Momjian [EMAIL PROTECTED] EnterpriseDBhttp://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. + ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
Re: [PATCHES] [HACKERS] Performance testing of COPY (SELECT) TO
On Mon, Aug 28, 2006 at 19:35:11 +0200, Zoltan Boszormenyi [EMAIL PROTECTED] wrote: (BTW, is there anyone as high-ranking as them, or the committee is a duumvirate? :-) ) There is a group referred to as core that is the final arbitrator of things. Tom and Bruce are both members of this group. Tom and Bruce tend to be the most visibly active committers for getting patches committed for people that can't do it themselves. So you will see them speak up more than others on the patches list. ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [HACKERS] [PATCHES] log_statement output for protocol
Guillaume Smet wrote:
On 8/7/06, Bruce Momjian [EMAIL PROTECTED] wrote:
Updated patch attached. It prints the text bind parameters on a single
detail line. I still have not seen portal names generated by libpq.
I'm currently testing CVS tip to generate sample log files. I noticed
that Bruce only patched log_statement and not
log_min_duration_statement which still has the old behaviour ie:
[1-1] LOG: duration: 0.097 ms execute my_query: SELECT * FROM shop
WHERE name = $1
The problem of not having the bind parameters still remains.
A lot of people use log_min_duration_statement and it's usually
recommended to use it instead of log_statement because log_statement
generates far too much output.
I tried to find a way to fix it but it's not so simple as when we bind
the statement, we don't know if the query will be slower than
log_min_duration_statement.
My first idea is that we should add a DETAIL line with the parameter
values to the execute log line when we are in the
log_min_duration_statement case. AFAICS the values are in the portal
but I don't know the overhead introduced by generating the detail line
from the portal.
Does anyone have a better idea on how we could fix it?
Yes, I do. I have applied the attached patch to fix this issue and
several others. The fix was to save the bind parameters in the portal,
and display those in the executor output, if available.
The other changes were to use single quotes for bind values, instead of
double quotes, and double literal single quotes in bind values (and
document that). I also made use of the DETAIL line to have much cleaner
output.
With the new output, bind displays prepare as detail, and execute
displays prepare and optionally bind. I re-added the statement: label
so people will understand why the line is being printed (it is
log_*statement behavior). I am now happy that the display is clear and
not cluttered.
-- SQL using log_statement
LOG: set log_statement = 'all';
LOG: statement: begin;
LOG: statement: prepare x as select $1::text;
LOG: statement: execute x ('a');
DETAIL: prepare: prepare x as select $1::text;
LOG: statement: commit;
-- SQL using log_min_duration_statement
LOG: statement: set log_statement = 'none';
LOG: duration: 0.242 ms statement: set log_min_duration_statement = 0;
LOG: duration: 0.155 ms statement: begin;
LOG: duration: 0.174 ms statement: prepare y as select $1::text;
LOG: duration: 0.252 ms statement: execute y ('a');
DETAIL: prepare: prepare y as select $1::text;
LOG: duration: 0.093 ms statement: commit;
-- protocol using log_statement
LOG: statement: prepare sel1, SELECT $1;
LOG: statement: bind sel1, $1 = 'a''b'
DETAIL: prepare: SELECT $1;
LOG: statement: execute sel1
DETAIL: prepare: SELECT $1; bind: $1 = 'a''b'
-- protocol using log_min_duration_statement
LOG: duration: 0.497 ms statement: SET log_min_duration_statement = 0;
LOG: duration: 0.027 ms execute sel1
DETAIL: prepare: SELECT $1; bind: $1 = 'a''b'
The last output doesn't have bind or prepare because we don't print
those because we don't do any duration timing for them. Should we print
the statement: lines of log_min_duration_statement == 0?
Also, this code assumes that a protocol bind and execute always has
prepared statement text, which I believe is true.
The use of brackets is gone. :-)
--
Bruce Momjian [EMAIL PROTECTED]
EnterpriseDBhttp://www.enterprisedb.com
+ If your life is a hard drive, Christ can be your backup. +
Index: doc/src/sgml/config.sgml
===
RCS file: /cvsroot/pgsql/doc/src/sgml/config.sgml,v
retrieving revision 1.75
diff -c -c -r1.75 config.sgml
*** doc/src/sgml/config.sgml 17 Aug 2006 23:04:03 - 1.75
--- doc/src/sgml/config.sgml 28 Aug 2006 23:59:41 -
***
*** 2839,2845
prepare, bind, and execute commands are logged only if
varnamelog_statement/ is literalall/. Bind parameter
values are also logged if they are supplied in literaltext/
! format.
/para
para
The default is literalnone/. Only superusers can change this
--- 2839,2845
prepare, bind, and execute commands are logged only if
varnamelog_statement/ is literalall/. Bind parameter
values are also logged if they are supplied in literaltext/
! format (literal single quotes are doubled).
/para
para
The default is literalnone/. Only superusers can change this
Index: src/backend/commands/portalcmds.c
===
RCS file: /cvsroot/pgsql/src/backend/commands/portalcmds.c,v
retrieving revision 1.50
diff
Re: [PATCHES] [HACKERS] Performance testing of COPY (SELECT)
Bruno Wolff III wrote: On Mon, Aug 28, 2006 at 19:35:11 +0200, Zoltan Boszormenyi [EMAIL PROTECTED] wrote: (BTW, is there anyone as high-ranking as them, or the committee is a duumvirate? :-) ) There is a group referred to as core that is the final arbitrator of things. Tom and Bruce are both members of this group. Tom and Bruce tend to be the most visibly active committers for getting patches committed for people that can't do it themselves. So you will see them speak up more than others on the patches list. We do try not to be decision-makers, but rather give our opinions and see how the group decides. -- Bruce Momjian [EMAIL PROTECTED] EnterpriseDBhttp://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. + ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [HACKERS] [PATCHES] log_statement output for protocol
Bruce Momjian [EMAIL PROTECTED] writes: Yes, I do. I have applied the attached patch to fix this issue and several others. The fix was to save the bind parameters in the portal, and display those in the executor output, if available. I have a feeling you just blew away the 4% savings in psql I've spent the evening on. What's the overhead of this patch? regards, tom lane ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [HACKERS] [PATCHES] log_statement output for protocol
BTom Lane wrote: Bruce Momjian [EMAIL PROTECTED] writes: Yes, I do. I have applied the attached patch to fix this issue and several others. The fix was to save the bind parameters in the portal, and display those in the executor output, if available. I have a feeling you just blew away the 4% savings in psql I've spent the evening on. What's the overhead of this patch? The only overhead I see is calling log_after_parse() all the time, rather than only when log_statement is all. I could fix it by checking log_statement and log_min_duration_statement = 0. Does log_after_parse() look heavy to you? -- Bruce Momjian [EMAIL PROTECTED] EnterpriseDBhttp://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. + ---(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] [PATCHES] log_statement output for protocol
bruce wrote: BTom Lane wrote: Bruce Momjian [EMAIL PROTECTED] writes: Yes, I do. I have applied the attached patch to fix this issue and several others. The fix was to save the bind parameters in the portal, and display those in the executor output, if available. I have a feeling you just blew away the 4% savings in psql I've spent the evening on. What's the overhead of this patch? The only overhead I see is calling log_after_parse() all the time, rather than only when log_statement is all. I could fix it by checking log_statement and log_min_duration_statement = 0. Does log_after_parse() look heavy to you? OK, I applied this patch to call log_after_parse() only if necessary. The 'if' statement looked pretty ugly, so the optimization seemed overkill, but maybe it will be useful. -- Bruce Momjian [EMAIL PROTECTED] EnterpriseDBhttp://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. + Index: src/backend/tcop/postgres.c === RCS file: /cvsroot/pgsql/src/backend/tcop/postgres.c,v retrieving revision 1.500 diff -c -c -r1.500 postgres.c *** src/backend/tcop/postgres.c 29 Aug 2006 02:11:29 - 1.500 --- src/backend/tcop/postgres.c 29 Aug 2006 02:29:48 - *** *** 871,877 parsetree_list = pg_parse_query(query_string); /* Log immediately if dictated by log_statement */ ! was_logged = log_after_parse(parsetree_list, query_string, prepare_string); /* * Switch back to transaction context to enter the loop. --- 871,879 parsetree_list = pg_parse_query(query_string); /* Log immediately if dictated by log_statement */ ! if (log_statement != LOGSTMT_NONE || log_duration || ! log_min_duration_statement = 0) ! was_logged = log_after_parse(parsetree_list, query_string, prepare_string); /* * Switch back to transaction context to enter the loop. ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
