Re: [HACKERS] Add min and max execute statement time in pg_stat_statement

[KONDO Mitsumasa]

(2014/01/29 15:51), Tom Lane wrote:
> KONDO Mitsumasa  writes:
>> By the way, latest pg_stat_statement might affect performance in Windows 
>> system.
>> Because it uses fflush() system call every creating new entry in
>> pg_stat_statements, and it calls many fread() to warm file cache.
> This statement doesn't seem to have much to do with the patch as
> committed.  There are no fflush calls, and no notion of warming the
> file cache either.
Oh, all right.

> We do assume that the OS is smart enough to keep
> a frequently-read file in cache ... is Windows too stupid for that?
I don't know about it. But I think Windows cache feature is stupid.
It seems to always write/read data to/from disk, nevertheless having large 
memory...
I'd like to know test result on Windows, if we can...

Regards,
--
Mitsumasa KONDO
NTT Open Source Software Center




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

Re: [HACKERS] Add min and max execute statement time in pg_stat_statement

[Peter Geoghegan]

On Tue, Jan 28, 2014 at 10:51 PM, Tom Lane  wrote:
> KONDO Mitsumasa  writes:
>> By the way, latest pg_stat_statement might affect performance in Windows 
>> system.
>> Because it uses fflush() system call every creating new entry in
>> pg_stat_statements, and it calls many fread() to warm file cache.
>
> This statement doesn't seem to have much to do with the patch as
> committed.

You could make a strong case for the patch improving performance in
practice for many users, by allowing us to increase the
pg_stat_statements.max default value to 5,000, 5 times the previous
value. The real expense of creating a new entry is the exclusive
locking of the hash table, which blocks *everything* in
pg_stat_statements. That isn't expanded at all, since queries are only
written with a shared lock, which only blocks the creation of new
entries which was already relatively expensive. In particular, it does
not block the maintenance of costs for all already observed entries in
the hash table. It's obvious that simply reducing the pressure on the
cache will improve matters a lot, which for many users the external
texts patch does.

Since Mitsumasa has compared that patch and at least one of his
proposed pg_stat_statements patches on several occasions, I will
re-iterate the difference: any increased overhead from the external
texts patch is paid only when a query is first entered into the hash
table. Then, there is obviously and self-evidently no additional
overhead from contention for any future execution of the same query,
no matter what, indefinitely (the exclusive locking section of
creating a new entry does not do I/O, except in fantastically unlikely
circumstances). So for many of the busy production systems I work
with, that's the difference between a cost paid perhaps tens of
thousands of times a second, and a cost paid every few days or weeks.
Even if he is right about a write() taking an unreasonably large
amount of time on Windows, the consequences felt as pg_stat_statements
LWLock contention would be very limited.

I am not opposed in principle to adding new things to the counters
struct in pg_stat_statements. I just think that the fact that the
overhead of installing the module on a busy production system is
currently so low is of *major* value, and therefore any person that
proposes to expand that struct should be required to very conclusively
demonstrate that there is no appreciably increase in overhead. Having
a standard deviation column would be nice, but it's still not that
important. Maybe when we have portable atomic addition we can afford
to be much more inclusive of that kind of thing.


-- 
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] drop duplicate buffers in OS

[KONDO Mitsumasa]

Hi,

Attached is latest patch.
I change little bit at PinBuffer() in bufmgr.c. It will evict target clean file 
cache in OS more exactly.


- if (!(buf->flags & BM_FADVED) && !(buf->flags & BM_JUST_DIRTIED))
+ if (!(buf->flags & BM_DIRTY) && !(buf->flags & BM_FADVED) && !(buf->flags & 
BM_JUST_DIRTIED))


(2014/01/29 8:20), Jeff Janes wrote:

On Wed, Jan 15, 2014 at 10:34 AM, Robert Haas mailto:robertmh...@gmail.com>> wrote:

On Wed, Jan 15, 2014 at 1:53 AM, KONDO Mitsumasa
mailto:kondo.mitsum...@lab.ntt.co.jp>> 
wrote:
 > I create patch that can drop duplicate buffers in OS using usage_count
 > alogorithm. I have developed this patch since last summer. This feature
seems to
 > be discussed in hot topic, so I submit it more faster than my schedule.
 >
 > When usage_count is high in shared_buffers, they are hard to drop from
 > shared_buffers. However, these buffers wasn't required in file cache. 
Because
 > they aren't accessed by postgres(postgres access to shared_buffers).
 > So I create algorithm that dropping file cache which is high usage_count 
in
 > shared_buffers and is clean state in OS. If file cache are clean state in
OS, and
 > executing posix_fadvice DONTNEED, it can only free in file cache without
writing
 > physical disk. This algorithm will solve double-buffered situation 
problem and
 > can use memory more efficiently.
 >
 > I am testing DBT-2 benchmark now...


Have you had any luck with it?  I have reservations about this approach.  Among
other reasons, if the buffer is truly nailed in shared_buffers for the long 
term,
the kernel won't see any activity on it and will be able to evict it fairly
efficiently on its own.
My patch aims not to evict other useful file cache in OS. If we doesn't evict 
useful file caches in shered_buffers, they will be evicted with other useful file 
cache in OS. But if we evict them as soon as possible, it will be difficult to 
evict other useful file cache all the more.



So I'm reluctant to do a detailed review if the author cannot demonstrate a
performance improvement.  I'm going to mark it waiting-on-author for that 
reason.

Will you review my patch? Thank you so much! However, My patch performance is be
little bit better. It might be error range. Optimize kernel readahead patch is 
grate.
Too readahead in OS is too bad, and to be full of not useful file cache in OS.
Here is the test result. Plain result is tested before(readahead patch test).

* Test server
  Server: HP Proliant DL360 G7
  CPU:Xeon E5640 2.66GHz (1P/4C)
  Memory: 18GB(PC3-10600R-9)
  Disk:   146GB(15k)*4 RAID1+0
  RAID controller: P410i/256MB
  OS: RHEL 6.4(x86_64)
  FS: Ext4

* DBT-2 result(WH400, SESSION=100, ideal_score=5160)
Method | score | average | 90%tile | Maximum

plain  | 3589  | 9.751   | 33.680  | 87.8036
patched| 3799  | 9.914   | 22.451  | 119.4259

* Main Settings
shared_buffers= 2458MB
drop_duplicate_buffers = 5 // patched only

I tested benchmark with drop_duplicate_buffers = 3 and 4 settings. But I didn't 
get good result. So I will test with more larger shared_buffers and these settings.


[detail settings]
http://pgstatsinfo.projects.pgfoundry.org/readahead_dbt2/normal_20140109/HTML/dbserver/param.out


* Detail results (uploading now. please wait for a hour...)
[plain]
http://pgstatsinfo.projects.pgfoundry.org/readahead_dbt2/normal_20140109/HTML/index_thput.html 


[patched]
http://pgstatsinfo.projects.pgfoundry.org/drop_os_cache/drop_dupulicate_cache20140129/HTML/index_thput.html

We can see faster response time at OS witeback situation(maybe) and executing 
CHECKPOINT. Because when these are happened, read transactions hit file cache 
more in my patch. So responce times are better than plain.


Regards,
--
Mitsumasa KONDO
NTT Open Source Software Center
*** a/doc/src/sgml/config.sgml
--- b/doc/src/sgml/config.sgml
***
*** 1107,1112  include 'filename'
--- 1107,1130 

   
  
+  
+   drop-duplicate-buffers (integer)
+   
+drop-duplicate-buffers configuration parameter
+   
+   
+
+ Sets target min usage count in shared_buffers, which is droped in file cache.
+ When you use this parameter, you set large shared_buffers that is about 50% 
+ or higher, and set parameter with 4 or 5. It need to set carefuly. If you 
+ use this parameter with small shared_buffers, transaction performance will be
+ decliend. This parameter is needed to support posix_fadvise() system call.
+ If your system doesn't support posix_fadvise(), it doesn't work at all.
+
+ 
+   
+  
+ 
   
temp_buffers (integer)

*** a/src/backend/storage/buffer/bufmgr.c
--- b/src/backend/storage/buffer/bufmgr.c
***
*** 69,74 
--- 69,75 
  /* GUC variables */
  bool		zero_damaged_pages = false;
  int			bgw

Re: [HACKERS] WIP patch (v2) for updatable security barrier views

[Kouhei Kaigai]

> Kouhei Kaigai  writes:
> > Let me ask an elemental question. What is the reason why inheritance
> > expansion logic should be located on the planner stage, not on the
> > tail of rewriter?
> 
> I think it's mostly historical.  You would however have to think of a way
> to preserve the inheritance relationships in the parsetree representation.
> In the current code, expand_inherited_tables() adds AppendRelInfo nodes
> to the planner's data structures as it does the expansion; but I don't think
> AppendRelInfo is a suitable structure for the rewriter to work with, and
> in any case there's no place to put it in the Query representation.
> 
> Actually though, isn't this issue mostly about inheritance of a query
> *target* table?  Moving that expansion to the rewriter is a totally
> different and perhaps more tractable change.  It's certainly horribly ugly
> as it is; hard to see how doing it at the rewriter could be worse.
>
It's just an idea, so might not be a deep consideration.

Isn't ii available to describe a parse tree as if some UPDATE/DELETE statements
are combined with UNION ALL? Of course, even if it is only internal form.

  UPDATE parent SET x = 2*x, y = y || '_update' WHERE x % 10 = 5
UNION ALL
  UPDATE child_1 SET x = 2*x, y = y || '_update' WHERE x % 10 = 5
 :

Right now, only SELECT statement is allowed being placed under set-operations.
If rewriter can expand inherited relations into multiple individual selects
with UNION ALL, it may be a reasonable internal representation.

In similar way,  both of UPDATE/DELETE takes a scan on relation once, then
it modifies the target relation. Probably, here is no significant different
on the earlier half; that performs as if multiple SELECTs with UNION ALL are
running, except for it fetches ctid system column as junk attribute and 
acquires row-level locks.

On the other hand, it may need to adjust planner code to construct
ModifyTable node running on multiple relations. Existing code pulls
resultRelations from AppendRelInfo, doesn't it? It needs to take the list
of result relations using recursion of set operations, if not flatten.
Once we can construct ModifyTable with multiple relations on behalf of
multiple relation's scan, here is no difference from what we're doing now.

How about your opinion?

Thanks,
--
NEC OSS Promotion Center / PG-Strom Project
KaiGai Kohei 


> -Original Message-
> From: Tom Lane [mailto:t...@sss.pgh.pa.us]
> Sent: Wednesday, January 29, 2014 3:43 PM
> To: Kaigai, Kouhei(海外, 浩平)
> Cc: Craig Ringer; Simon Riggs; Dean Rasheed; PostgreSQL Hackers; Kohei
> KaiGai; Robert Haas
> Subject: Re: [HACKERS] WIP patch (v2) for updatable security barrier views
> 
> Kouhei Kaigai  writes:
> > Let me ask an elemental question. What is the reason why inheritance
> > expansion logic should be located on the planner stage, not on the
> > tail of rewriter?
> 
> I think it's mostly historical.  You would however have to think of a way
> to preserve the inheritance relationships in the parsetree representation.
> In the current code, expand_inherited_tables() adds AppendRelInfo nodes
> to the planner's data structures as it does the expansion; but I don't think
> AppendRelInfo is a suitable structure for the rewriter to work with, and
> in any case there's no place to put it in the Query representation.
> 
> Actually though, isn't this issue mostly about inheritance of a query
> *target* table?  Moving that expansion to the rewriter is a totally
> different and perhaps more tractable change.  It's certainly horribly ugly
> as it is; hard to see how doing it at the rewriter could be worse.
> 
>   regards, tom lane


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

[HACKERS] Re: Patch: regexp_matches variant returning an array of matching positions

[David Johnston]

Erik Rijkers wrote
> On Wed, January 29, 2014 05:16, David Johnston wrote:
>>
>> How does this resolve in the patch?
>>
>> SELECT regexp_matches('abcabc','((a)(b)(c))','g');
>>
> 
> With the patch:
> 
> testdb=# SELECT regexp_matches('abcabc','((a)(b)(c))','g'),
> regexp_matches_positions('abcabc','((a)(b)(c))');
>  regexp_matches | regexp_matches_positions
> +--
>  {abc,a,b,c}| {1,1,2,3}
>  {abc,a,b,c}| {1,1,2,3}
> (2 rows)

The {1,1,2,3} in the second row is an artifact/copy from
set-value-function-in-select-list repetition and has nothing to do with the
second match.


> testdb=# SELECT regexp_matches('abcabc','((a)(b)(c))','g'),
> regexp_matches_positions('abcabc','((a)(b)(c))', 'g');
>  regexp_matches | regexp_matches_positions
> +--
>  {abc,a,b,c}| {1,1,2,3}
>  {abc,a,b,c}| {4,4,5,6}
> (2 rows)

As expected.

David J.





--
View this message in context: 
http://postgresql.1045698.n5.nabble.com/Patch-regexp-matches-variant-returning-an-array-of-matching-positions-tp5789321p5789434.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] Re: Patch: regexp_matches variant returning an array of matching positions

[Erik Rijkers]

On Wed, January 29, 2014 05:16, David Johnston wrote:
>
> How does this resolve in the patch?
>
> SELECT regexp_matches('abcabc','((a)(b)(c))','g');
>

With the patch:

testdb=# SELECT regexp_matches('abcabc','((a)(b)(c))','g'), 
regexp_matches_positions('abcabc','((a)(b)(c))');
 regexp_matches | regexp_matches_positions
+--
 {abc,a,b,c}| {1,1,2,3}
 {abc,a,b,c}| {1,1,2,3}
(2 rows)

testdb=# SELECT regexp_matches('abcabc','((a)(b)(c))','g'), 
regexp_matches_positions('abcabc','((a)(b)(c))', 'g');
 regexp_matches | regexp_matches_positions
+--
 {abc,a,b,c}| {1,1,2,3}
 {abc,a,b,c}| {4,4,5,6}
(2 rows)



( in HEAD:

testdb=# SELECT regexp_matches('abcabc','((a)(b)(c))','g');
 regexp_matches

 {abc,a,b,c}
 {abc,a,b,c}
(2 rows)
)






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

Re: [HACKERS] Add min and max execute statement time in pg_stat_statement

[Tom Lane]

KONDO Mitsumasa  writes:
> By the way, latest pg_stat_statement might affect performance in Windows 
> system. 
> Because it uses fflush() system call every creating new entry in 
> pg_stat_statements, and it calls many fread() to warm file cache.

This statement doesn't seem to have much to do with the patch as
committed.  There are no fflush calls, and no notion of warming the
file cache either.  We do assume that the OS is smart enough to keep
a frequently-read file in cache ... is Windows too stupid for that?

regards, tom lane


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

Re: [HACKERS] WIP patch (v2) for updatable security barrier views

[Tom Lane]

Kouhei Kaigai  writes:
> Let me ask an elemental question. What is the reason why inheritance
> expansion logic should be located on the planner stage, not on the tail
> of rewriter?

I think it's mostly historical.  You would however have to think of a
way to preserve the inheritance relationships in the parsetree
representation.  In the current code, expand_inherited_tables() adds
AppendRelInfo nodes to the planner's data structures as it does the
expansion; but I don't think AppendRelInfo is a suitable structure
for the rewriter to work with, and in any case there's no place to
put it in the Query representation.

Actually though, isn't this issue mostly about inheritance of a query
*target* table?  Moving that expansion to the rewriter is a totally
different and perhaps more tractable change.  It's certainly horribly ugly
as it is; hard to see how doing it at the rewriter could be worse.

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] updated emacs configuration

[Tom Lane]

Robert Haas  writes:
> If this only affects a handful of places, then sure, let's go ahead
> and fix it.  But if it's going to create a massive enough diff that
> we've gotta think about back-patching it, then IMHO it's totally not
> worth it.

A quick grep search says there are well over 3000 comment lines containing
'.' followed by a tab.  grep isn't smart enough to tell if the tabs expand
to exactly two spaces, but I bet the vast majority do.  So it might not
be quite as bad as the 8.1 wrap-margin change, but it'd be extensive.

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] ALTER TABLE lock strength reduction patch is unsafe

[Tom Lane]

Robert Haas  writes:
> On Tue, Jan 28, 2014 at 12:36 PM, Alvaro Herrera
>  wrote:
>> Honestly, I would prefer that we push a patch that has been thoroughly
>> reviewed and in which we have more confidence, so that we can push
>> without a GUC.  This means mark in CF as needs-review, then some other
>> developer reviews it and marks it as ready-for-committer.

> I also believe that would be the correct procedure.  Personally, I
> think it would be great if Noah can review this, as he's very good at
> finding the kind of problems that are likely to crop up here, and has
> examined previous versions.  I also have some interest in looking at
> it myself.  But I doubt that either of us (or any other senior hacker)
> can do that by Thursday.  I think all such people are hip-deep in
> other patches at the moment; I certainly am.

Yeah.  I actually have little doubt that the patch is sane on its own
terms of discussion, that is that Simon has chosen locking levels that
are mutually consistent in an abstract sense.  What sank the previous
iteration was that he'd failed to consider an implementation detail,
namely possible inconsistencies in SnapshotNow-based catalog fetches.
I'm afraid that there may be more such problems lurking under the
surface.  Noah's pretty good at finding such things, but really two
or three of us need to sit down and think about it for awhile before
I'd have much confidence in such a fundamental change.  And I sure don't
have time for this patch right now myself.

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] Wait free LW_SHARED acquisition - v0.2

[Peter Geoghegan]

On Fri, Nov 15, 2013 at 11:47 AM, Andres Freund  wrote:
> 1) I've added an abstracted atomic ops implementation. Needs a fair
>amount of work, also submitted as a separate CF entry. (Patch 1 & 2)

Commit 220b34331f77effdb46798ddd7cca0cffc1b2858 caused bitrot when
applying 0002-Very-basic-atomic-ops-implementation.patch. Please
rebase.


-- 
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] Retain dynamic shared memory segments for postmaster lifetime

[Amit Kapila]

On Tue, Jan 28, 2014 at 4:42 PM, Kyotaro HORIGUCHI
 wrote:
> Hello,
>
>> Currently there is no way user can keep the dsm
>> segments if he wants for postmaster lifetime, so I
>> have exposed a new API dsm_keep_segment()
>> to implement the same.
>
> I had a short look on this patch.

   Thanks.

>  - DSM implimentation seems divided into generic part (dsm.c) and
>platform dependent part(dsm_impl.c). This dsm_keep_segment
>puts WIN32 specific part directly into dms.c. I suppose it'd
>be better defining DSM_OP_KEEP_SEGMENT and calling dms_impl_op
>from dms_keep_segment, or something.
>
>  - Repeated calling of dsm_keep_segment even from different
>backends creates new (orphan) handles as many as it is called.
>Simplly invoking this function in some of extensions intending
>to stick segments might results in so many orphan
>handles. Something to curb that situation would be needed.

I think the right way to fix above 2 comments is as suggested by Robert.

>  - "Global/PostgreSQL.%u" is the same literal constant with that
>occurred in dsm_impl_windows. It should be defined as a
>constant (or a macro).
>
>  - dms_impl_windows uses errcode_for_dynamic_shared_memory() for
>ereport and it finally falls down to
>errcode_for_file_access(). I think it is preferable, maybe.

Okay, will take care of these in new version after your verification
on Windows.

>> The specs and need for this API is already discussed
>> in thread:
>> http://www.postgresql.org/message-id/ca+tgmoakogujqbedgeykysxud9eaidqx77j2_hxzrgfo3hr...@mail.gmail.com
>>
>> I had used dsm_demo (hacked it a bit) module used
>> during initial tests for dsm API's to verify the working on
>> Windows. So one idea could be that I can extend
>> that module to use this new API, so that it can be tested
>> by others as well or if you have any other better way, please
>> do let me know.
>
> I'll run on windows sooner:-)

Please update me once the verification is done on windows.


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


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

Re: [HACKERS] ALTER TABLE lock strength reduction patch is unsafe

[Robert Haas]

On Tue, Jan 28, 2014 at 12:36 PM, Alvaro Herrera
 wrote:
> Bruce Momjian escribió:
>> > I have no problem removing the parameter if required to. In that case,
>> > I would like to leave the parameter in until mid beta, to allow
>> > greater certainty.
>
> Uhm.  If we remove a GUC during beta we don't need to force an initdb.
> At worst we will need to keep a no-op GUC variable that is removed in
> the next devel cycle.  That said, if we're going to have a GUC that's
> going to disappear later, I think it's better to wait for 2 releases as
> proposed, not remove mid-beta.
>
>> > In any case, I would wish to retain as a minimum an extern bool
>> > variable allowing it to be turned off by C function if desired.
>
> I think this amounts to having an undocumented GUC that is hard to
> change.  I prefer the GUC, myself.
>
>> Anything changed to postgresql.conf during beta is going to require an
>> initdb.
>> Also, lots of backward-compatibility infrastructure, as you are
>> suggesting above, add complexity to the system.
>>
>> I am basically against a GUC on this.  We have far larger problems with
>> 9.3 than backward compatibility, and limited resources.
>
> If we have a clear plan on removing the parameter, it's easy enough to
> follow through.  I don't think lack of resources is a good argument,
> because at that point there will be little to discuss and it's an easy
> change to make.  And I think the plan is clear: if no bug is found the
> parameter is removed.  If a bug is found, it is fixed and the parameter
> is removed anyway.
>
> Honestly, I would prefer that we push a patch that has been thoroughly
> reviewed and in which we have more confidence, so that we can push
> without a GUC.  This means mark in CF as needs-review, then some other
> developer reviews it and marks it as ready-for-committer.

I also believe that would be the correct procedure.  Personally, I
think it would be great if Noah can review this, as he's very good at
finding the kind of problems that are likely to crop up here, and has
examined previous versions.  I also have some interest in looking at
it myself.  But I doubt that either of us (or any other senior hacker)
can do that by Thursday.  I think all such people are hip-deep in
other patches at the moment; I certainly am.

-- 
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] [bug fix] pg_ctl always uses the same event source

[Amit Kapila]

On Fri, Jan 24, 2014 at 4:38 PM, MauMau  wrote:
>

>> How about below message:
>>
>> event source "" is already registered.

>> OK, I added several lines for this.  Please check the attached patch.

It gives the proper message, but even after error, the second message
box it shows "DLLInstall ... succeeded." I think the reason is that caller
of function DllRegisterServer() doesn't check the return value.

 + char message[1024];

why you have kept message as a global buffer, can't we just declare locally
inside the function?

>> What I had in mind was to change it during initdb, we are already doing it
>> for some other parameter (unix_socket_directories), please refer below
>> code in initdb.c
> Yes, It seems we can do this.  However, could you forgive me for leaving this 
> untouched?  I'm afraid postgresql.conf.sample's issue is causing
> unnecessary war among people here.  That doesn't affect the point of this 
> patch --- make pg_ctl use the event_source setting.  Anyway, not all
> parameters in postgresql.conf cannot be used just by uncommenting them. 
> That's another issue.

Okay, I think we can leave it and also remove it from other parts of patch.
Although I found it is the right way, but Tom is not convinced with the idea,
so lets keep the Default event source name handling as it is.

As suggested by Tom, please update documentation.
"> Possibly there's room for a documentation patch reminding users to
> make sure that event_source is set appropriately before they turn
> on eventlog."
I think right place to update this information is where we are explaining
about setting of event log i.e at below link or may be if you find some other
better place:
http://www.postgresql.org/docs/devel/static/runtime-config-logging.html#GUC-LOG-DESTINATION


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


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

Re: [HACKERS] Add force option to dropdb

[Robert Haas]

On Tue, Jan 28, 2014 at 9:01 AM, salah jubeh  wrote:
> Hello Sawada,
>
>>- This patch is not patched to master branch
> Sorry, My mistake
>>//new connections are not allowed
> Corrected.
>
> I hope now the patch in better state, if somthing left, I will be glad to
> fix it
> Regards

I'm not particularly in favor of implementing this as client-side
functionality, because then it's only available to people who use that
particular client.  Simon Riggs proposed a server-side option to the
DROP DATABASE command some time ago, and I think that might be the way
to go.

-- 
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] updated emacs configuration

[Andres Freund]

On 2014-01-28 18:39:43 -0300, Alvaro Herrera wrote:
> Bruce Momjian wrote:
> > 
> > I see I already asked about entab -s 3.  Let me see how hard it would be
> > to modify entab.
> 
> I cannot tell you what to waste your time on, of course, but I wonder if
> it'd be better spent trying to make something like astyle work for us.
> The entab/pg_bsd_indent combo is a bit "long on the tooth" by now.

I've wondered about getting the required options into clang's
clang-format. That would guarantee it's parsing things the right way and
they have many of the required options already and are adding others
(http://llvm-reviews.chandlerc.com/D2610).

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] Observed Compilation warning in WIN32 build

[Robert Haas]

On Tue, Jan 28, 2014 at 6:28 AM, Andres Freund  wrote:
> On 2014-01-28 09:13:15 +, Rajeev rastogi wrote:
>> I observed below WIN32 compilation warnings in postmaster.c (seems 
>> introduced by commit ea9df812d8502fff74e7bc37d61bdc7d66d77a7f "Relax the 
>> requirement that all lwlocks be stored in a single array.").
>
> This strikes me as the wrong fix, the types in BackendParams should be
> changed instead.

Agreed, fixed that way.

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


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

[HACKERS] Re: Patch: regexp_matches variant returning an array of matching positions

[David Johnston]

Alvaro Herrera-9 wrote
> Björn Harrtell wrote:
>> I've written a variant of regexp_matches called regexp_matches_positions
>> which instead of returning matching substrings will return matching
>> positions. I found use of this when processing OCR scanned text and
>> wanted
>> to prioritize matches based on their position.
> 
> Interesting.  I didn't read the patch but I wonder if it would be of
> more general applicability to return more info in a fell swoop a
> function returning a set (position, length, text of match), rather than
> an array.  So instead of first calling one function to get the match and
> then their positions, do it all in one pass.
> 
> (See pg_event_trigger_dropped_objects for a simple example of a function
> that returns in that fashion.  There are several others but AFAIR that's
> the simplest one.)

Confused as to your thinking. Like regexp_matches this returns "SETOF
type[]".  In this case integer but text for the matches.  I could see adding
a generic function that returns a SETOF named composite (match varchar[],
position int[], length int[]) and the corresponding type.  I'm not imagining
a situation where you'd want the position but not the text and so having to
evaluate the regexp twice seems wasteful.  The length is probably a waste
though since it can readily be gotten from the text and is less often
needed.  But if it's pre-calculated anyway...

My question is what position is returned in a multiple-match situation? The
supplied test only covers the simple, non-global, situation.  It needs to
exercise empty sub-matches and global searches.  One theory is that the
first array slot should cover the global position of match zero (i.e., the
entire pattern) within the larger document while sub-matches would be
relative offsets within that single match.  This conflicts, though, with the
fact that _matches only returns array elements for () items and never for
the full match - the goal in this function being parallel un-nesting. But as
nesting is allowed it is still possible to have occur.

How does this resolve in the patch?

SELECT regexp_matches('abcabc','((a)(b)(c))','g');

David J.







--
View this message in context: 
http://postgresql.1045698.n5.nabble.com/Patch-regexp-matches-variant-returning-an-array-of-matching-positions-tp5789321p5789414.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] updated emacs configuration

[Robert Haas]

On Tue, Jan 28, 2014 at 11:03 PM, Tom Lane  wrote:
> Bruce Momjian  writes:
>> On Tue, Jan 28, 2014 at 10:40:10PM -0500, Tom Lane wrote:
>>> TBH I'm not sure we should be changing pgindent at this late date, even if
>>> there is a good fix for its minor annoyances.  When you changed its wrap
>>> behavior in 8.1, I spent the next several years cursing that decision
>>> every time I had to back-patch something, because it caused apply failures
>>> for just about every nontrivial patch.  Doing what we're talking about
>>> here would be just as bad.
>
>> Would you like to see how many comments get changed by the adjustment?
>> Could we run it in all back branches, perhaps only on C comments?
>
> Hm.  Reindenting the active back branches would fix the back-patching
> issue, but it would also be a pain in the rear for anybody carrying
> out-of-tree patches; which is probably the majority of our packagers.
>
> On the third hand, updating such patches would only be a one-time
> chore (as long as we fix pgindent only *once*, not anytime the mood
> strikes us), so maybe it wouldn't be impossible.  Especially if we
> constrain ourselves to just fixing tabs vs. spaces, so that "patch
> --ignore-whitespace" could be used to apply old patches.

If we do it once, we're likely to do it again; and even if we only do
it once, so what?  It's still a nuisance.

If this only affects a handful of places, then sure, let's go ahead
and fix it.  But if it's going to create a massive enough diff that
we've gotta think about back-patching it, then IMHO it's totally not
worth it.

-- 
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] Changeset Extraction v7.3

[Robert Haas]

On Tue, Jan 28, 2014 at 7:43 PM, Andreas Karlsson  wrote:
> I think "physical" and "logical" are fine and they seem to be well known
> terminology. Oracle uses those words and I have also seen many places use
> "physical backup" and "logical backup", for example on Barman's homepage.

There's certainly something to be said for this.

Another idea I had this evening was "logical replication" and
"WAL-based replication", but that's a bit confusing too since logical
rep. is going to use WAL as an underlying technology.

-- 
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] updated emacs configuration

[Tom Lane]

Bruce Momjian  writes:
> On Tue, Jan 28, 2014 at 10:40:10PM -0500, Tom Lane wrote:
>> TBH I'm not sure we should be changing pgindent at this late date, even if
>> there is a good fix for its minor annoyances.  When you changed its wrap
>> behavior in 8.1, I spent the next several years cursing that decision
>> every time I had to back-patch something, because it caused apply failures
>> for just about every nontrivial patch.  Doing what we're talking about
>> here would be just as bad.

> Would you like to see how many comments get changed by the adjustment? 
> Could we run it in all back branches, perhaps only on C comments?

Hm.  Reindenting the active back branches would fix the back-patching
issue, but it would also be a pain in the rear for anybody carrying
out-of-tree patches; which is probably the majority of our packagers.

On the third hand, updating such patches would only be a one-time
chore (as long as we fix pgindent only *once*, not anytime the mood
strikes us), so maybe it wouldn't be impossible.  Especially if we
constrain ourselves to just fixing tabs vs. spaces, so that "patch
--ignore-whitespace" could be used to apply old patches.

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] updated emacs configuration

[Bruce Momjian]

On Tue, Jan 28, 2014 at 10:40:10PM -0500, Tom Lane wrote:
> Bruce Momjian  writes:
> > I see I already asked about entab -s 3.  Let me see how hard it would be
> > to modify entab.
> 
> TBH I'm not sure we should be changing pgindent at this late date, even if
> there is a good fix for its minor annoyances.  When you changed its wrap
> behavior in 8.1, I spent the next several years cursing that decision
> every time I had to back-patch something, because it caused apply failures
> for just about every nontrivial patch.  Doing what we're talking about
> here would be just as bad.

Would you like to see how many comments get changed by the adjustment? 
Could we run it in all back branches, perhaps only on C comments?

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

Re: [HACKERS] updated emacs configuration

[Tom Lane]

Bruce Momjian  writes:
> I see I already asked about entab -s 3.  Let me see how hard it would be
> to modify entab.

TBH I'm not sure we should be changing pgindent at this late date, even if
there is a good fix for its minor annoyances.  When you changed its wrap
behavior in 8.1, I spent the next several years cursing that decision
every time I had to back-patch something, because it caused apply failures
for just about every nontrivial patch.  Doing what we're talking about
here would be just as bad.

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] updated emacs configuration

[Bruce Momjian]

On Tue, Jan 28, 2014 at 06:39:43PM -0300, Alvaro Herrera wrote:
> Bruce Momjian wrote:
> > 
> > I see I already asked about entab -s 3.  Let me see how hard it would be
> > to modify entab.
> 
> I cannot tell you what to waste your time on, of course, but I wonder if
> it'd be better spent trying to make something like astyle work for us.
> The entab/pg_bsd_indent combo is a bit "long on the tooth" by now.

I know I tried GNU indent long ago several times, and I thought astyle
didn't have the options we wanted, but it has been so long ago I can't
be sure.

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

Re: [HACKERS] Retain dynamic shared memory segments for postmaster lifetime

[Robert Haas]

On Tue, Jan 28, 2014 at 6:12 AM, Kyotaro HORIGUCHI
 wrote:
> I had a short look on this patch.
>
>  - DSM implimentation seems divided into generic part (dsm.c) and
>platform dependent part(dsm_impl.c). This dsm_keep_segment
>puts WIN32 specific part directly into dms.c. I suppose it'd
>be better defining DSM_OP_KEEP_SEGMENT and calling dms_impl_op
>from dms_keep_segment, or something.

That might not be a very good fit, but maybe there should be a
separate function exposed by dsm_impl.c to do the
implementation-dependent part of the work; it would be a no-op except
on Windows.

>  - Repeated calling of dsm_keep_segment even from different
>backends creates new (orphan) handles as many as it is called.
>Simplly invoking this function in some of extensions intending
>to stick segments might results in so many orphan
>handles. Something to curb that situation would be needed.

I don't really think this is a problem.  Let's just document that
dsm_keep_segment() should not be called more than once per segment.

-- 
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] Suspicion of a compiler bug in clang: using ternary operator in ereport()

[Tom Lane]

Andres Freund  writes:
>> Absolutely.  Probably best to save errno into a local just before the
>> ereport.

> I think just resetting to edata->saved_errno is better and sufficient?

-1 --- that field is nobody's business except elog.c's.

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] [PATCH] Support for pg_stat_archiver view

[Michael Paquier]

On Wed, Jan 29, 2014 at 10:55 AM, Fujii Masao  wrote:
> On Wed, Jan 29, 2014 at 3:07 AM, Alvaro Herrera
>  wrote:
>>
>> Anybody knows about this patch?
>> http://www.postgresql.org/message-id/508dfec9.4020...@uptime.jp
>
> Though I'm not sure whether Nagayasu is still working on that patch,
> it's worth thinking to introduce that together with pg_stat_archiver.
This patch uses globalStats to implement the new stat fields for
walwriter, I think that we should use a different structure for
clarity and to be in-line with what is done now with the archiver
part.

Btw, I agree that renaming the globalStats part to something more
appropriate related to bgwriter is a sane target for this CF, but
isn't it too late to late to consider this walwriter patch for this
release? It introduces a new feature as it tracks xlog dirty writes
and CF is already half way.
Regards,
-- 
Michael


-- 
Sent 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] Support for pg_stat_archiver view

[Fujii Masao]

On Wed, Jan 29, 2014 at 3:07 AM, Alvaro Herrera
 wrote:
>
> Anybody knows about this patch?
> http://www.postgresql.org/message-id/508dfec9.4020...@uptime.jp

Though I'm not sure whether Nagayasu is still working on that patch,
it's worth thinking to introduce that together with pg_stat_archiver.

Regards,

-- 
Fujii Masao


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

Re: [HACKERS] Weird error messages from Windows upon client death

[Florian Pflug]

On Jan28, 2014, at 19:19 , Jeff Janes  wrote:
> On windows, if the client gets terminated while sending data to the server, 
> in a
> COPY for example, it results in some rather head-scratcher messages in the 
> server
> log, for example:
> 
> LOG:  could not receive data from client: No connection could be made because
> the target machine actively refused it.
> 
> Since the server was reading from the client and never tries to initiate a
> connection, the %m part of the message is a bit baffling.  The errno at this
> point is 10061.

My guess is that the server received a TCP RST, indicating that the client's
socket has gone away, and the the error message is the same for a RST received
during connection setup and a RST received later on.

During connection setup, it absolutely makes sense to say that the "client has
actively refused the connection" if it responds to a SYN packet with RST...

best regards,
Florian Pflug



-- 
Sent 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] Support for pg_stat_archiver view

[Michael Paquier]

On Wed, Jan 29, 2014 at 3:03 AM, Fujii Masao  wrote:
> On Tue, Jan 28, 2014 at 3:42 AM, Fujii Masao  wrote:
>> On Sat, Jan 25, 2014 at 3:19 PM, Michael Paquier
>>  wrote:
>>> On Sat, Jan 25, 2014 at 5:41 AM, Fujii Masao  wrote:
 On Thu, Jan 23, 2014 at 4:10 PM, Michael Paquier
  wrote:
> On Thu, Jan 9, 2014 at 6:36 AM, Gabriele Bartolini
>  wrote:
>> Il 08/01/14 18:42, Simon Riggs ha scritto:
>>> Not sure I see why it needs to be an SRF. It only returns one row.
>> Attached is version 4, which removes management of SRF stages.
>
> I have been looking at v4 a bit more, and found a couple of small things:
> - a warning in pgstatfuncs.c
> - some typos and format fixing in the sgml docs
> - some corrections in a couple of comments
> - Fixed an error message related to pg_stat_reset_shared referring
> only to bgwriter and not the new option archiver. Here is how the new
> message looks:
> =# select pg_stat_reset_shared('popo');
> ERROR:  22023: unrecognized reset target: "popo"
> HINT:  Target must be "bgwriter" or "archiver"
> A new version is attached containing those fixes. I played also with
> the patch and pgbench, simulating some archive failures and successes
> while running pgbench and the reports given by pg_stat_archiver were
> correct, so I am marking this patch as "Ready for committer".

 I refactored the patch further.

 * Remove ArchiverStats global variable to simplify pgarch.c.
 * Remove stats_timestamp field from PgStat_ArchiverStats struct because
it's not required.
>>> Thanks, pgstat_send_archiver is cleaner now.
>>>

 I have some review comments:

 +s.archived_wals,
 +s.last_archived_wal,
 +s.last_archived_wal_time,
 +s.failed_attempts,
 +s.last_failed_wal,
 +s.last_failed_wal_time,

 The column names of pg_stat_archiver look not consistent at least to me.
 What about the followings?

   archive_count
   last_archived_wal
   last_archived_time
   fail_count
   last_failed_wal
   last_failed_time
>>> And what about archived_count and failed_count instead of respectively
>>> archive_count and failed_count? The rest of the names are better now
>>> indeed.
>>>
 I think that it's time to rename all the variables related to 
 pg_stat_bgwriter.
 For example, it's better to change PgStat_GlobalStats to PgStat_Bgwriter.
 I think that it's okay to make this change as separate patch, though.
>>> Yep, this is definitely a different patch.
>>>
 +char last_archived_wal[MAXFNAMELEN];/* last WAL file archived */
 +TimestampTz last_archived_wal_timestamp;/* last archival success 
 */
 +PgStat_Counter failed_attempts;
 +char last_failed_wal[MAXFNAMELEN];/* last WAL file involved
 in failure */
 Some hackers don't like the increase of the size of the statsfile. In 
 order to
 reduce the size as much as possible, we should use the exact size of WAL 
 file
 here instead of MAXFNAMELEN?
>>> The first versions of the patch used a more limited size length more
>>> inline with what you say:
>>> +#define MAX_XFN_CHARS40
>>> But this is inconsistent with xlog_internal.h.
>>
>> Using MAX_XFN_CHARS instead of MAXFNAMELEN has a clear advantage
>> to reduce the size of the statistics file. Though I'm not sure how much this
>> change improve the performance of the statistics collector, basically
>> I'd like to
>> use MAX_XFN_CHARS here.
>
> I changed the patch in this way, fixed some existing bugs (e.g.,
> correct the column
> names of pg_stat_archiver in rules.out), and then just committed it.
Thanks, after re-reading the code using MAX_XFN_CHARS makes sense. I
didn't look at the comments on top of pg_arch.c. Thanks as well for
fixing the regression output :)
Regards,
-- 
Michael


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

Re: [HACKERS] Changeset Extraction v7.3

[Andreas Karlsson]

On 01/28/2014 10:56 PM, Andres Freund wrote:

On 2014-01-28 21:48:09 +, Thom Brown wrote:

On 28 January 2014 21:37, Robert Haas  wrote:

On Tue, Jan 28, 2014 at 11:53 AM, Robert Haas  wrote:
The point of Andres's patch set is to introduce a new technology
called logical decoding; that is, the ability to get a replication
stream that is based on changes to tuples rather than changes to
blocks.  It could also be called logical replication.  In these
patches, our existing replication is referred to as "physical"
replication, which sounds kind of funny to me.  Anyone have another
suggestion?


Logical and Binary replication?


Unfortunately changeset extraction output's can be binary data...


I think "physical" and "logical" are fine and they seem to be well known 
terminology. Oracle uses those words and I have also seen many places 
use "physical backup" and "logical backup", for example on Barman's 
homepage.


--
Andreas Karlsson


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

Re: [HACKERS] updated emacs configuration

[Alvaro Herrera]

Bruce Momjian wrote:
> 
> I see I already asked about entab -s 3.  Let me see how hard it would be
> to modify entab.

I cannot tell you what to waste your time on, of course, but I wonder if
it'd be better spent trying to make something like astyle work for us.
The entab/pg_bsd_indent combo is a bit "long on the tooth" by now.

-- 
Álvaro Herrerahttp://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] Suspicion of a compiler bug in clang: using ternary operator in ereport()

[Alvaro Herrera]

Stephen Frost wrote:
> * Alvaro Herrera (alvhe...@2ndquadrant.com) wrote:
> > Well, we already have Coverity reports and the VIVA64 stuff posted last
> > month.  Did they not see these problems?  Maybe they did, maybe not, but
> > since there's a large number of false positives it's hard to tell.  I
> > don't know how many false positives we would get from a Splint run, but
> > my guess is that it'll be a lot.
> 
> I've whittled down most of the false positives and gone through just
> about all of the rest.

Really?  Excellent, thanks.  I haven't looked at it in quite a while
apparently ...

-- 
Álvaro Herrerahttp://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] drop duplicate buffers in OS

[Jeff Janes]

On Wed, Jan 15, 2014 at 10:34 AM, Robert Haas  wrote:

> On Wed, Jan 15, 2014 at 1:53 AM, KONDO Mitsumasa
>  wrote:
> > I create patch that can drop duplicate buffers in OS using usage_count
> > alogorithm. I have developed this patch since last summer. This feature
> seems to
> > be discussed in hot topic, so I submit it more faster than my schedule.
> >
> > When usage_count is high in shared_buffers, they are hard to drop from
> > shared_buffers. However, these buffers wasn't required in file cache.
> Because
> > they aren't accessed by postgres(postgres access to shared_buffers).
> > So I create algorithm that dropping file cache which is high usage_count
> in
> > shared_buffers and is clean state in OS. If file cache are clean state
> in OS, and
> > executing posix_fadvice DONTNEED, it can only free in file cache without
> writing
> > physical disk. This algorithm will solve double-buffered situation
> problem and
> > can use memory more efficiently.
> >
> > I am testing DBT-2 benchmark now...
>

Have you had any luck with it?  I have reservations about this approach.
 Among other reasons, if the buffer is truly nailed in shared_buffers for
the long term, the kernel won't see any activity on it and will be able to
evict it fairly efficiently on its own.

So I'm reluctant to do a detailed review if the author cannot demonstrate a
performance improvement.  I'm going to mark it waiting-on-author for that
reason.


>
> The thing about this is that our usage counts for shared_buffers don't
> really work right now; it's common for everything, or nearly
> everything, to have a usage count of 5.



I'm surprised that that is common.  The only cases I've seen that was
either when the database exactly fits in shared_buffers, or when the
database is mostly appended, and the appends are done with inserts in a
loop rather than COPY.

Cheers,

Jeff

Re: [HACKERS] Changeset Extraction v7.3

[David Fetter]

On Tue, Jan 28, 2014 at 05:31:25PM -0500, Rod Taylor wrote:
> On Tue, Jan 28, 2014 at 4:56 PM, Andres Freund wrote:
> 
> > On 2014-01-28 21:48:09 +, Thom Brown wrote:
> > > On 28 January 2014 21:37, Robert Haas  wrote:
> > > > On Tue, Jan 28, 2014 at 11:53 AM, Robert Haas 
> > wrote:
> > > >> I've rebased it here and am hacking on it still.
> > > >
> > > > Andres and I are going back and forth between our respective git repos
> > > > hacking on this, and I think we're getting there, but I have a
> > > > terminological question which I'd like to submit to a wider audience:
> > > >
> > > > The point of Andres's patch set is to introduce a new technology
> > > > called logical decoding; that is, the ability to get a replication
> > > > stream that is based on changes to tuples rather than changes to
> > > > blocks.  It could also be called logical replication.  In these
> > > > patches, our existing replication is referred to as "physical"
> > > > replication, which sounds kind of funny to me.  Anyone have another
> > > > suggestion?
> > >
> > > Logical and Binary replication?
> >
> > Unfortunately changeset extraction output's can be binary data...
> >
> 
> Perhaps Logical and Block?
> 
> The existing replication mechanism is similar to block-based disk backups.
> It's the whole thing (not parts) and doesn't have any concept of
> database/directory.

+1 for this terminology.  It's descriptive.

Cheers,
David.
-- 
David Fetter  http://fetter.org/
Phone: +1 415 235 3778  AIM: dfetter666  Yahoo!: dfetter
Skype: davidfetter  XMPP: david.fet...@gmail.com
iCal: webcal://www.tripit.com/feed/ical/people/david74/tripit.ics

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] new json funcs

[Andrew Dunstan]

On 01/28/2014 05:07 PM, Marko Tiikkaja wrote:

Hi Andrew,

On 1/24/14, 7:26 PM, Andrew Dunstan wrote:

OK, here's the patch, this time with docs, thanks to Merlin Moncure and
Josh Berkus for help with that.


Thanks, this one is looking pretty good.  A couple of small issues:

  - The oid 3195 of json_object_agg_transfn has been taken by a recent 
commit, so that had to be changed.  The patch compiled and passed 
tests after that.


Yeah. These days you can't even build if there's a duplicate oid, so 
fixing that and a catalog version bump would be part of committing.




  - Typo in the description of json_build_array: "agument list"


will fix.



  - I find (perhaps due to not being a native speaker) the description 
of json_object a bit painful to read.  I would've expected something 
like:


- Builds a JSON object out of a text array.  The array must 
have exactly one dimension
+ Builds a JSON object out of a text array.  The array must 
have either exactly one dimension
  with an even number of members, in which case they are taken 
as alternating name/value
- pairs, or two dimensions with such that each inner array has 
exactly two elements, which
+ pairs, or two dimensions such that each inner array has 
exactly two elements, which

  are taken as a name/value pair.

 but I'm not sure about that either.


Yes, yours looks better.



  - There are a few cases of curly braces around a single-statement 
else, which I believe is against the project's code style guidelines.



IIRC we actually stopped pgindent removing that quite a few years ago, 
and the formatting guidelines at 
 don't 
say anything about it. Personally, I prefer consistency - I think either 
both branches of an if/else should use curly braces or neither should. I 
find it quite ugly and jarring when one does and the other doesn't.



Thanks for the review.

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] Changeset Extraction v7.3

[Rod Taylor]

On Tue, Jan 28, 2014 at 4:56 PM, Andres Freund wrote:

> On 2014-01-28 21:48:09 +, Thom Brown wrote:
> > On 28 January 2014 21:37, Robert Haas  wrote:
> > > On Tue, Jan 28, 2014 at 11:53 AM, Robert Haas 
> wrote:
> > >> I've rebased it here and am hacking on it still.
> > >
> > > Andres and I are going back and forth between our respective git repos
> > > hacking on this, and I think we're getting there, but I have a
> > > terminological question which I'd like to submit to a wider audience:
> > >
> > > The point of Andres's patch set is to introduce a new technology
> > > called logical decoding; that is, the ability to get a replication
> > > stream that is based on changes to tuples rather than changes to
> > > blocks.  It could also be called logical replication.  In these
> > > patches, our existing replication is referred to as "physical"
> > > replication, which sounds kind of funny to me.  Anyone have another
> > > suggestion?
> >
> > Logical and Binary replication?
>
> Unfortunately changeset extraction output's can be binary data...
>

Perhaps Logical and Block?

The existing replication mechanism is similar to block-based disk backups.
It's the whole thing (not parts) and doesn't have any concept of
database/directory.

Re: [HACKERS] Changeset Extraction v7.3

[Thom Brown]

On 28 January 2014 21:56, Andres Freund  wrote:
> On 2014-01-28 21:48:09 +, Thom Brown wrote:
>> On 28 January 2014 21:37, Robert Haas  wrote:
>> > On Tue, Jan 28, 2014 at 11:53 AM, Robert Haas  
>> > wrote:
>> >> I've rebased it here and am hacking on it still.
>> >
>> > Andres and I are going back and forth between our respective git repos
>> > hacking on this, and I think we're getting there, but I have a
>> > terminological question which I'd like to submit to a wider audience:
>> >
>> > The point of Andres's patch set is to introduce a new technology
>> > called logical decoding; that is, the ability to get a replication
>> > stream that is based on changes to tuples rather than changes to
>> > blocks.  It could also be called logical replication.  In these
>> > patches, our existing replication is referred to as "physical"
>> > replication, which sounds kind of funny to me.  Anyone have another
>> > suggestion?
>>
>> Logical and Binary replication?
>
> Unfortunately changeset extraction output's can be binary data...

"system"?
"cluster"?
"full"?
"complete"?

-- 
Thom


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

Re: [HACKERS] new json funcs

[Marko Tiikkaja]

Hi Andrew,

On 1/24/14, 7:26 PM, Andrew Dunstan wrote:

OK, here's the patch, this time with docs, thanks to Merlin Moncure and
Josh Berkus for help with that.


Thanks, this one is looking pretty good.  A couple of small issues:

  - The oid 3195 of json_object_agg_transfn has been taken by a recent 
commit, so that had to be changed.  The patch compiled and passed tests 
after that.


  - Typo in the description of json_build_array: "agument list"

  - I find (perhaps due to not being a native speaker) the description 
of json_object a bit painful to read.  I would've expected something like:


- Builds a JSON object out of a text array.  The array must have 
exactly one dimension
+ Builds a JSON object out of a text array.  The array must have 
either exactly one dimension
  with an even number of members, in which case they are taken 
as alternating name/value
- pairs, or two dimensions with such that each inner array has 
exactly two elements, which
+ pairs, or two dimensions such that each inner array has 
exactly two elements, which

  are taken as a name/value pair.

 but I'm not sure about that either.

  - There are a few cases of curly braces around a single-statement 
else, which I believe is against the project's code style guidelines.


Otherwise this patch looks good to my eyes.


Regards,
Marko Tiikkaja


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

Re: [HACKERS] Suspicion of a compiler bug in clang: using ternary operator in ereport()

[Stephen Frost]

* Alvaro Herrera (alvhe...@2ndquadrant.com) wrote:
> Well, we already have Coverity reports and the VIVA64 stuff posted last
> month.  Did they not see these problems?  Maybe they did, maybe not, but
> since there's a large number of false positives it's hard to tell.  I
> don't know how many false positives we would get from a Splint run, but
> my guess is that it'll be a lot.

I've whittled down most of the false positives and gone through just
about all of the rest.  I do not recall any reports in Coverity for this
issue and that makes me doubt that it checks for it.

I'll try and take a look at what splint reports this weekend.

Thanks,

Stephen


signature.asc
Description: Digital signature

Re: [HACKERS] Fwd: Request for error explaination || Adding a new integer in indextupleData Structure

[Rohit Goyal]

>
> Hello,
>>
>> I started all the process again and configured my eclipse with raw
>> postgresql code. First change i made in the code is
>>
>> I added *int i; *in indextupleData structure in itup.h.
>>
>
> You should show us *exactly* where you added it.  (Doing so is what "diff"
> was developed for, so please use that or a similar tool.)
>
>
>>
>> I got the same error message. Please help me to understand and solve the
>> issue. I want to add an integer in index tuple for btree.
>>
>
> The data from IndexTupleData is written to disk, and then read back in
> again.  Did you initdb a new database cluster after you made your change?
>  If you did the initdb with the original code, and then tried to point your
> new code at the old disk files, that is very unlikely to work, as format is
> now different.
>
> Cheers,
>
> Jeff
>

Hi Jeff and Tom,

Thanks you so much. I was making the mistake you mentioned in the last
mail. :)

Regards,
Rohit Goyal


-- 
Regards,
Rohit Goyal

Re: [HACKERS] Changeset Extraction v7.3

[Andres Freund]

On 2014-01-28 21:48:09 +, Thom Brown wrote:
> On 28 January 2014 21:37, Robert Haas  wrote:
> > On Tue, Jan 28, 2014 at 11:53 AM, Robert Haas  wrote:
> >> I've rebased it here and am hacking on it still.
> >
> > Andres and I are going back and forth between our respective git repos
> > hacking on this, and I think we're getting there, but I have a
> > terminological question which I'd like to submit to a wider audience:
> >
> > The point of Andres's patch set is to introduce a new technology
> > called logical decoding; that is, the ability to get a replication
> > stream that is based on changes to tuples rather than changes to
> > blocks.  It could also be called logical replication.  In these
> > patches, our existing replication is referred to as "physical"
> > replication, which sounds kind of funny to me.  Anyone have another
> > suggestion?
> 
> Logical and Binary replication?

Unfortunately changeset extraction output's can be binary data...

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] Changeset Extraction v7.3

[Thom Brown]

On 28 January 2014 21:37, Robert Haas  wrote:
> On Tue, Jan 28, 2014 at 11:53 AM, Robert Haas  wrote:
>> I've rebased it here and am hacking on it still.
>
> Andres and I are going back and forth between our respective git repos
> hacking on this, and I think we're getting there, but I have a
> terminological question which I'd like to submit to a wider audience:
>
> The point of Andres's patch set is to introduce a new technology
> called logical decoding; that is, the ability to get a replication
> stream that is based on changes to tuples rather than changes to
> blocks.  It could also be called logical replication.  In these
> patches, our existing replication is referred to as "physical"
> replication, which sounds kind of funny to me.  Anyone have another
> suggestion?

Logical and Binary replication?

-- 
Thom


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

Re: [HACKERS] Changeset Extraction v7.3

[Robert Haas]

On Tue, Jan 28, 2014 at 11:53 AM, Robert Haas  wrote:
> I've rebased it here and am hacking on it still.

Andres and I are going back and forth between our respective git repos
hacking on this, and I think we're getting there, but I have a
terminological question which I'd like to submit to a wider audience:

The point of Andres's patch set is to introduce a new technology
called logical decoding; that is, the ability to get a replication
stream that is based on changes to tuples rather than changes to
blocks.  It could also be called logical replication.  In these
patches, our existing replication is referred to as "physical"
replication, which sounds kind of funny to me.  Anyone have another
suggestion?

There are a lot of ways to slice the space of possible replication
solutions.  We currently talk about "streaming replication" (as
opposed to "archiving") and "synchronous replication" (as opposed to
asynchronous), but this is a new distinction.  At least in theory,
whether replication is "physical" or logical is independent of whether
it's based on streaming or archiving and also of whether it's
synchronous or asynchronous.  So we can't for example talk about
"logical replication" in opposition to "streaming replication"; that's
comparing apples and oranges.  We need a pair of new terms, and I
can't immediately think of anything better than physical/logical, but
it still sounds somewhat awkward to me so ... anyone else have an
idea?

Thanks,

-- 
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] Suspicion of a compiler bug in clang: using ternary operator in ereport()

[Andres Freund]

On 2014-01-28 18:31:59 -0300, Alvaro Herrera wrote:
> Jason Petersen wrote:
> > I realize Postgres’ codebase is probably intractably large to begin
> > using a tool like splint (http://www.splint.org ), but this is exactly
> > the sort of thing it’ll catch. I’m pretty sure it would have warned in
> > this case that the code relies on an ordering of side effects that is
> > left undefined by C standards (and as seen here implemented
> > differently by two different compilers).
> 
> Well, we already have Coverity reports and the VIVA64 stuff posted last
> month.  Did they not see these problems?  Maybe they did, maybe not, but
> since there's a large number of false positives it's hard to tell.  I
> don't know how many false positives we would get from a Splint run, but
> my guess is that it'll be a lot.

Well, this isn't really a case of classical undefined beaviour. Most of
the code is actually perfectly well setup to handle the differing
evaluation, it's just that some bits of code forgot to restore errno.

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] updated emacs configuration

[Bruce Momjian]

On Thu, Jun 27, 2013 at 09:54:45PM -0400, Tom Lane wrote:
> Bruce Momjian  writes:
> > Should we be using entab -s 3?
> 
> IIUC, that wouldn't affect leading whitespace at all.  What it would
> affect I think (outside of comment blocks) is whitespace between code
> and a same-line /* ... */ comment.  Personally I'd prefer that a
> tab-stop-aligned /* ... */ comment always have a tab before it, even
> if the expansion of the tab is only *one* space.  That is, in
> 
> foo(bar,/* comment */
> bar1,   /* comment */
> bar12,  /* comment */
> bar123, /* comment */
> baz);   /* comment */
> 
> I think each of those comments ought to have a tab before it, not
> space(s).  pgindent currently does this inconsistently --- the bar123
> line will have a space instead.  Moving to -s 3 would presumably make
> this worse (even less consistent) not better, since now the bar12 line
> would also be emitted with spaces not a tab.
> 
> Inside a comment, though, probably the behavior of -s 3 would be just
> fine.  So the real problem here IMO is that use of tabs ought to be
> context sensitive (behavior inside a comment different from outside),
> and I don't think entab can do that.  I see though that it understands
> about C quoted strings, so maybe we could teach it about comments too?
> 
> No idea whether astyle is any smarter about this.

I see I already asked about entab -s 3.  Let me see how hard it would be
to modify entab.

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

Re: [HACKERS] Suspicion of a compiler bug in clang: using ternary operator in ereport()

[Alvaro Herrera]

Jason Petersen wrote:
> I realize Postgres’ codebase is probably intractably large to begin
> using a tool like splint (http://www.splint.org ), but this is exactly
> the sort of thing it’ll catch. I’m pretty sure it would have warned in
> this case that the code relies on an ordering of side effects that is
> left undefined by C standards (and as seen here implemented
> differently by two different compilers).

Well, we already have Coverity reports and the VIVA64 stuff posted last
month.  Did they not see these problems?  Maybe they did, maybe not, but
since there's a large number of false positives it's hard to tell.  I
don't know how many false positives we would get from a Splint run, but
my guess is that it'll be a lot.

> The workaround is to make separate assignments on separate lines,
> which act as sequence points to impose a total order on the
> side-effects in question.

Not sure how that would work with a complex macro such as ereport.
Perhaps the answer is to use C99 variadic macros if available, but that
would leave bugs such as this one open on compilers that don't support
variadic macros.

-- 
Álvaro Herrerahttp://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] updated emacs configuration

[Bruce Momjian]

On Thu, Jun 27, 2013 at 05:31:54PM -0400, Tom Lane wrote:
> Alvaro Herrera  writes:
> > Noah Misch wrote:
> >> Note that emacs and pgindent remain at odds over interior tabs in comments.
> >> When pgindent finds a double-space (typically after a sentence) ending at a
> >> tab stop, it replaces the double-space with a tab.  c-fill-paragraph will
> >> convert that tab to a *single* space, and that can be enough to change many
> >> line break positions.
> 
> > We should really stop pgindent from converting those double-spaces to
> > tabs.  Those tabs are later changed to three or four spaces when wording
> > of the comment is changed, and things start looking very odd.
> 
> +1.  That's probably the single most annoying bit of behavior in pgindent.
> Being a two-spaces-after-a-period kind of guy, it might bite me more
> often than other people, but now that somebody else has brought it up...

Sorry I am just getting to this.  I actually have an easy fix for this,
and it is a feature of entab:

$ entab -h
USAGE: entab [ -cdqst ] [file ...]
-c (clip trailing whitespace)
-d (delete tabs)
-q (protect quotes)
-s minimum_spaces
-t tab_width

-s minimum_spaces defaults to 2, and pgindent doesn't change the
default. If we change the entab call in pgindent from

$entab -t4 -qc

to

$entab -t4 -qc -s3

we will no longer place a tab in this string, "friend.  Hopefully"
  --

even if there is a tab stop before the 'H'.  It will use a 3-space
break.  Does that help?  Other ideas?  How about requiring 4?

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

Re: [HACKERS] WIP patch (v2) for updatable security barrier views

[Robert Haas]

On Tue, Jan 28, 2014 at 5:02 AM, Simon Riggs  wrote:
> I agree that this is being seen the wrong way around. The planner
> contains things it should not do, and as a result we are now
> discussing enhancing the code that is in the wrong place, which of
> course brings objections.
>
> I think we would be best served by stopping inheritance in its tracks
> and killing it off. It keeps getting in the way. What we need is real
> partitioning. Other uses are pretty obscure and we should re-examine
> things.

I actually think that inheritance is a pretty good foundation for real
partitioning.  If we were to get rid of it, we'd likely end up needing
to add most of the same functionality back when we tried to do some
kind of real partitioning later, and that doesn't sound fun.  I don't
see any reason why some kind of partitioning syntax couldn't be added
that leverages the existing inheritance mechanism but stores
additional metadata allowing for better optimization.

Well... I'm lying, a little bit.  If our chosen implementation of
"real partitioning" involved some kind of partition objects that could
be created and dropped but never directly accessed via DML commands,
then we might not need anything that looks like the current planner
support for partitioned tables.  But I think that would be a
surprising choice for this community.  IMV, the problem with the
planner and inheritance isn't that there's too much cruft in there
already, but that there are still key optimizations that are missing.
Still, I'd rather try to fix that than start over.

> In the absence of that, releasing this updateable-security views
> without suppport for inheritance is a good move. It gives us most of
> what we want now and continuing to have some form of restriction is
> better than having a much greater restriction of it not working at
> all.

-1.  Inheritance may be a crappy substitute for real partitioning, but
there are plenty of people using it that way.

-- 
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] Suspicion of a compiler bug in clang: using ternary operator in ereport()

[Andres Freund]

On 2014-01-28 16:19:11 -0500, Tom Lane wrote:
> Christian Kruse  writes:
> > According to http://llvm.org/bugs/show_bug.cgi?id=18644#c5 this is not
> > a compiler bug but a difference between gcc and clang. Clang seems to
> > use a left-to-right order of evaluation while gcc uses a right-to-left
> > order of evaluation. So if errmsg changes errno this would lead to
> > errno == ENOMEM evaluated to false.
> 
> Oh!  Yeah, that is our own bug then.

Pretty nasty too. Surprising that it didn't cause more issues. It's not
like it would only be capable to cause problems because of the
evaluation order...

> > Should we work on this issue?
> 
> Absolutely.  Probably best to save errno into a local just before the
> ereport.

I think just resetting to edata->saved_errno is better and sufficient?

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] Suspicion of a compiler bug in clang: using ternary operator in ereport()

[Jason Petersen]

I realize Postgres’ codebase is probably intractably large to begin using a 
tool like splint (http://www.splint.org ), but this is exactly the sort of 
thing it’ll catch. I’m pretty sure it would have warned in this case that the 
code relies on an ordering of side effects that is left undefined by C 
standards (and as seen here implemented differently by two different compilers).

The workaround is to make separate assignments on separate lines, which act as 
sequence points to impose a total order on the side-effects in question.

—Jason

On Jan 28, 2014, at 2:12 PM, Christian Kruse  wrote:

> Hi,
> 
> On 28/01/14 16:43, Christian Kruse wrote:
>>  ereport(FATAL,
>>  (errmsg("could not map anonymous shared memory: 
>> %m"),
>>   (errno == ENOMEM) ?
>>   errhint("This error usually means that 
>> PostgreSQL's request "
>>   "for a shared memory segment 
>> exceeded available memory "
>>   "or swap space. To reduce the 
>> request size (currently "
>>   "%zu bytes), reduce 
>> PostgreSQL's shared memory usage, "
>>   "perhaps by reducing 
>> shared_buffers or "
>>   "max_connections.",
>>   *size) : 0));
>> 
>> did not emit a errhint when using clang, although errno == ENOMEM was
>> true. The same code works with gcc.
> 
> According to http://llvm.org/bugs/show_bug.cgi?id=18644#c5 this is not
> a compiler bug but a difference between gcc and clang. Clang seems to
> use a left-to-right order of evaluation while gcc uses a right-to-left
> order of evaluation. So if errmsg changes errno this would lead to
> errno == ENOMEM evaluated to false. I added a watch point on errno and
> it turns out that exactly this happens: in src/common/psprintf.c line
> 114
> 
>   nprinted = vsnprintf(buf, len, fmt, args);
> 
> errno gets set to 0. This means that we will miss errhint/errdetail if
> we use errno in a ternary operator and clang.
> 
> Should we work on this issue?
> 
> Best regards,
> 
> -- 
> Christian Kruse   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] Patch: regexp_matches variant returning an array of matching positions

[Alvaro Herrera]

Björn Harrtell wrote:
> I've written a variant of regexp_matches called regexp_matches_positions
> which instead of returning matching substrings will return matching
> positions. I found use of this when processing OCR scanned text and wanted
> to prioritize matches based on their position.

Interesting.  I didn't read the patch but I wonder if it would be of
more general applicability to return more info in a fell swoop a
function returning a set (position, length, text of match), rather than
an array.  So instead of first calling one function to get the match and
then their positions, do it all in one pass.

(See pg_event_trigger_dropped_objects for a simple example of a function
that returns in that fashion.  There are several others but AFAIR that's
the simplest one.)

-- 
Álvaro Herrerahttp://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] Suspicion of a compiler bug in clang: using ternary operator in ereport()

[Tom Lane]

Christian Kruse  writes:
> According to http://llvm.org/bugs/show_bug.cgi?id=18644#c5 this is not
> a compiler bug but a difference between gcc and clang. Clang seems to
> use a left-to-right order of evaluation while gcc uses a right-to-left
> order of evaluation. So if errmsg changes errno this would lead to
> errno == ENOMEM evaluated to false.

Oh!  Yeah, that is our own bug then.

> Should we work on this issue?

Absolutely.  Probably best to save errno into a local just before the
ereport.

regards, tom lane


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

Re: [HACKERS] Suspicion of a compiler bug in clang: using ternary operator in ereport()

[Christian Kruse]

Hi,

On 28/01/14 16:43, Christian Kruse wrote:
>   ereport(FATAL,
>   (errmsg("could not map anonymous shared memory: 
> %m"),
>(errno == ENOMEM) ?
>errhint("This error usually means that 
> PostgreSQL's request "
>"for a shared memory segment 
> exceeded available memory "
>"or swap space. To reduce the 
> request size (currently "
>"%zu bytes), reduce 
> PostgreSQL's shared memory usage, "
>"perhaps by reducing 
> shared_buffers or "
>"max_connections.",
>*size) : 0));
> 
> did not emit a errhint when using clang, although errno == ENOMEM was
> true. The same code works with gcc.

According to http://llvm.org/bugs/show_bug.cgi?id=18644#c5 this is not
a compiler bug but a difference between gcc and clang. Clang seems to
use a left-to-right order of evaluation while gcc uses a right-to-left
order of evaluation. So if errmsg changes errno this would lead to
errno == ENOMEM evaluated to false. I added a watch point on errno and
it turns out that exactly this happens: in src/common/psprintf.c line
114

nprinted = vsnprintf(buf, len, fmt, args);

errno gets set to 0. This means that we will miss errhint/errdetail if
we use errno in a ternary operator and clang.

Should we work on this issue?

Best regards,

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



pgpz5JCRgHOM0.pgp
Description: PGP signature

Re: [HACKERS] fixing pg_ctl with relative paths

[Bruce Momjian]

On Mon, Jul  1, 2013 at 08:10:14PM -0400, Josh Kupershmidt wrote:
> On Thu, Jun 27, 2013 at 11:47 AM, Fujii Masao  wrote:
> > On Thu, Jun 27, 2013 at 10:36 AM, Josh Kupershmidt  
> > wrote:
> >> On Wed, Jun 26, 2013 at 12:22 PM, Fujii Masao  
> >> wrote:
> >>> Though this is a corner case, the patch doesn't seem to handle properly 
> >>> the case
> >>> where "-D" appears as other option value, e.g., -k option value, in
> >>> postmaster.opts
> >>> file.
> >>
> >> Could I see a command-line example of what you mean?
> >
> > postmaster -k "-D", for example. Of course, it's really a corner case :)
> 
> Oh, I see. I was able to trip up strip_datadirs() with something like
> 
> $ PGDATA="/my/data/" postmaster -k "-D" -S 100 &
> $ pg_ctl -D /my/data/ restart
> 
> that example causes pg_ctl to fail to start the server after stopping
> it, although perhaps you could even trick the server into starting
> with the wrong options. Of course, similar problems exists today in
> other cases, such as with the relative paths issue this patch is
> trying to address, or a datadir containing embedded quotes.
> 
> I am eager to see the relative paths issue fixed, but maybe we need to
> bite the bullet and sort out the escaping of command-line options in
> the rest of pg_ctl first, so that a DataDir like "/tmp/here's a \"
> quote" can consistently be used by pg_ctl {start|stop|restart} before
> we can fix this wart.

Where are we on this patch?

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

Re: [HACKERS] new json funcs

[Andrew Dunstan]

On 01/28/2014 01:22 PM, Alvaro Herrera wrote:

Josh Berkus wrote:

On 01/27/2014 01:06 PM, Alvaro Herrera wrote:

Andrew Dunstan escribió:


I'm not sure I understand the need. This is the difference between
the _text variants and their parents. Why would you call
json_object_field when you want the dequoted text?

Because I first need to know its type.  Sometimes it's an array, or an
object, or a boolean, and for those I won't call the _text version
afterwards but just use the original.

It would make more sense to extract them as JSON, check the type, and
convert.

That's what I'm already doing.  My question is how do I convert it?
I have this, but would like to get rid of it:

/*
  * dequote_jsonval
  * Take a string value extracted from a JSON object, and return a 
copy of it
  * with the quoting removed.
  *
  * Another alternative to this would be to run the extraction routine again,
  * using the "_text" variant which returns the value without quotes; but this
  * complicates the logic a lot because not all values are extracted in
  * the same way (some are extracted using json_object_field, others
  * using json_array_element).  Dequoting the object already at hand is a
  * lot easier.
  */
static char *
dequote_jsonval(char *jsonval)
{
char   *result;
int inputlen = strlen(jsonval);
int i;
int j = 0;

result = palloc(strlen(jsonval) + 1);

/* skip the start and end quotes right away */
for (i = 1; i < inputlen - 1; i++)
{
/*
 * XXX this skips the \ in a \" sequence but leaves other 
escaped
 * sequences in place.  Are there other cases we need to handle
 * specially?
 */
if (jsonval[i] == '\\' &&
jsonval[i + 1] == '"')
{
i++;
continue;
}

result[j++] = jsonval[i];
}
result[j] = '\0';

return result;
}





Well, TIMTOWTDI. Here's roughly what I would do, in json.c, making the 
json lexer do the work for us:


   extern char *
   dequote_scalar_json_string(char *jsonval)
   {
text *json = cstring_to_text(jsonval);
JsonLexContext *lex = makeJsonLexContext(json, true);
JsonTokenType tok;
char *ret;

json_lex(lex);
tok = lex_peek(lex);
if (tok == JSON_TOKEN_STRING)
ret=pstrdup(lex->strval->data);
else
ret = jsonval;
pfree(lex->strval->data);
pfree(lex->strval);
pfree(lex);
pfree(json);

return ret;
   }


I'm not sure if we should have a gadget like this at the SQL level also.


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] Planning time in explain/explain analyze

[Tom Lane]

Robert Haas  writes:
> On Thu, Jan 9, 2014 at 11:45 PM, Tom Lane  wrote:
>> Uh, no, wasn't my suggestion.  Doesn't that design imply measuring *every*
>> planning cycle, explain or no?  I was thinking more of just putting the
>> timing calls into explain.c.

> The problem is that you really can't do it that way.
> ExplainOneQuery() is a good place to add the timing calls in general,
> but ExplainExecuteQuery() in prepare.c never calls it; it does
> GetCachedPlan(), which ultimately calls pg_plan_query() after about
> four levels of indirection, and then passes the resulting plan
> directly to ExplainOnePlan().  So if you only add timing calls to
> explain.c, you can't handle anything that goes through
> ExplainExecuteQuery(), which is confusingly in prepare.c rather than
> explain.c.

Yeah, the factoring between explain.c and prepare.c has never been very
nice.  I'm not sure what would be a cleaner design offhand, but I suspect
there is one.

> One reasonably principled solution is to just give up on showing the
> plan time for prepared queries altogether.

That would work for me, especially given the lack of clarity about what
ought to be measured in that case.

> If we don't want to adopt
> that approach, then I think the right way to do this is to add a "bool
> timing" argument to pg_plan_query().  When set, pg_plan_query()
> measures the time before and after calling planner() and stores it in
> the PlannedStmt.

I don't find that to be exactly a "low footprint" change; it's dumping
an EXPLAIN concern all over a public API *and* a public data structure.
It might be roughly comparable in terms of number of lines in the patch,
but in terms of modularity and abstraction it's not comparable in the
least.

I'm for doing the measurement in ExplainOneQuery() and not printing
anything in code paths that don't go through there.

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: hide application_name from other users

[Stephen Frost]

Greg,

* Greg Stark (st...@mit.edu) wrote:
> On Tue, Jan 28, 2014 at 11:56 AM, Josh Berkus  wrote:
> > For example, I would really like to GRANT an unpriv user access to the
> > WAL columns in pg_stat_replication so that I can monitor replication
> > delay without granting superuser permissions.
> 
> So you can do this now by defining a security definer function that
> extracts precisely the information you need and grant execute access
> to precisely the users you want. There was some concern upthread about
> defining security definer functions being tricky but I'm not sure what
> conclusion to draw from that argument.

Yeah, but that sucks if you want to build a generic monitoring system
like check_postgres.pl.  Telling users to grant certain privileges may
work out, telling them to install these pl/pgsql things you write as
security-definer-to-superuser isn't going to be nearly as easy when
these users are (understandably, imv) uncomfortable having a monitor
role have superusr privs.

Thanks,

Stephen


signature.asc
Description: Digital signature

Re: [HACKERS] [PATCH] Negative Transition Aggregate Functions (WIP)

[Florian Pflug]

On Jan27, 2014, at 23:28 , Dean Rasheed  wrote:
> strict transfn vs non-strict inv_transfn
> 
> 
> This case is explicitly forbidden, both in CREATE AGGREGATE and in the
> executor. To me, that seems overly restrictive --- if transfn is
> strict, then you know for sure that no NULL values are added to the
> aggregate state, so surely it doesn't matter whether or not
> inv_transfn is strict. It will never be asked to remove NULL values.
> 
> I think there are definite use-cases where a user might want to use a
> pre-existing function as the inverse transition function, so it seems
> harsh to force them to wrap it in a strict function in this case.
> 
> AFAICS, the code in advance/retreat_windowaggregate should just work
> if those checks are removed.

True. It didn't use to in earlier version of the patch because the advance
and retreat functions looked at the strict settings directly, but now that
there's an ignore_nulls flag in the executor node, the only requirement
should be that ignore_nulls is set if either of the transition functions
is strict.

I'm not sure that the likelihood of someone wanting to combine a strict
forward with a non-strict inverse function is very hight, but neither

> non-strict transfn vs strict inv_transfn
> 
> 
> At first this seems as though it must be an error --- the forwards
> transition function allows NULL values into the aggregate state, and
> the inverse transition function is strict, so it cannot remove them.
> 
> But actually what the patch is doing in this case is treating the
> forwards transition function as partially strict --- it won't be
> passed NULL values (at least in a window context), but it may be
> passed a NULL state in order to build the initial state when the first
> non-NULL value arrives.

Exactly.

> It looks like this behaviour is intended to support aggregates that
> ignore NULL values, but cannot actually be made to have a strict
> transition function. I think typically this is because the aggregate's
> initial state is NULL and it's state type differs from the type of the
> values being aggregated, and so can't be automatically created from
> the first non-NULL value.

Yes. I added this because the alternative would haven been to count
non-NULL values in most of the existing SUM() aggregates.

> That all seems quite ugly though, because now you have a transition
> function that is not strict, which is passed NULL values when used in
> a normal aggregate context, and not passed NULL values when used in a
> window context (whether sliding or not), except for the NULL state for
> the first non-NULL value.

Ugh, true. Clearly, nodeAgg.c needs to follow what nodeWindowAgg.c does
and skip NULL inputs if the aggregate has a strict inverse transition
function. That fact that we never actually *call* the inverse doesn't
matter. Will fix that once we decided on the various strictness issues.

> I'm not sure if there is a better way to do it though. If we disallow
> this case, these aggregates would have to use non-strict functions for
> both the forward and inverse transition functions, which means they
> would have to implement their own non-NULL value counting.
> Alternatively, allowing strict transition functions for these
> aggregates would require that we provide some other way to initialise
> the state from the first non-NULL input, such as a new initfn.

I'm not sure an initfn would really help. It would allow us to return
to the initial requirement that the strict settings match - but you
already deem the check that the forward transition function can only
be strict if the inverse is also overly harsh, so would that really be
an improvement? It's also cost us an additional pg_proc entry per aggregate.

Another idea would be to have an explicit nulls=ignore|process option
for CREATE AGGREGATE. If nulls=process and either of the transition
functions are strict, we could either error out, or simply do what
normal functions calls do and pretend they return NULL for NULL inputs.
Not sure how the rule that forward transition functions may not return
NULL if there's an inverse transition function would fit in if we do
the latter, though.

The question is - is it worth it the effort to add that flag?

best regards,
Florian Pflug



-- 
Sent 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: hide application_name from other users

[Josh Berkus]

On 01/28/2014 12:10 PM, Tom Lane wrote:
> Josh Berkus  writes:
>> For example, I would really like to GRANT an unpriv user access to the
>> WAL columns in pg_stat_replication so that I can monitor replication
>> delay without granting superuser permissions.
> 
> Just out of curiosity, why is that superuser-only at all?  AFAICS the
> hidden columns are just some LSNs ... what is the security argument
> for hiding them in the first place?

Beats me, I can't find any discussion on it at all.

-- 
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] proposal: hide application_name from other users

[Stephen Frost]

Josh,

* Josh Berkus (j...@agliodbs.com) wrote:
> Really the only way we're going to solve this is to make column
> permissions on special system views fully configurable.

We really need to fully support column and row-level security to provide
the kind of granularty which we do today (but force on users through
using C functions which hide data depending on who you are instead of
giving them the ability to configure it themselves).

> For example, I would really like to GRANT an unpriv user access to the
> WAL columns in pg_stat_replication so that I can monitor replication
> delay without granting superuser permissions.

+1000

Thanks,

Stephen


signature.asc
Description: Digital signature

Re: [HACKERS] proposal: hide application_name from other users

[Tom Lane]

Josh Berkus  writes:
> For example, I would really like to GRANT an unpriv user access to the
> WAL columns in pg_stat_replication so that I can monitor replication
> delay without granting superuser permissions.

Just out of curiosity, why is that superuser-only at all?  AFAICS the
hidden columns are just some LSNs ... what is the security argument
for hiding them in the first place?

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: hide application_name from other users

[Greg Stark]

On Tue, Jan 28, 2014 at 11:56 AM, Josh Berkus  wrote:
> Really the only way we're going to solve this is to make column
> permissions on special system views fully configurable.
>
> For example, I would really like to GRANT an unpriv user access to the
> WAL columns in pg_stat_replication so that I can monitor replication
> delay without granting superuser permissions.

So you can do this now by defining a security definer function that
extracts precisely the information you need and grant execute access
to precisely the users you want. There was some concern upthread about
defining security definer functions being tricky but I'm not sure what
conclusion to draw from that argument.

Even if we had column level privileges this would still be necessary
in many cases and might be preferable to keep things consistent. For
example, you might not want the monitor account to have access to
sql_query but be able to check for backends running specific queries
(perhaps vacuum or ddl or a known problematic query).


-- 
greg


-- 
Sent 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: Request for error explaination || Adding a new integer in indextupleData Structure

[Jeff Janes]

On Tue, Jan 28, 2014 at 10:57 AM, Rohit Goyal  wrote:

Hello,
>
> I started all the process again and configured my eclipse with raw
> postgresql code. First change i made in the code is
>
> I added *int i; *in indextupleData structure in itup.h.
>

You should show us *exactly* where you added it.  (Doing so is what "diff"
was developed for, so please use that or a similar tool.)


>
> I got the same error message. Please help me to understand and solve the
> issue. I want to add an integer in index tuple for btree.
>

The data from IndexTupleData is written to disk, and then read back in
again.  Did you initdb a new database cluster after you made your change?
 If you did the initdb with the original code, and then tried to point your
new code at the old disk files, that is very unlikely to work, as format is
now different.

Cheers,

Jeff

Re: [HACKERS] UNION ALL on partitioned tables won't use indices.

[Noah Misch]

On Tue, Jan 28, 2014 at 06:53:32PM +0900, Kyotaro HORIGUCHI wrote:
> Hello, Thank you, and I' sorry to have kept you waiting.

No hurry.

> > > The attached two patches are rebased to current 9.4dev HEAD and
> > > make check at the topmost directory and src/test/isolation are
> > > passed without error. One bug was found and fixed on the way. It
> > > was an assertion failure caused by probably unexpected type
> > > conversion introduced by collapse_appendrels() which leads
> > > implicit whole-row cast from some valid reltype to invalid
> > > reltype (0) into adjust_appendrel_attrs_mutator().
> > 
> > What query demonstrated this bug in the previous type 2/3 patches?

I would still like to know the answer to the above question.

> > [transvar_merge_mutator()] has roughly the same purpose as
> > adjust_appendrel_attrs_mutator(), but it propagates the change to far fewer
> > node types.  Why this case so much simpler?  The parent translated_vars of
> > parent_appinfo may bear mostly-arbitrary expressions.
> 
> There are only two places where AppendRelInfo node is generated,
> expand_inherited_rtentry and
> pull_up_union_leaf_queries.(_copyAppendrelInfo is irrelevant to
> this discussion.) The core part generating translated_vars for
> them are make_inh_translation_list and
> make_setop_translation_list respectively. That's all. And they
> are essentially works on the same way, making a Var for every
> referred target entry of children like following.
> 
> | makeVar(varno,
> | tle->resno,
> | exprType((Node *) tle->expr),
> | exprTypmod((Node *) tle->expr),
> | exprCollation((Node *) tle->expr),
> | 0);

It's true that each AppendRelInfo is initially created that way, but they need
not remain so simple.  You can assume ctx->child_appinfo->translated_vars
contains only these Var nodes, but parent_appinfo->translated_vars may contain
arbitrary expressions.  (I think the pullup_replace_vars() call at
prepjointree.c:954 is where an AppendRelInfo can get non-Var expressions.)

> So all we should do to collapse nested appendrels is simplly
> connecting each RTEs directly to the root (most ancient?) RTE in
> the relationship, resolving Vars like above, as seen in patched
> expand_inherited_rtentry.
> 
> # If translated_vars are generated always in the way shown above,
> # using tree walker might be no use..
> 
> This can be done apart from all other stuffs compensating
> translation skew done in adjust_appendrel_attrs. I believe they
> are in orthogonal relationship.

Here is a test case that provokes an assertion failure under the patch; I
believe the cause is oversimplification in transvars_merge_mutator():

create table parent (a int, b int);
create table child () inherits (parent);
select parent from parent union all select parent from parent;


While poking at that test case, I noticed that the following test emits three
rows on HEAD and wrongly emits four rows with the patch:

create table parent (a int, b int);
create table child () inherits (parent);
insert into parent values (1,10);
insert into child values (2,20);
select a, b from parent union all select a, b from child;

-- 
Noah Misch
EnterpriseDB http://www.enterprisedb.com


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

Re: [HACKERS] proposal: hide application_name from other users

[Josh Berkus]

On 01/28/2014 07:27 AM, Greg Stark wrote:
> Why is application_name useful for users who aren't the DBA and aren't
> the user in question. The sql_query would probably be more useful than
> application_name but we hide that...

I have non-privileged monitoring scripts do counts of connections by
application name all the time as a way of checking for runaway
applications, and would be quite put out by restricting this to superusers.

Really the only way we're going to solve this is to make column
permissions on special system views fully configurable.

For example, I would really like to GRANT an unpriv user access to the
WAL columns in pg_stat_replication so that I can monitor replication
delay without granting superuser permissions.

-- 
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] proposal: hide application_name from other users

[Greg Stark]

On Tue, Jan 28, 2014 at 11:28 AM, Greg Stark  wrote:
> Well maybe. Or we want this useful information at a finer granularity
> than "everyone or nobody" and given the choice we prefer to have it
> than not.

Anyways, I don't feel incredibly strongly about this. I think we
should default any user-data to being visible only that user as a
general principle but I also think a system predicated on data like
argv or application_name being kept private is pretty fragile and
should be avoided so I'm not super tense about additional ways these
things can leak. I feel like this is an example where -hackers has a
bit of a blind spot when it comes to smaller databases by users who
aren't expert DBAs and don't need a dedicated box.


-- 
greg


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

Re: [HACKERS] [pgsql-advocacy] GSoC 2014 - mentors, students and admins

[Thom Brown]

On 28 January 2014 19:43, Stephen Frost  wrote:
> Thom,
>
> * Thom Brown (t...@linux.com) wrote:
>> Application to Google Summer of Code 2014 can be made as of next
>> Monday (3rd Feb), and then there will be a 12 day window in which to
>> submit an application.
>
> This is just for PG to be a participating organization, right?  There's
> a while before mentors and students get invovled, as I understand it.

Yes, correct.  Students and mentors don't need to be signed up until April.

>> Who would be up for mentoring this year?  And are there any project
>> ideas folk would like to suggest?
>
> I'm interested in mentoring and, unlike previous years, I've been
> collecting a personal list of things that I'd like to see worked on for
> PG which could be GSoC projects and will provide such in the next few
> days to this list (unless there's a different list that people want such
> posted to..?).

That's great.  I don't see any problem with posting suggestions here,
although I'd suggest refraining from going in-depth as that can come
later.  If there's enough interest and agreement, we'll go ahead and
apply.

Thom


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

Re: [HACKERS] [pgsql-advocacy] GSoC 2014 - mentors, students and admins

[Maxence AHLOUCHE]

Hi all,

I'd very interesting in taking part in the GSoC with PostgreSQL, as a
student.
Being quite busy at the moment (exam time), I haven't had time to think of
a subject yet, even though I could see some topics I found interesting.

2014-01-28 Josh Berkus 

> On 01/28/2014 09:46 AM, Atri Sharma wrote:
> > I would like to bring up the addition to MADLIB algorithms again this
> year.
> We can only take MADLIB this year if we have confirmed mentors who are
> MADLIB committers before the end of the application period (Feb 15).  We
> can't have a repeat of last year.
>

Indeed, I've been unlucky last year when I tried to apply to a subject with
Madlib, mostly due to their lack of reactivity. Even though their subjects
interest me, I don't intend to try my luck again.

Atri Sharma said:

> Also, some work on the foreign table constraints could be helpful.
>

What kind of work do you have in mind?

Stephen Frost said:

> I'm interested in mentoring and, unlike previous years, I've been
> collecting a personal list of things that I'd like to see worked on for
> PG which could be GSoC projects and will provide such in the next few
> days to this list (unless there's a different list that people want such
> posted to..?).
>
IMO, the best place would be the wiki page for GSoC (
http://wiki.postgresql.org/wiki/GSoC_2014), it would avoid interested
students (including me :) ) having to look for possible subjects in lots of
different places.


Regards,
Maxence
-- 
Maxence Ahlouche
06 06 66 97 00

Re: [HACKERS] [pgsql-advocacy] GSoC 2014 - mentors, students and admins

[Stephen Frost]

Thom,

* Thom Brown (t...@linux.com) wrote:
> Application to Google Summer of Code 2014 can be made as of next
> Monday (3rd Feb), and then there will be a 12 day window in which to
> submit an application.

This is just for PG to be a participating organization, right?  There's
a while before mentors and students get invovled, as I understand it.

> I'd like to gauge interest from both mentors and students as to
> whether we'll want to do this.

Yes.

> And I'd be fine with being admin again this year, unless there's
> anyone else who would like to take up the mantle?

Having you do it works for me. :)

> Who would be up for mentoring this year?  And are there any project
> ideas folk would like to suggest?

I'm interested in mentoring and, unlike previous years, I've been
collecting a personal list of things that I'd like to see worked on for
PG which could be GSoC projects and will provide such in the next few
days to this list (unless there's a different list that people want such
posted to..?).

Thanks,

Stephen


signature.asc
Description: Digital signature

Re: [HACKERS] proposal: hide application_name from other users

[Greg Stark]

On Tue, Jan 28, 2014 at 7:49 AM, Tom Lane  wrote:
> Oh really.  So, to clean up after their own ill-considered decision,
> they'd like to take useful information away from everybody.

Well maybe. Or we want this useful information at a finer granularity
than "everyone or nobody" and given the choice we prefer to have it
than not.


-- 
greg


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

[HACKERS] Patch: regexp_matches variant returning an array of matching positions

[Björn Harrtell]

I've written a variant of regexp_matches called regexp_matches_positions
which instead of returning matching substrings will return matching
positions. I found use of this when processing OCR scanned text and wanted
to prioritize matches based on their position.

The patch is for discussion. I'd also appriciate general suggestions as
this is my first experience with the postgresql code base.

The patch is against the master branch and includes a simple regression
test.
*** /tmp/DQoMjJ_regexp.c2014-01-28 19:59:37.470271459 +0100
--- src/backend/utils/adt/regexp.c  2014-01-28 19:44:47.298288383 +0100
***
*** 113,118 
--- 113,119 
 bool ignore_degenerate);
  static void cleanup_regexp_matches(regexp_matches_ctx *matchctx);
  static ArrayType *build_regexp_matches_result(regexp_matches_ctx *matchctx);
+ static ArrayType *build_regexp_matches_positions_result(regexp_matches_ctx 
*matchctx);
  static Datum build_regexp_split_result(regexp_matches_ctx *splitctx);
  
  
***
*** 833,838 
--- 834,898 
return regexp_matches(fcinfo);
  }
  
+ 
+ /*
+  * regexp_matches_positions()
+  *Return a table of matched locations of a pattern within a 
string.
+  */
+ Datum
+ regexp_matches_positions(PG_FUNCTION_ARGS)
+ {
+   FuncCallContext *funcctx;
+   regexp_matches_ctx *matchctx;
+ 
+   if (SRF_IS_FIRSTCALL())
+   {
+   text   *pattern = PG_GETARG_TEXT_PP(1);
+   text   *flags = PG_GETARG_TEXT_PP_IF_EXISTS(2);
+   MemoryContext oldcontext;
+ 
+   funcctx = SRF_FIRSTCALL_INIT();
+   oldcontext = 
MemoryContextSwitchTo(funcctx->multi_call_memory_ctx);
+ 
+   /* be sure to copy the input string into the multi-call ctx */
+   matchctx = setup_regexp_matches(PG_GETARG_TEXT_P_COPY(0), 
pattern,
+   
flags,
+   
PG_GET_COLLATION(),
+   
false, true, false);
+ 
+   /* Pre-create workspace that 
build_regexp_matches_positions_result needs */
+   matchctx->elems = (Datum *) palloc(sizeof(Datum) * 
matchctx->npatterns);
+   matchctx->nulls = (bool *) palloc(sizeof(bool) * 
matchctx->npatterns);
+ 
+   MemoryContextSwitchTo(oldcontext);
+   funcctx->user_fctx = (void *) matchctx;
+   }
+ 
+   funcctx = SRF_PERCALL_SETUP();
+   matchctx = (regexp_matches_ctx *) funcctx->user_fctx;
+ 
+   if (matchctx->next_match < matchctx->nmatches)
+   {
+   ArrayType  *result_ary;
+ 
+   result_ary = build_regexp_matches_positions_result(matchctx);
+   matchctx->next_match++;
+   SRF_RETURN_NEXT(funcctx, PointerGetDatum(result_ary));
+   }
+ 
+   /* release space in multi-call ctx to avoid intraquery memory leak */
+   cleanup_regexp_matches(matchctx);
+ 
+   SRF_RETURN_DONE(funcctx);
+ }
+ 
+ /* This is separate to keep the opr_sanity regression test from complaining */
+ Datum
+ regexp_matches_positions_no_flags(PG_FUNCTION_ARGS)
+ {
+   return regexp_matches_positions(fcinfo);
+ }
+ 
  /*
   * setup_regexp_matches --- do the initial matching for regexp_matches()
   *or regexp_split()
***
*** 1035,1040 
--- 1095,1140 
  }
  
  /*
+  * build_regexp_matches_positions_result - build output array for current 
match
+  */
+ static ArrayType *
+ build_regexp_matches_positions_result(regexp_matches_ctx *matchctx)
+ {
+   Datum  *elems = matchctx->elems;
+   bool   *nulls = matchctx->nulls;
+   int dims[1];
+   int lbs[1];
+   int loc;
+   int i;
+ 
+   /* Extract matching substrings from the original string */
+   loc = matchctx->next_match * matchctx->npatterns * 2;
+   for (i = 0; i < matchctx->npatterns; i++)
+   {
+   int so = matchctx->match_locs[loc++];
+   int eo = matchctx->match_locs[loc++];
+ 
+   if (so < 0 || eo < 0)
+   {
+   elems[i] = (Datum) 0;
+   nulls[i] = true;
+   }
+   else
+   {
+   elems[i] = Int32GetDatum(so)+1;
+   nulls[i] = false;
+   }
+   }
+ 
+   /* And form an array */
+   dims[0] = matchctx->npatterns;
+   lbs[0] = 1;
+   /* XXX: this hardcodes assumptions about the int4 type */
+   return construct_md_array(elems, nulls, 1, dims, lbs,
+ INT4OID, 4, true, 
'i');
+ }
+ 
+ /*
   * regexp_split

Re: [HACKERS] jsonb and nested hstore

[Josh Berkus]

On 01/28/2014 10:56 AM, Alvaro Herrera wrote:
> Josh Berkus escribió:
> 
>> Or is this just about whitespace and line breaks?
> 
> If the docs are going to be rehauled, please ignore my whitespace
> comments.

I'm sure you'll find plenty to criticize in my version.  ;-)


-- 
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] jsonb and nested hstore

[Alvaro Herrera]

Josh Berkus escribió:

> Or is this just about whitespace and line breaks?

If the docs are going to be rehauled, please ignore my whitespace
comments.

-- 
Álvaro Herrerahttp://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] Fwd: Request for error explaination || Adding a new integer in indextupleData Structure

[Rohit Goyal]

>
> Rohit Goyal  writes:
>> > Hi All,
>> > I was trying to modify indextupledata structure by adding an integer
>> > variable. ButI faced an error message "psql: FATAL:  could not find
>> tuple
>> > for opclass 10032".
>>
>> > Could anyone please help me in resolving this issue.
>>
>> You broke a system catalog index.  Without seeing what you changed and
>> where, it's impossible to say just how, but that's the bottom line.
>>
>> This is the first line which i want to write in the code. Can you tel me
> how to add an integer in  indextupledata structure if I have a fresh system
> with no changes?
>
> In recent versions of PG, opclass 10032 is btree name_ops (unless you've
>> also added/removed system catalog entries), which is a pretty plausible
>> thing to be one of the first indexscanned fetches during relcache.c
>> initialization, so I don't think there's any great significance in this
>> particular error message.  It's likely that you broke *all* indexscans
>> not just one specific one.
>
> I am not sure how i can break all indexscan as this is the first line I
> wrote in the code.
>
>>
>>
> regards, tom lane
>
>
Hello,

I started all the process again and configured my eclipse with raw
postgresql code. First change i made in the code is

I added *int i; *in indextupleData structure in itup.h.

I got the same error message. Please help me to understand and solve the
issue. I want to add an integer in index tuple for btree.

Regards,
Rohit Goyal

Re: [HACKERS] jsonb and nested hstore

[Josh Berkus]

On 01/28/2014 10:29 AM, Merlin Moncure wrote:
>> In addition to this, the JSON vs JSONB datatype page really needs
>> expansion and clarification.
> 
> right: exactly.  I'd be happy to help (such as I can) ...I wanted to
> see if jsonb to make it in on this 'fest (doc issues notwithstanding);
> it hasn't been formally reviewed yet AFAICT.  So my thinking here is
> to get docs to minimum acceptable standards in the short term and
> focus on the structural code issues for the 'fest (if jsonb slips then
> it's moot obviously).

Well, having reviewed the docs before Andrew sent them in, I felt they
already *were* "minimum acceptable".  Certainly they're as complete as
the original JSON docs were.

Or is this just about whitespace and line breaks?

-- 
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] [pgsql-advocacy] GSoC 2014 - mentors, students and admins

[Josh Berkus]

On 01/28/2014 09:46 AM, Atri Sharma wrote:
> I would like to bring up the addition to MADLIB algorithms again this year.
> 
> Also, some work on the foreign table constraints could be helpful.

We can only take MADLIB this year if we have confirmed mentors who are
MADLIB committers before the end of the application period (Feb 15).  We
can't have a repeat of last year.

-- 
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] alternative back-end block formats

[Christian Convey]

On Tue, Jan 28, 2014 at 12:39 PM, Tom Lane  wrote:

> TBH, I'd rather we waited till the commitfest is over.  This is certainly
> material for 9.5, if not even further out, so there's no pressing need for
> a debate right now; and we have plenty of stuff we do need to deal with
> right now.


Works for me.  I'll just lurk in the meantime, and see what I can figure
out.  Thanks.

- Christian

Re: [HACKERS] jsonb and nested hstore

[Merlin Moncure]

On Tue, Jan 28, 2014 at 12:09 PM, Josh Berkus  wrote:
> On 01/28/2014 09:58 AM, Merlin Moncure wrote:
>> yeah. note: I think the json documentation needs *major* overhaul. too
>> much is going in inside the function listings where there really
>> should be a big breakout discussing the "big picture" of json/jsonb
>> with examples of various use cases.  I want to give it a shot but
>> unfortunately can not commit to do that by the end of the 'fest.
>
> FWIW, I've promised Andrew that I'll overhaul this by the end of beta.
> Given that we have all of beta for doc refinements.
>
> In addition to this, the JSON vs JSONB datatype page really needs
> expansion and clarification.

right: exactly.  I'd be happy to help (such as I can) ...I wanted to
see if jsonb to make it in on this 'fest (doc issues notwithstanding);
it hasn't been formally reviewed yet AFAICT.  So my thinking here is
to get docs to minimum acceptable standards in the short term and
focus on the structural code issues for the 'fest (if jsonb slips then
it's moot obviously).

merlin


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

Re: [HACKERS] new json funcs

[Alvaro Herrera]

Josh Berkus wrote:
> On 01/27/2014 01:06 PM, Alvaro Herrera wrote:
> > Andrew Dunstan escribió:
> > 
> >> I'm not sure I understand the need. This is the difference between
> >> the _text variants and their parents. Why would you call
> >> json_object_field when you want the dequoted text?
> > 
> > Because I first need to know its type.  Sometimes it's an array, or an
> > object, or a boolean, and for those I won't call the _text version
> > afterwards but just use the original.
> 
> It would make more sense to extract them as JSON, check the type, and
> convert.

That's what I'm already doing.  My question is how do I convert it?
I have this, but would like to get rid of it:

/*
 * dequote_jsonval
 *  Take a string value extracted from a JSON object, and return a 
copy of it
 *  with the quoting removed.
 *
 * Another alternative to this would be to run the extraction routine again,
 * using the "_text" variant which returns the value without quotes; but this
 * complicates the logic a lot because not all values are extracted in
 * the same way (some are extracted using json_object_field, others
 * using json_array_element).  Dequoting the object already at hand is a
 * lot easier.
 */
static char *
dequote_jsonval(char *jsonval)
{
char   *result;
int inputlen = strlen(jsonval);
int i;
int j = 0;

result = palloc(strlen(jsonval) + 1);

/* skip the start and end quotes right away */
for (i = 1; i < inputlen - 1; i++)
{
/*
 * XXX this skips the \ in a \" sequence but leaves other 
escaped
 * sequences in place.  Are there other cases we need to handle
 * specially?
 */
if (jsonval[i] == '\\' &&
jsonval[i + 1] == '"')
{
i++;
continue;
}

result[j++] = jsonval[i];
}
result[j] = '\0';

return result;
}

-- 
Álvaro Herrerahttp://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

[HACKERS] Weird error messages from Windows upon client death

[Jeff Janes]

On windows, if the client gets terminated while sending data to the server,
in a COPY for example, it results in some rather head-scratcher messages in
the server log, for example:

LOG:  could not receive data from client: No connection could be made
because the target machine actively refused it.

Since the server was reading from the client and never tries to initiate a
connection, the %m part of the message is a bit baffling.  The errno at
this point is 10061.

Googling for "No connection could be made because the target machine
actively refused it", I can't find any mentions for it that occur in a
context in which a connection is not being attempted, except for from
PostgreSQL.  So I think we must be doing something wrong but I can't figure
out what that would be (no strace, not gdb).  Any tips on how to figure out
why this is happening?

I run the below, and then terminated it with a ctrl-C.  This is with 9.4dev
compiled with MinGW, but I've seen (unconfirmed by me) reports of the same
%m from the Windows binary distribution for a production version.

perl -le 'print rand() foreach 1..1000' | psql -c 'copy foo from stdin'

Cheers,

Jeff

Re: [HACKERS] new json funcs

[Josh Berkus]

On 01/27/2014 01:06 PM, Alvaro Herrera wrote:
> Andrew Dunstan escribió:
> 
>> I'm not sure I understand the need. This is the difference between
>> the _text variants and their parents. Why would you call
>> json_object_field when you want the dequoted text?
> 
> Because I first need to know its type.  Sometimes it's an array, or an
> object, or a boolean, and for those I won't call the _text version
> afterwards but just use the original.

It would make more sense to extract them as JSON, check the type, and
convert.

-- 
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] jsonb and nested hstore

[Josh Berkus]

On 01/28/2014 09:58 AM, Merlin Moncure wrote:
> yeah. note: I think the json documentation needs *major* overhaul. too
> much is going in inside the function listings where there really
> should be a big breakout discussing the "big picture" of json/jsonb
> with examples of various use cases.  I want to give it a shot but
> unfortunately can not commit to do that by the end of the 'fest.

FWIW, I've promised Andrew that I'll overhaul this by the end of beta.
Given that we have all of beta for doc refinements.

In addition to this, the JSON vs JSONB datatype page really needs
expansion and clarification.

-- 
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] Support for pg_stat_archiver view

[Alvaro Herrera]

Anybody knows about this patch?
http://www.postgresql.org/message-id/508dfec9.4020...@uptime.jp

-- 
Álvaro Herrerahttp://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] [PATCH] Support for pg_stat_archiver view

[Fujii Masao]

On Tue, Jan 28, 2014 at 3:42 AM, Fujii Masao  wrote:
> On Sat, Jan 25, 2014 at 3:19 PM, Michael Paquier
>  wrote:
>> On Sat, Jan 25, 2014 at 5:41 AM, Fujii Masao  wrote:
>>> On Thu, Jan 23, 2014 at 4:10 PM, Michael Paquier
>>>  wrote:
 On Thu, Jan 9, 2014 at 6:36 AM, Gabriele Bartolini
  wrote:
> Il 08/01/14 18:42, Simon Riggs ha scritto:
>> Not sure I see why it needs to be an SRF. It only returns one row.
> Attached is version 4, which removes management of SRF stages.

 I have been looking at v4 a bit more, and found a couple of small things:
 - a warning in pgstatfuncs.c
 - some typos and format fixing in the sgml docs
 - some corrections in a couple of comments
 - Fixed an error message related to pg_stat_reset_shared referring
 only to bgwriter and not the new option archiver. Here is how the new
 message looks:
 =# select pg_stat_reset_shared('popo');
 ERROR:  22023: unrecognized reset target: "popo"
 HINT:  Target must be "bgwriter" or "archiver"
 A new version is attached containing those fixes. I played also with
 the patch and pgbench, simulating some archive failures and successes
 while running pgbench and the reports given by pg_stat_archiver were
 correct, so I am marking this patch as "Ready for committer".
>>>
>>> I refactored the patch further.
>>>
>>> * Remove ArchiverStats global variable to simplify pgarch.c.
>>> * Remove stats_timestamp field from PgStat_ArchiverStats struct because
>>>it's not required.
>> Thanks, pgstat_send_archiver is cleaner now.
>>
>>>
>>> I have some review comments:
>>>
>>> +s.archived_wals,
>>> +s.last_archived_wal,
>>> +s.last_archived_wal_time,
>>> +s.failed_attempts,
>>> +s.last_failed_wal,
>>> +s.last_failed_wal_time,
>>>
>>> The column names of pg_stat_archiver look not consistent at least to me.
>>> What about the followings?
>>>
>>>   archive_count
>>>   last_archived_wal
>>>   last_archived_time
>>>   fail_count
>>>   last_failed_wal
>>>   last_failed_time
>> And what about archived_count and failed_count instead of respectively
>> archive_count and failed_count? The rest of the names are better now
>> indeed.
>>
>>> I think that it's time to rename all the variables related to 
>>> pg_stat_bgwriter.
>>> For example, it's better to change PgStat_GlobalStats to PgStat_Bgwriter.
>>> I think that it's okay to make this change as separate patch, though.
>> Yep, this is definitely a different patch.
>>
>>> +char last_archived_wal[MAXFNAMELEN];/* last WAL file archived */
>>> +TimestampTz last_archived_wal_timestamp;/* last archival success */
>>> +PgStat_Counter failed_attempts;
>>> +char last_failed_wal[MAXFNAMELEN];/* last WAL file involved
>>> in failure */
>>> Some hackers don't like the increase of the size of the statsfile. In order 
>>> to
>>> reduce the size as much as possible, we should use the exact size of WAL 
>>> file
>>> here instead of MAXFNAMELEN?
>> The first versions of the patch used a more limited size length more
>> inline with what you say:
>> +#define MAX_XFN_CHARS40
>> But this is inconsistent with xlog_internal.h.
>
> Using MAX_XFN_CHARS instead of MAXFNAMELEN has a clear advantage
> to reduce the size of the statistics file. Though I'm not sure how much this
> change improve the performance of the statistics collector, basically
> I'd like to
> use MAX_XFN_CHARS here.

I changed the patch in this way, fixed some existing bugs (e.g.,
correct the column
names of pg_stat_archiver in rules.out), and then just committed it.

Regards,

-- 
Fujii Masao


-- 
Sent 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 and nested hstore

[Merlin Moncure]

On Tue, Jan 28, 2014 at 10:46 AM, Andrew Dunstan  wrote:
>
> On 01/28/2014 11:29 AM, Tom Lane wrote:
>>
>> Andrew Dunstan  writes:
>>>
>>> The problem is not the indexterm element, it's the space that might
>>> exist outside it. Are we using block level elements like  inside
>>> entry elements anywhere else?
>>
>> Probably not, and I wonder why you're trying to.  Whole paras inside
>> a table entry (this is a table no?) don't sound like they are going
>> to lead to nice-looking results.
>
> See 

yeah. note: I think the json documentation needs *major* overhaul. too
much is going in inside the function listings where there really
should be a big breakout discussing the "big picture" of json/jsonb
with examples of various use cases.  I want to give it a shot but
unfortunately can not commit to do that by the end of the 'fest.

merlin


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

Re: [HACKERS] ALTER TABLE lock strength reduction patch is unsafe

[Simon Riggs]

On 28 January 2014 17:21, Heikki Linnakangas  wrote:

> I don't understand why anyone would want to turn this feature off, ie.
> require stronger locks than necessary for a DDL change.

Nobody would *want* to turn it off. They might need to, as explained.

> If we're not confident that the patch is correct, then it should not be
> applied. If we are confident enough to commit it, and a bug crops up later,
> we'll fix the bug as usual.

I'd like to point out here that my own customers are already well
covered by the support services we offer. They will receive any such
fix very quickly.

My proposal was of assistance only to those without such contracts in
place, as are many of my proposals.

It doesn't bother me at all if you insist it should not be added. Just
choose v16 of the patch for review rather than v17.

-- 
 Simon Riggs   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] ALTER TABLE lock strength reduction patch is unsafe

[Andres Freund]

On 2014-01-27 15:25:22 -0500, Robert Haas wrote:
> On Mon, Jan 27, 2014 at 12:58 PM, Simon Riggs  wrote:
> > This version adds a GUC called ddl_exclusive_locks which allows us to
> > keep the 9.3 behaviour if we wish it. Some people may be surprised
> > that their programs don't wait in the same places they used to. We
> > hope that is a positive and useful behaviour, but it may not always be
> > so.
> >
> > I'll commit this on Thurs 30 Jan unless I hear objections.
> 
> I haven't reviewed the patch, but -1 for adding a GUC.

Dito. I don't think the patch in a bad shape otherwise. I'd very quickly
looked at a previous version and it did look rather sane, and several
other people had looked at earlier versions as well. IIRC Noah had a
fairly extensive look at some intricate race conditions...

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] [pgsql-advocacy] GSoC 2014 - mentors, students and admins

[Atri Sharma]

On Tue, Jan 28, 2014 at 11:16 PM, Atri Sharma  wrote:


>
>
> On Tue, Jan 28, 2014 at 11:04 PM, Thom Brown  wrote:
>
>> Hi all,
>>
>> Application to Google Summer of Code 2014 can be made as of next
>> Monday (3rd Feb), and then there will be a 12 day window in which to
>> submit an application.
>>
>> I'd like to gauge interest from both mentors and students as to
>> whether we'll want to do this.
>>
>> And I'd be fine with being admin again this year, unless there's
>> anyone else who would like to take up the mantle?
>>
>> Who would be up for mentoring this year?  And are there any project
>> ideas folk would like to suggest?
>>
>>
>
> Hi,
>
> I would like to bring up the addition to MADLIB algorithms again this year.
>
> Also, some work on the foreign table constraints could be helpful.
>
>
Hi,

Also, can we consider a project in an extension to be a project in GSoC
2014 as GSoC 2014 under PostgreSQL?

I was thinking of having some support for writable FDW in JDBC_FDW, if
possible.

Regards,

Atri
-- 
Regards,

Atri
*l'apprenant*

Re: [HACKERS] [pgsql-advocacy] GSoC 2014 - mentors, students and admins

[Atri Sharma]

On Tue, Jan 28, 2014 at 11:04 PM, Thom Brown  wrote:

> Hi all,
>
> Application to Google Summer of Code 2014 can be made as of next
> Monday (3rd Feb), and then there will be a 12 day window in which to
> submit an application.
>
> I'd like to gauge interest from both mentors and students as to
> whether we'll want to do this.
>
> And I'd be fine with being admin again this year, unless there's
> anyone else who would like to take up the mantle?
>
> Who would be up for mentoring this year?  And are there any project
> ideas folk would like to suggest?
>
>

Hi,

I would like to bring up the addition to MADLIB algorithms again this year.

Also, some work on the foreign table constraints could be helpful.

Regards,

Atri

-- 
Regards,

Atri
*l'apprenant*

Re: [HACKERS] ALTER TABLE lock strength reduction patch is unsafe

[Simon Riggs]

On 28 January 2014 17:15, Bruce Momjian  wrote:
> On Tue, Jan 28, 2014 at 04:36:39PM +, Simon Riggs wrote:
>> For me, reducing the strength of DDL locking is a major change in
>> RDBMS behaviour that could both delight and surprise our users. Maybe
>> a few actually depend upon the locking behaviour, maybe. After some
>> years of various people looking at this, I think we've got it right.
>> Experience tells me that while I think this is the outcome, we are
>> well advised to protect against the possibility that it is not correct
>> and that if we have corner case issues, it would be good to easily
>> disable this in the field. In the current case, a simple parameter
>> works very well to disable the feature; in other cases, not.
>>
>> Summary: This is an atypical case. I do not normally propose such
>> things - this is the third time in 10 years, IIRC.
>
> Uh, in my memory, you are the person who is most likely to suggest a
> GUC of all our developers.

(smiles) I have suggested parameters for many features, mostly in the
early days of my developments before I saw the light if autotuning.
But those were control parameters for tuning features. So yes, I have
probably introduced more parameters than most, amongst the many things
I've done. I'm guessing you don't recall how much trouble I went to in
order to allow sync rep to have only 1 parameter, c'est la vie.

What I'm discussing here is a compatibility parameter to allow new
features to be disabled. This would be the third time in 10 years I
suggested a parameter for that reason, i.e. one that the user would
hardly ever wish to set.

-- 
 Simon Riggs   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] ALTER TABLE lock strength reduction patch is unsafe

[Tom Lane]

Alvaro Herrera  writes:
> Honestly, I would prefer that we push a patch that has been thoroughly
> reviewed and in which we have more confidence, so that we can push
> without a GUC.  This means mark in CF as needs-review, then some other
> developer reviews it and marks it as ready-for-committer.

FWIW, that was the point I was trying to make 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] ALTER TABLE lock strength reduction patch is unsafe

[Alvaro Herrera]

Bruce Momjian escribió:

> > I have no problem removing the parameter if required to. In that case,
> > I would like to leave the parameter in until mid beta, to allow
> > greater certainty.

Uhm.  If we remove a GUC during beta we don't need to force an initdb.
At worst we will need to keep a no-op GUC variable that is removed in
the next devel cycle.  That said, if we're going to have a GUC that's
going to disappear later, I think it's better to wait for 2 releases as
proposed, not remove mid-beta.

> > In any case, I would wish to retain as a minimum an extern bool
> > variable allowing it to be turned off by C function if desired.

I think this amounts to having an undocumented GUC that is hard to
change.  I prefer the GUC, myself.

> Anything changed to postgresql.conf during beta is going to require an
> initdb.
> Also, lots of backward-compatibility infrastructure, as you are
> suggesting above, add complexity to the system.
> 
> I am basically against a GUC on this.  We have far larger problems with
> 9.3 than backward compatibility, and limited resources.

If we have a clear plan on removing the parameter, it's easy enough to
follow through.  I don't think lack of resources is a good argument,
because at that point there will be little to discuss and it's an easy
change to make.  And I think the plan is clear: if no bug is found the
parameter is removed.  If a bug is found, it is fixed and the parameter
is removed anyway.

Honestly, I would prefer that we push a patch that has been thoroughly
reviewed and in which we have more confidence, so that we can push
without a GUC.  This means mark in CF as needs-review, then some other
developer reviews it and marks it as ready-for-committer.

-- 
Álvaro Herrerahttp://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] A minor correction in comment in heaptuple.c

[Bruce Momjian]

On Tue, Jan 28, 2014 at 06:30:40PM +0100, Andres Freund wrote:
> > OK, reverted.  I have to question how well-balanced we are when a word
> > change in a C comment can cause so much contention.
> 
> The question is rather why to do such busywork changes in the first
> place imo. Without ever looking at more than one a few lines up/down
> especially.

I see what you mean that the identical comment appears in the same C
file.  :-(

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

Re: [HACKERS] alternative back-end block formats

[Tom Lane]

Christian Convey  writes:
>> There are a couple of really huge issues that would have to be argued out
>> before any progress could be made.

> Is this something that people want to spend time on right now?  As I
> mentioned earlier, I'm game.  But it doesn't sound like I'll get very far
> without adult supervision.

TBH, I'd rather we waited till the commitfest is over.  This is certainly
material for 9.5, if not even further out, so there's no pressing need for
a debate right now; and we have plenty of stuff we do need to deal with
right now.

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] ALTER TABLE lock strength reduction patch is unsafe

[Bruce Momjian]

On Tue, Jan 28, 2014 at 07:30:47PM +0200, Heikki Linnakangas wrote:
> On 01/28/2014 07:26 PM, Bruce Momjian wrote:
> >On Tue, Jan 28, 2014 at 07:21:50PM +0200, Heikki Linnakangas wrote:
> I have no problem removing the parameter if required to. In that case,
> I would like to leave the parameter in until mid beta, to allow
> greater certainty. In any case, I would wish to retain as a minimum an
> extern bool variable allowing it to be turned off by C function if
> desired.
> >>>
> >>>Anything changed to postgresql.conf during beta is going to require an
> >>>initdb.
> >>
> >>Huh? Surely not.
> >
> >Uh, if we ship beta1 with a GUC in postgresql.conf, and then we remove
> >support for the GUC in beta2, anyone starting a server initdb-ed with
> >beta1 is going to get an error and the server is not going to start:
> >
> > LOG:  unrecognized configuration parameter "xxx" in file 
> > "/u/pgsql/data/postgresql.conf" line 1
> > FATAL:  configuration file "/u/pgsql/data/postgresql.conf" contains 
> > errors
> >
> >so, yeah, it isn't going to require an initdb, but it is going to
> >require everyone to edit their postgresql.conf.
> 
> Only if you uncommented the value in the first place.

Oh, I had forgotten that.  Right.  It would still be odd to have a
commented-out line in postgresql.conf that was not support.

-- 
  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] GSoC 2014 - mentors, students and admins

[Thom Brown]

Hi all,

Application to Google Summer of Code 2014 can be made as of next
Monday (3rd Feb), and then there will be a 12 day window in which to
submit an application.

I'd like to gauge interest from both mentors and students as to
whether we'll want to do this.

And I'd be fine with being admin again this year, unless there's
anyone else who would like to take up the mantle?

Who would be up for mentoring this year?  And are there any project
ideas folk would like to suggest?

Thanks

Thom


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

Re: [HACKERS] Storing pg_stat_statements query texts externally, pg_stat_statements in core

[Tom Lane]

Peter Geoghegan  writes:
> I noticed a minor omission in the patch as committed. Attached patch
> corrects this.

Duh.  Thanks.

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] A minor correction in comment in heaptuple.c

[Alvaro Herrera]

Bruce Momjian escribió:
> On Tue, Jan 28, 2014 at 11:20:39AM -0500, Tom Lane wrote:
> > Andres Freund  writes:
> > > On 2014-01-28 11:14:49 -0500, Bruce Momjian wrote:
> > >> OK, so does anyone object to removing this comment line?
> > 
> > > Let's just not do anything. This is change for changes sake. Not
> > > improving anything the slightest.
> > 
> > Indeed.  I'd actually request that you revert your previous change to the
> > comment, as it didn't improve matters and is only likely to cause pain for
> > future back-patching.
> 
> OK, so we have a don't change anything and a revert. I am thinking the
> new wording as a super-minor improvement.  Anyone else want to vote?

I vote to revert to the original and can we please wait for longer than
a few hours on a weekend before applying this kind of change that is
obviously not without controversy.

-- 
Álvaro Herrerahttp://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] Performance Improvement by reducing WAL for Update Operation

[Jinyu]

I think sort by string column is lower during merge join,  maybe comparing 
function in sort need be refined to save some cycle. It’s the hot function when 
do sort.  


Heikki Linnakangas 编写：

>On 01/27/2014 07:03 PM, Amit Kapila wrote:
>> I have tried to improve algorithm in another way so that we can get
>> benefit of same chunks during find match (something similar to lz).
>> The main change is to consider chunks at fixed boundary (4 byte)
>> and after finding match, try to find if there is a longer match than
>> current chunk. While finding longer match, it still takes care that
>> next bigger match should be at chunk boundary. I am not
>> completely sure about the chunk boundary may be 8 or 16 can give
>> better results.
>
>Since you're only putting a value in the history every 4 bytes, you 
>wouldn't need to calculate the hash in a rolling fashion. You could just 
>take next four bytes, calculate hash, put it in history table. Then next 
>four bytes, calculate hash, and so on. Might save some cycles when 
>building the history table...
>
>- Heikki
>
>
>-- 
>Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
>To make changes to your subscription:
>http://www.postgresql.org/mailpref/pgsql-hackers

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