Re: [HACKERS] Support UTF-8 files with BOM in COPY FROM

2011-09-25 Thread David E. Wheeler
On Sep 25, 2011, at 9:58 PM, Itagaki Takahiro wrote:

> I'd like to support UTF-8 text or csv files that has BOM (byte order mark)
> in COPY FROM command. BOM will be automatically detected and ignored
> if the file encoding is UTF-8. WIP patch attached.

By my reading of http://unicode.org/faq/utf_bom.html#bom5, I'd say +1

So I think what you propose makes sense.

> I'm thinking about only COPY FROM for reads, but if someone wants to add
> BOM in COPY TO, we might also support COPY TO WITH BOM for writes.

I think it would have to be optional, since "some recipients of UTF-8 encoded 
data do not expect a BOM."

Best,

David


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


[HACKERS] Support UTF-8 files with BOM in COPY FROM

2011-09-25 Thread Itagaki Takahiro
Hi,

I'd like to support UTF-8 text or csv files that has BOM (byte order mark)
in COPY FROM command. BOM will be automatically detected and ignored
if the file encoding is UTF-8. WIP patch attached.

I'm thinking about only COPY FROM for reads, but if someone wants to add
BOM in COPY TO, we might also support COPY TO WITH BOM for writes.

Comments welcome.

-- 
Itagaki Takahiro


copy_from_bom.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] contrib/sepgsql regression tests are a no-go

2011-09-25 Thread Robert Haas
On Mon, Sep 26, 2011 at 12:06 AM, Tom Lane  wrote:
>> Then we
>> could get rid of the associated configure option, which no longer
>> serves any other purpose, and just say that if you want to build (or
>> regression-test) sepgsql, well, you gotta go to that directory and do
>> it by hand.
>
> The trouble with getting rid of configure support is that we lose the
> opportunity to verify that you have a new-enough version of libselinux.
> That's maybe not a big deal, but it's a point worth noting.

I suppose the main downside there is that you will get a more cryptic
error message if it isn't new enough.

On the other hand, even if you don't use --with-selinux, you can still
do "make -C contrib/sepgsql" and you'll have the same problem, so I'm
not sure that we're really heavily shielded against a failure of this
type as it is.

Perhaps we could remove --with-selinux, but *always* test whether we
have a new enough version of selinux.  If we don't, we set it up so
that configure succeeds anyway, but any attempt to build sepgsql
fails?  (Not sure if there's a clean way to do that.)

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

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


Re: [HACKERS] contrib/sepgsql regression tests are a no-go

2011-09-25 Thread Tom Lane
Robert Haas  writes:
> On Sun, Sep 25, 2011 at 9:07 PM, Tom Lane  wrote:
>> As a stopgap, what about removing sepgsql from the list of contrib
>> modules tested by "make -C contrib check"?

> +1.

> In fact, I've been wondering if we ought to go a step further and not
> recurse into the sepgsql directory for *any* of the targets.

Hmm.  That would be a lot cleaner than the alternatives I've thought
of so far for recursing for only some targets.

> Then we
> could get rid of the associated configure option, which no longer
> serves any other purpose, and just say that if you want to build (or
> regression-test) sepgsql, well, you gotta go to that directory and do
> it by hand.

The trouble with getting rid of configure support is that we lose the
opportunity to verify that you have a new-enough version of libselinux.
That's maybe not a big deal, but it's a point worth noting.

regards, tom lane

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


Re: [HACKERS] contrib/sepgsql regression tests are a no-go

2011-09-25 Thread Robert Haas
On Sun, Sep 25, 2011 at 9:07 PM, Tom Lane  wrote:
> As a stopgap, what about removing sepgsql from the list of contrib
> modules tested by "make -C contrib check"?

+1.

In fact, I've been wondering if we ought to go a step further and not
recurse into the sepgsql directory for *any* of the targets.  Then we
could get rid of the associated configure option, which no longer
serves any other purpose, and just say that if you want to build (or
regression-test) sepgsql, well, you gotta go to that directory and do
it by hand.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

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


Re: [HACKERS] Inlining comparators as a performance optimisation

2011-09-25 Thread Robert Haas
On Sun, Sep 25, 2011 at 10:12 PM, Peter Geoghegan  wrote:
> [ new results ]

Nice results.  I think these are far more convincing than the last
set, because (1) the gains are bigger and (2) they survive -O2 and (3)
you tested an actual query, not just qsort() itself.

I don't want to take time to review this in detail right now, because
I don't think it would be fair to put this ahead of things that were
submitted for the current CommitFest, but I'm impressed.

> On the subject of highly ambitious optimisations to sorting, one
> possibility I consider much more practicable than GPU-accelerated
> sorting is simple threading; quicksort can be parallelised very
> effectively, due to its divide-and-conquer nature. If we could agree
> on a threading abstraction more sophisticated than forking, it's
> something I'd be interested in looking at. To do so would obviously
> entail lots of discussion about how that relates to whatever way we
> eventually decide on implementing parallel query, and that's obviously
> a difficult discussion.

I have the same feeling about about this that I do about almost every
executor optimization that anyone proposes: the whole project would be
entirely simple and relatively painless if it weren't for the need to
make planner changes.  I mean, deciding on a threading interface is
likely to be a somewhat contentious discussion, with differences of
opinion on whether we should do it and what the API should look like.
But at the end of the day it's not rocket science, and I expect that
we would end up with something reasonable.  What seems much harder is
figuring out how to decide when to perform quicksort in parallel vs.
single-threaded, and how much parallelism would be appropriate.  I
haven't seen anyone propose even a shadow of an idea about how to make
such decisions intelligently, either in general or in specific cases.

The other issue is that, while threading would be possibly suitable
for this particular case, at least for built-in datatypes with
comparison operations that basically reduce to single machine-language
comparison instructions, it's hard to see how we could take it much
further.  It would be unsafe for these multiple threads of execution
to do anything that could possibly throw an error or anything that
touches a lightweight lock or, really, just about anything at all.
Trying to make the entire backend thread-safe - or even any
significant portion of it - seems like a colossal effort that will
most likely fail, but maybe not without eating an enormous amount of
developer time first.  And without doing that, I don't think we could
even extend this as far as, say, numeric, whose functions do things
like palloc() and ereport() internally.  So I feel like this whole
approach might be a dead-end - there's a narrow range of cases where
it could be made to work, I think, but after that I think you hit a
titanium wall.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

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


Re: [HACKERS] [v9.2] Fix Leaky View Problem

2011-09-25 Thread Robert Haas
On Sun, Sep 25, 2011 at 10:38 PM, Noah Misch  wrote:
> On Sun, Sep 25, 2011 at 11:22:03AM -0500, Kevin Grittner wrote:
>> Robert Haas  09/25/11 10:58 AM >>>
>>
>> > I'm not sure we've been 100% consistent about that, since we
>> > previously made CREATE OR REPLACE LANGUAGE not replace the owner
>> > with the current user.
>>
>> I think we've been consistent in *not* changing security on an
>> object when it is replaced.
>
>> [CREATE OR REPLACE FUNCTION does not change proowner or proacl]
>
> Good point.  C-O-R VIEW also preserves column default values.  I believe we 
> are
> consistent to the extent that everything possible to specify in each C-O-R
> statement gets replaced outright.  The preserved characteristics *require*
> commands like GRANT, COMMENT and ALTER VIEW to set in the first place.
>
> The analogue I had in mind is SECURITY DEFINER, which C-O-R FUNCTION reverts 
> to
> SECURITY INVOKER if it's not specified each time.  That default is safe, 
> though,
> while the proposed default of security_barrier=false is unsafe.

Even though I normally take the opposite position, I still like the
idea of dedicated syntax for this feature.  Not knowing what view
options we might end up with in the future, I hate having to decide on
what the general behavior ought to be.  But it would be easy to decide
that CREATE SECURITY VIEW followed by CREATE OR REPLACE VIEW leaves
the security flag set; it would be consistent with what we're doing
with owner and acl information and wouldn't back us into any
unpleasant decisions down the road.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

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


Re: [HACKERS] [v9.2] Fix Leaky View Problem

2011-09-25 Thread Noah Misch
On Sun, Sep 25, 2011 at 11:22:03AM -0500, Kevin Grittner wrote:
> Robert Haas  09/25/11 10:58 AM >>>
> 
> > I'm not sure we've been 100% consistent about that, since we
> > previously made CREATE OR REPLACE LANGUAGE not replace the owner
> > with the current user.
>  
> I think we've been consistent in *not* changing security on an
> object when it is replaced.

> [CREATE OR REPLACE FUNCTION does not change proowner or proacl]

Good point.  C-O-R VIEW also preserves column default values.  I believe we are
consistent to the extent that everything possible to specify in each C-O-R
statement gets replaced outright.  The preserved characteristics *require*
commands like GRANT, COMMENT and ALTER VIEW to set in the first place.

The analogue I had in mind is SECURITY DEFINER, which C-O-R FUNCTION reverts to
SECURITY INVOKER if it's not specified each time.  That default is safe, though,
while the proposed default of security_barrier=false is unsafe.

Thanks,
nm

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


Re: [HACKERS] Online base backup from the hot-standby

2011-09-25 Thread Steve Singer

On 11-09-22 09:24 AM, Fujii Masao wrote:

On Wed, Sep 21, 2011 at 11:50 AM, Fujii Masao  wrote:

2011/9/13 Jun Ishiduka:

Update patch.

Changes:
  * set 'on' full_page_writes by user (in document)
  * read "FROM: XX" in backup_label (in xlog.c)
  * check status when pg_stop_backup is executed (in xlog.c)

Thanks for updating the patch.

Before reviewing the patch, to encourage people to comment and
review the patch, I explain what this patch provides:

Attached is the updated version of the patch. I refactored the code, fixed
some bugs, added lots of source code comments, improved the document,
but didn't change the basic design. Please check this patch, and let's use
this patch as the base if you agree with that.



I have looked at both Jun's patch from Sept 13 and Fujii's updates to 
the patch.  I agree that Fujii's updated version should be used as the 
basis for changes going forward.   My comments below refer to that 
version (unless otherwise noted).



In backup.sgml  the new section titled "Making a Base Backup during 
Recovery"  I would prefer to see some mention in the title that this 
procedure is for standby servers ie "Making a Base Backup from a Standby 
Database".  Users who have setup a hot-standby database should be 
familiar with the 'standby' terminology. I agree that the "during 
recovery" description is technically correct but I'm not sure someone 
who is looking through the manual for instructions on making a base 
backup from here standby will realize this is the section they should read.


Around line 969 where you give an example of copying the control file I 
would be a bit clearer that this is an example command.  Ie (Copy the 
pg_control file from the cluster directory to the global sub-directory 
of the backup.  For example "cp $PGDATA/global/pg_control 
/mnt/server/backupdir/global")



Testing Notes
-

I created a standby server from a base backup of another standby server. 
On this new standby server I then


1. Ran pg_start_backup('3'); and left the psql connection open
2. touch /tmp/3 -- my trigger_file

ssinger@ssinger-laptop:/usr/local/pgsql92git/bin$ LOG:  trigger file 
found: /tmp/3

FATAL:  terminating walreceiver process due to administrator command
LOG:  restored log file "00010006" from archive
LOG:  record with zero length at 0/60002F0
LOG:  restored log file "00010006" from archive
LOG:  redo done at 0/6000298
LOG:  restored log file "00010006" from archive
PANIC:  record with zero length at 0/6000298
LOG:  startup process (PID 19011) was terminated by signal 6: Aborted
LOG:  terminating any other active server processes
WARNING:  terminating connection because of crash of another server process
DETAIL:  The postmaster has commanded this server process to roll back 
the current transaction and exit, because another server process exited 
abnormally and possibly corrupted shared memory.
HINT:  In a moment you should be able to reconnect to the database and 
repeat your command.


The new postmaster (the one trying to be promoted) dies.  This is 
somewhat repeatable.




If a base backup is in progress on a recovery database and that recovery 
database is promoted to master, following the promotion (if you don't 
restart the postmaster).  I see

select pg_stop_backup();
ERROR:  database system status mismatches between pg_start_backup() and 
pg_stop_backup()


If you restart the postmaster this goes away.  When the postmaster 
leaves recovery mode I think it should abort an existing base backup so 
pg_stop_backup() will say no backup in progress, or give an error 
message on pg_stop_backup() saying that the base backup won't be 
usable.  The above error doesn't really tell the user why there is a 
mismatch.


-

In my testing a few times I got into a situation where a standby server 
coming from a recovery target took a while to finish recovery (this is 
on a database with no activity).  Then when i tried promoting that 
server to master I got


LOG:  trigger file found: /tmp/3
FATAL:  terminating walreceiver process due to administrator command
LOG:  restored log file "00010009" from archive
LOG:  restored log file "00010009" from archive
LOG:  redo done at 0/9E8
LOG:  restored log file "00010009" from archive
PANIC:  unexpected pageaddr 0/600 in log file 0, segment 9, offset 0
LOG:  startup process (PID 1804) was terminated by signal 6: Aborted
LOG:  terminating any other active server processes


It is *possible* I mixed up the order of a step somewhere since my 
testing isn't script based. A standby server that 'looks' okay but can't 
actually be promoted is dangerous.


This version of the patch (I was testing the Sept 22nd version) seems 
less stable than how I remember the version from the July CF.  Maybe I'm 
just testing it harder or maybe something has been broken.





In the current patch, there is no safegua

Re: [HACKERS] Re: Optimizing pg_trgm makesign() (was Re: WIP: Fast GiST index build)

2011-09-25 Thread Kevin Grittner
> Tom Lane  wrote:
> "Kevin Grittner"  writes:
>> This is a review of the patch at this CF location:
>> https://commitfest.postgresql.org/action/patch_view?id=598
>> as posted here:
>>
http://archives.postgresql.org/message-id/4e04c099.3020...@enterprisedb.com
>  
> Hmm, why is that patch the one posted for review, when several
> better versions were already discussed? See thread starting here:
> http://archives.postgresql.org/pgsql-hackers/2011-07/msg00028.php
 
The patch I reviewed was added to the CF app before the post you cite
was written.  I didn't see it in following the links (probably
because it crossed a month boundary).  Thanks for pointing that out;
I'll update the CF app and review the later versions.
 
-Kevin

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


Re: [HACKERS] Re: Optimizing pg_trgm makesign() (was Re: WIP: Fast GiST index build)

2011-09-25 Thread Tom Lane
"Kevin Grittner"  writes:
> This is a review of the patch at this CF location:
> https://commitfest.postgresql.org/action/patch_view?id=598
> as posted here:
> http://archives.postgresql.org/message-id/4e04c099.3020...@enterprisedb.com

Hmm, why is that patch the one posted for review, when several better
versions were already discussed?  See thread starting here:
http://archives.postgresql.org/pgsql-hackers/2011-07/msg00028.php

regards, tom lane

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


[HACKERS] contrib/sepgsql regression tests are a no-go

2011-09-25 Thread Tom Lane
So I thought it would be a good idea to enable contrib/sepgsql in the
Fedora build of 9.1.  This soon crashed and burned, though, because

(1) if you build sepgsql, there is no way to omit the sepgsql regression
tests, other than by not regression-testing contrib at all.  I didn't
see that as a step forward.

(2) the sepgsql regression tests cannot be run without having done
assorted root-privilege-required tweaks to the SELinux configuration.
RPMs are not customarily built with root privilege, and even if they
were, having one fool with the SELinux configuration like that would
set off alarm bells for a lot of people.

In particular, point (2) means that "make check" is a joke.  You can't
do it without serious side-effects on the state of the host system,
which is contrary to the entire purpose of that make target.

I am of the opinion that these regression tests need to be reworked
so that they do not require a nonstandard SELinux environment.
I realize however that that's likely to take nontrivial work.
As a stopgap, what about removing sepgsql from the list of contrib
modules tested by "make -C contrib check"?  (I haven't looked at
exactly how ugly it might be to do that, nor whether we'd have to also
disable installcheck from recursing to sepgsql.)

regards, tom lane

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


Re: [HACKERS] [v9.2] Fix Leaky View Problem

2011-09-25 Thread Robert Haas
On Sun, Sep 25, 2011 at 3:25 PM, Kohei KaiGai  wrote:
>> I'm a bit nervous about storing security_barrier in the RTE.  What
>> happens to stored rules if the security_barrier option gets change
>> later?
>>
> The rte->security_barrier is evaluated when a query referencing security
> views get expanded. So, rte->security_barrier is not stored to catalog.

I think it is.  If you create a view that involves an RTE, the node
tree is going to get stored in pg_rewrite.ev_action.  And it's going
to include the security_barrier attribute, because you added outfuncs
support for it...

No?

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

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


Re: [HACKERS] index-only scans

2011-09-25 Thread Robert Haas
On Sun, Sep 25, 2011 at 2:43 PM, Marti Raudsepp  wrote:
> On Sun, Aug 14, 2011 at 00:31, Heikki Linnakangas
>  wrote:
>> That is somewhat compensated by the fact that tuples that are accessed more
>> often are also more likely to be in cache. Fetching the heap tuple to check
>> visibility is very cheap when the tuple is in cache.
>>
>> I'm not sure how far that compensates it, though. I'm sure there's typically
>> nevertheless a fairly wide range of pages that have been modified since the
>> last vacuum, but not in cache anymore.
>
> Would it make sense to re-evaluate the visibility bit just before a
> page gets flushed out from shared buffers? On a system with no long
> transactions, it seems likely that a dirty page is already all-visible
> by the time bgwriter (or shared buffers memory pressure) gets around
> to writing it out. That way we don't have to wait for vacuum to do it
> and would make your observation hold more often.

This has been suggested before, and, sure, there might be cases where
it helps.  But you need to choose your test case fairly carefully.
For example, if you're doing a large sequential scan on a table, the
ring-buffer logic causes processes to evict their own pages, and so
the background writer doesn't get a chance to touch any of those
pages.  You need some kind of a workload where pages are being evicted
from shared buffers slowly enough that it ends up being the background
writer, rather than the individual backends, that do the work.  But if
you have that kind of workload, then we can infer that most of your
working set fits into shared buffers.  And in that case you don't
really need index-only scans in the first place.  The main point of
index only scans is to optimize the case where you have a gigantic
table and you're trying to avoid swamping the system with random I/O.
I'm not saying that such a change would be a particularly bad idea,
but I'm not personally planning to work on it any time soon because I
can't convince myself that it really helps all that much.

I think the real solution to getting visibility map bits set is to
vacuum more frequently, but have it be cheaper each time.  Our default
autovacuum settings vacuum the table when the number of updates and
deletes reaches 20% of the table size.  But those settings were put in
place under the assumption that we'll have to scan the entire heap,
dirtying every page that contains dead tuples, scan all the indexes
and remove the associated index pointers, and then scan and dirty the
heap pages that contain now-dead line pointers a second time to remove
those.  The visibility map has eroded those assumptions to some
degree, because now we probably won't have to scan the entire heap
every time we vacuum; and I'm hoping we're going to see some further
erosion.  Pavan has a pending patch which, if we can work out the
details, will eliminate the second heap scan; and we've also talked
about doing the index scan only when there are enough dead line
pointers to justify the effort.  That, it seems to me, would open the
door to lowering the scale factor, maybe by quite a bit - which, in
turn, would help us control bloat better and get visibility map bits
set sooner.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

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


[HACKERS] Re: Optimizing pg_trgm makesign() (was Re: WIP: Fast GiST index build)

2011-09-25 Thread Kevin Grittner
This is a review of the patch at this CF location:
 
https://commitfest.postgresql.org/action/patch_view?id=598
 
as posted here:
 
http://archives.postgresql.org/message-id/4e04c099.3020...@enterprisedb.com
 
This patch applied cleanly and compiled without warning.  It
performed correctly.  Since the patch modifies one function which has
one input and one output (through a pointer parameter), with no other
side effects, it has low risk of surprising problems.
 
It is strictly a performance patch, and accomplishes its goals.  I
ran the entire dictionary through the patched and unpatched versions
five times each (using the supplied debugging patch) and saw no
changes to the behavior of the function -- identical results every
time.  I ran performance tests with building and reindexing the
sorted dictionary, a randomly ordered dictionary, a randomly ordered
dictionary with the entries randomly truncated to 0 to 3 characters,
and a large README file -- five times each with and without the
patch.  The patch was a clear winner with all of those except the
truncated dictionary, where the difference we well within the noise
(0.05% difference summing five runs when individual runs could vary
by up to about 4%).
 
While there was no reason to believe it could affect search
performance, I tested that anyway, and found no difference.
 
Once this message shows up on the list (so I can retrieve the message
ID) I will mark this "Ready for Committer".
 
-Kevin


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


Re: [HACKERS] fix for pg_upgrade

2011-09-25 Thread Bruce Momjian
panam wrote:
> OK, i started once again:
> 
> 
> I hope the following is the correct way of querying the table corresponding
> to a relid:
> 
> 
> 
> 
> 
> 
> 
> 
> 
> --
> View this message in context: 
> http://postgresql.1045698.n5.nabble.com/fix-for-pg-upgrade-tp3411128p4838427.html

Yes, that is very close to what I needed.  Ideally you would have
included the oid from pg_class:

select oid, * from pg_class where oid = 465783 or oid = 16505

Can you supply that?

Also can you email me privately the following output from the old
database?  It should only be the schema and not your data:

pg_dumpall --schema-only --binary-upgrade

I am looking for something like this in the file:

-- For binary upgrade, must preserve pg_class oids
SELECT 
binary_upgrade.set_next_heap_pg_class_oid('16385'::pg_catalog.oid);

CREATE TABLE test (
x integer
);

but for your case it would be the 'accounts' file.  You can email just
those lines if you want, and that you can probably email to hackers. 
Thanks.

-- 
  Bruce Momjian  http://momjian.us
  EnterpriseDB http://enterprisedb.com

  + It's impossible for everything to be true. +

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


Re: [HACKERS] [v9.2] DROP statement reworks

2011-09-25 Thread Dimitri Fontaine
Hi,

Kohei KaiGai  writes:
> 2011/8/15 Kohei KaiGai :
>> The attached three patches try to consolidate code path of DROP
>> statement on various kind of object classes.
>
> These are rebased to the latest tree, and the part-3 portion also consolidates
> DROP OPERATOR FAMILY/CLASS routines that I forgot to rework in the
> previous patch.

I've been reviewing those patches, that are all about code refactoring.
I like what it's doing, generalizing ad-hoc code by adding some more
knowledge about the source tree into some data structures.  Typically,
what catcache to use for a given object's class-id.

The patches are not in git am format nor in patch format, so I could
only read them, I didn't install them nor compiled the code, didn't run
the regression tests.

Regards,
-- 
Dimitri Fontaine
http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support

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


Re: [HACKERS] unite recovery.conf and postgresql.conf

2011-09-25 Thread Tom Lane
Joshua Berkus  writes:
> include_if_exists certainly solves the recovery.conf/recovery.done problem.  
> We can even phase it out, like this:

> 9.2: include_if_exists = 'recovery.conf' in the default postgresql.conf file.
> 9.3: include_if_exists = 'recovery.conf' commented out by default
> 9.4: renaming recovery.conf to recovery.done by core PG code removed.

> This gives users/vendors 3 years to update their scripts to remove dependence 
> on recovery.conf.  I'm afraid that I agree with Simon that there's already a 
> whole buncha 3rd-party code out there to support the current system.

If there is indeed code out there that depends on the current system,
why do you think that waiting several releases to change it will make
things better?  I think it's more likely that we'd just have *more* code
that needs to be changed, and no reduction in the pain level when the
transition does finally happen.

In any case, I thought we'd agreed that the use of that file as a flag
should go away now, so I quite fail to understand your comment about 9.4.

regards, tom lane

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


Re: [HACKERS] unite recovery.conf and postgresql.conf

2011-09-25 Thread Joshua Berkus


> I rather like Tom's suggestion of include_if_exists.

include_if_exists certainly solves the recovery.conf/recovery.done problem.  We 
can even phase it out, like this:

9.2: include_if_exists = 'recovery.conf' in the default postgresql.conf file.
9.3: include_if_exists = 'recovery.conf' commented out by default
9.4: renaming recovery.conf to recovery.done by core PG code removed.

This gives users/vendors 3 years to update their scripts to remove dependence 
on recovery.conf.  I'm afraid that I agree with Simon that there's already a 
whole buncha 3rd-party code out there to support the current system.

--Josh Berkus 

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


Re: [HACKERS] [v9.2] "database" object class of contrib/sepgsql

2011-09-25 Thread Kohei KaiGai
2011/9/23 Robert Haas :
> On Mon, Sep 12, 2011 at 5:45 AM, Kohei KaiGai  wrote:
>> The attached patch is a portion that we splitted off when we added
>> pg_shseclabel system catalog.
>>
>> It enables the control/sepgsql to assign security label on pg_database
>> objects that are utilized as a basis to compute a default security
>> label of schema object.
>
> Committed, although the fact that it didn't compile until I made
> schema.c include pg_database.h makes me wonder how thoroughly you
> tested this.
>
Hmm.. As I did usually, I might build the module and run installation
script and regression test when I submitted this patch.
However, it was fact I submitted a patch with an obvious miss.
Sorry, I'll be careful to check the code being tested.
-- 
KaiGai Kohei 

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


Re: [HACKERS] [v9.2] Fix Leaky View Problem

2011-09-25 Thread Kohei KaiGai
2011/9/24 Robert Haas :
> On Mon, Sep 12, 2011 at 3:31 PM, Kohei KaiGai  wrote:
>> I updated the patches of fix-leaky-view problem, according to the
>> previous discussion.
>> The "NOLEAKY" option was replaced by "LEAKPROOF" option, and several 
>> regression
>> test cases were added. Rest of stuffs are unchanged.
>
> You have a leftover reference to NOLEAKY.
>
Oops, I'll fix it.

>> For convenience of reviewer, below is summary of these patches:
>>
>> The Part-1 implements corresponding SQL syntax stuffs which are
>> "security_barrier"
>> reloption of views, and "LEAKPROOF" option on creation of functions to be 
>> stored
>> new pg_proc.proleakproof field.
>
> The way you have this implemented, we just blow away all view options
> whenever we do CREATE OR REPLACE VIEW.  Is that the behavior we want?
> If a security_barrier view gets accidentally turned into a
> non-security_barrier view, doesn't that create a security_hole?
>
> I'm also wondering if the way you're using ResetViewOptions() is the
> right way to handle this anyhow.  Isn't that going to update pg_class
> twice?  I guess that's probably harmless from a performance
> standpoint, but wouldn't it be better not to?  I guess we could define
> something like AT_ReplaceRelOptions to handle this case.
>
IIRC, we had a discussion that the behavior should follow the case
when a view is newly defined, even if it would be replaced actually.
However, it seems to me consistent way to keep existing setting
as long as user does not provide new option explicitly.
If so, I think AT_ReplaceRelOptions enables to simplify the code
to implement such a behavior.

> The documentation in general is not nearly adequate, at least IMHO.
>
Do you think the description is poor to introduce the behavior changes
corresponding to security_barrier option?
OK, I'll try to update the documentation.

> I'm a bit nervous about storing security_barrier in the RTE.  What
> happens to stored rules if the security_barrier option gets change
> later?
>
The rte->security_barrier is evaluated when a query referencing security
views get expanded. So, rte->security_barrier is not stored to catalog.
Even if security_barrier option was changed after PREPARE statement
with references to security view, our existing mechanism re-evaluate
the query on EXECUTE, thus, it shall be executed as we expected.
(As an aside, I didn't know this mechanism and surprised at EXECUTE
works correctly, even if VIEW definition was changed after PREPARE.)

Thanks,
-- 
KaiGai Kohei 

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


[HACKERS] Re: [PATCH] Caching for stable expressions with constant arguments v3

2011-09-25 Thread Joshua Berkus
All,

I'd love to see someone evaluate the impact of Marti's patch on JDBC 
applications which use named prepared statements.   Anyone have a benchmark 
handy?

--Josh

-- 
Sent 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 - symmetric

2011-09-25 Thread Joshua Berkus

> > Reminder:  BETWEEEN supports the SYMMETRIC keyword, so there is
> > a precedent for this.
> 
> And I don't see it as valuable enough to justify changing the
> grammar.

I agree that we should leave symmetry until 9.3.

--Josh Berkus

-- 
Sent 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 - symmetric

2011-09-25 Thread Jeff Davis
On Sat, 2011-09-24 at 10:49 -0400, Bruce Momjian wrote:
> > I'll add that it would also cause a little confusion with inclusivity.
> > What if you do: '[5,2)'::int4range? Is that really '[2,5)' or '(2,5]'?
> 
> Reminder:  BETWEEEN supports the SYMMETRIC keyword, so there is
> a precedent for this.

And I don't see it as valuable enough to justify changing the grammar.

Also, that still leaves confusion about inclusivity when applied to
range types. 

Regards,
Jeff Davis


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


Re: [HACKERS] index-only scans

2011-09-25 Thread Marti Raudsepp
On Sun, Aug 14, 2011 at 00:31, Heikki Linnakangas
 wrote:
> That is somewhat compensated by the fact that tuples that are accessed more
> often are also more likely to be in cache. Fetching the heap tuple to check
> visibility is very cheap when the tuple is in cache.
>
> I'm not sure how far that compensates it, though. I'm sure there's typically
> nevertheless a fairly wide range of pages that have been modified since the
> last vacuum, but not in cache anymore.

Would it make sense to re-evaluate the visibility bit just before a
page gets flushed out from shared buffers? On a system with no long
transactions, it seems likely that a dirty page is already all-visible
by the time bgwriter (or shared buffers memory pressure) gets around
to writing it out. That way we don't have to wait for vacuum to do it
and would make your observation hold more often.

Regards,
Marti

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


Re: [HACKERS] unite recovery.conf and postgresql.conf

2011-09-25 Thread Joshua Berkus


> There might be a use case for a separate directive include_if_exists,
> or some such name.  But I think the user should have to tell us very
> clearly that it's okay for the file to not be found.

Better to go back to include_directory, then.

--Josh Berkus

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


Re: [HACKERS] unite recovery.conf and postgresql.conf

2011-09-25 Thread Tom Lane
Joshua Berkus  writes:
> What happens currently if we have an \include in postgresql.conf for a file 
> which doesn't exist?  Is it ignored, or do we error out?

It's an error, and I think changing that would be a really bad idea.

> If it could just be ignored, maybe with a note in the logs, then we could be 
> a lot more flexible.

There might be a use case for a separate directive include_if_exists,
or some such name.  But I think the user should have to tell us very
clearly that it's okay for the file to not be found.

regards, tom lane

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


Re: [HACKERS] unite recovery.conf and postgresql.conf

2011-09-25 Thread Joshua Berkus
Folks,

What happens currently if we have an \include in postgresql.conf for a file 
which doesn't exist?  Is it ignored, or do we error out?

If it could just be ignored, maybe with a note in the logs, then we could be a 
lot more flexible.

--Josh Berkus

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


Re: [HACKERS] unite recovery.conf and postgresql.conf

2011-09-25 Thread Tom Lane
Robert Haas  writes:
> On Sep 24, 2011, at 1:04 PM, Tom Lane  wrote:
>> I don't exactly buy this argument.  If postgresql.conf is hard to
>> machine-edit, why is recovery.conf any easier?

> Because you generally just write a brand-new file, without worrying
> about preserving existing settings. You aren't really editing at all,
> just writing.

If that's all the requirement is, it's trivial to implement.

1. Write your-random-configuration-settings into recovery.conf (or any
other file name you choose ... something named after your tool would be
a better idea).

2. Temporarily append "include recovery.conf" to the end of
postgresql.conf.  Restart server.

3. When done, remove "include recovery.conf" from the end of
postgresql.conf.

The hard cases involve merging user-supplied and tool-supplied settings,
but "let's overwrite recovery.conf in toto" never would have been able
to handle such cases either.

regards, tom lane

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


Re: [HACKERS] unite recovery.conf and postgresql.conf

2011-09-25 Thread Tom Lane
Simon Riggs  writes:
> On Sat, Sep 24, 2011 at 6:01 PM, Tom Lane  wrote:
>> Okay, so you do agree that eventually we want to be rid of
>> recovery.conf?  I think everyone else agrees on that.  But if we are
>> going to remove recovery.conf eventually, what is the benefit of
>> postponing doing so?

> My joyous rush into agreeing to removal has since been replaced with
> the cold reality that we must support backwards compatibility.
> Emphasise "must".

[ shrug... ]  I do not agree with your conclusion.  We have to break
some eggs to make this omelet.  The reason why we have a mess here is
that the recovery.conf mechanism, which was designed with only the
one-shot archive-recovery case in mind, has been abused beyond its
capacity.  If we don't break with past practice we are not going to be
able to fix it.  And it's not like we don't break configuration file
contents in most releases anyway, so I really fail to see why this one
has suddenly become sacrosanct.

regards, tom lane

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


Re: [HACKERS] Adding CORRESPONDING to Set Operations

2011-09-25 Thread Tom Lane
Kerem Kat  writes:
> On Sat, Sep 24, 2011 at 19:51, Tom Lane  wrote:
>> Why?  CORRESPONDING at a given set-operation level doesn't affect either
>> sub-query, so I don't see why you'd need a different representation for
>> the sub-queries.

> In the planner to construct a subquery out of SetOperationStmt or
> RangeTblRef, a new RangeTblRef is needed.
> To create a RangeTableRef, parser state is needed and planner assumes
> root->parse->rtable be not modified
> after generating simple_rte_array.

Actually, after looking at the code again, I don't think you need any of
that, since there's already a SubqueryScan node being inserted into the
plan.  You just need to improve generate_setop_tlist so that it can deal
with cases where the mapping from subplan targetlist to the setop output
columns isn't one-to-one.

regards, tom lane

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


Re: [HACKERS] Inserting heap tuples in bulk in COPY

2011-09-25 Thread Kevin Grittner
> Kohei KaiGai  wrote:
 
> I'm not clear the reason why the argument of
> CheckForSerializableConflictIn() was changed from the one in
> heap_insert().
 
The code was probably just based on heap_insert() before this recent
commit:
 
http://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=9d306c66e63eb7f45eab9475b3f96c3134bacac6
 
> Is it feasible that newly inserted tuples conflict with existing
> tuple locks when we expand insert to support multiple tuples at
> once?
 
No.
 
-Kevin

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


Re: [HACKERS] [v9.2] Fix Leaky View Problem

2011-09-25 Thread Kevin Grittner
Robert Haas  09/25/11 10:58 AM >>>

> I'm not sure we've been 100% consistent about that, since we
> previously made CREATE OR REPLACE LANGUAGE not replace the owner
> with the current user.
 
I think we've been consistent in *not* changing security on an
object when it is replaced.
 
test=# create user someoneelse;
CREATE ROLE
test=# create user yetanother;
CREATE ROLE
test=# create function one() returns int language sql as 'select 1;';
CREATE FUNCTION
test=# alter function one() owner to someoneelse;
ALTER FUNCTION
test=# revoke execute on function one() from public;
REVOKE
test=# create or replace function one() returns int language plpgsql as
$$begin return 1; end;$$;
CREATE FUNCTION
test=# \df+ one()
 List of
functions
 Schema | Name | Result data type | Argument data types |  Type  |
Volatility |Owner| Language | Source code  | Description

+--+--+-+++-+--+--+-
 public | one  | integer  | | normal |
volatile   | someoneelse | plpgsql  | begin return 1; end; | 
(1 row)

test=# set role yetanother;
SET
test=> select one();
ERROR:  permission denied for function one

-Kevin

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


[HACKERS] RES: [GENERAL] Alter column...using failure under 9.0.4

2011-09-25 Thread Edson Carlos Ericksson Richter
That's it: a check constraint I was not aware of...
Thanks!

Edson

-Mensagem original-
De: pgsql-general-ow...@postgresql.org
[mailto:pgsql-general-ow...@postgresql.org] Em nome de Tom Lane
Enviada em: domingo, 25 de setembro de 2011 13:04
Para: pgsql-gene...@postgresql.org; pgsql-hackers@postgresql.org
Assunto: Re: [GENERAL] Alter column...using failure under 9.0.4 

=?iso-8859-1?Q?Bj=F6rn_H=E4user?=  writes:
> Am 25.09.2011 um 17:17 schrieb Edson Carlos Ericksson Richter:
>> alter table usuario alter column ativo type smallint using (case when 
>> ativo then 1 else 0 end);
>> ERROR:  argument of IS FALSE must be type boolean, not type smallint

> you could check for indices or something like that.

Yeah, looks like expression index or CHECK constraint or something similar
that includes "ativo IS FALSE".

Note to hackers: I wonder whether we could make this a bit more
user-friendly by providing a CONTEXT line that shows which table property
we're trying to convert.

regards, tom lane

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


-- 
Sent 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] Alter column...using failure under 9.0.4

2011-09-25 Thread Tom Lane
=?iso-8859-1?Q?Bj=F6rn_H=E4user?=  writes:
> Am 25.09.2011 um 17:17 schrieb Edson Carlos Ericksson Richter:
>> alter table usuario alter column ativo type smallint using (case when ativo 
>> then 1 else 0 end);
>> ERROR:  argument of IS FALSE must be type boolean, not type smallint

> you could check for indices or something like that.

Yeah, looks like expression index or CHECK constraint or something
similar that includes "ativo IS FALSE".

Note to hackers: I wonder whether we could make this a bit more
user-friendly by providing a CONTEXT line that shows which table
property we're trying to convert.

regards, tom lane

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


Re: [HACKERS] Inserting heap tuples in bulk in COPY

2011-09-25 Thread Robert Haas
On Wed, Sep 14, 2011 at 6:52 AM, Heikki Linnakangas
 wrote:
>> Why do you need new WAL replay routines?  Can't you just use the existing
>> XLOG_HEAP_NEWPAGE support?
>>
>> By any large, I think we should be avoiding special-purpose WAL entries
>> as much as possible.
>
> I tried that, but most of the reduction in WAL-size melts away with that.
> And if the page you're copying to is not empty, logging the whole page is
> even more expensive. You'd need to fall back to retail inserts in that case
> which complicates the logic.

Where does it go?  I understand why it'd be a problem for partially
filled pages, but it seems like it ought to be efficient for pages
that are initially empty.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

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


Re: [HACKERS] unite recovery.conf and postgresql.conf

2011-09-25 Thread Robert Haas
On Sat, Sep 24, 2011 at 3:49 PM, Joshua Berkus  wrote:
>> Since we haven't yet come up with a reasonable way of machine-editing
>> postgresql.conf, this seems like a fairly serious objection to
>> getting
>> rid of recovery.conf.  I wonder if there's a way we can work around
>> that...
>
> Well, we *did* actually come up with a reasonable way, but it died under an 
> avalanche of bikeshedding and 
> "we-must-do-everything-the-way-we-always-have-done".  I refer, of course, to 
> the "configuration directory" patch, which was a fine solution, and would 
> indeed take care of the recovery.conf issues as well had we implemented it.  
> We can *still* implement it, for 9.2.

Well, I find that a fairly ugly solution to the problem, but I agree
that it is solvable, if we could get a critical mass on any some
particular solution.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

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


Re: [HACKERS] [v9.2] Fix Leaky View Problem

2011-09-25 Thread Robert Haas
On Sat, Sep 24, 2011 at 5:37 PM, Noah Misch  wrote:
> On Fri, Sep 23, 2011 at 06:25:01PM -0400, Robert Haas wrote:
>> On Mon, Sep 12, 2011 at 3:31 PM, Kohei KaiGai  wrote:
>> > The Part-1 implements corresponding SQL syntax stuffs which are
>> > "security_barrier"
>> > reloption of views, and "LEAKPROOF" option on creation of functions to be 
>> > stored
>> > new pg_proc.proleakproof field.
>>
>> The way you have this implemented, we just blow away all view options
>> whenever we do CREATE OR REPLACE VIEW.  Is that the behavior we want?
>> If a security_barrier view gets accidentally turned into a
>> non-security_barrier view, doesn't that create a security_hole?
>
> I think CREATE OR REPLACE needs to keep meaning just that, never becoming
> "replace some characteristics, merge others".  The consequence is less than
> delightful here, but I don't have an idea that avoids this problem without
> running afoul of some previously-raised design constraint.

Hmm, you might be right, although I'm not sure we've been 100%
consistent about that, since we previously made CREATE OR REPLACE
LANGUAGE not replace the owner with the current user.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

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


Re: [HACKERS] posix_fadvsise in base backups

2011-09-25 Thread Andres Freund
Hi Greg,

On Sunday, September 25, 2011 03:25:50 AM Greg Stark wrote:
> On Sat, Sep 24, 2011 at 4:16 PM, Magnus Hagander  
wrote:
> > I was assuming the kernel was smart enough to read this as "*this*
> > process is not going to be using this file anymore", not "nobody in
> > the whole machine is going to use this file anymore". And the process
> > running the base backup is certainly not going to read it again.
> > 
> > But that's a good point - do you know if that is the case, or does it
> > mandate more testing?
> It's not the case on Linux. I used to use DONTNEED to flush pages from
> cache before running a benchmark. I verified with mincore that the
> pages were actually getting removed from cache. Sometimes there was
> the occasional straggler but nearly all got flushed and after a second
> or third pass the stragglers were gone too.
Not sure what exactly is "not the case on Linux". Your answer could be read in 
a way that the fadvise/DONTNEED adheres to some sort of refcounting scheme 
(which it afaik does not) or that it doesn't.

> In case you're wondering, this was because using /proc/.../drop_caches
> caused flaky benchmarks. My theory was that it was causing pages of
> the executable to trigger page faults in the middle of the benchmark.
That should be easily possible to rule out by preloading the 
applications+libraries?
I think there were plans to teach the dynamic linker to enforce doing so, but 
I am not sure they were ever folloowed through.

Andres

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


Re: [HACKERS] TABLE tab completion

2011-09-25 Thread Dean Rasheed
On 24 September 2011 11:59, Magnus Hagander  wrote:
> TABLE tab completion in psql only completes to tables, not views. but
> the TABLE command works fine for both tables and views (and also
> sequences).
>
> Seems we should just complete it to relations and not tables - or can
> anyone see a particular reason why we shouldn't?
>

Doesn't that mean that "DROP TABLE " would offer up views as well
as tables, which would be incorrect?

Regards,
Dean

-- 
Sent 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 for pg_upgrade

2011-09-25 Thread panam
OK, i started once again:


I hope the following is the correct way of querying the table corresponding
to a relid:









--
View this message in context: 
http://postgresql.1045698.n5.nabble.com/fix-for-pg-upgrade-tp3411128p4838427.html
Sent from the PostgreSQL - hackers mailing list archive at Nabble.com.

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


Re: [HACKERS] Inserting heap tuples in bulk in COPY

2011-09-25 Thread Dean Rasheed
On 25 September 2011 09:43, Kohei KaiGai  wrote:
> Hi Heikki,
>
> I checked your patch, then I have a comment and two questions here.
>
> 2011/9/14 Heikki Linnakangas :
>>
>> Attached is a new version of the patch. It is now complete, including WAL
>> replay code.

Hi,

I had a quick look at the patch as well and spotted an oversight: the
multi-insert code branch fails to invoke AFTER ROW triggers.

Regards,
Dean

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


Re: [HACKERS] Inserting heap tuples in bulk in COPY

2011-09-25 Thread Kohei KaiGai
Hi Heikki,

I checked your patch, then I have a comment and two questions here.

The heap_prepare_insert() seems a duplication of code with earlier
half of existing heap_insert(). I think it is a good question to
consolidate these portion of the code.

I'm not clear the reason why the argument of
CheckForSerializableConflictIn() was
changed from the one in heap_insert(). Its source code comment describes as:
:
 * For a heap insert, we only need to check for table-level SSI locks.
 * Our new tuple can't possibly conflict with existing tuple locks, and
 * heap page locks are only consolidated versions of tuple locks; they do
 * not lock "gaps" as index page locks do.  So we don't need to identify
 * a buffer before making the call.
 */
Is it feasible that newly inserted tuples conflict with existing tuple
locks when
we expand insert to support multiple tuples at once?
It is a bit confusing for me.

This patch disallow multiple-insertion not only when before-row
trigger is configured,
but volatile functions are used to compute a default value also.
Is it a reasonable condition to avoid multiple-insertion?
All the datums to be delivered to heap_form_tuple() is calculated in
NextCopyFrom,
and default values are also constructed here; sequentially.
So, it seems to me we have no worry about volatile functions are not
invoked toward
each tuples. Or, am I missing something?

Thanks,

2011/9/14 Heikki Linnakangas :
> On 13.08.2011 17:33, Tom Lane wrote:
>>
>> Heikki Linnakangas  writes:
>>>
>>> The patch is WIP, mainly because I didn't write the WAL replay routines
>>> yet, but please let me know if you see any issues.
>>
>> Why do you need new WAL replay routines?  Can't you just use the existing
>> XLOG_HEAP_NEWPAGE support?
>>
>> By any large, I think we should be avoiding special-purpose WAL entries
>> as much as possible.
>
> I tried that, but most of the reduction in WAL-size melts away with that.
> And if the page you're copying to is not empty, logging the whole page is
> even more expensive. You'd need to fall back to retail inserts in that case
> which complicates the logic.
>
>> Also, I strongly object to turning regular heap_insert into a wrapper
>> around some other more complicated operation.  That will likely have
>> bad consequences for the performance of every other operation.
>
> Ok. I doubt it makes any noticeable difference for performance, but having
> done that, it's more readable, too, to duplicate the code.
>
> Attached is a new version of the patch. It is now complete, including WAL
> replay code.
>
> --
>  Heikki Linnakangas
>  EnterpriseDB   http://www.enterprisedb.com
>
>
> --
> Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers
>
>



-- 
KaiGai Kohei 

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