Re: [HACKERS] Proposal: knowing detail of config files via SQL

2015-01-30 Thread Sawada Masahiko
On Sat, Jan 31, 2015 at 3:20 PM, Sawada Masahiko  wrote:
> On Sat, Jan 31, 2015 at 2:00 PM, Sawada Masahiko  
> wrote:
>> On Sat, Jan 31, 2015 at 4:56 AM, Tom Lane  wrote:
>>> David Johnston  writes:
 On Fri, Jan 30, 2015 at 12:05 PM, David Fetter  wrote:
> Why might the contents of pg_settings be different from what would be
> in pg_file_settings, apart from the existence of this column?
>>>
 The contents of pg_settings uses the setting name as a primary key.
 The contents of pg_file_setting uses a compound key consisting of the
 setting name and the source file.
>>>
>>> [ raised eyebrow... ]  Seems insufficient in the case that multiple
>>> settings of the same value occur in the same source file.  Don't you
>>> need a line number in the key as well?
>>>
>>
>> Yep, a line number is also needed.
>> The source file and line number would be primary key of pg_file_settings 
>> view.
>> I need to change like that.
>>
>
> Attached patch is latest version including following changes.
> - This view is available to super use only
> - Add sourceline coulmn
>
> Also I registered CF app.
>

Sorry, I registered unnecessary (extra) threads to CF app.
How can I delete them?

Regards,

---
Sawada Masahiko


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


Re: [HACKERS] Proposal: knowing detail of config files via SQL

2015-01-30 Thread Sawada Masahiko
On Sat, Jan 31, 2015 at 2:00 PM, Sawada Masahiko  wrote:
> On Sat, Jan 31, 2015 at 4:56 AM, Tom Lane  wrote:
>> David Johnston  writes:
>>> On Fri, Jan 30, 2015 at 12:05 PM, David Fetter  wrote:
 Why might the contents of pg_settings be different from what would be
 in pg_file_settings, apart from the existence of this column?
>>
>>> The contents of pg_settings uses the setting name as a primary key.
>>> The contents of pg_file_setting uses a compound key consisting of the
>>> setting name and the source file.
>>
>> [ raised eyebrow... ]  Seems insufficient in the case that multiple
>> settings of the same value occur in the same source file.  Don't you
>> need a line number in the key as well?
>>
>
> Yep, a line number is also needed.
> The source file and line number would be primary key of pg_file_settings view.
> I need to change like that.
>

Attached patch is latest version including following changes.
- This view is available to super use only
- Add sourceline coulmn

Also I registered CF app.

Regards,

---
Sawada Masahiko


000_pg_file_settings_view_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] Providing catalog view to pg_hba.conf file - Patch submission

2015-01-30 Thread Amit Kapila
On Fri, Jan 30, 2015 at 10:58 PM, Robert Haas  wrote:
>
> On Thu, Jan 29, 2015 at 10:13 PM, Amit Kapila 
wrote:
> > I think the big problem you are mentioning can be resolved in
> > a similar way as we have done for ALTER SYSTEM which is
> > to have a separate file (.auto.conf) for settings done via
> > ALTER SYSTEM command, do you see any major problem
> > with that approach.
>
> Yes.  The contents of postgresql.conf are only mildly order-dependent.
> If you put the same setting in more than once, it matters which one is
> last.  Apart from that, though, it doesn't really matter:
> wal_keep_segments=10 means the same thing if it occurs before
> max_connections=401 that it means after that.  The same is not true of
> pg_hba.conf, where the order matters a lot.

Do you mean to say that as authentication system uses just the
first record that matches to perform authentication, it could lead
to problems if an order is not maintained?  Won't the same
set of problems can occur if user tries to that manually and do
it without proper care of such rules.  Now the problem with
command is that user can't see the order in which entries are
being made, but it seems to me that we can provide a view or some
way to user so that the order of entries is visible and the same is
allowed to be manipulated via command.

> This makes merging two
> files together much less feasible, and much more confusing.
>
> You are also a lot more likely to lock yourself out of the database by
> adjusting pg_hba.conf.

I think that could be even possible via Alter User .. password, if the
password is changed then also kind of user can be locked out of
database, basically it is also part of authentication mechanism.

> Even if I had a feature that would let me
> modify pg_hba.conf remotely, I'm not sure I'd be brave enough to use
> it.
>

Okay, but how about via some server side utility or some other way with
which users don't need to manually edit the file?

It seems to be that some of the other databases like Oracle also provide
a way for users to operate of similar files via commands, although in a
different way [1].

> Overall, this seems to me like a can of worms better left unopened.

Sure, I can understand the dangers you want to highlight, however
OTOH it seems to me that providing some way to users with which
they can change things without manually editing file is a good move.


[1]
http://docs.oracle.com/cd/B28359_01/network.111/b28317/lsnrctl.htm#i553656


With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com


Re: [HACKERS] Proposal: knowing detail of config files via SQL

2015-01-30 Thread Sawada Masahiko
On Sat, Jan 31, 2015 at 4:56 AM, Tom Lane  wrote:
> David Johnston  writes:
>> On Fri, Jan 30, 2015 at 12:05 PM, David Fetter  wrote:
>>> Why might the contents of pg_settings be different from what would be
>>> in pg_file_settings, apart from the existence of this column?
>
>> The contents of pg_settings uses the setting name as a primary key.
>> The contents of pg_file_setting uses a compound key consisting of the
>> setting name and the source file.
>
> [ raised eyebrow... ]  Seems insufficient in the case that multiple
> settings of the same value occur in the same source file.  Don't you
> need a line number in the key as well?
>

Yep, a line number is also needed.
The source file and line number would be primary key of pg_file_settings view.
I need to change like that.

Regards,

---
Sawada Masahiko


-- 
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: Overhauling our interrupt handling (was Escaping from blocked send() reprised.)

2015-01-30 Thread Heikki Linnakangas

On 01/15/2015 03:03 AM, Andres Freund wrote:

0002: Use a nonblocking socket for FE/BE communication and block using
   latches.


s/suceeds/succeeds/


0004: Process 'die' interrupts while reading/writing from the client socket.



+* Check for interrupts here, in addition to 
secure_write(),
+* because a interrupted write in secure_raw_write() 
can possibly
+* only return to here, not to secure_write().
 */
+   ProcessClientWriteInterrupt(true);


Took me a while to parse that sentence. How about:

Check for interrupts here, in addition to secure_write(), because an 
interrupted write in secure_raw_write() will return here, and we cannot 
return to secure_write() until we've written something.



0005: Move deadlock and other interrupt handling in proc.c out of signal 
handlers.

   I'm surprised how comparatively simple this turned out to be. For
   robustness and understanding I think this is a great improvement.

   New and not reviewed at all. Needs significant review. But works
   in my simple testcases.



@@ -799,6 +791,7 @@ ProcKill(int code, Datum arg)
proc = MyProc;
MyProc = NULL;
DisownLatch(&proc->procLatch);
+   SetLatch(MyLatch);

SpinLockAcquire(ProcStructLock);

@@ -868,6 +861,7 @@ AuxiliaryProcKill(int code, Datum arg)
proc = MyProc;
MyProc = NULL;
DisownLatch(&proc->procLatch);
+   SetLatch(MyLatch);

SpinLockAcquire(ProcStructLock);


These SetLatch calls are unnecessary. SwitchBackToLocalLatch() already 
sets the latch. (According to the comment in SwitchToSharedLatch() even 
that should not be necessary.)



0006: Don't allow immediate interupts during authentication anymore.


In commit message: s/interupts/interrupts/.


@@ -80,9 +73,6 @@ md5_crypt_verify(const Port *port, const char *role, char 
*client_pass,
if (*shadow_pass == '\0')
return STATUS_ERROR;/* empty password */

-   /* Re-enable immediate response to SIGTERM/SIGINT/timeout interrupts */
-   ImmediateInterruptOK = true;
-   /* And don't forget to detect one that already arrived */
CHECK_FOR_INTERRUPTS();

/*


That lone CHECK_FOR_INTERRUPTS() that remains looks pretty randomly 
placed now that the "ImmediateInterruptOK = true" is gone. I would just 
remove it. Add one to the caller if it's needed, but it probably isn't.



0007: Remove the option to service interrupts during PGSemaphoreLock().


s/don't/doesn't/ in commit message.


Questions I have about the series right now:

1) Do others agree that getting rid of ImmediateInterruptOK is
worthwile. I personally think that we really should pursue that
aggressively as a goal.


Yes.


2) Does anybody see a significant problem with the reduction of cases
where we immediately react to authentication_timeout? Since we still
react immediately when we're waiting for the client (except maybe in
sspi/kerberos?) I think that's ok. The waiting cases are slow
ident/radius/kerberos/ldap/... servers. But we really shouldn't jump
out of the relevant code anyway.


Yeah, I think that's OK. Doing those things with 
ImmediateInterruptOK=true was quite scary, and we need to stop doing 
that. It would be nice to have a wholesale solution for those lookups 
but I don't see one. So all the ident/radius/kerberos/ldap lookups will 
need to be done in a non-blocking way to have them react to the timeout. 
That requires some work, but we can leave that to a followup patch, if 
someone wants to write it.


Overall, 1-8 look good to me, except for that one incomplete comment in 
ProcessClientWriteInterrupt() I mentioned in a separate email. I haven't 
reviewed patches 9 and 10 yet.


- Heikki



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


Re: [HACKERS] PageRepairFragmentation performance

2015-01-30 Thread Peter Geoghegan
On Tue, Nov 18, 2014 at 10:03 AM, Heikki Linnakangas
 wrote:
> So there's clearly some room for improvement here. A couple of ideas:
>
> 1. Replace the qsort with something cheaper. The itemid arrays being sorted
> are small, a few hundred item at most, usually even smaller. In this pgbench
> test case I used, the typical size is about 60. With a small array a plain
> insertion sort is cheaper than the generic qsort(), because it can avoid the
> function overhead etc. involved with generic qsort. Or we could use
> something smarter, like a radix sort, knowing that we're sorting small
> integers. Or we could implement an inlineable version of qsort and use that.

> I spent some time hacking approach 1, and replaced the qsort() call with a
> bucket sort. I'm not sure if a bucket sort is optimal, or better than a
> specialized quicksort implementation, but it seemed simple.

Nice result.

Most quicksort implementations, including our own, switch to insertion
sort if the number of elements to be sorted is low enough (It occurs
when n less than 7, in our case). Also, specializing qsort() with
integers in isolation showed about a 3x improvement last I checked,
which was a few years ago now, so cache misses (reductions in which we
can gain through compile-time specialization) are likely by far the
dominant consideration here.

pgbench is probably a very conservative way of estimating how big the
array can get, given the structure of those tables. Even 60 seems high
if we're looking for some kind of rough idea of an average.

How did you arrive at this value? It seems rather high:

+#define N_SORT_BUCKETS 32

I think you're not sufficiently clear on why you're using bucket sort
rather than some other alternative. Maybe it's so obvious to you that
you forgot. I *think* it's because it's reasonable to suppose that for
this particular use-case, you know that in general there'll be a more
or less even distribution between the buckets. Right? That seems to
make sense to me. All the work that qsort() does to make sure it has a
good pivot at each level of recursion may be wasted here.

The refactoring patch certainly looks very reasonable.
-- 
Peter Geoghegan


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


Re: [HACKERS] parallel mode and parallel contexts

2015-01-30 Thread Jim Nasby

On 1/30/15 11:08 AM, Robert Haas wrote:

The final patch attached her (parallel-dummy-v2.patch) has been
updated slightly to incorporate some prefetching logic.  It's still
just demo code and is not intended for commit.  I'm not sure whether
the prefetching logic can actually be made to improve performance,
either; if anyone feels like playing with that and reporting results
back, that would be swell.


Wouldn't we want the prefetching to happen after ReadBuffer() instead of 
before? This way we risk pushing the buffer we actually want out, plus 
if it's not already available the OS probably won't try and read it 
until it's read all the prefetch blocks.

--
Jim Nasby, Data Architect, Blue Treble Consulting
Data in Trouble? Get it in Treble! http://BlueTreble.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] Fwd: [GENERAL] 4B row limit for CLOB tables

2015-01-30 Thread Jim Nasby

On 1/30/15 11:54 AM, Roger Pack wrote:

On 1/29/15, Roger Pack  wrote:

Hello.  I see on this page a mention of basically a 4B row limit for
tables that have BLOB's


Oops I meant for BYTEA or TEXT columns, but it's possible the
reasoning is the same...


It only applies to large objects, not bytea or text.


OK I think I figured out possibly why the wiki says this.  I guess
BYTEA entries > 2KB will be autostored via TOAST, which uses an OID in
its backend.  So BYTEA has a same limitation.  It appears that
disabling TOAST is not an option [1].
So I guess if the number of BYTEA entries (in the sum all tables?
partitioning doesn't help?) with size > 2KB is > 4 billion then there
is actually no option there?  If this occurred it might cause "all
sorts of things to break"? [2]


It's a bit more complex than that. First, toast isn't limited to bytea; 
it holds for ALL varlena fields in a table that are allowed to store 
externally. Second, the limit is actually per-table: every table gets 
it's own toast table, and each toast table is limited to 4B unique OIDs. 
Third, the OID counter is actually global, but the code should handle 
conflicts by trying to get another OID. See toast_save_datum(), which 
calls GetNewOidWithIndex().


Now, the reality is that GetNewOidWithIndex() is going to keep 
incrementing the global OID counter until it finds an OID that isn't in 
the toast table. That means that if you actually get anywhere close to 
using 4B OIDs you're going to become extremely unhappy with the 
performance of toasting new data.


I don't think it would be horrifically hard to change the way toast OIDs 
are assigned (I'm thinking we'd basically switch to creating a sequence 
for every toast table), but I don't think anyone's ever tried to push 
toast hard enough to hit this kind of limit.

--
Jim Nasby, Data Architect, Blue Treble Consulting
Data in Trouble? Get it in Treble! http://BlueTreble.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] Providing catalog view to pg_hba.conf file - Patch submission

2015-01-30 Thread Jim Nasby

On 1/29/15 9:13 PM, Amit Kapila wrote:

 > Aside from Tom's concern about sets not being a good way to handle
this (which I agree with), the idea of "editing" pg_hba.conf via SQL
raises all the problems that were brought up when ALTER SYSTEM was being
developed. One of the big problems is a question of how you can safely
modify a text file that's full of comments and what-not. You'd need to
address those issues if you hope to modify pg_hba.conf via SQL.
 >

I think the big problem you are mentioning can be resolved in
a similar way as we have done for ALTER SYSTEM which is
to have a separate file (.auto.conf) for settings done via
ALTER SYSTEM command, do you see any major problem
with that approach.


Yes I do. pg_hba.conf is completely depending on ordering, so there's no 
way you can simply toss another file into the mix. It's bad enough that 
we do that with postgresql.auto.conf, but at least that's a simple 
over-ride. With HBA a single ALTER SYSTEM could activate (or deactivate) 
a huge swath of pg_hba.conf. That makes for a system that's fragile, and 
since it's security related, dangerous.


I could maybe see an interface where we allowed users to perform 
line-level operations on pg_hba.conf via SQL: UPDATE line X, INSERT 
BEFORE/AFTER line X, DELETE line X. At least that would preserve the 
critical nature of rules ordering.

--
Jim Nasby, Data Architect, Blue Treble Consulting
Data in Trouble? Get it in Treble! http://BlueTreble.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] INSERT ... ON CONFLICT {UPDATE | IGNORE} 2.0

2015-01-30 Thread Peter Geoghegan
On Fri, Jan 30, 2015 at 6:59 AM, Geoff Winkless  wrote:
> I appreciate the work you're doing and (as a user rather than a
> pg-hacker) don't want to butt in but if it would be possible to allow
> support for IGNORE for unique but not exclusion constraints that would
> be really helpful for my own use cases, where being able to insert
> from a dataset into a table containing unique constraints without
> having to first check the dataset for uniqueness (within both itself
> and the target table) is very useful.
>
> It's possible that I've misunderstood anyway and that you mean purely
> that exclusion constraint IGNORE should be shelved until 9.6, in which
> case I apologise.

Well, the issue is that we can't really add exclusion constraints to
the IGNORE case later. So the fact that we cannot do exclusion
constraints kind of implies that we can either shelve IGNORE and maybe
look at it later, or accept that we'll never support exclusion
constraints with IGNORE. We'd then include IGNORE without exclusion
constraint support now and forever. I tend to think that we'll end up
doing the latter anyway, but I really don't want to add additional
risk of this not getting into 9.5 by arguing about that now. It
doesn't matter that much.

> I suppose there's no reason why we couldn't use a no-op ON CONFLICT
> UPDATE anyway

Right. IGNORE isn't really all that compelling for that reason. Note
that this will still lock the unmodified row, though.

-- 
Peter Geoghegan


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


Re: [HACKERS] tablespaces inside $PGDATA considered harmful

2015-01-30 Thread Josh Berkus
Robert, Stephen, etc.:

Apparently you can create a tablespace in the tablespace directory:

josh=# create tablespace tbl location '/home/josh/pg94/data/pg_tblspc/';
CREATE TABLESPACE
josh=# create table test_tbl ( test text ) tablespace tbl;
CREATE TABLE
josh=# \q
josh@Radegast:~/pg94/data/pg_tblspc$ ls
17656  PG_9.4_201409291
josh@Radegast:~/pg94/data/pg_tblspc$ ls -l
total 4
lrwxrwxrwx 1 josh josh   30 Jan 30 13:02 17656 ->
/home/josh/pg94/data/pg_tblspc
drwx-- 3 josh josh 4096 Jan 30 13:02 PG_9.4_201409291
josh@Radegast:~/pg94/data/pg_tblspc$

In theory if I could guess the next OID, I could cause a failure there,
but that appears to be obscure enough to be not worth bothering about.

What is a real problem is that we don't block creating tablespaces
anywhere at all, including in obviously problematic places like the
transaction log directory:

josh=# create tablespace tbl2 location '/home/josh/pg94/data/pg_xlog/';
CREATE TABLESPACE

It really seems like we ought to block *THAT*.  Of course, if we block
tablespace creation in PGDATA generally, then that's covered.

-- 
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.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] Parallel Seq Scan

2015-01-30 Thread Stephen Frost
Daniel,

* Daniel Bausch (bau...@dvs.tu-darmstadt.de) wrote:
> I have been researching this topic long time ago.  One notably fact is
> that active prefetching disables automatic readahead prefetching (by
> Linux kernel), which can occour in larger granularities than 8K.
> Automatic readahead prefetching occours when consecutive addresses are
> read, which may happen by a seqscan but also by "accident" through an
> indexscan in correlated cases.

That strikes me as a pretty good point to consider.

> My consequence was to NOT prefetch seqscans, because OS does good enough
> without advice.  Prefetching indexscan heap accesses is very valuable
> though, but you need to detect the accidential sequential accesses to
> not hurt your performance in correlated cases.

Seems like we might be able to do that, it's not that different from
what we do with the bitmap scan case, we'd just look at the bitmap and
see if there's long runs of 1's.

> In general I can give you the hint to not only focus on HDDs with their
> single spindle.  A single SATA SSD scales up to 32 (31 on Linux)
> requests in parallel (without RAID or anything else).  The difference in
> throughput is extreme for this type of storage device.  While single
> spinning HDDs can only gain up to ~20% by NCQ, SATA SSDs can easily gain
> up to 700%.

I definitely agree with the idea that we should be looking at SSD-based
systems but I don't know if anyone happens to have easy access to server
gear with SSDs.  I've got an SSD in my laptop, but that's not really the
same thing.

Thanks!

Stephen


signature.asc
Description: Digital signature


Re: [HACKERS] Possible typo in create_policy.sgml

2015-01-30 Thread Stephen Frost
* Dean Rasheed (dean.a.rash...@gmail.com) wrote:
> Don't forget the ALTER POLICY page. This and some of the other things
> being discussed on this thread ought to be copied there too.

Thanks, I've fixed this also.

Stephen


signature.asc
Description: Digital signature


Re: [HACKERS] Possible typo in create_policy.sgml

2015-01-30 Thread Stephen Frost
* Dean Rasheed (dean.a.rash...@gmail.com) wrote:
> On 30 January 2015 at 03:40, Stephen Frost  wrote:
> > * Robert Haas (robertmh...@gmail.com) wrote:
> >> On Thu, Jan 29, 2015 at 9:04 PM, Stephen Frost  wrote:
> >> > A policy grants the ability to SELECT, INSERT, UPDATE, or DELETE rows
> >> > which match the relevant policy expression. Existing table rows are
> >> > checked against the expression specified via USING, while new rows
> >> > that would be created via INSERT or UPDATE are checked against the
> >> > expression specified via WITH CHECK.  When a USING expression returns
> >> > false for a given row, that row is not visible to the user.  When a WITH
> >> > CHECK expression returns false for a row which is to be added, an error
> >> > occurs.
> >>
> >> Yeah, that's not bad.  I think it's an improvement, in fact.
> 
> Yes I like that too. My main concern was that we should be describing
> policies in terms of permitting access to the table, not limiting
> access, because of the default-deny policy, and this new text clears
> that up.

Great, thanks, pushed.

> One additional quibble -- it's misleading to say "expression returns
> false" here (and later in the check_expression parameter description)
> because if the expression returns null, that's also a failure. So it
> ought to be "false or null", but perhaps it could just be described in
> terms of rows matching the expression, with a separate note to say
> that a row only matches a policy expression if that expression returns
> true, not false or null.

Good point, I've made a few minor changes to address that also, please
let me know if you see any issus.

Thanks!

Stephen


signature.asc
Description: Digital signature


Re: [HACKERS] Buildfarm broken for 9.3 and up

2015-01-30 Thread Andrew Dunstan


On 01/30/2015 11:00 AM, Tom Lane wrote:



I do see that macque is having issues, but it's been busted for the past
3 days it looks like.

Yeah.  Several of the critters have been having git issues for weeks
to months.  I wonder whether we couldn't teach the buildfarm script
to recover from this automatically ...






Could get very messy, especially when the error is in the mirror, as is 
apparently the case here. As I mentioned the other day on the buildfarm 
list, I'm playing with a mode that would largely obviate the need for a 
mirror. Failure cases are what's worrying me, though.


In this particular case, the owner should probably remove the mirror and 
each of the branch pgsql directories.


cheers

andrew


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


Re: [HACKERS] basebackups during ALTER DATABASE ... SET TABLESPACE ... not safe?

2015-01-30 Thread Andres Freund
On 2015-01-27 20:16:43 +0100, Andres Freund wrote:
> Here's an alternative approach. I think it generally is superior and
> going in the right direction, but I'm not sure it's backpatchable.
> 
> It basically consists out of:
> 1) Make GetLockConflicts() actually work.

already commited as being a independent problem.

> 2) Allow the startup process to actually acquire locks other than
>AccessExclusiveLocks. There already is code acquiring other locks,
>but it's currently broken because they're acquired in blocking mode
>which isn't actually supported in the startup mode. Using this
>infrastructure we can actually fix a couple small races around
>database creation/drop.
> 3) Allow session locks during recovery to be heavier than
>RowExclusiveLock - since relation/object session locks aren't ever
>taken out by the user (without a plain lock first) that's safe.

merged and submitted independently.

> 5) Make walsender actually react to recovery conflict interrupts

submitted here. (0003)

> 4) Perform streaming base backups from within a transaction command, to
>provide error handling.
> 6) Prevent access to the template database of a CREATE DATABASE during
>WAL replay.
> 7) Add an interlock to prevent base backups and movedb() to run
>concurrently. What we actually came here for.

combined, submitted here. (0004)

I think this actually doesn't look that bad.

Greetings,

Andres Freund

-- 
 Andres Freund http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services
>From 8531ca4d200d24ee45265774f7ead613563adca4 Mon Sep 17 00:00:00 2001
From: Andres Freund 
Date: Fri, 30 Jan 2015 09:28:17 +0100
Subject: [PATCH 1/4] Allow recovery lock infrastructure to not only hold
 relation locks.

This is a preparatory commit to fix several bugs, separated out for
easier review.

The startup process could so far only properly acquire access exlusive
locks on transactions. As it does not setup enough state to do lock
queuing - primarily to be able to issue recovery conflict interrupts,
and to release them when the primary releases locks - it has it's own
infrastructure to manage locks. That infrastructure so far assumed
that all locks are access exlusive locks on relations.

Unfortunately some code in the startup process has to acquire other
locks than what's supported by the aforementioned infrastructure in
standby.c. Namely dbase_redo() has to acquire locks on the database
objects.  Also further such locks will be added soon, to fix a another
bug.

So this patch shanges the infrastructure to be able to acquire locks
of different modes and locktags.

Additionally allow acquiring more heavyweight relation logs on the
standby than RowExclusive when acquired in session mode.

Discussion: 20150120152819.gc24...@alap3.anarazel.de

Backpatch all the way.
---
 src/backend/storage/ipc/standby.c | 120 ++
 src/backend/storage/lmgr/lock.c   |   7 +--
 src/include/storage/standby.h |   2 +-
 3 files changed, 61 insertions(+), 68 deletions(-)

diff --git a/src/backend/storage/ipc/standby.c b/src/backend/storage/ipc/standby.c
index 292bed5..0502aab 100644
--- a/src/backend/storage/ipc/standby.c
+++ b/src/backend/storage/ipc/standby.c
@@ -38,10 +38,16 @@ int			max_standby_archive_delay = 30 * 1000;
 int			max_standby_streaming_delay = 30 * 1000;
 
 static List *RecoveryLockList;
+typedef struct RecoveryLockListEntry
+{
+	TransactionId	xid;
+	LOCKMODE		lockmode;
+	LOCKTAG			locktag;
+} RecoveryLockListEntry;
 
 static void ResolveRecoveryConflictWithVirtualXIDs(VirtualTransactionId *waitlist,
 	   ProcSignalReason reason);
-static void ResolveRecoveryConflictWithLock(Oid dbOid, Oid relOid);
+static void ResolveRecoveryConflictWithLock(LOCKTAG *tag, LOCKMODE mode);
 static void SendRecoveryConflictWithBufferPin(ProcSignalReason reason);
 static XLogRecPtr LogCurrentRunningXacts(RunningTransactions CurrRunningXacts);
 static void LogAccessExclusiveLocks(int nlocks, xl_standby_lock *locks);
@@ -320,10 +326,10 @@ ResolveRecoveryConflictWithDatabase(Oid dbid)
 	 * us. This is rare enough that we do this as simply as possible: no wait,
 	 * just force them off immediately.
 	 *
-	 * No locking is required here because we already acquired
-	 * AccessExclusiveLock. Anybody trying to connect while we do this will
-	 * block during InitPostgres() and then disconnect when they see the
-	 * database has been removed.
+	 * No locking is required here because we already acquired a
+	 * AccessExclusiveLock on the database in dbase_redo(). Anybody trying to
+	 * connect while we do this will block during InitPostgres() and then
+	 * disconnect when they see the database has been removed.
 	 */
 	while (CountDBBackends(dbid) > 0)
 	{
@@ -338,14 +344,11 @@ ResolveRecoveryConflictWithDatabase(Oid dbid)
 }
 
 static void
-ResolveRecoveryConflictWithLock(Oid dbOid, Oid relOid)
+ResolveRecoveryConflictWithLock(LOCKTAG *locktag, 

Re: [HACKERS] Hot Standby WAL reply uses heavyweight session locks, but doesn't have enough infrastructure set up

2015-01-30 Thread Andres Freund
On 2015-01-29 11:01:51 -0500, Robert Haas wrote:
> On Wed, Jan 28, 2015 at 2:41 AM, Michael Paquier
>  wrote:
> > Andres Freund wrote:
> >> I think this isn't particularly pretty, but it seems to be working well
> >> enough, and changing it would be pretty invasive. So keeping in line
> >> with all that code seems to be easier.
> > OK, I'm convinced with this part to remove the call of
> > LockSharedObjectForSession that uses dontWait and replace it by a loop
> > in ResolveRecoveryConflictWithDatabase.
> 
> That seems right to me, too.

It's slightly more complicated than that. The lock conflict should
actually be resolved using ResolveRecoveryConflictWithLock()... That,
combined with the race of connecting a actually already deleted database
(see the XXXs I removed) seem to make the approach in here.

Attached are two patches:
1) Infrastructure for attaching more kinds of locks on the startup
   process.
2) Use that infrastructure for database locks during replay.

I'm not sure 2) alone would be sufficient justification for 1), but the
nearby thread about basebackups also require similar infrastructure...

Greetings,

Andres Freund

-- 
 Andres Freund http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services
>From 8531ca4d200d24ee45265774f7ead613563adca4 Mon Sep 17 00:00:00 2001
From: Andres Freund 
Date: Fri, 30 Jan 2015 09:28:17 +0100
Subject: [PATCH 1/4] Allow recovery lock infrastructure to not only hold
 relation locks.

This is a preparatory commit to fix several bugs, separated out for
easier review.

The startup process could so far only properly acquire access exlusive
locks on transactions. As it does not setup enough state to do lock
queuing - primarily to be able to issue recovery conflict interrupts,
and to release them when the primary releases locks - it has it's own
infrastructure to manage locks. That infrastructure so far assumed
that all locks are access exlusive locks on relations.

Unfortunately some code in the startup process has to acquire other
locks than what's supported by the aforementioned infrastructure in
standby.c. Namely dbase_redo() has to acquire locks on the database
objects.  Also further such locks will be added soon, to fix a another
bug.

So this patch shanges the infrastructure to be able to acquire locks
of different modes and locktags.

Additionally allow acquiring more heavyweight relation logs on the
standby than RowExclusive when acquired in session mode.

Discussion: 20150120152819.gc24...@alap3.anarazel.de

Backpatch all the way.
---
 src/backend/storage/ipc/standby.c | 120 ++
 src/backend/storage/lmgr/lock.c   |   7 +--
 src/include/storage/standby.h |   2 +-
 3 files changed, 61 insertions(+), 68 deletions(-)

diff --git a/src/backend/storage/ipc/standby.c b/src/backend/storage/ipc/standby.c
index 292bed5..0502aab 100644
--- a/src/backend/storage/ipc/standby.c
+++ b/src/backend/storage/ipc/standby.c
@@ -38,10 +38,16 @@ int			max_standby_archive_delay = 30 * 1000;
 int			max_standby_streaming_delay = 30 * 1000;
 
 static List *RecoveryLockList;
+typedef struct RecoveryLockListEntry
+{
+	TransactionId	xid;
+	LOCKMODE		lockmode;
+	LOCKTAG			locktag;
+} RecoveryLockListEntry;
 
 static void ResolveRecoveryConflictWithVirtualXIDs(VirtualTransactionId *waitlist,
 	   ProcSignalReason reason);
-static void ResolveRecoveryConflictWithLock(Oid dbOid, Oid relOid);
+static void ResolveRecoveryConflictWithLock(LOCKTAG *tag, LOCKMODE mode);
 static void SendRecoveryConflictWithBufferPin(ProcSignalReason reason);
 static XLogRecPtr LogCurrentRunningXacts(RunningTransactions CurrRunningXacts);
 static void LogAccessExclusiveLocks(int nlocks, xl_standby_lock *locks);
@@ -320,10 +326,10 @@ ResolveRecoveryConflictWithDatabase(Oid dbid)
 	 * us. This is rare enough that we do this as simply as possible: no wait,
 	 * just force them off immediately.
 	 *
-	 * No locking is required here because we already acquired
-	 * AccessExclusiveLock. Anybody trying to connect while we do this will
-	 * block during InitPostgres() and then disconnect when they see the
-	 * database has been removed.
+	 * No locking is required here because we already acquired a
+	 * AccessExclusiveLock on the database in dbase_redo(). Anybody trying to
+	 * connect while we do this will block during InitPostgres() and then
+	 * disconnect when they see the database has been removed.
 	 */
 	while (CountDBBackends(dbid) > 0)
 	{
@@ -338,14 +344,11 @@ ResolveRecoveryConflictWithDatabase(Oid dbid)
 }
 
 static void
-ResolveRecoveryConflictWithLock(Oid dbOid, Oid relOid)
+ResolveRecoveryConflictWithLock(LOCKTAG *locktag, LOCKMODE mode)
 {
 	VirtualTransactionId *backends;
 	bool		lock_acquired = false;
 	int			num_attempts = 0;
-	LOCKTAG		locktag;
-
-	SET_LOCKTAG_RELATION(locktag, dbOid, relOid);
 
 	/*
 	 * If blowing away everybody with conflicting locks doesn't work, after
@@ -358,7 +361,7 @@ ResolveRecoveryC

Re: [HACKERS] Proposal: knowing detail of config files via SQL

2015-01-30 Thread Tom Lane
David Johnston  writes:
> On Fri, Jan 30, 2015 at 12:05 PM, David Fetter  wrote:
>> Why might the contents of pg_settings be different from what would be
>> in pg_file_settings, apart from the existence of this column?

> ​​The contents of pg_settings uses the setting name as a primary key.
> The contents of pg_file_setting uses a compound key consisting of the
> setting name and the source file.

[ raised eyebrow... ]  Seems insufficient in the case that multiple
settings of the same value occur in the same source file.  Don't you
need a line number in the key as well?

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] Proposal: knowing detail of config files via SQL

2015-01-30 Thread David Johnston
On Fri, Jan 30, 2015 at 12:05 PM, David Fetter  wrote:

> On Sat, Jan 31, 2015 at 12:50:20AM +0900, Sawada Masahiko wrote:
> >
> > [postgres][5432](1)=# select * from pg_file_settings where name =
> 'work_mem';
> > -[ RECORD 1 ]--
> > name   | work_mem
> > setting| 128MB
> > sourcefile | /home/masahiko/pgsql/master/3data/hoge.conf
> > -[ RECORD 2 ]--
> > name   | work_mem
> > setting| 64MB
> > sourcefile | /home/masahiko/pgsql/master/3data/postgresql.auto.conf
>
> Masuhiko-san,
>
> I apologize for not communicating clearly.  My alternative proposal is
> to add a NULL-able sourcefile column to pg_settings.  This is as an
> alternative to creating a new pg_file_settings view.
>
> Why might the contents of pg_settings be different from what would be
> in pg_file_settings, apart from the existence of this column?
>
>
​
​​The contents of pg_settings uses the setting name as a primary key.

The contents of pg_file_setting uses a compound key consisting of the
setting name and the source file.

See "work_mem" in the provided example.

More specifically pg_settings only care about the "currently active
setting" while this cares about "inactive" settings as well.

David J.
​


Re: [HACKERS] Proposal: knowing detail of config files via SQL

2015-01-30 Thread David Fetter
On Sat, Jan 31, 2015 at 12:50:20AM +0900, Sawada Masahiko wrote:
> On Sat, Jan 31, 2015 at 12:24 AM, David Fetter  wrote:
> > On Fri, Jan 30, 2015 at 09:38:10PM +0900, Sawada Masahiko wrote:
> >> On Tue, Jan 27, 2015 at 3:34 AM, Robert Haas  wrote:
> >> > On Thu, Jan 22, 2015 at 5:19 PM, Tom Lane  wrote:
> >> >> David Johnston  writes:
> >> >>> On Thu, Jan 22, 2015 at 3:04 PM, Tom Lane  wrote:
> >>  Is that a requirement, and if so why?  Because this proposal doesn't
> >>  guarantee any such knowledge AFAICS.
> >> >>
> >> >>> The proposal provides for SQL access to all possible sources of 
> >> >>> variable
> >> >>> value setting and, ideally, a means of ordering them in priority 
> >> >>> order, so
> >> >>> that a search for TimeZone would return two records, one for
> >> >>> postgresql.auto.conf and one for postgresql.conf - which are numbered 
> >> >>> 1 and
> >> >>> 2 respectively - so that in looking at that result if the
> >> >>> postgresql.auto.conf entry were to be removed the user would know that 
> >> >>> what
> >> >>> the value is in postgresql.conf that would become active.  
> >> >>> Furthermore, if
> >> >>> postgresql.conf has a setting AND there is a mapping in an #included 
> >> >>> file
> >> >>> that information would be accessible via SQL as well.
> >> >>
> >> >> I know what the proposal is.  What I am questioning is the use-case that
> >> >> justifies having us build and support all this extra mechanism.  How 
> >> >> often
> >> >> does anyone need to know what the "next down" variable value would be?
> >> >
> >> > I believe I've run into situations where this would be useful.  I
> >> > wouldn't be willing to put in the effort to do this myself, but I
> >> > wouldn't be prepared to vote against a high-quality patch that someone
> >> > else felt motivated to write.
> >> >
> >>
> >> Attached patch adds new view pg_file_settings (WIP version).
> >> This view behaves like followings,
> >> [postgres][5432](1)=# select * from pg_file_settings ;
> >> name|  setting   |
> >>   sourcefile
> >> ++
> >>  max_connections| 100|
> >> /home/masahiko/pgsql/master/3data/postgresql.conf
> >>  shared_buffers | 128MB  |
> >> /home/masahiko/pgsql/master/3data/postgresql.conf
> >
> > This looks great!
> >
> > Is there a reason not to have the sourcefile as a column in
> > pg_settings?
> >
> 
> pg_file_settings view also has source file column same as pg_settings.
> It might was hard to confirm that column in previous mail.
> I put example of pg_file_settings again as follows.
> 
> [postgres][5432](1)=# select * from pg_file_settings where name = 'work_mem';
> -[ RECORD 1 ]--
> name   | work_mem
> setting| 128MB
> sourcefile | /home/masahiko/pgsql/master/3data/hoge.conf
> -[ RECORD 2 ]--
> name   | work_mem
> setting| 64MB
> sourcefile | /home/masahiko/pgsql/master/3data/postgresql.auto.conf

Masuhiko-san,

I apologize for not communicating clearly.  My alternative proposal is
to add a NULL-able sourcefile column to pg_settings.  This is as an
alternative to creating a new pg_file_settings view.

Why might the contents of pg_settings be different from what would be
in pg_file_settings, apart from the existence of this column?

Cheers,
David.
-- 
David Fetter  http://fetter.org/
Phone: +1 415 235 3778  AIM: dfetter666  Yahoo!: dfetter
Skype: davidfetter  XMPP: david.fet...@gmail.com

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate


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


Re: [HACKERS] tablespaces inside $PGDATA considered harmful

2015-01-30 Thread Josh Berkus
On 01/30/2015 09:19 AM, Stephen Frost wrote:
> * Robert Haas (robertmh...@gmail.com) wrote:
>> Given all this, it seems like a good idea to at least give a warning
>> if somebody tries to create a tablespace instead the data directory.
> 
> A warning seems like a good idea.  I actually thought we *did* prevent
> it..
> 
>> Arguably, we should prohibit it altogether, but there are obviously
>> people that want to do it, and there could even be somewhat valid
>> reasons for that, like wanting to set per-tablespace settings
>> differently for different tablespaces.  Possibly we should prohibit it
>> anyway, or maybe there should be an option to create a tablespace
>> whose directory is a real directory, not a symlink.  So then:
>>
>> CREATE TABLESPACE foo LOCATION '/home/rhaas/pgdata/pg_tblspc/foo';
>>
>> ...would fail, but if you really want a separate tablespace inside the
>> data directory, we could allow:
>>
>> CREATE TABLESPACE foo NO LOCATION;
>>
>> ...which would just create a bare directory where the symlink would normally 
>> go.
> 
> I actually really like this 'NO LOCATION' idea.  Are there reasons why
> that would be difficult or ill-advised to do?
> 
> I could see the NO LOCATION approach being useful for migrating between
> systems, in particular, or a way to have pg_basebackup work that doesn't
> involve having to actually map all the tablespaces...

I like this idea too.  And it would make tablespaces more manageable for
people who are using them for reasons other than putting them on
different disks.

-- 
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.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] PATCH: Reducing lock strength of trigger and foreign key DDL

2015-01-30 Thread Andreas Karlsson

On 01/30/2015 07:48 AM, Michael Paquier wrote:

Ok, so the deal is to finally reduce the locks to
ShareRowExclusiveLock for the following commands :
- CREATE TRIGGER
- ALTER TABLE ENABLE/DISABLE
- ALTER TABLE ADD CONSTRAINT


Correct. I personally still find this useful enough to justify a patch.


Looking at the latest patch, it seems that in
AlterTableGetLockLevel@tablecmds.c we ought to put AT_ReAddConstraint,
AT_AddConstraintRecurse and AT_ProcessedConstraint under the same
banner as AT_AddConstraint. Thoughts?


Good point. I think moving those would be a good thing even though it is 
technically not necessary for AT_AddConstraintRecurse, since that one 
should only be related to check constraints.


--
Andreas



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


[HACKERS]

2015-01-30 Thread jeff . janes
>From nobody Fri Jan 30 18:20:23 2015
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Subject: Re: Parallel Seq Scan
To: pgsql-hackers@postgresql.org
From: Jeff Janes 
Date: Fri, 30 Jan 2015 18:20:23 +
User-Agent: pgcommitfest
X-cfsender: jjanes
In-Reply-To: 

References: 
 
Message-ID: <20150130182023.2534.29459.p...@coridan.postgresql.org>

This patch depends on https://commitfest.postgresql.org/3/22/ 


-- 
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: Overhauling our interrupt handling (was Escaping from blocked send() reprised.)

2015-01-30 Thread Heikki Linnakangas

On 01/15/2015 03:03 AM, Andres Freund wrote:

0004: Process 'die' interrupts while reading/writing from the client socket.

   This is the reason Horiguchi-san started this thread.



+   ProcessClientWriteInterrupt(!port->noblock);

...

+/*
+ * ProcessClientWriteInterrupt() - Process interrupts specific to client writes
+ *
+ * This is called just after low-level writes. That might be after the read
+ * finished successfully, or it was interrupted via interrupt. 'blocked' tells
+ * us whether the
+ *
+ * Must preserve errno!
+ */
+void
+ProcessClientWriteInterrupt(bool blocked)


You're passing port->noblock as argument, but I thought the argument is 
supposed to mean whether the write would've blocked, i.e. if the write 
buffer was full. port->noblock doesn't mean that. But perhaps I 
misunderstood this - the comment on the 'blocked' argument above is a 
bit incomplete ;-).


- Heikki



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


Re: [HACKERS] Safe memory allocation functions

2015-01-30 Thread Robert Haas
On Fri, Jan 30, 2015 at 1:10 AM, Michael Paquier
 wrote:
> I wrote:
>> Yes, this refactoring was good for testing actually...
> Oops, I have been too hasty when sending previous patch, there was a
> bug related to huge allocations. Patch correcting this bug is
> attached.

Committed.  I didn't think we really need to expose two separate flags
for the aligned and unaligned cases, so I ripped that out.  I also
removed the duplicate documentation of the new constants in the
function header; having two copies of the documentation, one far
removed from the constants themselves, is a recipe for them eventually
getting out of sync.  I also moved the function to what I thought was
a more logical place in the file, and rearranged the order of tests
slightly so that, in the common path, we test only whether ret == NULL
and not anything else.

-- 
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] Fwd: [GENERAL] 4B row limit for CLOB tables

2015-01-30 Thread Roger Pack
>> On 1/29/15, Roger Pack  wrote:
>>> Hello.  I see on this page a mention of basically a 4B row limit for
>>> tables that have BLOB's
>>
>> Oops I meant for BYTEA or TEXT columns, but it's possible the
>> reasoning is the same...
>
> It only applies to large objects, not bytea or text.

OK I think I figured out possibly why the wiki says this.  I guess
BYTEA entries > 2KB will be autostored via TOAST, which uses an OID in
its backend.  So BYTEA has a same limitation.  It appears that
disabling TOAST is not an option [1].
So I guess if the number of BYTEA entries (in the sum all tables?
partitioning doesn't help?) with size > 2KB is > 4 billion then there
is actually no option there?  If this occurred it might cause "all
sorts of things to break"? [2]
Thanks!
-roger-

[1] 
http://www.postgresql.org/message-id/20130405140348.gc4...@awork2.anarazel.de
[2] 
http://www.postgresql.org/message-id/CAL1QdWfb-p5kE9DT2pMqBxohaKG=vxmdremsbjc+7tkboek...@mail.gmail.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] tablespaces inside $PGDATA considered harmful

2015-01-30 Thread David G Johnston
Robert Haas wrote
> Arguably, we should prohibit it altogether, but there are obviously
> people that want to do it, and there could even be somewhat valid
> reasons for that, 

Lots of hand-waving here and it is just as likely they simply are not aware
of the downsides and the only reason they put it is $PGDATA is that
it seemed like a logical place to put a directory that is intended to hold
database data.


> like wanting to set per-tablespace settings differently for different
> tablespaces.

I do not follow where this has anything to do with the location of the
physical tablespace directory?


> Possibly we should prohibit it
> anyway, or maybe there should be an option to create a tablespace
> whose directory is a real directory, not a symlink.  So then:
> 
> CREATE TABLESPACE foo LOCATION '/home/rhaas/pgdata/pg_tblspc/foo';
> 
> ...would fail, but if you really want a separate tablespace inside the
> data directory, we could allow:
> 
> CREATE TABLESPACE foo NO LOCATION;
> 
> ...which would just create a bare directory where the symlink would
> normally go.

CREATE TABLE foo LOCATION INTERNAL

The creators of tablespaces seem to have envisioned their usage as a means
of pulling in
disparate file systems and not simply for namespaces within the main
filesystem
that $PGDATA exists on.

This seems arbitrary and while the internal location specification likely
doesn't buy one much in terms of real options it doesn't seem like it has
any serious downsides either.


> In the short term, I favor just adding a warning, so that people get
> some clue that they are doing something that might be a bad idea.  In
> the long term, we might want to do more.  Thoughts?

If this is intended to be back-patched then I'd go with just a warning.  If
this is strictly 9.5 material then I'd say that since our own tools behave
badly in the current situation we should simply outright disallow it.  In
either case we should consider what tools we can provide to detect the
now-illegal configuration and, during pg_upgrade, configure the new cluster
to adhere to the correct configuration or help the user migrate their
internalized tablespaces to a different part of their filesystem.

Writing this I ponder the situation that someone would mount a different
file system directly under $PGDATA so that they get both benefits - single
parent and the different properties of the filesystems they are using.  If
we force "Internal" to be in the same location as the default tablespace we
only accomplish half of their goals.

David J.



--
View this message in context: 
http://postgresql.nabble.com/tablespaces-inside-PGDATA-considered-harmful-tp5836161p5836180.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] Providing catalog view to pg_hba.conf file - Patch submission

2015-01-30 Thread Robert Haas
On Thu, Jan 29, 2015 at 10:13 PM, Amit Kapila  wrote:
> I think the big problem you are mentioning can be resolved in
> a similar way as we have done for ALTER SYSTEM which is
> to have a separate file (.auto.conf) for settings done via
> ALTER SYSTEM command, do you see any major problem
> with that approach.

Yes.  The contents of postgresql.conf are only mildly order-dependent.
If you put the same setting in more than once, it matters which one is
last.  Apart from that, though, it doesn't really matter:
wal_keep_segments=10 means the same thing if it occurs before
max_connections=401 that it means after that.  The same is not true of
pg_hba.conf, where the order matters a lot.  This makes merging two
files together much less feasible, and much more confusing.

You are also a lot more likely to lock yourself out of the database by
adjusting pg_hba.conf.  You can do that by modifying postgresql.conf,
say by putting an invalid combination of parameters in there or
getting it to request more semaphores or more RAM than the system can
accommodate or changing listen_addresses to 127.0.0.1, but there are
lots of things that you can do that carry no such risk.  This is much
less true with pg_hba.conf.  Even if I had a feature that would let me
modify pg_hba.conf remotely, I'm not sure I'd be brave enough to use
it.

Overall, this seems to me like a can of worms better left unopened.

-- 
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] tablespaces inside $PGDATA considered harmful

2015-01-30 Thread Stephen Frost
* Robert Haas (robertmh...@gmail.com) wrote:
> Given all this, it seems like a good idea to at least give a warning
> if somebody tries to create a tablespace instead the data directory.

A warning seems like a good idea.  I actually thought we *did* prevent
it..

> Arguably, we should prohibit it altogether, but there are obviously
> people that want to do it, and there could even be somewhat valid
> reasons for that, like wanting to set per-tablespace settings
> differently for different tablespaces.  Possibly we should prohibit it
> anyway, or maybe there should be an option to create a tablespace
> whose directory is a real directory, not a symlink.  So then:
> 
> CREATE TABLESPACE foo LOCATION '/home/rhaas/pgdata/pg_tblspc/foo';
> 
> ...would fail, but if you really want a separate tablespace inside the
> data directory, we could allow:
> 
> CREATE TABLESPACE foo NO LOCATION;
> 
> ...which would just create a bare directory where the symlink would normally 
> go.

I actually really like this 'NO LOCATION' idea.  Are there reasons why
that would be difficult or ill-advised to do?

I could see the NO LOCATION approach being useful for migrating between
systems, in particular, or a way to have pg_basebackup work that doesn't
involve having to actually map all the tablespaces...

Thanks!

Stephen


signature.asc
Description: Digital signature


Re: [HACKERS] tablespaces inside $PGDATA considered harmful

2015-01-30 Thread David Steele
On 1/30/15 11:43 AM, Joshua D. Drake wrote:
> On 01/30/2015 08:19 AM, Bruce Momjian wrote:
>>
>> On Fri, Jan 30, 2015 at 11:12:43AM -0500, Robert Haas wrote:
>>> I think everyone who has read this mailing list for a while is
>>> probably already aware of this problem.  When you create a tablespace
>>> somewhere inside the data directory, weird things happen. If you
>>> pg_upgrade and then incautiously run the delete_old_cluster.sh script
>>> thus created, you will blow away large chunks of your data.[1]  If you
>>
>> pg_upgrade doesn't create the deletion script in this case, and warns
>> the user:
>>
>>  Could not create a script to delete the old cluster's data
>>  files because user-defined tablespaces exist in the old cluster
>>  directory.  The old cluster's contents must be deleted
>> manually.
>>
>>> In the short term, I favor just adding a warning, so that people get
>>> some clue that they are doing something that might be a bad idea.  In
>>> the long term, we might want to do more.  Thoughts?
>>
>> Yes, good idea.
>
> Uhm, wouldn't it be a rather simple patch to say:
>
> if tablespace_create() in $PGDATA:
>   ERROR!
>
> ?
>
> I mean yes a warning is good but it is after the fact, the tablespace
> is already created. We know that tablespaces in $PGDATA are a bad
> idea, why not protect the user?

I would be in favor of an error.  It would then be OK for basebackup,
pg_upgrade, and friends to error when a tablespace lives in $PGDATA,
rather than trying to deal with the situation in strange ways.

If the user really wants tablespaces in $PGDATA they can always change
the links manually in the filesystem and deal with any consequences on
their own. 

-- 
- David Steele
da...@pgmasters.net




signature.asc
Description: OpenPGP digital signature


Re: [HACKERS] tablespaces inside $PGDATA considered harmful

2015-01-30 Thread Robert Haas
On Fri, Jan 30, 2015 at 11:43 AM, Joshua D. Drake  
wrote:
> I mean yes a warning is good but it is after the fact, the tablespace is
> already created. We know that tablespaces in $PGDATA are a bad idea, why not
> protect the user?

Please go back and read the discussion of that option in the OP.

-- 
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] tablespaces inside $PGDATA considered harmful

2015-01-30 Thread Joshua D. Drake


On 01/30/2015 08:19 AM, Bruce Momjian wrote:


On Fri, Jan 30, 2015 at 11:12:43AM -0500, Robert Haas wrote:

I think everyone who has read this mailing list for a while is
probably already aware of this problem.  When you create a tablespace
somewhere inside the data directory, weird things happen. If you
pg_upgrade and then incautiously run the delete_old_cluster.sh script
thus created, you will blow away large chunks of your data.[1]  If you


pg_upgrade doesn't create the deletion script in this case, and warns
the user:

 Could not create a script to delete the old cluster's data
 files because user-defined tablespaces exist in the old cluster
 directory.  The old cluster's contents must be deleted manually.


In the short term, I favor just adding a warning, so that people get
some clue that they are doing something that might be a bad idea.  In
the long term, we might want to do more.  Thoughts?


Yes, good idea.


Uhm, wouldn't it be a rather simple patch to say:

if tablespace_create() in $PGDATA:
  ERROR!

?

I mean yes a warning is good but it is after the fact, the tablespace is 
already created. We know that tablespaces in $PGDATA are a bad idea, why 
not protect the user?


JD







--
Command Prompt, Inc. - http://www.commandprompt.com/  503-667-4564
PostgreSQL Support, Training, Professional Services and Development
High Availability, Oracle Conversion, @cmdpromptinc
"If we send our children to Caesar for their education, we should
 not be surprised when they come back as Romans."


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


Re: [HACKERS] Proposal: knowing detail of config files via SQL

2015-01-30 Thread Sawada Masahiko
On Sat, Jan 31, 2015 at 12:58 AM, Mike Blackwell  wrote:
> This would default to being available to superusers only, right?  Details of
> the file system shouldn't be available to any random user.
>

This WIP patch does not behave like that, but I agree.
This view would be effective combine with ALTER SYSTEM,  and ALTER
SYSTEM command is executable to superuser only also.

Regards,

---
Sawada Masahiko


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


Re: [HACKERS] tablespaces inside $PGDATA considered harmful

2015-01-30 Thread Bruce Momjian
On Fri, Jan 30, 2015 at 11:12:43AM -0500, Robert Haas wrote:
> I think everyone who has read this mailing list for a while is
> probably already aware of this problem.  When you create a tablespace
> somewhere inside the data directory, weird things happen. If you
> pg_upgrade and then incautiously run the delete_old_cluster.sh script
> thus created, you will blow away large chunks of your data.[1]  If you

pg_upgrade doesn't create the deletion script in this case, and warns
the user:

Could not create a script to delete the old cluster's data
files because user-defined tablespaces exist in the old cluster
directory.  The old cluster's contents must be deleted manually.

> In the short term, I favor just adding a warning, so that people get
> some clue that they are doing something that might be a bad idea.  In
> the long term, we might want to do more.  Thoughts?

Yes, good idea.

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

  + Everyone has their own god. +


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


[HACKERS] tablespaces inside $PGDATA considered harmful

2015-01-30 Thread Robert Haas
I think everyone who has read this mailing list for a while is
probably already aware of this problem.  When you create a tablespace
somewhere inside the data directory, weird things happen. If you
pg_upgrade and then incautiously run the delete_old_cluster.sh script
thus created, you will blow away large chunks of your data.[1]  If you
try to use pg_basebackup, it will back up your data twice and maybe
throw some warnings.[2]  You can also induce pg_database_size() to
give wrong results --- it'll count pg_tblspace/$TABLESPACE_OID as well
as pg_tblspace/some-stupid-tablespace-name, the former being a symlink
to the latter.

Given all this, it seems like a good idea to at least give a warning
if somebody tries to create a tablespace instead the data directory.
Arguably, we should prohibit it altogether, but there are obviously
people that want to do it, and there could even be somewhat valid
reasons for that, like wanting to set per-tablespace settings
differently for different tablespaces.  Possibly we should prohibit it
anyway, or maybe there should be an option to create a tablespace
whose directory is a real directory, not a symlink.  So then:

CREATE TABLESPACE foo LOCATION '/home/rhaas/pgdata/pg_tblspc/foo';

...would fail, but if you really want a separate tablespace inside the
data directory, we could allow:

CREATE TABLESPACE foo NO LOCATION;

...which would just create a bare directory where the symlink would normally go.

In the short term, I favor just adding a warning, so that people get
some clue that they are doing something that might be a bad idea.  In
the long term, we might want to do more.  Thoughts?

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

[1] 
http://www.postgresql.org/message-id/b6f6fd62f2624c4c9916ac0175d56d880ce46...@jenmbs01.ad.intershop.net
[2] 
http://www.postgresql.org/message-id/cabuevexkhe+kcqa+flueaizp5i5qvcnnjz2j0zzqcamjfhe...@mail.gmail.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] Buildfarm broken for 9.3 and up

2015-01-30 Thread Tom Lane
Stephen Frost  writes:
> * Kevin Grittner (kgri...@ymail.com) wrote:
>> My push to branches 9.3 and up seems to have broken the buildfarm 
>> with this:

> I don't think it has anything to do with your push.  A number of members
> have built with your latest and look fine at this point, and I'm not
> seeing any issues here either.

> I do see that macque is having issues, but it's been busted for the past
> 3 days it looks like.

Yeah.  Several of the critters have been having git issues for weeks
to months.  I wonder whether we couldn't teach the buildfarm script
to recover from this automatically ...

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] Proposal: knowing detail of config files via SQL

2015-01-30 Thread Mike Blackwell
​This would default to being available to superusers only, right?  Details
of the file system shouldn't be available to any random user.​

__
*Mike Blackwell | Technical Analyst, Distribution Services/Rollout
Management | RR Donnelley*
1750 Wallace Ave | St Charles, IL 60174-3401
Office: 630.313.7818
mike.blackw...@rrd.com
http://www.rrdonnelley.com



* *

On Fri, Jan 30, 2015 at 9:50 AM, Sawada Masahiko 
wrote:

> On Sat, Jan 31, 2015 at 12:24 AM, David Fetter  wrote:
> > On Fri, Jan 30, 2015 at 09:38:10PM +0900, Sawada Masahiko wrote:
> >> On Tue, Jan 27, 2015 at 3:34 AM, Robert Haas 
> wrote:
> >> > On Thu, Jan 22, 2015 at 5:19 PM, Tom Lane  wrote:
> >> >> David Johnston  writes:
> >> >>> On Thu, Jan 22, 2015 at 3:04 PM, Tom Lane 
> wrote:
> >>  Is that a requirement, and if so why?  Because this proposal
> doesn't
> >>  guarantee any such knowledge AFAICS.
> >> >>
> >> >>> The proposal provides for SQL access to all possible sources of
> variable
> >> >>> value setting and, ideally, a means of ordering them in priority
> order, so
> >> >>> that a search for TimeZone would return two records, one for
> >> >>> postgresql.auto.conf and one for postgresql.conf - which are
> numbered 1 and
> >> >>> 2 respectively - so that in looking at that result if the
> >> >>> postgresql.auto.conf entry were to be removed the user would know
> that what
> >> >>> the value is in postgresql.conf that would become active.
> Furthermore, if
> >> >>> postgresql.conf has a setting AND there is a mapping in an
> #included file
> >> >>> that information would be accessible via SQL as well.
> >> >>
> >> >> I know what the proposal is.  What I am questioning is the use-case
> that
> >> >> justifies having us build and support all this extra mechanism.  How
> often
> >> >> does anyone need to know what the "next down" variable value would
> be?
> >> >
> >> > I believe I've run into situations where this would be useful.  I
> >> > wouldn't be willing to put in the effort to do this myself, but I
> >> > wouldn't be prepared to vote against a high-quality patch that someone
> >> > else felt motivated to write.
> >> >
> >>
> >> Attached patch adds new view pg_file_settings (WIP version).
> >> This view behaves like followings,
> >> [postgres][5432](1)=# select * from pg_file_settings ;
> >> name|  setting   |
> >>   sourcefile
> >>
> ++
> >>  max_connections| 100|
> >> /home/masahiko/pgsql/master/3data/postgresql.conf
> >>  shared_buffers | 128MB  |
> >> /home/masahiko/pgsql/master/3data/postgresql.conf
> >
> > This looks great!
> >
> > Is there a reason not to have the sourcefile as a column in
> > pg_settings?
> >
>
> pg_file_settings view also has source file column same as pg_settings.
> It might was hard to confirm that column in previous mail.
> I put example of pg_file_settings again as follows.
>
> [postgres][5432](1)=# select * from pg_file_settings where name =
> 'work_mem';
> -[ RECORD 1 ]--
> name   | work_mem
> setting| 128MB
> sourcefile | /home/masahiko/pgsql/master/3data/hoge.conf
> -[ RECORD 2 ]--
> name   | work_mem
> setting| 64MB
> sourcefile | /home/masahiko/pgsql/master/3data/postgresql.auto.conf
>
> Regards,
>
> ---
> Sawada Masahiko
>
>
> --
> Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers
>


Re: [HACKERS] Proposal: knowing detail of config files via SQL

2015-01-30 Thread Sawada Masahiko
On Sat, Jan 31, 2015 at 12:24 AM, David Fetter  wrote:
> On Fri, Jan 30, 2015 at 09:38:10PM +0900, Sawada Masahiko wrote:
>> On Tue, Jan 27, 2015 at 3:34 AM, Robert Haas  wrote:
>> > On Thu, Jan 22, 2015 at 5:19 PM, Tom Lane  wrote:
>> >> David Johnston  writes:
>> >>> On Thu, Jan 22, 2015 at 3:04 PM, Tom Lane  wrote:
>>  Is that a requirement, and if so why?  Because this proposal doesn't
>>  guarantee any such knowledge AFAICS.
>> >>
>> >>> The proposal provides for SQL access to all possible sources of variable
>> >>> value setting and, ideally, a means of ordering them in priority order, 
>> >>> so
>> >>> that a search for TimeZone would return two records, one for
>> >>> postgresql.auto.conf and one for postgresql.conf - which are numbered 1 
>> >>> and
>> >>> 2 respectively - so that in looking at that result if the
>> >>> postgresql.auto.conf entry were to be removed the user would know that 
>> >>> what
>> >>> the value is in postgresql.conf that would become active.  Furthermore, 
>> >>> if
>> >>> postgresql.conf has a setting AND there is a mapping in an #included file
>> >>> that information would be accessible via SQL as well.
>> >>
>> >> I know what the proposal is.  What I am questioning is the use-case that
>> >> justifies having us build and support all this extra mechanism.  How often
>> >> does anyone need to know what the "next down" variable value would be?
>> >
>> > I believe I've run into situations where this would be useful.  I
>> > wouldn't be willing to put in the effort to do this myself, but I
>> > wouldn't be prepared to vote against a high-quality patch that someone
>> > else felt motivated to write.
>> >
>>
>> Attached patch adds new view pg_file_settings (WIP version).
>> This view behaves like followings,
>> [postgres][5432](1)=# select * from pg_file_settings ;
>> name|  setting   |
>>   sourcefile
>> ++
>>  max_connections| 100|
>> /home/masahiko/pgsql/master/3data/postgresql.conf
>>  shared_buffers | 128MB  |
>> /home/masahiko/pgsql/master/3data/postgresql.conf
>
> This looks great!
>
> Is there a reason not to have the sourcefile as a column in
> pg_settings?
>

pg_file_settings view also has source file column same as pg_settings.
It might was hard to confirm that column in previous mail.
I put example of pg_file_settings again as follows.

[postgres][5432](1)=# select * from pg_file_settings where name = 'work_mem';
-[ RECORD 1 ]--
name   | work_mem
setting| 128MB
sourcefile | /home/masahiko/pgsql/master/3data/hoge.conf
-[ RECORD 2 ]--
name   | work_mem
setting| 64MB
sourcefile | /home/masahiko/pgsql/master/3data/postgresql.auto.conf

Regards,

---
Sawada Masahiko


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


Re: [HACKERS] Buildfarm broken for 9.3 and up

2015-01-30 Thread Stephen Frost
Kevin,

* Kevin Grittner (kgri...@ymail.com) wrote:
> My push to branches 9.3 and up seems to have broken the buildfarm 
> with this:

I don't think it has anything to do with your push.  A number of members
have built with your latest and look fine at this point, and I'm not
seeing any issues here either.

I do see that macque is having issues, but it's been busted for the past
3 days it looks like.

We have backups from prior to your commit, just in case, but I don't
think there's anything wrong.

Thanks!

Stephen


signature.asc
Description: Digital signature


Re: [HACKERS] Buildfarm broken for 9.3 and up

2015-01-30 Thread Andres Freund
On 2015-01-30 15:34:02 +, Kevin Grittner wrote:
> My push to branches 9.3 and up seems to have broken the buildfarm 
> with this:

I think this isn't anything related to your commit. Both racoon and
macaque have failed for a while. They just happened to run quickly after
your commit, giving the impression that two animals failed
consecutively.

Greetings,

Andres Freund

-- 
 Andres Freund http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services


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


Re: [HACKERS] Buildfarm broken for 9.3 and up

2015-01-30 Thread Kevin Grittner
Kevin Grittner  wrote:

> My push to branches 9.3 and up seems to have broken the buildfarm
> with this:
>
> error: object file 
> /home/pgfarm/buildroot/pgmirror.git/objects/93/d7706cbf2ce58e63ab8bbc9d16453b2c792ed4
>  is empty
> error: unable to find 93d7706cbf2ce58e63ab8bbc9d16453b2c792ed4
> fatal: SHA1 COLLISION FOUND WITH 93d7706cbf2ce58e63ab8bbc9d16453b2c792ed4 !
> fatal: index-pack failed
> Your configuration specifies to merge with the ref 'master'
> from the remote, but no such ref was fetched.
>
> I don't see any problems on my machine

Since I posted the above some animals have built successfully, so 
it seems to be only affecting some animals.

--
Kevin Grittner
EDB: 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] Buildfarm broken for 9.3 and up

2015-01-30 Thread Kevin Grittner
My push to branches 9.3 and up seems to have broken the buildfarm 
with this:

error: object file 
/home/pgfarm/buildroot/pgmirror.git/objects/93/d7706cbf2ce58e63ab8bbc9d16453b2c792ed4
 is empty
error: unable to find 93d7706cbf2ce58e63ab8bbc9d16453b2c792ed4
fatal: SHA1 COLLISION FOUND WITH 93d7706cbf2ce58e63ab8bbc9d16453b2c792ed4 !
fatal: index-pack failed
Your configuration specifies to merge with the ref 'master'
from the remote, but no such ref was fetched.

I don't see any problems on my machine, although when I attempt to 
checkout the referenced SHA1 value on branch REL9_3_STABLE, I get 
this:

kgrittn@Kevin-Desktop:~/pg/REL9_3_STABLE$ git checkout 
93d7706cbf2ce58e63ab8bbc9d16453b2c792ed4
fatal: reference is not a tree: 93d7706cbf2ce58e63ab8bbc9d16453b2c792ed4

I can't find that SHA1 in my git log.

I have no clue what to do about this.

-- 
Kevin Grittner
EDB: 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] Exposing the stats snapshot timestamp to SQL

2015-01-30 Thread David Fetter
On Fri, Jan 30, 2015 at 12:07:22AM -0500, Tom Lane wrote:
> Matt Kelly  writes:
> > On Thu, Jan 29, 2015 at 8:42 PM, Tom Lane  wrote:
> >> ... or double.  Have you checked that the code behaves properly with
> >> --disable-integer-timestamps?
> 
> > I presume you meant --disable-integer-datetimes.  I just ran that test case
> > now, all set.
> 
> Right, my own sloppiness there.
> 
> > For my own edification, was there really any risk of something so trivial
> > breaking due to that setting, or was it just for completeness? (which is a
> > perfectly valid reason)
> 
> I hadn't looked at your code at that point, and now that I have, I agree
> it's unlikely to have had a problem with this.  Still, it's a good thing
> to check, particularly considering the lack of buildfarm coverage of this
> option :-(.

Given that this has been deprecated for years, maybe it's time we took
it out in 9.5.  That the interested parties haven't bothered to put
buildfarm members in that use the option tells me that they're not all
that interested.

Cheers,
David.
-- 
David Fetter  http://fetter.org/
Phone: +1 415 235 3778  AIM: dfetter666  Yahoo!: dfetter
Skype: davidfetter  XMPP: david.fet...@gmail.com

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate


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


Re: [HACKERS] Proposal: knowing detail of config files via SQL

2015-01-30 Thread David Fetter
On Fri, Jan 30, 2015 at 09:38:10PM +0900, Sawada Masahiko wrote:
> On Tue, Jan 27, 2015 at 3:34 AM, Robert Haas  wrote:
> > On Thu, Jan 22, 2015 at 5:19 PM, Tom Lane  wrote:
> >> David Johnston  writes:
> >>> On Thu, Jan 22, 2015 at 3:04 PM, Tom Lane  wrote:
>  Is that a requirement, and if so why?  Because this proposal doesn't
>  guarantee any such knowledge AFAICS.
> >>
> >>> The proposal provides for SQL access to all possible sources of variable
> >>> value setting and, ideally, a means of ordering them in priority order, so
> >>> that a search for TimeZone would return two records, one for
> >>> postgresql.auto.conf and one for postgresql.conf - which are numbered 1 
> >>> and
> >>> 2 respectively - so that in looking at that result if the
> >>> postgresql.auto.conf entry were to be removed the user would know that 
> >>> what
> >>> the value is in postgresql.conf that would become active.  Furthermore, if
> >>> postgresql.conf has a setting AND there is a mapping in an #included file
> >>> that information would be accessible via SQL as well.
> >>
> >> I know what the proposal is.  What I am questioning is the use-case that
> >> justifies having us build and support all this extra mechanism.  How often
> >> does anyone need to know what the "next down" variable value would be?
> >
> > I believe I've run into situations where this would be useful.  I
> > wouldn't be willing to put in the effort to do this myself, but I
> > wouldn't be prepared to vote against a high-quality patch that someone
> > else felt motivated to write.
> >
> 
> Attached patch adds new view pg_file_settings (WIP version).
> This view behaves like followings,
> [postgres][5432](1)=# select * from pg_file_settings ;
> name|  setting   |
>   sourcefile
> ++
>  max_connections| 100|
> /home/masahiko/pgsql/master/3data/postgresql.conf
>  shared_buffers | 128MB  |
> /home/masahiko/pgsql/master/3data/postgresql.conf

This looks great!

Is there a reason not to have the sourcefile as a column in
pg_settings?

Cheers,
David.
-- 
David Fetter  http://fetter.org/
Phone: +1 415 235 3778  AIM: dfetter666  Yahoo!: dfetter
Skype: davidfetter  XMPP: david.fet...@gmail.com

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate


-- 
Sent 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_dump with both --serializable-deferrable and -j

2015-01-30 Thread Kevin Grittner
Tom Lane  wrote:
> Kevin Grittner  writes:

>> I propose to apply the attached to master and back-patch to 9.3

>> Objections?
>
> Only the nit-picky one that I quite dislike putting a comment 
> block inside an if-condition like that.

Comment moved above the if-condition, and pushed.

Thanks for the report, Alexander!

--
Kevin Grittner
EDB: 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: Overhauling our interrupt handling (was Escaping from blocked send() reprised.)

2015-01-30 Thread Andres Freund
On 2015-01-30 09:29:59 -0500, Robert Haas wrote:
> On Wed, Jan 14, 2015 at 9:03 PM, Andres Freund  wrote:
> > 0002: Use a nonblocking socket for FE/BE communication and block using
> >   latches.
> >
> >   Has previously been reviewed by Heikki. I think Noah also had a
> >   look, although I'm not sure how close that was.
> >
> >   I think this can be committed soon.
> 
> Doesn't this significantly increase the number of system calls?  I
> worry there could be a performance issue here.

I've posted benchmarks upthread and I only could start to measure any
overhead in pretty absurd cases (i.e. several hundred connections on a
few core machine, all doing SELECT 1;statements). As we try
the read before the poll/select it's not that bad - there's no
superflous work done if we're actually busy.

> > 0003: Introduce and use infrastructure for interrupt processing during 
> > client reads.
> >
> >   From here on ImmediateInterruptOK isn't set during client
> >   communication. Normal interrupts and sinval/async interrupts are
> >   processed outside of signal handlers. Especially the sinval/async
> >   greatly simplify the respective code.
> 
> ProcessNotifyInterrupt() seems like it could lead to a failure to
> respond to other interrupts if there is a sufficiently vigorous stream
> of notify interrupts.

That's nothing new though. It just used to be executed inside interrupts
directly, with looping. And we looped when enabling the notify
interrupts. Since I can't recall a report of this being problematic I'm
not that inclined to change even more than the patch already does. Given
that queuing notifies requires a lock I have a hard time seing this ever
fast enough to cause that problem.

Greetings,

Andres Freund

-- 
 Andres Freund http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services


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


Re: [HACKERS] INSERT ... ON CONFLICT {UPDATE | IGNORE} 2.0

2015-01-30 Thread Geoff Winkless
On Thu, Jan 29, 2015 at 11:38 PM, Peter Geoghegan  wrote:
> Simply removing IGNORE support until such time as we straighten
> that all out (9.6?) seems like the simplest solution. No need to block
> the progress of "UPSERT", since exclusion constraint support was
> only ever going to be useful for the less compelling IGNORE variant.
> What do other people think? Do you agree with my view that we should
> shelve IGNORE support for now, Heikki?

I appreciate the work you're doing and (as a user rather than a
pg-hacker) don't want to butt in but if it would be possible to allow
support for IGNORE for unique but not exclusion constraints that would
be really helpful for my own use cases, where being able to insert
from a dataset into a table containing unique constraints without
having to first check the dataset for uniqueness (within both itself
and the target table) is very useful.

It's possible that I've misunderstood anyway and that you mean purely
that exclusion constraint IGNORE should be shelved until 9.6, in which
case I apologise.

Of course if there's no way to allow unique constraint IGNORE without
exclusion constraints then just ignore me; I (along I'm sure with all
the others who are following this conversation from afar) will be
incredibly grateful for the work you've done either way.

I suppose there's no reason why we couldn't use a no-op ON CONFLICT
UPDATE anyway, but that does seem rather messy and (I imagine) would
involve rather more work (unless the optimizer were able to optimize
away the "update"? I don't know enough to be able to say if it would).

Thanks

Geoff


-- 
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: Overhauling our interrupt handling (was Escaping from blocked send() reprised.)

2015-01-30 Thread Robert Haas
On Wed, Jan 14, 2015 at 9:03 PM, Andres Freund  wrote:
> 0002: Use a nonblocking socket for FE/BE communication and block using
>   latches.
>
>   Has previously been reviewed by Heikki. I think Noah also had a
>   look, although I'm not sure how close that was.
>
>   I think this can be committed soon.

Doesn't this significantly increase the number of system calls?  I
worry there could be a performance issue here.

> 0003: Introduce and use infrastructure for interrupt processing during client 
> reads.
>
>   From here on ImmediateInterruptOK isn't set during client
>   communication. Normal interrupts and sinval/async interrupts are
>   processed outside of signal handlers. Especially the sinval/async
>   greatly simplify the respective code.

ProcessNotifyInterrupt() seems like it could lead to a failure to
respond to other interrupts if there is a sufficiently vigorous stream
of notify interrupts.  I think there ought to be one loop that goes
through and tries to handle each kind of interrupt in turn and then
loops until no interrupts remain.

-- 
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] Proposal: knowing detail of config files via SQL

2015-01-30 Thread Sawada Masahiko
On Tue, Jan 27, 2015 at 3:34 AM, Robert Haas  wrote:
> On Thu, Jan 22, 2015 at 5:19 PM, Tom Lane  wrote:
>> David Johnston  writes:
>>> On Thu, Jan 22, 2015 at 3:04 PM, Tom Lane  wrote:
 Is that a requirement, and if so why?  Because this proposal doesn't
 guarantee any such knowledge AFAICS.
>>
>>> The proposal provides for SQL access to all possible sources of variable
>>> value setting and, ideally, a means of ordering them in priority order, so
>>> that a search for TimeZone would return two records, one for
>>> postgresql.auto.conf and one for postgresql.conf - which are numbered 1 and
>>> 2 respectively - so that in looking at that result if the
>>> postgresql.auto.conf entry were to be removed the user would know that what
>>> the value is in postgresql.conf that would become active.  Furthermore, if
>>> postgresql.conf has a setting AND there is a mapping in an #included file
>>> that information would be accessible via SQL as well.
>>
>> I know what the proposal is.  What I am questioning is the use-case that
>> justifies having us build and support all this extra mechanism.  How often
>> does anyone need to know what the "next down" variable value would be?
>
> I believe I've run into situations where this would be useful.  I
> wouldn't be willing to put in the effort to do this myself, but I
> wouldn't be prepared to vote against a high-quality patch that someone
> else felt motivated to write.
>

Attached patch adds new view pg_file_settings (WIP version).
This view behaves like followings,
[postgres][5432](1)=# select * from pg_file_settings ;
name|  setting   |
  sourcefile
++
 max_connections| 100|
/home/masahiko/pgsql/master/3data/postgresql.conf
 shared_buffers | 128MB  |
/home/masahiko/pgsql/master/3data/postgresql.conf
 dynamic_shared_memory_type | posix  |
/home/masahiko/pgsql/master/3data/postgresql.conf
 log_timezone   | Japan  |
/home/masahiko/pgsql/master/3data/postgresql.conf
 datestyle  | iso, mdy   |
/home/masahiko/pgsql/master/3data/postgresql.conf
 timezone   | Japan  |
/home/masahiko/pgsql/master/3data/postgresql.conf
 lc_messages| C  |
/home/masahiko/pgsql/master/3data/postgresql.conf
 lc_monetary| C  |
/home/masahiko/pgsql/master/3data/postgresql.conf
 lc_numeric | C  |
/home/masahiko/pgsql/master/3data/postgresql.conf
 lc_time| C  |
/home/masahiko/pgsql/master/3data/postgresql.conf
 default_text_search_config | pg_catalog.english |
/home/masahiko/pgsql/master/3data/postgresql.conf
 work_mem   | 128MB  |
/home/masahiko/pgsql/master/3data/hoge.conf
 checkpoint_segments| 300|
/home/masahiko/pgsql/master/3data/hoge.conf
 enable_indexscan   | off|
/home/masahiko/pgsql/master/3data/postgresql.auto.conf
 work_mem   | 64MB   |
/home/masahiko/pgsql/master/3data/postgresql.auto.conf

[postgres][5432](1)=# select name, setting from pg_settings where name
= 'work_mem';
   name   | setting
--+-
 work_mem | 65536
(1 row)

Above query result shows that both hoge.conf and postgresql.auto.conf
have same config parameter work_mem, and work_mem in auto.conf is
effecitve.
We can confirm these parameter to decide if the postgresql.auto.conf
is useful or not.

Regards,

---
Sawada Masahiko


000_pg_file_settings_view_v1.patch
Description: Binary data

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


[HACKERS] NULL checks of deferenced pointers in picksplit method of intarray

2015-01-30 Thread Michael Paquier
Hi all,

Coverity is pointing out that _int_split.c has unnecessary checks for
deferenced pointers in 5 places.

-   if (inter_d != (ArrayType *) NULL)
-   pfree(inter_d);
+   pfree(inter_d);
In this case inter_d is generated by inner_int_inter, a routine that
always generates an ArrayType with at least new_intArrayType.

In two places there is as well this pattern:
-   if (datum_l)
-   pfree(datum_l);
-   if (union_dr)
-   pfree(union_dr);
+   pfree(datum_l);
+   pfree(union_dr);
And that one:
-   if (datum_r)
-   pfree(datum_r);
-   if (union_dl)
-   pfree(union_dl);
+   pfree(datum_r);
+   pfree(union_dl);
union_dr and union_dl are generated by inner_int_union which never
returns NULL. Similarly, datum_r and datum_l are created with
copy_intArrayType the first time, which never returns NULL, and their
values are changed at each loop step. Also, as far as I understood
from this code, no elements manipulated are NULL, perhaps this is
worth an assertion?

Attached is a patch to adjust those things.
Regards,
-- 
Michael
diff --git a/contrib/intarray/_int_gist.c b/contrib/intarray/_int_gist.c
index 53abcc4..876a7b9 100644
--- a/contrib/intarray/_int_gist.c
+++ b/contrib/intarray/_int_gist.c
@@ -416,9 +416,7 @@ g_int_picksplit(PG_FUNCTION_ARGS)
 			size_waste = size_union - size_inter;
 
 			pfree(union_d);
-
-			if (inter_d != (ArrayType *) NULL)
-pfree(inter_d);
+			pfree(inter_d);
 
 			/*
 			 * are these a more promising split that what we've already seen?
@@ -517,10 +515,8 @@ g_int_picksplit(PG_FUNCTION_ARGS)
 		/* pick which page to add it to */
 		if (size_alpha - size_l < size_beta - size_r + WISH_F(v->spl_nleft, v->spl_nright, 0.01))
 		{
-			if (datum_l)
-pfree(datum_l);
-			if (union_dr)
-pfree(union_dr);
+			pfree(datum_l);
+			pfree(union_dr);
 			datum_l = union_dl;
 			size_l = size_alpha;
 			*left++ = i;
@@ -528,10 +524,8 @@ g_int_picksplit(PG_FUNCTION_ARGS)
 		}
 		else
 		{
-			if (datum_r)
-pfree(datum_r);
-			if (union_dl)
-pfree(union_dl);
+			pfree(datum_r);
+			pfree(union_dl);
 			datum_r = union_dr;
 			size_r = size_beta;
 			*right++ = i;

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


Re: [HACKERS] Performance improvement for joins where outer side is unique

2015-01-30 Thread David Rowley
On 1 January 2015 at 02:47, David Rowley  wrote:

> Hi,
>
> I've been hacking a bit at the join code again... This time I've been
> putting some effort into  optimising the case where the inner side of the
> join is known to be unique.
> For example, given the tables:
>
> create table t1 (id int primary key);
> create table t2 (id int primary key);
>
> And query such as:
>
> select * from t1 left outer join t2 on t1.id=t2.id;
>
> It is possible to deduce at planning time that "for each row in the outer
> relation, only 0 or 1 rows can exist in the inner relation", (inner being
> t2)
>

I've been hacking at this unique join idea again and I've now got it
working for all join types -- Patch attached.

Here's how the performance is looking:

postgres=# create table t1 (id int primary key);
CREATE TABLE
postgres=# create table t2 (id int primary key);
CREATE TABLE
postgres=# insert into t1 select x.x from generate_series(1,100) x(x);
INSERT 0 100
postgres=# insert into t2 select x.x from generate_series(1,100) x(x);
INSERT 0 100
postgres=# vacuum analyze;
VACUUM
postgres=# \q

Query: select count(t1.id) from t1 inner join t2 on t1.id=t2.id;

With Patch on master as of 32bf6ee

D:\Postgres\install\bin>pgbench -f d:\unijoin3.sql -T 60 -n postgres
transaction type: Custom query
scaling factor: 1
query mode: simple
number of clients: 1
number of threads: 1
duration: 60 s
number of transactions actually processed: 78
latency average: 769.231 ms
tps = 1.288260 (including connections establishing)
tps = 1.288635 (excluding connections establishing)

Master as of 32bf6ee

D:\Postgres\install\bin>pgbench -f d:\unijoin3.sql -T 60 -n postgres
transaction type: Custom query
scaling factor: 1
query mode: simple
number of clients: 1
number of threads: 1
duration: 60 s
number of transactions actually processed: 70
latency average: 857.143 ms
tps = 1.158905 (including connections establishing)
tps = 1.159264 (excluding connections establishing)

That's a 10% performance increase.

I still need to perform more thorough benchmarking with different data
types.

One weird thing that I noticed before is that in an earlier revision of the
patch in the executor's join Initialise node code, I had set the
unique_inner to true for semi joins and replaced the SEMI_JOIN check for a
unique_join check in the execute node for each join method. With this the
performance results barely changed from standard... I've yet to find out
why.

The patch also has added a property to the EXPLAIN (VERBOSE) output which
states if the join was found to be unique or not.

The patch also still requires a final pass of comment fix-ups. I've just
plain run out of time for now.

I'll pick this up in 2 weeks time.

Regards

David Rowley


unijoin_2015-01-31_9af9cfa.patch
Description: Binary data

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


[HACKERS] Odd behavior of updatable security barrier views on foreign tables

2015-01-30 Thread Etsuro Fujita
Hi,

I noticed that when updating security barrier views on foreign tables,
we fail to give FOR UPDATE to selection queries issued at ForeignScan.
Here is an example.

postgres=# create foreign table base_ftbl (person text, visibility text)
server loopback options (table_name 'base_tbl');
CREATE FOREIGN TABLE
postgres=# create view rw_view as select person from base_ftbl where
visibility = 'public';
CREATE VIEW
postgres=# explain verbose delete from rw_view;
  QUERY PLAN
---
 Delete on public.base_ftbl  (cost=100.00..144.40 rows=14 width=6)
   Remote SQL: DELETE FROM public.base_tbl WHERE ctid = $1
   ->  Foreign Scan on public.base_ftbl  (cost=100.00..144.40 rows=14
width=6)
 Output: base_ftbl.ctid
 Remote SQL: SELECT ctid FROM public.base_tbl WHERE ((visibility
= 'public'::text)) FOR UPDATE
(5 rows)

postgres=# alter view rw_view set (security_barrier = true);
ALTER VIEW
postgres=# explain verbose delete from rw_view;
QUERY PLAN
--
 Delete on public.base_ftbl base_ftbl_1  (cost=100.00..144.54 rows=14
width=6)
   Remote SQL: DELETE FROM public.base_tbl WHERE ctid = $1
   ->  Subquery Scan on base_ftbl  (cost=100.00..144.54 rows=14 width=6)
 Output: base_ftbl.ctid
 ->  Foreign Scan on public.base_ftbl base_ftbl_2
(cost=100.00..144.40 rows=14 width=6)
   Output: base_ftbl_2.ctid
   Remote SQL: SELECT ctid FROM public.base_tbl WHERE
((visibility = 'public'::text))
(7 rows)

Correct me if I am wrong.

Best regards,
Etsuro Fujita


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


Re: [HACKERS] Redesigning checkpoint_segments

2015-01-30 Thread Heikki Linnakangas

On 01/30/2015 04:48 AM, Venkata Balaji N wrote:

I performed series of tests for this patch and would like to share the
results. My comments are in-line.


Thanks for the testing!


*Test 1 :*

In this test, i see removed+recycled segments = 3 (except for the first 3
checkpoint cycles) and has been steady through out until the INSERT
operation completed.

Actual calculation of CheckPointSegments = 3.2 ( is getting rounded up to 3
)

pg_xlog size is 128M and has increased to 160M max during the INSERT
operation.

shared_buffers = 128M
checkpoint_wal_size = 128M
min_recycle_wal_size = 80M
checkpoint_timeout = 5min


Hmm, did I understand correctly that pg_xlog peaked at 160MB, but most 
of the stayed at 128 MB? That sounds like it's working as designed; 
checkpoint_wal_size is not a hard limit after all.



b) Are the two GUCs, checkpoint_wal_size, and min_recycle_wal_size,
intuitive to set?


During my tests, I did not observe the significance of min_recycle_wal_size
parameter yet. Ofcourse, i had sufficient disk space for pg_xlog.

I would like to understand more about "min_recycle_wal_size" parameter. In
theory, i only understand from the note in the patch that if the disk space
usage falls below certain threshold, min_recycle_wal_size number of WALs
will be removed to accommodate future pg_xlog segments. I will try to test
this out. Please let me know if there is any specific test to understand
min_recycle_wal_size behaviour.


min_recycle_wal_size comes into play when you have only light load, so 
that checkpoints are triggered by checkpoint_timeout rather than 
checkpoint_wal_size. In that scenario, the WAL usage will shrink down to 
min_recycle_wal_size, but not below that. Did that explanation help? Can 
you suggest changes to the docs to make it more clear?


- Heikki



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


Re: [HACKERS] Possible typo in create_policy.sgml

2015-01-30 Thread Dean Rasheed
On 30 January 2015 at 03:40, Stephen Frost  wrote:
> * Robert Haas (robertmh...@gmail.com) wrote:
>> On Thu, Jan 29, 2015 at 9:04 PM, Stephen Frost  wrote:
>> > A policy grants the ability to SELECT, INSERT, UPDATE, or DELETE rows
>> > which match the relevant policy expression. Existing table rows are
>> > checked against the expression specified via USING, while new rows
>> > that would be created via INSERT or UPDATE are checked against the
>> > expression specified via WITH CHECK.  When a USING expression returns
>> > false for a given row, that row is not visible to the user.  When a WITH
>> > CHECK expression returns false for a row which is to be added, an error
>> > occurs.
>>
>> Yeah, that's not bad.  I think it's an improvement, in fact.
>

Yes I like that too. My main concern was that we should be describing
policies in terms of permitting access to the table, not limiting
access, because of the default-deny policy, and this new text clears
that up.

One additional quibble -- it's misleading to say "expression returns
false" here (and later in the check_expression parameter description)
because if the expression returns null, that's also a failure. So it
ought to be "false or null", but perhaps it could just be described in
terms of rows matching the expression, with a separate note to say
that a row only matches a policy expression if that expression returns
true, not false or null.

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] jsonb, unicode escapes and escaped backslashes

2015-01-30 Thread Peter Geoghegan
On Thu, Jan 29, 2015 at 11:28 PM, Tom Lane  wrote:
>> The point of JSONB is that we take a position on certain aspects like
>> this. We're bridging a pointedly loosey goosey interchange format,
>> JSON, with native PostgreSQL types. For example, we take a firm
>> position on encoding. The JSON type is a bit more permissive, to about
>> the extent that that's possible. The whole point is that we're
>> interpreting JSON data in a way that's consistent with *Postgres*
>> conventions. You'd have to interpret the data according to *some*
>> convention in order to do something non-trivial with it in any case,
>> and users usually want that.
>
> I quite agree with you, actually, in terms of that perspective.

Sure, but I wasn't sure that that was evident to others.

To emphasize: I think it's appropriate that the JSON spec takes
somewhat of a back seat approach to things like encoding and the
precision of numbers. I also think it's appropriate that JSONB does
not, up to and including where JSONB forbids things that the JSON spec
supposes could be useful. We haven't failed users by (say) not
accepting NULs, even though the spec suggests that that might be
useful - we have provided them with a reasonable, concrete
interpretation of that JSON data, with lots of useful operators, that
they may take or leave. It really isn't historical that we have both a
JSON and JSONB type. For other examples of this, see every "document
database" in existence.

Depart from this perspective, as an interchange standard author, and
you end up with something like XML, which while easy to reason about
isn't all that useful, or BSON, the binary interchange format, which
is an oxymoron.

> But my point remains: "\u" is not invalid JSON syntax, and neither is
> "\u1234".  If we choose to throw an error because we can't interpret or
> process that according to our conventions, fine, but we should call it
> something other than "invalid syntax".
>
> ERRCODE_UNTRANSLATABLE_CHARACTER or ERRCODE_CHARACTER_NOT_IN_REPERTOIRE
> seem more apropos from here.

I see. I'd go with ERRCODE_UNTRANSLATABLE_CHARACTER, then.
-- 
Peter Geoghegan


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