Re: [HACKERS] Visibility map, partial vacuums
Tom Lane wrote: * ISTM that the patch is designed on the plan that the PD_ALL_VISIBLE page header flag *must* be correct, but it's really okay if the backing map bit *isn't* correct --- in particular we don't trust the map bit when performing antiwraparound vacuums. This isn't well documented. Right. Will add comments. We can't use the map bit for antiwraparound vacuums, because the bit doesn't tell you when the tuples have been frozen. And we can't advance relfrozenxid if we've skipped any pages. I've been thinking that we could add one frozenxid field to each visibility map page, for the oldest xid on the heap pages covered by the visibility map page. That would allow more fine-grained anti-wraparound vacuums as well. * Also, I see that vacuum has a provision for clearing an incorrectly set PD_ALL_VISIBLE flag, but shouldn't it fix the map too? Yes, will fix. Although, as long as we don't trust the visibility map, no real damage would be done. * It would be good if the visibility map fork were never created until there is occasion to set a bit in it; this would for instance typically mean that temp tables would never have one. I think that visibilitymap.c doesn't get this quite right --- in particular vm_readbuf seems willing to create/extend the fork whether its extend argument is true or not, so it looks like an inquiry operation would cause the map fork to be created. It should be possible to act as though a nonexistent fork just means all zeroes. The visibility map won't be inquired unless you vacuum. This is a bit tricky. In vacuum, we only know whether we can set a bit or not, after we've acquired a cleanup lock on the page, and scanned all the tuples. While we're holding a cleanup lock, we don't want to do I/O, which could potentially block out other processes for a long time. So it's too late to extend the visibility map at that point. I agree that vm_readbuf should not create the fork if 'extend' is false, that's an oversight, but it won't change the actual behavior because visibilitymap_test calls it with 'extend' true. Because of the above. I will add comments about that, though, there's nothing describing that currently. * heap_insert's all_visible_cleared variable doesn't seem to get initialized --- didn't your compiler complain? Hmph, I must've been compiling with -O0. * You missed updating SizeOfHeapDelete and SizeOfHeapUpdate Thanks. -- 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
Re: [HACKERS] Minor race-condition problem during database startup
Tom Lane napsal(a):
What seems to have happened is that the bgwriter didn't get as far as
the first line of BackgroundWriterMain before the client backend tried
to issue a checkpoint request.
This is obviously a pretty minor issue, but it still seems worth fixing.
We could either try to make sure that BgWriterShmem-bgwriter_pid gets
set before the postmaster opens its doors for clients, or allow
RequestCheckpoint() to wait a little bit if needed for the bgwriter
to come ready. The latter seems like a more localized change.
I think, postmaster should wait until bgwriter is not up.
Another strange thing in RequestCheckpoint() is following code:
00926 else if (kill(BgWriterShmem-bgwriter_pid, SIGINT) != 0)
00927 {
00928 if (ntries = 20) /* max wait 2.0 sec */
00929 {
00930 elog((flags CHECKPOINT_WAIT) ? ERROR : LOG,
00931 could not signal for checkpoint: %m);
00932 break;
00933 }
00934 }
By my opinion there is not reason to retry kill call, because it fails only in
situation if process does not exist or caller does not have permission to send a
signal. If one of these situation happens it means that bgwriter is dead or
memory is corrupted. Maybe it is time for panic (or fatal)?
Zdenek
--
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] TODO item: adding VERBOSE option to CLUSTER [with patch]
Jim Cox wrote: A patch s/b attached which adds a VERBOSE option to the CLUSTER command as mentioned in the following TODO item for CLUSTER: Add VERBOSE option to report tables as they are processed, like VACUUM VERBOSE. I have committed this version of your patch, but moving the VERBOSE option directly after the CLUSTER word, to be more consistent with VACUUM and ANALYZE. Also I added the corresponding support to the clusterdb command. Additional processing information as discussed later in the thread can now be added easily, but I think there was no consensus on what exactly to print. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] huge query tree cost too much time to copyObject()
Hi, recently, I wrote a really complex SELECT statement which consists of about 20 relations using NATURAL JOIN method and every single relation consists 50 columns. It looks like: PREPARE ugly_stmt AS SELECT * FROM t1 NATURAL JOIN t2 NATURAL JOIN t3 ... NATURAL JOIN t20 WHERE id = $1; All tables share only one common column id which is also defined as primay key. I set join_collapse_limit to 1 and just write a prepare statement for calling multi-times. It seems Postgres cost lots of time to copyObject(). So can I just allocate a new context from TopMemoryContext before doing QueryRewrite() and pg_plan_queries(), and save them into hash table without copying query_list and plan_list again(i think they are lived in the context I created). I know I am subjected a long term memory leak until I deallocate the prepared statement, but it save lots of time in my situation. And I can bear with it... Thanks in advance -- 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: default values for function parameters
Pavel Stehule wrote: I have problem with sending patch, so I am send link http://www.pgsql.cz/patches/defaults.diff.gz Example: postgres=# create function fx(a int, b int default 30, c int default 40) Could you explain why you store the default expressions in a new posexpr type rather than in an array of text (compare pg_attrdef.adbin)? -- 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: default values for function parameters
2008/11/24 Peter Eisentraut [EMAIL PROTECTED]: Pavel Stehule wrote: I have problem with sending patch, so I am send link http://www.pgsql.cz/patches/defaults.diff.gz Example: postgres=# create function fx(a int, b int default 30, c int default 40) Could you explain why you store the default expressions in a new posexpr type rather than in an array of text (compare pg_attrdef.adbin)? I would to implement named params - and there expressions, that are used as default params, should not be continual. I don't store params as array of text because I would to eliminate repeated expression's parsing. So I use similar machanism used for rules or views. Pavel -- 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] Minor race-condition problem during database startup
Zdenek Kotala napsal(a):
Tom Lane napsal(a):
What seems to have happened is that the bgwriter didn't get as far as
the first line of BackgroundWriterMain before the client backend tried
to issue a checkpoint request.
This is obviously a pretty minor issue, but it still seems worth fixing.
We could either try to make sure that BgWriterShmem-bgwriter_pid gets
set before the postmaster opens its doors for clients, or allow
RequestCheckpoint() to wait a little bit if needed for the bgwriter
to come ready. The latter seems like a more localized change.
I think, postmaster should wait until bgwriter is not up.
Another strange thing in RequestCheckpoint() is following code:
00926 else if (kill(BgWriterShmem-bgwriter_pid, SIGINT) != 0)
00927 {
00928 if (ntries = 20) /* max wait 2.0 sec */
00929 {
00930 elog((flags CHECKPOINT_WAIT) ? ERROR : LOG,
00931 could not signal for checkpoint: %m);
00932 break;
00933 }
00934 }
By my opinion there is not reason to retry kill call, because it fails
only in situation if process does not exist or caller does not have
permission to send a signal. If one of these situation happens it means
that bgwriter is dead or memory is corrupted. Maybe it is time for panic
(or fatal)?
If I think more about it, that code is here because it probably tries to bypass
time when bgwriter is restarted. But It could invoke another race condition in
situation when old bgwriter pid is recycled to another backend (or another
postgresql server) in mean time. Should postmaster/bgwriter clean
BgWriterShmem-bgwriter_pid at the end? I think it should be zeroed in places
where BgWriterPID is set to zero (in postmaster.c).
Zdenek
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] [Review] Grouping Sets patch
Hi Stehule,
I have looked at the patch and it looks great. Here are my observation
Compilation
1 - Patch applied successfully on CVS-HEAD
2 - No compilation error found
Code
---
1 - Style of code is very close to the existing PG code.
2 - Comments look OK to me.
3 - I haven't looked deep into the code. As this is a WIP patch so I
gave my emphasis on testing and verifying the concept.
BTW I have not tested the cases you have mentioned. Here are the some
sample test cases (I haven't paste complete test cases I have used)
CREATE TABLE population_tbl (country varchar, male NUMERIC, female NUMERIC);
INSERT INTO population_tbl values ('Australia',1,100);
INSERT INTO population_tbl values ('Denmark',2,200);
INSERT INTO population_tbl values ('Germany',3,300);
INSERT INTO population_tbl values ('Netherlands',4,400);
INSERT INTO population_tbl values ('United States',5,500);
INSERT INTO population_tbl values ('Pakistan',6,600);
--GROUPING SET
SELECT country,male,female FROM population_tbl GROUP BY GROUPING
SETS(country,male,female,());
SELECT country,male,female FROM population_tbl GROUP BY GROUPING
SETS((country,male,female));
SELECT country,male,female,sum(male) FROM population_tbl GROUP BY
GROUPING SETS(country,(male,female));
SELECT country,male,female FROM population_tbl GROUP BY ALL GROUPING
SETS(country,male,female,());
SELECT country,male,female FROM population_tbl GROUP BY ALL GROUPING
SETS((country,male,female));
SELECT country,male,female,sum(male) FROM population_tbl GROUP BY ALL
GROUPING SETS(country,(male,female));
SELECT country,male,female FROM population_tbl GROUP BY DISTINCT
GROUPING SETS(country,male,female,());
SELECT country,male,female FROM population_tbl GROUP BY DISTINCT
GROUPING SETS((country,male,female));
SELECT country,male,female,sum(male) FROM population_tbl GROUP BY
DISTINCT GROUPING SETS(country,(male,female));
SELECT grouping(country),country,male,female FROM population_tbl GROUP
BY GROUPING SETS(country,male,female,());
SELECT grouping(country),country,male,female FROM population_tbl GROUP
BY GROUPING SETS((country,male,female));
SELECT grouping(country),country,male,female,sum(male) FROM
population_tbl GROUP BY GROUPING SETS(country,(male,female));
SELECT grouping(country),country,male,female FROM population_tbl GROUP
BY ALL GROUPING SETS(country,male,female,());
SELECT grouping(country),country,male,female FROM population_tbl GROUP
BY ALL GROUPING SETS((country,male,female));
SELECT grouping(country),country,male,female,sum(male) FROM
population_tbl GROUP BY ALL GROUPING SETS(country,(male,female));
SELECT grouping(country),country,male,female FROM population_tbl GROUP
BY DISTINCT GROUPING SETS(country,male,female,());
SELECT grouping(country),country,male,female FROM population_tbl GROUP
BY DISTINCT GROUPING SETS((country,male,female));
SELECT grouping(country),country,male,female,sum(male) FROM
population_tbl GROUP BY DISTINCT GROUPING SETS(country,(male,female));
SELECT grouping_id(country),country,male,female FROM population_tbl
GROUP BY GROUPING SETS(country,male,female,());
SELECT grouping_id(country),country,male,female FROM population_tbl
GROUP BY GROUPING SETS((country,male,female));
SELECT grouping_id(country),country,male,female,sum(male) FROM
population_tbl GROUP BY GROUPING SETS(country,(male,female));
SELECT grouping_id(country),country,male,female FROM population_tbl
GROUP BY ALL GROUPING SETS(country,male,female,());
SELECT grouping_id(country),country,male,female FROM population_tbl
GROUP BY ALL GROUPING SETS((country,male,female));
SELECT grouping_id(country),country,male,female,sum(male) FROM
population_tbl GROUP BY ALL GROUPING SETS(country,(male,female));
SELECT grouping_id(country),country,male,female FROM population_tbl
GROUP BY DISTINCT GROUPING SETS(country,male,female,());
SELECT grouping_id(country),country,male,female FROM population_tbl
GROUP BY DISTINCT GROUPING SETS((country,male,female));
SELECT grouping_id(country),country,male,female,sum(male) FROM
population_tbl GROUP BY DISTINCT GROUPING SETS(country,(male,female));
--Neg: SELECT country,male,female,sum(male) FROM population_tbl GROUP
BY GROUPING SETS((country),(male),female,());
--ROLLUP
SELECT country,male,female FROM population_tbl GROUP BY
ROLLUP(country,male,female);
SELECT country,male,female FROM population_tbl GROUP BY
ROLLUP(country,(male,female));
SELECT country,male,female FROM population_tbl GROUP BY
ROLLUP(country,(male),(female));
SELECT country,male,female FROM population_tbl GROUP BY
ROLLUP((country),(male),(female));
SELECT country,male,female FROM population_tbl GROUP BY
ROLLUP((country,male),(female));
SELECT country,male,female FROM population_tbl GROUP BY ALL
ROLLUP(country,male,female);
SELECT country,male,female FROM population_tbl GROUP BY ALL
ROLLUP(country,(male,female));
SELECT country,male,female FROM population_tbl GROUP BY ALL
ROLLUP(country,(male),(female));
SELECT country,male,female FROM population_tbl GROUP BY ALL
[HACKERS] blatantly a bug in the documentation
I hope this is the right place to report a bug/issue in the official
documentation...
In
http://www.postgresql.org/docs/current/interactive/plpgsql-control-structures.html
there is an example for a function, cs_refresh_mviews(), chapter 38.6.4.
Within this function there are a PERFORM cs_log('Refreshing
materialized views...');, but such a function cs_log() are not in the
core distibution.
I think, this is a problem for users which read the doc and try to
reproduce the example or write own code based on the example.
Regards, and many thanks for PostgreSQL to the developer, Andreas
--
Andreas Kretschmer
Kontakt: Heynitz: 035242/47150, D1: 0160/7141639 (mehr: - Header)
GnuPG-ID: 0x3FFF606C, privat 0x7F4584DA http://wwwkeys.de.pgp.net
--
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] [Review] Grouping Sets patch
Hello,
thank you, for you review. Grouping Sets is nice feature, but this
patch patch has some issues, and I don't expect commiting into 8.4. I
sent it, because it is interaction with window function's patch and
(maybe) some code should be shared - and this information should be
usefull for other developers (commiters). Actually almoust all of
planner part should be rewriten. Now GS is in separate branch than
classic GROUP BY clause. I am sure, so GROUP BY is special case of GS
- it means relativelly big changes in GROUP BY planner part. So now I
am wainting on commiting window functions - and then I'll continue.
Regards
Pavel Stehule
2008/11/24 Ibrar Ahmed [EMAIL PROTECTED]:
Hi Stehule,
I have looked at the patch and it looks great. Here are my observation
Compilation
1 - Patch applied successfully on CVS-HEAD
2 - No compilation error found
Code
---
1 - Style of code is very close to the existing PG code.
2 - Comments look OK to me.
3 - I haven't looked deep into the code. As this is a WIP patch so I
gave my emphasis on testing and verifying the concept.
BTW I have not tested the cases you have mentioned. Here are the some
sample test cases (I haven't paste complete test cases I have used)
CREATE TABLE population_tbl (country varchar, male NUMERIC, female NUMERIC);
INSERT INTO population_tbl values ('Australia',1,100);
INSERT INTO population_tbl values ('Denmark',2,200);
INSERT INTO population_tbl values ('Germany',3,300);
INSERT INTO population_tbl values ('Netherlands',4,400);
INSERT INTO population_tbl values ('United States',5,500);
INSERT INTO population_tbl values ('Pakistan',6,600);
--GROUPING SET
SELECT country,male,female FROM population_tbl GROUP BY GROUPING
SETS(country,male,female,());
SELECT country,male,female FROM population_tbl GROUP BY GROUPING
SETS((country,male,female));
SELECT country,male,female,sum(male) FROM population_tbl GROUP BY
GROUPING SETS(country,(male,female));
SELECT country,male,female FROM population_tbl GROUP BY ALL GROUPING
SETS(country,male,female,());
SELECT country,male,female FROM population_tbl GROUP BY ALL GROUPING
SETS((country,male,female));
SELECT country,male,female,sum(male) FROM population_tbl GROUP BY ALL
GROUPING SETS(country,(male,female));
SELECT country,male,female FROM population_tbl GROUP BY DISTINCT
GROUPING SETS(country,male,female,());
SELECT country,male,female FROM population_tbl GROUP BY DISTINCT
GROUPING SETS((country,male,female));
SELECT country,male,female,sum(male) FROM population_tbl GROUP BY
DISTINCT GROUPING SETS(country,(male,female));
SELECT grouping(country),country,male,female FROM population_tbl GROUP
BY GROUPING SETS(country,male,female,());
SELECT grouping(country),country,male,female FROM population_tbl GROUP
BY GROUPING SETS((country,male,female));
SELECT grouping(country),country,male,female,sum(male) FROM
population_tbl GROUP BY GROUPING SETS(country,(male,female));
SELECT grouping(country),country,male,female FROM population_tbl GROUP
BY ALL GROUPING SETS(country,male,female,());
SELECT grouping(country),country,male,female FROM population_tbl GROUP
BY ALL GROUPING SETS((country,male,female));
SELECT grouping(country),country,male,female,sum(male) FROM
population_tbl GROUP BY ALL GROUPING SETS(country,(male,female));
SELECT grouping(country),country,male,female FROM population_tbl GROUP
BY DISTINCT GROUPING SETS(country,male,female,());
SELECT grouping(country),country,male,female FROM population_tbl GROUP
BY DISTINCT GROUPING SETS((country,male,female));
SELECT grouping(country),country,male,female,sum(male) FROM
population_tbl GROUP BY DISTINCT GROUPING SETS(country,(male,female));
SELECT grouping_id(country),country,male,female FROM population_tbl
GROUP BY GROUPING SETS(country,male,female,());
SELECT grouping_id(country),country,male,female FROM population_tbl
GROUP BY GROUPING SETS((country,male,female));
SELECT grouping_id(country),country,male,female,sum(male) FROM
population_tbl GROUP BY GROUPING SETS(country,(male,female));
SELECT grouping_id(country),country,male,female FROM population_tbl
GROUP BY ALL GROUPING SETS(country,male,female,());
SELECT grouping_id(country),country,male,female FROM population_tbl
GROUP BY ALL GROUPING SETS((country,male,female));
SELECT grouping_id(country),country,male,female,sum(male) FROM
population_tbl GROUP BY ALL GROUPING SETS(country,(male,female));
SELECT grouping_id(country),country,male,female FROM population_tbl
GROUP BY DISTINCT GROUPING SETS(country,male,female,());
SELECT grouping_id(country),country,male,female FROM population_tbl
GROUP BY DISTINCT GROUPING SETS((country,male,female));
SELECT grouping_id(country),country,male,female,sum(male) FROM
population_tbl GROUP BY DISTINCT GROUPING SETS(country,(male,female));
--Neg: SELECT country,male,female,sum(male) FROM population_tbl GROUP
BY GROUPING SETS((country),(male),female,());
--ROLLUP
SELECT
Re: [HACKERS] Windowing Function Patch Review - Standard Conformance
Hitoshi Harada wrote: 2008/11/20 David Rowley [EMAIL PROTECTED]: I won't be around until Monday evening (Tuesday morning JST). I'll pickup the review again there. It's really time for me to push on with this review. The patch is getting closer to getting the thumbs up from me. I'm really hoping that can happen on Monday. Then it's over to Heikki for more code feedback. This time I only fixed some bugs and rebased against the HEAD, but did not refactored. I can figure out some part of what Heikki claimed but not all, so I'd like to see what he will send and post another patch if needed. Thanks! Here's what I've got this far I merged your latest patch into this, as well as latest changes from CVS HEAD. It's a bit of a mess, but here's the big changes I've done: - Removed all the iterator stuff. You can just call WinGetPart/FrameGetArg repeatedly. It ought to be just as fast if done right in nodeWindow.c. - Removed all the Shrinked/Extended stuff, as it's not needed until we have full support for window frames. - Removed all the WinRow* functions, you can just call WinFrame/PartGet* functions, using WINDOW_SEEK_CURRENT - Removed WinFrame/PartGetTuple functions. They were only used for determining if two tuples are peer with each other, so I added a WinRowIsPeer function instead. - Removed all the buffering strategy stuff. Currently, the whole partition is always materialized. That's of course not optimal; I'm still thinking we should just read the tuples from the outer node lazily, on-demand, instead. To avoid keeping all tuples in the partition in tuplestore, when not needed, should add an extra function to trim the tuplestore. There's now a lot less code in nodeWindow.c, and I'm starting to understand most of what-s left :-). TODO - clean up the comments and other mess. - Modify WinPart/FrameGetArg so that it's passed the parameter number instead of an Expr node. I'll continue working on the executor, but please let me know what you think. -- 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
Re: [HACKERS] Autoconf, libpq and replacement function
Tom Lane wrote: I'd suggest making the callers do something like #ifdef HAVE_FNMATCH #include fnmatch.h #else #include port/pg_fnmatch.h #endif The way Autoconf suggests to organize this is to provide a fake fnmatch.h (they call it fnmatch_.h) and link it to fnmatch.h if it is needed. This way, callers don't need to know the difference. -- 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] Autoconf, libpq and replacement function
Tom Lane wrote: Also, right now we have got src/include/getaddrinfo.h src/include/getopt_long.h which really make me itch now that I am contemplating the possibility that we try to use libc-supplied code for these functions along with our own header definitions. These particular header files look like they are robust enough to deal with this case. But overall this isn't a great idea. I think the reason we've avoided putting such stuff into include/port/ is that right now that directory consists exclusively of OS-specific files. Maybe we need another include subdirectory? I think the idea is rather that you don't need to change call sites to include the system header or our own. The symlink approach I mentioned earlier might work. -- 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] blatantly a bug in the documentation
On Mon, Nov 24, 2008 at 6:02 AM, A. Kretschmer
[EMAIL PROTECTED] wrote:
I hope this is the right place to report a bug/issue in the official
documentation...
In
http://www.postgresql.org/docs/current/interactive/plpgsql-control-structures.html
there is an example for a function, cs_refresh_mviews(), chapter 38.6.4.
Within this function there are a PERFORM cs_log('Refreshing
materialized views...');, but such a function cs_log() are not in the
core distibution.
The example in the docs is pseudo-code. None of the tables referenced
exist either...the purpose of the example was to demonstrate the
syntax of for loops.
merlin
--
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] Autoconf, libpq and replacement function
Peter Eisentraut wrote: Tom Lane wrote: I think the reason we've avoided putting such stuff into include/port/ is that right now that directory consists exclusively of OS-specific files. Maybe we need another include subdirectory? I think the idea is rather that you don't need to change call sites to include the system header or our own. The symlink approach I mentioned earlier might work. Anything that needs symlinks will need a set of workarounds on Windows, having us manually do a copy of the files. We already do this in a couple of cases, but relying more on it would make things even more kludgy I think... //Magnus -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Re: [COMMITTERS] pgsql: Explicitly bind gettext() to the UTF8 locale when in use.
Hiroshi Inoue wrote: Hi Magnus and all, Magnus Hagander wrote: Log Message: --- Explicitly bind gettext() to the UTF8 locale when in use. This is required on Windows due to the special locale handling for UTF8 that doesn't change the full environment. Thanks to this change UTF-8 case was solved but Japanese users are still unhappy with Windows databases with EUC_JP encoding. Shift_JIS which is a Japanese encoding under Windows doesn't match any server encoding and causes a crash with the use of gettext. So Saito-san removed ja message catalog just before the 8.3 release. Attached is a simple patch to avoid the crash and enable the use of Japanese message catalog. Please apply the patch if there's no problem. Hi! It will clearly also need an update to the comment, but I can take care of that. I assume you have tested this? The comment says that it works because we are handling UTF8 on a special way on Windows, but AFAIK we *don't* handle EUC_JP in a special way there? If your database is in EUC_JP, I don't see why gettext() isn't picking it up properly in the first place.. And why do we need that on Windows only, and not on other platforms? //Magnus -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Updates of SE-PostgreSQL 8.4devel patches (r1244)
I updated the patch set of SE-PostgreSQL (revision 1244). [1/6] http://sepgsql.googlecode.com/files/sepostgresql-sepgsql-8.4devel-3-r1244.patch [2/6] http://sepgsql.googlecode.com/files/sepostgresql-pg_dump-8.4devel-3-r1244.patch [3/6] http://sepgsql.googlecode.com/files/sepostgresql-policy-8.4devel-3-r1244.patch [4/6] http://sepgsql.googlecode.com/files/sepostgresql-docs-8.4devel-3-r1244.patch [5/6] http://sepgsql.googlecode.com/files/sepostgresql-tests-8.4devel-3-r1244.patch [6/6] http://sepgsql.googlecode.com/files/sepostgresql-row_acl-8.4devel-3-r1244.patch Draft of the SE-PostgreSQL documentation is here: http://wiki.postgresql.org/wiki/SEPostgreSQL This revision contains some fixes required by some persons. (Thanks for Simon, Bruce and Tom.) List of updates: - Rebase to the latest CVS HEAD. - The fixed length security field of HeapTupleHeader becomes optimal. It enables enhanced security mechanism to control its allocation on heap_form_tuple(), and to reduce unnecessary storage consumption. The TupleDesc structure got a new variable of tdhassecurity. When it is true, heap_form_tuple() allocates an additional field to store security identifier. The enhanced security mechanism can control value of the flag via a new hook: pgaceTupleDescHasSecurity(). - SE-PostgreSQL got a new GUC variable: sepostgresql_row_level. When it turned off, SE-PostgreSQL does not apply its row-level access controls, and does not assign per-tuple security context. - The following two hooks are removed: * pgaceIsAllowPlannerHook() * pgaceIsAllowExecutorRunHook() And, the following hook is added * pgaceGramRelationOption() This hook gives a chance to handle relation options. - The row-level acl got two new relation options: * row_level_acl=on|off When it is tuened off, the row-level access controls are not applied, and security field is not allocated. * default_row_acl='...' It enables to specify a default for newly inserted tuples. - pg_security system catalog is added to the regression test of sanity_check. - Code cleanups related to module installation checks. Thanks, -- KaiGai Kohei [EMAIL PROTECTED] -- 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] [PATCHES] Solve a problem of LC_TIME of windows.
The way I read this patch we do:
1) Get the time in CP_ACP
2) Convert to UTF16
3) Convert to UTF8
4) If necessary, convert to the charset in the database
We could be more efficient by at least folding 1 and 2 into a single
step, which means getting it in UTF16 from the beginning. How about
attached patch? It builds, but please verify that it fixes the problem.
(I've updated the comment as well)
Attached pg_locale_utf16.patch. I'm also attaching
pg_locale_diffdiff.patch which contains the changes I've made against
your patch only.
//Magnus
Hiroshi Saito wrote:
Hi.
All suggestion is appropriate and has been checked.
CVS-HEAD was examined by MinGW. $ make check NO_LOCALE=true
...
===
All 118 tests passed. ===
Then, It continues and a review is desired. Thanks!
Regatrds,
Hiroshi Saito
- Original Message - From: Hiroshi Saito
[EMAIL PROTECTED]
Hi ITAGAKI-san.
Sorry, very late reaction..
I lost time resources in an individual my machine trouble and
busyness.:-(
Now, I appreciate your exact work. ! Then, I desire the best patch for
PostgreSQL. Probably, I think that it is finally helpful in not a
problem of only Japan but many countries. Tom-san, and Alvaro-san,
Magnus-san understands the essence of this problem. Therefore, the
suggestion is expected for me.
Anyway, thank you very much.!!
Regards,
Hiroshi Saito
- Original Message - From: ITAGAKI Takahiro
[EMAIL PROTECTED]
Jaime Casanova [EMAIL PROTECTED] wrote:
i'm confused, original patch has this signature:
+ conv_strftime(char *src, size_t len, const char *format, const
struct tm *tm)
your's has:
+strftime_win32(char *dst, size_t dstlen, const char *format, const
you change all src for dst, just a variable name decision but a
radical one... why was that (i honestly doesn't understand this patch
very well ;)?
That's because the first argument is not an input buffer,
but an output buffer. MSDN also calls it 'strDest'.
http://msdn.microsoft.com/en-us/library/fe06s4ak(VS.71).aspx
Linux manpage calls it 's', but I think it means String, not Src.
http://man.cx/strftime
If you can review the patch, please use the attached one instead.
I modified it in response to the discussion of
pg_do_encoding_conversion.
BTW, I cannot understand the comment in the function head,
+ * result is obtained by locale setup of LC_TIME in the environment
+ * of windows at present CP_ACP. Therefore, conversion is needed
+ * for SERVER_ENCODING. SJIS which is not especially made to server
+ * encoding in Japan returns.
but it probably says:
strftime in Windows returns in CP_ACP encoding, but it could be
different from SERVER_ENCODING. Especially, Windows Japanese edition
requires conversions because it uses SJIS as CP_ACP, but we don't
support SJIS as a server encoding.
I hope you would review my English not only C ;-)
Regards,
---
ITAGAKI Takahiro
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
*** a/src/backend/utils/adt/pg_locale.c
--- b/src/backend/utils/adt/pg_locale.c
***
*** 54,59
--- 54,60
#include utils/memutils.h
#include utils/pg_locale.h
+ #include mb/pg_wchar.h
#define MAX_L10N_DATA 80
***
*** 452,457 PGLC_localeconv(void)
--- 453,507
return CurrentLocaleConv;
}
+ #ifdef WIN32
+ /*
+ * On win32, strftime() returns the encoding in CP_ACP, which is likely
+ * different from SERVER_ENCODING. This is especially important in Japanese
+ * versions of Windows which will use SJIS encoding, which we don't support
+ * as a server encoding.
+ *
+ * Replace strftime() with a version that gets the string in UTF16 and then
+ * converts it to the appropriate encoding as necessary.
+ */
+ static size_t
+ strftime_win32(char *dst, size_t dstlen, const char *format, const struct tm *tm)
+ {
+ size_t len;
+ wchar_t wbuf[MAX_L10N_DATA];
+ int encoding;
+
+ encoding = GetDatabaseEncoding();
+ if (encoding == PG_SQL_ASCII)
+ return len;
+
+ len = wcsftime(wbuf, sizeof(wbuf), format, tm);
+ if (len == 0)
+ /* strftime called failed - return 0 with the contents of dst unspecified */
+ return 0;
+
+ len = WideCharToMultiByte(CP_UTF8, 0, wbuf, len, dst, dstlen, NULL, NULL);
+ if (len == 0)
+ ereport(ERROR,
+ (errmsg(could not convert string to UTF-8:error %lu, GetLastError(;
+
+ dst[len] = '\0';
+ if (encoding != PG_UTF8)
+ {
+ char *convstr = pg_do_encoding_conversion(dst, len, PG_UTF8, encoding);
+ if (dst != convstr)
+ {
+ StrNCpy(dst, convstr, dstlen);
+ len = strlen(dst);
+ }
+ }
+
+ return len;
+ }
+
+ #define strftime(a,b,c,d) strftime_win32(a,b,c,d)
+
+ #endif /* WIN32 */
+
/*
* Update the lc_time localization cache variables if needed.
diff --git
Re: [HACKERS] Updates of SE-PostgreSQL 8.4devel patches (r1197)
Bruce Momjian wrote: KaiGai Kohei wrote: What I am saying is for the default compile, SQL-level ACLs should be possible because, since the ACL field has optional storage, there is no downside to have it be available by default. I think it is a possible and desirable desicion from the viewpoint of security folks. However, I think we have a few issues, and it makes unclear whether we can make an agreement in the community. The one is a cost of security hooks. They consume a bit more CPU steps when a security mechanism is enabled. The other is prevention to override a few hooks (ExecutorRun_hook and planner_hook), because they assume standard implementations to be executed. Which is more desirable option in the default? Well, my assumption is that if a table doesn't have SQL-level row permissions then there is no overhead becaues there are no permissions to check. When the binary is built with the SQL-level row permissions and scanned table does not activated it, all it does is checking a flag variable at Relation-rd_options. I guess it will be acceptable cost. In this case, DBA disables row-level permission on the table, so no additional security field is required. For example, I might want to put SQL-level row permissions on an audit table, but none of my other tables, and in that case I assume there is only a performance impact on queries that use the audit table. It is not a zero, but tiny as far as we can ignore it in my opinion. Thanks. -- KaiGai Kohei [EMAIL PROTECTED] -- 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] blatantly a bug in the documentation
am Mon, dem 24.11.2008, um 7:24:47 -0500 mailte Merlin Moncure folgendes:
On Mon, Nov 24, 2008 at 6:02 AM, A. Kretschmer
[EMAIL PROTECTED] wrote:
I hope this is the right place to report a bug/issue in the official
documentation...
In
http://www.postgresql.org/docs/current/interactive/plpgsql-control-structures.html
there is an example for a function, cs_refresh_mviews(), chapter 38.6.4.
Within this function there are a PERFORM cs_log('Refreshing
materialized views...');, but such a function cs_log() are not in the
core distibution.
The example in the docs is pseudo-code. None of the tables referenced
exist either...the purpose of the example was to demonstrate the
syntax of for loops.
Okay, it is an argument. On the other side, it was a question today in
the irc-channel (#postgresql) today, someone asked, why his funktion
don't work. I think, such examples should not contain such code. It is
not apparent that this function are not available.
Again, thx to all people behind PostgreSQL, it is a great thing and it
has a really informatively documentation. My post was only a hint to
make it better.
Andreas
--
Andreas Kretschmer
Kontakt: Heynitz: 035242/47150, D1: 0160/7141639 (mehr: - Header)
GnuPG-ID: 0x3FFF606C, privat 0x7F4584DA http://wwwkeys.de.pgp.net
--
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: default values for function parameters
Pavel Stehule [EMAIL PROTECTED] writes: 2008/11/24 Peter Eisentraut [EMAIL PROTECTED]: Could you explain why you store the default expressions in a new posexpr type rather than in an array of text (compare pg_attrdef.adbin)? I would to implement named params - and there expressions, that are used as default params, should not be continual. I don't store params as array of text because I would to eliminate repeated expression's parsing. So I use similar machanism used for rules or views. Say again? The representation Peter is suggesting *is* what is used in rules and views. If you've re-invented that wheel, undo it. 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] Minor race-condition problem during database startup
Zdenek Kotala [EMAIL PROTECTED] writes: Zdenek Kotala napsal(a): By my opinion there is not reason to retry kill call, because it fails only in situation if process does not exist or caller does not have permission to send a signal. If I think more about it, that code is here because it probably tries to bypass time when bgwriter is restarted. Correct. Did you read the commit log message? http://archives.postgresql.org/pgsql-committers/2008-11/msg00269.php The bgwriter-exit case is currently treated as a crash and I'm not immediately proposing to change that, but this code or something close to it will be needed if we ever do want to recover from bgwriter exit nicely. 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] WIP: default values for function parameters
2008/11/24 Tom Lane [EMAIL PROTECTED]: Pavel Stehule [EMAIL PROTECTED] writes: 2008/11/24 Peter Eisentraut [EMAIL PROTECTED]: Could you explain why you store the default expressions in a new posexpr type rather than in an array of text (compare pg_attrdef.adbin)? I would to implement named params - and there expressions, that are used as default params, should not be continual. I don't store params as array of text because I would to eliminate repeated expression's parsing. So I use similar machanism used for rules or views. Say again? The representation Peter is suggesting *is* what is used in rules and views. If you've re-invented that wheel, undo it. Then I am blind. I store serialised transformed expression, but if better solution exists, then I'll use it. regards Pavel Stehule 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] blatantly a bug in the documentation
A. Kretschmer wrote: Okay, it is an argument. On the other side, it was a question today in the irc-channel (#postgresql) today, someone asked, why his funktion don't work. I think, such examples should not contain such code. It is not apparent that this function are not available. Perhaps the whole chapter could be improved if all the examples referred to a set of tables and functions previously defined in the introduction of the chapter. It would be good if the user could cut'n paste the code and try it out directly, instead of having to reverse engineer the tables/columns used for each example (and they're quite inconsistent). Also, some examples give the complete code including CREATE FUNCTION and others don't. Feel like submitting a patch? -- Alvaro Herrerahttp://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc. -- 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] Minor race-condition problem during database startup
Tom Lane napsal(a): Zdenek Kotala [EMAIL PROTECTED] writes: Zdenek Kotala napsal(a): By my opinion there is not reason to retry kill call, because it fails only in situation if process does not exist or caller does not have permission to send a signal. If I think more about it, that code is here because it probably tries to bypass time when bgwriter is restarted. Correct. Did you read the commit log message? http://archives.postgresql.org/pgsql-committers/2008-11/msg00269.php No. I thought that I look on old code. :( Zdenek -- 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] Windowing Function Patch Review - Standard Conformance
2008/11/24 Heikki Linnakangas [EMAIL PROTECTED]: Hitoshi Harada wrote: 2008/11/20 David Rowley [EMAIL PROTECTED]: I won't be around until Monday evening (Tuesday morning JST). I'll pickup the review again there. It's really time for me to push on with this review. The patch is getting closer to getting the thumbs up from me. I'm really hoping that can happen on Monday. Then it's over to Heikki for more code feedback. This time I only fixed some bugs and rebased against the HEAD, but did not refactored. I can figure out some part of what Heikki claimed but not all, so I'd like to see what he will send and post another patch if needed. Thanks! Here's what I've got this far I merged your latest patch into this, as well as latest changes from CVS HEAD. It's a bit of a mess, but here's the big changes I've done: - Removed all the iterator stuff. You can just call WinGetPart/FrameGetArg repeatedly. It ought to be just as fast if done right in nodeWindow.c. - Removed all the Shrinked/Extended stuff, as it's not needed until we have full support for window frames. - Removed all the WinRow* functions, you can just call WinFrame/PartGet* functions, using WINDOW_SEEK_CURRENT - Removed WinFrame/PartGetTuple functions. They were only used for determining if two tuples are peer with each other, so I added a WinRowIsPeer function instead. - Removed all the buffering strategy stuff. Currently, the whole partition is always materialized. That's of course not optimal; I'm still thinking we should just read the tuples from the outer node lazily, on-demand, instead. To avoid keeping all tuples in the partition in tuplestore, when not needed, should add an extra function to trim the tuplestore. There's now a lot less code in nodeWindow.c, and I'm starting to understand most of what-s left :-). TODO - clean up the comments and other mess. - Modify WinPart/FrameGetArg so that it's passed the parameter number instead of an Expr node. I'll continue working on the executor, but please let me know what you think. It is amazing changes and I like it. Actually I wanted to get it slimer but hadn't found the way. two points, frame concept and window_gettupleslot If you keep this in your mind, please don't be annoyed but the current frame concept is wrong. -- yours sample=# select depname, empno, salary, last_value(empno) over(order by salary) from empsalary; depname | empno | salary | last_value ---+---++ personnel | 5 | 3500 | 5 personnel | 2 | 3900 | 2 develop | 7 | 4200 | 7 develop | 9 | 4500 | 9 sales | 4 | 4800 | 4 sales | 3 | 4800 | 3 sales |b1 | 5000 | 1 develop |11 | 5200 | 11 develop |10 | 5200 | 10 develop | 8 | 6000 | 8 (10 rows) -- mine sample=# select depname, empno, salary, last_value(empno) over(order by salary) from empsalary; depname | empno | salary | last_value ---+---++ personnel | 5 | 3500 | 5 personnel | 2 | 3900 | 2 develop | 7 | 4200 | 7 develop | 9 | 4500 | 9 sales | 4 | 4800 | 3 sales | 3 | 4800 | 3 sales | 1 | 5000 | 1 develop |11 | 5200 | 10 develop |10 | 5200 | 10 develop | 8 | 6000 | 8 (10 rows) Note that on empno=4 then last_value=4(yours)/3(mine), which means the frame is applied to last_value() as well as nth_value() and first_value(). Not only to aggregates. So these lines in nodeWindow.c, /* * If there is an ORDER BY (we don't support other window frame * specifications yet), the frame runs from first row of the partition * to the current row. Otherwise the frame is the whole partition. */ if (((Window *) winobj-winstate-ss.ps.plan)-ordNumCols == 0) max_pos = winobj-partition_rows - 1; else max_pos = winobj-currentpos; max_pos is bogus. RANGE BETWEEN UNBOUNDED PRECEDING AND CURRENT ROW means max_pos would be currentpos + rows_peers. I looked over the patch and aggregates care it with winobj-aggregatedupto but what about other window functions? Then window_gettupleslot looks like killing performance in some cases. What if the partition has millions of rows and wfunc1 seeks the head and wfunc2 seeks the tail? So as you point it is possible to define frame and partition while scanning current rows rather than before scanning, I felt it'd cost more. But I know this is work in progress, so you probably think about the solutions. What do you think? 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] Snapshot warning
Pavan Deolasee escribió:
Following test case gives a warning of snapshot not destroyed at commit
time.
CREATE TABLE test (a int);
INSERT INTO test VALUES (1);
BEGIN;
DECLARE c CURSOR FOR SELECT * FROM test FOR update;
SAVEPOINT A;
FETCH -2 FROM c;
ROLLBACK TO SAVEPOINT A;
COMMIT;
Should we call FreeQueryDesc() even for failed portals in PortalCleanup() ?
Or PortalDrop() is a better(right) place to do that ?
That doesn't work; doing it causes a crash:
TRAP: FailedAssertion(«!(qdesc-estate == ((void *)0))», Archivo:
«/pgsql/source/00head/src/backend/tcop/pquery.c», Línea: 126)
which is here:
void
FreeQueryDesc(QueryDesc *qdesc)
{
/* Can't be a live query */
Assert(qdesc-estate == NULL);
BTW I noticed that AtSubAbort_Portals() is neglecting to set
portal-cleanup to NULL after calling it.
--
Alvaro Herrerahttp://www.CommandPrompt.com/
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] blatantly a bug in the documentation
Dave Page wrote: On Mon, Nov 24, 2008 at 1:38 PM, Alvaro Herrera [EMAIL PROTECTED] wrote: A. Kretschmer wrote: Okay, it is an argument. On the other side, it was a question today in the irc-channel (#postgresql) today, someone asked, why his funktion don't work. I think, such examples should not contain such code. It is not apparent that this function are not available. Perhaps the whole chapter could be improved if all the examples referred to a set of tables and functions previously defined in the introduction of the chapter. It would be good if the user could cut'n paste the code and try it out directly, instead of having to reverse engineer the tables/columns used for each example (and they're quite inconsistent). Also, some examples give the complete code including CREATE FUNCTION and others don't. It might also be useful to create such a database at initdb time so newbies have something interesting to look at right away. No, there is no need to clutter every installation in the world with such a database. You could make it an addon module, or a pgfoundry project. 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] blatantly a bug in the documentation
am Mon, dem 24.11.2008, um 13:47:54 + mailte Dave Page folgendes: On Mon, Nov 24, 2008 at 1:38 PM, Alvaro Herrera [EMAIL PROTECTED] wrote: A. Kretschmer wrote: Okay, it is an argument. On the other side, it was a question today in the irc-channel (#postgresql) today, someone asked, why his funktion don't work. I think, such examples should not contain such code. It is not apparent that this function are not available. Perhaps the whole chapter could be improved if all the examples referred to a set of tables and functions previously defined in the introduction of the chapter. It would be good if the user could cut'n paste the code and try it out directly, instead of having to reverse engineer the tables/columns used for each example (and they're quite inconsistent). Also, some examples give the complete code including CREATE FUNCTION and others don't. It might also be useful to create such a database at initdb time so newbies have something interesting to look at right away. Nice idea. Andreas -- Andreas Kretschmer Kontakt: Heynitz: 035242/47150, D1: 0160/7141639 (mehr: - Header) GnuPG-ID: 0x3FFF606C, privat 0x7F4584DA http://wwwkeys.de.pgp.net -- 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] blatantly a bug in the documentation
On Mon, Nov 24, 2008 at 1:53 PM, Andrew Dunstan [EMAIL PROTECTED] wrote: It might also be useful to create such a database at initdb time so newbies have something interesting to look at right away. No, there is no need to clutter every installation in the world with such a database. You could make it an addon module, or a pgfoundry project. That would defeat the point. Not that I have any great feelings either way, but fwiw, Microsoft and Oracle both create a sample database iirc. -- Dave Page EnterpriseDB UK: 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
[HACKERS] Re: [COMMITTERS] pgsql: Explicitly bind gettext() to the UTF8 locale when in use.
Magnus Hagander wrote: Hiroshi Inoue wrote: Hi Magnus and all, Magnus Hagander wrote: Log Message: --- Explicitly bind gettext() to the UTF8 locale when in use. This is required on Windows due to the special locale handling for UTF8 that doesn't change the full environment. Thanks to this change UTF-8 case was solved but Japanese users are still unhappy with Windows databases with EUC_JP encoding. Shift_JIS which is a Japanese encoding under Windows doesn't match any server encoding and causes a crash with the use of gettext. So Saito-san removed ja message catalog just before the 8.3 release. Attached is a simple patch to avoid the crash and enable the use of Japanese message catalog. Please apply the patch if there's no problem. Hi! It will clearly also need an update to the comment, but I can take care of that. I assume you have tested this? Though I myself didn't test it, Saito-san tested it. The comment says that it works because we are handling UTF8 on a special way on Windows, ISTM UTF-8 isn't a special case. In fact the comment also mentions the following. * In future we might want to call bind_textdomain_codeset * unconditionally, but that If your database is in EUC_JP, I don't see why gettext() isn't picking it up properly in the first place.. In Japan 2 encodings (EUC_JP and Shift_JIS) are often used. EUC_JP is mainly used on *nix and on Windows Shift_JIS is used. We use EUC_JP not Shift_JIS as the server encoding And why do we need that on Windows only, and not on other platforms? because Shift_JIS isn't allowed as a server encoding. So the Japanese Windows native message encoding Shift_JIS never matches the server encoding EUC_JP and a conversion between Shitt_jis and EUC_JP is necessarily needed. regards, Hiroshi Inoue -- 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] blatantly a bug in the documentation
Dave Page wrote: On Mon, Nov 24, 2008 at 1:53 PM, Andrew Dunstan [EMAIL PROTECTED] wrote: It might also be useful to create such a database at initdb time so newbies have something interesting to look at right away. No, there is no need to clutter every installation in the world with such a database. You could make it an addon module, or a pgfoundry project. That would defeat the point. Not that I have any great feelings either way, but fwiw, Microsoft and Oracle both create a sample database iirc. Last I checked, MS did it optionally only, no? //Magnus -- 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] blatantly a bug in the documentation
Andrew Dunstan [EMAIL PROTECTED] writes: Dave Page wrote: It might also be useful to create such a database at initdb time so newbies have something interesting to look at right away. No, there is no need to clutter every installation in the world with such a database. You could make it an addon module, or a pgfoundry project. The whole thing strikes me as extreme overkill, not to mention a misunderstanding of what an example is supposed to be for. If we're going to insist that every example in the docs work when copied-and-pasted into an empty database, then simple and to-the-point examples will be history. Instead of one-liners we'll have clutter. 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] blatantly a bug in the documentation
On Mon, Nov 24, 2008 at 2:02 PM, Tom Lane [EMAIL PROTECTED] wrote: Andrew Dunstan [EMAIL PROTECTED] writes: Dave Page wrote: It might also be useful to create such a database at initdb time so newbies have something interesting to look at right away. No, there is no need to clutter every installation in the world with such a database. You could make it an addon module, or a pgfoundry project. The whole thing strikes me as extreme overkill, not to mention a misunderstanding of what an example is supposed to be for. If we're going to insist that every example in the docs work when copied-and-pasted into an empty database, then simple and to-the-point examples will be history. Instead of one-liners we'll have clutter. That's the point of having a sample database ready to play with. The docs needn't have clutter, but the user can try out any of the examples without needing to setup anything first. It's a simple usability tweak that can help ease the learning curve for new users. -- Dave Page EnterpriseDB UK: 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
[HACKERS] Re: [COMMITTERS] pgsql: Explicitly bind gettext() to the UTF8 locale when in use.
Hiroshi Inoue wrote: Magnus Hagander wrote: Hiroshi Inoue wrote: Hi Magnus and all, Magnus Hagander wrote: Log Message: --- Explicitly bind gettext() to the UTF8 locale when in use. This is required on Windows due to the special locale handling for UTF8 that doesn't change the full environment. Thanks to this change UTF-8 case was solved but Japanese users are still unhappy with Windows databases with EUC_JP encoding. Shift_JIS which is a Japanese encoding under Windows doesn't match any server encoding and causes a crash with the use of gettext. So Saito-san removed ja message catalog just before the 8.3 release. Attached is a simple patch to avoid the crash and enable the use of Japanese message catalog. Please apply the patch if there's no problem. Hi! It will clearly also need an update to the comment, but I can take care of that. I assume you have tested this? Though I myself didn't test it, Saito-san tested it. Ok, good. The comment says that it works because we are handling UTF8 on a special way on Windows, ISTM UTF-8 isn't a special case. In fact the comment also mentions the following. * In future we might want to call bind_textdomain_codeset * unconditionally, but that I think that's partially unrelated. UTF8 is special in that the environment that the backend runs in is different from SERVER_ENCODING in this case. For other encodings, we have setlocale():d to the same encoding. If your database is in EUC_JP, I don't see why gettext() isn't picking it up properly in the first place.. In Japan 2 encodings (EUC_JP and Shift_JIS) are often used. EUC_JP is mainly used on *nix and on Windows Shift_JIS is used. We use EUC_JP not Shift_JIS as the server encoding And why do we need that on Windows only, and not on other platforms? because Shift_JIS isn't allowed as a server encoding. So the Japanese Windows native message encoding Shift_JIS never matches the server encoding EUC_JP and a conversion between Shitt_jis and EUC_JP is necessarily needed. Ah, so we're basically hardcoding that information? The system will go up in SJIS, but since we can't deal with it, we switch it to EUC_JP? Ok, I think I understand. I've made some minor stylistic changes (we don't normally use if (NULL != whatever) in the pg sources), and will apply with those. This is for HEAD only, correct? Or is it something we should backpatch? //Magnus -- 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] blatantly a bug in the documentation
On Mon, Nov 24, 2008 at 2:01 PM, Magnus Hagander [EMAIL PROTECTED] wrote: Dave Page wrote: That would defeat the point. Not that I have any great feelings either way, but fwiw, Microsoft and Oracle both create a sample database iirc. Last I checked, MS did it optionally only, no? Yes, but it defaults on iirc. We can emulate that behaviour in an installer (and do in Postgres Plus for example), but that doesn't help users of other distributions. -- Dave Page EnterpriseDB UK: 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
Re: [HACKERS] SQL/MED compatible connection manager
Martin Pihlak wrote:
Here's another revision of the connection manager. This adds:
* reference documentation
* psql, tab-completion, \dw, \dr, \dm commands.
* pg_dump support
Still todo:
* more comprehensive regression tests
* introductory documentation
* dblink support
I hope to finish these items during next week, and remove the WIP
status then.
Looks very good, please continue.
Most of this is straight out of the standard, so there isn't much to
discuss on the interfaces. I have two small things to wonder about:
On your wiki page, you have this example:
CREATE SERVER userdb TYPE 'plproxy_cluster'
FOREIGN DATA WRAPPER plproxy
OPTIONS (
server='dbname=userdb_p0 host=127.0.0.1 port=6000',
server='dbname=userdb_p1 host=127.0.0.1 port=6000',
server='dbname=userdb_p2 host=127.0.0.1 port=6000',
server='dbname=userdb_p3 host=127.0.0.1 port=6000',
connection_lifetime=3600
);
If I read this right, SQL/MED requires option names to be unique for a
server. To this needs to be rethought.
Do we really need the function pg_get_remote_connection_info()? This
could be done directly with the information schema. E.g., your example
SELECT * FROM pg_get_remote_connection_info('userdb');
appears to be the same as
SELECT option_name, option_value
FROM information_schema.foreign_server_options
WHERE foreign_server_name = 'userdb';
This view also appears to have the necessary access control provisions.
And similarly, pg_get_user_mapping_options() is equivalent to
information_schema.user_mapping_options.
--
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] blatantly a bug in the documentation
Dave Page [EMAIL PROTECTED] writes: On Mon, Nov 24, 2008 at 1:53 PM, Andrew Dunstan [EMAIL PROTECTED] wrote: No, there is no need to clutter every installation in the world with such a database. You could make it an addon module, or a pgfoundry project. That would defeat the point. By that logic, we should install the sample stuff in template1, to ensure that the examples would just work wherever $newbie tries to execute them :-( 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] blatantly a bug in the documentation
On Mon, Nov 24, 2008 at 2:14 PM, Tom Lane [EMAIL PROTECTED] wrote: Dave Page [EMAIL PROTECTED] writes: On Mon, Nov 24, 2008 at 1:53 PM, Andrew Dunstan [EMAIL PROTECTED] wrote: No, there is no need to clutter every installation in the world with such a database. You could make it an addon module, or a pgfoundry project. That would defeat the point. By that logic, we should install the sample stuff in template1, to ensure that the examples would just work wherever $newbie tries to execute them :-( *sigh*. Or one of us could visit their office and show them what to do, but I think we know the difference between making something usable out of the box and going overboard. -- Dave Page EnterpriseDB UK: 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
Re: [HACKERS] blatantly a bug in the documentation
am Mon, dem 24.11.2008, um 9:02:37 -0500 mailte Tom Lane folgendes: Andrew Dunstan [EMAIL PROTECTED] writes: Dave Page wrote: It might also be useful to create such a database at initdb time so newbies have something interesting to look at right away. No, there is no need to clutter every installation in the world with such a database. You could make it an addon module, or a pgfoundry project. The whole thing strikes me as extreme overkill, not to mention a misunderstanding of what an example is supposed to be for. If we're going to insist that every example in the docs work when copied-and-pasted into an empty database, then simple and to-the-point examples will be history. Instead of one-liners we'll have clutter. For a beginner, a relation 'foo' does not exist is a clean message, but a function foo() does not exist from an example in the doc are a real problem. And, in this example in my first post, the call to the nonexistent function are neither necessary nor 'to-the-point' to explain a for-loop. The starting point for my post was a real question on IRC today. Someone was really confused because he tried to use this nonexistent funktion to log something into a log-file. I think, this example can rewritten, either without this funktion-call or with raise notice ... Regards, Andreas -- Andreas Kretschmer Kontakt: Heynitz: 035242/47150, D1: 0160/7141639 (mehr: - Header) GnuPG-ID: 0x3FFF606C, privat 0x7F4584DA http://wwwkeys.de.pgp.net -- 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] blatantly a bug in the documentation
On Mon, Nov 24, 2008 at 9:02 AM, Tom Lane [EMAIL PROTECTED] wrote: Andrew Dunstan [EMAIL PROTECTED] writes: Dave Page wrote: It might also be useful to create such a database at initdb time so newbies have something interesting to look at right away. No, there is no need to clutter every installation in the world with such a database. You could make it an addon module, or a pgfoundry project. The whole thing strikes me as extreme overkill, not to mention a misunderstanding of what an example is supposed to be for. If we're going to insist that every example in the docs work when copied-and-pasted into an empty database, then simple and to-the-point examples will be history. Instead of one-liners we'll have clutter. Clutter is the problem. The cs_log functions in the example do not serve any purpose that is helpful to describe a for loop. They serve no real purpose...why not 'raise notice' or just remove them? It should be clear to distinguish from real and non-real elements. merlin -- 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] Visibility map, partial vacuums
Heikki Linnakangas [EMAIL PROTECTED] writes: I've been thinking that we could add one frozenxid field to each visibility map page, for the oldest xid on the heap pages covered by the visibility map page. That would allow more fine-grained anti-wraparound vacuums as well. This doesn't strike me as a particularly good idea. Right now the map is only hints as far as vacuum is concerned --- if you do the above then the map becomes critical data. And I don't really think you'll buy much. The visibility map won't be inquired unless you vacuum. This is a bit tricky. In vacuum, we only know whether we can set a bit or not, after we've acquired a cleanup lock on the page, and scanned all the tuples. While we're holding a cleanup lock, we don't want to do I/O, which could potentially block out other processes for a long time. So it's too late to extend the visibility map at that point. This is no good; I think you've made the wrong tradeoffs. In particular, even though only vacuum *currently* uses the map, you want to extend it to be used by indexscans. So it's going to uselessly spring into being even without vacuums. I'm not convinced that I/O while holding cleanup lock is so bad that we should break other aspects of the system to avoid it. However, if you want to stick to that, how about * vacuum page, possibly set its header bit * release page lock (but not pin) * if we need to set the bit, fetch the corresponding map page (I/O might happen here) * get share lock on heap page, then recheck its header bit; if still set, set the map bit 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] Re: [COMMITTERS] pgsql: Add support for matching wildcard server certificates to the new
Magnus Hagander wrote: Tom Lane wrote: Magnus Hagander [EMAIL PROTECTED] writes: ... The other option is to have autoconf substitute our own local version of fnmatch when this happens. How would one go about to do that in autoconf asks the autoconf n00b? I'd try adding a little test program that tries to compile #include fnmatch.h ... n = fnmatch(foo,bar, FNM_CASEFOLD); and see if that succeeds. For extra credit, incorporate that into a new PGAC_FUNC_FNMATCH macro, but I think you'd need to get Peter's help on the details --- I'm just a duffer on autoconf myself. Yeah, likely that someone who knows this will make it happen a lot faster. Peter, got time to help me out with this? Thanks! Well, FNM_CASEFOLD is not POSIX, and Autoconf thinks it's a GNU extension, which has obviously crept into other systems. So you'd need to use AC_FUNC_FNMATCH_GNU, but that also requires you to use the GNU replacement implementation. (A bit stupid, but then again, if you are trying to use GNU features, whose replacement implementation are you going to use.) If you google for FNM_CASEFOLD, you will get about a million hits of other open-source projects having run into this same issue. Then again, having looked into the libpq source now, is using fnmatch() even appropriate here? The matching rules for https are in RFC 2818: Matching is performed using the matching rules specified by [RFC2459]. If more than one identity of a given type is present in the certificate (e.g., more than one dNSName name, a match in any one of the set is considered acceptable.) Names may contain the wildcard character * which is considered to match any single domain name component or component fragment. E.g., *.a.com matches foo.a.com but not bar.foo.a.com. f*.com matches foo.com but not bar.com. Using fnmatch(), however, will also treat ? and [] special and it will not follow the any single domain name component rule. -- 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] Autoconf, libpq and replacement function
Magnus Hagander wrote: Anything that needs symlinks will need a set of workarounds on Windows, having us manually do a copy of the files. We already do this in a couple of cases, but relying more on it would make things even more kludgy I think... Autoconf automatically uses cp if ln doesn't work. This is already a solved problem that we currently use in several other situations. -- 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] Visibility map, partial vacuums
Heikki Linnakangas [EMAIL PROTECTED] writes: Tom Lane wrote: So it seems like we do indeed want to rejigger autovac's rules a bit to account for the possibility of wanting to apply vacuum to get visibility bits set. I'm not too excited about triggering an extra vacuum. As Matthew pointed out, the point of this patch is to reduce the number of vacuums required, not increase it. If you're not going to vacuum a table, you don't care if the bits in the visibility map are set or not. But it's already the case that the bits provide a performance increase to other things besides vacuum. We could set the PD_ALL_VISIBLE flag more aggressively, outside VACUUMs, if we want to make the seqscan optimization more effective. For example, a seqscan could set the flag too, if it sees that all the tuples were visible, and had the XMIN_COMMITTED and XMAX_INVALID hint bits set. I was wondering whether we could teach heap_page_prune to set the flag without adding any extra tuple visibility checks. A seqscan per se shouldn't be doing this because it doesn't normally call HeapTupleSatifiesVacuum. 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] Autoconf, libpq and replacement function
Peter Eisentraut wrote: Magnus Hagander wrote: Anything that needs symlinks will need a set of workarounds on Windows, having us manually do a copy of the files. We already do this in a couple of cases, but relying more on it would make things even more kludgy I think... Autoconf automatically uses cp if ln doesn't work. This is already a solved problem that we currently use in several other situations. I was talking about the MSVC part of the build system. //Magnus -- 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] [PATCHES] Solve a problem of LC_TIME of windows.
Hi Magnus-san.
Um, Though regrettable, it is not in a good state.
http://winpg.jp/~saito/pg_work/LC_MESSAGE_CHECK/LC_TIME_PATCH/Mugnus-patch.png
- Original Message -
From: Magnus Hagander [EMAIL PROTECTED]
The way I read this patch we do:
1) Get the time in CP_ACP
2) Convert to UTF16
3) Convert to UTF8
4) If necessary, convert to the charset in the database
We could be more efficient by at least folding 1 and 2 into a single
step, which means getting it in UTF16 from the beginning. How about
attached patch? It builds, but please verify that it fixes the problem.
(I've updated the comment as well)
Attached pg_locale_utf16.patch. I'm also attaching
pg_locale_diffdiff.patch which contains the changes I've made against
your patch only.
//Magnus
Hiroshi Saito wrote:
Hi.
All suggestion is appropriate and has been checked.
CVS-HEAD was examined by MinGW. $ make check NO_LOCALE=true
...
===
All 118 tests passed. ===
Then, It continues and a review is desired. Thanks!
Regatrds,
Hiroshi Saito
- Original Message - From: Hiroshi Saito
[EMAIL PROTECTED]
Hi ITAGAKI-san.
Sorry, very late reaction..
I lost time resources in an individual my machine trouble and
busyness.:-(
Now, I appreciate your exact work. ! Then, I desire the best patch for
PostgreSQL. Probably, I think that it is finally helpful in not a
problem of only Japan but many countries. Tom-san, and Alvaro-san,
Magnus-san understands the essence of this problem. Therefore, the
suggestion is expected for me.
Anyway, thank you very much.!!
Regards,
Hiroshi Saito
- Original Message - From: ITAGAKI Takahiro
[EMAIL PROTECTED]
Jaime Casanova [EMAIL PROTECTED] wrote:
i'm confused, original patch has this signature:
+ conv_strftime(char *src, size_t len, const char *format, const
struct tm *tm)
your's has:
+strftime_win32(char *dst, size_t dstlen, const char *format, const
you change all src for dst, just a variable name decision but a
radical one... why was that (i honestly doesn't understand this patch
very well ;)?
That's because the first argument is not an input buffer,
but an output buffer. MSDN also calls it 'strDest'.
http://msdn.microsoft.com/en-us/library/fe06s4ak(VS.71).aspx
Linux manpage calls it 's', but I think it means String, not Src.
http://man.cx/strftime
If you can review the patch, please use the attached one instead.
I modified it in response to the discussion of
pg_do_encoding_conversion.
BTW, I cannot understand the comment in the function head,
+ * result is obtained by locale setup of LC_TIME in the environment
+ * of windows at present CP_ACP. Therefore, conversion is needed
+ * for SERVER_ENCODING. SJIS which is not especially made to server
+ * encoding in Japan returns.
but it probably says:
strftime in Windows returns in CP_ACP encoding, but it could be
different from SERVER_ENCODING. Especially, Windows Japanese edition
requires conversions because it uses SJIS as CP_ACP, but we don't
support SJIS as a server encoding.
I hope you would review my English not only C ;-)
Regards,
---
ITAGAKI Takahiro
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
*** a/src/backend/utils/adt/pg_locale.c
--- b/src/backend/utils/adt/pg_locale.c
***
*** 54,59
--- 54,60
#include utils/memutils.h
#include utils/pg_locale.h
+ #include mb/pg_wchar.h
#define MAX_L10N_DATA 80
***
*** 452,457 PGLC_localeconv(void)
--- 453,507
return CurrentLocaleConv;
}
+ #ifdef WIN32
+ /*
+ * On win32, strftime() returns the encoding in CP_ACP, which is likely
+ * different from SERVER_ENCODING. This is especially important in Japanese
+ * versions of Windows which will use SJIS encoding, which we don't support
+ * as a server encoding.
+ *
+ * Replace strftime() with a version that gets the string in UTF16 and then
+ * converts it to the appropriate encoding as necessary.
+ */
+ static size_t
+ strftime_win32(char *dst, size_t dstlen, const char *format, const struct tm
*tm)
+ {
+ size_t len;
+ wchar_t wbuf[MAX_L10N_DATA];
+ int encoding;
+
+ encoding = GetDatabaseEncoding();
+ if (encoding == PG_SQL_ASCII)
+ return len;
+
+ len = wcsftime(wbuf, sizeof(wbuf), format, tm);
+ if (len == 0)
+ /* strftime called failed - return 0 with the contents of dst unspecified */
+ return 0;
+
+ len = WideCharToMultiByte(CP_UTF8, 0, wbuf, len, dst, dstlen, NULL, NULL);
+ if (len == 0)
+ ereport(ERROR,
+ (errmsg(could not convert string to UTF-8:error %lu, GetLastError(;
+
+ dst[len] = '\0';
+ if (encoding != PG_UTF8)
+ {
+ char *convstr = pg_do_encoding_conversion(dst, len, PG_UTF8, encoding);
+ if (dst != convstr)
+ {
+ StrNCpy(dst, convstr, dstlen);
+ len = strlen(dst);
Re: [HACKERS] [PATCHES] Solve a problem of LC_TIME of windows.
Magnus Hagander [EMAIL PROTECTED] writes:
*** a/src/backend/utils/adt/pg_locale.c
--- b/src/backend/utils/adt/pg_locale.c
***
*** 54,59
--- 54,60
#include utils/memutils.h
#include utils/pg_locale.h
+ #include mb/pg_wchar.h
#define MAX_L10N_DATA 80
Please stick to the convention of including include files in
alphabetical order.
+ strftime_win32(char *dst, size_t dstlen, const char *format, const struct
tm *tm)
+ {
+ size_t len;
+ wchar_t wbuf[MAX_L10N_DATA];
+ int encoding;
+
+ encoding = GetDatabaseEncoding();
+ if (encoding == PG_SQL_ASCII)
+ return len;
Surely this is returning an uninitialized variable, not to mention
failing to accomplish any of the goals of the function. I don't think
breaking things completely for SQL_ASCII was part of the plan.
+ ereport(ERROR,
+ (errmsg(could not convert string to UTF-8:error %lu,
GetLastError(;
This is not exactly per message style guidelines. Maybe it's just a
can't-happen case, but if so make it elog not ereport.
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] [PATCHES] Solve a problem of LC_TIME of windows.
Tom Lane wrote:
Magnus Hagander [EMAIL PROTECTED] writes:
*** a/src/backend/utils/adt/pg_locale.c
--- b/src/backend/utils/adt/pg_locale.c
***
*** 54,59
--- 54,60
#include utils/memutils.h
#include utils/pg_locale.h
+ #include mb/pg_wchar.h
#defineMAX_L10N_DATA 80
Please stick to the convention of including include files in
alphabetical order.
Check.
+ strftime_win32(char *dst, size_t dstlen, const char *format, const struct
tm *tm)
+ {
+size_t len;
+wchar_t wbuf[MAX_L10N_DATA];
+int encoding;
+
+encoding = GetDatabaseEncoding();
+if (encoding == PG_SQL_ASCII)
+return len;
Surely this is returning an uninitialized variable, not to mention
failing to accomplish any of the goals of the function. I don't think
breaking things completely for SQL_ASCII was part of the plan.
Gah, true, that's me breaking it. That was correct in Hiroshi-san's
patch. My bad, sorry.
+ereport(ERROR,
+(errmsg(could not convert string to UTF-8:error %lu,
GetLastError(;
This is not exactly per message style guidelines. Maybe it's just a
can't-happen case, but if so make it elog not ereport.
Check.
//Magnus
--
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] [PATCHES] Solve a problem of LC_TIME of windows.
Does it work when you're not using C-locale? This looks like the
codepath Tom pointed out was wrong.
//Magnus
Hiroshi Saito wrote:
Hi Magnus-san.
Um, Though regrettable, it is not in a good state.
http://winpg.jp/~saito/pg_work/LC_MESSAGE_CHECK/LC_TIME_PATCH/Mugnus-patch.png
- Original Message - From: Magnus Hagander [EMAIL PROTECTED]
The way I read this patch we do:
1) Get the time in CP_ACP
2) Convert to UTF16
3) Convert to UTF8
4) If necessary, convert to the charset in the database
We could be more efficient by at least folding 1 and 2 into a single
step, which means getting it in UTF16 from the beginning. How about
attached patch? It builds, but please verify that it fixes the problem.
(I've updated the comment as well)
Attached pg_locale_utf16.patch. I'm also attaching
pg_locale_diffdiff.patch which contains the changes I've made against
your patch only.
//Magnus
Hiroshi Saito wrote:
Hi.
All suggestion is appropriate and has been checked.
CVS-HEAD was examined by MinGW. $ make check NO_LOCALE=true
...
===
All 118 tests passed. ===
Then, It continues and a review is desired. Thanks!
Regatrds,
Hiroshi Saito
- Original Message - From: Hiroshi Saito
[EMAIL PROTECTED]
Hi ITAGAKI-san.
Sorry, very late reaction..
I lost time resources in an individual my machine trouble and
busyness.:-(
Now, I appreciate your exact work. ! Then, I desire the best patch for
PostgreSQL. Probably, I think that it is finally helpful in not a
problem of only Japan but many countries. Tom-san, and Alvaro-san,
Magnus-san understands the essence of this problem. Therefore, the
suggestion is expected for me.
Anyway, thank you very much.!!
Regards,
Hiroshi Saito
- Original Message - From: ITAGAKI Takahiro
[EMAIL PROTECTED]
Jaime Casanova [EMAIL PROTECTED] wrote:
i'm confused, original patch has this signature:
+ conv_strftime(char *src, size_t len, const char *format, const
struct tm *tm)
your's has:
+strftime_win32(char *dst, size_t dstlen, const char *format, const
you change all src for dst, just a variable name decision but a
radical one... why was that (i honestly doesn't understand this patch
very well ;)?
That's because the first argument is not an input buffer,
but an output buffer. MSDN also calls it 'strDest'.
http://msdn.microsoft.com/en-us/library/fe06s4ak(VS.71).aspx
Linux manpage calls it 's', but I think it means String, not Src.
http://man.cx/strftime
If you can review the patch, please use the attached one instead.
I modified it in response to the discussion of
pg_do_encoding_conversion.
BTW, I cannot understand the comment in the function head,
+ * result is obtained by locale setup of LC_TIME in the environment
+ * of windows at present CP_ACP. Therefore, conversion is needed
+ * for SERVER_ENCODING. SJIS which is not especially made to server
+ * encoding in Japan returns.
but it probably says:
strftime in Windows returns in CP_ACP encoding, but it could be
different from SERVER_ENCODING. Especially, Windows Japanese edition
requires conversions because it uses SJIS as CP_ACP, but we don't
support SJIS as a server encoding.
I hope you would review my English not only C ;-)
Regards,
---
ITAGAKI Takahiro
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
*** a/src/backend/utils/adt/pg_locale.c
--- b/src/backend/utils/adt/pg_locale.c
***
*** 54,59
--- 54,60
#include utils/memutils.h
#include utils/pg_locale.h
+ #include mb/pg_wchar.h
#define MAX_L10N_DATA 80
***
*** 452,457 PGLC_localeconv(void)
--- 453,507
return CurrentLocaleConv;
}
+ #ifdef WIN32
+ /*
+ * On win32, strftime() returns the encoding in CP_ACP, which is likely
+ * different from SERVER_ENCODING. This is especially important in
Japanese
+ * versions of Windows which will use SJIS encoding, which we don't
support
+ * as a server encoding.
+ *
+ * Replace strftime() with a version that gets the string in UTF16
and then
+ * converts it to the appropriate encoding as necessary.
+ */
+ static size_t
+ strftime_win32(char *dst, size_t dstlen, const char *format, const
struct tm *tm)
+ {
+ size_t len;
+ wchar_t wbuf[MAX_L10N_DATA];
+ int encoding;
+
+ encoding = GetDatabaseEncoding();
+ if (encoding == PG_SQL_ASCII)
+ return len;
+
+ len = wcsftime(wbuf, sizeof(wbuf), format, tm);
+ if (len == 0)
+ /* strftime called failed - return 0 with the contents of dst
unspecified */
+ return 0;
+
+ len = WideCharToMultiByte(CP_UTF8, 0, wbuf, len, dst, dstlen, NULL,
NULL);
+ if (len == 0)
+ ereport(ERROR,
+ (errmsg(could
[HACKERS] Re: [COMMITTERS] pgsql: Add support for matching wildcard server certificates to the new
Peter Eisentraut wrote: Magnus Hagander wrote: Tom Lane wrote: Magnus Hagander [EMAIL PROTECTED] writes: ... The other option is to have autoconf substitute our own local version of fnmatch when this happens. How would one go about to do that in autoconf asks the autoconf n00b? I'd try adding a little test program that tries to compile #include fnmatch.h ... n = fnmatch(foo,bar, FNM_CASEFOLD); and see if that succeeds. For extra credit, incorporate that into a new PGAC_FUNC_FNMATCH macro, but I think you'd need to get Peter's help on the details --- I'm just a duffer on autoconf myself. Yeah, likely that someone who knows this will make it happen a lot faster. Peter, got time to help me out with this? Thanks! Well, FNM_CASEFOLD is not POSIX, and Autoconf thinks it's a GNU extension, which has obviously crept into other systems. So you'd need to use AC_FUNC_FNMATCH_GNU, but that also requires you to use the GNU replacement implementation. (A bit stupid, but then again, if you are trying to use GNU features, whose replacement implementation are you going to use.) Meh, I looked at that, and considered having to implement it the GNU way was bad. Since we can't just import the GNU sourcecode. If you google for FNM_CASEFOLD, you will get about a million hits of other open-source projects having run into this same issue. Then again, having looked into the libpq source now, is using fnmatch() even appropriate here? The matching rules for https are in RFC 2818: Matching is performed using the matching rules specified by [RFC2459]. If more than one identity of a given type is present in the certificate (e.g., more than one dNSName name, a match in any one of the set is considered acceptable.) Names may contain the wildcard character * which is considered to match any single domain name component or component fragment. E.g., *.a.com matches foo.a.com but not bar.foo.a.com. f*.com matches foo.com but not bar.com. Using fnmatch(), however, will also treat ? and [] special and it will not follow the any single domain name component rule. Grr. I didn't find that RFC when I was googling around for the rules. Must've been a bad-google-day for me. And that does *not* match what I found on a couple of SSL-certificate-selling-websites that told you how it worked :-( I guess it's back to the drawingboard. Can probably still base it on the fnmatch stuff, but it'll need to be ripped apart. Basically, it should match only with *, and * should not match . - do you agree that's a reasonable interpretation? //Magnus -- 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] [PATCHES] Solve a problem of LC_TIME of windows.
Magnus Hagander wrote:
Tom Lane wrote:
Magnus Hagander [EMAIL PROTECTED] writes:
*** a/src/backend/utils/adt/pg_locale.c
--- b/src/backend/utils/adt/pg_locale.c
***
*** 54,59
--- 54,60
#include utils/memutils.h
#include utils/pg_locale.h
+ #include mb/pg_wchar.h
#define MAX_L10N_DATA 80
Please stick to the convention of including include files in
alphabetical order.
Check.
+ strftime_win32(char *dst, size_t dstlen, const char *format, const struct
tm *tm)
+ {
+ size_t len;
+ wchar_t wbuf[MAX_L10N_DATA];
+ int encoding;
+
+ encoding = GetDatabaseEncoding();
+ if (encoding == PG_SQL_ASCII)
+ return len;
Surely this is returning an uninitialized variable, not to mention
failing to accomplish any of the goals of the function. I don't think
breaking things completely for SQL_ASCII was part of the plan.
Gah, true, that's me breaking it. That was correct in Hiroshi-san's
patch. My bad, sorry.
+ ereport(ERROR,
+ (errmsg(could not convert string to UTF-8:error %lu,
GetLastError(;
This is not exactly per message style guidelines. Maybe it's just a
can't-happen case, but if so make it elog not ereport.
Check.
Forgot the attachment.
//Magnus
*** a/src/backend/utils/adt/pg_locale.c
--- b/src/backend/utils/adt/pg_locale.c
***
*** 51,56
--- 51,57
#include time.h
#include catalog/pg_control.h
+ #include mb/pg_wchar.h
#include utils/memutils.h
#include utils/pg_locale.h
***
*** 452,457 PGLC_localeconv(void)
--- 453,507
return CurrentLocaleConv;
}
+ #ifdef WIN32
+ /*
+ * On win32, strftime() returns the encoding in CP_ACP, which is likely
+ * different from SERVER_ENCODING. This is especially important in Japanese
+ * versions of Windows which will use SJIS encoding, which we don't support
+ * as a server encoding.
+ *
+ * Replace strftime() with a version that gets the string in UTF16 and then
+ * converts it to the appropriate encoding as necessary.
+ */
+ static size_t
+ strftime_win32(char *dst, size_t dstlen, const char *format, const struct tm *tm)
+ {
+ size_t len;
+ wchar_t wbuf[MAX_L10N_DATA];
+ int encoding;
+
+ encoding = GetDatabaseEncoding();
+ if (encoding == PG_SQL_ASCII)
+ return strftime(dst, dstlen, format, tm);
+
+ len = wcsftime(wbuf, sizeof(wbuf), format, tm);
+ if (len == 0)
+ /* strftime call failed - return 0 with the contents of dst unspecified */
+ return 0;
+
+ len = WideCharToMultiByte(CP_UTF8, 0, wbuf, len, dst, dstlen, NULL, NULL);
+ if (len == 0)
+ elog(ERROR,
+ could not convert string to UTF-8:error %lu, GetLastError());
+
+ dst[len] = '\0';
+ if (encoding != PG_UTF8)
+ {
+ char *convstr = pg_do_encoding_conversion(dst, len, PG_UTF8, encoding);
+ if (dst != convstr)
+ {
+ StrNCpy(dst, convstr, dstlen);
+ len = strlen(dst);
+ }
+ }
+
+ return len;
+ }
+
+ #define strftime(a,b,c,d) strftime_win32(a,b,c,d)
+
+ #endif /* WIN32 */
+
/*
* Update the lc_time localization cache variables if needed.
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] [bugfix] DISCARD ALL does not release advisory locks
It was brought to my attention that DISCARD ALL does not release advisory locks: http://pgfoundry.org/tracker/index.php?func=detailaid=1010499group_id=1000258atid=983 Thus connection managers / poolers need to do that manually. This obviously means DISCARD ALL needs fixing. Applied patch adds LockReleaseAll(USER_LOCKMETHOD, true) call to DiscardAll(). Should this also be fixed in 8.3? -- marko diff --git a/doc/src/sgml/ref/discard.sgml b/doc/src/sgml/ref/discard.sgml index ecae00b..f2d6ea2 100644 --- a/doc/src/sgml/ref/discard.sgml +++ b/doc/src/sgml/ref/discard.sgml @@ -82,6 +82,7 @@ CLOSE ALL; UNLISTEN *; DISCARD PLANS; DISCARD TEMP; +SELECT pg_advisory_unlock_all(); /programlisting /para /listitem diff --git a/src/backend/commands/discard.c b/src/backend/commands/discard.c index d7bddbd..2917b76 100644 --- a/src/backend/commands/discard.c +++ b/src/backend/commands/discard.c @@ -68,4 +68,5 @@ DiscardAll(bool isTopLevel) Async_UnlistenAll(); ResetPlanCache(); ResetTempTableNamespace(); + LockReleaseAll(USER_LOCKMETHOD, 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] [bugfix] DISCARD ALL does not release advisory locks
Marko Kreen [EMAIL PROTECTED] writes: It was brought to my attention that DISCARD ALL does not release advisory locks: What is the argument that it should? 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] Re: [COMMITTERS] pgsql: Add support for matching wildcard server certificates to the new
Peter Eisentraut [EMAIL PROTECTED] writes: Well, FNM_CASEFOLD is not POSIX, and Autoconf thinks it's a GNU extension, which has obviously crept into other systems. So you'd need to use AC_FUNC_FNMATCH_GNU, but that also requires you to use the GNU replacement implementation. (A bit stupid, but then again, if you are trying to use GNU features, whose replacement implementation are you going to use.) It's also fair to wonder whether an appropriate set of case folding rules is going to get used. 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] [PATCHES] Solve a problem of LC_TIME of windows.
Hi Magnus-san.
Umm, format operand seems to be a wide character sequence.
- Original Message -
From: Magnus Hagander [EMAIL PROTECTED]
Magnus Hagander wrote:
Tom Lane wrote:
Magnus Hagander [EMAIL PROTECTED] writes:
*** a/src/backend/utils/adt/pg_locale.c
--- b/src/backend/utils/adt/pg_locale.c
***
*** 54,59
--- 54,60
#include utils/memutils.h
#include utils/pg_locale.h
+ #include mb/pg_wchar.h
#define MAX_L10N_DATA 80
Please stick to the convention of including include files in
alphabetical order.
Check.
+ strftime_win32(char *dst, size_t dstlen, const char *format, const struct tm
*tm)
+ {
+ size_t len;
+ wchar_t wbuf[MAX_L10N_DATA];
+ int encoding;
+
+ encoding = GetDatabaseEncoding();
+ if (encoding == PG_SQL_ASCII)
+ return len;
Surely this is returning an uninitialized variable, not to mention
failing to accomplish any of the goals of the function. I don't think
breaking things completely for SQL_ASCII was part of the plan.
Gah, true, that's me breaking it. That was correct in Hiroshi-san's
patch. My bad, sorry.
+ ereport(ERROR,
+ (errmsg(could not convert string to UTF-8:error %lu, GetLastError(;
This is not exactly per message style guidelines. Maybe it's just a
can't-happen case, but if so make it elog not ereport.
Check.
Forgot the attachment.
//Magnus
*** a/src/backend/utils/adt/pg_locale.c
--- b/src/backend/utils/adt/pg_locale.c
***
*** 51,56
--- 51,57
#include time.h
#include catalog/pg_control.h
+ #include mb/pg_wchar.h
#include utils/memutils.h
#include utils/pg_locale.h
***
*** 452,457 PGLC_localeconv(void)
--- 453,507
return CurrentLocaleConv;
}
+ #ifdef WIN32
+ /*
+ * On win32, strftime() returns the encoding in CP_ACP, which is likely
+ * different from SERVER_ENCODING. This is especially important in Japanese
+ * versions of Windows which will use SJIS encoding, which we don't support
+ * as a server encoding.
+ *
+ * Replace strftime() with a version that gets the string in UTF16 and then
+ * converts it to the appropriate encoding as necessary.
+ */
+ static size_t
+ strftime_win32(char *dst, size_t dstlen, const char *format, const struct tm
*tm)
+ {
+ size_t len;
+ wchar_t wbuf[MAX_L10N_DATA];
+ int encoding;
+
+ encoding = GetDatabaseEncoding();
+ if (encoding == PG_SQL_ASCII)
+ return strftime(dst, dstlen, format, tm);
+
+ len = wcsftime(wbuf, sizeof(wbuf), format, tm);
+ if (len == 0)
+ /* strftime call failed - return 0 with the contents of dst unspecified */
+ return 0;
+
+ len = WideCharToMultiByte(CP_UTF8, 0, wbuf, len, dst, dstlen, NULL, NULL);
+ if (len == 0)
+ elog(ERROR,
+ could not convert string to UTF-8:error %lu, GetLastError());
+
+ dst[len] = '\0';
+ if (encoding != PG_UTF8)
+ {
+ char *convstr = pg_do_encoding_conversion(dst, len, PG_UTF8, encoding);
+ if (dst != convstr)
+ {
+ StrNCpy(dst, convstr, dstlen);
+ len = strlen(dst);
+ }
+ }
+
+ return len;
+ }
+
+ #define strftime(a,b,c,d) strftime_win32(a,b,c,d)
+
+ #endif /* WIN32 */
+
/*
* Update the lc_time localization cache variables if needed.
--
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] Visibility map, partial vacuums
Tom Lane [EMAIL PROTECTED] writes: Heikki Linnakangas [EMAIL PROTECTED] writes: I've been thinking that we could add one frozenxid field to each visibility map page, for the oldest xid on the heap pages covered by the visibility map page. That would allow more fine-grained anti-wraparound vacuums as well. This doesn't strike me as a particularly good idea. Right now the map is only hints as far as vacuum is concerned --- if you do the above then the map becomes critical data. And I don't really think you'll buy much. Hm, that depends on how critical the critical data is. It's critical that the frozenxid that autovacuum sees is no more recent than the actual frozenxid, but not critical that it be entirely up-to-date otherwise. So if it's possible for the frozenxid in the visibility map to go backwards then it's no good, since if that update is lost we might skip a necessary vacuum freeze. But if we guarantee that we never update the frozenxid in the visibility map forward ahead of recentglobalxmin then it can't ever go backwards. (Well, not in a way that matters) However I'm a bit puzzled how you could possibly maintain this frozenxid. As soon as you freeze an xid you'll have to visit all the other pages covered by that visibility map page to see what the new value should be. -- Gregory Stark EnterpriseDB http://www.enterprisedb.com Ask me about EnterpriseDB's 24x7 Postgres 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] [bugfix] DISCARD ALL does not release advisory locks
On 11/24/08, Tom Lane [EMAIL PROTECTED] wrote: Marko Kreen [EMAIL PROTECTED] writes: It was brought to my attention that DISCARD ALL does not release advisory locks: What is the argument that it should? DISCARD ALL is supposed to be used by poolers to reset connection back to startup state to reuse server connection after client disconnect. New client should see no difference between fresh backend and old backend where DISCARD ALL was issued. IOW, DISCARD ALL should be functionally equivalent to backend exit. If user want more explicit control over what resources are released, he should avoid use of DISCARD ALL, instead he should manually pick individual components from the command sequence DISCARD ALL is equivalent to. Eg. when user wants to keep old plans or advisory locks around, he should manually construct command list that resets everything except those. But DISCARD ALL should release everything possible, never should additional commands be needed in addition to it to do full reset. -- marko -- 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] [PATCHES] Solve a problem of LC_TIME of windows.
Gregory Stark [EMAIL PROTECTED] writes: Tom Lane [EMAIL PROTECTED] writes: Please stick to the convention of including include files in alphabetical order. I didn't realize we had such a convention. Is this for all include files or only within a directory? Are there exceptions where the order matters or have we managed to make sure it doesn't everywhere? If the order matters, it's a bug in the include files. Bruce runs a script periodically to check that no include file depends on anything except postgres.h (or postgres_fe.h, as appropriate). 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] Visibility map, partial vacuums
Gregory Stark wrote: However I'm a bit puzzled how you could possibly maintain this frozenxid. As soon as you freeze an xid you'll have to visit all the other pages covered by that visibility map page to see what the new value should be. Right, you could only advance it when you scan all the pages covered by the visibility map page. But that's better than having to scan the whole relation. -- 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
Re: [HACKERS] Updates of SE-PostgreSQL 8.4devel patches (r1197)
Tom Lane wrote: KaiGai Kohei [EMAIL PROTECTED] writes: However, I think we have a few issues, and it makes unclear whether we can make an agreement in the community. The one is a cost of security hooks. They consume a bit more CPU steps when a security mechanism is enabled. The other is prevention to override a few hooks (ExecutorRun_hook and planner_hook), because they assume standard implementations to be executed. I think your chances of taking those hooks away are zero. It would cripple a lot of other facilities that people are more interested in than they are in SEPostgres. In any case, the only way to use those hooks is to load C code into the backend, and anyone who can do that already has the keys to the kingdom. I hope you are not suffering from any illusions about being able to defend against arbitrary add-on C code. I removed the two hooks at the r1244 patch set. As you said, it is fundamentally danger to load uncertain binary modules. Thus, what we should do is checks on module loading. The default security policy requires loadable modules to be labeled as 'lib_t' type which means shared library files installed correctly. Thanks, -- KaiGai Kohei [EMAIL PROTECTED] -- 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] Proposed Patch to Improve Performance of Multi-Batch Hash Join for Skewed Data Sets
-Original Message- From: Tom Lane [mailto:[EMAIL PROTECTED] I'm a tad worried about what happens when the values that are frequently occurring in the outer relation are also frequently occurring in the inner (which hardly seems an improbable case). Don't you stand a severe risk of blowing out the in-memory hash table? It doesn't appear to me that the code has any way to back off once it's decided that a certain set of join key values are to be treated in-memory. Splitting the main join into more batches certainly doesn't help with that. Also, AFAICS the benefit of this patch comes entirely from avoiding dump and reload of tuples bearing the most common values, which means it's a significant waste of cycles when there's only one batch. It'd be better to avoid doing any of the extra work in the single-batch case. One thought that might address that point as well as the difficulty of getting stats in nontrivial cases is to wait until we've overrun memory and are forced to start batching, and at that point determine on-the-fly which are the most common hash values from inspection of the hash table as we dump it out. This would amount to optimizing on the basis of frequency in the *inner* relation not the outer, but offhand I don't see any strong theoretical basis why that wouldn't be just as good. It could lose if the first work_mem worth of inner tuples isn't representative of what follows; but this hardly seems more dangerous than depending on MCV stats that are for the whole outer relation rather than the portion of it being selected. regards, tom lane You are correct with both observations. The patch only has a benefit when there is more than one batch. Also, there is a potential issue with MCV hash table overflows if the number of tuples that match the MCVs in the build relation is very large. Bryce has created a patch (attached) that disables the code for one batch joins. This patch also checks for MCV hash table overflows and handles them by flushing from the MCV hash table back to the main hash table. The main hash table will then resolve overflows as usual. Note that this will cause the worse case of a build table with all the same values to be handled the same as the current hash code, i.e., it will attempt to re-partition until it eventually gives up and then allocates the entire partition in memory. There may be a better way to handle this case, but the new patch will remain consistent with the current hash join implementation. The issue with determining and using the MCV stats is more challenging than it appears. First, knowing the MCVs of the build table will not help us. What we need are the MCVs of the probe table because by knowing those values we will keep the tuples with those values in the build relation in memory. For example, consider a join between tables Part and LineItem. Assume 1 popular part accounts for 10% of all LineItems. If Part is the build relation and LineItem is the probe relation, then by keeping that 1 part record in memory, we will guarantee that we do not need to write out 10% of LineItem. If a selection occurs on LineItem before the join, it may change the distribution of LineItem (the MCVs) but it is probable that they are still a good estimate of the MCVs in the derived LineItem relation. (We did experiments on trying to sample the first few thousand tuples of the probe relation to dynamically determine the MCVs but generally found this was inaccurate due to non-random samples.) In essence, the goal is to smartly pick the tuples that remain in the in-memory batch before probing begins. Since the number of MCVs is small, incorrectly selecting build tuples to remain in memory has negligible cost. If we assume that LineItem has been filtered so much that it is now smaller than Part and is the build relation then the MCV approach does not apply. There is no skew in Part on partkey (since it is the PK) and knowing the MCV partkeys in LineItem does not help us because they each only join with a single tuple in Part. In this case, the MCV approach should not be used because no benefit is possible, and it will not be used because there will be no MCVs for Part.partkey. The bad case with MCV hash table overflow requires a many-to-many join between the two relations which would not occur on the more typical PK-FK joins. -- Dr. Ramon Lawrence Assistant Professor, Department of Computer Science, University of British Columbia Okanagan E-mail: [EMAIL PROTECTED] histojoin_v3.patch Description: histojoin_v3.patch -- 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] Visibility map, partial vacuums
Heikki Linnakangas [EMAIL PROTECTED] writes: Gregory Stark wrote: However I'm a bit puzzled how you could possibly maintain this frozenxid. As soon as you freeze an xid you'll have to visit all the other pages covered by that visibility map page to see what the new value should be. Right, you could only advance it when you scan all the pages covered by the visibility map page. But that's better than having to scan the whole relation. Is it? It seems like that would just move around the work. You'll still have to visit every page once ever 2B transactions or so. You'll just do it 64MB at a time. It's nice to smooth the work but it would be much nicer to detect that a normal vacuum has already processed all of those pages since the last insert/update/delete on those pages and so avoid the work entirely. To avoid the work entirely you need some information about the oldest xid on those pages seen by regular vacuums (and/or prunes). We would want to skip any page which: a) Has been visited by vacuum freeze and not been updated since b) Has been visited by a regular vacuum and the oldest xid found was more recent than freeze_threshold. c) Has been updated frequently such that no old tuples remain Ideally (b) should completely obviate the need for anti-wraparound freezes entirely. -- Gregory Stark EnterpriseDB http://www.enterprisedb.com Ask me about EnterpriseDB's 24x7 Postgres 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] Re: [COMMITTERS] pgsql: Explicitly bind gettext() to the UTF8 locale when in use.
Magnus Hagander [EMAIL PROTECTED] writes: Hiroshi Inoue wrote: because Shift_JIS isn't allowed as a server encoding. So the Japanese Windows native message encoding Shift_JIS never matches the server encoding EUC_JP and a conversion between Shitt_jis and EUC_JP is necessarily needed. Ah, so we're basically hardcoding that information? The system will go up in SJIS, but since we can't deal with it, we switch it to EUC_JP? I'm not following this either. If the patch is really necessary then it seems it must be working around a bug in the Windows version of gettext, ie failure to distinguish CP932 from CP20932. Is that correct? Ok, I think I understand. I've made some minor stylistic changes (we don't normally use if (NULL != whatever) in the pg sources), and will apply with those. It definitely needs a comment explaining why this is needed. 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] blatantly a bug in the documentation
On Mon, Nov 24, 2008 at 1:38 PM, Alvaro Herrera [EMAIL PROTECTED] wrote: A. Kretschmer wrote: Okay, it is an argument. On the other side, it was a question today in the irc-channel (#postgresql) today, someone asked, why his funktion don't work. I think, such examples should not contain such code. It is not apparent that this function are not available. Perhaps the whole chapter could be improved if all the examples referred to a set of tables and functions previously defined in the introduction of the chapter. It would be good if the user could cut'n paste the code and try it out directly, instead of having to reverse engineer the tables/columns used for each example (and they're quite inconsistent). Also, some examples give the complete code including CREATE FUNCTION and others don't. It might also be useful to create such a database at initdb time so newbies have something interesting to look at right away. -- Dave Page EnterpriseDB UK: 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
Re: [HACKERS] blatantly a bug in the documentation
On Monday 24 November 2008 16:07:49 Dave Page wrote: That's the point of having a sample database ready to play with. The docs needn't have clutter, but the user can try out any of the examples without needing to setup anything first. It's a simple usability tweak that can help ease the learning curve for new users. I find that the regression test database usually works for that. -- 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] Snapshot warning
Tom Lane escribió: Alvaro Herrera [EMAIL PROTECTED] writes: Pavan Deolasee escribi�: Should we call FreeQueryDesc() even for failed portals in PortalCleanup() ? Or PortalDrop() is a better(right) place to do that ? That doesn't work; doing it causes a crash: I think the fundamental bug here is that you tried to skip using the ResourceOwner mechanism for snapshot references. That's basically not gonna work. Right :-( I'll see how to go about this. -- Alvaro Herrerahttp://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc. -- 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] blatantly a bug in the documentation
Clutter is the problem. The cs_log functions in the example do not serve any purpose that is helpful to describe a for loop. They serve no real purpose...why not 'raise notice' or just remove them? It should be clear to distinguish from real and non-real elements. +1 for RAISE NOTICE. ...Robert -- 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] Snapshot warning
Alvaro Herrera [EMAIL PROTECTED] writes: Tom Lane escribió: I think the fundamental bug here is that you tried to skip using the ResourceOwner mechanism for snapshot references. That's basically not gonna work. Right :-( I'll see how to go about this. It strikes me that you might be able to remove the registered-snapshot infrastructure in snapmgr.c, and end up with about the same amount of code overall, just with the responsibility in resowner.c. 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] [PATCHES] Solve a problem of LC_TIME of windows.
Tom Lane [EMAIL PROTECTED] writes: Please stick to the convention of including include files in alphabetical order. I didn't realize we had such a convention. Is this for all include files or only within a directory? Are there exceptions where the order matters or have we managed to make sure it doesn't everywhere? -- Gregory Stark EnterpriseDB http://www.enterprisedb.com Get trained by Bruce Momjian - ask me about EnterpriseDB's PostgreSQL training! -- 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] Visibility map, partial vacuums
Gregory Stark [EMAIL PROTECTED] writes: So if it's possible for the frozenxid in the visibility map to go backwards then it's no good, since if that update is lost we might skip a necessary vacuum freeze. Seems like a lost disk write would be enough to make that happen. Now you might argue that the odds of that are no worse than the odds of losing an update to one particular heap page, but in this case the single hiccup could lead to losing half a gigabyte of data (assuming 8K page size). The leverage you get for saving vacuum freeze work is exactly equal to the magnification factor for data loss. 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] Snapshot warning
Alvaro Herrera [EMAIL PROTECTED] writes: Pavan Deolasee escribió: Should we call FreeQueryDesc() even for failed portals in PortalCleanup() ? Or PortalDrop() is a better(right) place to do that ? That doesn't work; doing it causes a crash: I think the fundamental bug here is that you tried to skip using the ResourceOwner mechanism for snapshot references. That's basically not gonna work. 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] [GENERAL] literal limits in 8.3
Sam Mason [EMAIL PROTECTED] writes:
On Mon, Nov 24, 2008 at 10:45:42AM -0500, Tom Lane wrote:
Well, I can't reproduce that here. Something strange about your
configuration maybe?
Not that I know of. I've just created a test cluster to make sure and I
get the same behaviour.
Hmm ... the third machine I tried was able to reproduce the problem.
What it boils down to is lack of error checking in psql (not the
backend). Specifically, we fail to enlarge the output buffer for
psqlscan.l, which causes appendBinaryPQExpBuffer to silently not insert
the chunk it's currently being passed. Which you might think would be
some random subset of the input string, leading to a parse error on
the backend side --- but no, this is the output of a lexical scan which
means what is dropped is exactly the contents of the multi-megabyte
string literal, not less or more. And then later insertions work fine
since *they* aren't provoking an out-of-memory problem. So eventually
the backend receives
INSERT INTO test (col) VALUES ('');
which of course it finds nothing wrong with.
This is sort of a PITA to fix :-(. The easiest solution from the point
of view of psql would be to have realloc failure just print out of
memory and exit(1), but pqexpbuffer.c is part of libpq and so it's not
too reasonable for it to do that. And we have also got to think about
the prospect of similarly non-random lossage in other uses of
PQexpbuffer, anyhow.
The least API-damaging solution I can think of is to add an error
indicator flag to PQexpbuffer, comparable to ferror() on stdio files.
Callers would have to check this after loading up a PQexpbuffer if
they wanted to be sure there was no memory overrun. But that seems
pretty fragile, and it wouldn't be back-patchable.
A variant on that is to clear the buffer and insert out of memory
in this scenario, but that's not too pleasant either.
Better ideas anyone?
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] [GENERAL] literal limits in 8.3
On Mon, Nov 24, 2008 at 02:46:45PM -0500, Tom Lane wrote: What it boils down to is lack of error checking in psql (not the backend). Specifically, we fail to enlarge the output buffer for psqlscan.l, which causes appendBinaryPQExpBuffer to silently not insert the chunk it's currently being passed. This is sort of a PITA to fix :-(. The easiest solution from the point of view of psql would be to have realloc failure just print out of memory and exit(1), but pqexpbuffer.c is part of libpq and so it's not too reasonable for it to do that. And we have also got to think about the prospect of similarly non-random lossage in other uses of PQexpbuffer, anyhow. The least API-damaging solution I can think of is to add an error indicator flag to PQexpbuffer, comparable to ferror() on stdio files. Callers would have to check this after loading up a PQexpbuffer if they wanted to be sure there was no memory overrun. But that seems pretty fragile, and it wouldn't be back-patchable. Not much of a better idea; but from a correctness point of view an option could be to have appendBinaryPQExpBuffer() abort if it runs out of space and export an alternative function that allows the error to propagate instead of aborting. Not sure if that's allowed though; is there a link to the project's guidelines for ABI compatibility? Sam -- 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: default values for function parameters
On Monday 24 November 2008 11:40:31 Pavel Stehule wrote: I would to implement named params - and there expressions, that are used as default params, should not be continual. I don't store params as array of text because I would to eliminate repeated expression's parsing. So I use similar machanism used for rules or views. You mean you want to avoid repeated parsing of expressions in case the same expression is used as a default value for several parameters? How common would that be? -- 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: Add support for matching wildcard server certificates to the new
On Monday 24 November 2008 16:55:17 Magnus Hagander wrote: Then again, having looked into the libpq source now, is using fnmatch() even appropriate here? The matching rules for https are in RFC 2818: Using fnmatch(), however, will also treat ? and [] special and it will not follow the any single domain name component rule. I guess it's back to the drawingboard. Can probably still base it on the fnmatch stuff, but it'll need to be ripped apart. Basically, it should match only with *, and * should not match . - do you agree that's a reasonable interpretation? Some more information on this: https://www.switch.ch/pki/meetings/2007-01/namebased_ssl_virtualhosts.pdf slide 5 lists the matching rules for email, HTTP, and LDAP over TLS, respectively, which are not all the same. Also note that these methods have rules for interpreting fields in the certificate other than the common name for the host name. I think it is safest and easiest to allow a * wildcard only as the first character and only when followed immediately by a dot. Maybe some DNS expert around here can offer advice on what a morally sound solution would be. -- 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: default values for function parameters
Pavel Stehule escribió: 2008/11/24 Tom Lane [EMAIL PROTECTED]: Say again? The representation Peter is suggesting *is* what is used in rules and views. If you've re-invented that wheel, undo it. Then I am blind. I store serialised transformed expression, but if better solution exists, then I'll use it. Seem to me you just want to store the output of nodeToString. -- Alvaro Herrerahttp://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc. -- 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: Add support for matching wildcard server certificates to the new
I wrote: Some more information on this: https://www.switch.ch/pki/meetings/2007-01/namebased_ssl_virtualhosts.pdf slide 5 lists the matching rules for email, HTTP, and LDAP over TLS, respectively, which are not all the same. Also note that these methods have rules for interpreting fields in the certificate other than the common name for the host name. I think it is safest and easiest to allow a * wildcard only as the first character and only when followed immediately by a dot. Maybe some DNS expert around here can offer advice on what a morally sound solution would be. This page summarizes the sadness pretty well: http://wiki.cacert.org/wiki/WildcardCertificates -- 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: default values for function parameters
2008/11/24 Peter Eisentraut [EMAIL PROTECTED]: On Monday 24 November 2008 11:40:31 Pavel Stehule wrote: I would to implement named params - and there expressions, that are used as default params, should not be continual. I don't store params as array of text because I would to eliminate repeated expression's parsing. So I use similar machanism used for rules or views. You mean you want to avoid repeated parsing of expressions in case the same expression is used as a default value for several parameters? How common would that be? no - I am reading default parameters in call statement parsing. Default parameters are implemented similar to variadic functions - so no changes on PL part - all changes are on caller part. Pavel -- 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: default values for function parameters
On Monday 24 November 2008 23:21:15 Pavel Stehule wrote: You mean you want to avoid repeated parsing of expressions in case the same expression is used as a default value for several parameters? How common would that be? no - I am reading default parameters in call statement parsing. Default parameters are implemented similar to variadic functions - so no changes on PL part - all changes are on caller part. Then I don't understand why you need this special data type instead of using an array of text with nulls for parameters without default. -- 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: default values for function parameters
2008/11/24 Peter Eisentraut [EMAIL PROTECTED]: On Monday 24 November 2008 23:21:15 Pavel Stehule wrote: You mean you want to avoid repeated parsing of expressions in case the same expression is used as a default value for several parameters? How common would that be? no - I am reading default parameters in call statement parsing. Default parameters are implemented similar to variadic functions - so no changes on PL part - all changes are on caller part. Then I don't understand why you need this special data type instead of using an array of text with nulls for parameters without default. I expect some overhead with classic array - but this overhead will be small and array of text with nulls is better variant, Tomorrow I'll send updated version. 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] Transactions and temp tables
Heikki, The following test fails with your patch on my system. Could you check if you can reproduce? psql (8.4devel) Type help for help. test=# begin; BEGIN test=# create table paul(x int); CREATE TABLE test=# insert into paul values(1); INSERT 0 1 test=# prepare transaction 'persistentTableShouldSucceed'; server closed the connection unexpectedly This probably means the server terminated abnormally before or while processing the request. The connection to the server was lost. Attempting reset: Failed. --- LOG: database system is ready to accept connections TRAP: FailedAssertion(!(on_commits != ((void *)0)), File: tablecmds.c, Line: 7823) LOG: server process (PID 15969) was terminated by signal 6: Aborted LOG: terminating any other active server processes FATAL: the database system is in recovery mode Thanks, manu -- 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: default values for function parameters
Peter Eisentraut [EMAIL PROTECTED] writes: On Monday 24 November 2008 23:21:15 Pavel Stehule wrote: Default parameters are implemented similar to variadic functions - so no changes on PL part - all changes are on caller part. Then I don't understand why you need this special data type instead of using an array of text with nulls for parameters without default. I'm not even sure you need to store any nulls. We're going to require defaults to be provided for the last N parameters consecutively, right? So that's just what the array contents are. Or maybe it's not an array at all but a single text item containing the representation of a List --- compare the storage of index expressions. There shouldn't be any need to read the contents of the value during function resolution; an appropriate representation will have the number of non-defaultable parameters stored as a separate integer column. 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] blatantly a bug in the documentation
On Monday 24 November 2008 09:10:29 Dave Page wrote: On Mon, Nov 24, 2008 at 2:01 PM, Magnus Hagander [EMAIL PROTECTED] wrote: Dave Page wrote: That would defeat the point. Not that I have any great feelings either way, but fwiw, Microsoft and Oracle both create a sample database iirc. Last I checked, MS did it optionally only, no? Yes, but it defaults on iirc. We can emulate that behaviour in an installer (and do in Postgres Plus for example), but that doesn't help users of other distributions. We actually have such a database on pgfoundry already (http://pgfoundry.org/frs/download.php/1719/pagila-0.10.1.zip), which i think devrim may have packaged into an rpm; it wouldn't hurt to add it to the win32 installer, but would you feel better if it were a contrib module or something? -- Robert Treat Conjecture: http://www.xzilla.net Consulting: http://www.omniti.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Comments to Synchronous replication patch v3
Fujii Masao [EMAIL PROTECTED] wrote:
On Fri, Nov 14, 2008 at 7:15 PM, Fujii Masao [EMAIL PROTECTED] wrote:
Attached is the latest version of Synch Rep patch. Sorry for my late
posting.
I fixed some bugs in v1patch and integrated walreceiver into core.
I have some comments about v3 patch.
[1] Should walsender database be real or not?
[2] User-configurable replication_timeout is dangerous
[3] How to configure wal_buffers and wal_sender_delay?
[4] XLogArchivingActive on replication mode
[5] Usage of XLog Send-Flush-Wait
[6] Bit-OR or Logical-OR
[1] Should walsender database be real or not?
Index: bin/initdb/initdb.c
+ CREATE DATABASE walsender;\n,
You create walsender as a *real* database, but is it really needed?
Can we make it a virtual database only for walreceiver?
And backend process crashes when I login the walsender database:
$ psql walsender
psql: server closed the connection unexpectedly
This probably means the server terminated abnormally
before or while processing the request.
Even if we need to have the database in real, I'd like to use another
name for it. The name 'walsender' seems to be an internal module name
but it should be a feature name (ex. 'replication').
[2] User-configurable replication_timeout is dangerous
Index: backend/utils/misc/guc.c
+ {replication_timeout, PGC_USERSET, WAL_REPLICATION,
You export replication_timeout as a PGC_USERSET variable, but it is
dangerous. It allows non-superusers to kill servers easily by setting it
too low value. Walsender dies with FATAL on timeout.
BTW, how about adding an option to choose FATAL or ERROR on timeout?
I think FATAL is too strong for some cases.
[3] How to configure wal_buffers and wal_sender_delay?
The parameter wal_buffers might be more important in synch rep,
but we don't have a mean to know whether wal_buffers is enough or not.
Should we have a new system view 'pg_stat_xlog' to find shortage
of wal_buffers and wal_sender_delay?
[4] XLogArchivingActive on replication mode
XLogArchivingActive is not modified in the patch, but is it safe?
For example, we might need to disable COPY-without-wal optimization
when replication is enabled.
[5] Usage of XLog Send-Flush-Wait
There are many following sequences:
RequestXLogSend(WriteRqstPtr, true);
XLogFlush(WriteRqstPtr);
WaitXLogSend(WriteRqstPtr, false, true);
It introduces additional complexities to use XLogFlush.
Is it possible to push RequestXLogSend and WaitXLogSend into XLogFlush?
It might require a new flag argument in XLogFlush.
[6] Bit-OR or Logical-OR
WaitXLogSend(XactLastRecEnd, false, forceSyncCommit | haveNonTemp);
should be
WaitXLogSend(XactLastRecEnd, false, forceSyncCommit || haveNonTemp);
| is bit OR and || is logical OR.
Regards,
---
ITAGAKI Takahiro
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] huge query tree cost too much time to copyObject()
Tao Ma [EMAIL PROTECTED] writes: recently, I wrote a really complex SELECT statement which consists of about 20 relations using NATURAL JOIN method and every single relation consists 50 columns. It looks like: Maybe you should rethink your schema design... It seems Postgres cost lots of time to copyObject(). You're completely wasting your time to optimize *that*. 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] Re: [COMMITTERS] pgsql: Explicitly bind gettext() to the UTF8 locale when in use.
Tom Lane wrote: Magnus Hagander [EMAIL PROTECTED] writes: Hiroshi Inoue wrote: because Shift_JIS isn't allowed as a server encoding. So the Japanese Windows native message encoding Shift_JIS never matches the server encoding EUC_JP and a conversion between Shitt_jis and EUC_JP is necessarily needed. Ah, so we're basically hardcoding that information? The system will go up in SJIS, but since we can't deal with it, we switch it to EUC_JP? I'm not following this either. If the patch is really necessary then it seems it must be working around a bug in the Windows version of gettext, ie failure to distinguish CP932 from CP20932. Is that correct? I'm afraid I don't understand what you mean exactly. AFAIK the output encoding of Windows gettext is detemined by the ANSI system code page which is usualy CP932(Shift_JIS) in Japan and unrelated to the locale settings. In addition CP20932 is rarely used in Japan IIRC. I've never used it and don't know what it is correctly (maybe a kind of EUC_JP). regards, Hiroshi Inoue -- 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: Explicitly bind gettext() to the UTF8 locale when in use.
Hiroshi Inoue [EMAIL PROTECTED] writes: Tom Lane wrote: I'm not following this either. If the patch is really necessary then it seems it must be working around a bug in the Windows version of gettext, ie failure to distinguish CP932 from CP20932. Is that correct? I'm afraid I don't understand what you mean exactly. AFAIK the output encoding of Windows gettext is detemined by the ANSI system code page which is usualy CP932(Shift_JIS) in Japan and unrelated to the locale settings. If that's true then this code is presently broken for *every* locale under Windows, not only Japanese. To my mind the really correct thing to be doing here would be to call bind_textdomain_codeset in all cases, rather than trusting gettext to guess correctly about which encoding we want. As the comment notes, we have not attempted that because the codeset names aren't well standardized. But it seems to me that we could certainly find out what codeset names are used on Windows, and apply bind_textdomain_codeset all the time on Windows. That would make a lot more sense than ad-hoc treatment of UTF-8 and EUC-JP if you ask me ... 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] Re: [COMMITTERS] pgsql: Explicitly bind gettext() to the UTF8 locale when in use.
Tom Lane wrote: Hiroshi Inoue [EMAIL PROTECTED] writes: Tom Lane wrote: I'm not following this either. If the patch is really necessary then it seems it must be working around a bug in the Windows version of gettext, ie failure to distinguish CP932 from CP20932. Is that correct? I'm afraid I don't understand what you mean exactly. AFAIK the output encoding of Windows gettext is detemined by the ANSI system code page which is usualy CP932(Shift_JIS) in Japan and unrelated to the locale settings. If that's true then this code is presently broken for *every* locale under Windows, not only Japanese. Maybe there are a few languages/countires where 2 encodings are widely used. To my mind the really correct thing to be doing here would be to call bind_textdomain_codeset in all cases, rather than trusting gettext to guess correctly about which encoding we want. As the comment notes, we have not attempted that because the codeset names aren't well standardized. But it seems to me that we could certainly find out what codeset names are used on Windows, and apply bind_textdomain_codeset all the time on Windows. That would make a lot more sense than ad-hoc treatment of UTF-8 and EUC-JP if you ask me ... I fundamentally agree with you. What we hope is to enable the use of Japanese message catalog which we gave up in 8.3 Windows-version release. regards, Hiroshi Inoue -- 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: Explicitly bind gettext() to the UTF8 locale when in use.
Hiroshi Inoue [EMAIL PROTECTED] writes: Tom Lane wrote: If that's true then this code is presently broken for *every* locale under Windows, not only Japanese. Maybe there are a few languages/countires where 2 encodings are widely used. UTF8 vs Latin-N? In any case I think the problem is that gettext is looking at a setting that is not what we are looking at. Particularly with the 8.4 changes to allow per-database locale settings, this has got to be fixed in a bulletproof way. 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] Windowing Function Patch Review - Standard Conformance
2008/11/25 Heikki Linnakangas [EMAIL PROTECTED]: Hitoshi Harada wrote: If you keep this in your mind, please don't be annoyed but the current frame concept is wrong. ... Note that on empno=4 then last_value=4(yours)/3(mine), which means the frame is applied to last_value() as well as nth_value() and first_value(). Not only to aggregates. So these lines in nodeWindow.c, Yes, you're right. Hmm, I wonder if we should implement first_value, last_value and nth_value as regular aggregates. That way, all the window functions would operate on the partition, not caring about the frame, and all the aggregates would operate on the frame. No way. Current specification that we don't get other frames than with/without ORDER BY doesn't have conflict with your proposal. However, thinking about the future, we decided to add window aggregates with wfunc='t', with subfunc for shrinking frame performance up. The direction we should head for is convert aggregates to subset of window functions, not vice versa. Then window_gettupleslot looks like killing performance in some cases. What if the partition has millions of rows and wfunc1 seeks the head and wfunc2 seeks the tail? Yeah, we probably should do something about that. You used several different read pointers to the tuplestore in your patch, one for frame head, another for frame tail, another for partition head etc., but I think that potentially suffers from the same problem. Perhaps we should have one read pointer for each window function, plus one reading the current row? It seems likely that one window function is not going to hop around wildly, and the behavior wouldn't depend so much on what other window functions are being used. Yes. My patch also has performance problem in seeking but is better than one read pointer. So I like your proposal to attach a read pointer with each wfuncs. I am not sure what kind of window functions the user will define in the future, but for current use cases it comes reasonable. Anyway, only one read pointer will make problems, I guess. So as you point it is possible to define frame and partition while scanning current rows rather than before scanning, I felt it'd cost more. But I know this is work in progress, so you probably think about the solutions. What do you think? Here's an updated patch, where the rows are fetched on-demand. Good! And I like the fetching args by number better. Let me take more time to look in detail... 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] WIP: default values for function parameters
2008/11/25 Tom Lane [EMAIL PROTECTED]: Peter Eisentraut [EMAIL PROTECTED] writes: On Monday 24 November 2008 23:21:15 Pavel Stehule wrote: Default parameters are implemented similar to variadic functions - so no changes on PL part - all changes are on caller part. Then I don't understand why you need this special data type instead of using an array of text with nulls for parameters without default. I'm not even sure you need to store any nulls. We're going to require defaults to be provided for the last N parameters consecutively, right? So that's just what the array contents are. Or maybe it's not an array at all but a single text item containing the representation of a List --- compare the storage of index expressions. There shouldn't be any need to read the contents of the value during function resolution; an appropriate representation will have the number of non-defaultable parameters stored as a separate integer column. this can be the most simple solution, I used special datatype because a) I am afraid add more columns to system tables, b) I dislike serialisation into text type, because simple select from this values returns some strange values. But maybe I am thinking and searching to much complicate solutions. I'll try to simplify patch. Regards Pavel Stehule 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] Re: [COMMITTERS] pgsql: Explicitly bind gettext() to the UTF8 locale when in use.
On 25 nov 2008, at 05.00, Tom Lane [EMAIL PROTECTED] wrote: Hiroshi Inoue [EMAIL PROTECTED] writes: Tom Lane wrote: If that's true then this code is presently broken for *every* locale under Windows, not only Japanese. Maybe there are a few languages/countires where 2 encodings are widely used. UTF8 vs Latin-N? We already special-cases utf8... I think the thing us that as long as the encodings are compatible (latin1 with different names for example) it worked fine. In any case I think the problem is that gettext is looking at a setting that is not what we are looking at. Particularly with the 8.4 changes to allow per-database locale settings, this has got to be fixed in a bulletproof way. Agreed. /Magnus -- 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] How should pg_standby get over the gap of timeline?
On Sat, Nov 22, 2008 at 6:28 PM, Simon Riggs [EMAIL PROTECTED] wrote: My worry is that there has not been an exhaustive analysis. Almost correct and probably correct is not the same thing as correct. We need to look through all of the changes that occur at the end of recovery to be certain we can do this. Luckily normal data blocks don't know anything about such state changes, so that is a good start. We must look at It's reasonable worry. Thanks a lot, Simon. I will examine it next time (probably 8.5). And, I'd like to clear up which recovery method is safe now. Althogh I think as follows, is it right? Safe (proved to be safe): - PITR with a base backup. That is, we don't always need a fresh backup when setting up, and can make the standby catch up by using an old or fresh backup. If we can use an old backup, I think it might be worth changing pg_standby to get over the gap of timeline. What is your opinion? - PITR with a database cluster including a recovery restart point. That is, we can make the standby catch up without a base backup after it fails. Not safe (further examination is needed): - PITR with a database cluster not including a recovery restart point. That is, we cannot make the standby (old primary) catch up without a base backup. 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
