Re: [HACKERS] patch (for 9.1) string functions

2010-07-08 Thread Takahiro Itagaki

Pavel Stehule pavel.steh...@gmail.com wrote:

 updated version, concat function doesn't use separator

BTW, didn't you forget stringfunc.sql.in for contrib/stringfunc ?
So, I have not check stringfunc module yet.

I reviewed your patch, and format() in the core is almost ok. It's very cool!
On the other hand, contrib/stringfunc tries to implement safe-sprintf. It's
very complex, and I have questions about multi-byte character handling in it.

* How to print NULL value.
format() function prints NULL as NULL, but RAISE statement in PL/pgSQL
does as NULL. Do we need the same result for them?

postgres=# SELECT format('% vs %', 'NULL', NULL);
format
--
 NULL vs NULL
(1 row)

postgres=# DO $$ BEGIN RAISE NOTICE '% vs %', 'NULL', NULL; END; $$;
NOTICE:  NULL vs NULL
DO

* Error messages: too few/many parameters
  For the same reason, too few/many parameters specified for format()
  might be better for the messages.

  For RAISE in PL/pgSQL:
ERROR:  too few parameters specified for RAISE
ERROR:  too many parameters specified for RAISE

* Why do you need convert multi-byte characters to wide char?
Length specifier in stringfunc_sprintf() means character length.
But is pg_encoding_mbcliplen() enough for the purpose?

* Character-length vs. disp-length in length specifier for sprintf()
For example, '%10s' for sprintf() means 10 characters in the code.
But there might be usages to format text values for display. In such
case, display length might be better for the length specifier.
How about having both s and S?
%10s -- 10 characters
%10S -- 10 disp length; we could use pg_dsplen() for the purpse.

Regards,
---
Takahiro Itagaki
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] patch (for 9.1) string functions

2010-07-08 Thread Takahiro Itagaki
2010/7/8 Pavel Stehule pavel.steh...@gmail.com:
 sorry, attached fixed patch

Make installcheck for contrib/stringfunc is broken.
Please run regression test with --enable-cassert build.
  test stringfunc   ... TRAP:
FailedAssertion(!(!lc_ctype_is_c()), File: mbutils.c, Line: 715)
  LOG:  server process (PID 15121) was terminated by signal 6: Aborted

This patch contains several functions.
- format(fmt text, VARIADIC args any)
- sprintf(fmt text, VARIADIC args any)
- concat(VARIADIC args any)
- concat_ws(separator text, VARIADIC args any)
- concat_json(VARIADIC args any)
- concat_sql(VARIADIC args any)
- rvrs(str text)
- left(str text, n int)
- right(str text, n int)

The first one is in the core, and others are in contrib/stringfunc.
But I think almost
all of them should be in the core, because users want to write portable SQLs.
Contrib modules are not always available.  Note that concat() is
supported by Oracle,
MySQL, and DB2. Also left() and right() are supported by MySQL, DB2,
and SQL Server.

Functions that depend on GUC settings should be marked as VOLATILE
instead of IMMUTABLE. I think format(), sprintf(), and all of
concat()s should be
volatile because at least timestamp depends on datestyle parameter.

concat_ws() and rvrs() should be renamed to non-abbreviated forms.
How about concat_with_sep() and reverse() ?

I think we should avoid concat_json() at the moment because there is another
development project for JSON support. The result type will be JOIN type rather
than text then.

I'm not sure usefulness of concat_sql(). Why don't you just quote all values
with quotes and separate them with comma?

 format() function prints NULL as NULL, but RAISE statement in PL/pgSQL
 does as NULL.
 I prefer just NULL.
 maybe some GUC variable
 stringfunc.null_string = 'NULL' in future??

We have some choices for NULL representation. For example, empty string,
NULL, NULL, or (null) . What will be our choice?   Each of them looks
equally reasonable for me. GUC idea is also good because we need to
mark format() as VOLATILE anyway. We have nothing to lose.

---
Takahiro Itagaki

-- 
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] Partitioning syntax

2010-07-07 Thread Takahiro Itagaki

Robert Haas robertmh...@gmail.com wrote:

 I've taken a little bit more of a look at this patch and I guess I'm
 not too happy with the design.

Thanks. I was thinking about only syntax for partitioning in the patch,
but I need more consideration about insert-aware catalog design.

 5. The use of the term partition is not very consistent.  For
 example, we use CREATE PARTITION to create a partition, but we use
 DROP TABLE to get rid of it (there is no DROP PARTITION).  I think
 that the right syntax to use here is ALTER TABLE ... ADD/DROP
 PARTITION; both Oracle and MySQL do it that way. And meanwhile
 OCLASS_PARTITION means the partitioning information associated with
 the parent table, not a partition of a parent table.

ALTER TABLE ... ADD/DROP PARTITION was discussed many times,
but I cannot solve syntax confict with ALTER TABLE ... ADD [COLUMN].
Since we can omit COLUMN, parser treats ADD PARTITION as adding
a column named PARTITION. We need to add PARTITION into the reserved
keyword list to avoid shift/reduce errors.

Do you have any better idea?

Regards,
---
Takahiro Itagaki
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: I: [HACKERS] About Our CLUSTER implementation is pessimal patch

2010-07-07 Thread Takahiro Itagaki

Leonardo F m_li...@yahoo.it wrote:

 I saw that you also changed the writing:
(snip)
 Are we sure it's 100% equivalent?

I think writetup_rawheap() and readtup_rawheap() are a little complex,
but should work as long as there are no padding between t_len and t_self
in HeapTupleData struct.

- It might be cleaner if you write the total item length
  and tuple data separately.
- (char *) tuple + sizeof(tuplen) might be more robust
  than tuple-t_self.

Here is a sample code. writetup() and readtup() will be alike.

BTW, we could have LogicalTapeReadExact() as an alias of
LogicalTapeRead() and checking the result because we have
many duplicated codes for unexpected end of data errors.


static void
writetup_rawheap(Tuplesortstate *state, int tapenum, SortTuple *stup)
{
HeapTuple   tuple = (HeapTuple) stup-tuple;
int tuplen = tuple-t_len + HEAPTUPLESIZE;

LogicalTapeWrite(state-tapeset, tapenum,
 tuplen, sizeof(tuplen));
LogicalTapeWrite(state-tapeset, tapenum,
 (char *) tuple + sizeof(tuplen),
 HEAPTUPLESIZE - sizeof(tuplen);
LogicalTapeWrite(state-tapeset, tapenum, tuple-t_data, tuple-t_len);
if (state-randomAccess)/* need trailing length word? */
LogicalTapeWrite(state-tapeset, tapenum, tuplen, 
sizeof(tuplen));

FREEMEM(state, GetMemoryChunkSpace(tuple));
heap_freetuple(tuple);
}

static void
readtup_rawheap(Tuplesortstate *state, SortTuple *stup,
int tapenum, unsigned int tuplen)
{
HeapTuple   tuple = (HeapTuple) palloc(tuplen);

USEMEM(state, GetMemoryChunkSpace(tuple));

tuple-t_len = tuplen - HEAPTUPLESIZE;
if (LogicalTapeRead(state-tapeset, tapenum,
 (char *) tuple + 
sizeof(tuplen),
HEAPTUPLESIZE - sizeof(tuplen)) != HEAPTUPLESIZE - 
sizeof(tuplen))
elog(ERROR, unexpected end of data);
tuple-t_data = (HeapTupleHeader) ((char *) tuple + HEAPTUPLESIZE);
if (LogicalTapeRead(state-tapeset, tapenum,
tuple-t_data, tuple-t_len) != 
tuple-t_len)
elog(ERROR, unexpected end of data);
if (state-randomAccess)/* need trailing length word? */
if (LogicalTapeRead(state-tapeset, tapenum, tuplen,
sizeof(tuplen)) != 
sizeof(tuplen))
elog(ERROR, unexpected end of data);


Regards,
---
Takahiro Itagaki
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] patch: preload dictionary new version

2010-07-07 Thread Takahiro Itagaki

Pavel Stehule pavel.steh...@gmail.com wrote:

 this version has enhanced AllocSet allocator - it can use a  mmap API.

I review your patch and will report some comments. However, I don't have
test cases for the patch because there is no large dictionaries in the
default postgres installation. I'd like to ask you to supply test data
for the patch.

This patch allocates memory with non-file-based mmap() to preload text search
dictionary files at the server start. Note that dist files are not mmap'ed
directly in the patch; mmap() is used for reallocatable shared memory.

The dictinary loader is also modified a bit to use simple_alloc() instead
of palloc() for long-lived cache. It can reduce calls of AllocSetAlloc(),
that have some overheads to support pfree(). Since the cache is never
released, simple_alloc() seems to have better performance than palloc().
Note that the optimization will also work for non-preloaded dicts.

=== Questions ===
- How do backends share the dict cache? You might expect postmaster's
  catalog is inherited to backends with fork(), but we don't use fork()
  on Windows.

- Why are SQL functions dpreloaddict_init() and dpreloaddict_lexize()
  defined but not used?

=== Design ===
- You added 3 custom parameters (dict_preload.dictfile/afffile/stopwords),
  but I think text search configuration names is better than file names.
  However, it requires system catalog access but we cannot access any
  catalog at the moment of preloading. If config-name-based setting is
  difficult, we need to write docs about where we can get the dict names
  to be preloaded instead. (from \dFd+ ?)

- Do we need to support multiple preloaded dicts? I think dict_preload.*
  should accept a list of items to be loaded. GUC_LIST_INPUT will be a help.

- Server doesn't start when I added dict_preload to
  shared_preload_libraries and didn't add any custom parameters.
FATAL:  missing AffFile parameter
  But server should start with no effects or print WARNING messages
  for no dicts are preloaded in such case.

- We could replace simple_alloc() to a new MemoryContextMethods that
  doesn't support pfree() but has better performance. It doesn't look
  ideal for me to implement simple_alloc() on the top of palloc().

=== Implementation ===
I'm sure that your patch is WIP, but I'll note some issues just in case.

- We need Makefile for contrib/dict_preload.

- mmap() is not always portable. We should check the availability
  in configure, and also have an alternative implementation for Win32.


Regards,
---
Takahiro Itagaki
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: I: [HACKERS] About Our CLUSTER implementation is pessimal patch

2010-07-06 Thread Takahiro Itagaki

Leonardo F m_li...@yahoo.it wrote:

 Attached the updated patch (should solve a bug) and a script.

I reviewed your patch. It seems to be in good shape, and worked as
expected. I suppressed a compiler warning in the patch and cleaned up
whitespaces in it. Patch attached.

I think we need some documentation for the change. The only downside
of the feature is that sorted cluster requires twice disk spaces of
the target table (temp space for disk sort and the result table).
Could I ask you to write documentation about the new behavior?
Also, code comments can be improved; especially we need better
description than copypaste from FormIndexDatum.

Regards,
---
Takahiro Itagaki
NTT Open Source Software Center



sorted_cluster-20100706.patch
Description: Binary data

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Does mbutils.c really need to use L'\0' ?

2010-07-06 Thread Takahiro Itagaki

Tom Lane t...@sss.pgh.pa.us wrote:

  I'm dubious that it's worth the trouble.  I suggest that it might be
  best to replace these usages of L'\0' by plain scalar 0.
 I'd tend to go with just 0,
 which is a reasonably common substitute for non-wide '\0' ...

I think all of the following codes work in the same way
at least on Windows, where the codes are actually used.

utf16[dstlen] = L'\0';
utf16[dstlen] = '\0';
utf16[dstlen] = 0;
utf16[dstlen] = (WCHAR) 0;

Regards,
---
Takahiro Itagaki
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] Always truncate segments before unlink

2010-07-05 Thread Takahiro Itagaki

Tom Lane t...@sss.pgh.pa.us wrote:
 Truncating seems like an ugly kluge that's not fixing the real problem.
 Why are there open descriptors for a dropped relation?  They should all
 get closed as a consequence of relcache flush.

Relcache will be flushed at the next command, but there could be some
*idle backends* kept by connection pooling. They won't close dropped files
until shared cache invalidation queue are almost filled, that might take
long time.

There might be another solution that we send PROCSIG_CATCHUP_INTERRUPT
signal not only on the threshold of queue length but also on timeout,
where the signal is sent when we have some old messages in the queue
longer than 30sec - 1min.

Regards,
---
Takahiro Itagaki
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


[HACKERS] Always truncate segments before unlink

2010-07-04 Thread Takahiro Itagaki
I have a report from an user that postgres server gave up REINDEX
commands on the almost-disk-full machine. The disk spaces were
filled with old index segments, that should be replaced with
re-constructed files made by the REINDEX.

In mdunlink(), we truncate the first main fork to zero length
and actually unlink at the next checkpoint, but other segments
are not truncated and only unlinked. Then, if another backend
open the segments, disk spaces occupied by them are not reclaimed
until all of the backends close their file descriptors. Longer
checkpoint timeout and connection pooling make things worse.

I'd like to suggest that we always truncate any segments before
unlink them. The truncate-and-unlink hack seems to be developed
to avoid reuse of relfilenode:
| Leaving the empty file in place prevents that relfilenode
| number from being reused.
but is also useful to release disk spaces in the early stages.

Am I missing something? Comments welcome.

Regards,
---
Takahiro Itagaki
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] Additional startup logging

2010-06-30 Thread Takahiro Itagaki

Kevin Grittner kevin.gritt...@wicourts.gov wrote:

 It seems potentially useful to LOG the version() string in the log
 file during startup.  It might also help to LOG any settings which
 might result in the loss of committed transactions or in database
 corruption during startup.  (After a crash, the postgresql.conf file
 might not show the values which were in effect during startup, and
 it is too late to show the values.)

I think such logs depends on purposes, so they should be customizable.

You could write a module, that is registered in 'shared_preload_libraries'
and logs internal information you want from _PG_init() or shmem_startup_hook.

Regards,
---
Takahiro Itagaki
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] system views for walsender activity

2010-06-21 Thread Takahiro Itagaki

Tom Lane t...@sss.pgh.pa.us wrote:

 I'm of the opinion that this is a 9.1 problem.  It needs more thought
 than we can put into it now --- one obvious question is what about
 monitoring on the slave side?  Another is who should be able to see the
 data?

Sure. We should research user's demands for monitoring and management
of replication. I'll report some voices from users as of this moment:

* Managers often ask DBAs How long standby servers are behind the master?
  We should provide such methods for DBAs. We have pg_xlog_location()
  functions, but they should be improved for:
- The returned values are xxx/yyy texts, but more useful information
  is the difference of two values. Subtraction functions are required.
- For easier management, the master server should provide not only
  sent/flush locations but also received/replayed locations for each
  standby servers. Users don't want to access both master and slaves.

* Some developers want to pause and restart replication from the master
  server. They're going to use replication for application version
  managements. They'll pause all replications, and test their new features
  at the master, and restart replication to spread the changes to slaves.

Regards,
---
Takahiro Itagaki
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] Partitioning syntax

2010-06-17 Thread Takahiro Itagaki

Jaime Casanova ja...@2ndquadrant.com wrote:

 This one, doesn't apply to head anymore... please update

Thank you for reviewing my patch!

I attached an updated patch set for partitioning syntax.

The latest codes are available at: http://repo.or.cz/w/pgsql-fdw.git
(I'm recycling FDW repo for the feature.)
* master branch is a copy of postgres' HEAD.
* 'partition' branch contains codes for partitioning. 

The details and discussion for partitioning are in the wiki page:
http://wiki.postgresql.org/wiki/Table_partitioning

Regards,
---
Takahiro Itagaki
NTT Open Source Software Center



partition_20100618.tar.gz
Description: Binary data

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


[HACKERS] system views for walsender activity

2010-06-17 Thread Takahiro Itagaki
Hi,

We don't have any statistic views for walsenders in SR's master server
in 9.0, but such views would be useful to monitor and manage standby
servers from the master server. I have two ideas for the solution -
adding a new system view or recycling pg_stat_activity:

1. Add another system view for walsenders, ex. pg_stat_replication.
   It would show pid, remote host, and sent location for each walsender.

2. Make pg_stat_activity to show walsenders. We could use current_query
   to display walsender-specific information, like:
=# SELECT * FROM my_stat_activity ;
-[ RECORD 1 ]+-
datid| 16384
snip
current_query| SELECT * FROM my_stat_activity ;
-[ RECORD 2 ]+-
datid| 0
datname  |
procpid  | 4300
usesysid | 10
usename  | itagaki
application_name |
client_addr  | ::1
client_port  | 37710
backend_start| 2010-06-16 16:47:35.646486+09
xact_start   |
query_start  |
waiting  | f
current_query| walsender: sent=0/701AAA8

The attached patch is a WIP codes for the case 2, but it might be
better to design management policy via SQL in 9.1 before such detailed
implementation.  Comments welcome.

Regards,
---
Takahiro Itagaki
NTT Open Source Software Center


walsender_activity-20100618.patch
Description: Binary data

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


[HACKERS] GetOldestWALSendPointer() in header

2010-06-16 Thread Takahiro Itagaki
Hi,

GetOldestWALSendPointer() is commented out in the source code
with NOT_USED block, but is still declared in the header file.
Should we remove the function prototype from walsender.h ?

[walsender.h]
extern XLogRecPtr GetOldestWALSendPointer(void);

Regards,
---
Takahiro Itagaki
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] debug log in pg_archivecleanup

2010-06-15 Thread Takahiro Itagaki

Fujii Masao masao.fu...@gmail.com wrote:

 This is because pg_archivecleanup puts the line break \n in the head of
 debug message. Why should we do so?
 
 ---
  if (debug)
 fprintf(stderr, \n%s:  removing \%s\, progname, WALFilePath);
 ---

We also need \n at line 308.
  L.125: fprintf(stderr, \n%s:  removing \%s\, progname, WALFilePath);
  L.308: fprintf(stderr, %s:  keep WAL file %s and later, progname, 
exclusiveCleanupFileName);

Note that we don't need a line break at Line 130
because strerror() fills the last %s.
  L.130: fprintf(stderr, \n%s: ERROR failed to remove \%s\: %s,

Regards,
---
Takahiro Itagaki
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] ps display waiting for max_standby_delay

2010-06-10 Thread Takahiro Itagaki

Bruce Momjian br...@momjian.us wrote:

  how about showing actual waiting time instead? 
waiting for max_standby_delay (%d ms),
   MaxStandbyDelay)
 
 Sounds interesting, but how often would the ps statust display be
 updated?  I hope not too often.

We can change the interval of updates to 500ms or so if do it,
but I rethink ps display is not the best place for the information.

I'd like to modify the additonal message waiting for max_standby_delay
just to waiting, because we don't use waiting for statement_timeout
for normal queries.

If we need additional information about conflictions in recovery,
we would supply them with SQL views instead of ps display in 9.1.

Regards,
---
Takahiro Itagaki
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] LLVM / clang

2010-06-10 Thread Takahiro Itagaki

Peter Eisentraut pete...@gmx.net wrote:

 Some new warnings, however:
 
 xlog.c:7759:22: warning: self-comparison always results in a constant
 value
 max_locks_per_xact != max_locks_per_xact)
^
 
 Looks like a bug.

Ah, it should be compared with the same name field in ControlFile.

Index: src/backend/access/transam/xlog.c
===
--- src/backend/access/transam/xlog.c   (HEAD)
+++ src/backend/access/transam/xlog.c   (fixed)
@@ -7756,7 +7756,7 @@
if (wal_level != ControlFile-wal_level ||
MaxConnections != ControlFile-MaxConnections ||
max_prepared_xacts != ControlFile-max_prepared_xacts ||
-   max_locks_per_xact != max_locks_per_xact)
+   max_locks_per_xact != ControlFile-max_locks_per_xact)
{
/*
 * The change in number of backend slots doesn't need to be



Regards,
---
Takahiro Itagaki
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] failover vs. read only queries

2010-06-09 Thread Takahiro Itagaki

Fujii Masao masao.fu...@gmail.com wrote:

 To fix the problem, when the trigger file is found, I think
 that we should cancel all the running read only queries
 immediately (or forcibly use -1 as the max_standby_delay
 since that point) and make the recovery go ahead.

Hmmm, does the following sequence work as your expect instead of the chanage?
It requires text-file manipulation in 1, but seems to be more flexible.

  1. Reset max_standby_delay = 0 in postgresql.conf
  2. pg_ctl reload
  3. Create a trigger file

BTW, I hope we will have pg_ctl failover --timeout=N in 9.1
instead of the trigger file based management.

Regards,
---
Takahiro Itagaki
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] failover vs. read only queries

2010-06-09 Thread Takahiro Itagaki

Fujii Masao masao.fu...@gmail.com wrote:

  1. Reset max_standby_delay = 0 in postgresql.conf
  2. pg_ctl reload
  3. Create a trigger file
 
 As far as I read the HS code, SIGHUP is not checked while a recovery
 is waiting for queries :(  So pg_ctl reload would have no effect on
 the conflicting queries.
 
 Independently from the problem I raised, I think that we should call
 HandleStartupProcInterrupts() in that sleep loop.

Hmmm, if reload doesn't work, can we write a query like below?

  SELECT pg_terminate_backend(pid)
FROM pg_locks
   WHERE conflicted-with-recovery-process;

Regards,
---
Takahiro Itagaki
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


[HACKERS] InvalidXLogRecPtr in docs

2010-06-09 Thread Takahiro Itagaki
I found a term InvalidXLogRecPtr in 9.0 docs.
http://developer.postgresql.org/pgdocs/postgres/functions-admin.html#FUNCTIONS-RECOVERY-INFO-TABLE
| ... then the return value will be InvalidXLogRecPtr (0/0). 

I think it should not appear in docs because it's a name for an internal
constant variable. I'd like to rewrite the description like:

... then the return value will be 0/0, that is never used in normal cases.

Comments?

Regards,
---
Takahiro Itagaki
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] [COMMITTERS] pgsql: Fix dblink to treat connection names longer than NAMEDATALEN-2

2010-06-08 Thread Takahiro Itagaki

itag...@postgresql.org (Takahiro Itagaki) wrote:

 Fix dblink to treat connection names longer than NAMEDATALEN-2 (62 bytes).
 Now long names are adjusted with truncate_identifier() and NOTICE messages
 are raised if names are actually truncated.
 
 Modified Files:
 --
 pgsql/contrib/dblink:
 dblink.c (r1.91 - r1.92)
 
 (http://anoncvs.postgresql.org/cvsweb.cgi/pgsql/contrib/dblink/dblink.c?r1=1.91r2=1.92)

I had a mistake in the fix below. get_connect_string() is called
not only in the case of server-based connections, but also in
connection-string-based connections. If connection strings longer
than 63 bytes are passed the routine emits identifier will be truncated
warning. It is an incompatbile change.

Should we revert the change, or just adjust to call truncate_identifier
with warn=false ?


*** get_connect_string(const char *servernam
*** 2390,2399 
StringInfo  buf = makeStringInfo();
ForeignDataWrapper *fdw;
AclResult   aclresult;
  
/* first gather the server connstr options */
!   if (strlen(servername)  NAMEDATALEN)
!   foreign_server = GetForeignServerByName(servername, true);
  
if (foreign_server)
{
--- 2390,2401 
StringInfo  buf = makeStringInfo();
ForeignDataWrapper *fdw;
AclResult   aclresult;
+   char   *srvname;
  
/* first gather the server connstr options */
!   srvname = pstrdup(servername);
!   truncate_identifier(srvname, strlen(srvname), true);
!   foreign_server = GetForeignServerByName(srvname, true);
  
if (foreign_server)
{



Regards,
---
Takahiro Itagaki
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


[HACKERS] Connection leak in dblink on duplicate names

2010-06-08 Thread Takahiro Itagaki
I found contrib/dblink leaks a connection or a small amout of memory
when dblink_connect() ends with a duplicate connection name error.
We should disconnect the connection before raise any ERRORs.

Patch attached.

Regards,
---
Takahiro Itagaki
NTT Open Source Software Center


dblink-connection-leak.patch
Description: Binary data

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Command to prune archive at restartpoints

2010-06-08 Thread Takahiro Itagaki

Robert Haas robertmh...@gmail.com wrote:

 On Tue, Jun 8, 2010 at 6:45 PM, Tom Lane t...@sss.pgh.pa.us wrote:
  Andrew Dunstan and...@dunslane.net writes:
  I prefer archive_cleanup_command. We should name things after their
  principal function, not an implementation detail, IMNSHO.
 
  Weak preference for archive_cleanup_command here.
 
 OK, sounds like we have consensus on that.  Who wants to do it?

Do we just need to replace all of them? If so, patch attached.
I replaced 3 terms: recovery_end_command, recovery-end-command,
and recoveryEndCommand.


Regards,
---
Takahiro Itagaki
NTT Open Source Software Center



archive_cleanup_command.patch
Description: Binary data

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Command to prune archive at restartpoints

2010-06-08 Thread Takahiro Itagaki

Robert Haas robertmh...@gmail.com wrote:
 I think we're replacing restartpoint_command, not recovery_end_command.

Ah, sorry. I did the same replacement for restartpoint_command
in _, -, and camel case words.

BTW, should we also have a release note for the command?
I added a simple description for it in the patch.

Regards,
---
Takahiro Itagaki
NTT Open Source Software Center



estartpoint-to-archive_cleanup.patch
Description: Binary data

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] pgstatindex still throws ERROR: value 3220078592 is out of range for type integer

2010-06-07 Thread Takahiro Itagaki

Dave Cramer p...@fastcrypt.com wrote:

 I noted on line 169 that max_avail is still an int ? Where else would
 it be having problems ?

It should not a problem because the local variable only stores byte
size in a page. It will be at most only BLCKSZ (=8192).

I wonder why you had ERROR: value ... is out of range for type integer
message because we don't use any integer data types for sizes in
pgstatindex. The error should have been thrown by SQL typin functions
rather than C routines.

CREATE OR REPLACE FUNCTION pgstatindex(IN relname text,
OUT version INT,
OUT tree_level INT,
OUT index_size BIGINT,
OUT root_block_no BIGINT,
OUT internal_pages BIGINT,
OUT leaf_pages BIGINT,
OUT empty_pages BIGINT,
OUT deleted_pages BIGINT,
OUT avg_leaf_density FLOAT8,
OUT leaf_fragmentation FLOAT8)
AS 'MODULE_PATHNAME', 'pgstatindex'
LANGUAGE C STRICT;

Regards,
---
Takahiro Itagaki
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


[HACKERS] ps display waiting for max_standby_delay

2010-06-06 Thread Takahiro Itagaki
Hi,

We have codes to change ps display for recovery process during hot standby.
The current code always shows max_standby_delay for the message, but how
about showing actual waiting time instead? Since DBAs can always get the
parameter from postgresql.conf they wrote, so the parameter value itself
is not so useful. Actual waiting time might be more useful to determine
which values to be set to max_standby_delay, no?

[backend/storage/ipc/standby.c]
snprintf(new_status + len, 50,
  waiting for max_standby_delay (%d ms),
 MaxStandbyDelay);  == GetCurrentTimestamp() - waitStart
set_ps_display(new_status, false);

I think SQL-based activity view will be more useful than ps display,
but it's an item for 9.1.

Regards,
---
Takahiro Itagaki
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


[HACKERS] Open item: slave to standby in docs

2010-06-03 Thread Takahiro Itagaki
Ther is an open item:
Standby instead of slave in documentation
http://archives.postgresql.org/message-id/1273682033.12754.1.ca...@vanquo.pezone.net

I replacesd almost all slave to standby or standby servers
not only in HS+SR but also in other places like third-party tools.

There are still 3 places where slave is used.
  - Terminology: ... are called standby or slave servers.
  - Words in old release notes for 8.2 and 8.4.

Could you check those replacements are proper?

Regards,
---
Takahiro Itagaki
NTT Open Source Software Center


slave_to_standby.patch
Description: Binary data

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] fillfactor gets set to zero for toast tables

2010-06-03 Thread Takahiro Itagaki

Takahiro Itagaki itagaki.takah...@oss.ntt.co.jp wrote:

 Could you explain why default_only entries idea is
 better than adjusting those fields in the toast-specific codes?
 It's my understanding that reloption-framework is just a tool to fill
 reloption parameters, and it's not responsible for unused fields.

Here is my proposal to fix the issue. I didn't introduce default_only
field but simply adjust parameters for TOAST reloptions.


BTW, not only heap and toast relations but also btree, hash, and gist
indexes use StdRdOptions. However, they actually don't support autovacuum
options. So, StdRdOptions is a bloated all-in-one descriptor now.
Instead, they should use fillfactor-only reloptions that is defined
separately from options for heap.

Regards,
---
Takahiro Itagaki
NTT Open Source Software Center



toast-default-only-relopts.patch
Description: Binary data

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] [BUGS] BUG #5487: dblink failed with 63 bytes connection names

2010-06-02 Thread Takahiro Itagaki

Heikki Linnakangas heikki.linnakan...@enterprisedb.com wrote:

 Hmm, seems that dblink should call truncate_identifier() for the 
 truncation, to be consistent with truncation of table names etc.

Hmmm, we need the same routine with truncate_identifier(), but we hard
to use the function because it modifies the input buffer directly.
Since all of the name strings in dblink is const char *, I added
a bit modified version of the function as truncate_identifier_copy()
in the attached v2 patch.

 I also spotted this in dblink.c:
 
  /* first gather the server connstr options */
  if (strlen(servername)  NAMEDATALEN)
  foreign_server = GetForeignServerByName(servername, true);
 
 I think that's wrong. We normally consistently truncate identifiers at 
 creation and at use, so that if you create an object with a very long 
 name and it's truncated, you can still refer to it with the untruncated 
 name because all such references are truncated too.

Absolutely. I re-use the added function for the fix.

Regards,
---
Takahiro Itagaki
NTT Open Source Software Center



dblink_63bytes-2010602.patch
Description: Binary data

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] [BUGS] BUG #5487: dblink failed with 63 bytes connection names

2010-06-02 Thread Takahiro Itagaki

Heikki Linnakangas heikki.linnakan...@enterprisedb.com wrote:

 Well, looking at the callers, most if not all of them seem to actually 
 pass a palloc'd copy, allocated by text_to_cstring().
 
 Should we throw a NOTICE like vanilla truncate_identifier() does?
 
 I feel it would be better to just call truncate_identifier() than roll 
 your own. Assuming we want the same semantics in dblink, we'll otherwise 
 have to remember to update truncate_identifier_copy() with any changes 
 to truncate_identifier().

That makes sense. Now I use truncate_identifier(warn=true) for the fix.

Regards,
---
Takahiro Itagaki
NTT Open Source Software Center



dblink_63bytes-2010-603.patch
Description: Binary data

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] fillfactor gets set to zero for toast tables

2010-05-31 Thread Takahiro Itagaki

Tom Lane t...@sss.pgh.pa.us wrote:

 Alvaro Herrera alvhe...@commandprompt.com writes:
  Do we really need default_only entries in user-defined reloptions?

I think we don't, but I also think we don't need it at all even in the
core because it just set a few variables to the default values with
complex code flow. Could you explain why default_only entries idea is
better than adjusting those fields in the toast-specific codes?
It's my understanding that reloption-framework is just a tool to fill
reloption parameters, and it's not responsible for unused fields.

  We have yet to see any indication that anybody is using user-defined
  reloptions at all ...  It'd be good to have an use case at least (if
  only to ensure that the API we're providing is sufficient).

I use it my textsearch_senna extension :-).
But I don't need default_only entries at this time.

 I suggest that 9.0 would be a good time to add an int flags parameter
 to the add_xxx_reloption functions.  The first flag could be
 default_only and we'd have room to add more later without another API
 break.

I agree the idea when we reach a conclusion to introduce default_only.

Regards,
---
Takahiro Itagaki
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] [BUGS] BUG #5487: dblink failed with 63 bytes connection names

2010-05-31 Thread Takahiro Itagaki

Takahiro Itagaki itagaki.takah...@oss.ntt.co.jp wrote:

 Bug reference:  5487
 Logged by:  Takahiro Itagaki
 Email address:  itagaki.takah...@oss.ntt.co.jp
 Description:dblink failed with 63 bytes connection names
 Details: 
 
 Contib/dblink module seems to have a bug in handling
 connection names in NAMEDATALEN-1 bytes.

Here is a patch to fix the bug. I think it comes from wrong usage
of snprintf(NAMEDATALEN - 1). It just copies 62 bytes + \0.

In addition, it should be safe to use pg_mbcliplen() to truncate
extra bytes in connection names because we might return invalid
text when a multibyte character is at 62 or 63 bytes.

Note that the fix should be ported to previous versions, too.


 It cannot use exiting connections with 63 bytes name
 in some cases. For example, we cannot disconnect
 such connections. Also, we can reconnect with the
 same name and will have two connections with the name.
 
 =# SELECT dblink_connect(repeat('1234567890', 6) || 'ABC',
 'host=localhost');
  dblink_connect
 
  OK
 (1 row)
 
 =# SELECT dblink_get_connections();
   dblink_get_connections
 ---
  {123456789012345678901234567890123456789012345678901234567890ABC}
 (1 row)
 
 =# SELECT dblink_disconnect(repeat('1234567890', 6) || 'ABC');
 ERROR:  connection
 123456789012345678901234567890123456789012345678901234567890ABC not
 available

Regards,
---
Takahiro Itagaki
NTT Open Source Software Center



dblink_63bytes.patch
Description: Binary data

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] fillfactor gets set to zero for toast tables

2010-05-30 Thread Takahiro Itagaki
This is still an open item for 9.0, and we should also backport it to 8.4.
Which do we take on? Or is there better idea?

Alvaro's idea:
   Maybe a better solution is to have some kind of notion of a default-only
   entry, which is sufficient to insert the default into the struct but
   isn't accepted as a user-settable item.

Itagaki's idea:
 It just fills fillfactor (and analyze_threshold)
 to default values for TOAST relations.

Regards,
---
Takahiro Itagaki
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] Unexpected page allocation behavior on insert-only tables

2010-05-30 Thread Takahiro Itagaki

Tom Lane t...@sss.pgh.pa.us wrote:

 3. After awhile, autovacuum notices all the insert activity and kicks
 off an autoanalyze on the bid table.  When committed, this forces a
 relcache flush for each other backend's relcache entry for bid.
 In particular, the smgr targblock gets reset.
 
 4. Now, all the backends again decide to try to insert into the last
 available block.  So everybody jams into the partly-filled block 10,
 until it gets filled.

The autovacuum process runs only analyze, but does not run vacuum at 3
because the workload is insert-only. Partially filled pages are never
tracked by freespace map. We could re-run an autovacuum if we saw the
report from the autoanalyze that says the table is low-density,
but the additional vacuum might be overhead in other cases.

 The obvious thing to do about this would be to not reset targblock
 on receipt of a relcache flush event

Even if we don't reset targblock, can we solve the issue when clients
connect and disconnect for each insert? New backends only check the end
of the table, and extend it as same as this case. If we are worrying
about the worst caase, we might need to track newly added pages with
freespace map. Of course we can ignore the case because frequent
connections and disconnections should be always avoided.

Regards,
---
Takahiro Itagaki
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] mingw initdb failure on HEAD

2010-05-27 Thread Takahiro Itagaki

Andrew Dunstan and...@dunslane.net wrote:

 Several buildfarm mingw members are getting failures like this, when 
 running initdb:
 http://www.pgbuildfarm.org/cgi-bin/show_log.pl?nm=dawn_batdt=2010-05-27%2019:45:18
 
 Could it have been caused by the PGDLLIMPORT/PGDLLEXPORT changes?

Probably, but it's curious because MSVC members are OK.
Do we have special treatments for exported functions in mingw?
It might export 'dllimport' funtions/variables, but not 'dllexport' ones.

Regards,
---
Takahiro Itagaki
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] fillfactor gets set to zero for toast tables

2010-05-26 Thread Takahiro Itagaki

Alvaro Herrera alvhe...@commandprompt.com wrote:

 Excerpts from Tom Lane's message of vie may 14 15:03:57 -0400 2010:
 
  Maybe a better solution is to have some kind of notion of a default-only
  entry, which is sufficient to insert the default into the struct but
  isn't accepted as a user-settable item.
 
 This patch (for 8.4, but applies fuzzily to 9.0) implements this idea.
 Note that there's no explicit check that every heap option has a
 corresponding toast option; that's left to the developer's judgement to
 add.  I added the new member to relopt_gen struct so that existing
 entries did not require changes in initializers.

The new default_only field can be initialized only from the internal codes
and is not exported to user definded reloptions. We could add an additional
argument to add_xxx_reloption() functions, but it breaks ABI.

How about the attached patch? It just fills fillfactor (and analyze_threshold)
to default values for TOAST relations. I think responsibility for filling
reloptions with proper values is not in the generic option routines but in
AM-specific reloption handlers.

Regards,
---
Takahiro Itagaki
NTT Open Source Software Center



toast-ff-fix.patch
Description: Binary data

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] exporting raw parser

2010-05-26 Thread Takahiro Itagaki

Tatsuo Ishii is...@postgresql.org wrote:

 I'm thinking about exporting the raw parser and related modules as a C
 library. Though this will not be an immediate benefit of PostgreSQL
 itself, it will be a huge benefit for any PostgreSQL
 applications/middle ware those need to parse SQL statements.

I read your proposal says postgres.exe will link to libSQL.dll,
and pgpool.exe will also link to the DLL, right?

I think it is reasonable, but I'm not sure what part of postgres
should be in the DLL. Obviously we should avoid code duplication
between the DLL and postgres.exe.

 - create an exportable version of memory manager
 - create an exportable exception handling routines(i.e. elog)

Are there any other issues? For example,
  - How to split headers for raw parser nodes?
  - Which module do we define T_xxx enumerations and support functions?
(outfuncs, readfuncs, copyfuncs, and equalfuncs)

The proposal will be acceptable only when all of the technical issues
are solved. The libSQL should also be available in stand-alone.
It should not be a collection of half-baked functions.

Regards,
---
Takahiro Itagaki
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] exporting raw parser

2010-05-26 Thread Takahiro Itagaki

Tatsuo Ishii is...@sraoss.co.jp wrote:

  The proposal will be acceptable only when all of the technical issues
  are solved. The libSQL should also be available in stand-alone.
  It should not be a collection of half-baked functions.
 
 What do you mean by should also be available in stand-alone? If you
 want more abstract API than libSQL, you could invent such a thing
 based on it as much as you like. IMO anything need to parse/operate
 the raw parse tree should be in libSQL.

My stand-alone means libSQL can be used from many modules
without duplicated codes. For example, copy routines for raw
parse trees should be in the DLL rather than in postgres.exe.

Then, we need to consider other products than pgpool. Who will
use the dll? If pgpool is the only user, we might not allow to
modify core codes only for one usecase. More research other than
pgpool is required to decide the interface routines for libSQL.

Regards,
---
Takahiro Itagaki
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] pg_stat_transaction patch

2010-05-25 Thread Takahiro Itagaki

Joel Jacobson j...@gluefinance.com wrote:

 I applied all the changes on 9.0beta manually and then it compiled without
 any assertion failures.
 
 I also changed the oids to a different unused range, since the ones I used
 before had been taken in 9.0beta1.

Thanks, but you still need to test your patch:

 - You need to check your patch with make check, because it requires
   adjustments in rule test; Your pg_stat_transaction_function is the
   longest name in the system catalog.

 - You need to configure postgres with --enable-cassert to enable internal
   varidations. The attached test case failed with the following TRAP.
TRAP: FailedAssertion(!(entry-trans == ((void *)0)), File: pgstat.c, Line: 
715)
TRAP: FailedAssertion(!(tabstat-trans == trans), File: pgstat.c, Line: 
1758)

 I suspect it is because get_tabstat_entry for some reason returns NULL, in
 for example pg_stat_get_transaction_tuples_inserted(PG_FUNCTION_ARGS).
 
 Does the function look valid? If you can find the error in it, the other
 functions probably have the same problem.

For the above trap, we can see the comment:
/* Shouldn't have any pending transaction-dependent counts */
We don't expect to read stats entries during transactions. I'm not sure
whether accessing transitional stats during transaction is safe or not.

We might need to go other directions, for example:
  - Use session stats instead transaction stats. You can see the same
information in difference of counters between before and after the
transaction.
  - Export pgBufferUsage instead of relation counters. They are
buffer counters for all relations, but we can obviously export
them because they are just plain variables.

Regards,
---
Takahiro Itagaki
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


[HACKERS] Open Item: pg_controldata - machine readable?

2010-05-25 Thread Takahiro Itagaki
There is an open item pg_controldata - machine readable? in the list:
http://wiki.postgresql.org/wiki/PostgreSQL_9.0_Open_Items

The proposal by Joe Conway is adding a new contib module.
http://archives.postgresql.org/message-id/4b959d7a.6010...@joeconway.com
http://github.com/jconway/pg_controldata

Should we add the module to 9.0? If we do so, SGML documentation is required.

IMHO, I'd like to put the feature into the core instead of a contrib
module, but we cannot change the catalog version in this time.
So, how about providing control file information through pg_settings
view? We will retrieve those variables as GUC options.

Regards,
---
Takahiro Itagaki
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


[HACKERS] Open Item: invalid declspec for PG_MODULE_MAGIC

2010-05-25 Thread Takahiro Itagaki
This open item is for replacing PGDLLIMPORT markers for PG_MODULE_MAGIC
and PG_FUNCTION_INFO_V1 to __declspec(dllexport) because they are always
expored by user modules rather than by the core codes.
http://archives.postgresql.org/message-id/20100329184705.a60e.52131...@oss.ntt.co.jp

The fix is simple, so I think we can include it to 9.0.
Arguable issues for the patch are:
  * Are there better name than PGMODULEEXPORT?  I like PGDLLEXPORT
because it is similar to PGDLLIMPORT, but it might be too similar.
  * Should we backport the fix to previous releases?
I'd like to backport it because it should not break any existing
third party modules because they cannot be even built on Windows.

Regards,
---
Takahiro Itagaki
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] Open Item: pg_controldata - machine readable?

2010-05-25 Thread Takahiro Itagaki

Joe Conway m...@joeconway.com wrote:

  There is an open item pg_controldata - machine readable? in the list:
  http://wiki.postgresql.org/wiki/PostgreSQL_9.0_Open_Items
  Should we add the module to 9.0?
  
  No.  This is a new feature that wasn't even under consideration,
  let alone written, at the time of 9.0 feature freeze.  It does not
  get into either core or contrib this time around.
 
 Yup, agreed. That was why I put it on github instead of sending a patch
 to the list. It was also a quick and dirty hack -- maybe it could be
 cleaned up for 9.1, but I'm not sure there was consensus that it was
 really needed.

OK, I moved it from 9.0 open items to new features for 9.1.

Regards,
---
Takahiro Itagaki
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] (9.1) btree_gist support for searching on not equals

2010-05-24 Thread Takahiro Itagaki

Marko Tiikkaja marko.tiikk...@cs.helsinki.fi wrote:

 On 5/21/10 11:47 PM +0300, Jeff Davis wrote:
  It also allows you to enforce the constraint that only one tuple exists
  in a table by doing something like:
 
 create table a
 (
   i int,
   exclude using gist (i with),
   unique (i)
 );

+1.  I've not read the code, but it might be considerable that we can
abort index scans if we find a first index entry for i. While we must
scan all candidates for WHERE i  ?, but we can abort for the constraint
case because we know existing values are all the same.

 FWIW, this is achievable a lot more easily:
 CREATE UNIQUE INDEX a_single_row ON a ((1));

The former exclusion constraint means one same value for all rows,
but your alternative means a_single_row, right?

Regards,
---
Takahiro Itagaki
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] Japanies translation breaks solaris build

2010-05-14 Thread Takahiro Itagaki

Zdenek Kotala zdenek.kot...@sun.com wrote:

 http://www.pgbuildfarm.org/cgi-bin/show_log.pl?nm=comet_mothdt=2010-05-13%2021:06:01
 The problem is that it contains mix of DOS/Unix end of lines.

I removed two CRs in ja.po.

Regards,
---
Takahiro Itagaki
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


[HACKERS] pg_upgrade code questions

2010-05-13 Thread Takahiro Itagaki
I read pg_upgrade code glance over, and found 4 issues in it.
Are there any issues to be fixed before 9.0 release?

1. NAMEDATASIZE
2. extern PGDLLIMPORT
3. pathSeparator
4. EDB_NATIVE_LANG

 1. NAMEDATASIZE 
pg_upgrade has the following definition, but should it be just NAMEDATALEN?

/* Allocate for null byte */
#define NAMEDATASIZE(NAMEDATALEN + 1)

Table names should be in NAMEDATELEN - 1 bytes. At least 64th bytes in 
name data is always '\0'.

=# CREATE TABLE 1234567890...(total 70 chars)...1234567890 (i int);
NOTICE:  identifier 123...890 will be truncated to 123...0123

 2. extern PGDLLIMPORT 
pg_upgrade has own definitions of
extern PGDLLIMPORT Oid binary_upgrade_next_xxx
in pg_upgrade_sysoids.c. But those variables are not declared as
PGDLLIMPORT in the core. Can we access unexported variables here?

 3. pathSeparator 
Path separator for Windows is not only \ but also /. The current code
ignores /. Also, it might not work if the path string including multi-byte
characters that have \ (0x5c) in the second byte.

 4. EDB_NATIVE_LANG 
Of course it is commented out with #ifdef, but do we have codes
for EDB in core?

Regards,
---
Takahiro Itagaki
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] pg_upgrade code questions

2010-05-13 Thread Takahiro Itagaki

Bruce Momjian br...@momjian.us wrote:

    2. extern PGDLLIMPORT 
   pg_upgrade has own definitions of
   extern PGDLLIMPORT Oid binary_upgrade_next_xxx
  
   The issue here is that you use PGDLLIMPORT where you are importing the
   variable, not where it is defined.  For example, look at
   'seq_page_cost'.  You can see PGDLLIMPORT used where it is imported with
   'extern', but not where is it defined.
  
  Right.  Also we are intentionally not exposing those variables in any
  backend .h file, because they are not meant for general use.  So the
  extern PGDLLIMPORT isn't going to be in the main backend and has to
  be in pg_upgrade.  This was discussed awhile ago when we put in those
  variables, I believe.
 
 Yes, this was discussed.

I wonder some compilers or linkers might hide unexported global variables
from postgres.lib as if they are declared with 'static' specifiers.
I'm especially worried about Windows and MSVC. So, if Windows testers
can see it works, there was nothing to worry about.

Regards,
---
Takahiro Itagaki
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] nvarchar notation accepted?

2010-05-13 Thread Takahiro Itagaki

Jaime Casanova ja...@2ndquadrant.com wrote:

 i migrate a ms sql server database to postgres and was trying some
 queries from the application to find if everything works right...
 when i was looking to those queries i found some that has a notation
 for nvarchar (ej: campo = N'sometext')

Do you have documentation for N'...' literal in SQLServer?
Does it mean unicode literal? What is the difference from U literal?
http://developer.postgresql.org/pgdocs/postgres/sql-syntax-lexical.html

PostgreSQL doesn't have nvarchar types (UTF16 in MSSQL), and only
have mutlti-tyte characters. So I think you can remove N and just
use SET client_encoding = UTF8 in the cases.

Regards,
---
Takahiro Itagaki
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


[HACKERS] Generalized Inverted Generalized Search Tree

2010-05-13 Thread Takahiro Itagaki
We can index multiple scalar values per row with GIN access method,
and also can index single vector value per row with GiST AM.

Is it worth having a new AM to index multiple vector values per row?
It will be an AM for the missing feature in below:

| scalar | vector |
+++
 single per row | btree  | gist   |
 multi per row  | gin| *HERE* |

We can call the new AM gigist. Or, there might be another idea
to support expression indexes for SRF functions, like
  =# CREATE TABLE tbl (c circle[]);
  =# CREATE INDEX ON tbl USING gist (unnest(c));

Comments and ideas welcome.

Regards,
---
Takahiro Itagaki
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] Re: [ANNOUNCE] Bug-fix and new feature of pg_lesslog is released

2010-05-12 Thread Takahiro Itagaki

Tom Lane t...@sss.pgh.pa.us wrote:

 Bruce Momjian br...@momjian.us writes:
  Yes, I would love to get this into /contrib for PG 9.1!
 
 How much are people really going to care about pg_lesslog now that
 we've got streaming replication?  There might be some small use-case
 still left, but it's hard to believe that it would be worth carrying
 it in contrib.

I hope pg_lesslog would work as a WAL filter of streaming replication.
It might be hard-coded in WAL sender, or be an addon based on a new
common filtering infrastructure of WAL streaming.

Also, there is a long-standing issue in pg_lesslog; It slows down recovery
because we need to read data pages before write in recovery. We're avoiding
reading pages for full-page image in 8.3, but pg_lesslog will disable
the optimization. Recovery routine in core also needs to be adjusted to use
read-ahead, like posix_fadvise().

There was another idea, full-page image logs separated with WAL logging.
In theory, full-page images don't have to be written at commit, but only
by writing corresponding data pages, I'm not sure whether it is an actually
good idea or not, but if we go the direction, we won't need pg_lesslog.

Regards,
---
Takahiro Itagaki
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] Patch for PKST timezone

2010-05-11 Thread Takahiro Itagaki

Aftab Hussain aftab...@gmail.com wrote:

 Please accept attached patch for the following problem.

This patch has not been replied, but I can reproduce the error with:
=# SET timezone = 'Asia/Karachi';
=# SELECT timeofday()::timestamptz;
ERROR:  invalid input syntax for type timestamp with time zone: Tue May 11 
13:26:53.264293 2010 PKST

Should we add PKST timezone? Also, I found a list of timezones[1]
and we don't have 36 tznames in the list. Should we also need them?

ACDT, AEDT, AWDT
BIT
CBT, CDBT, CIST
HMT
PKST, PMT
R*T and R*DT series,
WCDT, WCT, WIB, WITA, WKT

[1] http://www.world-time-zones.org/zones/

 af...@aftab-laptop:/opt/dev/pgsql/install/dbserver/bin$
 af...@aftab-laptop:/opt/dev/pgsql/install/dbserver/bin$ ./psql postgres
 psql (9.0beta1)
 Type help for help.
 
 postgres=# SHOW timezone;
TimeZone
 --
  Asia/Karachi
 (1 row)
 
 postgres=#
 postgres=# CREATE TABLE test_table (c1 INT, c2 TIMESTAMP DEFAULT
 timeofday()::TIMESTAMP);
 CREATE TABLE
 postgres=# INSERT INTO test_table VALUES (1);
 ERROR:  invalid input syntax for type timestamp: Fri Apr 30 15:36:43.906075
 2010 PKST
 postgres=#
 
 And here is a little bit information about the system I am using.
 
 af...@aftab-laptop:/opt/dev/pgsql/install/dbserver/bin$  uname -a
 Linux aftab-laptop 2.6.31-20-generic #58-Ubuntu SMP Fri Mar 12 05:23:09 UTC
 2010 i686 GNU/Linux
 af...@aftab-laptop:/opt/dev/pgsql/install/dbserver/bin$
 af...@aftab-laptop:/opt/dev/pgsql/install/dbserver/bin$ ./pg_config
 --version
 PostgreSQL 9.0beta1
 af...@aftab-laptop:/opt/dev/pgsql/install/dbserver/bin$


Regards,
---
Takahiro Itagaki
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] [BUGS] SET search_path clause ignored during function creation

2010-05-10 Thread Takahiro Itagaki

Erwin Brandstetter brsaw...@gmail.com wrote:

 Function bodies are checked using the _current_ search_path instead of
 the search_path supplied by the SET search_path clause.
 
 Proposed solution: Function bodies should be checked with the
 search_path provided by SET search_path an _not_ with the current
 search path at the time pof creation.

Thanks for the report!  Please check whether the attached patch
is the correct fix. An additional regression test is included.

Regards,
---
Takahiro Itagaki
NTT Open Source Software Center



create_function_with_search_path.patch
Description: Binary data

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] [BUGS] SET search_path clause ignored during function creation

2010-05-10 Thread Takahiro Itagaki

Tom Lane t...@sss.pgh.pa.us wrote:

 Takahiro Itagaki itagaki.takah...@oss.ntt.co.jp writes:
  Thanks for the report!  Please check whether the attached patch
  is the correct fix. An additional regression test is included.
 
 That's going to provoke uninitialized variable compiler warnings,
 but otherwise it seems reasonably sane.

I applied a revised version that can surpress compiler warnings
to 9.0beta, 8.4 and 8.3.

Regards,
---
Takahiro Itagaki
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] pg_stat_transaction patch

2010-05-06 Thread Takahiro Itagaki

Joel Jacobson j...@gluefinance.com wrote:

 I propose a set of new statistics functions and system views.
 
 I need these functions in order to do automated testing of our system,
 consisting of hundreds of stored procedures in plpgsql.
 My plan is to develop some additional functions to pgTAP, benefiting from
 the new system tables I've added.

I ported your patch into 9.0beta, but it doesn't work well.
I had two assertion failures from the run.sql:

TRAP: FailedAssertion(!(entry-trans == ((void *)0)), File: pgstat.c, Line: 
715)
TRAP: FailedAssertion(!(tabstat-trans == trans), File: pgstat.c, Line: 
1756)

Also, pg_stat_transaction_functions returned no rows from the test case even
after I removed those assertions. There are no rows in your test/run.out, too.

I like your idea itself, but more works are required for the implementation.

Regards,
---
Takahiro Itagaki
NTT Open Source Software Center



pg_stat_transaction-9.0beta.patch
Description: Binary data

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] CP949 for EUC-KR?

2010-05-05 Thread Takahiro Itagaki

Ioseph Kim pgsql...@postgresql.kr wrote:

 CP51949 is EUC-KR correct.
 {PG_EUC_KR, CP51949}, /* or 20949 ? */

Thank you for the information. I removed or 20949 ? from the line.

Regards,
---
Takahiro Itagaki
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] COPY is not working

2010-04-30 Thread Takahiro Itagaki

Jaime Casanova jcasa...@systemguards.com.ec wrote:

 ah! this is because COPY doesn't follow inherited tables... should it?

Yes. You can use COPY (SELECT * FROM a) TO  instead to copy all tuples.

http://developer.postgresql.org/pgdocs/postgres/sql-copy.html
| COPY can only be used with plain tables, not with views.
| However, you can write COPY (SELECT * FROM viewname) TO  

Should we add or parent tables after not with views?
To be exact, it would be 'COPY a parent table TO' only copies
tuples in the parent table and does not copy inherited child tables.


regression=# CREATE TABLE a (aa integer);
CREATE TABLE
regression=# CREATE TABLE b () INHERITS (a);
CREATE TABLE
regression=# INSERT INTO b VALUES(32), (56);
INSERT 0 2
regression=# select * from a;
 aa

 32
 56
(2 rows)

regression=# COPY a TO '/tmp/copy_test';
COPY 0
regression=# COPY (SELECT * FROM a) TO '/tmp/copy_test';
COPY 2

Regards,
---
Takahiro Itagaki
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] Invalidating dependent views and functions

2010-04-30 Thread Takahiro Itagaki

Scott Bailey arta...@comcast.net wrote:

 Problem: We need to change the last_name column of the people table from 
 varchar(30) to varchar(50).
 Proposal: Add an invalid flag to pg_class.

Your example is one of the simplest cases, but there are other complex
usages. For example, shrinking varchar length, altering indexed columns,
CREATE FUNCTION RETURNS altered_table_type, and so on.
Can your proposal solve all (or almost all) use-cases? I think we need to
have such flag fields for each catalog tables if we support invalid status.

 ALTER TABLE people ALTER last_name VARCHAR(50) INVALIDATE;
 -- Alters column and invalidates any dependent objects

IMHO, I don't like the invalid flags. If we can recompile objects later,
why don't we recomple them at the same time?

  ALTER TABLE people ALTER last_name TYPE varchar(50) CASCADE;
  -- Alters column and *recompile* any dependent objects

However, dependent objects are not only in the database, but also in
the client applications. That's why we allow CREATE OR REPLACE VIEW
only to add columns, but disallow to modify existing columns.

Regards,
---
Takahiro Itagaki
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] COPY is not working

2010-04-29 Thread Takahiro Itagaki

Jaime Casanova jcasa...@systemguards.com.ec wrote:

 COPY is not working on latest HEAD?
 
 regression=# select * from a;
  aa
 
  32
  56
 (2 rows)
 
 regression=# COPY a TO '/tmp/copy_test';
 COPY 0
 
 
 -- 

Please send the actual test pattern and your environment information
to reproduce the misbehavior. It works fine on my machine.

regression=# CREATE TABLE a (aa integer);
CREATE TABLE
regression=# INSERT INTO a VALUES(32), (56);
INSERT 0 2
regression=# select * from a;
 aa

 32
 56
(2 rows)

regression=# COPY a TO '/tmp/copy_test';
COPY 2
regression=# \! cat /tmp/copy_test
32
56

$ uname -a
Linux xxx 2.6.29.4-167.fc11.x86_64 #1 SMP Wed May 27 17:27:08 EDT 2009 x86_64 
x86_64 x86_64 GNU/Linux
$ postgres --version
postgres (PostgreSQL) 9.0beta1

Regards,
---
Takahiro Itagaki
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


[HACKERS] CP949 for EUC-KR?

2010-04-27 Thread Takahiro Itagaki
I heard pg_get_encoding_from_locale() failed in kor locale.

WARNING:  could not determine encoding for locale kor: codeset is CP949

I found the following description in the web:
CP949 is EUC-KR, extended with UHC (Unified Hangul Code).

http://www.opensource.apple.com/source/libiconv/libiconv-13.2/libiconv/lib/cp949.h

but we define CP51949 for EUC-KR in chklocale.c.
{PG_EUC_KR, CP51949}, /* or 20949 ? */

Which is the compatible codeset with our PG_EUC_KR encoding?
949, 51949, or 20949? Should we add (or replace) CP949 for EUC-KR?

Regards,
---
Takahiro Itagaki
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] CP949 for EUC-KR?

2010-04-27 Thread Takahiro Itagaki

Heikki Linnakangas heikki.linnakan...@enterprisedb.com wrote:

  Should we add (or replace) CP949 for EUC-KR?
 
 No. CP949 is not plain EUC-KR, but EUC-KR with some extensions (UHC). At
 least on CVS HEAD, we recognize CP949 as an alias for the PostgreSQL
 PG_UHC encoding.

That's it! We should have added an additional alias to chklocale, too.

Index: src/port/chklocale.c
===
--- src/port/chklocale.c(HEAD)
+++ src/port/chklocale.c(fixed)
@@ -172,6 +172,7 @@
{PG_GBK, CP936},
 
{PG_UHC, UHC},
+   {PG_UHC, CP949},
 
{PG_JOHAB, JOHAB},
{PG_JOHAB, CP1361},


Except UHC, we don't have any codepage aliases for the encodings below.
I assume we don't need to add CPxxx because Windows does not have
corresponding codepages for them, right?

{PG_LATIN6, ISO-8859-10},
{PG_LATIN7, ISO-8859-13},
{PG_LATIN8, ISO-8859-14},
{PG_LATIN10, ISO-8859-16},
{PG_SHIFT_JIS_2004, SJIS_2004},

Regards,
---
Takahiro Itagaki
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] Add column if not exists (CINE)

2010-04-27 Thread Takahiro Itagaki

Kjell Rune Skaaraas kjell...@yahoo.no wrote:

 I've been reading the earlier threads at:
 http://archives.postgresql.org/pgsql-hackers/2009-05/thrd7.php#00252
 http://archives.postgresql.org/pgsql-hackers/2005-10/thrd4.php#00632
 and I'm not sure I have anything that substantially new to add but:
 
 I saw some indications that this might be a minority opinion,
 well I would like to cast a vote FOR this functionality.

+1 for CINE, just because MySQL supports it.

But before developing, we need to decide how to handle an added object
that has the same name but has different definitions. 

Also, developers should consider not only ADD COLUMN but also other
CREATE or ADD commands. The patch will be large, including documentation
adjustments in many places -- it would be hard work.

Regards,
---
Takahiro Itagaki
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] CIText and pattern_ops

2010-04-26 Thread Takahiro Itagaki

Rod Taylor p...@rbt.ca wrote:

 Is there any particular reason why the citext module doesn't have
 citext_pattern_ops operator family?
 
 Specifically, I wish to index for this type of query:
 
 ... WHERE citext_column LIKE 'Foo%';

I think it is a reasonable suggestion.

=# \d tbl
 Table public.tbl
 Column |  Type  | Modifiers
++---
 t  | text   |
 c  | citext |
Indexes:
tbl_c_idx btree (c)
tbl_t_idx btree (t)

=# SET enable_seqscan = off;
SET
=# EXPLAIN SELECT * FROM tbl WHERE t LIKE 'abc%';
  QUERY PLAN
--
 Index Scan using tbl_t_idx on tbl  (cost=0.00..8.27 rows=1 width=64)
   Index Cond: ((t = 'abc'::text) AND (t  'abd'::text))
   Filter: (t ~~ 'abc%'::text)
(3 rows)

=# EXPLAIN SELECT * FROM tbl WHERE c LIKE 'abc%';
   QUERY PLAN

 Seq Scan on tbl  (cost=100.00..101.01 rows=1 width=64)
   Filter: (c ~~ 'abc%'::citext)
(2 rows)


Regards,
---
Takahiro Itagaki
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] [GENERAL] trouble with to_char('L')

2010-04-21 Thread Takahiro Itagaki

Takahiro Itagaki itagaki.takah...@oss.ntt.co.jp wrote:

 Revised patch attached. Please test it.

I applied this version of the patch.
Please check wheter the bug is fixed and any buildfarm failures.

Regards,
---
Takahiro Itagaki
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] [GENERAL] trouble with to_char('L')

2010-04-20 Thread Takahiro Itagaki

Magnus Hagander mag...@hagander.net wrote:

  1. setlocale(LC_CTYPE, lc_monetary)
  2. setlocale(LC_MONETARY, lc_monetary)
  3. lc = localeconv()
  4. pg_do_encoding_conversion(lc-xxx,
   FROM pg_get_encoding_from_locale(lc_monetary),
   TO GetDatabaseEncoding())
  5. Revert LC_CTYPE and LC_MONETARY.

A patch attached for the above straightforwardly. Does this work?
Note that #ifdef WIN32 parts in the patch are harmless on other platforms
even if they are enabled.

 Let's work off what we have now to start with at least. Bruce, can you
 comment on that thing about the extra parameter? And UTF8?

Regards,
---
Takahiro Itagaki
NTT Open Source Software Center



pg_locale_20100420.patch
Description: Binary data

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] [GENERAL] trouble with to_char('L')

2010-04-20 Thread Takahiro Itagaki

Hiroshi Inoue in...@tpf.co.jp wrote:

 1. How does it work when LC_MONETARY and LC_NUMERIC are different?

I think it is rarely used, but possible. Fixed.

 2. Calling db_encoding_strdup() for lconv-grouping is appropriate?

Ah, we didn't need it. Removed.

Revised patch attached. Please test it.

Regards,
---
Takahiro Itagaki
NTT Open Source Software Center



pg_locale_20100421.patch
Description: Binary data

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] row estimation off the mark when generate_series calls are involved

2010-04-19 Thread Takahiro Itagaki

Nikhil Sontakke nikhil.sonta...@enterprisedb.com wrote:

 I observed the following behavior on PG head:
 postgres=# explain verbose insert into public.x values 
 (generate_series(1,10));
 
 Insert (cost=0.00..0.01 rows=1 width=0)

Hmmm, there are differences between SELECT SRF() and SELECT * FROM SRF():

postgres=# EXPLAIN INSERT INTO public.x SELECT generate_series(1,10);
   QUERY PLAN

 Insert  (cost=0.00..0.02 rows=1 width=4)
   -  Result  (cost=0.00..0.01 rows=1 width=0)

postgres=# EXPLAIN INSERT INTO public.x SELECT * FROM generate_series(1,10);
  QUERY PLAN
--
 Insert  (cost=0.00..10.00 rows=1000 width=4)
   -  Function Scan on generate_series  (cost=0.00..10.00 rows=1000 width=4)


 I think the place where we set the
 targetlist of the result_plan to sub_tlist, immediately after that we
 should update the plan_rows estimate by walking this latest
 targetlist. I did that and now we seem to get proper row estimates.

I agree the estimation should be improved, but your patch *adds*
the estimated number of rows to the result:

postgres=# EXPLAIN INSERT INTO public.x SELECT generate_series(1,10);
QUERY PLAN
---
 Insert  (cost=0.00..12.52 rows=1002 width=4)
   -  Result  (cost=0.00..2.51 rows=1001 width=0)

Should it be 1000 rather than 1001?

Regards,
---
Takahiro Itagaki
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] [GENERAL] trouble with to_char('L')

2010-04-18 Thread Takahiro Itagaki

Magnus Hagander mag...@hagander.net wrote:

 But I'm unsure how that would work. We're talking about the output of
 localeconv(), right? I don't see a version of localeconv() that does
 wide chars anywhere. (You can't just set LC_CTYPE and use the regular
 function - Windows has a separate set of functions for dealing with
 UTF16).

Yeah, msvcrt doesn't have wlocaleconv :-( . Since localeconv() returns
characters in the encoding specified in LC_TYPE, we need to hande the
issue with codes something like:

1. setlocale(LC_CTYPE, lc_monetary)
2. setlocale(LC_MONETARY, lc_monetary)
3. lc = localeconv()
4. pg_do_encoding_conversion(lc-xxx,
  FROM pg_get_encoding_from_locale(lc_monetary),
  TO GetDatabaseEncoding())
5. Revert LC_CTYPE and LC_MONETARY.


Another idea is to use GetLocaleInfoW() [1], that is win32 native locale
functions, instead of the libc one. It returns locale characters in wide
chars, so we can safely convert them as UTF16-UTF8-db. But it requires
an additional branch in our locale codes only for Windows.

[1] http://msdn.microsoft.com/en-us/library/dd318101

Regards,
---
Takahiro Itagaki
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] GSOC PostgreSQL partitioning issue

2010-04-08 Thread Takahiro Itagaki

Greg Smith g...@2ndquadrant.com wrote:

 An introduction to the current state of work in progress for adding 
 improved partitioning features to PostgreSQL is documented at 
 http://wiki.postgresql.org/wiki/Table_partitioning

Also, here is my latest patch for it:
http://repo.or.cz/w/pgsql-fdw.git/shortlog/refs/heads/partition

I'd like to ask Necati what problem you will solve, rather than
what module you will develop. Performance? Usability? Or othres?

Regards,
---
Takahiro Itagaki
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] Set LC_COLLATE to de_DE_phoneb

2010-04-07 Thread Takahiro Itagaki

Frank Jagusch fr...@jagusch-online.de wrote:

 The german phone book order has the windows language setting
 de_DE_phoneb an the value 00010407 in the registry. Unfortunately I was
 not able to find a corresponding string for the LC_COLLATE setting.

I cannot find any resources for de_DE_phoneb in Web. What is the true
name for it? Locale names should be in Country_Language.CodePage
format on Windows. If you can find the counterpart name for it, you can
initialize PostgreSQL DB with the locale, and CodePage or UTF-8 encoding.

 Background: I moved an old application from a borland paradox database
 to postgesql. The speed gain is great but the sorting order isn't the
 usual to the user. I can't change the order by clauses of the select
 statements because they are generated by the borland database engine.

I'm afraid of de_DE_phoneb is an original locale implementation in your
old database. If so, PostgreSQL cannot support it because postgres depends
on locale libraries in each platform. (i.e., msvcrt on Windows)

Regards,
---
Takahiro Itagaki
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] fallback_application_name and pgbench

2010-04-07 Thread Takahiro Itagaki

Fujii Masao masao.fu...@gmail.com wrote:

 By default, the application_name of pgbench is [unknown]. But I think
 that pgbench should use fallback_application_name as psql, pg_dump,
 etc does. Is it worth creating the patch?

If we will take care of fallback_application_name for contrib modules,
we need to fix not only pgbench but also oid2name and dblink.
I think those fixes would be worth; at least for telling third-party
developers to handle the new parameter.

It might be better to set fallback_application_name automatically
in libpq, but it requires argv[0] away from main() function.
GetModuleFilename() is available on Windows for the purpose,
but I don't know what API is available on other platforms.

Regards,
---
Takahiro Itagaki
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] Set LC_COLLATE to de_DE_phoneb

2010-04-07 Thread Takahiro Itagaki

Frank Jagusch fr...@jagusch-online.de wrote:

 de_DE_phoneb is the name of an alternative sorting in german (only a
 few languages have alternate sorting). You may find some information
 when you search the MSDN for de_DE_phoneb, i.e.
 http://msdn.microsoft.com/en-en/library/ms404373.aspx
 These alternate sorting is supported by the OS, but I don't know how it
 is supported in the msvcrt.

Hmmm, I found de-DE_phoneb in MSDN:
http://msdn.microsoft.com/en-us/library/dd374060
but setlocale(de-DE_phoneb) always fails at least on my machine.

The doc says de-DE_phoneb is a locale name for 
MAKELCID(MAKELANGID(LANG_GERMAN, SUBLANG_GERMAN), SORT_GERMAN_PHONE_BOOK).
Some of native Win32 APIs could accept the locale and sort-order
combination, but setlocale() in CRT seems to reject it.

So, you could use the locale if you find a setlocale-compatible name of
de-DE_phoneb. Or, you cannot use it, unless we modify PostgreSQL to
use Win32 locale functions instead of standard libc ones -- but it is
hardly acceptable.

Regards,
---
Takahiro Itagaki
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


[HACKERS] Oddly indented raw_expression_tree_walker

2010-04-07 Thread Takahiro Itagaki
I found raw_expression_tree_walker() is oddly indented in 8.4 and HEAD.
I expected pgindent would fix those clutter, but it could not.
Should we cleanup it manually, or leave it as-is?
Also, should we backport such kind of cleanups to previous releases?

Index: src/backend/nodes/nodeFuncs.c
===
@@ -2198,7 +2198,7 @@
  * that could appear under it, but not other statement types.
  */
 bool
-   raw_expression_tree_walker(Node *node, bool (*walker) 
(), void *context)
+raw_expression_tree_walker(Node *node, bool (*walker) (), void *context)
 {
ListCell   *temp;
 

Regards,
---
Takahiro Itagaki
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] SELECT constant; takes 15x longer on 9.0?

2010-04-06 Thread Takahiro Itagaki

Josh Berkus j...@agliodbs.com wrote:

 SELECT 'DBD::Pg ping test';
 
 In our test, which does 5801 of these pings during the test, they take
 an average of 15x longer to execute on 9.0 as 8.4 ( 0.77ms vs. 0.05ms ).
 
 Any clue why this would be?

Did you use the same configure options between them?
For example, --enable-debug or --enable-cassert.

Regards,
---
Takahiro Itagaki
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] Compile fail, alpha5 gcc 4.3.3 in elog.c

2010-04-05 Thread Takahiro Itagaki

Tom Lane t...@sss.pgh.pa.us wrote:

  -   cygwin_conv_to_full_win32_path(cmdLine, buf);
  +   cygwin_conv_path(CCP_POSIX_TO_WIN_A, cmdLine, buf, sizeof(buf));
 
 Buildfarm member brown_bat didn't like this.  Seeing that that's the
 *only* active cygwin buildfarm member, that's not a good percentage.

Hmmm, but avoiding deprecated APIs would be good on the lastest cygwin.
How about checking the version with #ifdef?

 #ifdef __CYGWIN__
/* need to convert to windows path */
+#if CYGWIN_VERSION_DLL_MAJOR = 1007
cygwin_conv_path(CCP_POSIX_TO_WIN_A, cmdLine, buf, sizeof(buf));
+#else
+   cygwin_conv_to_full_win32_path(cmdLine, buf);
+#endif
strcpy(cmdLine, buf);
 #endif

Regards,
---
Takahiro Itagaki
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: xmlconcat (was [HACKERS] 9.0 release notes done)

2010-04-04 Thread Takahiro Itagaki

Tom Lane t...@sss.pgh.pa.us wrote:

 Takahiro Itagaki itagaki.takah...@oss.ntt.co.jp writes:
  Can we take the patch for 9.0? The bug is registered as an open item:
  http://wiki.postgresql.org/wiki/PostgreSQL_9.0_Open_Items
 
 Given that there are still problems with it, applying the patch for 9.0
 would mean changing the behavior of xmlconcat in 9.0 and then again in
 9.1.  I don't think that's a good idea.  Better to leave it alone until
 we have a full fix.

Ok, I added it in ToDo list, and removed it from 9.0 open items.
better handling of PIs and DTDs in xmlconcat()

Regards,
---
Takahiro Itagaki
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


[HACKERS] VACUUM FULL during initdb

2010-04-04 Thread Takahiro Itagaki
Do we still need VACUUM FULL in initdb? VACUUM FULL in 9.0 rewrites
all tables, so those operations are a little more expensive than
previous releases. Is it worth replacing them into VACUUM?

make_template0(void)
Finally vacuum to clean up dead rows in pg_database
VACUUM FULL pg_database;\n,

vacuum_db(void)
PG_CMD_PUTS(ANALYZE;\nVACUUM FULL;\nVACUUM FREEZE;\n);

Regards,
---
Takahiro Itagaki
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: xmlconcat (was [HACKERS] 9.0 release notes done)

2010-04-02 Thread Takahiro Itagaki

Andrew Dunstan and...@dunslane.net wrote:

 Hmm. OK. Well here is a patch that tries to fix the xmlconcat error, 
 anyway. It seems to work, but maybe could stand a little tightening.

Can we take the patch for 9.0? The bug is registered as an open item:
http://wiki.postgresql.org/wiki/PostgreSQL_9.0_Open_Items

As far as the patch, I found there are still two issues even after it applied:

1. A linebreak is added at the line end DOCTYPE exists.
=# SELECT xmlconcat('foo', xmlparse(DOCUMENT '!DOCTYPE htmlhtml/'));
 xmlconcat

 foohtml/+

(1 row)

2. DOCUMENT could have ?xml before DOCTYPE.
=# SELECT xmlconcat('foo', xmlparse(DOCUMENT '?xml version=1.0? 
!DOCTYPE  html html/'));
 xmlconcat
---
 foo
(1 row)

Regards,
---
Takahiro Itagaki
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] Compile fail, alpha5 gcc 4.3.3 in elog.c

2010-04-02 Thread Takahiro Itagaki

Josh Berkus j...@agliodbs.com wrote:

 Ok, this issue seems to be specific to some versions of gcc.  Note that
 in testing this nobody enabled any special compile or environment
 variables of any kind, so if there's a -Werror where it shouldn't be,
 it's in our code.

Hi, cygwin also has -Werror in default, and build was failed with a warning:

$ uname -a
CYGWIN_NT-5.1 name 1.7.2(0.225/5/3) 2010-03-24 21:12 i686 Cygwin

gcc -O2 -Wall -Wmissing-prototypes -Wpointer-arith -Wdeclaration-after-statement
 -Wendif-labels -fno-strict-aliasing -fwrapv -Werror -DDEF_PGPORT=5432 -I../../.
./src/interfaces/libpq -I../../../src/include   -c -o pg_ctl.o pg_ctl.c
pg_ctl.c: In function `pgwin32_CommandLine':
pg_ctl.c:1083: warning: `cygwin_conv_to_full_win32_path' is deprecated (declared
 at /usr/include/sys/cygwin.h:52)
make[3]: *** [pg_ctl.o] Error 1


Any objections for the following fix?

Index: src/bin/pg_ctl/pg_ctl.c
===
--- src/bin/pg_ctl/pg_ctl.c (HEAD)
+++ src/bin/pg_ctl/pg_ctl.c (fixed)
@@ -1080,7 +1080,7 @@
 
 #ifdef __CYGWIN__
/* need to convert to windows path */
-   cygwin_conv_to_full_win32_path(cmdLine, buf);
+   cygwin_conv_path(CCP_POSIX_TO_WIN_A, cmdLine, buf, sizeof(buf));
strcpy(cmdLine, buf);
 #endif
 

Regards,
---
Takahiro Itagaki
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] Problem of Magic Block in Postgres 8.2

2010-04-01 Thread Takahiro Itagaki

Pei He hepeim...@gmail.com wrote:

 The extension functions was developed by C++ mixed with C.
 ERROR:  incompatible library
 /home/hepei/bin/Chameleon/lib/libspgist_trie.so: missing magic block
 HINT:  Extension libraries are required to use the PG_MODULE_MAGIC macro.

You can use extern C blocks for PG_MODULE_MAGIC, PG_FUNCTION_INFO_V1,
and function declarations:

extern C
{
#include postgres.h
#include fmgr.h

PG_MODULE_MAGIC;

PG_FUNCTION_INFO_V1(your_function);
extern Datum your_function(PG_FUNCTION_ARGS);
}

However, you should very carefully use C++ exceptions and destructors
in your module because PostgreSQL uses siglongjmp; C++ unwind semantics
don't work on postgres' errors. You cannot use those C++ features and
postgres' APIs together.

Regards,
---
Takahiro Itagaki
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: [SPAM]Re: [HACKERS] Questions about 9.0 release note

2010-03-31 Thread Takahiro Itagaki

Robert Haas robertmh...@gmail.com wrote:

  * (seq_page_cost/(random_page_cost))
  * EXCLUDE constraints has no tags to be linked.
  * EXCLUDE constraints is not indexed from the Index page.

 CREATE TABLE ... CONSTRAINT ... EXCLUDE rather than CREATE TABLE
 CONSTRAINT ... EXCLUDE.

Here is a patch to fix the documentation.

For exclusion constraints, I added a tag SQL-CREATETABLE-exclude
to varlistentry of EXCLUDE in CREATE TABLE documentation. Also,
Exclusion constraints section is added to the constraints doc.
But the section is very short and just links to the CREATE TABLE doc.
We could move some contents from CREATE TABLE to the constraints doc.

Regards,
---
Takahiro Itagaki
NTT Open Source Software Center



docfix_20100331.patch
Description: Binary data

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] [BUGS] BUG #5394: invalid declspec for PG_MODULE_MAGIC

2010-03-29 Thread Takahiro Itagaki

Vladimir Barzionov snego.bar...@gmail.com wrote:

 Same problem was already discussed for example here
 http://dbaspot.com/forums/postgresql/393683-re-general-custom-c-function-palloc-broken.html
 
 Looks like the simplest way for correcting the issue is declaring additional
 macro (something like PGMODULEEXPORT)

Sure, I agree it is a longstanding bug in PostgreSQL. Developers who use
MSVC (not mingw) always encounter the bug; machines in the buildfarm can
build Windows binaries just because they have non-standard equipments.

A patch attached. The name of PGMODULEEXPORT might be arguable.

Regards,
---
Takahiro Itagaki
NTT Open Source Software Center



PGMODULEEXPORT.patch
Description: Binary data

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


[HACKERS] Questions about 9.0 release note

2010-03-29 Thread Takahiro Itagaki
Hi, I have some questions about 9.0 release note.
I'd like to work for some of them if required. Comments welcome.

* Allow per-tablespace sequential and random page cost variables
  (seq_page_cost/(random_page_cost)) via ALTER TABLESPACE ... SET/RESET
  ^^
Are those parentheses around random_page_cost intentional?
They seems to mean just (seq_page_cost and random_page_cost).

* EXCLUDE constraints has no tags to be linked.
The item in release note is pointing the following page,
http://developer.postgresql.org/pgdocs/postgres/sql-createtable.html#SQL-CREATETABLE-DESCRIPTION
but the actual description about EXCLUDE constraints are in the subentry
of Parameters section. Can we add a tag to varlistentry for EXCLUDE?

* EXCLUDE constraints is not indexed from the Index page.
Should we have for it? Unique Constraints have a section for them:
http://developer.postgresql.org/pgdocs/postgres/ddl-constraints.html#AEN2431

Regards,
---
Takahiro Itagaki
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] Patch for 9.1: initdb -C option

2010-03-28 Thread Takahiro Itagaki

David Christensen da...@endpoint.com wrote:

 Enclosed is a patch to add a -C option to initdb to allow you to easily
 append configuration directives to the generated postgresql.conf file

Why don't you use just echo 'options'  $PGDATA/postgresql.conf ?
Could you explain where the -C options is better than initdb + echo?

Regards,
---
Takahiro Itagaki
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] WIP: preloading of ispell dictionary

2010-03-22 Thread Takahiro Itagaki

Pavel Stehule pavel.steh...@gmail.com wrote:

 I wrote some small patch, that allow preloading of  selected ispell
 dictionary. It solve the problem with slow tsearch initialisation with
 some language configuration.
 
 I afraid so this module doesn't help on MS Windows.

I think it should work on all platforms if we include it into the core.
We should continue to research shared memory or mmap approaches.

The fundamental issue seems to be in the slow initialization of
dictionaries. If so, how about adding a pre-complile tool to convert
a dictionary into a binary file, and each backend simply mmap it?

BTW, SimpleAllocContextCreate() is not used at all in the patch.
Do you still need it?

Regards,
---
Takahiro Itagaki
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] Ragged latency log data in multi-threaded pgbench

2010-03-22 Thread Takahiro Itagaki

Greg Smith g...@2ndquadrant.com wrote:

 Attached is an updated version that I think is ready to commit.  Only 
 changes are docs--I rewrote those to improve the wording some.

Thanks for the correction. Applied.

Regards,
---
Takahiro Itagaki
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] 9.0 release notes done

2010-03-22 Thread Takahiro Itagaki

Bruce Momjian br...@momjian.us wrote:

 I have completed the 9.0 release notes:
   http://developer.postgresql.org/pgdocs/postgres/release-9-0.html

There is an additional incompatibilitiy in pg_largeobject catalog.
We need to rewrite queries to test existences of large objests
from SELECT DISTINCT(loid) FROM pg_largeobject
to   SELECT oid FROM pg_largeobject_metadata
because an empty large object doesn't have rows in pg_largeobject.

Regards,
---
Takahiro Itagaki
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] Ragged latency log data in multi-threaded pgbench

2010-03-22 Thread Takahiro Itagaki

Greg Smith g...@2ndquadrant.com wrote:

 By the way: the pgbench.sgml that you committed looks like it passed 
 through a system that added a CR to every line in it.  Probably not the 
 way you intended to commit that.

Oops, fixed. Thanks.

Regards,
---
Takahiro Itagaki
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] Partitioning syntax

2010-03-17 Thread Takahiro Itagaki

Dmitry Fefelov fo...@ac-sw.com wrote:

  Here is a revised partitioning syntax patch. It implements only syntax and
  on-disk structure mentioned below:
  Table Partitioning#Syntax
http://wiki.postgresql.org/wiki/Table_partitioning#Syntax
  Table Partitioning#On-disk structure
http://wiki.postgresql.org/wiki/Table_partitioning#On-disk_structure
 
 Will 9.1 partitions allow to reference partitioned tables in foreign keys?

Not in my first goals, but it might be possible if we could support row locks
for UNION plans:

=# SELECT * FROM tbl1 UNION ALL SELECT * FROM tbl2 FOR SHARE;
ERROR:  SELECT FOR UPDATE/SHARE is not allowed with UNION/INTERSECT/EXCEPT
(in 9.0)

Regards,
---
Takahiro Itagaki
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] [GENERAL] trouble with to_char('L')

2010-03-17 Thread Takahiro Itagaki

Bruce Momjian br...@momjian.us wrote:

 Takahiro Itagaki wrote:
  Since 9.0 has GetPlatformEncoding() for the purpose, we could simplify
  db_encoding_strdup() with the function. Like this:
 
 OK, I don't have any Win32 people testing this patch so if we want this
 fixed for 9.0 someone is going to have to test my patch to see that it
 works.  Can you make the adjustments suggested above to my patch and
 test it to see that it works so we can apply it for 9.0?

Here is a full patch that can be applied cleanly to HEAD.
Can anyone test it on Windows?

I'm not sure why temporary changes of lc_ctype was required in the
original patch. The codes are not included in my patch, but please
notice me it is still needed.

Regards,
---
Takahiro Itagaki
NTT Open Source Software Center



pg_locale_20100318.patch
Description: Binary data

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Ragged latency log data in multi-threaded pgbench

2010-03-15 Thread Takahiro Itagaki

Greg Smith g...@2ndquadrant.com wrote:

 It looks like the switch between clients running on separate workers can 
 lead to a mix of their respective lines showing up though.

Oops. There might be two solutions for the issue:
  1. Use explicit locks. The lock primitive will be pthread_mutex for
 multi-threaded implementations or semaphore for multi-threaded ones.
  2. Use per-thread log files.
 File names would be pgbench_log.main-process-id.thread-id.

Which is better, or another idea?

Regards,
---
Takahiro Itagaki
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] Ragged latency log data in multi-threaded pgbench

2010-03-15 Thread Takahiro Itagaki

Tom Lane t...@sss.pgh.pa.us wrote:

 Takahiro Itagaki itagaki.takah...@oss.ntt.co.jp writes:
  Oops. There might be two solutions for the issue:
1. Use explicit locks. The lock primitive will be pthread_mutex for
   multi-threaded implementations or semaphore for multi-threaded ones.
2. Use per-thread log files.
   File names would be pgbench_log.main-process-id.thread-id.
 
 I think #1 is out of the question, as the synchronization overhead will
 do serious damage to the whole point of having a multithreaded pgbench.
 #2 might be a reasonable idea.

Ok, I'll go for #2.

Regards,
---
Takahiro Itagaki
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] Ragged latency log data in multi-threaded pgbench

2010-03-15 Thread Takahiro Itagaki

Takahiro Itagaki itagaki.takah...@oss.ntt.co.jp wrote:

 2. Use per-thread log files.
File names would be pgbench_log.main-process-id.thread-id.

Here is a patch to implement per-thread log files for pgbench -l.

The log filenames are pgbench_log.main-process-id.thread-serial-number
for each thread, but the first thread (including single-threaded) still uses
pgbench_log.main-process-id for the name because of compatibility.

Example:
  $ pgbench -c16 -j4 -l
  $ ls
  pgbench_log.2196  pgbench_log.2196.1  pgbench_log.2196.2  pgbench_log.2196.3

Comments and suggenstions welcome.

Regards,
---
Takahiro Itagaki
NTT Open Source Software Center



pgbench_log_20100316.patch
Description: Binary data

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] [GENERAL] trouble with to_char('L')

2010-03-11 Thread Takahiro Itagaki

Bruce Momjian br...@momjian.us wrote:

 OK, I have created a new function, win32_wchar_to_db_encoding(), to
 share the conversion from wide characters to the database encoding.
 New patch attached.

Since 9.0 has GetPlatformEncoding() for the purpose, we could simplify
db_encoding_strdup() with the function. Like this:

static char *
db_encoding_strdup(const char *str)
{
char   *pstr;
char   *mstr;

/* convert the string to the database encoding */
pstr = (char *) pg_do_encoding_conversion(
(unsigned char *) str, 
strlen(str),
GetPlatformEncoding(), 
GetDatabaseEncoding());
mstr = strdup(pstr);
if (pstr != str)
pfree(pstr);

return mstr;
}

I beleive the code is harmless on all platforms and we can use it
instead of strdup() without any #ifdef WIN32 quotes.


BTW, I found we'd better to add ANSI_X3.4-1968 as an alias for
PG_SQL_ASCII. My Fedora 12 returns the name when --no-locale is used.

Regards,
---
Takahiro Itagaki
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] lock mode for ControlFileLock which pg_start_backup uses

2010-03-09 Thread Takahiro Itagaki

Fujii Masao masao.fu...@gmail.com wrote:

 Currently pg_start_backup() accesses the shared ControlFile
 by using ControlFileLock with LW_EXCLUSIVE lock mode. But
 since that access is read-only operation, LW_SHARED should
 be chosen instead of LW_EXCLUSIVE.
 
 The attached patch changes the lock mode which pg_start_backup()
 uses. Is it worth applying this patch?

Thanks, applied.

Regards,
---
Takahiro Itagaki
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] lock mode for ControlFileLock which pg_start_backup uses

2010-03-08 Thread Takahiro Itagaki

Fujii Masao masao.fu...@gmail.com wrote:

 Currently pg_start_backup() accesses the shared ControlFile
 by using ControlFileLock with LW_EXCLUSIVE lock mode. But
 since that access is read-only operation, LW_SHARED should
 be chosen instead of LW_EXCLUSIVE.

Almost all operations of ControlFileLock is in LW_EXCLUSIVE, but
there is one usage of LWLockConditionalAcquire(ControlFileLock, LW_SHARED)
in XLogNeedsFlush().

 The attached patch changes the lock mode which pg_start_backup()
 uses. Is it worth applying this patch?

I think the patch is reasonable to represent what we are doing,
even if there is no performance benefits from it.

Regards,
---
Takahiro Itagaki
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] Visual Studio 2005, C-language function - avoiding hacks?

2010-03-08 Thread Takahiro Itagaki

Magnus Hagander mag...@hagander.net wrote:

  The existing mechanism
  works (as demonstrated by the fact that the contrib modules work on
  Windows).
 
  Did we use non-standard tools except msvc in the build framework
  for core code? And what should I do for an external project?
 
 Yes, we use mingw.

Hmmm, it means the existing mechanism can only work on limited environments.
Should we make the code to be more portable for standard developers?

Third party developer might not build the postgres server,
but they would want to build their extension modules without mingw.

Regards,
---
Takahiro Itagaki
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


[HACKERS] ecpg compiler warning about char* comparison

2010-03-07 Thread Takahiro Itagaki
There is a complier warning in ecpg/ecpglib/error.c on HEAD:

error.c: In function 'eecpg_raise_backend':
error.c:309: warning: comparison with string literal results in unspecified 
behavior

It comes from the following coparison:
---
#define ECPG_SQLSTATE_ECPG_INTERNAL_ERROR   YE000
char   *sqlstate;
if (sqlstate == ECPG_SQLSTATE_ECPG_INTERNAL_ERROR)
---

Instead, should we use if (strcmp(...) == 0) here?

Regards,
---
Takahiro Itagaki
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] Visual Studio 2005, C-language function - avoiding hacks?

2010-03-07 Thread Takahiro Itagaki

Kevin Flanagan kevi...@linkprior.com wrote:

 1. you have to define the symbol BUILDING_DLL in your code before
 including postgres.h

No, BUILDING_DLL does not work. We use PGDLLIMPORT both exported global
variables and PG_MODULE_MAGIC/PG_FUNCTION_INFO_V1 for now, but actually
we should always use __declspec (dllexport) for the latter cases.
They are exported from *user dlls*, not the postgres' one.

I'd like to propose to define PGALWAYSEXPORT macro:
#ifdef WIN32
#define PGALWAYSEXPORT  __declspec (dllexport)
#endif
and modify PG_MODULE_MAGIC and PG_FUNCTION_INFO_V1 to use it
instead of PGDLLEXPORT.

Regards,
---
Takahiro Itagaki
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] Visual Studio 2005, C-language function - avoiding hacks?

2010-03-07 Thread Takahiro Itagaki

Tom Lane t...@sss.pgh.pa.us wrote:

 Takahiro Itagaki itagaki.takah...@oss.ntt.co.jp writes:
  I'd like to propose to define PGALWAYSEXPORT macro:
  #ifdef WIN32
  #define PGALWAYSEXPORT  __declspec (dllexport)
  #endif
  and modify PG_MODULE_MAGIC and PG_FUNCTION_INFO_V1 to use it
  instead of PGDLLEXPORT.
 
 This seems like change for the sake of change.  The existing mechanism
 works (as demonstrated by the fact that the contrib modules work on
 Windows).

I wonder why the contrib modules can be compiled correctly because:
1. PG_MODULE_MAGIC requires dllexport.
2. Other exported variables from postgres requires dllimport.
3. Exported functions from the contrib DLLs require dllexport,
   but they don't have any PGDLLEXPORT tags in their functions.

Did we use non-standard tools except msvc in the build frameword
for core code? And what should I do for an external project?

Regards,
---
Takahiro Itagaki
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] C libpq frontend library fetchsize

2010-03-01 Thread Takahiro Itagaki

Yeb Havinga yebhavi...@gmail.com wrote:

 I'm wondering if there would be community support for adding using the 
 execute message with a rownum  0 in the c libpq client library, as it 
 is used by the jdbc driver with setFetchSize.

The setFetchSize for libpq is difficult because of the interface
mismatch -- libpq uses array-based APIs (PGresult) and JDBC uses a
cursor-like API (ResultSet). Instead, you can use CURSOR and FETCH
commands to retrieve rows in separated PGresult objects.

Regards,
---
Takahiro Itagaki
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


[HACKERS] code cleanup: ss_currentScanDesc

2010-02-25 Thread Takahiro Itagaki
ScanState.ss_currentScanDesc is currently used by only SeqScan and
BitmapHeapScan. Other scan nodes don't use the field at all, right?

Can we move the field into SeqScanState and BitmapHeapScanState
for code cleanup? This change will not improve any performance,
but it can clear up what we do actually.

Regards,
---
Takahiro Itagaki
NTT Open Source Software Center


ss_currentScanDesc_20100226.patch
Description: Binary data

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


  1   2   3   >