Re: [HACKERS] query cancel issues in contrib/dblink
Robert Haas robertmh...@gmail.com wrote: I'm pretty sure it's the same as this: https://commitfest.postgresql.org/action/patch_view?id=263 Yes, (2) are resolved with the patch with a different implementation. (2) is fixed by RegisterXactCallback(AtEOXact_dblink). However, there might be any better solutions -- for example, ResourceOwner framework. (1) still exists, but we had a consensus that we don't have to fix it because we have async functions. (1) is fixed by using non-blocking APIs in libpq. I think we should always use non-blocking APIs even if the dblink function itself is a blocking-function. 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] dblink for 8.4 should work without user-mappings
Bruce Momjian br...@momjian.us wrote: What happened to this patch? It was rejected because SQL standard always requires an user mapping. I agree about the decision, too. --- Itagaki Takahiro wrote: contrib/dblink in 8.4 supports a server name by CREATE SERVER for connection string, but it always requires an user-mapping (by CREATE USER MAPPING). However, I think it should work user-mappings because it works when the connection string is passed directly. The attached patch adds 'missing_ok' parameter to GetUserMapping() and made dblink to use it. There should be no additional security issues here because dblink's original security check works even for server name mode. 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] auto_explain log_verbose causes regression failure
Bruce Momjian br...@momjian.us wrote: Uh, I don't see this patch as applied. Was it not necessary? No, the bug was reported again and fixed then with: - Force READY portals into FAILED state when a transaction or subtransaction is aborted Message-Id: 20100218030646.500a3754...@cvs.postgresql.org - Fix ExecEvalArrayRef to pass down the old value of the array element or slice being assigned to Message-Id: 20100218184147.a9ec9754...@cvs.postgresql.org --- Itagaki Takahiro wrote: Robert Haas robertmh...@gmail.com wrote: It looks like this is enough to reproduce the cache lookup failure: The cache loopup failure part could be fixed by the attached patch. It forbids explaining if the transaction is in error state. I cannot reproduce unexpected refassgnexpr and unexpected FieldStore errors yet. We might need another fix for them. 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] Directory fsync and other fun
Andres Freund and...@anarazel.de wrote: I started setting up some halfway automated method of simulating hard crashes and even while setting those up I found some pretty unsettling results... Now its not unlikely that my testing is flawed but unfortunately I don't see where right now (its 3am now and I have a 8h trainride behind me, so ...) I think the reported behavior is a TODO item to research: * Research use of fsync to a newly created directory. There is no guarantee that mkdir() flushes metadata of the directory. Also, I heard ext4 has a feature in that rename() might truncate the renamed file to zero bytes on crash. The user data in the file might be lost if the machine crashes just after rename(). * Research whether our use of rename() is safe on various file systems. Especially on ext4, the contents of the renamed file might be lost on crash. Comments and suggestion for improvements of words welcome. If no objection, I'll add them at the Fsync section in the TODO page. 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][HACKERS] function side effects
Tatsuo Ishii is...@postgresql.org wrote: VOLATILE functions such as random() and timeofday() apparently do not write and sending those queries that include such functions is overkill. Can we VOLATILE property divide into two categories, say, VOLATILE without write, and VOLATILE with write? I think it's possible. We might borrow words and semantics from unctional programming languages for functions with side effects. How do they handle the issue? BTW, random() *writes* the random seed, though no one will mind 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] A thought on Index Organized Tables
Gokulakannan Somasundaram gokul...@gmail.com wrote: a) IOT has both table and index in one structure. So no duplication of data b) With visibility maps, we have three structures a) Table b) Index c) Visibility map. So the disk footprint of the same data will be higher in postgres ( 2x + size of the visibility map). c) More than that, inserts and updates will incur two extra random i/os every time. - one for updating the table and one for updating the visibility map. I think IOT is a good match for overwrite storage systems, but postgres is a non-overwrite storage systems. If we will update rows in IOT, we need much more initial page free spaces than index-only scans where we can avoid key updates with HOT. Instead, how about excluding columns in primary keys from table data? We cannot drop those primary keys and cannot seqscan the tables, but there are no duplication of data, only small overheads (index tuple headers and ctid links), and would work well with HOT and index-only scans. If we don't have any non-key columns, that behaves just as IOT. 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] tie user processes to postmaster
Jaime Casanova jcasa...@systemguards.com.ec wrote: integrated_user_processes = 'x, y, z' API would be user_process_startup(), user_process_shutdown(). FYI, pg_statsinfo version 2 emulates the same behavior with shared_preload_libraries and spawn an user process in _PG_init(). But it's still ugly and not so reliable. Official APIs would be better. http://cvs.pgfoundry.org/cgi-bin/cvsweb.cgi/pgstatsinfo/pg_statsinfo/lib/libstatsinfo.c It came from voices from end users that an extension should behave as a postgres internal daemon rather than a wrapper of postgres. 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] A thought on Index Organized Tables
Tom Lane t...@sss.pgh.pa.us wrote: Takahiro Itagaki itagaki.takah...@oss.ntt.co.jp writes: Instead, how about excluding columns in primary keys from table data? How will you implement select * from mytable ? Or even select * from mytable where non_primary_key = something ? Use index full scans. We can do it even for now with enable_seqscan = off. Of course, it requires an additional step to merge index keys and heap tuples. Also, we're using the same technique for TOASTed values. The cost can be compared with select * from mytable where toasted_value = something, no? If you can't do either of those without great expense, I think a savings on primary-key lookups is not going to be adequate recompense. I don't think it will be the default, but it would be a reasonable trade-off for users who want IOTs, unless they often scan the whole table. 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] A thought on Index Organized Tables
Tom Lane t...@sss.pgh.pa.us wrote: Also, we're using the same technique for TOASTed values. The cost can be compared with select * from mytable where toasted_value = something, no? No, because toast pointers point in the direction you need to traverse. AFAICS, this proposal involves scanning the whole index to find the matching entry, because the available pointers link from the wrong end, that is from the index to the table. Ah, I see there are differences if we have secondary indexes. I misunderstood that the toast case requires scanning the whole *table* to find the matching entry and should be compared with the whole *index* scans, but there is another story if we have secondary indexes. We can have indexes on toasted values, and find the related tuples directly with CTIDs, but scans on secondary indexes on PK-excluded tables requires not only heap tuples but also primary key values. The secondary index issue should be considered also with the original IOT proposal also has the same issue. Using PK-values instead of CTIDs might require many changes in index access methods and/or the executor. 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] PGXS: REGRESS_OPTS=--load-language=plpgsql
David Fetter da...@fetter.org wrote: Any external projects will fail on this if they're attempting to support both pre-9.0 and post-9.0 PostgreSQLs. David Wheeler has suggested that we special-case PL/pgsql for 9.0 and greater, as it's in template0, where those tests are based. +1 for the CREATE LANGUAGE IF NOT EXISTS behavior. The regression test in the core is targeting only its version, but some external projects have version-independent tests. I hope --load-language works as ensure the language is installed rather than always install the language. 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] possible bug with inheritance?
Bruce Momjian br...@momjian.us wrote: Summary: ALTER TABLE SET NOT NULL on a parent table is passed to the child, while ALTER TABLE ADD PRIMARY KEY is not, particularly the NOT NULL part of the PRIMARY KEY specification. That does seem like something that should be fixed. Yeah, the issue is in our TODO list: http://wiki.postgresql.org/wiki/Todo | Move NOT NULL constraint information to pg_constraint | Currently NOT NULL constraints are stored in pg_attribute without | any designation of their origins, e.g. primary keys. One manifest | problem is that dropping a PRIMARY KEY constraint does not remove | the NOT NULL constraint designation. Another issue is that we should | probably force NOT NULL to be propagated from parent tables to children, | just as CHECK constraints are. (But then does dropping PRIMARY KEY | affect children?) And the same bug report has been here: http://archives.postgresql.org/message-id/200909181005.n8ia5ris061...@wwwmaster.postgresql.org | BUG #5064: not-null constraints is not inherited 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: [BUGS] BUG #4566: pg_stop_backup() reports incorrect STOP WAL LOCATION
I'd like to apply the patch to HEAD and previous releases because the issue seems to be a bug in the core. Any comments or objections? Some users actually use STOP WAL LOCATION in their backup script, and they've countered the bug with 1/256 probability in recent days. Fujii Masao masao.fu...@gmail.com wrote: On Fri, Feb 5, 2010 at 9:08 AM, Takahiro Itagaki itagaki.takah...@oss.ntt.co.jp wrote: Fujii Masao masao.fu...@gmail.com wrote: On Fri, Dec 5, 2008 at 11:41 PM, Randy Isbell jisb...@cisco.com wrote: An inconsistency exists between the segment name reported by pg_stop_backup() and the actual WAL file name. START WAL LOCATION: 10/FE1E2BAC (file 0002001000FE) STOP WAL LOCATION: 10/FF00 (file 0002001000FF) But it was rejected because its change might break the existing app. It might break existing applications if it returns FE instead of FF, but never-used filename surprises users. (IMO, the existing apps probably crash if FF returned, i.e, 1/256 of the time.) Should it return the *next* reasonable log filename instead of FF? For example, 00020020 for the above case. Here is the patch that avoids a nonexistent file name, according to Itagaki-san's suggestion. If we are crossing a logid boundary, the next reasonable file name is used instead of a nonexistent one. 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] Tightening binary receive functions
James William Pye li...@jwp.name wrote:
Need a special case for the infinities as well?
postgres=# create table foo (d date);
postgres=# INSERT INTO foo VALUES ('infinity');
postgres=# COPY foo TO '/Users/jwp/foo.copy' WITH BINARY;
postgres=# COPY foo FROM '/Users/jwp/foo.copy' WITH BINARY;
ERROR: date out of range
Exactly. Patch attached.
We have special treatments of infinity in timestamp_recv,
but don't have in date_recv.
Regards,
---
Takahiro Itagaki
NTT Open Source Software Center
date_recv_infinity_20100218.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] auto_explain causes regression failures
Andrew Dunstan and...@dunslane.net wrote: With the following settings custom_variable_classes = 'auto_explain' auto_explain.log_min_duration = 0 auto_explain.log_format = 'xml' auto_explain.log_analyze = on auto_explain.log_verbose = on shared_preload_libraries = 'auto_explain' I am getting regression failures on the rowtypes, transactions and arrays tests. Diff file is attached. I'm going to look into it, but if anyone has a good idea what's going on please speak up ASAP. Thank you for the bug report. Auto_explan tries to explain the query even if it is failed, but schema objects that are created in the same transaction might not be available. cache lookup failed erros can be avoided if auto_explain skips explaining queries in aborted transactions. The attached patch will fix the bug, but I'm not sure whether this usage of TransactionBlockStatusCode() is sane. Comments or better ideas? Regards, --- Takahiro Itagaki NTT Open Source Software Center auto_explain.fix.diff 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] [COMMITTERS] pgsql: Add psql tab completion for DO blocks.
Tom Lane t...@sss.pgh.pa.us wrote:
itag...@postgresql.org (Takahiro Itagaki) writes:
This syntax synopsis is completely nuts:
DO { [ LANGUAGE lang_name ] | code } ...
I think that it would be logically correct without the square brackets,
Oops, that's correct.
but as a matter of clarity I really doubt it's an improvement over the
original.
We cannot write down the accurate syntax with BNF, right? We can have
0..1 LANGUAGE lang_name, but must have just 1 code in any order.
How about the following description?
DO [ LANGUAGE lang_name ] code
because the psql tab completion adds LANGUAGE just after DO.
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] ToDo: preload for fulltext dictionary
Pavel Stehule pavel.steh...@gmail.com wrote: The dictionary data could be shared or minimally dictionary could be preloaded like some PL language. What do you think about this? Surely preloading is the most realistic approach, but I hope we would support dynamic allocation of shared memory, and load dictionaries in the area and share it with backends. We should avoid additonal calls of shmget() or mmap() in the additional shared memory allocation, but we can shrink shared buffers and reuse the area for general purposes. We often have serveral GB of shared buffers nowadays, so dividing some MB of buffers will not be problem. 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] psql tab-completion for new syntax
Here is a patch to support new syntax in psql tab completion and fix bugs to complete after an open parenthesis. Supported additonal syntax are: - ALTER TABLE/INDEX/TABLESPACE SET/RESET with options - ALTER TABLE ALTER COLUMN SET/RESET with options - ALTER TABLE ALTER COLUMN SET STORAGE - CREATE TRIGGER with events - CREATE INDEX CONCURRENTLY - CREATE INDEX ON (without name) - CREATE INDEX ... USING with pg_am.amname instead of hard-corded names. Fixes bugs are: Bug 1: Double parenthesis =# INSERT INTO pgbench_history VALUES (TAB =# INSERT INTO pgbench_history VALUES (( = wrong Bug 2: We cannot complete words if no whitespaces around a parenthesis. =# CREATE INDEX idx ON pgbench_history( TAB ^ no whitespace here Bug 3: should be completed with ( before columns. =# CREATE INDEX foo ON pgbench_accounts USING BTREE TAB abalance aid bid filler= wrong, should be ( I adjusted previous_word() to split words not only with spaces but also with non-alphabets, and removed a hack with find_open_parenthesis(). Comments? Regards, --- Takahiro Itagaki NTT Open Source Software Center psql-tab-completion_20100216.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] psql tab completion for DO blocks
David Fetter da...@fetter.org wrote:
DO { [ LANGUAGE lang_name ] | code } ...
Good catch :)
The tab completion patch and documentation fix were committed.
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] psql tab completion for DO blocks
David Fetter da...@fetter.org wrote: Syntax of DO command is: DO code [ LANGUAGE lang_name ] That's not the only syntax. DO [LANGUAGE lang_name] code also works, e.g.: Hmmm, but we mention only above syntax in the documentation. http://developer.postgresql.org/pgdocs/postgres/sql-do.html Should we fix the documentation when we add the tab completion? 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] Largeobject Access Controls (r2460)
KaiGai Kohei kai...@ak.jp.nec.com wrote: I don't think this is necessarily a good idea. We might decide to treat both things separately in the future and it having them represented separately in the dump would prove useful. I agree. From design perspective, the single section approach is more simple than dual section, but its change set is larger than the dual. OK. When I tested a custom dump with pg_restore, --clean --single-transaction will fail with the new dump format because it always call lo_unlink() even if the large object doesn't exist. It comes from dumpBlobItem: ! dumpBlobItem(Archive *AH, BlobInfo *binfo) ! appendPQExpBuffer(dquery, SELECT lo_unlink(%s);\n, binfo-dobj.name); The query in DropBlobIfExists() could avoid errors -- should we use it here? | SELECT lo_unlink(oid) FROM pg_largeobject_metadata WHERE oid = %s; BTW, --clean option is ambiguous if combined with --data-only. Restoring large objects fails for the above reason if previous objects don't exist, but table data are restored *without* truncation of existing data. Will normal users expect TRUNCATE-before-load for --clean --data-only cases? Present behaviors are; Table data- Appended. (--clean is ignored) Large objects - End with an error if object doesn't exist. IMO, ideal behaviors are: Table data- Truncate existing data and load new ones. Large objects - Work like as MERGE (or REPLACE, UPSERT). 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] psql tab completion for DO blocks
Bruce Momjian br...@momjian.us wrote: Where are we on this patch? We should at least implement the completion for 'LANGUAGE' in 'DO', and use the existing pg_language query for completion. I am attaching a patch that does exactly this. I don't think we need the patch except adding DO to the top-level sql_commands. Syntax of DO command is: DO code [ LANGUAGE lang_name ] We need 'code' before LANGUAGE, but the patch completes DO to DO LANGUAGE. It will be just a syntax error. Also, we've already had a completion after LANGUAGE (see words_after_create), so we don't need an additional Query_for_list_of_languages after LANGUAGE. BTW, I'm working on psql tab-completion adjustment for new syntax in 9.0. I'll send it soon. 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] psql tab-completion for new syntax
We've added some new syntax in HEAD, but psql tab-completion is out of sync. Here is a patch to support the following syntax in tab-completion: - top-level DO - ALTER TABLE/INDEX/TABLESPACE SET/RESET with options - ALTER TABLE ALTER COLUMN SET/RESET with options - CREATE TRIGGER with events The fix is not a stopper to alpha release, but I'd like to add it before beta release. Comments welcome. Regards, --- Takahiro Itagaki NTT Open Source Software Center psql-tab-completion_20100210.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] Largeobject Access Controls (r2460)
KaiGai Kohei kai...@ak.jp.nec.com wrote: The attached patch fixed up the cleanup query as follows: + appendPQExpBuffer(dquery, + SELECT pg_catalog.lo_unlink(oid) + FROM pg_catalog.pg_largeobject_metadata + WHERE oid = %s;\n, binfo-dobj.name); And, I also noticed that lo_create() was not prefixed by pg_catalog., so I also add it. Thanks. Now the patch is ready to commit. 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] TRUNCATE+COPY optimization and --jobs=1 in pg_restore
We have an optimization to bulkload date in pg_restore, but the code
only works in parallel restore (--jobs = 2). Why don't we do the
same optimization in the serial restore (--jobs = 1) ?
We checks is_parallel to decide to use BEGIN-TRUNCATE-COPY:
if (is_parallel te-created)
but we can always do it unless --single-transaction, right?
if (!ropt-single_txn te-created)
[ in restore_toc_entry() ]
/*
* In parallel restore, if we created the table earlier in
* the run then we wrap the COPY in a transaction and
* precede it with a TRUNCATE. If archiving is not on
* this prevents WAL-logging the COPY. This obtains a
* speedup similar to that from using single_txn mode in
* non-parallel restores.
*/
if (is_parallel te-created) HERE
{
/*
* Parallel restore is always talking directly to a
* server, so no need to see if we should issue BEGIN.
*/
StartTransaction(AH);
/*
* If the server version is = 8.4, make sure we issue
* TRUNCATE with ONLY so that child tables are not
* wiped.
*/
ahprintf(AH, TRUNCATE TABLE %s%s;\n\n,
(PQserverVersion(AH-connection) = 80400 ?
ONLY : ),
fmtId(te-tag));
}
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] TRUNCATE+COPY optimization and --jobs=1 in pg_restore
Tom Lane t...@sss.pgh.pa.us wrote: Takahiro Itagaki itagaki.takah...@oss.ntt.co.jp writes: We have an optimization to bulkload date in pg_restore, but the code only works in parallel restore (--jobs = 2). Why don't we do the same optimization in the serial restore (--jobs = 1) ? The code is only trying to substitute for something you can't have in parallel restore, ie --single-transaction. Yeah, the comment says so. But it does not necessarily mean that we cannot optimize the copy also in non-single-transaction restore. The attached patch improve the judgment condition, I'll add it to the next commit-fest. Regards, --- Takahiro Itagaki NTT Open Source Software Center restore-wal-skip_20100210.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] TRUNCATE+COPY optimization and --jobs=1 in pg_restore
Andrew Dunstan and...@dunslane.net wrote: Takahiro-san is suggesting there is a case for doing the optimisation in non-parallel mode. But if we do that, is there still a case for --single-transaction? I think --single-transaction is useful to restore data into non-empty databases. A normal restore ignores errors, but it might make database inconsistent state. So, we'd better keep --single-transaction option to support all-or-nothing restore. 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_restore --single-transaction and --clean
As another glitch in pg_restore, a combination of options --single-transaction and --clean raises errors if we restore data into an empty database. The reason is pg_restore uses DROP OBJECT. The cleanup command fails if the target object doesn't exist. Is it a TODO item to replace DROP into DROP IF EXISTS for cleanup commands in pg_restore? 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: [BUGS] BUG #4566: pg_stop_backup() reports incorrect STOP WAL LOCATION
Fujii Masao masao.fu...@gmail.com wrote: On Fri, Dec 5, 2008 at 11:41 PM, Randy Isbell jisb...@cisco.com wrote: An inconsistency exists between the segment name reported by pg_stop_backup() and the actual WAL file name. START WAL LOCATION: 10/FE1E2BAC (file 0002001000FE) STOP WAL LOCATION: 10/FF00 (file 0002001000FF) But it was rejected because its change might break the existing app. It might break existing applications if it returns FE instead of FF, but never-used filename surprises users. (IMO, the existing apps probably crash if FF returned, i.e, 1/256 of the time.) Should it return the *next* reasonable log filename instead of FF? For example, 00020020 for the above case. 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] Largeobject Access Controls (r2460)
KaiGai Kohei kai...@kaigai.gr.jp wrote: default:both contents and metadata --data-only:same --schema-only: neither However, it means only large object performs an exceptional object class that dumps its owner, acl and comment even if --data-only is given. Is it really what you suggested, isn't it? I wonder we still need to have both BLOB ITEM and BLOB DATA even if we will take the all-or-nothing behavior. Can we handle BLOB's owner, acl, comment and data with one entry kind? 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] Writeable CTEs patch
Marko Tiikkaja marko.tiikk...@cs.helsinki.fi wrote: Here's an updated patch. Only changes from the previous patch are fixing the above issue and a regression test for it. A brief report for of the patch: * The patch has the following error cases, and also have one regression test for each case. - DML WITH is not allowed in a cursor declaration - DML WITH is not allowed in a view definition - DML WITH without RETURNING is only allowed inside an unreferenced CTE - DML WITH is only allowed at the top level - Recursive DML WITH statements are not supported ^-- might be better if DML WITH cannot have the self-reference or so? - Conditional DO INSTEAD rules are not supported in DML WITH statements - DO ALSO rules are not supported in DML WITH statements - Multi-statement DO INSTEAD rules are not supported in DML WITH statements - DO INSTEAD NOTHING rules are not supported in DML WITH statements * In the regression tests, almost all of them don't have ORDER BY clause. They just work, but we might need ORDER BY to get robust output. What did we do in other regression tests? * I feel odd the following paragraph in the docs, but should be checked by native English speakers. *** a/doc/src/sgml/ref/create_rule.sgml --- b/doc/src/sgml/ref/create_rule.sgml *** *** 222,227 CREATE [ OR REPLACE ] RULE replaceable class=parametername/replaceable AS --- 222,234 /para para +In an literalINSERT/literal, literalUPDATE/literal or +literalDELETE/literal query within a literalWITH/literal clause, +only unconditional, single-statement literalINSTEAD/literal rules are ^-- and? which comma is the sentence separator? +implemented. ^-- might be available rather than implemented? + /para 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] Review of Writeable CTE Patch
Marko Tiikkaja marko.tiikk...@cs.helsinki.fi wrote: Here's an updated patch. This includes the fix mentioned earlier, some comment improvements and making CopySnapshot() static again. I also changed all references to this feature to DML WITH for consistency. I'm not sure if we want to keep it, but it should now be easier to change in the future. Hi, I'm reviewing the writable CTE patch. The code logic seems to be pretty good, but I have a couple of comments about error cases: * Did we have a consensus about user-visible DML WITH messages? The term is used in error messages in many places, for example: DML WITH without RETURNING is only allowed inside an unreferenced CTE Since we don't use DML WITH nor CTE in documentation, I'd like to avoid such technical acronyms in logs if we had better names, or we should have a section to explain them in docs. * What can I do to get Recursive DML WITH statements are not supported message? I get syntax errors before I get the message because We don't support UNIONs with RETURNING queries. Am I missing something? =# UPDATE tbl SET i = i + 1 WHERE i = 1 UNION ALL UPDATE tbl SET i = i + 1 WHERE i = 2; ERROR: syntax error at or near UNION * The patch includes regression tests, but no error cases in it. More test cases are needed for stupid queries. 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] Largeobject Access Controls (r2460)
KaiGai Kohei kai...@ak.jp.nec.com wrote: Can we remove such path and raise an error instead? Also, even if we support the older servers in the routine, the new bytea format will be another problem anyway. OK, I'll fix it. I think we might need to discuss about explicit version checks in pg_restore. It is not related with large objects, but with pg_restore's capability. We've not supported to restore a dump to older servers, but we don't have any version checks, right? Should we do the checks at the beginning of restoring? If we do so, LO patch could be more simplified. The --schema-only with large objects might be unnatural, but the --data-only with properties of large objects are also unnatural. Which behavior is more unnatural? I think large object metadata is a kind of row-based access controls. How do we dump and restore ACLs per rows when we support them for normal tables? We should treat LO metadata as same as row-based ACLs. In my opinion, I'd like to treat them as part of data (not of schema). 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] New VACUUM FULL crashes on temp relations
Simon Riggs si...@2ndquadrant.com wrote: TRAP: FailedAssertion(!(typeNamespace == typ-typnamespace), File: pg_type.c, Line: 658) Test case attached, repeated, consistent failure on CVS HEAD. I see the same assertion failure on 8.4.2, too. I'll investigating it... -- minimum reproducible pattern drop table if exists footemp; create temp table footemp (col1 serial, col2 text); create index footemp_col1_idx on footemp (col1); cluster footemp using footemp_col1_idx; 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] New VACUUM FULL crashes on temp relations
Takahiro Itagaki itagaki.takah...@oss.ntt.co.jp wrote: TRAP: FailedAssertion(!(typeNamespace == typ-typnamespace), File: pg_type.c, Line: 658) It comes from swapping toast relations: DEBUG: typeNamespace mismatch: 99 (pg_toast) vs. 16386 (pg_toast_temp_2) AFAICS, the assertion is broken, but the code is correct. We just need to adjust the expression in the assertion. Patch attached, including new regression tests for clustering a temp table. I see the same assertion failure on 8.4.2, too. I'll go for backpatcting if the attached fix is correct. Comments welcome. Regards, --- Takahiro Itagaki NTT Open Source Software Center cluster_assert_20100202.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] Review: listagg aggregate
Robert Haas robertmh...@gmail.com wrote: ok - I am only one who like original behave - so I ?am withdrawing. Robert, If you like, please commit Itagaki's patch. + add note about behave when second parameter isn't stable. I haven't even looked at this code - I sort of assumed Itagaki was handling this one. But it might be good to make sure that the docs have been read through by a native English speaker prior to commit... Applied with some editorialization. Please check the docs are good enough. I removed a test pattern for variable delimiters from regression test because it might be an undefined behavior. We might change the behavior in the future, so we guarantee only constant delimiters. The transition functions are renamed to string_agg_transfn and string_agg_delim_transfn. We cannot use string_agg_transfn for both names because we cast the function name as regproc in bootstrap; it complains about duplicated function names. 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] Largeobject Access Controls (r2460)
KaiGai Kohei kai...@ak.jp.nec.com wrote: The attached patch uses one TOC entry for each blob objects. This patch does not only fix the existing bugs, but also refactor the dump format of large objects in pg_dump. The new format are more similar to the format of tables: SectionTables New LOOld LO - Schema TABLE BLOB ITEM BLOBS Data TABLE DATA BLOB DATA BLOBS Comments COMMENTCOMMENT BLOB COMMENTS We will allocate BlobInfo in memory for each large object. It might consume much more memory compared with former versions if we have many large objects, but we discussed it is an acceptable change. As far as I read, the patch is almost ready to commit except the following issue about backward compatibility: * BLOB DATA This section is same as existing BLOBS section, except for _LoadBlobs() does not create a new large object before opening it with INV_WRITE, and lo_truncate() will be used instead of lo_unlink() when --clean is given. The legacy sections (BLOBS and BLOB COMMENTS) are available to read for compatibility, but newer pg_dump never create these sections. I wonder we need to support older versions in pg_restore. You add a check PQserverVersion = 80500 in CleanupBlobIfExists(), but out documentation says we cannot use pg_restore 9.0 for 8.4 or older servers: http://developer.postgresql.org/pgdocs/postgres/app-pgdump.html | it is not guaranteed that pg_dump's output can be loaded into | a server of an older major version Can we remove such path and raise an error instead? Also, even if we support the older servers in the routine, the new bytea format will be another problem anyway. One remained issue is the way to decide whether blobs to be dumped, or not. Right now, --schema-only eliminate all the blob dumps. However, I think it should follow the manner in any other object classes. -a, --data-only... only BLOB DATA sections, not BLOB ITEM -s, --schema-only ... only BLOB ITEM sections, not BLOB DATA -b, --blobs... both of BLOB ITEM and BLOB DATA independently from --data-only and --schema-only? I cannot image situations that require data-only dumps -- for example, restoring database has a filled pg_largeobject_metadata and an empty or broken pg_largeobject -- but it seems to be unnatural. I'd prefer to keep the existing behavior: * default or data-only : dump all attributes and data of blobs * schema-only : don't dump any blobs and have independent options to control blob dumps: * -b, --blobs: dump all blobs even if schema-only * -B, --no-blobs : [NEW] don't dump any blobs even if default or data-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] Review: listagg aggregate
Pavel Stehule pavel.steh...@gmail.com wrote:
2010/1/28 Pavel Stehule pavel.steh...@gmail.com:
2010/1/28 David E. Wheeler da...@kineticode.com:
On Jan 27, 2010, at 6:47 PM, Takahiro Itagaki wrote:
* I think we cannot cache the delimiter at the first call.
For example,
SELECT string_agg(elem, delim)
FROM (VALUES('A', ','), ('B', '+'), ('C', '*')) t(elem, delim);
should return 'A+B*C' rather than 'A,B,C'.
no, has not.
What is use case for this behave??
I also think this usage is nonsense, but seems to be the most consistent
behavior for me. I didn't say anything about use-cases, but just capability.
Since we allow such kinds of usage for now, you need to verify the
delimiter is not changed rather than ignoring it if you want disallow
to change the delimiter during an aggregation.
Of course you can cache the first delimiter at start, and check delimiters
are not changed every calls -- but I think it is just a waste of cpu cycle.
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] Largeobject Access Controls (r2460)
KaiGai Kohei kai...@ak.jp.nec.com wrote: The attached patch uses one TOC entry for each blob objects. When I'm testing the new patch, I found ALTER LARGE OBJECT command returns ALTER LARGEOBJECT tag. Should it be ALTER LARGE(space)OBJECT instead? As I remember, we had decided not to use LARGEOBJECT (without a space) in user-visible messages, 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] Largeobject Access Controls (r2460)
KaiGai Kohei kai...@ak.jp.nec.com wrote: When I'm testing the new patch, I found ALTER LARGE OBJECT command returns ALTER LARGEOBJECT tag. Should it be ALTER LARGE(space)OBJECT instead? Sorry, I left for fix this tag when I was pointed out LARGEOBJECT should be LARGE(space)OBJECT. Committed. 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] 64-bit size pgbench
Greg Smith g...@2ndquadrant.com wrote: Attached is a patch that fixes a long standing bug in pgbench: it won't handle scale factors above ~4000 (around 60GB) because it uses 32-bit integers for its computations related to the number of accounts, and it just crashes badly when you exceed that. This month I've run into two systems where that was barely enough to exceed physical RAM, so I'd expect this to be a significant limiting factor during 9.0's lifetime. A few people have complained about it already in 8.4. +1 for the fix. Do we also need to adjust tuples done messages during dataload? It would be too verbose for large scale factor. I think a message every 1% is reasonable. if (j % 1 == 0) fprintf(stderr, INT64_FORMAT tuples done.\n, j); 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] Review: listagg aggregate
Pavel Stehule pavel.steh...@gmail.com wrote:
with actualised oids
I'm checking the patch for commit, and have a couple of comments.
* I think we cannot cache the delimiter at the first call.
For example,
SELECT string_agg(elem, delim)
FROM (VALUES('A', ','), ('B', '+'), ('C', '*')) t(elem, delim);
should return 'A+B*C' rather than 'A,B,C'.
* Can we use StringInfo directly as the aggregate context instead of
StringAggState? For the first reason, we need to drop 'delimiter' field
from struct StringAggState. Now it has only StringInfo field.
* We'd better avoiding to call text_to_cstring() for delimitors and elements
for performance reason. We can use appendBinaryStringInfo() here.
My proposal patch attached.
Also, I've not changed it yet, but it might be considerable:
* Do we need better names for string_agg1_transfn and string_agg2_transfn?
They are almost internal names, but we could have more
like string_agg_with_sep_transfn.
Comments?
Regards,
---
Takahiro Itagaki
NTT Open Source Software Center
string_agg_20100128.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] Syntax supplements for SET options
Robert Haas robertmh...@gmail.com wrote: With respect to tab completion, ALTER TABLESPACE x currently completes with only OWNER TO or RENAME TO; we need to add SET to that list. My bad. Ok, I'll go for it. I see non-tab-completion parts are too late for 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
[HACKERS] Syntax supplements for SET options
I found a couple of incomplete syntax when I was adjusting psql automatic tab completion for HEAD. 1. We cannot specify tablespace options on CREATE TABLESPACE though ALTER TABLESPACE supports generic options by SET (...). 2. Should we still support ALTER COLUMN SET STATISTICS though we can set generic options with ALTER COLUMN SET (...) ? 3. We cannot specify column options on CREATE TABLE though ALTER TABLE ALTER COLUMN SET ... supports it. Which item should we fix? I think the 1st is simple to be fixed if needed. Since the 2nd was added during alpha, we could revert it if needed. The 3rd is relatively hard to fix because we need to adjust the syntax all of the kinds of columns options - STATISTICS, STORAGE, and generic options - without keywords confliction. 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] Mammoth in Core?
Tatsuo Ishii is...@postgresql.org wrote: splitting existing projects into some 'modules', and getting the modules one by one into core was not the concluion, actually. For example, see below from above URL: This means that we expect PostgreSQL exports it's parser so that existing cluster softwares can use it. Not opposite direction. I think they says the same practically -- at least have the same impact. It says postgres need to export the the internal feature *only for* some of external cluster softwares. So, if you are thinking about exporting some features from the core, the exported features would better to be stable enough and shared by several third-party tools. 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] Mammoth in Core?
Tom Lane t...@sss.pgh.pa.us wrote: It's going to be a really, really, *really* hard sell to get us to export any sort of external API to the parser internals. At least if by API you mean something other than we will whack this around to an indefinite degree on no notice, and don't even think about complaining. What exactly is the goal that you think such a thing would serve, anyway? The fragments on the referenced web page don't leave me with any warm feelings about how well the idea has been thought through. Some of items in the referenced web page are just voted results form cluster projects. At this time, we should read them as what is needed, but not how to do it. They have been not reviewd yet and not well-considered to be official TODO items. I知 not sure what pgpool team think about, but I do NOT intend to export the existing internal functions as-is. As for my personal goal, I think pgpool should be re-implemented on the layers of SQL/MED FDW or planner/executor hooks. I'd say the SQL/MED FDW apporach is one by one into core (from projects), and the hook apporach is external API (from 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] About Our CLUSTER implementation is pessimal patch
Leonardo F m_li...@yahoo.it wrote: Anyone? I'd like some feedback before moving on to do the seq scan + sort in those CLUSTER cases where use_index_scan returns false... +1 for CLUSTER using sort. I have a couple of comments for the current implementation: * Do we need to disable sort-path for tables clustered on a gist index? * I'd prefer to separate cost calculation routines from create_index_path() and cost_sort(), rather than using a dummy planner. 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] Largeobject Access Controls (r2460)
KaiGai Kohei kai...@ak.jp.nec.com wrote: I'm not sure whether we need to make groups for each owner of large objects. If I remember right, the primary issue was separating routines for dump BLOB ACLS from routines for BLOB COMMENTS, right? Why did you make the change? When --use-set-session-authorization is specified, pg_restore tries to change database role of the current session just before creation of database objects to be restored. Ownership of the database objects are recorded in the section header, and it informs pg_restore who should be owner of the objects to be restored in this section. Then, pg_restore can generate ALTER xxx OWNER TO after creation, or SET SESSION AUTHORIZATION before creation in runtime. So, we cannot put creation of largeobjects with different ownership in same section. It is the reason why we have to group largeobjects by database user. Ah, I see. Then... What happen if we drop or rename roles who have large objects during pg_dump? Does the patch still work? It uses pg_get_userbyid(). 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] About Our CLUSTER implementation is pessimal patch
Tom Lane t...@sss.pgh.pa.us wrote: What I do think is that the quoted code snippet has no business being outside the planner proper. It'd be better to put it in planner.c or someplace like that. Ah, I see. My concern was the dummy planner approach is using internal functions of planner. It would be better if planner module exports a cost estimate function for cluster. 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
Robert Haas robertmh...@gmail.com wrote: people get bogged down and don't have time to finish the work. Ok, I moved this patch to the next commit fest for 9.1 alpha 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] Fix auto-prepare #2
Boszormenyi Zoltan z...@cybertec.at wrote: I only wanted to call ECPGprepare() in case it wasn't already prepared. ECPGprepare() also checks for the statement being already prepared with ecpg_find_prepared_statement() but in case it exists it DEALLOCATEs the statement and PREPAREs again so there's would be no saving for auto-prepare calling it unconditionally and we are doing a little extra work by calling ecpg_find_prepared_statement() twice. We need a common function shared by ECPGprepare() and ecpg_auto_prepare() to not do extra work in the auto-prepare case. The attached patch implements this and also your leak fixes plus includes your change for the autoprep.pgc regression test. Good. I think the patch is ready to commit. A comment for committer (Michael?) : I was cofused by the AddStmtToCache's 2nd argument char *stmtID because it doesn't have a const. Should it be const char * ? If the argument has a const, callers assume that they can pass a not-strdup'ed string as the argument. 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] Largeobject Access Controls (r2460)
KaiGai Kohei kai...@ak.jp.nec.com wrote: This patch renamed the hasBlobs() by getBlobs(), and changed its purpose. It registers DO_BLOBS, DO_BLOB_COMMENTS and DO_BLOB_ACLS for each large objects owners, if necessary. This patch adds DumpableObjectType DO_BLOB_ACLS and struct BlobsInfo. We use three BlobsInfo objects for DO_BLOBS, DO_BLOB_COMMENTS, and DO_BLOB_ACLS _for each distinct owners_ of large objects. So, even if we have many large objects in the database, we just keep at most (3 * num-of-roles) BlobsInfo in memory. For older versions of server, we assume that blobs are owned by only one user with an empty name. We have no BlobsInfo if no large objects. I'm not sure whether we need to make groups for each owner of large objects. If I remember right, the primary issue was separating routines for dump BLOB ACLS from routines for BLOB COMMENTS, right? Why did you make the change? Another concern is their confusable identifier names -- getBlobs() returns BlobsInfo for each owners. Could we rename them something like getBlobOwners() and BlobOwnerInfo? Also, DumpableObject.name is not used in BlobsInfo. We could reuse DumpableObject.name instead of the rolname field in BlobsInfo. 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][HACKERS] Mammoth in Core?
Joshua D. Drake j...@commandprompt.com wrote: My question is, do we have any interest in working on getting this into core? We had a discussion how replication projects work together with the core in the developer meeting on PGCon 2009 Japan. http://wiki.postgresql.org/wiki/PGCon2009JapanClusterDeveloperMeeting The conclusion is splitting existing projects into some 'modules', and getting the modules one by one into core. Voted features are here: http://wiki.postgresql.org/wiki/ClusterFeatures Mammoth Replicator seems to modify the core heavily, no? I hope you would split the mammoth patch into several independent features, and submit them separately. It is much better if some of them can be shared by other replication projects. 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] Pretty printed trigger in psql
Brad T. Sliger b...@sliger.org wrote: I tried to apply this patch to the latest version of PostgreSQL in git (bbfc96e). Some of the patch did not apply. Please find attached the output from patch. The full path of the ruleutils.c.rej is src/backend/utils/adt/ruleutils.c.rej The attached patch is rebased to current CVS. Regards, --- Takahiro Itagaki NTT Open Source Software Center pretty-printed-trigger_20100119.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] Fix auto-prepare #2
Hi, I'm reviewing your patch and have a couple of comments. Boszormenyi Zoltan z...@cybertec.at wrote: we have found that auto-prepare causes a problem if the connection is closed and re-opened and the previously prepared query is issued again. You didn't attach actual test cases for the bug, so I verified it by executing the routines twice in ecpg/test/preproc/autoprep.pgc. The attached 6-pg85-fix-auto-prepare-multiconn-3-test.patch is an additional regression test for it. Is it enough for your case? This fix is that after looking up the query and having it found in the cache we also have to check whether this query was prepared in the current connection. The attached patch implements this fix and solves the problem described above. Also, the missing return false; is also added to ECPGdo() in execute.c. In addition to the two issues, I found memory leaks by careless calls of ecpg_strdup() in ecpg_auto_prepare(). Prepared stetements also might leak in a error path. I tryd to fix both of them in the attached 6-pg85-fix-auto-prepare-multiconn-3.patch, but please recheck the issues. Regards, --- Takahiro Itagaki NTT Open Source Software Center 6-pg85-fix-auto-prepare-multiconn-3.patch Description: Binary data 6-pg85-fix-auto-prepare-multiconn-3-test.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] plpgsql: open for execute - add USING clause
Pavel Stehule pavel.steh...@gmail.com wrote: ok, I accept all comments. revised version are attached. Good. This patch is ready to commit. I'll do it soon if no objections. BTW, I found inconsistent parameter dumps in the codes. Some of them add '$', but others does not. Are they intentional? Or, should we adjust them to use one of the formats? [pl_funcs.c] dump_dynexecute() dump_raise() printf(parameter %d: , i++); dump_dynfors() dump_open() dump_return_query() printf(parameter $%d: , i++); 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] improving log management
Euler Taveira de Oliveira eu...@timbira.com wrote: other log management softwares have a way to do that so why our logger doesn't have such capability? I want to propose a new command to be execute after the log file is rotated. A GUC parameter log_after_rotation_command that takes a (set of) command(s) that will be executed after a log file is rotated. If you have better loggers already, why don't you use them? In another word, should we cooperate with them instead of re-inventing alternative loggers? We have Logging Brainstorm topic in out wiki. It might help you. http://wiki.postgresql.org/wiki/Logging_Brainstorm 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] Pretty printed trigger in psql
Tom Lane t...@sss.pgh.pa.us wrote: Takahiro Itagaki itagaki.takah...@oss.ntt.co.jp writes: The attached patch eliminates unneeded parentheses by using pg_get_triggerdef(pretty = true) in psql. Is this patch reversed? It seems so but the listed file timestamps don't match that idea ... Sorry, I cannot understand what you mean... My English might be broken. May I explain what I did again? 1. psql has been used pg_get_triggerdef(oid). 2. I added pg_get_triggerdef(oid, pretty = false) at the last commit fest for pg_dump to support dumping triggeres with WHEN cluase. In that time, PRETTYFLAG_PAREN and PRETTYFLAG_INDENT are used when pretty = true. 3 psql still uses pg_get_triggerdef(oid [, pretty=false] ). Also, pg_dump should use (pretty=false) for safer migration. No programs use pg_get_triggerdef(pretty=true) is for now. 4. psql will be better to use pg_get_triggerdef(oid, true) to display trigger definitions cleanly, but it also should print them in one line. For the purpose, the patch changes two things: - Modify psql to use pg_get_triggerdef(oid, true) when server version = 8.5. - Remove PRETTYFLAG_INDENT from pg_get_triggerdef(). It will partially revert the changes in 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
[HACKERS] Pretty printed trigger in psql
Psql shows too many parentheses when it prints triggers with WHEN clause. postgres=# \d t1 Table public.t1 Column | Type | Modifiers +-+--- c1 | integer | Triggers: mytrig AFTER UPDATE ON t1 FOR EACH ROW WHEN ((old.c1 new.c1)) EXECUTE PROCEDURE myfunc() ^^ The attached patch eliminates unneeded parentheses by using pg_get_triggerdef(pretty = true) in psql. Triggers: mytrig AFTER UPDATE ON t1 FOR EACH ROW WHEN (old.c1 new.c1) EXECUTE PROCEDURE myfunc() I think this change is harmless because we don't use pg_get_triggerdef(pretty = true) in any programs, including pg_dump. Is this change ok? Regards, --- Takahiro Itagaki NTT Open Source Software Center pretty-printed-trigger_20100112.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] plpgsql: open for execute - add USING clause
Hi, I'm reviewing OPEN FOR EXECUTE USING patch and have a couple of trivial comments. Pavel Stehule pavel.steh...@gmail.com wrote: this small patch add missing USING clause to OPEN FOR EXECUTE statement + cleaning part of exec_stmt_open function - Can we use read_sql_expression2() instead of read_sql_construct() in gram.y? It could simplify the code a bit. - I'd prefer to change the new argument for exec_dynquery_with_params() from char *portalname to const char *curname. Other than the above minor issues, this patch is ready to commit. 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] NOT NULL violation and error-message
Andreas Joseph Krogh andr...@officenet.no wrote: ERROR: null value in column created violates not-null constraint It is easy to add the table name to the message, but ... ERROR: null value in column public.mytable.created violates not-null constraint Oracle does this btw... Do we have any guideline about the message for identifier names? We've already had serveral table.column messages, but schema.table.column might be preferred if there are tables with the same name in different schema. In addition, separated quotes (schema.table.column) are more SQL-ish than single outer quotes. Which should we use? At any rate, we need to adjust many regression test and .po files if we change such kinds of messages. Index: src/backend/executor/execMain.c === --- src/backend/executor/execMain.c (HEAD) +++ src/backend/executor/execMain.c (fixed) @@ -1316,7 +1316,8 @@ slot_attisnull(slot, attrChk)) ereport(ERROR, (errcode(ERRCODE_NOT_NULL_VIOLATION), -errmsg(null value in column \%s\ violates not-null constraint, +errmsg(null value in column \%s.%s\ violates not-null constraint, + RelationGetRelationName(rel), NameStr(rel-rd_att-attrs[attrChk - 1]-attname; } } 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] Fix for memory leak in dblink
There is a memory leak in dblink when we cancel a query during returning tuples. It could leak a PGresult because memory used by it is not palloc'ed one. I wrote a patch[1] before, but I've badly used global variables to track the resource. The attached is a cleaned up patch rewritten to use a tuplestore (SFRM_Materialize mode) to return tuples suggested at [2]. Since we don't return from the dblink function in tuplestore mode, we can surely release the PGresult with a PG_CATCH block even on error. Also, dblink_record_internal() and dblink_fetch() are rearranged to share the same code to return tuples for code refactoring. [1] http://archives.postgresql.org/pgsql-hackers/2009-06/msg01358.php [2] http://archives.postgresql.org/pgsql-hackers/2009-10/msg00292.php Regards, --- Takahiro Itagaki NTT Open Source Software Center dblink-memleak_20100112.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] new full vacuum doesn't work
Pavel Stehule pavel.steh...@gmail.com wrote:
I am testing vacuum changes, and I found some strange behave:
Did you need SET (fillfactor=100) before vACUUM FULL?
=# select * from pgstattuple('pgbench_accounts');
-[ RECORD 1 ]--+---
table_len | 1365336064
tuple_count| 100
tuple_len | 12100
tuple_percent | 8.86
dead_tuple_count | 0
dead_tuple_len | 0
dead_tuple_percent | 0
free_space | 1228669388
free_percent | 89.99
=# ALTER TABLE pgbench_accounts SET (fillfactor=100);
ALTER TABLE
=# vacuum full pgbench_accounts;
VACUUM
=# select * from pgstattuple('pgbench_accounts');
-[ RECORD 1 ]--+--
table_len | 134299648
tuple_count| 100
tuple_len | 12800
tuple_percent | 95.31
dead_tuple_count | 0
dead_tuple_len | 0
dead_tuple_percent | 0
free_space | 1840616
free_percent | 1.37
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] new full vacuum doesn't work
Pavel Stehule pavel.steh...@gmail.com wrote: Personally I thing, so this behave is bad. Or there is wrong default fillfactor 0. No, you used fillfactor=10 here: [pa...@nemesis src]$ /usr/local/pgsql/bin/pgbench -i -F 10 -s 10 test ~ Pgbench sets the table's fillfactor to 10. 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] Buffer statistics for pg_stat_statements
Tom Lane t...@sss.pgh.pa.us wrote: I don't necessarily know what the right thing to do with the new ones is, but I am pretty sure that pg_indent will revert any changes you make to the existing ones. That it will. The proposed changes to the existing lines are an exercise in uselessness; and to the extent that you format the added lines with this layout in mind, the final result could be worse than what you'd get if you adapt to pg_indent's rules to start with. Here is the proposed patch to adjust white spaces. It does not indent variables, but indents comments of the variables to adjust other fields. Are those changes ok? Regards, --- Takahiro Itagaki NTT Open Source Software Center pg_stat_statements_bufusage_20100107.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] Buffer statistics for pg_stat_statements
Robert Haas robertmh...@gmail.com wrote: I think so. I'm not sure if it will push out the comment that is immediately adjacent to the trailing semicolon, but I don't think it will decrease the indent on the ones you've indented more. I think this is close enough for now and you should go ahead and commit it. Thanks for your review. I committed 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] Type modifiers for DOMAIN
Tom Lane t...@sss.pgh.pa.us wrote: Because the domain is supposed to be opaque as to exactly what its underlying type is. In particular, you're supposed to do this: CREATE DOMAIN varchar2 AS pg_catalog.varchar(10); If you look in the SQL spec you will not find any suggestion that it should work the way you propose. Hmmm, it means we need to create domains for each length of character types. If we allowed domains with pass-through-modifiers, it could save codes than CREATE (scalar) TYPE. =# CREATE DOMAIN digit_varchar AS varchar ( pass-through-modifiers ) CHECK (VALUE ~ E'^\\d*$'); =# CREATE TABLE tbl (digit10 digit_varchar(10)); 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] Bug with PATHs having non-ASCII characters
Chuck McDevitt cmcdev...@greenplum.com wrote: Just an FYI regarding this bug: http://archives.postgresql.org/pgsql-bugs/2009-12/msg00267.php The wide-char version of any WIN32 API call will accept or return data in UTF-16 encoded Unicode, regardless of the local environment's single-byte (MBCS) encoding settings (codepage). I have a Windows-specific patch for open(), attached for reference. But we need to consider about other issues: - We need to consider about not only only open(), but also opendir(), stat() and symlink(). - An entirely-different fix is needed for non-Windows platforms. Probably we will convert encodings from GetDatabaseEncoding() to GetPlatformEncoding() in MBCS, but this is not needed on Windows. We should consider avoiding random ifdef blocks for the switching. - Those conversions are not free. We might need to avoid conversions for paths under $PGDATA because we only use ascii names in the server. I used a test with IS_HIGHBIT_SET in the attached patch, but I'm not sure whether it is the best method. Regards, --- Takahiro Itagaki NTT Open Source Software Center pgwin32_open.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] New VACUUM FULL
Simon Riggs si...@2ndquadrant.com wrote: 1. Commit your patch, as-is (you/me) I assume this is OK with you now? I just applied the patch with a few additional comments. Also, I adjusted some messages for vacuumdb to be look-alike for recently-committed --only-analyze patch. Remaining ToDo items are: 2. Work on infrastructure for VFC (VACUUM FULL using CLUSTER) for system relations (Simon) 3. Enable CLUSTER and REINDEX on critical system catalogs (Itagaki) 4. Optimise VFC, as discussed earlier (Itagaki) and we might also need: 5. Make CLUSTER VERBOSE to be more verbose because VACUUM FULL VERBOSE is less verbose than VFI VERBOSE for now. 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] Type modifiers for DOMAIN
Hi, I'm trying to use DOMAIN as just synonym types for database migration. For example, =# CREATE DOMAIN varchar2 AS pg_catalog.varchar; =# CREATE DOMAIN number AS pg_catalog.numeric; Domains were created successfully, but I cannot use type modifiers for them. =# CREATE TABLE tbl (v varchar2(10)); ERROR: type modifier is not allowed for type varchar2 What reason do we have not to inherit typmodin/typmodout from the base type? I found a comment in DefineDomain(), /* Domains never accept typmods, so no typmodin/typmodout needed */ but can we relax the restriction? This feature would be useful for migration from other DBMSes that have non-standard data types. 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] Verifying variable names in pgbench
Takahiro Itagaki itagaki.takah...@oss.ntt.co.jp wrote: We can remove the complexity if we give up showing the command (arg0) in error messages. Shall we remove it? Simplified patch attached. Here is the proposal for the arg0 issue. I added context argument to putVariable(). The context is a command name for \setXXX commands, 'option' for -D option, or 'startup' for internal assignment in startup. Regards, --- Takahiro Itagaki NTT Open Source Software Center pgbench_verify_varname_20100105.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] Verifying variable names in pgbench
Robert Haas robertmh...@gmail.com wrote: What is currently done for other, similar error messages? Current error messages are: for commands: context: out of memory for others : Couldn't allocate memory for variable The new message is: context: out of memory for variable 'name' 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] Verifying variable names in pgbench
Robert Haas robertmh...@gmail.com wrote: The attached patch verifies variable names at definition. $ pgbench -D var:name=value (global): invalid variable name 'var:name' I have reviewed this patch. I think that the basic idea of rejecting invalid variable names is probably a good one, but I'm not totally happy with the implementation. In particular: 1. The code that prints the invalid variable name message seems bizarrely complex and inexplicable relative to the way errors are handled elsewhere in the code. If we really need to do this, it should be in its own function, not buried inside putVariable(), but isn't there some simpler alternative? We can remove the complexity if we give up showing the command (arg0) in error messages. Shall we remove it? Simplified patch attached. 2. I think it would be worth abstracting the actual test into a separate function, like isLegalVariableName(). 3. In the department of nitpicking, I believe that the for loop test should be written as something like name[i] != '\0' rather than just name[i], for clarity. Adjusted. Regards, --- Takahiro Itagaki NTT Open Source Software Center pgbench_verify_varname_20100104.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] pg_read_file() and non-ascii input file
Robert Haas robertmh...@gmail.com wrote:
If we want to keep backward compatibility, the issue can be fixed
by adding pg_verifymbstr() to the function.
I don't feel good about changing the return type of an existing
function, so I guess +1 from me on the approach quoted above.
Ok, I just added pg_verifymbstr() instead of changing the result type.
I didn't add any additinal file reading functions in the patch, but
I'm willing to add them if someone want them:
- pg_read_file_with_encoding()
- pg_read_binary_file() RETURNS bytea
- pg_read_text_file() RETURNS SETOF text -- returns set of lines
One thing bothering me is the HINT message on error is just pointless;
The encoding is controlled by server_encoding here. We will have the
same error message in server-side COPY commands. We'd better improving
the message, though it should be done by another patch.
=# SELECT pg_read_file('invalid.txt', 0, (pg_stat_file('invalid.txt')).size);
ERROR: invalid byte sequence for encoding UTF8: 0x93
HINT: This error can also happen if the byte sequence does not match the
encoding expected by the server, which is controlled by
client_encoding.
=# COPY tbl FROM 'invalid.txt'; -- server-side copy from a local file.
(the same message -- but the encoding should match with server_encoding)
Regards,
---
Takahiro Itagaki
NTT Open Source Software Center
pg_read_file_20100104.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] New VACUUM FULL
Robert Haas robertmh...@gmail.com wrote: So, what is the roadmap for getting this done? It seems like to get rid of VFI completely, we would need to implement something like what Tom described here: http://archives.postgresql.org/pgsql-hackers/2009-09/msg00249.php I'm not sure whether the current patch is a good intermediate step towards that ultimate goal, or whether events have overtaken it. I think the most desirable roadmap is: 1. Enable CLUSTER to non-critical system catalogs. 2. Also enable CLUSTER and REINDEX to critical system catalogs. 3. Remove VFI and re-implement VACUUM FULL with CLUSTER-based approach. It should be also optimized as Simon's suggestion. My patch was intended to do 3, but we should not skip 1 and 2. In the roadmap, we don't have two versions of VACUUM FULL (INPLACE and REWRITE) at a time. I think we can do 1 immediately. The comment in cluster says might work, and I also think so. CLUSTERable toast tables are obviously useful. /* * Disallow clustering system relations. This will definitely NOT work * for shared relations (we have no way to update pg_class rows in other * databases), nor for nailed-in-cache relations (the relfilenode values * for those are hardwired, see relcache.c). It might work for other * system relations, but I ain't gonna risk it. */ For 2, we need some kinds of relfilenode mapper for shared relations and critical local tables (pg_class, pg_attribute, pg_proc, and pg_type). I'm thinking that we only store virtual relfilenodes for them in pg_class and remember the actual relfilenodes in shared memory. For example, smgropen(1248:pg_database) is redirected to smgropen(mapper[1248]). Since we cannot touch pg_class in non-login databases, we need to avoid updating pg_class when we assign new relfilenodes for shared relations. We also need to store the nodes in additional flat file. There might be another approach to store them in control file for shared relation (ControlFileData.shared_relfilenode_mapper as Oid[]), or pg_database for local tables (pg_database.datclsssnode, datprocnode etc.) What approach would be better? 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] Buffer statistics for pg_stat_statements
Robert Haas robertmh...@gmail.com wrote: I have reviewed this patch and I think it looks pretty good. A couple of minor nits: - There are needless whitespace changes in the definition of struct Counters. The changes to the existing four members should be reverted, and the new members should be made to match the existing members. That's because the 'shared_blks_written' field is too long to keep the existing indentations. Since we still have some rooms in 80 columns, I'd like to change all of them as the previous patch. - In the part that reads /* calc differences of buffer counters */, all the lines go past 80 columns. I wonder if it would be better to insert a line break just after the equals sign and indent the next line by an extra tab stop. See, e.g. src/backend/commands/user.c line 338. Ok, I'll adjust them so. 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] Verifying variable names in pgbench
We can define variables with any names in pgbench, but only can refer them with names that consist of [A-Za-z0-9_]+ . It could cause confusion discussed here: http://archives.postgresql.org/message-id/4b272833.8080...@2ndquadrant.com The attached patch verifies variable names at definition. $ pgbench -D var:name=value (global): invalid variable name 'var:name' It would help users to determine why their custom scripts failed when they misuse \setXXX :varname (the colon should be removed). Regards, --- Takahiro Itagaki NTT Open Source Software Center pgbench_verify_varname_20091228.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] Buffer statistics for pg_stat_statements
Cedric Villemain cedric.villem...@dalibo.com wrote: Le vendredi 18 decembre 2009 09:44:40, Takahiro Itagaki a ecrit : I'd like to add per-query buffer usage into contrib/pg_stat_statements. Here is a patch to add buffer usage columns into pg_stat_statements. It also changes initialzation of the result TupleDesc from manually coded routines to get_call_result_type(). Perhaps it can be usefull to have the percentage for hit/read ratio computed in the view ? I think different DBAs want different derived values; Some of them might want the buffer hit ratio, but others might request numbers per query. I'd like to privide only raw values from pg_stat_statements to keep it simple, but I added a computational expression of hit percentage in the documentation. ! bench=# SELECT query, calls, total_time, rows, 100.0 * shared_blks_hit / !nullif(shared_blks_hit + shared_blks_read, 0) AS hit_percent ! FROM pg_stat_statements ORDER BY total_time DESC LIMIT 5; ! -[ RECORD 1 ]- ! query | UPDATE pgbench_branches SET bbalance = bbalance + $1 WHERE bid = $2; ! calls | 3000 ! total_time | 9.6090010002 ! rows| 2836 ! hit_percent | 99.977897200936 Regards, --- Takahiro Itagaki NTT Open Source Software Center pg_stat_statements_bufusage_20091222.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] New VACUUM FULL
Simon Riggs si...@2ndquadrant.com wrote: I think we either need to implement that or document that vacuum will not skip all-visible pages when running VACUUM FULL. All-visible does not always mean filled enough, no? We will need to check both visibility and fillfactor when we optimize copying tuples. Old VACUUM FULL was substantially faster than this on tables that had nothing to remove. Yeah, that's why traditional VACUUM FULL is still kept as INPLACE vacuum. Please can you arrange for the cluster operation to skip rebuilding indexes if the table is not reduced in size? Do you think we should dispose the rewritten table when we find the VACUUM FULL (or CLUSTER) is useless? We could save the time to reindex, but we've already consumed time to rewrite tables. It will be still slower than VACUUM FULL INPLACE because it is read-only. Instead, I'd suggest to have conditional database-wide maintenance commands, something like: VACUUM FULL WHERE the table is fragmented We don't have to support the feature by SQL syntax; it could be done in client tools. How about pg_maintain or pg_ctl maintain that cleans up each relation with appropriate command? We could replace vacuumdb, clusterdb, and reindexdb with it then. Part of the reason why these happen is that the code hasn't been refactored much at all from its original use for cluster. There are almost zero comments to explain the additional use of this code for VACUUM, or at least to explain it still all works even when there is no index. Ok, I'll check for additional comments. The flow of code might be still confusable because vacuum() calls cluster(), but we need major replacement of codes to refactor it. I'm not sure we need the code rewrites for it. Also, I think we need additional messages for VACUUM VERBOSE (and CLUSTER VERBOSE), though it might be adjusted in another patch. 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] New VACUUM FULL
Simon Riggs si...@2ndquadrant.com wrote: Our perception of acceptable change is much higher than most users. If we tell people use VACUUM FULL or vacuumdb -f, then that command should, if possible, continue to work well across many releases. vacuumdb in most people's minds is the command you run to ease maintenance and make everything right, rather than a specific set of features. We have It just works as a principle. I think the corollary of that is that we should also have It just continues to work the same way. I used VACUUM FULL because we were discussing to drop VFI completely, but I won't replace the behavior if hot-standby can support VFI. We can use any keywords without making it reserved in VACUUM (...) syntax. VACUUM (REWRITE), the first idea, can be used here. We can also take on entirely-different syntax for it -- ex, ALTER TABLE REWRITE or SHRINK. I think the ALTER TABLE idea is not so bad because it does _not_ support database-wide maintenance. REWRITE is not the best maintenance in normal cases because a database should contain some rarely-updated tables. 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] LIKE INCLUDING COMMENTS code is a flight of fancy
Tom Lane t...@sss.pgh.pa.us wrote: I suggest that we might want to just rip out the support for copying comments on indexes. Or maybe even the whole copy-comments aspect of it. We have two related ToDo items below. They are a bit inconsintent, but they mean we should forbid COMMENT on columns of an index, or must have full-support of the feature. Which direction should we go? As for me, forbidding comments on index columns seems to be a saner way because index can have arbitrary key names in some cases. - Forbid COMMENT on columns of an index Postgres currently allows comments to be placed on the columns of an index, but pg_dump doesn't handle them and the column names themselves are implementation-dependent. http://archives.postgresql.org/message-id/27676.1237906...@sss.pgh.pa.us - pg_dump / pg_restore: Add dumping of comments on index columns and composite type columns http://archives.postgresql.org/pgsql-hackers/2009-03/msg00931.php (XXX: Comments on composite type columns can work now?) 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] Buffer statistics for pg_stat_statements
We have infrastructure to count numbers buffer access in 8.5 Alpha 3.
I'd like to add per-query buffer usage into contrib/pg_stat_statements.
The pg_stat_statements view will have the same contents with
struct BufferUsage. Fields named shared_blks_{hit|read|written},
local_blks_{hit|read|written}, and temp_blks_{read|written}
will be added to the view.
We can determine slow queries not only based on durations but also
based on I/O or memory access count. Also, queries with non-zero
temp_blks_read means DBA need to consider increasing work_mem. Those
information would be useful to find where the server's bottleneck is.
Additional management costs cannot be avoided, but I think it should be
not so high because we accumulate buffer usage only once per query,
while EXPLAIN BUFFERS is slow because we need per-tuple calculation.
I'll submit this pg_stat_statements enhancement to the next commit fest.
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] Largeobject Access Controls (r2460)
Robert Haas robertmh...@gmail.com wrote:
Another comment is I'd like to keep link
linkend=catalog-pg-largeobject-metadata
for the first structnamepg_largeobject/structname in each topic.
Those two things aren't the same. Perhaps you meant link
linkend=catalog-pg-largeobject?
Oops, yes. Thank you for the correction.
We also have 8.4.x series in the core code. Do you think we also
rewrite those messages? One of them is an use-visible message.
LargeObjectAlterOwner() : pg_largeobject.c
* The 'lo_compat_privileges' is not checked here, because we
* don't have any access control features in the 8.4.x series
* or earlier release.
* So, it is not a place we can define a compatible behavior.
guc.c
{lo_compat_privileges, PGC_SUSET, COMPAT_OPTIONS_PREVIOUS,
gettext_noop(Enables backward compatibility in privilege checks on
large objects),
gettext_noop(When turned on, privilege checks on large objects perform
with backward compatibility as 8.4.x or earlier
releases.)
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] Largeobject Access Controls (r2460)
Robert Haas robertmh...@gmail.com wrote: In both cases, I'm lost. Help? They might be contrasted with the comments for myLargeObjectExists. Since we use MVCC visibility in loread(), metadata for large object also should be visible in MVCC rule. If I understand them, they say: * pg_largeobject_aclmask_snapshot requires a snapshot which will be used in loread(). * Don't use LargeObjectExists if you need MVCC visibility. In acldefault(), there is this comment: /* Grant SELECT,UPDATE by default, for now */ This doesn't seem to match what the code is doing, so I think we should remove it. Ah, ACL_NO_RIGHTS is the default. I also notice that dumpBlobComments() is now misnamed, but it seems we've chosen to add a comment mentioning that fact rather than fixing it. Hmmm, now it dumps not only comments but also ownership of large objects. Should we rename it dumpBlobMetadata() or so? 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] Largeobject Access Controls (r2460)
Robert Haas robertmh...@gmail.com wrote: 2009/12/16 KaiGai Kohei kai...@ak.jp.nec.com: ? ?long desc: When turned on, privilege checks on large objects perform with ? ? ? ? ? ? ? backward compatibility as 8.4.x or earlier releases. Mostly English quality, but there are some other issues too. Proposed patch attached. I remember we had discussions about the version number, like Don't use '8.5' because it might be released as '9.0', no? Another comment is I'd like to keep link linkend=catalog-pg-largeobject-metadata for the first structnamepg_largeobject/structname in each topic. 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] New VACUUM FULL
Alvaro Herrera alvhe...@commandprompt.com wrote: Hmm. With this patch, if I do vacuumdb -f it will not vacuum the special system catalogs that can only be vacuumed with INPLACE, correct? No. It will vacuum normal tables with FULL (rewrite), and system catalogs with FULL INPLACE. FULL vacuums for system catalogs always fall back to INPLACE vacuum silently. But certainly we cannot recommend to use daily database-wide VACUUM FULLs because they have higher costs than repeated FULL INPLACE vacuums. FULL (rewrite) will not be cheaper for tables that have little dead tuples. Just an idea, something like vacuumdb -f --threshold=some baseline might be useful for users running daily vacuumdb -f. 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] Range types
Scott Bailey arta...@comcast.net wrote: CREATE TYPE periodtz AS RANGE (timestamptz, '1 microsecond'::interval); What does the second argument mean? Is it a default interval? So basically I have a pg_range table to store the base typeid, a text field for the granule value and the granule typeid. As another approach, what about storing typeid in typmod? (Oid can be assumed to be stored in int32.) For example, CREATE TABLE tbl ( r range(timestamp) ); SELECT '[ 2.0, 3.0 )'::range(float); There might be some overhead to store typeid for each range instance, but the typmod approach does not require additinal catalogs and syntax changes. It can be possible even on 8.4. 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] Crash on pg_ctl initdb without options
pg_ctl initdb crashes if no options are passed. Do we need additional if-null test for pgdata_opt just like post_opts? $ pg_ctl initdb sh: -c: line 0: syntax error near unexpected token `null' sh: -c: line 0: `/usr/local/pgsql/bin/initdb (null)' pg_ctl: database system initialization failed Index: src/bin/pg_ctl/pg_ctl.c === --- src/bin/pg_ctl/pg_ctl.c (head) +++ src/bin/pg_ctl/pg_ctl.c (fix) @@ -656,6 +656,9 @@ if (exec_path == NULL) exec_path = find_other_exec_or_die(argv0, initdb, initdb (PostgreSQL) PG_VERSION \n); + if (pgdata_opt == NULL) + pgdata_opt = ; + if (post_opts == NULL) post_opts = ; 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] pgbench: new feature allowing to launch shell commands
Michael Paquier michael.paqu...@gmail.com wrote: Yeah the grammar :variable is used to set a parameter, the user will feel disorientated if there is no support. The attached patch supports this new case, based on Itagaki-san's last version. What is the spec of \setshell :variable ? Literally interpreted, it declares a variable with name :variable. But pgbench cannot use such variables because only variables named with alphabets, numbers, and _ can be accepted. Should we forbid to assign variables that name contain invalid characters instead? I also added a warning to tell the user that pgbench is slowing down when using this feature. This change seems to be nonsense. Do you want to fill your terminal with such messages? Proposed patch attached. It checks for variable names whether they consist of isalnum or _. The check is only performed when a new variable is assigned to avoid overhead. In normal workload, variables with the same names are re-used repeatedly. I don't think the additinal check would decrease performance of pgbench. Regards, --- Takahiro Itagaki NTT Open Source Software Center pgbenchshell_20091215.patch Description: Binary data pgbenchshell_test.sql 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] New VACUUM FULL still needed?
Simon Riggs si...@2ndquadrant.com wrote: I have enough items emerging from HS to keep me busy much longer than I thought. I'll run with VF if that's OK, since I have some other related changes in that area and it makes sense to understand that code also, if OK with you. Sure. Many users want to see HS. BTW, New VACUUM FULL patch is waiting for being applied. https://commitfest.postgresql.org/action/patch_view?id=202 But I heard HS is attempting to modify VFI in another way or remove it completely. Do we still need the patch, or reject it and fix VFI in HS? 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] pgbench: new feature allowing to launch shell commands
Greg Smith g...@2ndquadrant.com wrote: How about this: the version you updated at http://archives.postgresql.org/pgsql-hackers/2009-12/msg00847.php , your pgbenchshell_20091210.patch, worked perfectly for me except for one complaint I've since dropped. How about we move toward committing that one, and maybe we look at this whole variable name handling improvement as a separate patch later if you think it's worth pursuing? It's fine idea. I'll commit the previous lite version, and discuss whether we need the difference patch for fool proof. 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] Largeobject Access Controls and pg_migrator
KaiGai Kohei kai...@kaigai.gr.jp wrote: Can SELECT lo_create(16385); help this situation? SELECT lo_create(loid) FROM (SELECT DISTINCT loid FROM pg_largeobject) AS t would work for pg_migrator. I'm not clear whether we also check pg_largeobejct has chunks with same LOID on lo_create(). In the regular operation, it shall never happen. I think the omission is a reasonable optimization. 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] Largeobject Access Controls (r2460)
KaiGai Kohei kai...@kaigai.gr.jp wrote: We don't have any reason why still CASE ... WHEN and subquery for the given LOID. Right? Ah, I see. I used your suggestion. I applied the bug fixes. Our tools and contrib modules will always use pg_largeobject_metadata instead of pg_largeobject to enumerate large objects. I removed GRANT SELECT (loid) ON pg_largeobject TO PUBLIC from initdb because users must use pg_largeobject_metadata.oid when they want to check OIDs of large objects; If not, they could misjudge the existence of objects. This is an unavoidable incompatibility unless we always have corresponding tuples in pg_largeobject even for zero-length large 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
Re: [HACKERS] EXPLAIN BUFFERS
Robert Haas robertmh...@gmail.com wrote: OK, done, see attached. I also noticed when looking through this that the documentation says that auto_explain.log_buffers is ignored unless auto_explain.log_analyze is set. That is true and seems right to me, but for some reason explain_ExecutorEnd() had been changed to set es.analyze if either log_analyze or log_buffers was set. Thanks. It was my bug. Could you apply the patch? Or, may I do by myself? 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] EXPLAIN BUFFERS
Robert Haas robertmh...@gmail.com wrote: I have a question about the comment in InstrStopNode(), which reads: Adds delta of buffer usage to node's count and resets counter to start so that the counters are not double counted by parent nodes. It then calls BufferUsageAccumDiff(), but that function doesn't actually reset anything, so it seems like the comment is wrong. Oops, it's wrong. It just does Adds delta of buffer usage to node's count. Two other thoughts: 1. It doesn't appear that there is any provision to ever zero pgBufferUsage. Shouldn't we do this, say, once per explain, just to avoid the possibility of overflowing the counters? I think the overflowing will not be a problem because we only use the differences of values. The delta is always corrent unless we use 2^32 buffer accesses during one execution of a node. 2. We seem to do all the work associated with pgBufferUsage even when the buffers option is not passed to explain. The overhead of incrementing the counters is probably negligible (and we were paying the equivalent overhead before anyway) but I'm not sure whether saving the starting counters and accumulating the deltas might be enough to slow down EXPLAIN ANALYZE. That's sorta slow already so I'd hate to whack it any more - have you benchmarked this at all? There are 5% of overheads in the worst cases. The difference will be little if we have more complex operations or some disk I/Os. Adding Instrumentation-count_bufusage flag could reduce the overheads. if (instr-count_bufusage) BufferUsageAccumDiff(...) Should I add countBufferUsage boolean arguments to all places doInstrument booleans are currently used? This requires several minor modifications of codes in many places. [without patch] =# EXPLAIN (ANALYZE) SELECT * FROM pgbench_accounts; QUERY PLAN Seq Scan on pgbench_accounts (cost=0.00..263935.00 rows=1000 width=97) (actual time=0.003..571.794 rows=1000 loops=1) Total runtime: 899.427 ms [with patch] =# EXPLAIN (ANALYZE) SELECT * FROM pgbench_accounts; QUERY PLAN Seq Scan on pgbench_accounts (cost=0.00..263935.00 rows=1000 width=97) (actual time=0.003..585.885 rows=1000 loops=1) Total runtime: 955.280 ms - shared_buffers = 1500MB - pgbench -i -s100 - Read all pages in the pgbench_accounts into shared buffers before runs. 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] EXPLAIN BUFFERS
Tom Lane t...@sss.pgh.pa.us wrote: Takahiro Itagaki itagaki.takah...@oss.ntt.co.jp writes: Should I add countBufferUsage boolean arguments to all places doInstrument booleans are currently used? This requires several minor modifications of codes in many places. Pushing extra arguments around would create overhead of its own ... overhead that would be paid even when not using EXPLAIN at all. I cannot understand what you mean... The additional argument should not be a performance overhead because the code path is run only once per execution. Instrumentation structures are still not allocated in normal or EXPLAIN queries; allocated only in EXPLAIN ANALYZE. Or, are you suggesting to separate buffer counters with Instrumentation structure? It still requires extra arguments, but it could minimize the overhead when we use EXPLAIN ANALYZE without BUFFERS. However, we need additional codes around InstrStartNode/InstrStopNode calls. Or, are you complaining about non-performance overheads, something like overheads of code maintenance? 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] EXPLAIN BUFFERS
Robert Haas robertmh...@gmail.com wrote: Well, I think we need to do something. I don't really want to tack another 5-6% overhead onto EXPLAIN ANALYZE. Maybe we could recast the doInstrument argument as a set of OR'd flags? I'm thinking the same thing (OR'd flags) right now. The attached patch adds INSTRUMENT_TIMER and INSTRUMENT_BUFFERS flags. The types of QueryDesc.doInstrument (renamed to instrument_options) and EState.es_instrument are changed from bool to int, and they store OR of InstrumentOption flags. INSTRUMENT_TIMER is always enabled when instrumetations are initialized, but INSTRUMENT_BUFFERS is enabled only if we use EXPLAIN BUFFERS. I think the flag options are not so bad idea because of extensibility. For example, we could support EXPLAIN CPU_USAGE someday. One issue is in the top-level instrumentation (queryDesc-totaltime). Since the field might be used by multiple plugins, the first initializer need to initialize the counter with all options. I used INSTRUMENT_ALL for it in the patch. =# EXPLAIN (ANALYZE) SELECT * FROM pgbench_accounts; QUERY PLAN Seq Scan on pgbench_accounts (cost=0.00..263935.00 rows=1000 width=97) (actual time=0.003..572.126 rows=1000 loops=1) Total runtime: 897.729 ms =# EXPLAIN (ANALYZE, BUFFERS) SELECT * FROM pgbench_accounts; QUERY PLAN Seq Scan on pgbench_accounts (cost=0.00..263935.00 rows=1000 width=97) (actual time=0.002..580.642 rows=1000 loops=1) Buffers: shared hit=163935 Total runtime: 955.744 ms Regards, --- Takahiro Itagaki NTT Open Source Software Center explain_buffers_20091214.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] Largeobject Access Controls (r2460)
KaiGai Kohei kai...@ak.jp.nec.com wrote:
We have to reference pg_largeobject_metadata to check whether a certain
large objct exists, or not.
It is a case when we create a new large object, but write nothing.
OK, that makes sense.
In addition of the patch, we also need to fix pg_restore with
--clean option. I added DropBlobIfExists() in pg_backup_db.c.
A revised patch attached. Please check further mistakes.
BTW, we can optimize lo_truncate because we allow metadata-only large
objects. inv_truncate() doesn't have to update the first data tuple to
be zero length. It only has to delete all corresponding tuples like as:
DELETE FROM pg_largeobject WHERE loid = {obj_desc-id}
Regards,
---
Takahiro Itagaki
NTT Open Source Software Center
pgsql-blob-priv-fix_v2.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] Largeobject Access Controls (r2460)
Takahiro Itagaki itagaki.takah...@oss.ntt.co.jp wrote: In addition of the patch, we also need to fix pg_restore with --clean option. I added DropBlobIfExists() in pg_backup_db.c. A revised patch attached. Please check further mistakes. ...and here is an additional fix for contrib modules. Regards, --- Takahiro Itagaki NTT Open Source Software Center fix-lo-contrib.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] pgbench: new feature allowing to launch shell commands
Michael Paquier michael.paqu...@gmail.com wrote: Please find attached the latest version of the patch, with the threading bug corrected and the documentation updated as well. Please don't describe your-specific usage of shell command in the documentation Why do you use BUFFER_SIZE-1 for snprintf? snprintf(commandShell, SHELL_COMMAND_SIZE-1, ...) Trailing nulls are also included in the length, so snprintf(commandShell, SHELL_COMMAND_SIZE, ...) would be ok. (removed -1) I found several bugs and matters. * The calcuration of the shell command length is wrong. * You cannot assign 0 to variables with \setshell meta command. * Comparison with == true is a bad manner. * You called \shell shell command and \setsell shell script, but we don't need the difference. I think shell command is enough. Heavily cleaned up patch attached. Please review it. Regards, --- Takahiro Itagaki NTT Open Source Software Center pgbenchshell_20091210.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] Largeobject Access Controls (r2460)
KaiGai Kohei kai...@ak.jp.nec.com wrote:
we still allow SELECT * FROM pg_largeobject ...right?
It can be solved with revoking any privileges from anybody in the initdb
phase. So, we should inject the following statement for setup_privileges().
REVOKE ALL ON pg_largeobject FROM PUBLIC;
OK, I'll add the following description in the documentation of pg_largeobject.
structnamepg_largeobject/structname should not be readable by the
public, since the catalog contains data in large objects of all users.
structnamepg_largeobject_metadata/ is a publicly readable catalog
that only contains identifiers of large objects.
{lo_compat_privileges, PGC_SUSET, COMPAT_OPTIONS_PREVIOUS,
gettext_noop(Turn on/off privilege checks on large objects.),
The description is true, but gives a confusion because
lo_compat_privileges = on means privilege checks are turned off.
short desc: Enables backward compatibility in privilege checks on large
objects
long desc: When turned on, privilege checks on large objects are disabled.
Are those descriptions appropriate?
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
