[HACKERS] Re: [COMMITTERS] pgsql: Remove fmgr.h include in cube contrib --- caused crash on a Ge
On Sun, 4 Sep 2011, Tom Lane wrote: Jeremy Drake jere...@jdrake.com writes: I didn't see any changes that looked like they affected CurrentMemoryContext, but I attached the compressed context diff in case you want to look at it. Right now I have a feeling that this is a compiler bug. That's my feeling, also. Don't know whether you have the interest/energy to try to reduce it to a reportable test case. If you mean reporting it to the compiler vendor (Intel), I doubt that would be worthwhile. The version of the compiler on this machine is very out of date. It is version 9.0 20060222. I would bet that if I did track down and report an issue in a 5-year-old compiler version, their first question would be, does the issue duplicate in the current version. Given that my other buildfarm member is running 11.1 20100414 (albeit for the x64 platform instead of the x86) and had no issue, I would expect that the current x86 version would also have no problem. I have intentionally been keeping the compiler versions on my buildfarm members pretty much fixed, for the benefit of reproducable results. However, I would be interested in hearing any guidelines on how old is too old for buildfarm member versions. -- 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] Re: [COMMITTERS] pgsql: Remove fmgr.h include in cube contrib --- caused crash on a Ge
Jeremy Drake pg...@jdrake.com writes: On Sun, 4 Sep 2011, Tom Lane wrote: Right now I have a feeling that this is a compiler bug. That's my feeling, also. Don't know whether you have the interest/energy to try to reduce it to a reportable test case. If you mean reporting it to the compiler vendor (Intel), I doubt that would be worthwhile. The version of the compiler on this machine is very out of date. It is version 9.0 20060222. I would bet that if I did track down and report an issue in a 5-year-old compiler version, their first question would be, does the issue duplicate in the current version. Given that my other buildfarm member is running 11.1 20100414 (albeit for the x64 platform instead of the x86) and had no issue, I would expect that the current x86 version would also have no problem. I have intentionally been keeping the compiler versions on my buildfarm members pretty much fixed, for the benefit of reproducable results. However, I would be interested in hearing any guidelines on how old is too old for buildfarm member versions. Well, I'm still running this on one development machine: $ gcc -v Reading specs from /usr/local/lib/gcc-lib/hppa2.0-hp-hpux10.20/2.95.3/specs gcc version 2.95.3 20010315 (release) so I'm not in the it's too old camp. My perspective on tool bugs is whether there is something (a) well defined and (b) not costly that we can do to work around it. So far, this issue is failing test (a) ... we know that one header inclusion order triggers the crash and another not, but there is no theory as to why. What I would suggest is to see whether a more recent x86 version shows the problem or not. If not, let's just write it off as an already-fixed compiler bug. 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] regress test failed
Hello there is one regress test, that failed on my f14 [pavel@nemesis postgresql]$ uname -a Linux nemesis 2.6.35.14-95.fc14.i686.PAE #1 SMP Tue Aug 16 21:12:22 UTC 2011 i686 i686 i386 GNU/Linux regression.diffs 3676/3676 100% *** /home/pavel/src/postgresql/src/test/regress/expected/foreign_data.out 2011-09-04 14:17:23.463589991 +0200 --- /home/pavel/src/postgresql/src/test/regress/results/foreign_data.out 2011-09-04 14:20:06.649144130 +0200 *** *** 781,791 SELECT * FROM information_schema.foreign_servers ORDER BY 1, 2; foreign_server_catalog | foreign_server_name | foreign_data_wrapper_catalog | foreign_data_wrapper_name | foreign_server_type | foreign_server_version | authorization_identifier +-+--+---+-++-- regression | s4 | regression | foo | oracle | | foreign_data_user regression | s5 | regression | foo | | 15.0 | regress_test_role regression | s6 | regression | foo | | 16.0 | regress_test_indirect regression | s8 | regression | postgresql| | | foreign_data_user - regression | sc | regression | dummy | | | foreign_data_user regression | t1 | regression | foo | | | regress_test_indirect regression | t2 | regression | foo | | | regress_test_role (7 rows) --- 781,791 SELECT * FROM information_schema.foreign_servers ORDER BY 1, 2; foreign_server_catalog | foreign_server_name | foreign_data_wrapper_catalog | foreign_data_wrapper_name | foreign_server_type | foreign_server_version | authorization_identifier +-+--+---+-++-- + regression | sc | regression | dummy | | | foreign_data_user regression | s4 | regression | foo | oracle | | foreign_data_user regression | s5 | regression | foo | | 15.0 | regress_test_role regression | s6 | regression | foo | | 16.0 | regress_test_indirect regression | s8 | regression | postgresql| | | foreign_data_user regression | t1 | regression | foo | | | regress_test_indirect regression | t2 | regression | foo | | | regress_test_role (7 rows) tested on HEAD regards Pavel Stehule -- 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] regress test failed
On 09/04/2011 08:23 AM, Pavel Stehule wrote: Hello there is one regress test, that failed on my f14 [pavel@nemesis postgresql]$ uname -a Linux nemesis 2.6.35.14-95.fc14.i686.PAE #1 SMP Tue Aug 16 21:12:22 UTC 2011 i686 i686 i386 GNU/Linux regression.diffs 3676/3676 100% *** /home/pavel/src/postgresql/src/test/regress/expected/foreign_data.out 2011-09-04 14:17:23.463589991 +0200 --- /home/pavel/src/postgresql/src/test/regress/results/foreign_data.out 2011-09-04 14:20:06.649144130 +0200 *** *** 781,791 SELECT * FROM information_schema.foreign_servers ORDER BY 1, 2; foreign_server_catalog | foreign_server_name | foreign_data_wrapper_catalog | foreign_data_wrapper_name | foreign_server_type | foreign_server_version | authorization_identifier +-+--+---+-++-- regression | s4 | regression | foo | oracle | | foreign_data_user regression | s5 | regression | foo | | 15.0 | regress_test_role regression | s6 | regression | foo | | 16.0 | regress_test_indirect regression | s8 | regression | postgresql| | | foreign_data_user - regression | sc | regression | dummy | | | foreign_data_user regression | t1 | regression | foo | | | regress_test_indirect regression | t2 | regression | foo | | | regress_test_role (7 rows) --- 781,791 SELECT * FROM information_schema.foreign_servers ORDER BY 1, 2; foreign_server_catalog | foreign_server_name | foreign_data_wrapper_catalog | foreign_data_wrapper_name | foreign_server_type | foreign_server_version | authorization_identifier +-+--+---+-++-- + regression | sc | regression | dummy | | | foreign_data_user regression | s4 | regression | foo | oracle | | foreign_data_user regression | s5 | regression | foo | | 15.0 | regress_test_role regression | s6 | regression | foo | | 16.0 | regress_test_indirect regression | s8 | regression | postgresql| | | foreign_data_user regression | t1 | regression | foo | | | regress_test_indirect regression | t2 | regression | foo | | | regress_test_role (7 rows) tested on HEAD In what locale does 'sc' sort before 's4'? (And I'd humbly suggest that whatever locale it is is possibly broken.) cheers andrew -- 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] regress test failed
On 09/04/2011 08:57 AM, Andrew Dunstan wrote: In what locale does 'sc' sort before 's4'? (And I'd humbly suggest that whatever locale it is is possibly broken.) EBCDIC? Joe -- 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] regress test failed
On sön, 2011-09-04 at 08:57 -0400, Andrew Dunstan wrote: In what locale does 'sc' sort before 's4'? In Czech. (And I'd humbly suggest that whatever locale it is is possibly broken.) There were some discussions about this in the past; it's apparently based on a national standard and completely valid. -- 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] regress test failed
On 09/04/2011 10:23 AM, Peter Eisentraut wrote: On sön, 2011-09-04 at 08:57 -0400, Andrew Dunstan wrote: In what locale does 'sc' sort before 's4'? In Czech. (And I'd humbly suggest that whatever locale it is is possibly broken.) There were some discussions about this in the past; it's apparently based on a national standard and completely valid. Well, I don't think we are obliged to cater for locales that break ASCII ordering. (There's a reason the buildfarm client runs its first set of checks in C locale). And to answer Pavel's question in email to me (maybe he meant to hit reply-all instead of reply): This order is based on czech locale, but regress tests should be independent on locale? No, it's not really possible. Something has to determine sort order. cheers andrew -- 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] regress test failed
On sön, 2011-09-04 at 10:47 -0400, Andrew Dunstan wrote: Well, I don't think we are obliged to cater for locales that break ASCII ordering. We do cater for that. See commit 8cd375526790c5be8ae24c77f13ac446adda88b6 Author: Peter Eisentraut pete...@gmx.net Date: Mon Mar 9 15:04:21 2009 + Tweak the regression test case so that the ordering of numbers vs. letters doesn't matter. This fixes failures in the Czech locale. commit 71936fc5eb6330ace4205163b3d2f731a7db5a41 Author: Peter Eisentraut pete...@gmx.net Date: Thu Feb 12 15:11:44 2009 + The Czech (cs_CZ) and Slovak (sk_SK) locales sort numbers after letters, instead of vice versa. Update the regression test expectations to support that. In the plpgsql test, adjust the test data so that this isn't an issue. In the char and varchar tests, add new expected files. and more generally commit 8987c115f1e7256b17c1cd04a8471810185d1941 Author: Peter Eisentraut pete...@gmx.net Date: Mon Jan 19 13:38:47 2009 + Alter regression test cases that rely on the sort order of aa. Some locales (da_DK, fo_FO, kl_GL, nb_NO, nn_NO in glibc) sort aa after z. commit 2b01cbe34042ed9fdea38a0c2fad5ab81df032c9 Author: Peter Eisentraut pete...@gmx.net Date: Mon Jan 19 12:02:29 2009 + Alter the regression test cases that rely on the sort order of ch between cg and ci. This eliminates a test failure on the following glibc locales: br_FR, cs_CZ, cy_GB, es_EC, es_US, hsb_DE, ig_NG, ik_CA, sk_SK. commit a5d67a0a050a9d32c351183992c3f08631735c37 Author: Peter Eisentraut pete...@gmx.net Date: Sun Jan 11 09:41:45 2009 + Make tests pass with or without locale. And to answer Pavel's question in email to me (maybe he meant to hit reply-all instead of reply): This order is based on czech locale, but regress tests should be independent on locale? No, it's not really possible. Something has to determine sort order. Well, if the going gets tough, we could stick a COLLATE C here and there. But as you can see above, this case should work. If something broke it, let's fix it. -- 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] regress test failed
Andrew Dunstan and...@dunslane.net writes: On 09/04/2011 10:23 AM, Peter Eisentraut wrote: On sön, 2011-09-04 at 08:57 -0400, Andrew Dunstan wrote: In what locale does 'sc' sort before 's4'? In Czech. Well, I don't think we are obliged to cater for locales that break ASCII ordering. We've changed regression test cases to avoid this type of problem in the past. I see no reason not to do it here. The object sc could just as easily be named something else. 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] regress test failed
Andrew Dunstan and...@dunslane.net writes: Well, I don't think we are obliged to cater for locales that break ASCII ordering. The logical conclusion of that position is that there's no need to make the regression tests pass in any other locale than C. Which is not the project policy, and we (including you, with your buildfarm hat on) have expended plenty of sweat in support of that. I think the real question that needs to be asked here is why there's not a buildfarm member running the tests in Czech locale. And maybe some of the other ones that have been problematic in the past. We should not have to wait for random reports to find out about this. 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] regress test failed
On 09/04/2011 11:31 AM, Tom Lane wrote: Andrew Dunstanand...@dunslane.net writes: Well, I don't think we are obliged to cater for locales that break ASCII ordering. The logical conclusion of that position is that there's no need to make the regression tests pass in any other locale than C. Which is not the project policy, and we (including you, with your buildfarm hat on) have expended plenty of sweat in support of that. I thought that was more about things like letters with diacritical marks, Turkish i and so on. But I stand corrected. I think the real question that needs to be asked here is why there's not a buildfarm member running the tests in Czech locale. And maybe some of the other ones that have been problematic in the past. We should not have to wait for random reports to find out about this. Maybe we need a few members that test a large number of locales. (Anyone feel like donating resources? I'm currently providing resources for seven, which I think is sufficient :-) ) Or a few volunteers from among existing members that can test lots of locales. (My f14 box has 245 utf8 locales, 181 non-utf8 locales and 308 that don't specify an encoding. Or I could make the client get a list of available locales/encodings and then test a number of them each run cyclically. cheers andrew -- 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] regress test failed
Andrew Dunstan and...@dunslane.net writes: On 09/04/2011 11:31 AM, Tom Lane wrote: I think the real question that needs to be asked here is why there's not a buildfarm member running the tests in Czech locale. And maybe some of the other ones that have been problematic in the past. We should not have to wait for random reports to find out about this. Maybe we need a few members that test a large number of locales. (Anyone feel like donating resources? I'm currently providing resources for seven, which I think is sufficient :-) ) Or a few volunteers from among existing members that can test lots of locales. (My f14 box has 245 utf8 locales, 181 non-utf8 locales and 308 that don't specify an encoding. I'm not ready to buy into the position that we should make the regression tests pass on every last locale definition that anyone can find. That's probably impossible, and certainly more trouble than it'd be worth. I suspect there's only a dozen or two that need to get tested. But I would suggest that this could be the policy: don't complain about regression tests failing in a locale unless you're prepared to support a buildfarm member that tests that locale on a routine basis. 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] WAL low watermark during base backup
On Fri, Sep 2, 2011 at 6:52 PM, Magnus Hagander mag...@hagander.net wrote: Attached patch implements a low watermark wal location in the walsender shmem array. Setting this value in a walsender prevents transaction log removal prior to this point - similar to how wal_keep_segments work, except with an absolute number rather than relative. For now, this is set when running a base backup with WAL included - to prevent the required WAL to be recycled away while the backup is running, without having to guestimate the value for wal_keep_segments. (There could be other ways added to set it in the future, but that's the only one I've done for now) It obviously needs some documentation updates as well, but I wanted to get some comments on the way it's done before I work on those. I'm not yet fully available for a discussion on this, but not sure I like this. You don't have to guess the setting of wal_keep_segments, you calculate it exactly from the size of your WAL disk. No other calculation is easy or accurate. This patch implements fill disk until primary croaks behaviour which means you are making a wild and risky guess as to whether it will work. If it does not, you are hosed. -- Simon Riggs http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training 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] force_not_null option support for file_fdw
I tried to review this patch. It seems to me its implementation is reasonable and enough simple. All the works of this patch is pick-up force_not_null option from pg_attribute.attfdwoptions and transform its data structure into suitable form to the existing BeginCopyFrom(). So, I'd almost like to mark this patch Ready for Committer. Here are only two points I'd like to comment on. + tuple = SearchSysCache2(ATTNUM, + RelationGetRelid(rel), + Int16GetDatum(attnum)); + if (!HeapTupleIsValid(tuple)) + ereport(ERROR, + (errcode(ERRCODE_UNDEFINED_OBJECT), +errmsg(cache lookup failed for attribute %d of relation %u, + attnum, RelationGetRelid(rel; The tuple should be always found unless we have any bugs that makes mismatch between pg_class.relnatts and actual number of attributes. So, it is a case to use elog(), instead of ereport() with error code. One other point is diffset of regression test, when I run `make check -C contrib/file_fdw'. Do we have something changed corresponding to COPY TO/FROM statement since 8th-August to now? *** /home/kaigai/repo/sepgsql/contrib/file_fdw/expected/file_fdw.out 2011-09-04 20:36:23.670981921 +0200 --- /home/kaigai/repo/sepgsql/contrib/file_fdw/results/file_fdw.out 2011-09-04 20:36:51.202989681 +0200 *** *** 118,126 word1 | word2 ---+--- 123 | 123 ABC | ABC NULL | - abc | abc (4 rows) -- basic query tests --- 118,126 word1 | word2 ---+--- 123 | 123 + abc | abc ABC | ABC NULL | (4 rows) -- basic query tests == Thanks, 2011年8月8日9:14 Shigeru Hanada shigeru.han...@gmail.com: Hi, I propose to support force_not_null option for file_fdw too. In 9.1 development cycle, file_fdw had been implemented with exported COPY FROM routines, but only force_not_null option has not been supported yet. Originally, in COPY FROM, force_not_null is specified as a list of column which is not matched against null-string. Now per-column FDW option is available, so I implemented force_not_null options as boolean value for each column to avoid parsing column list in file_fdw. True means that the column is not matched against null-string, and it's equivalent to specify the column's name in force_not_null option of COPY FROM. Default value is false. The patch includes changes for code, document and regression tests. Any comments/questions are welcome. Regards, -- Shigeru Hanada -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers -- KaiGai Kohei kai...@kaigai.gr.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] limit in subquery causes poor selectivity estimation
I wrote: On a longer-term basis, I'm looking into what we could do with extracting stats from subqueries, but that doesn't seem like material for a backpatch. I have a draft patch that I've been playing with (attached). I've committed a heavily rewritten version of that patch. Git HEAD seems to do reasonably well on the test case you gave at the start of this thread. I'm not sure yet how well the logic will hold up in real-world queries as opposed to simplified test cases, but give it a try. 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] force_not_null option support for file_fdw
Thanks for the review.
(2011/09/05 3:55), Kohei KaiGai wrote:
I tried to review this patch.
It seems to me its implementation is reasonable and enough simple.
All the works of this patch is pick-up force_not_null option from
pg_attribute.attfdwoptions and transform its data structure into suitable
form to the existing BeginCopyFrom().
So, I'd almost like to mark this patch Ready for Committer.
Here are only two points I'd like to comment on.
+ tuple = SearchSysCache2(ATTNUM,
+ RelationGetRelid(rel),
+ Int16GetDatum(attnum));
+ if (!HeapTupleIsValid(tuple))
+ ereport(ERROR,
+ (errcode(ERRCODE_UNDEFINED_OBJECT),
+errmsg(cache lookup failed for attribute %d of
relation %u,
+ attnum, RelationGetRelid(rel;
The tuple should be always found unless we have any bugs that makes
mismatch between pg_class.relnatts and actual number of attributes.
So, it is a case to use elog(), instead of ereport() with error code.
Oh, I've missed that other similar errors use elog()...
Fixed.
One other point is diffset of regression test, when I run `make check
-C contrib/file_fdw'.
Do we have something changed corresponding to COPY TO/FROM statement
since 8th-August to now?
I don't know about such change, and src/backend/command/copy.c has not
been touched since Feb 23.
*** /home/kaigai/repo/sepgsql/contrib/file_fdw/expected/file_fdw.out
2011-09-04 20:36:23.670981921 +0200
--- /home/kaigai/repo/sepgsql/contrib/file_fdw/results/file_fdw.out
2011-09-04 20:36:51.202989681 +0200
***
*** 118,126
word1 | word2
---+---
123 | 123
ABC | ABC
NULL |
- abc | abc
(4 rows)
-- basic query tests
--- 118,126
word1 | word2
---+---
123 | 123
+ abc | abc
ABC | ABC
NULL |
(4 rows)
-- basic query tests
==
In my usual environment that test passed, but finally I've reproduced
the failure with setting $LC_COLLATE to es_ES.UTF-8. Do you have set
any $LC_COLLATE in your test environment?
Regards,
--
Shigeru Hanada
diff --git a/contrib/file_fdw/data/text.csv b/contrib/file_fdw/data/text.csv
index ...bd4c327 .
*** a/contrib/file_fdw/data/text.csv
--- b/contrib/file_fdw/data/text.csv
***
*** 0
--- 1,4
+ 123,123
+ abc,abc
+ NULL,NULL
+ ABC,ABC
diff --git a/contrib/file_fdw/file_fdw.c b/contrib/file_fdw/file_fdw.c
index 224e74f..548dcd2 100644
*** a/contrib/file_fdw/file_fdw.c
--- b/contrib/file_fdw/file_fdw.c
***
*** 23,30
--- 23,32
#include foreign/fdwapi.h
#include foreign/foreign.h
#include miscadmin.h
+ #include nodes/makefuncs.h
#include optimizer/cost.h
#include utils/rel.h
+ #include utils/syscache.h
PG_MODULE_MAGIC;
*** static struct FileFdwOption valid_option
*** 57,72
{escape, ForeignTableRelationId},
{null, ForeignTableRelationId},
{encoding, ForeignTableRelationId},
/*
* force_quote is not supported by file_fdw because it's for COPY TO.
*/
- /*
-* force_not_null is not supported by file_fdw. It would need a parser
-* for list of columns, not to mention a way to check the column list
-* against the table.
-*/
/* Sentinel */
{NULL, InvalidOid}
--- 59,70
{escape, ForeignTableRelationId},
{null, ForeignTableRelationId},
{encoding, ForeignTableRelationId},
+ {force_not_null, AttributeRelationId},/* specified as boolean
value */
/*
* force_quote is not supported by file_fdw because it's for COPY TO.
*/
/* Sentinel */
{NULL, InvalidOid}
*** static void fileGetOptions(Oid foreignta
*** 112,117
--- 110,116
static void estimate_costs(PlannerInfo *root, RelOptInfo *baserel,
const char *filename,
Cost *startup_cost, Cost *total_cost);
+ static List * get_force_not_null(Oid relid);
/*
*** file_fdw_validator(PG_FUNCTION_ARGS)
*** 145,150
--- 144,150
List *options_list = untransformRelOptions(PG_GETARG_DATUM(0));
Oid catalog = PG_GETARG_OID(1);
char *filename = NULL;
+ char *force_not_null = NULL;
List *other_options = NIL;
ListCell *cell;
*** file_fdw_validator(PG_FUNCTION_ARGS)
*** 198,204
buf.data)));
}
! /* Separate out filename, since ProcessCopyOptions won't allow
it */
if (strcmp(def-defname, filename) == 0)
{
if (filename)
