Re: [HACKERS] [REVIEW] Patch for cursor calling with named parameters
Yeb Havinga writes:
> Hello Royce,
> Thanks again for testing.
I looked this patch over but concluded that it's not ready to apply,
mainly because there are too many weird behaviors around error
reporting.
The biggest problem is that the patch cuts up and reassembles the source
text and then hands that off to check_sql_expr, ignoring the latter's
comment that says
* It is assumed that "stmt" represents a copy of the function source text
* beginning at offset "location", with leader text of length "leaderlen"
* (typically "SELECT ") prefixed to the source text. We use this assumption
* to transpose any error cursor position back to the function source text.
This means that syntax error positioning for errors in the argument
expressions is generally pretty wacko, but especially so if the
arguments are supplied in other than left-to-right order. An example is
create or replace function fooey() returns void as $$
declare
c1 cursor (p1 int, p2 int) for
select * from tenk1 where thousand = p1 and tenthous = p2;
begin
open c1 ( p2 := 42/, p1 := 77);
end $$ language plpgsql;
which gives this:
ERROR: syntax error at or near ";"
LINE 6: open c1 ( p2 := 42/, p1 := 77);
^
which is not going to impress anybody as helpful, either as to the message
text (the user didn't write any ";" nearby) or as to the cursor
positioning. And it doesn't look very much more professional if the error
is run-time rather than parse-time:
create or replace function fooey() returns void as $$
declare
c1 cursor (p1 int, p2 int) for
select * from tenk1 where thousand = p1 and tenthous = p2;
begin
open c1 ( p2 := 42/0, p1 := 77);
end $$ language plpgsql;
select fooey();
ERROR: division by zero
CONTEXT: SQL statement "SELECT 77
,42/0;"
PL/pgSQL function "fooey" line 6 at OPEN
where again you're displaying something almost completely unlike what the
user wrote.
I don't have any great suggestions about how to fix this :-(. Perhaps
it'd be acceptable to copy the argument-list text into the query string,
carefully replacing the parameters and := marks with appropriate amounts
of whitespace (the same number of characters, not the same number of
bytes). This would allow syntax errors from check_sql_expr to match up
correctly, and runtime errors would at least show a string that doesn't
look totally unlike what the user wrote. But it would be painful to get
the values assigned to the cursor parameters in the right order --- I
think most likely you'd need to generate a new "row" list having the
cursor parameter values in the order written in the OPEN.
Also, I concur with Royce that it would be a whole lot better to apply
check_sql_expr to each argument expression as soon as you've parsed it
off, so that typos like "p1 : = 42" get detected soon enough to be
helpful, rather than ending up with misleading error messages. Very
possibly that would also simplify getting parse-time syntax errors to
point to the right places, since you'd be calling check_sql_expr on text
not far separated from the source query. (In fact, personally I'd use
read_sql_expression2(',', ')', ...) to parse off each expression and check
it immediately.) Maybe with that fix, it'd be all right if the run-time
query rearranged the expressions into the order required by the cursor
... though I'd still counsel spending more sweat on making the run-time
string look nicer. Something like
ERROR: division by zero
CONTEXT: SQL statement "SELECT /* p1 := */ 77 , /* p2 := */ 42/0"
PL/pgSQL function "fooey" line 6 at OPEN
would be easier for users to make sense of, I think.
By accident I noticed that the parameter name matching is inaccurate,
for instance this fails to fail:
create or replace function fooey() returns void as $$
declare
c1 cursor (p1 int, p2 int) for
select * from tenk1 where thousand = p1 and tenthous = p2;
begin
open c1 ( p1 := 42, p2z := 77);
end $$ language plpgsql;
select fooey();
I think this is because of the use of strncmp() --- is that really needed
rather than just plain strcmp()?
Cursor positioning for some errors is a bit flaky, observe these cases:
ERROR: mixing positional and named parameter assignment not allowed in cursor
"c1"
LINE 6: open c1 ( p2 : = 42, p2 := 77);
^
ERROR: cursor "c1" argument 2 "p2" provided multiple times
LINE 6: open c1 ( p2 := 42, p2 := 77);
^
In both of these cases I'd have expected the syntax cursor to point at
the second "p2". I'd lose the "2" in that second message text, as well
--- the cursor argument name is sufficient, and as-is it does not read
very nicely.
On the documentation front, the patch includes a hunk that changes the
description of DECLARE to claim that the argument names are optional,
something I see no support for in the code. It also fails to document
that this patch affects the behavior of cursor FOR loops as well as OPEN,
since both of those places use read_cursor_args(
Re: [HACKERS] Call stacks and RAISE INFO
Hello 2011/10/15 Josh Berkus : > >> Now that we have syntax for adding miscellaneous options to RAISE >> statements, what I suggest we consider is a RAISE option that suppresses >> all context lines for the message, perhaps >> >> RAISE NOTICE 'fee, fi, fo, fum' USING context = false; > > Yeah, that would do it. Pavel? ;-) > I have no problem with this. A context can be false for info and true for other in default. Please, use a different identifier than "context", that can be use for reading context in future - maybe "attach_context" or some similar. Just note, when we are in this topis. I got a experience so debugging functions that contains a pattern BEGIN .. do some .. that can raise exception ... EXCEPT WHEN OTHERS .. .. do some else RAISE ... ; END IF is little bit difficult, because we lost a context information about original exception Regards Pavel > -- > Josh Berkus > PostgreSQL Experts Inc. > http://pgexperts.com > > -- > Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) > To make changes to your subscription: > http://www.postgresql.org/mailpref/pgsql-hackers > -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Underspecified window queries in regression tests
2011/10/15 Tom Lane : > I can't recall whether there was some good reason for underspecifying > these test queries. It looks like all the problematic ones were added in > commit ec4be2ee6827b6bd85e0813c7a8993cfbb0e6fa7 "Extend the set of frame > options supported for window functions", which means it was either me or > Hitoshi-san who wrote them that way, but memory is not serving this > afternoon. > I don't remember if I wrote that part or not, but I like to add explicit ORDER BY to the test cases. It doesn't appear that some reason stopped us to specify it. So +1 for adding the clauses. Regards, -- Hitoshi Harada -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Online base backup from the hot-standby
> > if (!shutdown && XLogStandbyInfoActive())
> > + {
> > LogStandbySnapshot(&checkPoint.oldestActiveXid,
> > &checkPoint.nextXid);
> > + XLogReportParameters(REPORT_ON_BACKEND);
> > + }
> >
> > Why doesn't the change of FPW need to be WAL-logged when
> > shutdown checkpoint is performed? It's helpful to add the comment
> > explaining why.
>
> Sure. I update the patch soon.
Done.
Please check this.
Regards.
Jun Ishizuka
NTT Software Corporation
TEL：045-317-7018
E-Mail: ishizuka@po.ntts.co.jp
standby_online_backup_09base-05fpw.patch
Description: Binary data
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Range Types - typo + NULL string constructor
Heikki Linnakangas writes:
> * Have you tested this on an architecture with strict alignment? I don't
> see any alignment bugs, but I think there's a lot of potential for them..
Well, fwiw, this patch passes its regression tests on HPPA, except for this
which seems more to do with the already-noted unacceptable dependency on
non-C collations:
*** /home/postgres/pgsql/src/test/regress/expected/rangetypes.out Fri Oct
14 21:19:19 2011
--- /home/postgres/pgsql/src/test/regress/results/rangetypes.outFri Oct
14 21:50:11 2011
***
*** 842,857
--
create type textrange_c as range(subtype=text, collation="C");
create type textrange_en_us as range(subtype=text, collation="en_US");
select textrange_c('a','Z') @> 'b'::text;
ERROR: range lower bound must be less than or equal to range upper bound
select textrange_en_us('a','Z') @> 'b'::text;
! ?column?
! --
! t
! (1 row)
!
drop type textrange_c;
drop type textrange_en_us;
--
-- Test out polymorphic type system
--
--- 842,858
--
create type textrange_c as range(subtype=text, collation="C");
create type textrange_en_us as range(subtype=text, collation="en_US");
+ ERROR: collation "en_US" for encoding "SQL_ASCII" does not exist
select textrange_c('a','Z') @> 'b'::text;
ERROR: range lower bound must be less than or equal to range upper bound
select textrange_en_us('a','Z') @> 'b'::text;
! ERROR: function textrange_en_us(unknown, unknown) does not exist
! LINE 1: select textrange_en_us('a','Z') @> 'b'::text;
!^
! HINT: No function matches the given name and argument types. You might need
to add explicit type casts.
drop type textrange_c;
drop type textrange_en_us;
+ ERROR: type "textrange_en_us" does not exist
--
-- Test out polymorphic type system
--
==
Also, I notice you forgot to update serial_schedule:
diff --git a/src/test/regress/serial_schedule b/src/test/regress/serial_schedule
index bb654f9c612970ef777e8cc39369a91e343f6afc..2e87d9eefd6fbb70a5603bc000ffe833
5945201f 100644
*** a/src/test/regress/serial_schedule
--- b/src/test/regress/serial_schedule
*** test: txid
*** 18,23
--- 18,24
test: uuid
test: enum
test: money
+ test: rangetypes
test: strings
test: numerology
test: point
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Online base backup from the hot-standby
> As I suggested in the reply to Simon, I think that the change of FPW
> should be WAL-logged separately from that of HS parameters. ISTM
> packing them in one WAL record makes XLogReportParameters()
> quite confusing. Thought?
I want to confirm the reply of Simon. I think we cannot decide how this
code should be if there is not the reply.
> if (!shutdown && XLogStandbyInfoActive())
> + {
> LogStandbySnapshot(&checkPoint.oldestActiveXid,
> &checkPoint.nextXid);
> + XLogReportParameters(REPORT_ON_BACKEND);
> + }
>
> Why doesn't the change of FPW need to be WAL-logged when
> shutdown checkpoint is performed? It's helpful to add the comment
> explaining why.
Sure. I update the patch soon.
Jun Ishizuka
NTT Software Corporation
TEL：045-317-7018
E-Mail: ishizuka@po.ntts.co.jp
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] about EDITOR_LINENUMBER_SWITCH
Tom Lane wrote: > Bruce Momjian writes: > > Oops, I see a problem. Right now, our first major release has no zero, > > e.g. 9.2, while our minors have a third digit, 9.2.5. The problem is > > that with this patch it is confusing to have a psql config file that > > matches 9.2.0, but not 9.2.5, because you can't write 9.2.0. > > Uh, this seems like nonsense. We've been labeling major releases with > a dot-zero for some time, and that's embedded in process now (cf > version_stamp.pl) to the point that we're unlikely to forget to do so. Ah, I see now. 9.1 has: #define PG_VERSION "9.1.0" I rarely see the non-dev trees. No problem then. Patch applied. -- Bruce Momjian http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Call stacks and RAISE INFO
Florian Pflug writes: > On Oct14, 2011, at 23:51 , Tom Lane wrote: >> It seems to me that a client-side facility would be unlikely to do the >> right things, because it has not got enough information to know which >> messages came from plpgsql RAISE commands. Moreover, it's not apparent >> that a one-size-fits-all approach is suitable anyhow: it may be that >> some RAISEs don't need context traceback while others could use it. > Hm, I think you'd usually want to adjust the verbosity of log messages > when you *run* code, not when you *write* it. That's the raison d'etre > for having logging infrastructure and verbosity settings, after all. No, I don't think so. The use-case for this sort of thing seems to me to be messages that are directed to the user or DBA, and don't want to be decorated with a lot of information about where they came from. That determination is usually pretty clear when you write the code. Moreover, if we don't attach the specification to particular RAISE commands, it's going to be "all or nothing", which is most definitely not the right thing. Having said that, if we allow "USING context = boolean_expression", it'd be possible for the plpgsql coder to make the behavior run-time adjustable if he wanted. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Call stacks and RAISE INFO
On Oct14, 2011, at 23:51 , Tom Lane wrote: > Josh Berkus writes: >>> I meant verbosity, not error level. This quick test shows what I meant >>> -- but it doesn't work; the server log is altered as I expected (and does >>> not >>> include the context lines), but not plpgsql's: > >> Yeah, what we'd need is a client_error_verbosity setting. > > It seems to me that a client-side facility would be unlikely to do the > right things, because it has not got enough information to know which > messages came from plpgsql RAISE commands. Moreover, it's not apparent > that a one-size-fits-all approach is suitable anyhow: it may be that > some RAISEs don't need context traceback while others could use it. Hm, I think you'd usually want to adjust the verbosity of log messages when you *run* code, not when you *write* it. That's the raison d'etre for having logging infrastructure and verbosity settings, after all. When I'm running a function from psql interactively, I probably want to suppress CONTEXT and maybe STATEMENT lines for NOTICEs - presumably the message itself tells me everything I need to know. When I'm running the same function in a setting where there messages go to a log file, I probably want to include as much context information as necessary in order to be able to debug possible problems post-mortem. Having said that, having the option to not emit CONTEXT lines in the first place wouldn't hurt of course. best regards, Florian Pflug -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Call stacks and RAISE INFO
On Oct14, 2011, at 20:00 , Josh Berkus wrote: >>> We do transmit the individual parts of a NOTICE separately, so I'd say >>> suppressing some of them is the client's job. So, instead of a GUC I'd say >>> we'd need a psql setting ERROR_VERBOSITY. >> >> Well, we do have a psql setting as well (\set VERBOSITY). But is Josh >> using psql? Yeah, I meant that we could split that into two settings, one for level >= ERROR and one for level < ERROR. It'd certainly be nice to be able to report progress messages from functions without seeing all the CONTEXT and STATEMENT noise when running them via psql, yet still get that information if an actual error occurs. > Not in this instance, no. But I still believe this is something that should be done client-side. best regards, Florian Pflug -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] DOMAINs and CASTs
On Fri, Oct 14, 2011 at 3:19 PM, Bruce Momjian wrote: > Where are we on this? Well, I don't know. We had a couple of different ideas on what to do about it, and I'm not sure anyone was completely in love with any of the available options. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Call stacks and RAISE INFO
> Now that we have syntax for adding miscellaneous options to RAISE > statements, what I suggest we consider is a RAISE option that suppresses > all context lines for the message, perhaps > > RAISE NOTICE 'fee, fi, fo, fum' USING context = false; Yeah, that would do it. Pavel? ;-) -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] WIP: collect frequency statistics for arrays
On Fri, Oct 14, 2011 at 3:24 PM, Bruce Momjian wrote: > Alexander Korotkov wrote: >> Version of patch with few more comments and some fixes. The CommitFest app page is here. https://commitfest.postgresql.org/action/patch_view?id=539 It was reviewed in July by Nate Boley, and never updated. Looking now, I see that Alexander wasn't Cc'd on the review, so it's possible he missed the message? -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Call stacks and RAISE INFO
Josh Berkus writes: > In any case, this doesn't address the inconsistency of supressing > CONTEXT for the first level of the call stack with RAISE INFO but not > for the others. (RAISE EXCEPTION shows CONTEXT for all levels of the > call stack). Oh? I don't see that. AFAICT the behavior is not dependent on the error severity level. RAISE just automatically suppresses the context line from the current plpgsql function. Not sure if we need to make that part of the behavior configurable or not. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Call stacks and RAISE INFO
Josh Berkus writes: >> I meant verbosity, not error level. This quick test shows what I meant >> -- but it doesn't work; the server log is altered as I expected (and does not >> include the context lines), but not plpgsql's: > Yeah, what we'd need is a client_error_verbosity setting. It seems to me that a client-side facility would be unlikely to do the right things, because it has not got enough information to know which messages came from plpgsql RAISE commands. Moreover, it's not apparent that a one-size-fits-all approach is suitable anyhow: it may be that some RAISEs don't need context traceback while others could use it. Now that we have syntax for adding miscellaneous options to RAISE statements, what I suggest we consider is a RAISE option that suppresses all context lines for the message, perhaps RAISE NOTICE 'fee, fi, fo, fum' USING context = false; regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Underspecified window queries in regression tests
So I'm testing a patch to make the planner use measured all-visible-page counts for index-only scans. I was expecting to possibly see some plan changes in the regression tests, but this diff scared me: *** *** 906,921 FROM tenk1 WHERE unique1 < 10; sum | unique1 -+- !4 | 4 !6 | 2 !3 | 1 !7 | 6 ! 15 | 9 ! 17 | 8 ! 13 | 5 !8 | 3 ! 10 | 7 !7 | 0 (10 rows) CREATE TEMP VIEW v_window AS --- 906,921 FROM tenk1 WHERE unique1 < 10; sum | unique1 -+- !0 | 0 !1 | 1 !3 | 2 !5 | 3 !7 | 4 !9 | 5 ! 11 | 6 ! 13 | 7 ! 15 | 8 ! 17 | 9 (10 rows) CREATE TEMP VIEW v_window AS On inspection, though, there's no bug. The plan did change, and that affected the order in which the rows are fetched, and that changed the window function outputs because this test case is effectively using SUM(x) OVER (ROWS 1 PRECEDING) without any ordering specifier. There are several adjacent tests that are underspecified in the same way, but their results didn't change because they aren't candidates for index-only scans. We could hack around this by adding more columns to the result so that an index-only scan doesn't work. But I wonder whether it wouldn't be smarter to add ORDER BY clauses to the window function calls. I've been known to argue against adding just-in-case ORDER BYs to the regression tests in the past; but these cases bother me more because a plan change will not just rearrange the result rows but change their contents, making it really difficult to verify that nothing's seriously wrong. I can't recall whether there was some good reason for underspecifying these test queries. It looks like all the problematic ones were added in commit ec4be2ee6827b6bd85e0813c7a8993cfbb0e6fa7 "Extend the set of frame options supported for window functions", which means it was either me or Hitoshi-san who wrote them that way, but memory is not serving this afternoon. Opinions? regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] about EDITOR_LINENUMBER_SWITCH
Bruce Momjian writes: > Oops, I see a problem. Right now, our first major release has no zero, > e.g. 9.2, while our minors have a third digit, 9.2.5. The problem is > that with this patch it is confusing to have a psql config file that > matches 9.2.0, but not 9.2.5, because you can't write 9.2.0. Uh, this seems like nonsense. We've been labeling major releases with a dot-zero for some time, and that's embedded in process now (cf version_stamp.pl) to the point that we're unlikely to forget to do so. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] about EDITOR_LINENUMBER_SWITCH
Alvaro Herrera wrote: > > Excerpts from Robert Haas's message of vie oct 14 09:36:47 -0300 2011: > > > > On Thu, Oct 13, 2011 at 11:31 AM, Bruce Momjian wrote: > > > Alvaro Herrera wrote: > > > >> Oh, true, we have that, though it's not very usable because you have to > > >> rename the file from .psqlrc-9.0.3 to .psqlrc-9.0.4 when you upgrade, > > >> which is kinda silly. > > > > > > True. ??We don't add configuration changes in minor versions so having > > > minor-version granularity makes no sense. > > > > > > The attached patch changes this to use the _major_ version number for > > > psql rc files. ??Does this have to be backward-compatible? ??Should I > > > check for minor and major matches? ??That is going to be confusing to > > > document. > > > > Checking for a minor match and then a major match seems sensible. > > And backwards compatible too! +1 to that. An idea that you can > describe in six words doesn't seem all that confusing. Oops, I see a problem. Right now, our first major release has no zero, e.g. 9.2, while our minors have a third digit, 9.2.5. The problem is that with this patch it is confusing to have a psql config file that matches 9.2.0, but not 9.2.5, because you can't write 9.2.0. A file called .psql-9.2 matches 9.2.0, but also matches 9.2.X if there is no matching minor release file. The bottom line is that with this patch, .psql-9.2 is both a minor and possibly minor matcher. I can't blame the patch, but rather our version numbering system. Prior to the patch 9.2 always meant just 9.2.0. This patch adds an additional confusion. -- Bruce Momjian http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] WIP: collect frequency statistics for arrays
Alexander Korotkov wrote: > Version of patch with few more comments and some fixes. Where are we on this? -- Bruce Momjian http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] DOMAINs and CASTs
Where are we on this? --- David Fetter wrote: > On Mon, Jun 13, 2011 at 03:39:39AM -0500, Jaime Casanova wrote: > > On Mon, Jun 6, 2011 at 6:36 AM, Peter Eisentraut wrote: > > > On tis, 2011-05-17 at 14:11 -0500, Jaime Casanova wrote: > > >> On Tue, May 17, 2011 at 12:19 PM, Robert Haas > > >> wrote: > > >> > > > >> > The more controversial question is what to do if someone tries to > > >> > create such a cast anyway. ?We could just ignore that as we do now, or > > >> > we could throw a NOTICE, WARNING, or ERROR. > > >> > > >> IMHO, not being an error per se but an implementation limitation i > > >> would prefer to send a WARNING > > > > > > Implementation limitations are normally reported as errors. ?I don't see > > > why it should be different here. > > > > > > > ok, patch reports an error... do we want to backpatch this? if we want > > to do so maybe we can backpatch as a warning > > Minor clarification attached. > > Cheers, > David. > -- > David Fetter http://fetter.org/ > Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter > Skype: davidfetter XMPP: david.fet...@gmail.com > iCal: webcal://www.tripit.com/feed/ical/people/david74/tripit.ics > > Remember to vote! > Consider donating to Postgres: http://www.postgresql.org/about/donate [ Attachment, skipping... ] > > -- > Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) > To make changes to your subscription: > http://www.postgresql.org/mailpref/pgsql-hackers -- Bruce Momjian http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pg_dump vs malloc
Magnus Hagander wrote: > On Wed, Jun 22, 2011 at 17:48, Tom Lane wrote: > > Magnus Hagander writes: > >> Something along the line of this? > > > > I think this is a seriously, seriously bad idea: > > > >> +#define strdup(x) pg_strdup(x) > >> +#define malloc(x) pg_malloc(x) > >> +#define calloc(x,y) pg_calloc(x, y) > >> +#define realloc(x,y) pg_realloc(x, y) > > > > as it will render the code unreadable to people expecting the normal > > behavior of these fundamental functions; not to mention break any > > call sites that have some other means of dealing with an alloc failure > > besides going belly-up. ?Please take the trouble to do > > s/malloc/pg_malloc/g and so on, instead. > > Ok, I'll try that approach. This seemed like a "nicer" approach, but I > think once written out, i agree with your arguments :-) Where are we on this? -- Bruce Momjian http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Large C files
Excerpts from Peter Geoghegan's message of vie oct 14 15:36:32 -0300 2011: > This evening, David Fetter described a problem to me that he was > having building the Twitter FDW. It transpired that it was down to its > dependence on an #include that was recently judged to be > redundant.This seems like another reason to avoid using pgrminclude - > even if we can be certain that the #includes are redundant within > Postgres, we cannot be sure that they are redundant in third party > code. I'm not sure about this. I mean, surely the problem was easily detected and fixed because the compile fails outright, yes? I do take time on ocassion to do some header reorganization and remove inclusions that are redundant or unnecessary. I don't want that work thrown aside because we have to support some hypothetical third party module that would fail to compile. (In fact, we purposedly removed some header from spi.h because it was unnecessary). -- Álvaro Herrera The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Large C files
This evening, David Fetter described a problem to me that he was having building the Twitter FDW. It transpired that it was down to its dependence on an #include that was recently judged to be redundant.This seems like another reason to avoid using pgrminclude - even if we can be certain that the #includes are redundant within Postgres, we cannot be sure that they are redundant in third party code. -- Peter Geoghegan http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training and Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] patch: move dumpUserConfig call in dumpRoles function of pg_dumpall.c
On Wed, Oct 12, 2011 at 7:07 PM, Tom Lane wrote: > Phil Sorber writes: >> Then there is a separate section of code that is called as a separate >> function 'dumpUserConfig()' that does other role attributes like >> 'ALTER ROLE bob SET role TO charlie'. These are the ALTER's that can >> have dependencies on other roles. > > Right. Phrased that way, this is an obvious improvement, and I concur > that it doesn't look like it could break anything that works now. > The more general problem remains to be solved, but we might as well > apply this. OK, done. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Call stacks and RAISE INFO
>> We do transmit the individual parts of a NOTICE separately, so I'd say >> suppressing some of them is the client's job. So, instead of a GUC I'd say >> we'd need a psql setting ERROR_VERBOSITY. > > Well, we do have a psql setting as well (\set VERBOSITY). But is Josh > using psql? Not in this instance, no. In any case, this doesn't address the inconsistency of supressing CONTEXT for the first level of the call stack with RAISE INFO but not for the others. (RAISE EXCEPTION shows CONTEXT for all levels of the call stack). -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Call stacks and RAISE INFO
Excerpts from Florian Pflug's message of vie oct 14 14:41:11 -0300 2011: > > On Oct14, 2011, at 19:27 , Josh Berkus wrote: > >> I meant verbosity, not error level. This quick test shows what I meant > >> -- but it doesn't work; the server log is altered as I expected (and does > >> not > >> include the context lines), but not plpgsql's: > > > > Yeah, what we'd need is a client_error_verbosity setting. > > We do transmit the individual parts of a NOTICE separately, so I'd say > suppressing some of them is the client's job. So, instead of a GUC I'd say > we'd need a psql setting ERROR_VERBOSITY. Well, we do have a psql setting as well (\set VERBOSITY). But is Josh using psql? -- Álvaro Herrera The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Call stacks and RAISE INFO
On Oct14, 2011, at 19:27 , Josh Berkus wrote: >> I meant verbosity, not error level. This quick test shows what I meant >> -- but it doesn't work; the server log is altered as I expected (and does not >> include the context lines), but not plpgsql's: > > Yeah, what we'd need is a client_error_verbosity setting. We do transmit the individual parts of a NOTICE separately, so I'd say suppressing some of them is the client's job. So, instead of a GUC I'd say we'd need a psql setting ERROR_VERBOSITY. best regards, Florian Pflug -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Bugs in information_schema.referential_constraints view
On Fri, Oct 14, 2011 at 12:05 PM, Tom Lane wrote: > Is this important enough to back-patch? IMHO, yes. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] tuning autovacuum
On Fri, Oct 14, 2011 at 12:59 PM, Josh Berkus wrote: >> Ideally we would have something like checkpoint_warning that warns users >> in the log when there are too few autovacuum workers and cleanup is >> being delayed. > > I don't think that any table-stats based approach is going to work. I > think you need to measure the queue of tables which need autovacuuming. > So you do something like: > > If > 10% of tables and > 10 tables need autovac/autoanalyze for more > than one polling interval in a row, then emit a warning. > > Note that there are solutions other than adding workers; the user could > also lower the polling interval, decrease vacuum_delay, or do other > things to make autovac faster. > > This would require tracking stats about the size of the autovac queue. Right. It's my feeling that that's exactly what we need to do. It's similar to what we already do for checkpoint spreading, except applied to a different system maintenance activity. What would be really neat is if we could not just detect the problem, but actually adjust the cost delay on the fly to try to fix it - again, like we do with checkpoints. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Call stacks and RAISE INFO
> I meant verbosity, not error level. This quick test shows what I meant > -- but it doesn't work; the server log is altered as I expected (and does not > include the context lines), but not plpgsql's: Yeah, what we'd need is a client_error_verbosity setting. -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] SLRU limits
Bruce Momjian wrote: > Is this a TODO? > Tom Lane wrote: >> Heikki Linnakangas writes: >>> If we don't want to change it wholesale, one option would be to >>> support different lengths of filenames in slru.c for different >>> slrus. At a quick glance, it seems pretty easy. That would allow >>> keeping clog unchanged - that's the one that's most likely to >>> have unforeseen consequences if changed. pg_subtrans and >>> pg_serial are more ephemeral, they don't need to be retained >>> over shutdown, so they seem less likely to cause trouble. That >>> seems like the best option to me. >> >> I agree with Robert that this is completely not urgent. If you >> want to fool with it for 9.2, fine, but let's not destabilize 9.1 >> for it. >> >> (BTW, while I've not looked at the SLRU code in several years, >> I'm quite unconvinced that this is only a matter of filename >> lengths.) I think it should be. I agree that the workaround is kinda ugly, and this is really the only clean solution I see. I don't think it's so ugly that it takes precedence over trying to bring some of Robert's LW locking improvements into the SSI area, so this one is likely not to get into 9.2, in spite of appearing to be a smaller change. So a TODO entry seems like the right way to deal with it. -Kevin -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Call stacks and RAISE INFO
Excerpts from Josh Berkus's message of vie oct 14 14:16:43 -0300 2011: > > > Maybe set the verbosity to a lower level in the function? I dunno if > > plpgsql lets you do that though. We have a GUC that controls the server > > log verbosity, and psql can do it too; but plpgsql is sort of in > > between. > > The problem is that there is no level of verbosity which will supress > the CONTEXT messages and not supress the INFO messages as well. Not > that I've found, anyway. I meant verbosity, not error level. This quick test shows what I meant -- but it doesn't work; the server log is altered as I expected (and does not include the context lines), but not plpgsql's: alvherre=# create function f() returns void language plpgsql as $$ begin raise info 'hello'; end $$; CREATE FUNCTION alvherre=# select f(); INFO: hello f --- (1 fila) alvherre=# create function g() returns void language plpgsql as $$ begin perform f(); end $$; CREATE FUNCTION alvherre=# select g(); INFO: hello CONTEXTO: SQL statement "SELECT f()" función PL/pgSQL «g» en la línea 1 en PERFORM g --- (1 fila) alvherre=# alter function g() set log_error_verbosity to 'terse'; ALTER FUNCTION alvherre=# select g(); INFO: hello CONTEXTO: SQL statement "SELECT f()" función PL/pgSQL «g» en la línea 1 en PERFORM g --- (1 fila) alvherre=# alter function f() set log_error_verbosity to 'terse'; ALTER FUNCTION alvherre=# select g(); INFO: hello CONTEXTO: SQL statement "SELECT f()" función PL/pgSQL «g» en la línea 1 en PERFORM g --- (1 fila) -- Álvaro Herrera The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Call stacks and RAISE INFO
> Maybe set the verbosity to a lower level in the function? I dunno if > plpgsql lets you do that though. We have a GUC that controls the server > log verbosity, and psql can do it too; but plpgsql is sort of in > between. The problem is that there is no level of verbosity which will supress the CONTEXT messages and not supress the INFO messages as well. Not that I've found, anyway. -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Call stacks and RAISE INFO
Excerpts from Josh Berkus's message of vie oct 14 13:52:32 -0300 2011: > All, > > I'm noticing some inconsistent and (I believe) undesirable behavior on > RAISE INFO. > > If you call a function, and it posts progress reports using RAISE INFO, > then you get the INFO statements plain back to the client. However, if > that function calls another function, then you also get a three-line > CONTEXT message ... and if the functions are recursively called, the > CONTEXT message will emit 3 lines for every level of the call stack. > > This seems like reasonable behavior for RAISE EXCEPTION but not RAISE > INFO. It pretty much makes INFO notices useless as end-user feedback if > you have functions calling other functions because of the amount of > garbage on the screen. Is this a bug? Maybe set the verbosity to a lower level in the function? I dunno if plpgsql lets you do that though. We have a GUC that controls the server log verbosity, and psql can do it too; but plpgsql is sort of in between. -- Álvaro Herrera The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Bugs in information_schema.referential_constraints view
> Is this important enough to back-patch? We can't force initdb in back > branches, but we could suggest that people could drop and re-create the > information_schema (I think that's supposed to work). I wouldn't worry about emphasizing the rebuild. Most users don't use information_schema, and for those who do the refconstraints view is known to be useless (just try using it for a multicolumn FK). -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Core Extensions relocation
On 14 October 2011 17:48, Bruce Momjian wrote: > > Is this going to be done for 9.2? > > --- I didn't spot this thread before. I posted something related yesterday: http://archives.postgresql.org/pgsql-hackers/2011-10/msg00781.php -- Thom Brown Twitter: @darkixion IRC (freenode): dark_ixion Registered Linux user: #516935 EnterpriseDB UK: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] tuning autovacuum
> Ideally we would have something like checkpoint_warning that warns users > in the log when there are too few autovacuum workers and cleanup is > being delayed. I don't think that any table-stats based approach is going to work. I think you need to measure the queue of tables which need autovacuuming. So you do something like: If > 10% of tables and > 10 tables need autovac/autoanalyze for more than one polling interval in a row, then emit a warning. Note that there are solutions other than adding workers; the user could also lower the polling interval, decrease vacuum_delay, or do other things to make autovac faster. This would require tracking stats about the size of the autovac queue. -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] SLRU limits
Is this a TODO? --- Tom Lane wrote: > Heikki Linnakangas writes: > > On 09.06.2011 15:50, Robert Haas wrote: > >> And I would guess that there's a lot more interest in raising BLCKSZ > >> than lowering it. It might not be a bad idea to adopt the fix you > >> propose anyway, but it doesn't seem urgent. > > > I guess we could fix pg_subtrans by not allowing BLCKSZ < 8k. That > > leaves the problem with pg_serial. Kevin has already worked around, but > > I'm not very happy with that workaround. > > > If we don't want to change it wholesale, one option would be to support > > different lengths of filenames in slru.c for different slrus. At a quick > > glance, it seems pretty easy. That would allow keeping clog unchanged - > > that's the one that's most likely to have unforeseen consequences if > > changed. pg_subtrans and pg_serial are more ephemeral, they don't need > > to be retained over shutdown, so they seem less likely to cause trouble. > > That seems like the best option to me. > > I agree with Robert that this is completely not urgent. If you want to > fool with it for 9.2, fine, but let's not destabilize 9.1 for it. > > (BTW, while I've not looked at the SLRU code in several years, I'm quite > unconvinced that this is only a matter of filename lengths.) > > regards, tom lane > > -- > Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) > To make changes to your subscription: > http://www.postgresql.org/mailpref/pgsql-hackers -- Bruce Momjian http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Call stacks and RAISE INFO
All, I'm noticing some inconsistent and (I believe) undesirable behavior on RAISE INFO. If you call a function, and it posts progress reports using RAISE INFO, then you get the INFO statements plain back to the client. However, if that function calls another function, then you also get a three-line CONTEXT message ... and if the functions are recursively called, the CONTEXT message will emit 3 lines for every level of the call stack. This seems like reasonable behavior for RAISE EXCEPTION but not RAISE INFO. It pretty much makes INFO notices useless as end-user feedback if you have functions calling other functions because of the amount of garbage on the screen. Is this a bug? -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Core Extensions relocation
Is this going to be done for 9.2? --- Greg Smith wrote: > Following up on the idea we've been exploring for making some extensions > more prominent, attached is the first rev that I think may be worth > considering seriously. Main improvement from the last is that I > reorganized the docs to break out what I decided to tentatively name > "Core Extensions" into their own chapter. No longer mixed in with the > rest of the contrib modules, and I introduce them a bit differently. > If you want to take a quick look at the new page, I copied it to > http://www.2ndquadrant.us/docs/html/extensions.html > > I'm not completely happy on the wordering there yet. The use of both > "modules" and "extensions" is probably worth eliminating, and maybe that > continues on to doing that against the language I swiped from the > contrib intro too. There's also a lot of shared text at the end there, > common wording from that and the contrib page about how to install and > migrate these extensions. Not sure how to refactor it out into another > section cleanly though. > > Regression tests came up last time I posted this. Doesn't look like > there are any for the modules I'm suggesting should be promoted. Only > code issue I noticed during another self-review here is that I didn't > rename contrib/pgrowlocks/pgrowlocks--unpackaged--1.0.sql cleanly, may > need to do that one over again to get the commits as clean as possible. > > Updated code is at > https://github.com/greg2ndQuadrant/postgres/tree/move-contrib too, and > since this is painful as a patch the compare view at > https://github.com/greg2ndQuadrant/postgres/compare/master...move-contrib > will be easier for browsing the code changes. > > -- > Greg Smith 2ndQuadrant USg...@2ndquadrant.com Baltimore, MD > PostgreSQL Training, Services, and 24x7 Support www.2ndQuadrant.us > > > > -- > Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) > To make changes to your subscription: > http://www.postgresql.org/mailpref/pgsql-hackers -- Bruce Momjian http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] tuning autovacuum
Robert Haas wrote: > On Wed, Jun 8, 2011 at 10:55 PM, Euler Taveira de Oliveira > wrote: > > Em 08-06-2011 20:35, Robert Haas escreveu: > >> Is the hint correct? ?I mean, what if there were 100 small tables that > >> needed vacuuming all at the same time. ?We'd hit this limit no matter > >> how high you set autovacuum_max_workers, but it wouldn't be right to > >> set it to 101 just because every once in a blue moon you might trip > >> over the limit. > >> > > I think so. You are picturing a scene with only one message. It is the same > > case of the too-frequent-checkpoint messages; i.e., you should look if those > > messages have some periodicity. > > Yeah, maybe. I'm just not sure there would be an easy way for users > to judge when they should or should not make a change. > > >> I think it'd be really useful to expose some more data in this area > >> though. ?One random idea is - remember the time at which a table was > >> first observed to need vacuuming. Clear the timestamp when it gets > >> vacuumed. ?Then you can do: > >> > > Hmmm. But this fine grained information alone doesn't help tuning the number > > of autovacuum workers. I consider counters easier to implement and simpler > > to analyze. But the timestamp idea has its merit because we already have a > > similar statistic (last timestamp table was vacuumed or analyzed). > > Well, it won't directly tell you how many you need. But certainly if > you see things getting further and further behind, you know you need > more. > > Or, alternatively, you need to reduce vacuum_cost_delay. IME, that's > actually the most common cause of this problem. This thread from June died because there was concern about the overhead of additional autovacuum statistics, and I have to say I am not super-excited about it either because most users will not use them. Ideally we would have something like checkpoint_warning that warns users in the log when there are too few autovacuum workers and cleanup is being delayed. The big trick is how to accurately measure this. The amount of time that a table waits to be vacuumed probably isn't relevant enough --- it might have been days since it was last vacuumed, and waiting 10 minutes isn't a big deal, so it is hard to say what _scale_ we would give users for that warning that would make sense. We could compare it to the time since the last autovacuum, but if the table is suddently heavily modified, that doesn't help either. I think it has to drive off of the 'n_dead_tuples' statistic value for the table. I was toying with the idea of comparing the n_dead_tuples value at the time the table is first scanned for autovacuum consideration, and the value at the time an autovacuum worker actually starts scanning the table. The problem there is that if someone does a massive DELETE in that time interval, or does an UPDATE on all the rows, it would think that autovacuum should have been there to mark some dead rows, but it was not. In the case of DELETE, having autovacuum work earlier would not have helped, but it would have helped in the UPDATE case. We could look at table size growth during that period. If the autovacuum had run earlier, we would have used that dead space, but is wasn't recorded by autovacuum yet, but again, it seems vague. Ideas? -- Bruce Momjian http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Isolation tests still falling over routinely
Alvaro Herrera writes: > Excerpts from Tom Lane's message of mar sep 20 21:30:42 -0300 2011: >> Could we please fix those tests to not have such >> fragile timing assumptions? > The fix has now been installed for two weeks and no new failure has > occured. The only failure in the IsolationCheck phase since then was > caused by a disk filling up (and it wasn't in the fk-* tests anyway). > I think we can consider this issue fixed. Yeah, it looks good. Thanks! regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Bugs in information_schema.referential_constraints view
Consider this example in an empty database:
db=# create table t1 (f1 int);
CREATE TABLE
db=# create unique index t1f1 on t1(f1);
CREATE INDEX
db=# create table t2 (f2 int references t1(f1));
CREATE TABLE
db=# create table t3(f3 int primary key);
NOTICE: CREATE TABLE / PRIMARY KEY will create implicit index "t3_pkey" for
table "t3"
CREATE TABLE
db=# select * from information_schema.referential_constraints;
constraint_catalog | constraint_schema | constraint_name |
unique_constraint_catalog | unique_constraint_schema | unique_constraint_name |
match_option | update_rule | delete_rule
+---+-+---+--++--+-+-
db | public| t2_f2_fkey |
| || NONE | NO
ACTION | NO ACTION
(1 row)
Okay so far. The lack of unique_constraint_name etc is correct because
there is no unique constraint supporting this FK constraint, only a
unique index. But now:
db=# alter table t1 add constraint t1_ref_t3 foreign key (f1) references t3;
ALTER TABLE
db=# select * from information_schema.referential_constraints;
constraint_catalog | constraint_schema | constraint_name |
unique_constraint_catalog | unique_constraint_schema | unique_constraint_name |
match_option | update_rule | delete_rule
+---+-+---+--++--+-+-
db | public| t1_ref_t3 | db
| public | t3_pkey| NONE | NO
ACTION | NO ACTION
(1 row)
Ooops, what became of t2_f2_fkey?
The reason is that the core of the view is
FROM (pg_namespace ncon
INNER JOIN pg_constraint con ON ncon.oid = con.connamespace
INNER JOIN pg_class c ON con.conrelid = c.oid)
LEFT JOIN
(pg_constraint pkc
INNER JOIN pg_namespace npkc ON pkc.connamespace = npkc.oid)
ON con.confrelid = pkc.conrelid
AND _pg_keysequal(con.confkey, pkc.conkey)
WHERE c.relkind = 'r'
AND con.contype = 'f'
AND (pkc.contype IN ('p', 'u') OR pkc.contype IS NULL)
and that last line is failing to consider the possibility that we'll
find an accidental match to a pkc row that has contype other than
'p' or 'u'. Instead of plastering on an IS NULL alternative, the
restriction on pkc.contype needs to be in or below the LEFT JOIN.
There might be other bugs of the same sort, I haven't looked.
But wait, there's more:
db=# drop table t1,t2,t3;
DROP TABLE
db=# create table t1 (f1 int primary key);
NOTICE: CREATE TABLE / PRIMARY KEY will create implicit index "t1_pkey" for
table "t1"
CREATE TABLE
db=# alter table t1 add constraint useless_duplicate unique(f1);
NOTICE: ALTER TABLE / ADD UNIQUE will create implicit index
"useless_duplicate" for table "t1"
ALTER TABLE
db=# create table t2 (f2 int references t1(f1));
CREATE TABLE
db=# select * from information_schema.referential_constraints;
constraint_catalog | constraint_schema | constraint_name |
unique_constraint_catalog | unique_constraint_schema | unique_constraint_name |
match_option | update_rule | delete_rule
+---+-+---+--++--+-+-
db | public| t2_f2_fkey | db
| public | t1_pkey| NONE | NO
ACTION | NO ACTION
db | public| t2_f2_fkey | db
| public | useless_duplicate | NONE | NO
ACTION | NO ACTION
(2 rows)
t2_f2_fkey is shown twice, because there are two matches to potential
supporting unique constraints. This is bogus because it violates
the supposed primary key of the view.
It gets worse:
db=# drop table t1,t2;
DROP TABLE
db=# create table t1 (f1 int);
CREATE TABLE
db=# create unique index t1f1 on t1(f1);
CREATE INDEX
db=# create table t2 (f2 int references t1(f1));
CREATE TABLE
db=# select * from information_schema.referential_constraints;
constraint_catalog | constraint_schema | constraint_name |
unique_constraint_catalog | unique_constraint_schema | unique_constraint_name |
match_option | update_rule | delete_rule
+---+-+---+--++--+-+-
db | public| t2_f2_fkey |
| || NONE | NO
ACTION | NO ACTION
(1 row)
db=# a
Re: [HACKERS] Isolation tests still falling over routinely
Excerpts from Tom Lane's message of mar sep 20 21:30:42 -0300 2011: > > The buildfarm is still showing isolation test failures more days than > not, eg > http://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=pika&dt=2011-09-17%2012%3A43%3A11 > and I've personally seen such failures when testing with > CLOBBER_CACHE_ALWAYS. Could we please fix those tests to not have such > fragile timing assumptions? The fix has now been installed for two weeks and no new failure has occured. The only failure in the IsolationCheck phase since then was caused by a disk filling up (and it wasn't in the fk-* tests anyway). I think we can consider this issue fixed. -- Álvaro Herrera The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [bug] relcache leaks in get_object_address
On Thu, Oct 13, 2011 at 12:27 PM, Kohei KaiGai wrote: > The attached patch fixes this problem. > Unfortunately, we have no code that invokes get_object_address() > with missing_ok = true now, so please apply a couple of patches > to rework DROP statement of mine. > > DROP TRIGGER no_such_trigger ON existing_table; > > shall cause a relcache reference leaks, without this patch. We could argue that that's a bug in your drop reworks rather than the existing code, but I'm willing to go along with this approach, so, committed. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] patch for new feature: Buffer Cache Hibernation
Alvaro Herrera wrote: > > Excerpts from Bruce Momjian's message of vie oct 14 12:12:22 -0300 2011: > > > > Alvaro Herrera wrote: > > > > The guideline, last I checked, was that before getting into coding any > > > item from the TODO list, the prospective hacker should check previous > > > discussions and initiate a new one on this list to ensure consensus. > > > Unless something is blatantly "not wanted", I don't think it should be > > > removed from the TODO list. There not being consensus does not mean > > > that there cannot ever be. > > > > OK. But if we are pretty sure we don't want something, e.g. hibernate, > > we shouldn't add it. > > If we're so sure we don't want it, we could add it to the "features we > do not want" section. But as Robert says downthread, I don't see us Those are for features that people often ask for, and we don't want. I am sure there are a lot of things we don't want. > being so sure that we don't want hibernation. So, add it? -- Bruce Momjian http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] patch for new feature: Buffer Cache Hibernation
Robert Haas writes: > On Fri, Oct 14, 2011 at 11:12 AM, Bruce Momjian wrote: >> OK. But if we are pretty sure we don't want something, e.g. hibernate, >> we shouldn't add it. > Fair enough, but I'm not even slightly sure that we don't want that. > I think having prewarming utilities available as contrib modules or on > PGXN would be useful, but integrating something into the backend would > allow it to be far more automated. Right. I think this one falls into my class #2, ie, we have no idea how to implement it usefully. Doesn't (necessarily) mean that the core concept is without merit. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] patch for new feature: Buffer Cache Hibernation
Excerpts from Bruce Momjian's message of vie oct 14 12:12:22 -0300 2011: > > Alvaro Herrera wrote: > > The guideline, last I checked, was that before getting into coding any > > item from the TODO list, the prospective hacker should check previous > > discussions and initiate a new one on this list to ensure consensus. > > Unless something is blatantly "not wanted", I don't think it should be > > removed from the TODO list. There not being consensus does not mean > > that there cannot ever be. > > OK. But if we are pretty sure we don't want something, e.g. hibernate, > we shouldn't add it. If we're so sure we don't want it, we could add it to the "features we do not want" section. But as Robert says downthread, I don't see us being so sure that we don't want hibernation. -- Álvaro Herrera The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] patch for new feature: Buffer Cache Hibernation
Alvaro Herrera writes: > Excerpts from Bruce Momjian's message of vie oct 14 11:56:22 -0300 2011: >> Tom Lane wrote: >>> There is plenty of stuff in the TODO list for which there is no >>> consensus. >> Uh, we should probably remove those then. Can you think of any? > Unless something is blatantly "not wanted", I don't think it should be > removed from the TODO list. There not being consensus does not mean > that there cannot ever be. Yeah. The reason why something is on the TODO list (and not already done) is typically one of 1. It's too hard, or too long/boring for the expected value. 2. There's no consensus about how to implement the feature. 3. There's no consensus about the user-visible design of the feature. Cases where there's debate about whether we want it at all seem to me to be a subset of #3. But for anything in #3, someone could do the legwork or have the bright idea needed to create consensus about how to design the feature. My gripe about the TODO list is not that we have some stuff in there that's not clearly wanted, it's that some of the entries fail to make it clear where the issue stands on this scale. That could lead people to waste time trying to code something that there's not consensus for the design or implementation of. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] patch for new feature: Buffer Cache Hibernation
On Fri, Oct 14, 2011 at 11:12 AM, Bruce Momjian wrote: > OK. But if we are pretty sure we don't want something, e.g. hibernate, > we shouldn't add it. Fair enough, but I'm not even slightly sure that we don't want that. I think having prewarming utilities available as contrib modules or on PGXN would be useful, but integrating something into the backend would allow it to be far more automated. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pg_comments (was: Allow \dd to show constraint comments)
On Wed, Oct 12, 2011 at 10:20 PM, Josh Kupershmidt wrote: >> On the third hand, Josh's previous batch of changes to clean up >> psql's behavior in this area are clearly a huge improvement: you can >> now display the comment for nearly anything by running the appropriate >> \d command for whatever the object type is. So ... is this still >> a good idea, or should we just forget about it? > > I think this question is a part of a broader concern, namely do we > want to create and support system views for easier access to > information which is already available in different ways through psql > commands, or by manually digging around in the catalogs? I believe > there are at least several examples of existing views we maintain > which are very similar to pg_comments: pg_seclabel seems quite > similar, for instance. That's one's a direct analogue, but I don't want to overbroaden the issue. I guess it just seems to me that if no one's going to champion adding this, maybe we shouldn't. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] patch for new feature: Buffer Cache Hibernation
Alvaro Herrera wrote: > > Excerpts from Bruce Momjian's message of vie oct 14 11:56:22 -0300 2011: > > Tom Lane wrote: > > > =?ISO-8859-1?Q?C=E9dric_Villemain?= > > > writes: > > > > 2011/10/14 Bruce Momjian : > > > >> Should this be marked as TODO? > > > > > > > I suppose TODO items *are* wanted and so working on them should remove > > > > the pain to convince people here to accept the feature, aren't they ? > > > > > > There is plenty of stuff in the TODO list for which there is no > > > consensus. > > > > Uh, we should probably remove those then. Can you think of any? > > The guideline, last I checked, was that before getting into coding any > item from the TODO list, the prospective hacker should check previous > discussions and initiate a new one on this list to ensure consensus. > Unless something is blatantly "not wanted", I don't think it should be > removed from the TODO list. There not being consensus does not mean > that there cannot ever be. OK. But if we are pretty sure we don't want something, e.g. hibernate, we shouldn't add it. -- Bruce Momjian http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] patch for new feature: Buffer Cache Hibernation
Excerpts from Bruce Momjian's message of vie oct 14 11:56:22 -0300 2011: > Tom Lane wrote: > > =?ISO-8859-1?Q?C=E9dric_Villemain?= > > writes: > > > 2011/10/14 Bruce Momjian : > > >> Should this be marked as TODO? > > > > > I suppose TODO items *are* wanted and so working on them should remove > > > the pain to convince people here to accept the feature, aren't they ? > > > > There is plenty of stuff in the TODO list for which there is no > > consensus. > > Uh, we should probably remove those then. Can you think of any? The guideline, last I checked, was that before getting into coding any item from the TODO list, the prospective hacker should check previous discussions and initiate a new one on this list to ensure consensus. Unless something is blatantly "not wanted", I don't think it should be removed from the TODO list. There not being consensus does not mean that there cannot ever be. -- Álvaro Herrera The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] about EDITOR_LINENUMBER_SWITCH
Excerpts from Robert Haas's message of vie oct 14 09:36:47 -0300 2011: > > On Thu, Oct 13, 2011 at 11:31 AM, Bruce Momjian wrote: > > Alvaro Herrera wrote: > >> Oh, true, we have that, though it's not very usable because you have to > >> rename the file from .psqlrc-9.0.3 to .psqlrc-9.0.4 when you upgrade, > >> which is kinda silly. > > > > True. We don't add configuration changes in minor versions so having > > minor-version granularity makes no sense. > > > > The attached patch changes this to use the _major_ version number for > > psql rc files. Does this have to be backward-compatible? Should I > > check for minor and major matches? That is going to be confusing to > > document. > > Checking for a minor match and then a major match seems sensible. And backwards compatible too! +1 to that. An idea that you can describe in six words doesn't seem all that confusing. -- Álvaro Herrera The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] about EDITOR_LINENUMBER_SWITCH
Robert Haas wrote:
> On Thu, Oct 13, 2011 at 11:31 AM, Bruce Momjian wrote:
> > Alvaro Herrera wrote:
> >> Excerpts from Tom Lane's message of mi? may 25 16:07:55 -0400 2011:
> >> > Alvaro Herrera writes:
> >> > > Excerpts from Tom Lane's message of mar may 24 17:11:17 -0400 2011:
> >> > >> Right. ?It would also increase the cognitive load on the user to have
> >> > >> to remember the command-line go-to-line-number switch for his editor.
> >> > >> So I don't particularly want to redesign this feature. ?However, I can
> >> > >> see the possible value of letting EDITOR_LINENUMBER_SWITCH be set from
> >> > >> the same place that you set EDITOR, which would suggest that we allow
> >> > >> the value to come from an environment variable. ?I'm not sure whether
> >> > >> there is merit in allowing both that source and ~/.psqlrc, though
> >> > >> possibly for Windows users it might be easier if ~/.psqlrc worked.
> >> >
> >> > > If we're going to increase the number of options in .psqlrc that do not
> >> > > work with older psql versions, can I please have .psqlrc-MAJORVERSION
> >> > > or
> >> > > some such? ?Having 8.3's psql complain all the time because it doesn't
> >> > > understand "linestyle" is annoying.
> >> >
> >> > 1. I thought we already did have that.
> >>
> >> Oh, true, we have that, though it's not very usable because you have to
> >> rename the file from .psqlrc-9.0.3 to .psqlrc-9.0.4 when you upgrade,
> >> which is kinda silly.
> >
> > True. ?We don't add configuration changes in minor versions so having
> > minor-version granularity makes no sense.
> >
> > The attached patch changes this to use the _major_ version number for
> > psql rc files. ?Does this have to be backward-compatible? ?Should I
> > check for minor and major matches? ?That is going to be confusing to
> > document.
>
> Checking for a minor match and then a major match seems sensible.
Done, and documented in the attached patch.
--
Bruce Momjian http://momjian.us
EnterpriseDB http://enterprisedb.com
+ It's impossible for everything to be true. +
diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml
new file mode 100644
index 662eab7..4eefe3b
*** a/doc/src/sgml/ref/psql-ref.sgml
--- b/doc/src/sgml/ref/psql-ref.sgml
*** PSQL_EDITOR_LINENUMBER_ARG='--line '
*** 3332,3339
Both the system-wide psqlrc file and the user's
~/.psqlrc file can be made version-specific
by appending a dash and the PostgreSQL
! release number, for example ~/.psqlrc-&version;.
! A matching version-specific file will be read in preference to a
non-version-specific file.
--- 3332,3341
Both the system-wide psqlrc file and the user's
~/.psqlrc file can be made version-specific
by appending a dash and the PostgreSQL
! major or minor release number, for example
! ~/.psqlrc-9.2 or
! ~/.psqlrc-9.2.5. The most specific
! version-matching file will be read in preference to a
non-version-specific file.
diff --git a/src/bin/psql/startup.c b/src/bin/psql/startup.c
new file mode 100644
index 3c17eec..71829eb
*** a/src/bin/psql/startup.c
--- b/src/bin/psql/startup.c
*** process_psqlrc(char *argv0)
*** 594,613
static void
process_psqlrc_file(char *filename)
{
! char *psqlrc;
#if defined(WIN32) && (!defined(__MINGW32__))
#define R_OK 4
#endif
! psqlrc = pg_malloc(strlen(filename) + 1 + strlen(PG_VERSION) + 1);
! sprintf(psqlrc, "%s-%s", filename, PG_VERSION);
! if (access(psqlrc, R_OK) == 0)
! (void) process_file(psqlrc, false, false);
else if (access(filename, R_OK) == 0)
(void) process_file(filename, false, false);
! free(psqlrc);
}
--- 594,620
static void
process_psqlrc_file(char *filename)
{
! char *psqlrc_minor, *psqlrc_major;
#if defined(WIN32) && (!defined(__MINGW32__))
#define R_OK 4
#endif
! psqlrc_minor = pg_malloc(strlen(filename) + 1 + strlen(PG_VERSION) + 1);
! sprintf(psqlrc_minor, "%s-%s", filename, PG_VERSION);
! psqlrc_major = pg_malloc(strlen(filename) + 1 + strlen(PG_MAJORVERSION) + 1);
! sprintf(psqlrc_major, "%s-%s", filename, PG_MAJORVERSION);
! /* check for minor version first, then major, then no version */
! if (access(psqlrc_minor, R_OK) == 0)
! (void) process_file(psqlrc_minor, false, false);
! else if (access(psqlrc_major, R_OK) == 0)
! (void) process_file(psqlrc_major, false, false);
else if (access(filename, R_OK) == 0)
(void) process_file(filename, false, false);
!
! free(psqlrc_minor);
! free(psqlrc_major);
}
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] patch for new feature: Buffer Cache Hibernation
Tom Lane wrote: > =?ISO-8859-1?Q?C=E9dric_Villemain?= > writes: > > 2011/10/14 Bruce Momjian : > >> Should this be marked as TODO? > > > I suppose TODO items *are* wanted and so working on them should remove > > the pain to convince people here to accept the feature, aren't they ? > > There is plenty of stuff in the TODO list for which there is no > consensus. Uh, we should probably remove those then. Can you think of any? -- Bruce Momjian http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] LIMITing number of results in a VIEW with global variables
Hello,
I am writing an extension to easily execute queries with conditions
expressing constraints in fuzzy logics.
I wrote some C functions that get or set global variables in C.
The variables are MU (FLOAT : degree of a fuzzy predicate), ALPHA
(FLOAT : threshold for filtering predicates) and K (INTEGER : limits
the number of results).
Here is an example for the variable ALPHA :
/*--- sqlf.c ---*/
static float8 ALPHA;
Datum get_alpha(PG_FUNCTION_ARGS);
Datum get_alpha(PG_FUNCTION_ARGS){
PG_RETURN_FLOAT8(ALPHA);
}
Datum set_alpha(PG_FUNCTION_ARGS);
Datum set_alpha(PG_FUNCTION_ARGS){
ALPHA = PG_GETARG_FLOAT8(0);
PG_RETURN_FLOAT8(ALPHA);
}
/*--- sqlf.sql ---*/
CREATE OR REPLACE FUNCTION set_alpha(alpha FLOAT) RETURNS FLOAT
AS '$libdir/sqlf', 'set_alpha'
LANGUAGE C STRICT;
These variables are parameters for filtering and sorting results.
The following cast operations are using MU and ALPHA.
CREATE OR REPLACE FUNCTION fuzzy2bool(FLOAT)
RETURNS BOOLEAN LANGUAGE SQL AS 'SELECT set_mu($1);SELECT $1 > get_alpha()';
CREATE CAST (FLOAT AS BOOLEAN)
WITH FUNCTION fuzzy2bool(FLOAT) AS IMPLICIT;
With this implicit cast, the query
SELECT age, young(age) FROM set_alpha(0.1), employees WHERE young(age);
is equivalent to
SELECT age, young(age) FROM set_alpha(0.1), employees WHERE
fuzzy2bool(young(age));
Here, young(age) is a fuzzy predicate returning a float value in [0,1].
The queries keep results satisfying young(age) > alpha :
age young(age)
161
240.6
260.4
210.9
260.4
I can sort the results in the view 'sorted_employees' according to
value MU of a fuzzy predicate thanks to fuzzy2bool cast function.
CREATE OR REPLACE VIEW sorted_employees AS
SELECT *, get_mu() as mu
FROM employees
ORDER BY mu DESC;
The query
SELECT age, mu FROM set_alpha(0.1), sorted_employees WHERE young(age);
gives the following results :
age mu
161
210.89976158142
240.60023841858
260.40005960464
260.40005960464
I am now trying to limit the number of results in the view according
to the global value K :
CREATE OR REPLACE VIEW filtered_employees AS
SELECT *, get_mu() as mu
FROM employees
ORDER BY mu DESC
LIMIT K;
The following query
SELECT age, mu FROM set_k(5), set_alpha(0.1), filtered_employees
WHERE young(age);
gives the results :
age mu
241
161
instead of :
age mu
161
210.89976158142
240.60023841858
260.40005960464
260.40005960464
It seems that the 'LIMIT K' instruction have side effects on the MU value.
Why is it not working ? How to fix this issue ?
Thanks by advance,
Thomas Girault
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] patch for new feature: Buffer Cache Hibernation
=?ISO-8859-1?Q?C=E9dric_Villemain?= writes: > 2011/10/14 Bruce Momjian : >> Should this be marked as TODO? > I suppose TODO items *are* wanted and so working on them should remove > the pain to convince people here to accept the feature, aren't they ? There is plenty of stuff in the TODO list for which there is no consensus. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] about EDITOR_LINENUMBER_SWITCH
On Thu, Oct 13, 2011 at 11:31 AM, Bruce Momjian wrote: > Alvaro Herrera wrote: >> Excerpts from Tom Lane's message of mié may 25 16:07:55 -0400 2011: >> > Alvaro Herrera writes: >> > > Excerpts from Tom Lane's message of mar may 24 17:11:17 -0400 2011: >> > >> Right. It would also increase the cognitive load on the user to have >> > >> to remember the command-line go-to-line-number switch for his editor. >> > >> So I don't particularly want to redesign this feature. However, I can >> > >> see the possible value of letting EDITOR_LINENUMBER_SWITCH be set from >> > >> the same place that you set EDITOR, which would suggest that we allow >> > >> the value to come from an environment variable. I'm not sure whether >> > >> there is merit in allowing both that source and ~/.psqlrc, though >> > >> possibly for Windows users it might be easier if ~/.psqlrc worked. >> > >> > > If we're going to increase the number of options in .psqlrc that do not >> > > work with older psql versions, can I please have .psqlrc-MAJORVERSION or >> > > some such? Having 8.3's psql complain all the time because it doesn't >> > > understand "linestyle" is annoying. >> > >> > 1. I thought we already did have that. >> >> Oh, true, we have that, though it's not very usable because you have to >> rename the file from .psqlrc-9.0.3 to .psqlrc-9.0.4 when you upgrade, >> which is kinda silly. > > True. We don't add configuration changes in minor versions so having > minor-version granularity makes no sense. > > The attached patch changes this to use the _major_ version number for > psql rc files. Does this have to be backward-compatible? Should I > check for minor and major matches? That is going to be confusing to > document. Checking for a minor match and then a major match seems sensible. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] WALInsertLock tuning
On Thu, Oct 13, 2011 at 9:35 PM, Bruce Momjian wrote: > > I assume this was addressed with this commit: > > commit 465883b0a2b4236ba6b31b648a9eabef3b7cdddb > Author: Simon Riggs > Date: Tue Jun 28 22:58:17 2011 +0100 > > Introduce compact WAL record for the common case of commit > (non-DDL). > XLOG_XACT_COMMIT_COMPACT leaves out invalidation messages and > relfilenodes, > saving considerable space for the vast majority of transaction > commits. > XLOG_XACT_COMMIT keeps same definition as XLOG_PAGE_MAGIC 0xD067 > and earlier. > > Leonardo Francalanci and Simon Riggs No, that's completely unrelated. I think it just never quite made it to prime time - it was analyzed theoretically but some of the testing discussed on the thread doesn't seem to have been done. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Online base backup from the hot-standby
2011/10/13 Jun Ishiduka :
> I updated to patch corresponded above-comments.
Thanks for updating the patch!
As I suggested in the reply to Simon, I think that the change of FPW
should be WAL-logged separately from that of HS parameters. ISTM
packing them in one WAL record makes XLogReportParameters()
quite confusing. Thought?
if (!shutdown && XLogStandbyInfoActive())
+ {
LogStandbySnapshot(&checkPoint.oldestActiveXid,
&checkPoint.nextXid);
+ XLogReportParameters(REPORT_ON_BACKEND);
+ }
Why doesn't the change of FPW need to be WAL-logged when
shutdown checkpoint is performed? It's helpful to add the comment
explaining why.
Regards,
--
Fujii Masao
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] loss of transactions in streaming replication
On Thu, Oct 13, 2011 at 10:08 AM, Fujii Masao wrote:
> On Wed, Oct 12, 2011 at 10:29 PM, Robert Haas wrote:
>> On Wed, Oct 12, 2011 at 5:45 AM, Fujii Masao wrote:
>>> In 9.2dev and 9.1, when walreceiver detects an error while sending data to
>>> WAL stream, it always emits ERROR even if there are data available in the
>>> receive buffer. This might lead to loss of transactions because such
>>> remaining data are not received by walreceiver :(
>>
>> Won't it just reconnect?
>
> Yes if the master is running normally. OTOH, if the master is not running
> (i.e.,
> failover case), the standby cannot receive again the data which it failed to
> receive.
>
> I found this issue when I shut down the master. When the master shuts down,
> it sends the shutdown checkpoint record, but I found that the standby failed
> to receive it.
Patch attached.
The patch changes walreceiver so that it doesn't emit ERROR just yet even
if it fails to send data to WAL stream. Then, after all available data have been
received and flushed to the disk, it emits ERROR.
If the patch is OK, it should be backported to v9.1.
Regards,
--
Fujii Masao
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center
*** a/src/backend/replication/libpqwalreceiver/libpqwalreceiver.c
--- b/src/backend/replication/libpqwalreceiver/libpqwalreceiver.c
***
*** 49,55 static char *recvBuf = NULL;
static bool libpqrcv_connect(char *conninfo, XLogRecPtr startpoint);
static bool libpqrcv_receive(int timeout, unsigned char *type,
char **buffer, int *len);
! static void libpqrcv_send(const char *buffer, int nbytes);
static void libpqrcv_disconnect(void);
/* Prototypes for private functions */
--- 49,55
static bool libpqrcv_connect(char *conninfo, XLogRecPtr startpoint);
static bool libpqrcv_receive(int timeout, unsigned char *type,
char **buffer, int *len);
! static bool libpqrcv_send(const char *buffer, int nbytes);
static void libpqrcv_disconnect(void);
/* Prototypes for private functions */
***
*** 404,417 libpqrcv_receive(int timeout, unsigned char *type, char **buffer, int *len)
/*
* Send a message to XLOG stream.
*
! * ereports on error.
*/
! static void
libpqrcv_send(const char *buffer, int nbytes)
{
if (PQputCopyData(streamConn, buffer, nbytes) <= 0 ||
PQflush(streamConn))
! ereport(ERROR,
! (errmsg("could not send data to WAL stream: %s",
! PQerrorMessage(streamConn;
}
--- 404,428
/*
* Send a message to XLOG stream.
*
! * Return true if successfully, false otherwise.
*/
! static bool
libpqrcv_send(const char *buffer, int nbytes)
{
+ /*
+ * Even if we could not send data to WAL stream, we don't emit ERROR
+ * just yet. There might be still data available in the receive buffer. We
+ * emit ERROR after all available data have been received and flushed to
+ * disk. Otherwise, such outstanding data would be lost.
+ *
+ * XXX: Should the result of PQerrorMessage() be returned so that the
+ * caller can report it? This doesn't seem worthwhile because in most cases,
+ * before reporting that, we will get another error and emit ERROR while
+ * trying to process all available data.
+ */
if (PQputCopyData(streamConn, buffer, nbytes) <= 0 ||
PQflush(streamConn))
! return false;
!
! return true;
}
*** a/src/backend/replication/walreceiver.c
--- b/src/backend/replication/walreceiver.c
***
*** 96,101 static struct
--- 96,104
static StandbyReplyMessage reply_message;
static StandbyHSFeedbackMessage feedback_message;
+ /* Did previous attempt to send data to WAL stream fail? */
+ static bool walrcv_send_error = false;
+
/*
* About SIGTERM handling:
*
***
*** 328,333 WalReceiverMain(void)
--- 331,345
else
{
/*
+ * If we didn't receive anything new after we had failed to send data
+ * to WAL stream, we can guarantee that there is no data available in
+ * the receive buffer. So we at last emit ERROR.
+ */
+ if (walrcv_send_error)
+ ereport(ERROR,
+ (errmsg("could not send data to WAL stream")));
+
+ /*
* We didn't receive anything new, but send a status update to the
* master anyway, to report any progress in applying WAL.
*/
***
*** 627,632 XLogWalRcvSendReply(void)
--- 639,651
wal_receiver_status_interval * 1000))
return;
+ /*
+ * If previous attempt to send data to WAL stream has failed, there is
+ * nothing to do here because this attempt would fail again.
+ */
+ if (walrcv_send_error)
+ return;
+
/* Construct a new message */
reply_message.write = LogstreamResult.Write;
reply_message.flush = LogstreamResult.Flush;
***
*** 641,647 XLogWalRcvSendReply(void)
/* Prepend with the message type and send it. */
buf[0] = 'r';
memcpy(&buf[1], &reply_message, sizeof(StandbyReplyMes
Re: [HACKERS] patch for new feature: Buffer Cache Hibernation
On 14.10.2011 11:44, Cédric Villemain wrote: 2011/10/14 Bruce Momjian: Should this be marked as TODO? I suppose TODO items *are* wanted and so working on them should remove the pain to convince people here to accept the feature, aren't they ? I don't think this is worthwhile to have in the backend. Someone could write it as an extension on pgfoundry, but I don't think that belongs on the TODO. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[GENERAL][HACKERS] register creation date of table
Hi, We have several users working on a 8.4 database, using it as a back-end for several related apps and transfering data to and from it. The database tends to get a bit messy, so i've made a little table to provide an overview. This table is truncated and refilled daily, it shows all tables and views in the database and : * the owner * number of records (estimation) * it's size on disk * the description There's a view on the table that shows the size as pg_size_pretty When you edit the description in the table (or the view, but no support in pgAdmin), the comment in the system tables is updated also. I attatched my code, hope some people find it handy, sorry for the names and comments being in dutch. Now, i would like to improve this thing and add a creation date for the table. I have some questions about that. 1. I think that there is no such information in the system tables. is that correct? I am planning to change the mechanism, so that the table is not truncated, but new tables are inserted in the overview and dropped tables are deleted from it. I need to do that in 2 steps (delete and insert). Then i can add a creation-date column which i will fill with 'today'. 2. i would like to go back in time. I think that i will just look up the creation date for the files in the data directory and translate their oid's to the object names and then update their dates. This would of course only work from the last restore. Is that a good way to do it? Thanks, WBL -- "Patriotism is the conviction that your country is superior to all others because you were born in it." -- George Bernard Shaw CREATE TABLE alg.tabellenenviews ( sorteren serial NOT NULL, relid integer, "type" text, "schema" name, tabelnaam name, eigenaar name, records_schatting bigint, grootte_bytes bigint, omschrijving text, CONSTRAINT tabellenenviews_pkey PRIMARY KEY (sorteren) ) WITH ( OIDS=FALSE ); GRANT SELECT ON TABLE alg.tabellenenviews TO public; GRANT UPDATE ON TABLE alg.tabellenenviews TO admins; COMMENT ON TABLE alg.tabellenenviews IS 'Overzicht van alle tabellen en views in de database. (meer commentaar na harde return..) Pas kolom "omschrijving" aan om commentaar van view of tabel ook aan te passen. Deze tabel wordt elke nacht automatisch opnieuw gegenereerd. Laatst geupdate op:2011-10-14 05:00:03.238905'; CREATE OR REPLACE FUNCTION alg.setcomment(p_type text, p_schema name, p_rel name, p_omschrijving text) RETURNS void AS $BODY$ DECLARE --t text; BEGIN --t:='COMMENT ON '||replace($1, 'tabel','TABLE')||' '||quote_ident($2)||'.'||quote_ident($3)||' IS '||quote_nullable($4); --raise notice 'commando is: %', t; EXECUTE 'COMMENT ON '||replace($1, 'tabel','TABLE')||' '||quote_ident($2)||'.'||quote_ident($3)||' IS '||quote_nullable($4); END $BODY$ LANGUAGE plpgsql VOLATILE COST 100; ALTER FUNCTION alg.setcomment(text, name, name, text) SET search_path=alg; COMMENT ON FUNCTION alg.setcomment(text, name, name, text) IS 'wordt gebruikt door de triggerfunctie alg.tabellenviews_setcomment_triggerfunctie() om het commentaar op tabellen te veranderen als dat wordt aangepast in de tabel alg.tabellenenviews'; CREATE OR REPLACE FUNCTION alg.tabellenenviews_setcomment_triggerfunction() RETURNS trigger AS $BODY$ BEGIN PERFORM alg.setcomment(OLD.type, OLD.schema, OLD.tabelnaam, NEW.omschrijving); NEW.sorteren:=OLD.sorteren; NEW.relid:=OLD.relid; NEW."type":=OLD."type"; NEW."schema":=OLD."schema"; NEW.tabelnaam:=OLD.tabelnaam; NEW.eigenaar:=OLD.eigenaar; NEW.records_schatting:=OLD.records_schatting; NEW.grootte_bytes:=OLD.grootte_bytes; --NEW.omschrijving:=OLD.omschrijving; --deze houdt dus de nieuwe waarde. RETURN NEW; END $BODY$ LANGUAGE plpgsql VOLATILE COST 100; COMMENT ON FUNCTION alg.tabellenenviews_setcomment_triggerfunction() IS 'past het commentaar van een tabel of view aan in de postgres systeemtabellen als dat wordt aangepast in de tabel alg.tabellenviews'; CREATE TRIGGER setcomment_trigger BEFORE UPDATE ON alg.tabellenenviews FOR EACH ROW EXECUTE PROCEDURE alg.tabellenenviews_setcomment_triggerfunction(); CREATE OR REPLACE VIEW alg.tabellenenviews_mooi AS SELECT tabellenenviews.type, tabellenenviews.schema, tabellenenviews.tabelnaam, tabellenenviews.eigenaar, tabellenenviews.records_schatting, pg_size_pretty(tabellenenviews.grootte_bytes) AS grootte_mooi, tabellenenviews.omschrijving FROM alg.tabellenenviews; GRANT SELECT ON TABLE alg.tabellenenviews_mooi TO public; COMMENT ON VIEW alg.tabellenenviews_mooi IS 'Overzicht van alle tabellen en views in de database, met tekstuele weergave van de bestandsgrootte van de tabellen'; /* --this is no use without support for editable views in pgAdmin CREATE RULE _UPDATE AS ON UPDATE TO alg.tabellenenviews_mooi DO INSTEAD UPDATE alg.tabellenenviews SET omschrijving = NEW.omschrijving WHERE schema = OLD.schema AND tabelnaam = OLD.tabelnaam; GRANT UPDATE ON TABLE alg.tabellenenviews_mooi TO public; */ --wordt afgevuurd va
Re: [HACKERS] patch for new feature: Buffer Cache Hibernation
2011/10/14 Bruce Momjian :
>
> Should this be marked as TODO?
I suppose TODO items *are* wanted and so working on them should remove
the pain to convince people here to accept the feature, aren't they ?
>
> ---
>
> Mitsuru IWASAKI wrote:
>> Hi,
>>
>> > On 05/07/2011 03:32 AM, Mitsuru IWASAKI wrote:
>> > > For 1, I've just finish my work. The latest patch is available at:
>> > > http://people.freebsd.org/~iwasaki/postgres/buffer-cache-hibernation-postgresql-20110507.patch
>> > >
>> >
>> > Reminder here--we can't accept code based on it being published to a web
>> > page. You'll need to e-mail it to the pgsql-hackers mailing list to be
>> > considered for the next PostgreSQL CommitFest, which is starting in a
>> > few weeks. Code submitted to the mailing list is considered a release
>> > of it to the project under the PostgreSQL license, which we can't just
>> > assume for things when given only a URL to them.
>>
>> Sorry about that, but I had enough time to revise my patches this week-end.
>> I attached the patches in this mail, and will update CommitFest page soon.
>>
>> > Also, you suggested you were out of time to work on this. If that's the
>> > case, we'd like to know that so we don't keep cc'ing you about things in
>> > expectation of an answer. Someone else may pick this up as a project to
>> > continue working on. But it's going to need a fair amount of revision
>> > before it matches what people want here, and I'm not sure how much of
>> > what you've written is going to end up in any commit that may happen
>> > from this idea.
>>
>> It seems that I don't have enough time to complete this work.
>> You don't need to keep cc'ing me, and I'm very happy if postgres to be
>> the first DBMS which support buffer cache hibernation feature.
>>
>> Thanks!
>>
>>
>> diff --git src/backend/access/transam/xlog.c
>> src/backend/access/transam/xlog.c
>> index b0e4c41..7a3a207 100644
>> --- src/backend/access/transam/xlog.c
>> +++ src/backend/access/transam/xlog.c
>> @@ -4834,6 +4834,19 @@ ReadControlFile(void)
>> #endif
>> }
>>
>> +bool
>> +GetControlFile(ControlFileData *controlFile)
>> +{
>> + if (ControlFile == NULL)
>> + {
>> + return false;
>> + }
>> +
>> + memcpy(controlFile, ControlFile, sizeof(ControlFileData));
>> +
>> + return true;
>> +}
>> +
>> void
>> UpdateControlFile(void)
>> {
>> diff --git src/backend/bootstrap/bootstrap.c
>> src/backend/bootstrap/bootstrap.c
>> index fc093cc..7ecf6bb 100644
>> --- src/backend/bootstrap/bootstrap.c
>> +++ src/backend/bootstrap/bootstrap.c
>> @@ -360,6 +360,15 @@ AuxiliaryProcessMain(int argc, char *argv[])
>> BaseInit();
>>
>> /*
>> + * Only StartupProcess can call ResumeBufferCacheHibernation() after
>> + * InitFileAccess() and smgrinit().
>> + */
>> + if (auxType == StartupProcess && BufferCacheHibernationLevel > 0)
>> + {
>> + ResumeBufferCacheHibernation();
>> + }
>> +
>> + /*
>> * When we are an auxiliary process, we aren't going to do the full
>> * InitPostgres pushups, but there are a couple of things that need to
>> get
>> * lit up even in an auxiliary process.
>> diff --git src/backend/storage/buffer/buf_init.c
>> src/backend/storage/buffer/buf_init.c
>> index dadb49d..52eb51a 100644
>> --- src/backend/storage/buffer/buf_init.c
>> +++ src/backend/storage/buffer/buf_init.c
>> @@ -127,6 +127,14 @@ InitBufferPool(void)
>>
>> /* Init other shared buffer-management stuff */
>> StrategyInitialize(!foundDescs);
>> +
>> + if (BufferCacheHibernationLevel > 0)
>> + {
>> +
>> ResisterBufferCacheHibernation(BUFFER_CACHE_HIBERNATION_TYPE_DESCRIPTORS,
>> + (char *)BufferDescriptors, sizeof(BufferDesc),
>> NBuffers);
>> +
>> ResisterBufferCacheHibernation(BUFFER_CACHE_HIBERNATION_TYPE_BLOCKS,
>> + (char *)BufferBlocks, BLCKSZ, NBuffers);
>> + }
>> }
>>
>> /*
>> diff --git src/backend/storage/buffer/bufmgr.c
>> src/backend/storage/buffer/bufmgr.c
>> index f96685d..dba8ebf 100644
>> --- src/backend/storage/buffer/bufmgr.c
>> +++ src/backend/storage/buffer/bufmgr.c
>> @@ -31,6 +31,7 @@
>> #include "postgres.h"
>>
>> #include
>> +#include
>> #include
>>
>> #include "catalog/catalog.h"
>> @@ -61,6 +62,13 @@
>> #define BUF_WRITTEN 0x01
>> #define BUF_REUSABLE 0x02
>>
>> +/*
>> + * Buffer Cache Hibernation stuff.
>> + */
>> +/* enable this to debug buffer cache hibernation. */
>> +#if 0
>> +#define DEBUG_BUFFER_CACHE_HIBERNATION
>> +#endif
>>
>> /* GUC variables */
>> bool zero_damaged_pages = false;
>> @@ -765,6 +773,16 @@ BufferAlloc(SMgrRelation smgr, char relpersistence,
>> ForkNumber forkNum,
>> }
>> }
>>
>> +#ifdef DEBUG_BUFFER_CACHE_HIBERNATION
>> + elog
