Re: [HACKERS] [PATCHES] extension for sql update
bruce wrote: > I have merged your patch into current CVS and applied it; attached. > There was quite a bit of code drift. One drift area was the new > RETURNING clause; that was easy to fix. A more complex case is the > code no longer has values as ResTargets --- it is a simple a_expr list, > so I changed the critical assignment in gram.y from: > > res_col->val = (Node *)copyObject(res_val->val); > > to: > > res_col->val = (Node *)copyObject(res_val); > > Hope that is OK. Without that fix, it crashed. I also merged your SGML > syntax and grammer addition into the exiting UPDATE main entry. The copyObject() is not required. Removed. -- 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] DOC: catalog.sgml
On Fri, Sep 01, 2006 at 12:36:11PM -0400, Alvaro Herrera wrote: > Tom Lane wrote: > > Zdenek Kotala <[EMAIL PROTECTED]> writes: > > > I little bit enhanced overview catalog tables. I added two new columns. > > > First one is OID of catalog table and second one contains attributes > > > which determine if the table is bootstrap, with oid and global. > > > > Why is this a good idea? It seems like mere clutter. > > What's "global"? A maybe-useful flag would be telling that a table is > shared. Is that it? Mind you, it's not useful to me because I know > which tables are shared, but I guess for someone not so familiar with > the catalogs it could have some use. > > The OIDs may be useful to people inspecting pg_depend, for example; but > then, it's foolish not to be using regclass in that case. > > Whether a table is "bootstrap" or not doesn't seem useful to me. Something that might be handy would be a method to determine if an object is a system object or not (perhaps what the OP means by bootstrap). We spent quite some time figuring out how to handle that when we were working on newsysviews. In that case, we wanted the info because it's handy to be able to query a view that's not cluttered up with a bunch of system-defined stuff. Having a way to get a list of only user-defined functions, for example. -- Jim C. Nasby, Database Architect [EMAIL PROTECTED] 512.569.9461 (cell) http://jim.nasby.net ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [HACKERS] [PATCHES] Use of backslash in tsearch2
Thanks. Yes, it is need for two reasons. In 8.2 you can set standard_conforming_strings to on, meaning \' is really treated as \ and ', and because some encodings now can't support \' for security reasons, though I don't think tsearch2 supports those multibyte encodings. Anyway, applied to 8.2 only, not backpatched. Thanks. --- Teodor Sigaev wrote: > > Patch isn't full, simple test (values are took from regression.diffs): > > and try dump table and restore: > > ERROR: syntax error > > CONTEXT: COPY tt, line 5, column tq: "'1 ''2'" > > > > Attached cumulative patch fixes problem, but I have some doubts, is it really > needed? > > > -- > Teodor Sigaev E-mail: [EMAIL PROTECTED] > WWW: http://www.sigaev.ru/ [ application/x-tar is not supported, skipping... ] > > ---(end of broadcast)--- > TIP 6: explain analyze is your friend -- 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] extension for sql update
Susanne Ebrecht wrote:
> >> Is it too hard to rip it back out once the full row support
> >> arrives? That seems speculation at best anyway.
> >>
> > That's what I was thinking. Glad someone else replied. ;-)
> >
> If you're looking for votes, +1. I'll gladly take a subset of the
> SQL standard UPDATE table SET (...) = (...) over having nothing.
>
> >>> +1 here, too. :)
> >>>
> >>>
> >> +1
> >>
> >
> > I am working now to get this into 8.2.
> >
> >
> I am glad to read this. But what does it mean to me? Shall I change the
> patch someway?
I have merged your patch into current CVS and applied it; attached.
There was quite a bit of code drift. One drift area was the new
RETURNING clause; that was easy to fix. A more complex case is the
code no longer has values as ResTargets --- it is a simple a_expr list,
so I changed the critical assignment in gram.y from:
res_col->val = (Node *)copyObject(res_val->val);
to:
res_col->val = (Node *)copyObject(res_val);
Hope that is OK. Without that fix, it crashed. I also merged your SGML
syntax and grammer addition into the exiting UPDATE main entry.
--
Bruce Momjian [EMAIL PROTECTED]
EnterpriseDBhttp://www.enterprisedb.com
+ If your life is a hard drive, Christ can be your backup. +
Index: doc/src/sgml/ref/update.sgml
===
RCS file: /cvsroot/pgsql/doc/src/sgml/ref/update.sgml,v
retrieving revision 1.38
retrieving revision 1.39
diff -c -r1.38 -r1.39
*** doc/src/sgml/ref/update.sgml 12 Aug 2006 02:52:03 - 1.38
--- doc/src/sgml/ref/update.sgml 2 Sep 2006 20:34:47 - 1.39
***
*** 21,27
UPDATE [ ONLY ] table [ [ AS ] alias ]
! SET column = { expression | DEFAULT } [, ...]
[ FROM fromlist ]
[ WHERE condition ]
[ RETURNING * | output_expression [ AS output_name ] [, ...] ]
--- 21,28
UPDATE [ ONLY ] table [ [ AS ] alias ]
! [ SET column = { expression | DEFAULT } [, ...] |
! SET ( column [, ...] ) = ( { expression | DEFAULT } [, ...] ) [, ...] ]
[ FROM fromlist ]
[ WHERE condition ]
[ RETURNING * | output_expression [ AS output_name ] [, ...] ]
***
*** 251,256
--- 252,261
UPDATE weather SET temp_lo = temp_lo+1, temp_hi = temp_lo+15, prcp = DEFAULT
WHERE city = 'San Francisco' AND date = '2003-07-03';
+
+ UPDATE weather SET (temp_lo, temp_hi, prcp) = (temp_lo+1, temp_lo+15, DEFAULT)
+ WHERE city = 'San Francisco' AND date = '2003-07-03';
+
Index: src/backend/parser/gram.y
===
RCS file: /cvsroot/pgsql/src/backend/parser/gram.y,v
retrieving revision 2.560
retrieving revision 2.562
diff -c -r2.560 -r2.562
*** src/backend/parser/gram.y 2 Sep 2006 18:17:17 - 2.560
--- src/backend/parser/gram.y 2 Sep 2006 20:52:01 - 2.562
***
*** 237,243
name_list from_clause from_list opt_array_bounds
qualified_name_list any_name any_name_list
any_operator expr_list attrs
! target_list update_target_list insert_column_list
values_list def_list indirection opt_indirection
group_clause TriggerFuncArgs select_limit
opt_select_limit opclass_item_list
--- 237,244
name_list from_clause from_list opt_array_bounds
qualified_name_list any_name any_name_list
any_operator expr_list attrs
! target_list update_col_list update_target_list
! update_value_list set_opt insert_column_list
values_list def_list indirection opt_indirection
group_clause TriggerFuncArgs select_limit
opt_select_limit opclass_item_list
***
*** 308,314
%type joined_table
%type relation_expr
%type relation_expr_opt_alias
! %type target_el update_target_el insert_column_item
%type Typename SimpleTypename ConstTypename
GenericType Numeric opt_float
--- 309,316
%type joined_table
%type relation_expr
%type relation_expr_opt_alias
! %type target_el update_target_el update_col_list_el insert_column_item
! %type update_target_lists_list update_target_lists_el
%type Typename SimpleTypename ConstTypename
GenericType Numeric opt_float
***
*** 5524,5530
*/
UpdateStmt: UPDATE relation_expr_opt_alias
! SET update_target_list
from_clause
where_clause
returning_clause
--- 5526,5532
*/
UpdateStmt: UPDATE relation_expr_opt_alias
! SET set_opt
from_clause
where_clause
returning_clause
***
*** 5539,5544
--- 5541,5551
}
;
+ set_opt:
+ update_target_list { $$ = $1; }
+ | update_target_lists_list{ $$ = $1; }
+ ;
+
/***
Re: [HACKERS] [PATCHES] Resurrecting per-page cleaner for
Bruce Momjian <[EMAIL PROTECTED]> writes: > Tom, should I apply this patch now? Are you still considering other > options for this? Please wait. This issue is very far down the to-list in terms of size or significance ... but I'll get to it. 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] [PATCHES] Resurrecting per-page cleaner for
Tom, should I apply this patch now? Are you still considering other options for this? --- Bruce Momjian wrote: > > Tom, I ran your tests with fsync off (as you did), and saw numbers > bouncing between 400-700 tps without my patch, and sticking at 700 tps > with my patch. > > --- > > Bruce Momjian wrote: > > > > The attached patch requires the new row to fit, and 10% to be free on > > the page. Would someone test that? > > > > --- > > > > Tom Lane wrote: > > > ITAGAKI Takahiro <[EMAIL PROTECTED]> writes: > > > > This is a revised patch originated by Junji TERAMOTO for HEAD. > > > > [BTree vacuum before page splitting] > > > > http://archives.postgresql.org/pgsql-patches/2006-01/msg00301.php > > > > I think we can resurrect his idea because we will scan btree pages > > > > at-atime now; the missing-restarting-point problem went away. > > > > > > I've applied this but I'm now having some second thoughts about it, > > > because I'm seeing an actual *decrease* in pgbench numbers from the > > > immediately prior CVS HEAD code. Using > > > pgbench -i -s 10 bench > > > pgbench -c 10 -t 1000 bench (repeat this half a dozen times) > > > with fsync off but all other settings factory-stock, what I'm seeing > > > is that the first run looks really good but subsequent runs tail off in > > > spectacular fashion :-( Pre-patch there was only minor degradation in > > > successive runs. > > > > > > What I think is happening is that because pgbench depends so heavily on > > > updating existing records, we get into a state where an index page is > > > about full and there's one dead tuple on it, and then for each insertion > > > we have > > > > > > * check for uniqueness marks one more tuple dead (the > > > next-to-last version of the tuple) > > > * newly added code removes one tuple and does a write > > > * now there's enough room to insert one tuple > > > * lather, rinse, repeat, never splitting the page. > > > > > > The problem is that we've traded splitting a page every few hundred > > > inserts for doing a PageIndexMultiDelete, and emitting an extra WAL > > > record, on *every* insert. This is not good. > > > > > > Had you done any performance testing on this patch, and if so what > > > tests did you use? I'm a bit hesitant to try to fix it on the basis > > > of pgbench results alone. > > > > > > One possible fix that comes to mind is to only perform the cleanup > > > if we are able to remove more than one dead tuple (perhaps about 10 > > > would be good). Or do the deletion anyway, but then go ahead and > > > split the page unless X amount of space has been freed (where X is > > > more than just barely enough for the incoming tuple). > > > > > > After all the thought we've put into this, it seems a shame to > > > just abandon it :-(. But it definitely needs more tweaking. > > > > > > regards, tom lane > > > > > > ---(end of broadcast)--- > > > TIP 4: Have you searched our list archives? > > > > > >http://archives.postgresql.org > > > > -- > > Bruce Momjian [EMAIL PROTECTED] > > EnterpriseDBhttp://www.enterprisedb.com > > > > + If your life is a hard drive, Christ can be your backup. + > > > > > > ---(end of broadcast)--- > > TIP 2: Don't 'kill -9' the postmaster > > -- > Bruce Momjian [EMAIL PROTECTED] > EnterpriseDBhttp://www.enterprisedb.com > > + If your life is a hard drive, Christ can be your backup. + -- Bruce Momjian [EMAIL PROTECTED] EnterpriseDBhttp://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. + ---(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] [PATCHES] Contrib module to examine client
Tom Lane wrote: > Bruce Momjian <[EMAIL PROTECTED]> writes: > > I assume this is something we want in /contrib, right? > > Peter posted an updated version, I believe. Ah, it was lower in my mailbox. Thanks. -- Bruce Momjian [EMAIL PROTECTED] EnterpriseDBhttp://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. + ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [HACKERS] [PATCHES] DOC: catalog.sgml
Tom Lane wrote: > Zdenek Kotala <[EMAIL PROTECTED]> writes: > > I little bit enhanced overview catalog tables. I added two new columns. > > First one is OID of catalog table and second one contains attributes > > which determine if the table is bootstrap, with oid and global. > > Why is this a good idea? It seems like mere clutter. What's "global"? A maybe-useful flag would be telling that a table is shared. Is that it? Mind you, it's not useful to me because I know which tables are shared, but I guess for someone not so familiar with the catalogs it could have some use. The OIDs may be useful to people inspecting pg_depend, for example; but then, it's foolish not to be using regclass in that case. Whether a table is "bootstrap" or not doesn't seem useful to me. -- 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: [HACKERS] [PATCHES] log_statement output for protocol
Guillaume Smet wrote: > On 8/30/06, Bruce Momjian <[EMAIL PROTECTED]> wrote: > > I thought about this, and because we are placing two pieces of > > information on the same line, it seems "|" is the best choice. > > Good idea. It's far more readable with a pipe. > > > Oh. You want to pull the parameters out of that. I am thinking you > > need something that will go over the line character by character with > > some type of state machine, rather than just regex. > > Yes, that's what I did but I usually prefer a regex. > > > Additional comments? > > I confirm it now works with NULL. I'm just wondering if the notation > is really consistent: > $result = pg_execute($dbconn, "insert_query", array(null)); > gives: > DETAIL: prepare: INSERT INTO shop (name) VALUES($1) | bind: $1 = NULL > However: > $result = pg_execute($dbconn, "insert_query", array(4)); > gives: > DETAIL: prepare: INSERT INTO shop (name) VALUES($1) | bind: $1 = '4' > > But I don't think it's possible to have 4 in this case. Can you confirm? All supplied parameters have single quotes around them. Only NULL doesn't. > I have all the different cases parsed correctly by my parser and I can > build the query from the logs so it's OK for me. In the above case, > with an int, I remove the quotes if the content is numeric. It's not > perfect but I suppose it will be OK most of the time. Well, the parameter is supplied as text, so I always quote it in the logs. -- 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: [HACKERS] [PATCHES] log_statement output for protocol
On 8/30/06, Bruce Momjian <[EMAIL PROTECTED]> wrote: I thought about this, and because we are placing two pieces of information on the same line, it seems "|" is the best choice. Good idea. It's far more readable with a pipe. Oh. You want to pull the parameters out of that. I am thinking you need something that will go over the line character by character with some type of state machine, rather than just regex. Yes, that's what I did but I usually prefer a regex. Additional comments? I confirm it now works with NULL. I'm just wondering if the notation is really consistent: $result = pg_execute($dbconn, "insert_query", array(null)); gives: DETAIL: prepare: INSERT INTO shop (name) VALUES($1) | bind: $1 = NULL However: $result = pg_execute($dbconn, "insert_query", array(4)); gives: DETAIL: prepare: INSERT INTO shop (name) VALUES($1) | bind: $1 = '4' But I don't think it's possible to have 4 in this case. Can you confirm? I have all the different cases parsed correctly by my parser and I can build the query from the logs so it's OK for me. In the above case, with an int, I remove the quotes if the content is numeric. It's not perfect but I suppose it will be OK most of the time. -- 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] [PATCHES] Updatable views
Peter Eisentraut <[EMAIL PROTECTED]> writes:
> Am Donnerstag, 31. August 2006 15:55 schrieb Tom Lane:
>> The proposed WITH CHECK OPTION implementation is unworkable for exactly
>> this reason --- it will give the wrong answers in the presence of
>> volatile functions such as nextval().
> I'm not sure why anyone would want to define a view condition containing a
> volatile function. At least it wouldn't put a major dent into this feature
> if such views were decreed not updatable.
The problem is not with the view condition. Consider
CREATE TABLE data (id serial primary key, ...);
CREATE VIEW only_new_data AS SELECT * FROM data WHERE id > 12345
WITH CHECK OPTION;
INSERT INTO only_new_data VALUES(nextval('data_id_seq'), ...);
The proposed implementation will execute nextval twice (bad), and will
apply the WITH CHECK OPTION test to the value that isn't the one stored
(much worse). It doesn't help if the id is defaulted.
regards, tom lane
---(end of broadcast)---
TIP 3: Have you checked our extensive FAQ?
http://www.postgresql.org/docs/faq
Re: [HACKERS] [PATCHES] Updatable views
Am Donnerstag, 31. August 2006 15:55 schrieb Tom Lane: > >> I'm unclear as to why you've got DO INSTEAD NOTHING rules in there --- > > > > You need to have one unconditional rule if you have a bunch of > > conditional ones. The system does not see through the fact that the > > conditional ones cover all cases. > > AFAICS, for the cases we are able to implement within the existing rule > mechanism, there should be exactly one unconditional rule. If you > propose more, then you are going to have insurmountable problems with > the usual sorts of multiple-evaluation risks. I'm not sure what you are saying here ... The implementation creates, for each of the three actions INSERT, UPDATE, DELETE, one conditional rule that redirects the action from the view into the unterlying table, conditional on the view condition being fulfilled. The unconditional DO INSTEAD NOTHING rule then catches the cases where the view condition is not fulfilled. So there is, for each action, exactly one conditional and one unconditional rule. Which is consistent with what you said above, so I don't see the problem. > The proposed WITH CHECK OPTION implementation is unworkable for exactly > this reason --- it will give the wrong answers in the presence of > volatile functions such as nextval(). I'm not sure why anyone would want to define a view condition containing a volatile function. At least it wouldn't put a major dent into this feature if such views were decreed not updatable. -- Peter Eisentraut http://developer.postgresql.org/~petere/ ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
Re: [HACKERS] [PATCHES] Updatable views
Peter Eisentraut <[EMAIL PROTECTED]> writes: > Am Mittwoch, 30. August 2006 18:01 schrieb Tom Lane: >> This is the first time I've actually looked at this patch, and I am >> dismayed. viewUpdate.c looks like nothing so much as a large program >> with a small program struggling to get out. > But later SQL versions allow some of that, so at least it shouldn't hurt to > have some parts of the code to be more general in preparation of that. If it bloats the code to unreadability, it's bad. >> I'm unclear as to why you've got DO INSTEAD NOTHING rules in there --- > You need to have one unconditional rule if you have a bunch of conditional > ones. The system does not see through the fact that the conditional ones > cover all cases. AFAICS, for the cases we are able to implement within the existing rule mechanism, there should be exactly one unconditional rule. If you propose more, then you are going to have insurmountable problems with the usual sorts of multiple-evaluation risks. The proposed WITH CHECK OPTION implementation is unworkable for exactly this reason --- it will give the wrong answers in the presence of volatile functions such as nextval(). I believe that we cannot implement WITH CHECK OPTION as a rule. It's a constraint, instead, and will have to be checked the way the executor presently checks constraints, ie after forming the finished new tuple(s). (Someday we're going to have to look into redesigning the rule system so that it can cope better with the kinds of situations that give rise to multiple-evaluation problems. But today is not that day.) It's possible that if we strip the patch down to SQL92-equivalent functionality (no multiple base rels) without WITH CHECK OPTION, we would have something that would work reliably atop the existing rule mechanism. It's getting mighty late in the 8.2 cycle to be doing major rework though. regards, tom lane ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [HACKERS] [PATCHES] Updatable views
Am Mittwoch, 30. August 2006 18:01 schrieb Tom Lane: > This is the first time I've actually looked at this patch, and I am > dismayed. viewUpdate.c looks like nothing so much as a large program > with a small program struggling to get out. What is all the stuff about > handling multiple base rels? SQL92, at least, does not say that a join > is updatable, and AFAICT this patch is rejecting that too ... But later SQL versions allow some of that, so at least it shouldn't hurt to have some parts of the code to be more general in preparation of that. > I'm unclear as to why you've got DO INSTEAD NOTHING rules in there --- You need to have one unconditional rule if you have a bunch of conditional ones. The system does not see through the fact that the conditional ones cover all cases. > The pg_dump changes seem pretty odd too. Why wouldn't you just > ignore implicit rules during a dump, expecting the system to > regenerate them when the view is reloaded? Right. -- Peter Eisentraut http://developer.postgresql.org/~petere/ ---(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
Guillaume Smet wrote:
> On 8/29/06, Bruce Momjian <[EMAIL PROTECTED]> wrote:
> > Good point. I thought it was clear enough, but obviously not. I had a
> > similar case with bind, and used a comma to separate them:
> >
> > LOG: statement: prepare sel1, SELECT $1;
> > LOG: statement: bind sel1, $1 = 'a''b'
>
> For this one, I must admit I prefer the colon we used before.
> Something like:
> LOG: statement: prepare sel1: SELECT $1;
> LOG: statement: bind sel1: $1 = 'a''b'
> seems better to me as after the colon, we have the details of the
> command which is the common sense of a colon.
OK, done.
> > I am concerned a dash isn't clear enough, and a semicolon is confusing.
> > Using a comma the new output is:
> >
> > LOG: duration: 0.023 ms execute sel1
> > DETAIL: prepare: SELECT $1;, bind: $1 = 'a''b'
>
> A dash is clearer in this case IMHO. ;, is not very readable. But I
> can parse easily both forms so it's not a problem for me if you prefer
> a comma.
I thought about this, and because we are placing two pieces of
information on the same line, it seems "|" is the best choice.
> > Is that OK? Patch attached and committed. I also fixed the null bind
> > parameter bug. It now displays $1 = NULL (no quotes used).
>
> Cool. I'll test that.
>
> > Other suggestions?
>
> I'll compile this new version and make tests in the next few days to
> check everything is consistent and let you know.
>
> I'm still struggling to find a regexp good enough to parse "statement:
> execute y ('a', 4, 'text, with a comma'::text);" :).
Oh. You want to pull the parameters out of that. I am thinking you
need something that will go over the line character by character with
some type of state machine, rather than just regex.
> Thanks a lot for your work on this subject. It will help us a lot when
> we use the JDBC driver.
Patch attached and applied. New output:
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;
LOG: statement: set log_statement = 'none';
LOG: duration: 0.222 ms statement: set log_min_duration_statement = 0;
LOG: duration: 0.061 ms statement: begin;
LOG: duration: 0.113 ms statement: prepare y as select $1::text;
LOG: duration: 0.213 ms statement: execute y ('a');
DETAIL: prepare: prepare y as select $1::text;
LOG: duration: 0.081 ms statement: commit;
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'
LOG: duration: 0.445 ms statement: SET log_min_duration_statement = 0;
LOG: duration: 0.018 ms execute sel1
DETAIL: prepare: SELECT $1; | bind: $1 = 'a''b'
Additional comments?
--
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.502
diff -c -c -r1.502 postgres.c
*** src/backend/tcop/postgres.c 29 Aug 2006 20:10:42 - 1.502
--- src/backend/tcop/postgres.c 30 Aug 2006 17:59:05 -
***
*** 1146,1152
if (log_statement == LOGSTMT_ALL)
ereport(LOG,
! (errmsg("statement: prepare %s, %s",
*stmt_name ? stmt_name : "",
query_string)));
--- 1146,1152
if (log_statement == LOGSTMT_ALL)
ereport(LOG,
! (errmsg("statement: prepare %s: %s",
*stmt_name ? stmt_name : "",
query_string)));
***
*** 1621,1627
*portal->name ? "/" : "",
*portal->name ? portal->name : "",
/* print bind parameters if we have them */
! bind_values_str.len ? ", " : "",
bind_values_str.len ? bind_values_str.data : ""),
errdetail("prepare: %s", pstmt->query_string)));
}
--- 1621,1627
*portal->name ? "/" : "",
*portal->name ? portal->name : "",
/* print bind parameters if we have them */
! bind_values_str.len ? ": " : "",
bind_values_str.len ? bind_values_str.data : ""),
errdetail("prepare: %s", pstmt->query_string)));
}
***
*** 1788,1794
*portal_name ? portal_name : ""),
errdetail("prepare: %s%s%s", sourceText,
/* optionally print bind parameters */
! bindText ? ", bind: " : "",
bindText ? bindText : "")));
BeginCommand(portal->commandTag, dest);
--- 1788,1794
*portal_name ? portal_name : ""),
errdetail("prepare: %s%s%s", sourceText,
/* optionally print bind parameters */
! bindText ? "
Re: [HACKERS] [PATCHES] Updatable views
On Wed, Aug 30, 2006 at 12:01:25PM -0400, Tom Lane wrote: > Bernd Helmle <[EMAIL PROTECTED]> writes: > > [ latest views patch ] > > This is the first time I've actually looked at this patch, and I am > dismayed. viewUpdate.c looks like nothing so much as a large program > with a small program struggling to get out. What is all the stuff about > handling multiple base rels? SQL92, at least, does not say that a join > is updatable, and AFAICT this patch is rejecting that too ... though > it's hard to tell with the conditions for allowing the join to be > updatable scattered through a lot of different functions. And some of > the code seems to be expecting multiple implicit rules and other parts > not. I get the impression that a lot of this code is left over from a > more ambitious first draft and ought to be removed in the name of > readability/maintainability. If that code is on the right path to allowing things like updates to the many side of a join then it would be worth adding comments to that effect. Or maybe a comment referencing whatever version of the file the code was yanked out of. -- Jim C. Nasby, Sr. Engineering Consultant [EMAIL PROTECTED] Pervasive Software http://pervasive.comwork: 512-231-6117 vcard: http://jim.nasby.net/pervasive.vcf cell: 512-569-9461 ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [HACKERS] [PATCHES] updated patch for selecting large results
On Tue, 2006-08-29 at 18:31 -0400, Tom Lane wrote: > <[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 > > Applied with revisions ... I didn't like the fact that the code was > restricted to handle only unaligned output format, so I fixed print.c > to be able to deal with emitting output in sections. This is not > ideal for aligned output mode, because we compute column widths > separately for each FETCH group, but all the other output modes work > nicely. I also did a little hacking to make \timing and pager output > work as expected. > > regards, tom lane Cool! I specially like that as a side effect of your work for applying this, psql is faster now. Thanks to all people that helped with this (lots...:) Bye, Chris. -- Chris Mair http://www.1006.org ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [HACKERS] [PATCHES] log_statement output for protocol
On 8/29/06, Bruce Momjian <[EMAIL PROTECTED]> wrote:
Good point. I thought it was clear enough, but obviously not. I had a
similar case with bind, and used a comma to separate them:
LOG: statement: prepare sel1, SELECT $1;
LOG: statement: bind sel1, $1 = 'a''b'
For this one, I must admit I prefer the colon we used before.
Something like:
LOG: statement: prepare sel1: SELECT $1;
LOG: statement: bind sel1: $1 = 'a''b'
seems better to me as after the colon, we have the details of the
command which is the common sense of a colon.
I am concerned a dash isn't clear enough, and a semicolon is confusing.
Using a comma the new output is:
LOG: duration: 0.023 ms execute sel1
DETAIL: prepare: SELECT $1;, bind: $1 = 'a''b'
A dash is clearer in this case IMHO. ;, is not very readable. But I
can parse easily both forms so it's not a problem for me if you prefer
a comma.
Is that OK? Patch attached and committed. I also fixed the null bind
parameter bug. It now displays $1 = NULL (no quotes used).
Cool. I'll test that.
Other suggestions?
I'll compile this new version and make tests in the next few days to
check everything is consistent and let you know.
I'm still struggling to find a regexp good enough to parse "statement:
execute y ('a', 4, 'text, with a comma'::text);" :).
Thanks a lot for your work on this subject. It will help us a lot when
we use the JDBC driver.
--
Guillaume
---(end of broadcast)---
TIP 2: Don't 'kill -9' the postmaster
Re: [HACKERS] [PATCHES] log_statement output for protocol
Guillaume Smet wrote:
> On 8/29/06, Bruce Momjian <[EMAIL PROTECTED]> wrote:
> > DETAIL: prepare: SELECT $1; bind: $1 = 'a''b'
>
> I attached a trivial patch to add a dash between the prepare part and
> the bind part. People usually don't finish their queries with a semi
> colon so it's more readable with a separator.
> DETAIL: prepare: SELECT $1 bind: $1 = 'a''b'
> becomes
> DETAIL: prepare: SELECT $1 - bind: $1 = 'a''b'
Good point. I thought it was clear enough, but obviously not. I had a
similar case with bind, and used a comma to separate them:
LOG: statement: prepare sel1, SELECT $1;
LOG: statement: bind sel1, $1 = 'a''b'
I am concerned a dash isn't clear enough, and a semicolon is confusing.
Using a comma the new output is:
LOG: duration: 0.023 ms execute sel1
DETAIL: prepare: SELECT $1;, bind: $1 = 'a''b'
or with no semicolon:
LOG: duration: 0.023 ms execute sel1
DETAIL: prepare: SELECT $1, bind: $1 = 'a''b'
Is that OK? Patch attached and committed. I also fixed the null bind
parameter bug. It now displays $1 = NULL (no quotes used). Other
suggestions?
--
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.501
diff -c -c -r1.501 postgres.c
*** src/backend/tcop/postgres.c 29 Aug 2006 02:32:41 - 1.501
--- src/backend/tcop/postgres.c 29 Aug 2006 19:54:08 -
***
*** 1539,1555
-1);
/* Save the parameter values */
! appendStringInfo(&bind_values_str, "%s$%d = '",
bind_values_str.len ? ", " : "",
paramno + 1);
! for (p = pstring; *p; p++)
{
! if (*p == '\'') /* double single quotes */
appendStringInfoChar(&bind_values_str, *p);
! appendStringInfoChar(&bind_values_str, *p);
}
! appendStringInfoChar(&bind_values_str, '\'');
!
/* Free result of encoding conversion, if any */
if (pstring && pstring != pbuf.data)
pfree(pstring);
--- 1539,1561
-1);
/* Save the parameter values */
! appendStringInfo(&bind_values_str, "%s$%d = ",
bind_values_str.len ? ", " : "",
paramno + 1);
! if (pstring)
{
! appendStringInfoChar(&bind_values_str, '\'');
! for (p = pstring; *p; p++)
! {
! if (*p == '\'') /* double single quotes */
! appendStringInfoChar(&bind_values_str, *p);
appendStringInfoChar(&bind_values_str, *p);
! }
! appendStringInfoChar(&bind_values_str, '\'');
}
! else
! appendStringInfo(&bind_values_str, "NULL");
!
/* Free result of encoding conversion, if any */
if (pstring && pstring != pbuf.data)
pfree(pstring);
***
*** 1782,1788
*portal_name ? portal_name : ""),
errdetail("prepare: %s%s%s", sourceText,
/* optionally print bind parameters */
! bindText ? " bind: " : "",
bindText ? bindText : "")));
BeginCommand(portal->commandTag, dest);
--- 1788,1794
*portal_name ? portal_name : ""),
errdetail("prepare: %s%s%s", sourceText,
/* optionally print bind parameters */
! bindText ? ", bind: " : "",
bindText ? bindText : "")));
BeginCommand(portal->commandTag, dest);
***
*** 1896,1902
*portal_name ? portal_name : ""),
errdetail("prepare: %s%s%s", sourceText,
/* optionally print bind parameters */
! bindText ? " bind: " : "",
bindText ? bindText : "")));
}
}
--- 1902,1908
*portal_name ? portal_name : ""),
errdetail("prepare: %s%s%s", sourceText,
/* optionally print bind parameters */
! bindText ? ", bind: " : "",
bindText ? bindText : "")));
}
}
---(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
On 8/29/06, Bruce Momjian <[EMAIL PROTECTED]> wrote:
DETAIL: prepare: SELECT $1; bind: $1 = 'a''b'
I attached a trivial patch to add a dash between the prepare part and
the bind part. People usually don't finish their queries with a semi
colon so it's more readable with a separator.
DETAIL: prepare: SELECT $1 bind: $1 = 'a''b'
becomes
DETAIL: prepare: SELECT $1 - bind: $1 = 'a''b'
--
Guillaume
Index: src/backend/tcop/postgres.c
===
RCS file: /projects/cvsroot/pgsql/src/backend/tcop/postgres.c,v
retrieving revision 1.501
diff -c -r1.501 postgres.c
*** src/backend/tcop/postgres.c 29 Aug 2006 02:32:41 - 1.501
--- src/backend/tcop/postgres.c 29 Aug 2006 11:46:15 -
***
*** 1782,1788
*portal_name ? portal_name :
""),
errdetail("prepare: %s%s%s",
sourceText,
/* optionally print bind
parameters */
! bindText ? " bind: " : "",
bindText ? bindText : "")));
BeginCommand(portal->commandTag, dest);
--- 1782,1788
*portal_name ? portal_name :
""),
errdetail("prepare: %s%s%s",
sourceText,
/* optionally print bind
parameters */
! bindText ? " - bind: " : "",
bindText ? bindText : "")));
BeginCommand(portal->commandTag, dest);
***
*** 1896,1902
*portal_name ?
portal_name : ""),
errdetail("prepare: %s%s%s", sourceText,
/* optionally
print bind parameters */
! bindText ? "
bind: " : "",
bindText ?
bindText : "")));
}
}
--- 1896,1902
*portal_name ?
portal_name : ""),
errdetail("prepare: %s%s%s", sourceText,
/* optionally
print bind parameters */
! bindText ? " -
bind: " : "",
bindText ?
bindText : "")));
}
}
---(end of broadcast)---
TIP 2: Don't 'kill -9' the postmaster
Re: [HACKERS] [PATCHES] log_statement output for protocol
Bruce, I made a few tests here and the backend terminates with a SIG11 when a parameter has the NULL value (it was logged as "(null)" before). I suspect the new code broke something (perhaps it's due to the escaping). -- Guillaume ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
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
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 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
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
log_statement is all. Bind parameter
values are also logged if they are supplied in text
! format.
The default is none. Only superusers can change this
--- 2839,2845
prepare, bind, and execute commands are logged only if
log_statement is all. Bind parameter
values are also logged if they are supplied in text
! format (literal single quotes are doubled).
The default is none. 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 -c -c -r1.50 portalcmds.c
*** src/backend/commands/po
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: [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 (nindex
Re: [HACKERS] [PATCHES] Trivial patch to double vacuum speed on tables with no indexes
Gregory Stark <[EMAIL PROTECTED]> writes: > Tom Lane <[EMAIL PROTECTED]> writes: >> How often does that case come up in the real world, for tables that are >> large enough that you'd care about vacuum performance? > I would have had the same objection if it resulted in substantially more > complex code but it was so simple that it doesn't seem like a concern. 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. regards, tom lane ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [HACKERS] [PATCHES] log_statement output for protocol
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? Regards, -- Guillaume ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [HACKERS] [PATCHES] psql 'none' as a HISTFILE special case
On 8/25/06, Tom Lane <[EMAIL PROTECTED]> wrote: "Martin Atukunda" <[EMAIL PROTECTED]> writes: > On 8/25/06, Tom Lane <[EMAIL PROTECTED]> wrote: >> There's probably no way to get Apple's libedit to not try the fchmod, >> so what do we want to do here? Maybe special-case the string >> "/dev/null"? > If this is OK, I can up with a patch that special cases /dev/null as a > HISTFILE if libedit is found. I was thinking of basically a one-liner addition to write_history to skip the whole thing if strcmp(fname, DEVNULL) == 0. Should be reasonably inoffensive on anyone's machine. I guess you meant saveHistory instead of write_history here. :) something like the attached diff - Martin - special_case_DEVNULL.diff Description: Binary data ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [HACKERS] [PATCHES] psql 'none' as a HISTFILE special case
"Martin Atukunda" <[EMAIL PROTECTED]> writes: > On 8/25/06, Tom Lane <[EMAIL PROTECTED]> wrote: >> There's probably no way to get Apple's libedit to not try the fchmod, >> so what do we want to do here? Maybe special-case the string >> "/dev/null"? > If this is OK, I can up with a patch that special cases /dev/null as a > HISTFILE if libedit is found. I was thinking of basically a one-liner addition to write_history to skip the whole thing if strcmp(fname, DEVNULL) == 0. Should be reasonably inoffensive on anyone's machine. regards, tom lane ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [HACKERS] [PATCHES] psql 'none' as a HISTFILE special case
On 8/25/06, Tom Lane <[EMAIL PROTECTED]> wrote: > When I set HISTFILE to /dev/null I get the following: > could not save history to file "/dev/null": Operation not permitted Hm. ktrace shows this happening: 23279 psql CALL open(0x302d70,0x601,0x1b6) 23279 psql NAMI "/dev/null" 23279 psql RET open 3 23279 psql CALL fchmod(0x3,0x180) 23279 psql RET fchmod -1 errno 1 Operation not permitted 23279 psql CALL close(0x3) 23279 psql RET close 0 23279 psql CALL write(0x2,0xb180,0x44) 23279 psql GIO fd 2 wrote 68 bytes "could not save history to file "/dev/null": Operation not permitted " 23279 psql RET write 68/0x44 23279 psql CALL exit(0) There's probably no way to get Apple's libedit to not try the fchmod, so what do we want to do here? Maybe special-case the string "/dev/null"? If this is OK, I can up with a patch that special cases /dev/null as a HISTFILE if libedit is found. - Martin - ---(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] psql 'none' as a HISTFILE special case
"Martin Atukunda" <[EMAIL PROTECTED]> writes:
> On 8/25/06, Peter Eisentraut <[EMAIL PROTECTED]> wrote:
>> Please elaborate on "doesn't work".
> without any .psqlrc file I get the following error when quitting a psql
> session:
> could not save history to file "/Users/matlads/.psql_history": Invalid
> argument
That is fixed in CVS HEAD. The current coding looks like:
/*
* return value of write_history is not standardized across GNU
* readline and libedit. Therefore, check for errno becoming set
* to see if the write failed.
*/
errno = 0;
(void) write_history(fname);
if (errno == 0)
return true;
psql_error("could not save history to file \"%s\": %s\n",
fname, strerror(errno));
> When I set HISTFILE to /dev/null I get the following:
> could not save history to file "/dev/null": Operation not permitted
Hm. ktrace shows this happening:
23279 psql CALL open(0x302d70,0x601,0x1b6)
23279 psql NAMI "/dev/null"
23279 psql RET open 3
23279 psql CALL fchmod(0x3,0x180)
23279 psql RET fchmod -1 errno 1 Operation not permitted
23279 psql CALL close(0x3)
23279 psql RET close 0
23279 psql CALL write(0x2,0xb180,0x44)
23279 psql GIO fd 2 wrote 68 bytes
"could not save history to file "/dev/null": Operation not permitted
"
23279 psql RET write 68/0x44
23279 psql CALL exit(0)
There's probably no way to get Apple's libedit to not try the fchmod,
so what do we want to do here? Maybe special-case the string
"/dev/null"?
regards, tom lane
---(end of broadcast)---
TIP 6: explain analyze is your friend
Re: [HACKERS] [PATCHES] psql 'none' as a HISTFILE special case
On 8/25/06, Peter Eisentraut <[EMAIL PROTECTED]> wrote: Am Freitag, 25. August 2006 17:03 schrieb Martin Atukunda: > hmm, setting HISTFILE to /dev/null doesn't work on my MacOSX here. Please elaborate on "doesn't work". without any .psqlrc file I get the following error when quitting a psql session: could not save history to file "/Users/matlads/.psql_history": Invalid argument When I set HISTFILE to /dev/null I get the following: could not save history to file "/dev/null": Operation not permitted - Martin - ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [HACKERS] [PATCHES] psql 'none' as a HISTFILE special case
Am Freitag, 25. August 2006 17:03 schrieb Martin Atukunda: > hmm, setting HISTFILE to /dev/null doesn't work on my MacOSX here. Please elaborate on "doesn't work". -- Peter Eisentraut http://developer.postgresql.org/~petere/ ---(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] [PATCHES] Updatable views
--On Donnerstag, August 24, 2006 11:02:43 -0500 Jaime Casanova <[EMAIL PROTECTED]> wrote: Actually the code delete implicit rules based on a field added to pg_rewrite but that catalog has a unique index on ev_class, rulename: "pg_rewrite_rel_rulename_index" UNIQUE, btree (ev_class, rulename) i guess bernd's comment is about this index giving an error if we try to insert the new rule with the same name on the same event... No, this wasn't the problem, since we are going to drop any implicit rule that collides with an user defined one (however, this approach is discussable, but nobody has put his comments on this yet and i think this is important for backwards compatibility). I don't think we need ev_kind in the index at all, in my opinion implicit and user defined rules of the same event shouldn't live together (_RETURN rules are marked as implicit ones now, too) -- Thanks Bernd ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [HACKERS] [PATCHES] COPY view
Zoltan Boszormenyi Ãrta: Zoltan Boszormenyi Ãrta: The option parsing and error checking is now common. I also changed it to use transformStmt() in analyze.c. However, both the UNION and sunselect cases give me something like this: ERROR: could not open relation 1663/16384/16723: No such file or directory What else can I do with it? But a single SELECT with two tables joined also works so it must be something trivial. Now UNIONs and subselects also work. Your concern about copy_dest_printtup() wasn't solved yet. Best regards, Zoltán Böszörményi pgsql-copyselect-5.patch.gz Description: Unix tar archive ---(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] [PATCHES] Updatable views
--On Donnerstag, August 24, 2006 11:00:45 -0400 Tom Lane <[EMAIL PROTECTED]> wrote: If the code is dependent on recognizing names to know what it's doing, then I'd say you have a fundamentally broken approach. Consider adding a flag column to pg_rewrite to distinguish these rules, instead. This is the approach the code already follows (it uses an additional ev_kind column which distinguishes rules between implicit rules with no, local or cascaded check option and explicit ones). Turns out that i was thinking too difficult when looking at the code which drops implicit rulessorry for the noise. -- Thanks Bernd ---(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] [PATCHES] COPY view
Zoltan Boszormenyi Ãrta: The option parsing and error checking is now common. I also changed it to use transformStmt() in analyze.c. However, both the UNION and sunselect cases give me something like this: ERROR: could not open relation 1663/16384/16723: No such file or directory What else can I do with it? But a single SELECT with two tables joined also works so it must be something trivial. Best regards, Zoltán Böszörményi ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
Re: [HACKERS] [PATCHES] COPY view
Tom Lane Ãrta: Zoltan Boszormenyi <[EMAIL PROTECTED]> writes: How about the callback solution for the SELECT case that was copied from the original? Should I consider open-coding in copy.c what ExecutorRun() does to avoid the callback? Adding a DestReceiver type is a good solution ... although that static variable is not. Instead define a DestReceiver extension struct that can carry the CopyState pointer for you. Done. You could also consider putting the copy-from-view-specific state fields into DestReceiver instead of CopyState, though this is a bit asymmetric with the relation case so maybe it's not really cleaner. Left it alone for now. BTW, lose the tuple_to_values function --- it's an extremely bad reimplementation of heap_deform_tuple. Done. copy_dest_printtup also seems coded without regard for the TupleTableSlot access API (read printtup() to see what to do instead). I am still interpreting it. Can you give me some hints besides using slot_getallattrs(slot)? And what's the point of factoring out the heap_getnext loop as CopyRelationTo? It's not like that lets you share any more code. The inside of the loop, ie what you've called CopyValuesTo, is the sharable part. Done. The option parsing and error checking is now common. I also changed it to use transformStmt() in analyze.c. However, both the UNION and sunselect cases give me something like this: ERROR: could not open relation 1663/16384/16723: No such file or directory What else can I do with it? Best regards, Zoltán Böszörményi pgsql-copyselect-4.patch.gz Description: Unix tar archive ---(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] COPY view
Alvaro Herrera Ãrta: Zoltan Boszormenyi wrote: OK, here's my current version. The reference leak is fixed. But as my testcase shows, it only works for single selects currently. The parser accepts it but COPY doesn't produce the expected output. Please, suggest a solution. I'm not sure I agree with the approach of creating a fake "SELECT * FROM foo" in analyze.c in the relation case and passing it back to the parser to create a Query node. That's not there in the original code and you shouldn't need it. Just let the case where COPY gets a relation continue to handle it as it does today, and add a separate case for the SELECT. The exact same code was there, e.g. parse and rewrite "SELECT * FROM view" just not in analyze.c. I will try without it, though. That doesn't help you with the UNION stuff though. :-( ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [HACKERS] [PATCHES] COPY view
Zoltan Boszormenyi wrote: > OK, here's my current version. The reference leak is fixed. > But as my testcase shows, it only works for single selects > currently. The parser accepts it but COPY doesn't produce > the expected output. Please, suggest a solution. I'm not sure I agree with the approach of creating a fake "SELECT * FROM foo" in analyze.c in the relation case and passing it back to the parser to create a Query node. That's not there in the original code and you shouldn't need it. Just let the case where COPY gets a relation continue to handle it as it does today, and add a separate case for the SELECT. That doesn't help you with the UNION stuff though. -- Alvaro Herrerahttp://www.CommandPrompt.com/ PostgreSQL Replication, Consulting, Custom Development, 24x7 support ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [HACKERS] [PATCHES] COPY view
Zoltan Boszormenyi Ãrta: Hi, Bruce Momjian Ãrta: Alvaro Herrera wrote: Bruce Momjian wrote: Andrew Dunstan wrote: Bruce Momjian wrote: I think Alvaro is saying we need it in a few days, not longer. I thought he was saying today ;-) He actually said "now", but I don't think we need it immediately, especially if he is still working on it. We are at least 1-2 weeks away from having all open patches applied. Yes, I'm saying today so that we can all look at it and point obvious mistakes now, not in 2 weeks from now. Release early, release often. If the patch contains a mistake and we find out in 2 weeks, are we going to fix it? No, we are going to reject it. OK, I understand. B?sz?rm?nyi, post now so we can see where you are, but keep working and send it to us again when you are done. No sense in not posting your working version. OK, here's my current version. The reference leak is fixed. But as my testcase shows, it only works for single selects currently. The parser accepts it but COPY doesn't produce the expected output. Please, suggest a solution. I meant that UNION selects, subselects don't work yet. BTW, my first name is Zoltán. Best regards, Zoltán Böszörményi ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [HACKERS] [PATCHES] COPY view
Hi, Bruce Momjian Ãrta: Alvaro Herrera wrote: Bruce Momjian wrote: Andrew Dunstan wrote: Bruce Momjian wrote: I think Alvaro is saying we need it in a few days, not longer. I thought he was saying today ;-) He actually said "now", but I don't think we need it immediately, especially if he is still working on it. We are at least 1-2 weeks away from having all open patches applied. Yes, I'm saying today so that we can all look at it and point obvious mistakes now, not in 2 weeks from now. Release early, release often. If the patch contains a mistake and we find out in 2 weeks, are we going to fix it? No, we are going to reject it. OK, I understand. B?sz?rm?nyi, post now so we can see where you are, but keep working and send it to us again when you are done. No sense in not posting your working version. OK, here's my current version. The reference leak is fixed. But as my testcase shows, it only works for single selects currently. The parser accepts it but COPY doesn't produce the expected output. Please, suggest a solution. BTW, my first name is Zoltán. Best regards, Zoltán Böszörményi pgsql-copyselect.patch.gz Description: Unix tar archive ---(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] COPY view
Alvaro Herrera wrote: > Bruce Momjian wrote: > > Andrew Dunstan wrote: > > > Bruce Momjian wrote: > > > > I think Alvaro is saying we need it in a few days, not longer. > > > > > > I thought he was saying today ;-) > > > > He actually said "now", but I don't think we need it immediately, > > especially if he is still working on it. We are at least 1-2 weeks away > > from having all open patches applied. > > Yes, I'm saying today so that we can all look at it and point obvious > mistakes now, not in 2 weeks from now. Release early, release often. > If the patch contains a mistake and we find out in 2 weeks, are we going > to fix it? No, we are going to reject it. OK, I understand. B?sz?rm?nyi, post now so we can see where you are, but keep working and send it to us again when you are done. No sense in not posting your working version. -- Bruce Momjian [EMAIL PROTECTED] EnterpriseDBhttp://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. + ---(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] [PATCHES] COPY view
Bruce Momjian wrote: > Andrew Dunstan wrote: > > Bruce Momjian wrote: > > > I think Alvaro is saying we need it in a few days, not longer. > > > > I thought he was saying today ;-) > > He actually said "now", but I don't think we need it immediately, > especially if he is still working on it. We are at least 1-2 weeks away > from having all open patches applied. Yes, I'm saying today so that we can all look at it and point obvious mistakes now, not in 2 weeks from now. Release early, release often. If the patch contains a mistake and we find out in 2 weeks, are we going to fix it? No, we are going to reject it. -- 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: [HACKERS] [PATCHES] COPY view
Andrew Dunstan wrote: > Bruce Momjian wrote: > > I think Alvaro is saying we need it in a few days, not longer. > > > > > > I thought he was saying today ;-) He actually said "now", but I don't think we need it immediately, especially if he is still working on it. We are at least 1-2 weeks away from having all open patches applied. -- 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: [HACKERS] [PATCHES] COPY view
Bruce Momjian wrote: I think Alvaro is saying we need it in a few days, not longer. I thought he was saying today ;-) cheers andrew ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [HACKERS] [PATCHES] COPY view
> Böszörményi Zoltán wrote: >> > B?sz?rm?nyi Zolt?n wrote: >> > >> >> > So when will you send in a revised patch? >> >> >> >> Soon. :-) >> > >> > No, don't send it "soon". We're in feature freeze already (and have >> > been for three weeks). You need to send it now. >> >> I have to test it some more but I will send it. > > I think Alvaro is saying we need it in a few days, not longer. Of course. Best regards, Zoltán Böszörményi ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [HACKERS] [PATCHES] COPY view
B?sz?rm?nyi Zolt?n wrote: > > B?sz?rm?nyi Zolt?n wrote: > > > >> > So when will you send in a revised patch? > >> > >> Soon. :-) > > > > No, don't send it "soon". We're in feature freeze already (and have > > been for three weeks). You need to send it now. > > I have to test it some more but I will send it. I think Alvaro is saying we need it in a few days, not longer. -- 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: [HACKERS] [PATCHES] COPY view
> Böszörményi Zoltán wrote: > >> > So when will you send in a revised patch? >> >> Soon. :-) > > No, don't send it "soon". We're in feature freeze already (and have > been for three weeks). You need to send it now. I have to test it some more but I will send it. Best regards, Zoltán Böszörményi ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [HACKERS] [PATCHES] COPY view
Böszörményi Zoltán wrote: > > So when will you send in a revised patch? > > Soon. :-) No, don't send it "soon". We're in feature freeze already (and have been for three weeks). You need to send it now. -- Alvaro Herrerahttp://www.CommandPrompt.com/ PostgreSQL Replication, Consulting, Custom Development, 24x7 support ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [HACKERS] [PATCHES] COPY view
> Böszörményi Zoltán wrote: >> Hi, >> >> >>> Tom Lane wrote: >>> At the moment, with the online-index and updatable-views patches both pretty seriously broken, and no sign that the bitmap-index people are awake at all, I might take it on myself to fix this one instead of those others. But is that what I should be spending my time on in the waning days of the 8.2 freeze cycle? Speak now or hold your peace. >>> Personally, I would say that this is less important than updatable >>> views >>> but more than online indexes. If it could be fixed just for the view >>> case in a day or so then I think it's worth it. >>> >>> cheers >>> >>> andrew >>> >> >> It seems I was able to get it working for both the VIEW and SELECT >> cases. I still have one issue, the reference to the select is left open >> and it complains on closing the transaction. But basically works. >> >> >> > > So when will you send in a revised patch? > > cheers > > andrew Soon. :-) Best regards, Zoltán Böszörményi ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
Re: [HACKERS] [PATCHES] COPY view
Böszörményi Zoltán wrote: Hi, Tom Lane wrote: At the moment, with the online-index and updatable-views patches both pretty seriously broken, and no sign that the bitmap-index people are awake at all, I might take it on myself to fix this one instead of those others. But is that what I should be spending my time on in the waning days of the 8.2 freeze cycle? Speak now or hold your peace. Personally, I would say that this is less important than updatable views but more than online indexes. If it could be fixed just for the view case in a day or so then I think it's worth it. cheers andrew It seems I was able to get it working for both the VIEW and SELECT cases. I still have one issue, the reference to the select is left open and it complains on closing the transaction. But basically works. So when will you send in a revised patch? cheers andrew ---(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] [PATCHES] COPY view
Hi, > Tom Lane wrote: >> >> At the moment, with the online-index and updatable-views patches both >> pretty seriously broken, and no sign that the bitmap-index people are >> awake at all, I might take it on myself to fix this one instead of those >> others. But is that what I should be spending my time on in the waning >> days of the 8.2 freeze cycle? Speak now or hold your peace. >> >> >> > > Personally, I would say that this is less important than updatable views > but more than online indexes. If it could be fixed just for the view > case in a day or so then I think it's worth it. > > cheers > > andrew It seems I was able to get it working for both the VIEW and SELECT cases. I still have one issue, the reference to the select is left open and it complains on closing the transaction. But basically works. Best regards, Zoltán Böszörményi ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [HACKERS] [PATCHES] COPY view
--On Mittwoch, August 23, 2006 08:24:55 -0400 Tom Lane <[EMAIL PROTECTED]> wrote: What are these open issues for the updatable views patch you are seeing exactly? Didn't Alvaro list a bunch of issues when he put the patch back up for comment? I have not looked at it myself yet. Indeed he did and this helps a lot to clean up some parts of the code (oh, thanks to him for reviewing this, i think i forgot that :( ). I thought you were refering to some specific showstoppers i've missed. -- Thanks Bernd ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [HACKERS] [PATCHES] COPY view
Bernd Helmle <[EMAIL PROTECTED]> writes: > What are these open issues for the updatable views patch you are seeing > exactly? Didn't Alvaro list a bunch of issues when he put the patch back up for comment? I have not looked at it myself yet. > i see the INSERT...RETURNING stuff as the only "big hurd" at the moment That's not the fault of the updatable-views patch, but it definitely is something we need to put some time into :-( 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: [HACKERS] [PATCHES] COPY view
Tom Lane wrote: At the moment, with the online-index and updatable-views patches both pretty seriously broken, and no sign that the bitmap-index people are awake at all, I might take it on myself to fix this one instead of those others. But is that what I should be spending my time on in the waning days of the 8.2 freeze cycle? Speak now or hold your peace. Personally, I would say that this is less important than updatable views but more than online indexes. If it could be fixed just for the view case in a day or so then I think it's worth it. 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: [HACKERS] [PATCHES] Use of backslash in tsearch2
Patch isn't full, simple test (values are took from regression.diffs): and try dump table and restore: ERROR: syntax error CONTEXT: COPY tt, line 5, column tq: "'1 ''2'" Attached cumulative patch fixes problem, but I have some doubts, is it really needed? -- Teodor Sigaev E-mail: [EMAIL PROTECTED] WWW: http://www.sigaev.ru/ ttt.gz Description: Unix tar archive ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
Leaving... (was: Re: [HACKERS] [PATCHES] COPY view)
Hi all, seriously... I don't have time to work on PostgreSQL. It's time to say that I'm leaving this project. So, if you found some my broken code or whatever in PostgreSQL you should go and fix it. It's community-driven project. It's about collaboration -- don't ask "why should I help" -- go and help! It was nice time and really big experience, but in the world is more projects and many of them need more help than already stable (do you remember PostgreSQL 6.5? :-) and very reliable PostgreSQL. Good bye! Karel On Tue, Aug 22, 2006 at 11:12:21PM -0400, Tom Lane wrote: > The patch submitter has neither provided an updated patch nor defended > his original submission as being the right thing. If he doesn't take it > seriously enough to have done any followup, why should the rest of us? -- Karel Zak <[EMAIL PROTECTED]> ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [HACKERS] [PATCHES] COPY view
On Tue, Aug 22, 2006 at 01:11:22PM -0400, Andrew Dunstan wrote: > There's nothing hidden (unless it's also hidden from me ;-) ) > > I take it that when you talk about "we did this" you are referring to > the patch from Karel Zak. Hans has been original author of COPY VIEW idea and I've wrote it for his customer (yes, it was sponsored work). Karel -- Karel Zak <[EMAIL PROTECTED]> ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
Re: [HACKERS] [PATCHES] COPY view
--On Dienstag, August 22, 2006 23:12:21 -0400 Tom Lane <[EMAIL PROTECTED]> wrote: At the moment, with the online-index and updatable-views patches both pretty seriously broken, and no sign that the bitmap-index people are awake at all, I might take it on myself to fix this one instead of those others. But is that what I should be spending my time on in the waning days of the 8.2 freeze cycle? Speak now or hold your peace. What are these open issues for the updatable views patch you are seeing exactly? I'm currently trying to update this patch based on alvaros comments in the code and i see the INSERT...RETURNING stuff as the only "big hurd" at the moment (however, i haven't looked at this closer, but saw your and Jaime's comments on this...). It would be nice if we could summarize all open things so everybody who is able to work on this gets a complete overview. -- Thanks Bernd ---(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] COPY view
Hi, > Robert Treat <[EMAIL PROTECTED]> writes: >> On Tuesday 22 August 2006 16:10, Tom Lane wrote: >>> As I see it, we've effectively got a patch that was rejected once, >>> and Bruce wants to apply it anyway because no replacement has been >>> forthcoming. > >> Well, unless someone is going to commit to doing it the other way, it >> seems >> the guy who actually codes something offers a better solution than >> handwaving... people have also had plenty of time to come up with a >> replacement if that's what they really wanted. > > The patch submitter has neither provided an updated patch nor defended > his original submission as being the right thing. If he doesn't take it > seriously enough to have done any followup, why should the rest of us? > > At the moment, with the online-index and updatable-views patches both > pretty seriously broken, and no sign that the bitmap-index people are > awake at all, I might take it on myself to fix this one instead of those > others. But is that what I should be spending my time on in the waning > days of the 8.2 freeze cycle? Speak now or hold your peace. > > regards, tom lane I am willing to get it up to shape and support both COPY (select) TO and COPY view TO, the second is rewritten as SELECT * FROM view. In fact, I already started. Best regards, Zoltán Böszörményi ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [HACKERS] [PATCHES] COPY view
Tom Lane wrote: > Robert Treat <[EMAIL PROTECTED]> writes: > > On Tuesday 22 August 2006 16:10, Tom Lane wrote: > >> As I see it, we've effectively got a patch that was rejected once, > >> and Bruce wants to apply it anyway because no replacement has been > >> forthcoming. > > > Well, unless someone is going to commit to doing it the other way, it seems > > the guy who actually codes something offers a better solution than > > handwaving... people have also had plenty of time to come up with a > > replacement if that's what they really wanted. > > The patch submitter has neither provided an updated patch nor defended > his original submission as being the right thing. If he doesn't take it > seriously enough to have done any followup, why should the rest of us? > > At the moment, with the online-index and updatable-views patches both > pretty seriously broken, and no sign that the bitmap-index people are > awake at all, I might take it on myself to fix this one instead of those > others. But is that what I should be spending my time on in the waning > days of the 8.2 freeze cycle? Speak now or hold your peace. Your analysis is accurate. You can spend your time on whatever _you_ think is important. If someone wants to take on COPY VIEW and do all the work to make it 100%, then they are welcome to do it, but if you don't feel it is worth it, you can just leave it. If it isn't 100% by the time we start beta, it is kept for a later release. Alvaro has already indicated some problems with the patch (the objection email is in the patches queue), so it is up to someone to correct at least that, and if other objections are found, they have to correct those too before 8.2 beta starts. -- Bruce Momjian [EMAIL PROTECTED] EnterpriseDBhttp://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. + ---(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] [PATCHES] COPY view
Robert Treat <[EMAIL PROTECTED]> writes: > On Tuesday 22 August 2006 16:10, Tom Lane wrote: >> As I see it, we've effectively got a patch that was rejected once, >> and Bruce wants to apply it anyway because no replacement has been >> forthcoming. > Well, unless someone is going to commit to doing it the other way, it seems > the guy who actually codes something offers a better solution than > handwaving... people have also had plenty of time to come up with a > replacement if that's what they really wanted. The patch submitter has neither provided an updated patch nor defended his original submission as being the right thing. If he doesn't take it seriously enough to have done any followup, why should the rest of us? At the moment, with the online-index and updatable-views patches both pretty seriously broken, and no sign that the bitmap-index people are awake at all, I might take it on myself to fix this one instead of those others. But is that what I should be spending my time on in the waning days of the 8.2 freeze cycle? Speak now or hold your peace. regards, tom lane ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
Re: [HACKERS] [PATCHES] COPY view
On Tuesday 22 August 2006 16:10, Tom Lane wrote:
> Alvaro Herrera <[EMAIL PROTECTED]> writes:
> > It sucks that patches are posted and no action is taken on them for
> > months. I agree with that.
>
> This particular patch was originally posted during the 8.1 feature
> freeze window (2005-09-29), so it was doomed to a certain amount of
> languishing on the to-worry-about-later list in any case. We should
> have gotten around to reviewing it sooner than we did (the followup
> discussion was around 2006-06-14), but there was still plenty of time
> at that point to rework it per the discussion and get it into 8.2.
>
> As I see it, we've effectively got a patch that was rejected once,
> and Bruce wants to apply it anyway because no replacement has been
> forthcoming.
>
Well, unless someone is going to commit to doing it the other way, it seems
the guy who actually codes something offers a better solution than
handwaving... people have also had plenty of time to come up with a
replacement if that's what they really wanted.
--
Robert Treat
Build A Brighter LAMP :: Linux Apache {middleware} PostgreSQL
---(end of broadcast)---
TIP 4: Have you searched our list archives?
http://archives.postgresql.org
Re: [HACKERS] [PATCHES] COPY view
Tom Lane wrote: > Alvaro Herrera <[EMAIL PROTECTED]> writes: > > It sucks that patches are posted and no action is taken on them for > > months. I agree with that. > > This particular patch was originally posted during the 8.1 feature > freeze window (2005-09-29), so it was doomed to a certain amount of > languishing on the to-worry-about-later list in any case. We should > have gotten around to reviewing it sooner than we did (the followup > discussion was around 2006-06-14), but there was still plenty of time > at that point to rework it per the discussion and get it into 8.2. > > As I see it, we've effectively got a patch that was rejected once, > and Bruce wants to apply it anyway because no replacement has been > forthcoming. Yea, that pretty much sums it up. Based on the number of people who wanted it applied, I think we need to have a discussion like this. I can easily go with rejecting it, but I think the discussion is needed to be fair to the patch author. So, what do we want to do with this? Where did we say we didn't want SELECT? I never remember that being discussed. I remember us saying we never wanted SELECT or VIEWs because it was going to be slow, but once the SELECT idea came up, I think we decided we wanted that, and views could be built on top of that. I certainly never remember us saying we didn't want SELECT but wanted views. -- 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] COPY view
Alvaro Herrera <[EMAIL PROTECTED]> writes: > It sucks that patches are posted and no action is taken on them for > months. I agree with that. This particular patch was originally posted during the 8.1 feature freeze window (2005-09-29), so it was doomed to a certain amount of languishing on the to-worry-about-later list in any case. We should have gotten around to reviewing it sooner than we did (the followup discussion was around 2006-06-14), but there was still plenty of time at that point to rework it per the discussion and get it into 8.2. As I see it, we've effectively got a patch that was rejected once, and Bruce wants to apply it anyway because no replacement has been forthcoming. 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: [HACKERS] [PATCHES] COPY view
Hans-Juergen Schoenig wrote: > It has been made as "COPY FROM / TO view" because people wanted it to be > done that way. > My original proposal was in favour of arbitrary SELECTs (just like > proposed by the TODO list) but this was rejected. So, we did it that way > (had to explain to customer why views are better). Now everybody wants > the original select which was proposed. This is not the first time this happens. It has happened to Simon Riggs at least once and to me as well. Sometimes "the community" just doesn't realize what it wants, until what it think it wants is done and then realizes it wants something else. It is frustrating, but I don't see how to do things differently. > Things have been submitted months ago and now we are short of time. I > think everybody on the list is going a superior job but after 6 years I > still have no idea how patches are treated ;). It sucks that patches are posted and no action is taken on them for months. I agree with that. -- Alvaro Herrerahttp://www.CommandPrompt.com/ PostgreSQL Replication, Consulting, Custom Development, 24x7 support ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [HACKERS] [PATCHES] COPY view
Hans-Juergen Schoenig wrote: Tom Lane wrote: Stefan Kaltenbrunner <[EMAIL PROTECTED]> writes: Bruce Momjian wrote: Well, the patch was submitted in time, and it is a desired feature. If we want to hold it for 8.3 due to lack of time, we can, but I don't think we can decide now that it must wait. well I thought the agreed approach to that was allowing COPY from arbitrary expressions without the need to go through the extra CREATE VIEW step? Exactly. This is not the feature that was agreed to. Just because we have a patch for it doesn't mean that we have to put it in. If we do put it in, we'll be stuck carrying that feature forever, even after someone gets around to doing it right. regards, tom lane It has been made as "COPY FROM / TO view" because people wanted it to be done that way. My original proposal was in favour of arbitrary SELECTs (just like proposed by the TODO list) but this was rejected. So, we did it that way (had to explain to customer why views are better). Now everybody wants the original select which was proposed. I can understand if things are not committed because of bad code quality or whatever but to be honest: It is more of less frustrating if things are done differently because of community wish and then rejected because things are not done the original way ... Things have been submitted months ago and now we are short of time. I think everybody on the list is going a superior job but after 6 years I still have no idea how patches are treated ;). There's nothing hidden (unless it's also hidden from me ;-) ) I take it that when you talk about "we did this" you are referring to the patch from Karel Zak. I have had a quick look at that patch, and apart from not applying cleanly to the current CVS tree (which isn't your fault as the patch has been sitting around for so long) it is also missing regression tests and docs. That's without even looking at code quality. So, how quickly can you fix those 3 things? 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: [HACKERS] [PATCHES] COPY view
Andrew Dunstan <[EMAIL PROTECTED]> writes: > It's a close call. On balance I'd be inclined to accept the patch if it > reviews OK, even though we will throw the code away soon (we hope). Well, the patch seems pretty ugly code-wise as well. I'd be willing to clean it up if I thought it wouldn't ultimately get yanked out again, but I'm not sure that I see the point if we think it's a dead end. It doesn't come close to applying to CVS HEAD, either :-( regards, tom lane ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
Re: [HACKERS] [PATCHES] COPY view
Stefan Kaltenbrunner wrote: Bruce Momjian wrote: Tom Lane wrote: Bruce Momjian <[EMAIL PROTECTED]> writes: OK, based on this feedback, I am adding COPY VIEW to the patches queue. I think we have other things that demand our attention more than a half-baked feature. Well, the patch was submitted in time, and it is a desired feature. If we want to hold it for 8.3 due to lack of time, we can, but I don't think we can decide now that it must wait. well I thought the agreed approach to that was allowing COPY from arbitrary expressions without the need to go through the extra CREATE VIEW step? Well, it's been a bit of a mess, unfortunately, and I can understand people feeling aggrieved. I think there is general agreement that we want to be able to do: COPY (SELECT ... ) TO ... When we have that it would not be unreasonable to have a special case for views which would transparently rewrite COPY VIEWNAME TO as COPY (SELECT * FROM VIEWNAME) TO So we would not necessarily be adopting a feature we don't want in the long run, from a user visibility angle. The issue seems to be that in adopting the present patch we would be incorporating some code we will essentially have to abandon when we get the feature we all really want, and which we hope will be available for 8.3. On that basis I can certainly appreciate Tom's reluctance to adopt the patch. It's a close call. On balance I'd be inclined to accept the patch if it reviews OK, even though we will throw the code away soon (we hope). cheers andrew ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
Re: [HACKERS] [PATCHES] COPY view
Stefan Kaltenbrunner <[EMAIL PROTECTED]> writes: > Bruce Momjian wrote: >> Well, the patch was submitted in time, and it is a desired feature. If >> we want to hold it for 8.3 due to lack of time, we can, but I don't >> think we can decide now that it must wait. > well I thought the agreed approach to that was allowing COPY from > arbitrary expressions without the need to go through the extra CREATE > VIEW step? Exactly. This is not the feature that was agreed to. Just because we have a patch for it doesn't mean that we have to put it in. If we do put it in, we'll be stuck carrying that feature forever, even after someone gets around to doing it right. 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: [HACKERS] [PATCHES] COPY view
Bruce Momjian wrote: > Tom Lane wrote: >> Bruce Momjian <[EMAIL PROTECTED]> writes: >>> OK, based on this feedback, I am adding COPY VIEW to the patches queue. >> I think we have other things that demand our attention more than a >> half-baked feature. > > Well, the patch was submitted in time, and it is a desired feature. If > we want to hold it for 8.3 due to lack of time, we can, but I don't > think we can decide now that it must wait. well I thought the agreed approach to that was allowing COPY from arbitrary expressions without the need to go through the extra CREATE VIEW step? Stefan ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [HACKERS] [PATCHES] [PATCH] Provide 8-byte transaction IDs to
On 8/21/06, Tom Lane <[EMAIL PROTECTED]> wrote:
Bruce Momjian <[EMAIL PROTECTED]> writes:
> Tom Lane wrote:
>> (I wouldn't do it like this though --- TransactionIdAdvance itself is
>> the place to bump the secondary counter.)
> Agreed.
I reconsidered after trying to do it that way --- although fixing
TransactionIdAdvance itself to maintain a 2-word counter isn't hard,
there are a whole lot of other places that can advance nextXid,
mostly bits like this in WAL recovery:
/* Make sure nextXid is beyond any XID mentioned in the record */
max_xid = xid;
for (i = 0; i < xlrec->nsubxacts; i++)
{
if (TransactionIdPrecedes(max_xid, sub_xids[i]))
max_xid = sub_xids[i];
}
if (TransactionIdFollowsOrEquals(max_xid,
ShmemVariableCache->nextXid))
{
ShmemVariableCache->nextXid = max_xid;
TransactionIdAdvance(ShmemVariableCache->nextXid);
}
We could hack all these places to know about maintaining an XID-epoch
value, but it's not looking like a simple single-place-to-touch fix :-(
As I was asked to rework the patch, I planned to use
TransactionIdAdvance(ShmemVariableCache), although that would
be conceptually ugly. Right Thing for this approach would be
to have special struct, but that would touch half the codebase.
That was also the reason I did not want to go that path.
There's still a lot more cruft in the submitted patch than I think
belongs in core, but I'll work on extracting something we can apply.
The only cruft I see is the snapshot on-disk "compression" and maybe
the pg_sync_txid() funtionality. Dropping the compression would not
matter much, snapshots would waste space, but at least for our
usage it would not be a problem. The reast of the functions are all
required for efficient handling.
Dropping the pg_sync_txid() would be loss, because that means that
user cannot just dump and restore the data and just continue where
it left off. Maybe its not a problem for replication but for generic
queueing it would need delicate juggling when restoring backup.
Although I must admit the pg_sync_txid() is indeed ugly part
of the patch, and it creates new mode for failure - wrapping
epoch. So I can kind of agree for removing it.
I hope you don't mean that none of the user-level functions belong
to core. It's not like there is several ways to expose the info.
And it not like there are much more interesting ways for using
the long xid in C level. Having long xid available in SQL level
means that efficient async replication can be done without any
use of C.
Now that I am back from vacation I can do some coding myself,
if you give hints what needs rework.
--
marko
---(end of broadcast)---
TIP 4: Have you searched our list archives?
http://archives.postgresql.org
Re: [HACKERS] [PATCHES] [PATCH] Provide 8-byte transaction IDs to
Bruce Momjian <[EMAIL PROTECTED]> writes:
> Tom Lane wrote:
>> The part of this that would actually be useful to put in core is
>> maintaining a 64-bit XID counter, ie, keep an additional counter that
>> bumps every time XID wraps around. This cannot be done very well from
>> outside core but it would be nearly trivial, and nearly free, to add
>> inside. Everything else in the patch could be done just as well as an
>> extension datatype.
>>
>> (I wouldn't do it like this though --- TransactionIdAdvance itself is
>> the place to bump the secondary counter.)
> Agreed.
I reconsidered after trying to do it that way --- although fixing
TransactionIdAdvance itself to maintain a 2-word counter isn't hard,
there are a whole lot of other places that can advance nextXid,
mostly bits like this in WAL recovery:
/* Make sure nextXid is beyond any XID mentioned in the record */
max_xid = xid;
for (i = 0; i < xlrec->nsubxacts; i++)
{
if (TransactionIdPrecedes(max_xid, sub_xids[i]))
max_xid = sub_xids[i];
}
if (TransactionIdFollowsOrEquals(max_xid,
ShmemVariableCache->nextXid))
{
ShmemVariableCache->nextXid = max_xid;
TransactionIdAdvance(ShmemVariableCache->nextXid);
}
We could hack all these places to know about maintaining an XID-epoch
value, but it's not looking like a simple single-place-to-touch fix :-(
So I'm now agreeing that the approach of maintaining an epoch counter
in checkpoints is best after all. That will work so long as the system
doesn't exceed 4G transactions between checkpoints ... and you'd have a
ton of other problems before that, so this restriction does not bother
me. Putting this in the core code still beats the alternatives
available to non-core code because of the impossibility of being sure
you get control on any fixed schedule, not to mention considerations of
what happens during WAL replay and PITR.
There's still a lot more cruft in the submitted patch than I think
belongs in core, but I'll work on extracting something we can apply.
There was some worry upthread about whether Slony would actually use
this in the near future, but certainly if we don't put it in then
they'll *never* be able to use it.
regards, tom lane
---(end of broadcast)---
TIP 4: Have you searched our list archives?
http://archives.postgresql.org
Re: [HACKERS] [PATCHES] selecting large result sets in psql using
Am Donnerstag, 17. August 2006 20:05 schrieb Chris Mair: > \gc sounds like a good idea to me :) Strictly speaking, in the randomly defined grammer of psql, \gc is \g with an argument of 'c' (try it, it works). I'm not sure what use case you envision for this feature. Obviously, this is for queries with large result sets. I'd guess that people will not normally look at those result sets interactively. If the target audience is instead psql scripting, you don't really need the most convenient command possible. A \set variable would make sense to me. -- Peter Eisentraut http://developer.postgresql.org/~petere/ ---(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] [PATCHES] selecting large result sets in psql using
> > >> Since buffer commands all have a single char I wanted a single char one > > >> too. The "c" for "cursor" was taken already, so i choose the "u" (second > > >> char in "cursor"). If somebody has a better suggestion, let us know ;) > > > > > I think a new backslash variable isn't the way to go. I would use a > > > \pset variable to control what is happening. > > > > That seems like it would be very awkward to use: you'd have to type > > quite a bit to go from one mode to the other. > > > > Personally I think that insisting on a one-letter command name is not > > such a good idea if you can't pick a reasonably memorable name. > > I'd suggest "\gc" (\g with a Cursor) or "\gb" (\g for a Big query) > > or some such. \gc sounds like a good idea to me :) (I must admit gc reminds me about 'garbage collector', which in a weired way is related with what we're doing here... At least more related than 'Great Britain' ;) > So add it as a modifyer to \g? Yea, that works, but it doesn't work for > ';' as a 'go' command, of course, which is perhaps OK. Yes, it was intended to differentiate this command from ';'; Bye, Chris. -- Chris Mair http://www.1006.org ---(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] selecting large result sets in psql using
Tom Lane wrote: > Bruce Momjian <[EMAIL PROTECTED]> writes: > > Chris Mair wrote: > >> Since buffer commands all have a single char I wanted a single char one > >> too. The "c" for "cursor" was taken already, so i choose the "u" (second > >> char in "cursor"). If somebody has a better suggestion, let us know ;) > > > I think a new backslash variable isn't the way to go. I would use a > > \pset variable to control what is happening. > > That seems like it would be very awkward to use: you'd have to type > quite a bit to go from one mode to the other. > > Personally I think that insisting on a one-letter command name is not > such a good idea if you can't pick a reasonably memorable name. > I'd suggest "\gc" (\g with a Cursor) or "\gb" (\g for a Big query) > or some such. So add it as a modifyer to \g? Yea, that works, but it doesn't work for ';' as a 'go' command, of course, which is perhaps OK. -- 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: [HACKERS] [PATCHES] selecting large result sets in psql using cursors
Bruce Momjian <[EMAIL PROTECTED]> writes: > Chris Mair wrote: >> Since buffer commands all have a single char I wanted a single char one >> too. The "c" for "cursor" was taken already, so i choose the "u" (second >> char in "cursor"). If somebody has a better suggestion, let us know ;) > I think a new backslash variable isn't the way to go. I would use a > \pset variable to control what is happening. That seems like it would be very awkward to use: you'd have to type quite a bit to go from one mode to the other. Personally I think that insisting on a one-letter command name is not such a good idea if you can't pick a reasonably memorable name. I'd suggest "\gc" (\g with a Cursor) or "\gb" (\g for a Big query) or some such. regards, tom lane ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
Re: [HACKERS] [PATCHES] selecting large result sets in psql using
Chris Mair wrote: > > > > > BTW, \u seems not to have any mnemonic value whatsoever ... isn't there > > > > some other name we could use? > > > > > > True :) > > > Since buffer commands all have a single char I wanted a single char one > > > too. The "c" for "cursor" was taken already, so i choose the "u" (second > > > char in "cursor"). If somebody has a better suggestion, let us know ;) > > > > I think a new backslash variable isn't the way to go. I would use a > > \pset variable to control what is happening. > > IMHO with \pset I'd have different places where I'd need to figure > out whether to do the cursor thing and I was a bit reluctant to add > stuff to existing code paths. Also the other \pset options are somewhat > orthogonal to this one. Just my two EUR cents, of course... :) Well, let's see what others say, but \pset seems _much_ more natural for this type of thing to me. -- 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] selecting large result sets in psql using
> > > BTW, \u seems not to have any mnemonic value whatsoever ... isn't there > > > some other name we could use? > > > > True :) > > Since buffer commands all have a single char I wanted a single char one > > too. The "c" for "cursor" was taken already, so i choose the "u" (second > > char in "cursor"). If somebody has a better suggestion, let us know ;) > > I think a new backslash variable isn't the way to go. I would use a > \pset variable to control what is happening. IMHO with \pset I'd have different places where I'd need to figure out whether to do the cursor thing and I was a bit reluctant to add stuff to existing code paths. Also the other \pset options are somewhat orthogonal to this one. Just my two EUR cents, of course... :) Bye, Chris. -- Chris Mair http://www.1006.org ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
Re: [HACKERS] [PATCHES] selecting large result sets in psql using
> > Patch with fix against current CVS is attached.
Forgot the attachment... soory.
--
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-17 16:50:58.0 +0200
--- pgsql/doc/src/sgml/ref/psql-ref.sgml 2006-08-17 18:02:29.0 +0200
***
*** 1201,1206
--- 1201,1231
+
+
+ \u [ { filename | |command } ]
+
+
+
+ Sends the current query input buffer to the server and
+ optionally stores the query's output in filename or pipes the output
+ into a separate Unix shell executing command.
+ Unlike \g, \u works only
+ for select statements and uses a cursor to retrieve the result set.
+ Therefore \u uses only a limited amount of memory,
+ regardless the size of the result set. It can be used whenever a result
+ set needs to be retrieved that exceeds the client's memory resources.
+ \u always gives unaligned output. It does, however
+ use the current field separator (see \pset).
+ \u gives an error when trying to execute something
+ that is not a SELECT statement.
+
+
+
+
+
\help (or \h) [ command ]
diff -rc pgsql.original/src/bin/psql/command.c pgsql/src/bin/psql/command.c
*** pgsql.original/src/bin/psql/command.c 2006-08-17 16:51:04.0 +0200
--- pgsql/src/bin/psql/command.c 2006-08-17 18:02:49.0 +0200
***
*** 830,835
--- 830,866
}
}
+ /*
+ * \u executes the current query buffer using a cursor
+ */
+ else if (strcmp(cmd, "u") == 0)
+ {
+ char *fname = psql_scan_slash_option(scan_state,
+ OT_FILEPIPE, NULL, false);
+
+ if (!fname)
+ pset.gfname = NULL;
+ else
+ {
+ expand_tilde(&fname);
+ pset.gfname = pg_strdup(fname);
+ }
+ free(fname);
+
+ if (query_buf && query_buf->len == 0)
+ {
+ if (!quiet)
+ {
+ puts(_("Query buffer is empty."));
+ fflush(stdout);
+ }
+ }
+ else
+ {
+ status = PSQL_CMD_SEND_USING_CURSOR;
+ }
+ }
+
/* \unset */
else if (strcmp(cmd, "unset") == 0)
{
diff -rc pgsql.original/src/bin/psql/command.h pgsql/src/bin/psql/command.h
*** pgsql.original/src/bin/psql/command.h 2006-08-17 16:51:04.0 +0200
--- pgsql/src/bin/psql/command.h 2006-08-17 16:55:25.0 +0200
***
*** 16,21
--- 16,22
{
PSQL_CMD_UNKNOWN = 0, /* not done parsing yet (internal only) */
PSQL_CMD_SEND, /* query complete; send off */
+ PSQL_CMD_SEND_USING_CURSOR, /* query complete; send off using cursor */
PSQL_CMD_SKIP_LINE,/* keep building query */
PSQL_CMD_TERMINATE,/* quit program */
PSQL_CMD_NEWEDIT,/* query buffer was changed (e.g., via \e) */
diff -rc pgsql.original/src/bin/psql/common.c pgsql/src/bin/psql/common.c
*** pgsql.original/src/bin/psql/common.c 2006-08-17 16:51:04.0 +0200
--- pgsql/src/bin/psql/common.c 2006-08-17 18:40:51.0 +0200
***
*** 28,33
--- 28,34
#include "command.h"
#include "copy.h"
#include "mb/pg_wchar.h"
+ #include "mbprint.h"
/* Workarounds for Windows */
***
*** 52,58
((T)->millitm - (U)->millitm))
#endif
!
static bool command_no_begin(const char *query);
/*
--- 53,59
((T)->millitm - (U)->millitm))
#endif
! static bool is_select_command(const char *query);
static bool command_no_begin(const char *query);
/*
***
*** 952,957
--- 953,1146
/*
+ * SendQueryUsingCursor: send the (SELECT) query string to the backend
+ * using a cursor and print out results.
+ *
+ * Unlike with SendQuery(), single step mode, ON_ERROR_ROLLBACK mode,
+ * timing and format settings (except delimiters) are NOT honoured.
+ *
+ * Returns true if the query executed successfully, false otherwise.
+ */
+ bool
+ SendQueryUsingCursor(const char *query)
+ {
+ PGresult *results;
+ bool started_txn = false;
+ PQExpBufferData buf;
+ FILE *queryFout_copy = NULL;
+ bool queryFoutPipe_copy = false;
+ intntuples, nfields = -1;
+ inti, j;
+
+ if (!pset.db)
+ {
+ psql_error("You are currently not connected to a database.\n");
+ return false;
+ }
+
+ if (!is_select_command(query))
+ {
+ psql_error("Need a SELECT command to perform cursor fetch.\n");
+ return false;
+ }
+
+ if (VariableEquals(pset.vars, "ECHO", "queries"))
+ {
+ puts(query);
+ fflush(stdout);
+ }
+
+ if (pset.logfile)
+ {
+ fprintf(pset.logfile,
+ _("* QUERY **\n"
+ "%s\n"
+ "**\n\n"), query);
+ fflush(pset.logfile);
+ }
+
+ SetCancelConn();
+
+ /* prepare to write output to \u argument, if any */
+ if (pset.gfname)
+ {
+ queryFout_copy = pset.queryFout
Re: [HACKERS] [PATCHES] selecting large result sets in psql using
Replying to myself... > Patch with fix against current CVS is attached. Alvaro Herrera sent two fixes off-list: a typo and at the end of SendQueryUsingCursor I sould COMMIT, not ROLLBACK. So, one more version (6) that fixes these too is attached. Bye, Chris. PS: I'm keeping this on both lists now, hope it's ok. -- Chris Mair http://www.1006.org ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [HACKERS] [PATCHES] selecting large result sets in psql using
Chris Mair wrote: > > At some point we ought to extend libpq enough to expose the V3-protocol > > feature that allows partial fetches from portals; that would be a > > cleaner way to implement this feature. However since nobody has yet > > proposed a good API for this in libpq, I don't object to implementing > > \u with DECLARE CURSOR for now. > > > > BTW, \u seems not to have any mnemonic value whatsoever ... isn't there > > some other name we could use? > > True :) > Since buffer commands all have a single char I wanted a single char one > too. The "c" for "cursor" was taken already, so i choose the "u" (second > char in "cursor"). If somebody has a better suggestion, let us know ;) I think a new backslash variable isn't the way to go. I would use a \pset variable to control what is happening. -- 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] selecting large result sets in psql using
On Thu, 2006-08-17 at 03:14 -0400, Bruce Momjian wrote: > Peter Eisentraut wrote: > > Tom Lane wrote: > > > BTW, \u seems not to have any mnemonic value whatsoever ... isn't > > > there some other name we could use? > > > > Ever since pgsql-patches replies started going to -hackers, threading > > doesn't work anymore, so I for one can't tell what this refers to at > > all. > > I see the original posting here: > > http://archives.postgresql.org/pgsql-patches/2006-07/msg00287.php > > but I don't remember seeing this posting at all, and it isn't saved in > my mailbox either. Strange. FWIW I saw it. -- Simon Riggs EnterpriseDB http://www.enterprisedb.com ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
Re: [HACKERS] [PATCHES] selecting large result sets in psql using
Peter Eisentraut wrote: > Tom Lane wrote: > > BTW, \u seems not to have any mnemonic value whatsoever ... isn't > > there some other name we could use? > > Ever since pgsql-patches replies started going to -hackers, threading > doesn't work anymore, so I for one can't tell what this refers to at > all. I see the original posting here: http://archives.postgresql.org/pgsql-patches/2006-07/msg00287.php but I don't remember seeing this posting at all, and it isn't saved in my mailbox either. Strange. -- 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] selecting large result sets in psql using cursors
Tom Lane wrote: > BTW, \u seems not to have any mnemonic value whatsoever ... isn't > there some other name we could use? Ever since pgsql-patches replies started going to -hackers, threading doesn't work anymore, so I for one can't tell what this refers to at all. -- Peter Eisentraut http://developer.postgresql.org/~petere/ ---(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] [PATCHES] Adjust autovacuum naptime automatically
Alvaro Herrera wrote: ITAGAKI Takahiro wrote: In the case of a heavily update workload, the default naptime (60 seconds) is too long to keep the number of dead tuples low. With my patch, the naptime will be adjusted around 3 seconds at the case of pgbench (scale=10, 80 tps) with default other autovacuum parameters. What is this based on? That is, based on what information is it deciding to reduce the naptime? Interesting. To be frank I don't know what the sleep scale factor was supposed to do. I'm not sure that sleep scale factor is a good idea or not at this point, but what I was thinking back in the day when i originally wrote the contrib autovacuum is that I didn't want the system to get bogged down constantly vacuuming. So, if it just spent a long time working on one database, it would sleep for long time. Given that we can now specify the vacuum cost delay settings for autovacuum and disable tables and everything else, I'm not sure we this anymore, at least not as it was originally designed. It sounds like Itagaki is doing things a little different with his patch, but I'm not sure I understand it. - I removed autovacuum_naptime guc variable, because it is adjusted automatically now. Is it appropriate? I think we should provide the user with a way to stop the naptime from changing at all. Eventually we will have the promised "maintenance windows" feature which will mean the user will not have to worry at all about the naptime, but in the meantime I think we should keep it. I'm not sure that's true. I believe we will want the naptime GUC option even after we have the maintenance window. I think we might ignore the naptime during the maintenance window, but even after we have the maintenance window, we will still vacuum during the day as required. My vision of the maintenance window has always been very simple, that is, during the maintenance window the thresholds get reduced by some factor (probably a GUC variable) so during the day it might take 1 updates on a table to cause a vacuum but during the naptime it might be 10% of that, 1000. Is this in-line with what others were thinking? ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [HACKERS] [PATCHES] Forcing current WAL file to be archived
On Wed, 2006-08-16 at 17:09 -0400, Tom Lane wrote:
> Simon Riggs <[EMAIL PROTECTED]> writes:
> > Wise one: what should my pg_proc look like?
>
> > DATA(insert OID = 2850 ( pg_xlogfile_name_offsetPGNSP PGUID 12 f f t f
> > i 1 2249 "25" "25 25 23" "i o o" _null_ pg_xlogfile_name_offset -
> > _null_ ));
>
> Oh, as far as that goes, the array columns need to look like something
> array_in will eat; and you should provide parameter names so that
> "select * from" will produce useful headings. So probably more like
>
> DATA(insert OID = 2850 ( pg_xlogfile_name_offset PGNSP PGUID 12 f f t f
> i 1 2249 "25" "{25,25,23}" "{i,o,o}" "{wal_offset,filename,offset}"
> pg_xlogfile_name_offset - _null_ ));
>
> I think you can get away without inner quotes (ie, not "{'i','o','o'}")
> as long as you aren't using anything weird like spaces in a parameter name.
archive_timeout++.patch re-submitted on other thread, now including
these changes also.
--
Simon Riggs
EnterpriseDB http://www.enterprisedb.com
---(end of broadcast)---
TIP 4: Have you searched our list archives?
http://archives.postgresql.org
Re: [HACKERS] [PATCHES] Forcing current WAL file to be archived
Simon Riggs <[EMAIL PROTECTED]> writes:
> Wise one: what should my pg_proc look like?
> DATA(insert OID = 2850 ( pg_xlogfile_name_offset PGNSP PGUID 12 f f t f
> i 1 2249 "25" "25 25 23" "i o o" _null_ pg_xlogfile_name_offset -
> _null_ ));
Oh, as far as that goes, the array columns need to look like something
array_in will eat; and you should provide parameter names so that
"select * from" will produce useful headings. So probably more like
DATA(insert OID = 2850 ( pg_xlogfile_name_offsetPGNSP PGUID 12 f f t f
i 1 2249 "25" "{25,25,23}" "{i,o,o}" "{wal_offset,filename,offset}"
pg_xlogfile_name_offset - _null_ ));
I think you can get away without inner quotes (ie, not "{'i','o','o'}")
as long as you aren't using anything weird like spaces in a parameter name.
regards, tom lane
---(end of broadcast)---
TIP 6: explain analyze is your friend
Re: [HACKERS] [PATCHES] Forcing current WAL file to be archived
On Wed, 2006-08-16 at 16:51 -0400, Tom Lane wrote: > Simon Riggs <[EMAIL PROTECTED]> writes: > > but my initdb fails with > > > creating template1 database in a/base/1 ... FATAL: cache lookup failed > > for type 26 > > Um ... when did you last "cvs update"? That was the behavior up till I > fixed array_in for bootstrap mode, yesterday afternoon ... Sounds like it must be so. -- Simon Riggs EnterpriseDB http://www.enterprisedb.com ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [HACKERS] [PATCHES] Forcing current WAL file to be archived
Simon Riggs <[EMAIL PROTECTED]> writes: > but my initdb fails with > creating template1 database in a/base/1 ... FATAL: cache lookup failed > for type 26 Um ... when did you last "cvs update"? That was the behavior up till I fixed array_in for bootstrap mode, yesterday afternoon ... regards, tom lane ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
Re: [HACKERS] [PATCHES] Forcing current WAL file to be archived
On Wed, 2006-08-16 at 11:45 -0400, Tom Lane wrote:
> Simon Riggs <[EMAIL PROTECTED]> writes:
> > We want a single row output, with two columns, yes?
> > Presumably:
> > xlogfilenameTEXT
> > offset INTEGER
>
> Sounds right to me. int4 should be wide enough for practical xlog
> segment sizes.
Wise one: what should my pg_proc look like?
I'm the lucky man to break the "_null_ _null_ _null_" rule...
I've tried
DATA(insert OID = 2850 ( pg_xlogfile_name_offsetPGNSP PGUID 12 f f t f
i 1 2249 "25" "25 25 23" "i o o" _null_ pg_xlogfile_name_offset -
_null_ ));
but my initdb fails with
selecting default shared_buffers/max_fsm_pages ... 2kB/100
creating configuration files ... ok
creating template1 database in a/base/1 ... FATAL: cache lookup failed
for type 26
child process exited with exit code 1
initdb: removing data directory "a"
Thinking this might be an 0-referenced array issue, I also tried "24 24
22" in the above, but that bombs with the same error.
Currently, if I just leave it as it is, then initdb runs but then
hangs/bombs when you invokle the function (as you might expect).
As far as I can tell, the function isn't ever called correctly without
this... copied here for info.
/*
* Compute an xlog file name and decimal byte offset given a WAL
location,
* such as is returned by pg_stop_backup() or pg_xlog_switch().
*
* Note that a location exactly at a segment boundary is taken to be in
* the previous segment. This is usually the right thing, since the
* expected usage is to determine which xlog file(s) are ready to
archive.
*/
Datum
pg_xlogfile_name_offset(PG_FUNCTION_ARGS)
{
text *location = PG_GETARG_TEXT_P(0);
char *locationstr;
unsigned int uxlogid;
unsigned int uxrecoff;
uint32 xlogid;
uint32 xlogseg;
uint32 xrecoff;
XLogRecPtr locationpoint;
charxlogfilename[MAXFNAMELEN];
TupleDesc returnTupleDesc;
Datum values[2];
boolisnull[2];
HeapTuple returnHeapTuple;
Datum result;
/*
* Read input and parse
*/
locationstr = DatumGetCString(DirectFunctionCall1(textout,
PointerGetDatum(location)));
if (sscanf(locationstr, "%X/%X", &uxlogid, &uxrecoff) != 2)
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
errmsg("could not parse xlog location \"%s\"",
locationstr)));
locationpoint.xlogid = uxlogid;
locationpoint.xrecoff = uxrecoff;
/* Construct a tuple descriptor for the result rows. */
returnTupleDesc = CreateTemplateTupleDesc(2, false);
TupleDescInitEntry(returnTupleDesc, (AttrNumber) 1, "xlogfilename",
TEXTOID, -1, 0);
TupleDescInitEntry(returnTupleDesc, (AttrNumber) 2, "offset",
INT4OID, -1, 0);
returnTupleDesc = BlessTupleDesc(returnTupleDesc);
/*
* xlogfilename
*/
XLByteToPrevSeg(locationpoint, xlogid, xlogseg);
XLogFileName(xlogfilename, ThisTimeLineID, xlogid, xlogseg);
values[0] = PointerGetDatum(xlogfilename);
isnull[0] = false;
/*
* offset
*/
xrecoff = locationpoint.xrecoff - xlogseg * XLogSegSize;
values[1] = UInt32GetDatum(xrecoff);
isnull[1] = false;
/*
* Tuple jam: Having first prepared your Datums, then squash
together
*/
returnHeapTuple = heap_form_tuple(returnTupleDesc, values, isnull);
result = HeapTupleGetDatum(returnHeapTuple);
PG_RETURN_DATUM(result);
}
--
Simon Riggs
EnterpriseDB http://www.enterprisedb.com
---(end of broadcast)---
TIP 5: don't forget to increase your free space map settings
Re: [HACKERS] [PATCHES] Forcing current WAL file to be archived
On Wed, 2006-08-16 at 08:51 -0400, Tom Lane wrote: > Simon Riggs <[EMAIL PROTECTED]> writes: > > On Tue, 2006-08-15 at 18:42 -0400, Tom Lane wrote: > >> So let's fix pg_xlogfile_name_offset() to have two OUT parameters > >> instead of returning a smushed-together string. > > > I'll do this, but I'm conscious that this is a cosmetic change. > > Well, it's cosmetic, but it's also an API change, which means that this > is our only opportunity to get it right. Once these functions are in a > release it will be too hard to change them. I've just started working this part, now I have the rest complete. We want a single row output, with two columns, yes? Presumably: xlogfilenameTEXT offset INTEGER -- Simon Riggs EnterpriseDB http://www.enterprisedb.com ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
Re: [HACKERS] [PATCHES] Forcing current WAL file to be archived
Simon Riggs <[EMAIL PROTECTED]> writes: > We want a single row output, with two columns, yes? > Presumably: > xlogfilenameTEXT > offset INTEGER Sounds right to me. int4 should be wide enough for practical xlog segment sizes. regards, tom lane ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [HACKERS] [PATCHES] Forcing current WAL file to be archived
Simon Riggs <[EMAIL PROTECTED]> writes: > On Tue, 2006-08-15 at 18:42 -0400, Tom Lane wrote: >> So let's fix pg_xlogfile_name_offset() to have two OUT parameters >> instead of returning a smushed-together string. > I'll do this, but I'm conscious that this is a cosmetic change. Well, it's cosmetic, but it's also an API change, which means that this is our only opportunity to get it right. Once these functions are in a release it will be too hard to change them. regards, tom lane ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [HACKERS] [PATCHES] Forcing current WAL file to be archived
On Tue, 2006-08-15 at 18:42 -0400, Tom Lane wrote: > I wrote: > > It'd definitely be nicer that way, but given the current limitations of > > bootstrap mode I see no non-kluge way to make a built-in function have > > OUT parameters. (Hint: array_in doesn't work in bootstrap mode.) > > Actually, that turns out not to be so hard to fix as I thought. > array_in only needs to work for the array types used in the core system > tables, and bootstrap.c already has a hard-wired table of that info ... > we only have to make it available to array_in. Which I just did. Cool; I'd noticed that this would have been the first such function. > So let's fix pg_xlogfile_name_offset() to have two OUT parameters > instead of returning a smushed-together string. I'll do this, but I'm conscious that this is a cosmetic change. I'm going on vacation very soon now, so test reports of the major functionality would be greatly appreciated. -- Simon Riggs EnterpriseDB http://www.enterprisedb.com ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [HACKERS] [PATCHES] Forcing current WAL file to be archived
I wrote: > It'd definitely be nicer that way, but given the current limitations of > bootstrap mode I see no non-kluge way to make a built-in function have > OUT parameters. (Hint: array_in doesn't work in bootstrap mode.) Actually, that turns out not to be so hard to fix as I thought. array_in only needs to work for the array types used in the core system tables, and bootstrap.c already has a hard-wired table of that info ... we only have to make it available to array_in. Which I just did. So let's fix pg_xlogfile_name_offset() to have two OUT parameters instead of returning a smushed-together string. The reason I knew about the array_in problem was I'd tried to make some other built-in function have OUT parameters ... I think it was probably one of the ones that we currently have underneath system views. It might be worthwhile converting some or all of these to use OUT parameters and not need the crutch of an AS clause in the view: pg_show_all_settings pg_lock_status pg_prepared_xact pg_stat_file pg_prepared_statement pg_cursor pg_timezonenames regards, tom lane ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
Re: [HACKERS] [PATCHES] Forcing current WAL file to be archived
"Jim C. Nasby" <[EMAIL PROTECTED]> writes: > True, but making people parse the output of a function to seperate the > two fields seems pretty silly. Is there some reason why > pg_xlogfile_name_offset shouldn't be a SRF, or use two out parameters? It'd definitely be nicer that way, but given the current limitations of bootstrap mode I see no non-kluge way to make a built-in function have OUT parameters. (Hint: array_in doesn't work in bootstrap mode.) And the other alternatives like a predefined complex type seem even more painful. If you can think of a way to do this that has pain not out of proportion to the gain, then I'm all for it ... regards, tom lane ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
