Re: [HACKERS] How to determine that a TransactionId is really aborted?

2017-10-22 Thread Jaime Casanova
On 22 October 2017 at 15:00, Eric Ridge <eeb...@gmail.com> wrote:
>> On Oct 22, 2017, at 1:50 PM, Peter Geoghegan <p...@bowt.ie> wrote:
>>
>> On Sun, Oct 22, 2017 at 12:23 PM, Eric Ridge <eeb...@gmail.com> wrote:
>>> Can anyone confirm or deny that this is correct?  I feel like it is 
>>> correct, but I'm no expert.
>>
>> What are you going to use the code for? I think that that context is
>> likely to matter here.
>
> I'm not exactly sure yet, but I'm thinking about storing transaction ids 
> externally and then regularly poking Postgres to see which ones are 
> aborted-and-no-longer-considered-visible so I can clean up my external list.
>

so, what you want is txid_status() [1]... while this is new in v10 you
can use the code as guide or just migrate to v10 ;)

[1] 
https://www.postgresql.org/docs/10/static/functions-info.html#functions-txid-snapshot

-- 
Jaime Casanova  www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, 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] Columnar storage support

2017-10-09 Thread Jaime Casanova
On 9 October 2017 at 15:03, legrand legrand <legrand_legr...@hotmail.com> wrote:
> Is there a chance that pluggable storage permits creation of a columnar rdbms
> as monetDB in PostgreSQL ?

pluggable storage is one of the pieces for it, yes...
but is not as simple as that, IIUC

-- 
Jaime Casanova  www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, 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] generated columns

2017-09-12 Thread Jaime Casanova
On 10 September 2017 at 00:08, Jaime Casanova
<jaime.casan...@2ndquadrant.com> wrote:
>
> During my own tests, though, i found some problems:
>

a few more tests:

create table t1 (
 id serial,
 height_cm int,
 height_in int generated always as (height_cm * 10)
) ;


"""
postgres=# alter table t1 alter height_cm type numeric;
ERROR:  unexpected object depending on column: table t1 column height_in
"""
should i drop the column and recreate it after the fact? this seems
more annoying than the same problem with views (drop view & recreate),
specially after you implement STORED


"""
postgres=# alter table t1 alter height_in type numeric;
ERROR:  found unexpected dependency type 'a'
"""
uh!?


also is interesting that in triggers, both before and after, the
column has a null. that seems reasonable in a before trigger but not
in an after trigger
"""
create function f_trg1() returns trigger as $$
  begin
 raise notice '%', new.height_in;
 return new;
  end
$$ language plpgsql;

create trigger trg1 before insert on t1
for each row execute procedure f_trg1();

postgres=# insert into t1 values(default, 100);
NOTICE:  
INSERT 0 1

create trigger trg2 after insert on t1
for each row execute procedure f_trg1();

postgres=# insert into t1 values(default, 100);
NOTICE:  
NOTICE:  
INSERT 0 1
"""

the default value shouldn't be dropped.
"""
postgres=# alter table t1 alter height_in drop default;
ALTER TABLE
postgres=# \d t1
  Table "public.t1"
  Column   |  Type   | Collation | Nullable |Default
+-+---+--+
 id | integer |   | not null |
nextval('t1_id_seq'::regclass)
 height_cm | integer |   |  |
 height_in   | integer |   |  | generated always as ()
"""
-- 
Jaime Casanova  www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, 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] generated columns

2017-09-09 Thread Jaime Casanova
On 30 August 2017 at 23:16, Peter Eisentraut
<peter.eisentr...@2ndquadrant.com> wrote:
> Here is another attempt to implement generated columns.  This is a
> well-known SQL-standard feature, also available for instance in DB2,
> MySQL, Oracle.
>
[...]
>
> In previous discussions, it has often been a source of confusion whether
> these generated columns are supposed to be computed on insert/update and
> stored, or computed when read.  The SQL standard is not explicit, but
> appears to lean toward stored.  DB2 stores.  Oracle computes on read.
> MySQL supports both.  So I target implementing both.  This makes sense:
> Both regular views and materialized views have their uses, too.  For the
> syntax, I use the MySQL/Oracle syntax of appending [VIRTUAL|STORED].  In
> this patch, only VIRTUAL is fully implemented.  I also have STORED kind
> of working, but it wasn't fully baked, so I haven't included it here.
>

Hi,

It applies and compiles without problems, it passes regression tests
and it does what it claims to do:

During my own tests, though, i found some problems:

-- UPDATEing the column, this is at least weird

postgres=# update t1 set height_in = 15;
ERROR:  column "height_in" can only be updated to DEFAULT
DETAIL:  Column "height_in" is a generated column.
postgres=# update t1 set height_in = default;
UPDATE 1


-- In a view it doesn't show any value

postgres=# create view v1 as select * from t1;
CREATE VIEW
postgres=# insert into t1(height_cm) values (10);
INSERT 0 1
postgres=# select * from t1;
   id   | height_cm | height_in
+---+---
 198000 |10 | 25.40
(1 row)

postgres=# select * from v1;
   id   | height_cm | height_in
+---+---
 198000 |10 |
(1 row)


-- In a inherits/partition tree, the default gets malformed

postgres=# create table t1_1 () inherits (t1);
CREATE TABLE
postgres=# \d t1_1
 Table "public.t1_1"
  Column   |  Type   | Collation | Nullable |Default
---+-+---+--+
 id| integer |   | not null | nextval('t1_id_seq'::regclass)
 height_cm | numeric |   |  |
 height_in | numeric |   |  | height_cm * 2.54
Inherits: t1

postgres=# insert into t1_1 values (11);
server closed the connection unexpectedly
This probably means the server terminated abnormally
before or while processing the request.
The connection to the server was lost. Attempting reset: Failed.
!> \q

-- 
Jaime Casanova  www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, 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] New command to monitor progression of long running queries

2017-05-05 Thread Jaime Casanova
On 5 May 2017 at 22:38, Vinayak Pokale <vinpok...@gmail.com> wrote:
>
> On Mon, Apr 17, 2017 at 9:09 PM, Remi Colinet <remi.coli...@gmail.com>
> wrote:
>>
>> Hello,
>>
>> I've implemented a new command named PROGRESS to monitor progression of
>> long running SQL queries in a backend process.
>>
> Thank you for the patch.
>

sorry if i'm bikeshedding to soon but... why a command instead of a function?
something like pg_progress_backend() will be in sync with
pg_cancel_backend()/pg_terminate_backend() and the result of such a
function could be usable by a tool to examine a slow query status

-- 
Jaime Casanova  www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, 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] SASL minor docs typo

2017-04-18 Thread Jaime Casanova
Hi,

reading SASL docs found this typo:

in protocol.sgml:1356
"""
To begin a SASL authentication exchange, the server an AuthenticationSASL
  message.
"""

I guess it should say "the server sends an AuthenticationSASL message"

-- 
Jaime Casanova  www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, 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] minor typo in client-auth.sgml

2017-04-14 Thread Jaime Casanova
Hi,

I was reading about SCRAM and the changes in the docs and found that
in this sentence (in doc/src/sgml/client-auth.sgml:925) there is a
missing comma (,) when listing the "password-based authentication
methods":

"""
The password-based authentication methods are scram
md5 and password.
"""

-- 
Jaime Casanova  www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, 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: Write Amplification Reduction Method (WARM)

2017-04-14 Thread Jaime Casanova
On 5 April 2017 at 13:32, Pavan Deolasee <pavan.deola...@gmail.com> wrote:
>
> Ok. I've extensively updated the README to match the current state of
> affairs. Updated patch set attached.

Hi Pavan,

I run a test on current warm patchset, i used pgbench with a scale of
20 and a fillfactor of 90 and then start the pgbench run with 6
clients in parallel i also run sqlsmith on it.

And i got a core dump after sometime of those things running.

The assertion that fails is:

"""
LOG:  statement: UPDATE pgbench_tellers SET tbalance = tbalance + 3519
WHERE tid = 34;
TRAP: FailedAssertion("!(((bool) (((const void*)(>t_ctid) !=
((void *)0)) && (((>t_ctid)->ip_posid & uint16) 1) << 13) -
1)) != 0", File: "../../../../src/include/access/htup_details.h",
Line: 659)
"""

-- 
Jaime Casanova  www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
#0  0x7f42d832e067 in __GI_raise (sig=sig@entry=6) at 
../nptl/sysdeps/unix/sysv/linux/raise.c:56
#1  0x7f42d832f448 in __GI_abort () at abort.c:89
#2  0x00842961 in ExceptionalCondition (conditionName=, 
errorType=, fileName=, lineNumber=)
at assert.c:54
#3  0x004c48e6 in HeapTupleHeaderGetRootOffset (tup=) at 
../../../../src/include/access/htup_details.h:659
#4  heap_update (relation=0x7f4294dc8168, otid=0x2660ff0, newtup=0x6, cid=2, 
crosscheck=0x7f42d91bc700, wait=-102 '\232', hufd=0x7ffd14fcebb0, 
lockmode=0x7ffd14fceb9c, modified_attrsp=0x7ffd14fceba8, 
warm_update=0x7ffd14fceb9b "") at heapam.c:4672
#5  0x0062e95d in ExecUpdate (tupleid=0x7ffd14fcec90, oldtuple=0x2324, 
slot=0x26606f8, planSlot=0x, epqstate=0x2660ff0, 
estate=0x25db9e0, 
canSetTag=1 '\001') at nodeModifyTable.c:1012
#6  0x0062f009 in ExecModifyTable (node=0x25dbc48) at 
nodeModifyTable.c:1609
#7  0x00616198 in ExecProcNode (node=node@entry=0x25dbc48) at 
execProcnode.c:424
#8  0x006116a6 in ExecutePlan (execute_once=, 
dest=0x262e498, direction=, numberTuples=0, 
sendTuples=, 
operation=CMD_UPDATE, use_parallel_mode=, 
planstate=0x25dbc48, estate=0x25db9e0) at execMain.c:1651
#9  standard_ExecutorRun (queryDesc=0x2666100, direction=, 
count=0, execute_once=) at execMain.c:360
#10 0x00746822 in ProcessQuery (plan=, 
sourceText=0x25ac4e0 "UPDATE pgbench_tellers SET tbalance = tbalance + 3494 
WHERE tid = 76;", 
params=0x0, queryEnv=0x0, dest=0x262e498, completionTag=0x7ffd14fcf010 "") 
at pquery.c:162
#11 0x00746a93 in PortalRunMulti (portal=portal@entry=0x25462d0, 
isTopLevel=isTopLevel@entry=1 '\001', setHoldSnapshot=setHoldSnapshot@entry=0 
'\000', 
dest=dest@entry=0x262e498, altdest=altdest@entry=0x262e498, 
completionTag=completionTag@entry=0x7ffd14fcf010 "") at pquery.c:1287
#12 0x007476a2 in PortalRun (portal=0x25462d0, 
count=9223372036854775807, isTopLevel=, run_once=, dest=0x262e498, 
altdest=0x262e498, completionTag=0x7ffd14fcf010 "") at pquery.c:800
#13 0x0074361b in exec_simple_query (query_string=0x2324 ) at postgres.c:1105
#14 0x00745254 in PostgresMain (argc=1, argv=0x25ac4e0, 
dbname=0x2555888 "pgbench", username=0x2528260 "jcasanov") at postgres.c:4075
#15 0x004780f3 in BackendRun (port=0x254dd70) at postmaster.c:4317
#16 BackendStartup (port=0x254dd70) at postmaster.c:3989
#17 ServerLoop () at postmaster.c:1729
#18 0x006d33d0 in PostmasterMain (argc=argc@entry=3, 
argv=argv@entry=0x25260c0) at postmaster.c:1337
#19 0x00478f4d in main (argc=3, argv=0x25260c0) at main.c:228


-- 
Sent 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 v1] Add and report the new "in_hot_standby" GUC pseudo-variable.

2017-03-22 Thread Jaime Casanova
On 18 March 2017 at 14:01, Elvis Pranskevichus <elpr...@gmail.com> wrote:
> On Saturday, March 18, 2017 3:33:16 AM EDT Michael Paquier wrote:
>>
>> Why adding a good chunk of code instead of using pg_is_in_recovery(),
>> which switches to false once a server exits recovery?
>
> That requires polling the database continuously, which may not be
> possible or desirable.
>
> My main motivation here is to gain the ability to manage a pool of
> connections in asyncpg efficiently.  A part of the connection release
> protocol is "UNLISTEN *;", which the server in Hot Standby would fail to
> process.  Polling the database for pg_is_in_recovery() is not feasible
> in this case, unfortunately.
>

Sorry, i still don't understand the motivation for this.
At one point you're going to poll for the value of the GUC in pg_settings, no?
Or how are you going to know the current value of the GUC that makes it
different to just poll for pg_is_in_recovery()?

-- 
Jaime Casanova  www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, 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] Logical replication launcher's bgworker enabled by default, and max_logical_replication_workers

2017-01-22 Thread Jaime Casanova
On 22 January 2017 at 23:37, Michael Paquier <michael.paqu...@gmail.com> wrote:
> Hi all,
>
> When spawning a new instance, I found the following thing, which is
> surprising at first sight:
> postgres: bgworker: logical replication launcher
>

This is because the downstream needs it
https://www.postgresql.org/message-id/CAMsr%2BYHH2XRUeqWT6pn_X0tFpP5ci7Fsrsn67TDXbFLeMknhBA%40mail.gmail.com

> In the same range of thoughts, it is also surprising to have the
> default value of max_logical_replication_workers set to 4, I would
> have thought that for a feature that has created more than 13k of code
> diffs, it would be disabled by default.
>

+1, we should that to 0 before release

-- 
Jaime Casanova  www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, 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] Logical Replication WIP

2017-01-20 Thread Jaime Casanova
On 20 January 2017 at 11:39, Petr Jelinek <petr.jeli...@2ndquadrant.com> wrote:
> On 20/01/17 17:33, Jaime Casanova wrote:
>>>
>>
>> surely wal_level < logical shouldn't start a logical replication
>> launcher, and after an initdb wal_level is only replica
>>
>
> Launcher is needed for subscriptions, subscriptions don't depend on
> wal_level.
>

mmm... ok, i need to read a little then. thanks

-- 
Jaime Casanova  www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, 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] Logical Replication WIP

2017-01-20 Thread Jaime Casanova
On 20 January 2017 at 11:25, Petr Jelinek <petr.jeli...@2ndquadrant.com> wrote:
> On 20/01/17 17:05, Fujii Masao wrote:
>> On Fri, Jan 20, 2017 at 11:08 PM, Peter Eisentraut
>> <peter.eisentr...@2ndquadrant.com> wrote:
>>> On 1/19/17 5:01 PM, Petr Jelinek wrote:
>>>> There were some conflicting changes committed today so I rebased the
>>>> patch on top of them.
>>>>
>>>> Other than that nothing much has changed, I removed the separate sync
>>>> commit patch, included the rename patch in the patchset and fixed the
>>>> bug around pg_subscription catalog reported by Erik Rijkers.
>>>
>>> Committed.
>>
>> Sorry I've not followed the discussion about logical replication at all, but
>> why does logical replication launcher need to start up by default?
>>
>
> Because running subscriptions is allowed by default. You'd need to set
> max_logical_replication_workers to 0 to disable that.
>

surely wal_level < logical shouldn't start a logical replication
launcher, and after an initdb wal_level is only replica


-- 
Jaime Casanova  www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, 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: Write Amplification Reduction Method (WARM)

2016-12-25 Thread Jaime Casanova
On 2 December 2016 at 07:36, Pavan Deolasee <pavan.deola...@gmail.com> wrote:
>
> I've updated the patches after fixing the issue. Multiple rounds of
> regression passes for me without any issue. Please let me know if it works
> for you.
>

Hi Pavan,

Today i was playing with your patch and running some tests and found
some problems i wanted to report before i forget them ;)

* You need to add a prototype in src/backend/utils/adt/pgstatfuncs.c:
extern Datum pg_stat_get_tuples_warm_updated(PG_FUNCTION_ARGS);

* The isolation test for partial_index fails (attached the regression.diffs)

* running a home-made test i have at hand i got this assertion:
"""
TRAP: FailedAssertion("!(buf_state & (1U << 24))", File: "bufmgr.c", Line: 837)
LOG:  server process (PID 18986) was terminated by signal 6: Aborted
"""
To reproduce:
1) run prepare_test.sql
2) then run the following pgbench command (sql scripts attached):
pgbench -c 24 -j 24 -T 600 -n -f inserts.sql@15 -f updates_1.sql@20 -f
updates_2.sql@20 -f deletes.sql@45 db_test


* sometimes when i have made the server crash the attempt to recovery
fails with this assertion:
"""
LOG:  database system was not properly shut down; automatic recovery in progress
LOG:  redo starts at 0/157F970
TRAP: FailedAssertion("!(!warm_update)", File: "heapam.c", Line: 8924)
LOG:  startup process (PID 14031) was terminated by signal 6: Aborted
LOG:  aborting startup due to startup process failure
"""
still cannot reproduce this one consistently but happens often enough

will continue playing with it...

-- 
Jaime Casanova  www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


regression.diffs
Description: Binary data


deletes.sql
Description: application/sql


inserts.sql
Description: application/sql


prepare_test.sql
Description: application/sql


updates_1.sql
Description: application/sql


updates_2.sql
Description: application/sql

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

2016-11-09 Thread Jaime Casanova
On 16 August 2016 at 09:06, Stephen Frost <sfr...@snowman.net> wrote:
> Greetings,
>
> * Venkata B Nagothi (nag1...@gmail.com) wrote:
>> The above said parameters can be configured to pause, shutdown or prevent
>> promotion only after reaching the recovery target point.
>> To clarify, I am referring to a scenario where recovery target point is not
>> reached at all ( i mean, half-complete or in-complete recovery) and there
>> are lots of WALs still pending to be replayed - in this situation,
>
> PG doesn't know that there are still WALs to be replayed.
>
>> PostgreSQL just completes the archive recovery until the end of the last
>> available WAL (WAL file "0001001E" in my case) and
>> starts-up the cluster by generating an error message (saying
>> "0001001F" not found).
>
> That's not a PG error, that's an error from cp.  From PG's perspective,
> your restore command has said that all of the WAL has been replayed.
>
> If that's not what you want then change your restore command to return
> an exit code > 125, which tells PG that it's unable to restore that WAL
> segment.
>

Ah! Is this documented somewhere?
if we expect people to use correct exit codes we should document them, no?

-- 
Jaime Casanova  www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, 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] Declarative partitioning - another take

2016-11-07 Thread Jaime Casanova
On 7 November 2016 at 12:15, Jaime Casanova
<jaime.casan...@2ndquadrant.com> wrote:
> On 28 October 2016 at 02:53, Amit Langote <langote_amit...@lab.ntt.co.jp> 
> wrote:
>>
>> Please find attached the latest version of the patches
>
> Hi,
>
> I started to review the functionality of this patch, so i applied all
> 9 patches. After that i found this warning, which i guess is because
> it needs a cast.
>

oh! i forgot the warning
"""
partition.c: In function ‘get_qual_for_list’:
partition.c:1159:6: warning: assignment from incompatible pointer type
   or = makeBoolExpr(OR_EXPR, list_make2(nulltest2, opexpr), -1);
  ^
"""

attached a list of the warnings that my compiler give me (basically
most are just variables that could be used uninitialized)

-- 
Jaime Casanova  www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
partition.c: In function ‘get_qual_for_list’:
partition.c:1159:6: warning: assignment from incompatible pointer type
   or = makeBoolExpr(OR_EXPR, list_make2(nulltest2, opexpr), -1);
  ^
partition.c: In function ‘RelationBuildPartitionDesc’:
partition.c:250:8: warning: ‘num_distinct_bounds’ may be used uninitialized in 
this function [-Wmaybe-uninitialized]
  int   num_distinct_bounds;
^
partition.c:587:9: warning: ‘distinct_bounds’ may be used uninitialized in this 
function [-Wmaybe-uninitialized]
 copy_range_bound(key, distinct_bounds[i]);
 ^
partition.c:536:5: warning: ‘all_values_count’ may be used uninitialized in 
this function [-Wmaybe-uninitialized]
 for (i = 0; i < all_values_count; i++)
 ^
partition.c:243:23: warning: ‘all_values’ may be used uninitialized in this 
function [-Wmaybe-uninitialized]
  PartitionListValue **all_values;
   ^
partition.c:619:35: warning: ‘oids’ may be used uninitialized in this function 
[-Wmaybe-uninitialized]
result->oids[mapping[i]] = oids[i];
   ^
partition.c: In function ‘get_qual_from_partbound’:
partition.c:973:10: warning: ‘my_qual’ may be used uninitialized in this 
function [-Wmaybe-uninitialized]
  my_qual = (List *) map_variable_attnos((Node *) my_qual,
  ^
partition.c: In function ‘get_partition_for_tuple’:
partition.c:1734:8: warning: ‘index’ may be used uninitialized in this function 
[-Wmaybe-uninitialized]
 if (node->index > index)
^

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


Re: [HACKERS] Declarative partitioning - another take

2016-11-07 Thread Jaime Casanova
On 28 October 2016 at 02:53, Amit Langote <langote_amit...@lab.ntt.co.jp> wrote:
>
> Please find attached the latest version of the patches

Hi,

I started to review the functionality of this patch, so i applied all
9 patches. After that i found this warning, which i guess is because
it needs a cast.

After that, i tried a case that i think is important: to partition an
already existing table. Because there is no ALTER TABL SET PARTITION
or something similar (which i think makes sense because such a command
would need to create the partitions and move the rows to follow the
rule that there is no rows in a parent table).

So, what i tried was:

1) rename original table
2) create a new partitioned table with old name
3) attach old table as a partition with bounds outside normal bounds
and no validate

the idea is to start attaching valid partitions and delete and insert
rows from the invalid one (is there a better way of doing that?), that
will allow to partition a table easily.
So far so good, until i decided points 1 to 3 should happen inside a
transaction to make things transparent to the user.

Attached is script that shows the failure when trying it:

script 1 (failing_test_1.sql) fails the assert
"Assert(RelationGetPartitionKey(parentRel) != NULL);" in
transformAttachPartition() at src/backend/parser/parse_utilcmd.c:3164

After that i tried the same but with an already partitioned (via
inheritance) table and got this (i did this first without a
transaction, doing it with a transaction will show the same failure as
before):

script 2 (failing_test_2.sql) fails the assert
"Assert(childrte->relkind == RELKIND_PARTITIONED_TABLE);" in
expand_inherited_rte_internal() at
src/backend/optimizer/prep/prepunion.c:1551

PS: i don't like the START END syntax but i don't have any ideas
there... except, there was a reason not to use expressions (like a
CHECK constraint?)

-- 
Jaime Casanova  www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


failing_test_1.sql
Description: application/sql


failing_test_2.sql
Description: application/sql

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


Re: [HACKERS] Is the last 9.1 release planned?

2016-10-04 Thread Jaime Casanova
On 4 October 2016 at 22:00, Tsunakawa, Takayuki
<tsunakawa.ta...@jp.fujitsu.com> wrote:
>
> and 9.0.23 was released in October 8.  So I guessed 9.1.24 will be released 
> in a week or so.
>

Well, no. We normally don't give special treatment to any minor
release not even if it is going to die.
What normally happens is that all minor releases are released the same day.

Taken your example, that same day were released: 9.0.23, 9.1.19,
9.2.14, 9.3.10 and 9.4.5

-- 
Jaime Casanova  www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, 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] autonomous transactions

2016-08-30 Thread Jaime Casanova
On 30 August 2016 at 20:50, Peter Eisentraut
<peter.eisentr...@2ndquadrant.com> wrote:
>
> - Patches to PL/pgSQL to implement Oracle-style autonomous transaction
> blocks:
>
> AS $$
> DECLARE
>   PRAGMA AUTONOMOUS_TRANSACTION;
> BEGIN
>   FOR i IN 0..9 LOOP
> START TRANSACTION;
> INSERT INTO test1 VALUES (i);
> IF i % 2 = 0 THEN
> COMMIT;
> ELSE
> ROLLBACK;
> END IF;
>   END LOOP;
>
>   RETURN 42;
> END;
> $$;
>

this is the syntax it will use?
i just compiled this in head and created a function based on this one.
The main difference is that the column in test1 it's a pk so i used
INSERT ON CONFLICT DO NOTHING

and i'm getting this error

postgres=# select foo();
LOG:  namespace item variable itemno 1, name val
CONTEXT:  PL/pgSQL function foo() line 7 at SQL statement
STATEMENT:  select foo();
ERROR:  null value in column "i" violates not-null constraint
DETAIL:  Failing row contains (null).
STATEMENT:  INSERT INTO test1 VALUES (val) ON CONFLICT DO NOTHING
ERROR:  null value in column "i" violates not-null constraint
DETAIL:  Failing row contains (null).
CONTEXT:  PL/pgSQL function foo() line 7 at SQL statement
STATEMENT:  select foo();
ERROR:  null value in column "i" violates not-null constraint
DETAIL:  Failing row contains (null).
CONTEXT:  PL/pgSQL function foo() line 7 at SQL statement

this happens even everytime i use the PRAGMA even if no START
TRANSACTION, COMMIT or ROLLBACK are used

-- 
Jaime Casanova  www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, 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] autonomous transactions

2016-08-30 Thread Jaime Casanova
On 30 August 2016 at 23:10, Joel Jacobson <j...@trustly.com> wrote:
>
> There should be a way to within the session and/or txn permanently
> block autonomous transactions.
>

This will defeat one of the use cases of autonomous transactions: auditing

>
> Coding conventions, rules and discipline are all good and will help
> against misuse of the feature, but some day someone will make a
> mistake and wrongly use the autonomous transaction and cause unwanted
> unknown side-effect I as a caller function didn't expect or know
> about.
>

well, if the feature is not guilty why do you want to put it in jail?

-- 
Jaime Casanova  www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, 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] to_date_valid()

2016-07-02 Thread Jaime Casanova
El 2/7/2016 20:33, "Euler Taveira" <eu...@timbira.com.br> escribió:
>
> On 02-07-2016 22:04, Andreas 'ads' Scherbaum wrote:
> > The attached patch adds a new function "to_date_valid()" which will
> > validate the date and return an error if the input and output date do
> > not match. Tests included, documentation update as well.
> >
> Why don't you add a third parameter (say, validate = true | false)
> instead of creating another function? The new parameter could default to
> false to not break compatibility.
>

Shouldn't we fix this instead? Sounds like a bug to me. We don't usually
want to be bug compatible so it doesn't matter if we break something.

--
Jaime Casanovahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services


[HACKERS] Rename synchronous_standby_names?

2016-05-31 Thread Jaime Casanova
Hi,

Are we going to change synchronous_standby_names? Certainly the GUC is
not *only* a list of names anymore.

synchronous_standby_config?
synchronous_standbys (adjust to correct english if necesary)?

-- 
Jaime Casanova  www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, 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] 10.0

2016-05-17 Thread Jaime Casanova
On 17 May 2016 at 10:37, David Steele <da...@pgmasters.net> wrote:
> On 5/17/16 10:51 AM, David Fetter wrote:
>
>> On Tue, May 17, 2016 at 01:45:09PM +0800, Craig Ringer wrote:
>>> On 14 May 2016 at 02:49, Tom Lane <t...@sss.pgh.pa.us> wrote:
>>>> * This year's major release will be 9.6.0, with minor updates 9.6.1,
>>>> 9.6.2, etc.  It's too late to do otherwise for this release cycle.
>>>>
>>>> * Next year's major release will be 10.0, with minor updates 10.1,
>>>> 10.2, etc.
>>>>
>>>> * The year after, 11.0.  Etc cetera.
>>>>
>>>>
>>> Yes. Please!
>>
>> Hear, hear!
>>
>> Sadly, we're too late for 9.6, but we can start with 10.0 and finish
>> this silliness once and for good.
>
> +1!
>

+1

-- 
Jaime Casanova  www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, 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] Disabling an index temporarily

2015-12-12 Thread Jaime Casanova
On 11 December 2015 at 22:03, Joshua D. Drake <j...@commandprompt.com> wrote:
> On 12/11/2015 06:25 PM, Tatsuo Ishii wrote:
>
>> What about inventing a new SET command something like:
>>
>> SET disabled_index to 
>>
>> This adds  to "disabled index list". The disabled index
>> list let the planner to disregard the indexes in the list.
>>
>> SET enabled_index to 
>>
>> This removes  from the disabled index list.
>>
>> SHOW disabled_index
>>
>> This shows the content of the disabled index list.
>
>
> Wouldn't something like:
>
> ALTER INDEX foo SET DISABLED;
>
> See more in line with our grammar?
>
> I assume the index is only disabled as far as the planner is concerned and
> all updates/inserts/deletes will still actually update the index
> appropriately?
>

BTW, you can do that today with

UPDATE pg_index SET indisvalid = false
 WHERE indexrelid = 'indexname'::regclass;

-- 
Jaime Casanova  www.2ndQuadrant.com
Professional PostgreSQL: Soporte 24x7 y capacitación


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


[HACKERS] REASSIGN OWNED doesn't know how to deal with USER MAPPINGs

2015-12-10 Thread Jaime Casanova
Hi,

We just notice $SUBJECT. Attached patch fixes it by ignoring USER
MAPPINGs in shdepReassignOwned() just like it happens with default
ACLs.

DROP OWNED manages it well.

-- 
Jaime Casanova  www.2ndQuadrant.com
Professional PostgreSQL: Soporte 24x7 y capacitación
diff --git a/src/backend/catalog/pg_shdepend.c b/src/backend/catalog/pg_shdepend.c
index 43076c9..027bc8b 100644
--- a/src/backend/catalog/pg_shdepend.c
+++ b/src/backend/catalog/pg_shdepend.c
@@ -43,6 +43,7 @@
 #include "catalog/pg_ts_config.h"
 #include "catalog/pg_ts_dict.h"
 #include "catalog/pg_type.h"
+#include "catalog/pg_user_mapping.h"
 #include "commands/alter.h"
 #include "commands/dbcommands.h"
 #include "commands/collationcmds.h"
@@ -1384,6 +1385,13 @@ shdepReassignOwned(List *roleids, Oid newrole)
 	AlterForeignDataWrapperOwner_oid(sdepForm->objid, newrole);
 	break;
 
+case UserMappingRelationId:
+
+	/* Ignore USER MAPPINGs; they should be handled by DROP
+	 * OWNED, not REASSIGN OWNED.
+	 */
+	break;
+
 case EventTriggerRelationId:
 	AlterEventTriggerOwner_oid(sdepForm->objid, newrole);
 	break;

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


Re: [HACKERS] REASSIGN OWNED doesn't know how to deal with USER MAPPINGs

2015-12-10 Thread Jaime Casanova
On 10 December 2015 at 13:04, Jaime Casanova
<jaime.casan...@2ndquadrant.com> wrote:
> Hi,
>
> We just notice $SUBJECT. Attached patch fixes it by ignoring USER
> MAPPINGs in shdepReassignOwned() just like it happens with default
> ACLs.
>

BTW, shouldn't we at least give a warning on those cases instead of
asuming that the user will know that some objects were ignored?

-- 
Jaime Casanova  www.2ndQuadrant.com
Professional PostgreSQL: Soporte 24x7 y capacitación


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


Re: [HACKERS] GIN pending list clean up exposure to SQL

2015-11-21 Thread Jaime Casanova
On 21 November 2015 at 03:54, Jim Nasby <jim.na...@bluetreble.com> wrote:
> On 11/19/15 10:47 AM, Jaime Casanova wrote:
>>
>> - only superusers?
>
>
> I would think the owner of the table (index?) should also be able to run
> this.

agreed, that makes sense

-- 
Jaime Casanova  www.2ndQuadrant.com
Professional PostgreSQL: Soporte 24x7 y capacitación


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


Re: [HACKERS] GIN pending list clean up exposure to SQL

2015-11-20 Thread Jaime Casanova
On 19 November 2015 at 14:57, Jaime Casanova
<jaime.casan...@2ndquadrant.com> wrote:
> On 19 November 2015 at 14:47, Jaime Casanova
> <jaime.casan...@2ndquadrant.com> wrote:
>> On 19 November 2015 at 14:18, Alvaro Herrera <alvhe...@2ndquadrant.com> 
>> wrote:
>>> Alvaro Herrera wrote:
>>>> Jeff Janes wrote:
>>>> > I've written a function which allows users to clean up the pending list.
>>>> > It takes the index name and returns the number of pending list pages
>>>> > deleted.
>>>>
>>>> I just noticed that your patch uses AccessShareLock on the index.  Is
>>>> that okay?  I would have assumed that you'd need ShareUpdateExclusive
>>>> (same as vacuum uses), but I don't really know.  Was that a carefully
>>>> thought-out choice?
>>>
>>> After reading gitPendingCleanup it becomes clear that there's no need
>>> for a stronger lock than what you've chosen.  Jaime Casanova just
>>> pointed this out to me.
>>>
>>
>> But it should do some checks, no?
>> - only superusers?
>> - what i received as parameter is a GIN index?
>>
>
> I just notice this:
>
> +   ginInsertCleanup(, true, );
>
> ginInsertCleanup() now has four parameters, so you should update the call
>

Btw, this is not in the commitfest and seems like a useful thing to have

-- 
Jaime Casanova  www.2ndQuadrant.com
Professional PostgreSQL: Soporte 24x7 y capacitación


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


Re: [HACKERS] GIN pending list clean up exposure to SQL

2015-11-19 Thread Jaime Casanova
On 12 August 2015 at 20:19, Jeff Janes <jeff.ja...@gmail.com> wrote:
>
> But where does this belong?  Core?  Its own separate extension?
>

I will say core. Gin indexes are in core and i don't see why this
function shouldn't.
FWIW, brin indexes has a similar function brin_summarize_new_values() in core

-- 
Jaime Casanova  www.2ndQuadrant.com
Professional PostgreSQL: Soporte 24x7 y capacitación


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


Re: [HACKERS] GIN pending list clean up exposure to SQL

2015-11-19 Thread Jaime Casanova
On 19 November 2015 at 14:18, Alvaro Herrera <alvhe...@2ndquadrant.com> wrote:
> Alvaro Herrera wrote:
>> Jeff Janes wrote:
>> > I've written a function which allows users to clean up the pending list.
>> > It takes the index name and returns the number of pending list pages
>> > deleted.
>>
>> I just noticed that your patch uses AccessShareLock on the index.  Is
>> that okay?  I would have assumed that you'd need ShareUpdateExclusive
>> (same as vacuum uses), but I don't really know.  Was that a carefully
>> thought-out choice?
>
> After reading gitPendingCleanup it becomes clear that there's no need
> for a stronger lock than what you've chosen.  Jaime Casanova just
> pointed this out to me.
>

But it should do some checks, no?
- only superusers?
- what i received as parameter is a GIN index?

-- 
Jaime Casanova  www.2ndQuadrant.com
Professional PostgreSQL: Soporte 24x7 y capacitación


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


Re: [HACKERS] GIN pending list clean up exposure to SQL

2015-11-19 Thread Jaime Casanova
On 19 November 2015 at 14:47, Jaime Casanova
<jaime.casan...@2ndquadrant.com> wrote:
> On 19 November 2015 at 14:18, Alvaro Herrera <alvhe...@2ndquadrant.com> wrote:
>> Alvaro Herrera wrote:
>>> Jeff Janes wrote:
>>> > I've written a function which allows users to clean up the pending list.
>>> > It takes the index name and returns the number of pending list pages
>>> > deleted.
>>>
>>> I just noticed that your patch uses AccessShareLock on the index.  Is
>>> that okay?  I would have assumed that you'd need ShareUpdateExclusive
>>> (same as vacuum uses), but I don't really know.  Was that a carefully
>>> thought-out choice?
>>
>> After reading gitPendingCleanup it becomes clear that there's no need
>> for a stronger lock than what you've chosen.  Jaime Casanova just
>> pointed this out to me.
>>
>
> But it should do some checks, no?
> - only superusers?
> - what i received as parameter is a GIN index?
>

I just notice this:

+   ginInsertCleanup(, true, );

ginInsertCleanup() now has four parameters, so you should update the call

-- 
Jaime Casanova  www.2ndQuadrant.com
Professional PostgreSQL: Soporte 24x7 y capacitación


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


Re: [HACKERS] bugs and bug tracking

2015-10-06 Thread Jaime Casanova
On 6 October 2015 at 12:32, Nathan Wagner <nw...@hydaspes.if.org> wrote:
>
> Also, the version numbers are user reported and a bit of a mess.  I
> don't think they could really be relied on as is for users trying to
> find out if a bug affects their version.  Someone would have to update
> that information, and communicate that update to the tracker.  The
> general concensus seems to be that magic phrases at the beginning of a
> line in a message body is the way to go.  I don't necessarily agree, but
> any consistent system can be made to work.  This obviously applies to
> any metadata, not just version numbers.
>

i also don't agree that everything will happen by magic...

for example, bug # 6150 (https://granicus.if.org/pgbugs/6150) was
reported for PostgreSQL version: 0.9
so we need a way to, at least, fix those.

also, while the bug report allow you to say which OS was affected that
is the one the user was using. still the bug could happen in all OSes
(versions?), only some, only a specifi OS in a specific version is
affected.

so we need, to have an interface to fix metada... and people taking
responsability for that

-- 
Jaime Casanova  www.2ndQuadrant.com
Professional PostgreSQL: Soporte 24x7 y capacitación


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


Re: [HACKERS] bugs and bug tracking

2015-10-06 Thread Jaime Casanova
On 6 October 2015 at 08:05, Nathan Wagner <nw...@hydaspes.if.org> wrote:
> So, in order to do some clean up and see how my pgbugs page
> (https://granicus.if.org/pgbugs/) might actually work I've been going
> through bugs and marking their status.  A lot of questions arise.
>

/* DISCLAIMER */

My opinion is not important in this issue

/* END DISCLAIMER */

I like how this page is looking now, good work.
Now, AFAIU from previous mails part of the reason to have a bug
tracker is to make easy to know when a bug was fixed. Which should be
interpreted as: which versions this bug affected? and which minor
versions on those branches fix the issue

for example bug # 13636 was reported for 9.4.4 but it existed in older
branches so Tom fixed it in all active branches (ie:
http://www.postgresql.org/message-id/e1zfjgx-0005lu...@gemulon.postgresql.org).
is it possible (even if not yet implemented) to add that information?

also i like that we can search on bugs but we can filter by version.
i'm just guessing that if someone looks for a bug he's probably
worrying about the specific version he is using.

having a bug tracker that allow us to not lose track of bugs is good,
having a bug tracker that allow us to have the whole information on a
bug is better, IMHO.

> A lot of the reports aren't bugs at all, but requests for help.  My
> guess is that the users either don't know where to ask or don't
> understand the difference between a bug and not knowing how to do what
> they want to do.  Perhaps a more thorough explaination on the submission
> form would be useful.
>

the real problem here is that fill the bug report doesn't force you to
register in a ML, while asking for help in a ML will. and a lot of
people don't want to register in a ML, they just want a specific
question answered so i don't think any change in the form will avoid
that.

-- 
Jaime Casanova  www.2ndQuadrant.com
Professional PostgreSQL: Soporte 24x7 y capacitación


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


Re: [HACKERS] synchronous_commit = apply

2015-09-01 Thread Jaime Casanova
On 1 September 2015 at 20:25, Thomas Munro
<thomas.mu...@enterprisedb.com> wrote:
> Hi
>
> Do you think it's reasonable to want to COMMIT a particular transaction on a
> master node, and then immediately run a read-only query on a hot standby
> node that is guaranteed to see that transaction?
>

well, that is important to make load balancing completely safe (not
returning old data when is important to get the latest).
Having said that, i have never seen a case where the apply lag
postgres has really matters or where the cause of the apply lag (I/O)
doesn't get worst if we try to apply immediatly.

Other solutions use a cache on top to apply in-memory at the cost of
getting inconsistent in a failure.

> A friend of mine who works with a different RDBMS technology that can do
> that asked me how to achieve this with Postgres, and I suggested waiting for
> the standby's pg_last_xlog_replay_location() to be >= the master's
> pg_current_xlog_location() after COMMIT, which might involve some looping
> and sleeping.
>
> As a quick weekend learning exercise/hack I recently went looking into how
> we could support $SUBJECT.  I discovered we already report the apply
> progress back to the master, and the synchronous waiting facility seemed to
> be all ready to support this.  In fact it seemed a little too easy so
> something tells me it must be wrong!  But anyway, please see the attached
> toy POC patch which does that.
>

i haven't seen the patch, but probably is as easy as you see it...
IIRC, Simon proposed a patch for this a few years ago and this was
actually contempleted from the beggining in the design of SR.

I guess there were good reasons the patch didn't get applied, i found
this thread and in this one Simon suggest is not the first time he
submitted that option so it should be other threads too:
http://www.postgresql.org/message-id/aanlktinxoymwowbsjxmnpjhjh_yan9vfmnmhnjdme...@mail.gmail.com

-- 
Jaime Casanova  www.2ndQuadrant.com
Professional PostgreSQL: Soporte 24x7 y capacitación


-- 
Sent 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: Implement failover on libpq connect level.

2015-08-18 Thread Jaime Casanova
On 17 August 2015 at 23:18, Victor Wagner vi...@wagner.pp.ru wrote:

 Rationale
 =

 Since introduction of the WAL-based replication into the PostgreSQL, it is
 possible to create high-availability and load-balancing clusters.

 However, there is no support for failover in the client libraries. So, only
 way to provide transparent for client application failover is IP address
 migration. This approach has some limitation, i.e. it requires that
 master and backup servers reside in the same subnet or may not be
 feasible for other reasons.


This is not completely true, you can always use something like
pgbouncer or other middleware to change the server to which clients
connect. you still need to solve the fact that you will have a
read-only server at the other side.

something like repmgr + pgbouncer will work fine.

i agree that once/if we ever have multimaster included then this could
be a good idea

-- 
Jaime Casanova  www.2ndQuadrant.com
Professional PostgreSQL: Soporte 24x7 y capacitación


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


Re: [HACKERS] Improving GEQO

2015-05-20 Thread Jaime Casanova
On Wed, May 20, 2015 at 1:06 PM, alejandro b...@uclv.cu wrote:
 hello, my partner and me are working with the goal of improve the GEQO's
 performance, we tried with Ant Colony Optimization, but it does not improve,
 actually we are trying with a new variant of Genetic Algorithm, specifically
 Micro-GA. This algorithm finds a better solution than GEQO in less time,
 however the total query execution time is higher. The fitness is calculated
 by geqo_eval function. Does anybody know why this happens?


It will be difficult for anyone here to figure out anything without
the code to look at

-- 
Jaime Casanova www.2ndQuadrant.com
Professional PostgreSQL: Soporte 24x7 y capacitación


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


Re: [HACKERS] Remove fsync ON/OFF as a visible option?

2015-03-21 Thread Jaime Casanova
On Fri, Mar 20, 2015 at 11:29 PM, Michael Paquier
michael.paqu...@gmail.com wrote:
 On Sat, Mar 21, 2015 at 2:47 AM, Peter Geoghegan p...@heroku.com wrote:
 On Fri, Mar 20, 2015 at 9:52 AM, Joshua D. Drake j...@commandprompt.com 
 wrote:
 There are just as many people that are running with scissors that are now
 running (or attempting to run) our elephant in production. Does it make
 sense to remove fsync (and possibly full_page_writes) from such a visible
 place as postgresql.conf?

 -1

 Anyone turning off fsync without even for a moment considering the
 consequences has only themselves to blame. I can't imagine why you'd
 want to remove full_page_writes or make it less visible either, since
 in principle it ought to be perfectly fine to turn it off in
 production once its verified as safe.

 -1 for its removal as well. It is still useful for developers to
 emulate CPU-bounded loads...

I fought to remove fsync before so i understand JD concerns. and yes,
i have seen fsync=off in the field too...

what about not removing it but not showing it in postgresql.conf? as a
side note, i wonder why trace_sort is not in postgresql.conf...
other option is to make it a compile setting, that why if you want to
have it you need to compile and postgres' developers do that routinely
anyway

just my 2c

-- 
Jaime Casanova www.2ndQuadrant.com
Professional PostgreSQL: Soporte 24x7 y capacitación


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


Re: [HACKERS] Remove fsync ON/OFF as a visible option?

2015-03-21 Thread Jaime Casanova
El mar 21, 2015 2:00 AM, Mark Kirkwood mark.kirkw...@catalyst.net.nz
escribió:

 On 21/03/15 19:28, Jaime Casanova wrote:

 what about not removing it but not showing it in postgresql.conf? as a
 side note, i wonder why trace_sort is not in postgresql.conf...
 other option is to make it a compile setting, that why if you want to
 have it you need to compile and postgres' developers do that routinely
 anyway


 -1

 Personally I'm against hiding *any* settings. Choosing sensible defaults
- yes! Hiding them - that reeks of secret squirrel nonsense and overpaid
Oracle dbas that knew the undocumented settings for various capabilities. I
think/hope that no open source project will try to emulate that meme!


That ship has already sailed.

http://www.postgresql.org/docs/9.4/static/runtime-config-developer.html

--
Jaime Casanova
2ndQuadrant Consultant
Your PostgreSQL partner


Re: [HACKERS] Remove fsync ON/OFF as a visible option?

2015-03-21 Thread Jaime Casanova
On Sat, Mar 21, 2015 at 2:33 PM, Joshua D. Drake j...@commandprompt.com wrote:

 On 03/20/2015 11:28 PM, Jaime Casanova wrote:



 I fought to remove fsync before so i understand JD concerns. and yes,
 i have seen fsync=off in the field too...

 what about not removing it but not showing it in postgresql.conf? as a
 side note, i wonder why trace_sort is not in postgresql.conf...


 That is the original proposal. I am not suggesting that it not be an option.
 I am suggesting that it is not in postgresql.conf by default.



you're right, i misunderstood... anyway i don't feel there's a need to
avoid people putting in there...
just don't ship with the guc in there, if someone puts it by himself
that's completely another thing

-- 
Jaime Casanova www.2ndQuadrant.com
Professional PostgreSQL: Soporte 24x7 y capacitación


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


Re: [HACKERS] pgaudit - an auditing extension for PostgreSQL

2014-12-25 Thread Jaime Casanova
On Thu, Dec 25, 2014 at 5:42 AM, Abhijit Menon-Sen a...@2ndquadrant.com wrote:

 To summarise for people who haven't followed the thread in detail, the
 idea is that you would do:

 grant select on foo to audit;

 ...and the server would audit-log any select ... from foo ... queries (by
 any user).

what if i want to audit different things for different users?

-- 
Jaime Casanova www.2ndQuadrant.com
Professional PostgreSQL: Soporte 24x7 y capacitación
Phone: +593 4 5107566 Cell: +593 987171157


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


Re: [HACKERS] TABLESAMPLE patch

2014-12-22 Thread Jaime Casanova
On Thu, Dec 18, 2014 at 7:14 AM, Petr Jelinek p...@2ndquadrant.com wrote:
 Hi,

 v2 version of this patch is attached.


a few more tests revealed that passing null as the sample size
argument works, and it shouldn't.
in repeatable it gives an error if i use null as argument but it gives
a syntax error, and it should be a data exception (data exception --
invalid repeat argument in a sample clause) according to the standard

also you need to add CHECK_FOR_INTERRUPTS somewhere, i tried with a
big table and had to wait a long time for it to finish


regression=# select count(1) from tenk1 tablesample system (null);
 count
---
28
(1 row)

regression=# select count(1) from tenk1 tablesample bernoulli (null);
 count
---
 0
(1 row)


-- 
Jaime Casanova www.2ndQuadrant.com
Professional PostgreSQL: Soporte 24x7 y capacitación
Phone: +593 4 5107566 Cell: +593 987171157


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


Re: [HACKERS] Commitfest problems

2014-12-16 Thread Jaime Casanova
On Tue, Dec 16, 2014 at 11:32 AM, Robert Haas robertmh...@gmail.com wrote:

 It has been proposed that we do a general list of people at the bottom
 of the release notes who helped review during that cycle.  That would
 be less intrusive and possibly a good idea, but would we credit the
 people who did a TON of reviewing?  Everyone who reviewed even one
 patch?  Somewhere in between? Would committers be excluded because we
 just expect them to help or included because credit is important to
 established community members too?  To what extent would this be
 duplicative of http://www.postgresql.org/community/contributors/ ?

Not much really, I tried to get my name on that list a couple of years
ago, when i reviewed more than i do now, and never got it.
And while my name is in a couple commit messages, that is a lot harder
to show to people...

you know, it's kind of frustrating when some not-yet customers ask for
certificated engineers, and there isn't any official (as in from
community) certificate so you need to prove you're a contributor so
let's see this random commit messages...

-- 
Jaime Casanova www.2ndQuadrant.com
Professional PostgreSQL: Soporte 24x7 y capacitación
Phone: +593 4 5107566 Cell: +593 987171157


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


Re: [HACKERS] TABLESAMPLE patch

2014-12-15 Thread Jaime Casanova
On Wed, Dec 10, 2014 at 6:24 PM, Petr Jelinek p...@2ndquadrant.com wrote:
 Hello,

 Attached is a basic implementation of TABLESAMPLE clause. It's SQL standard
 clause and couple of people tried to submit it before so I think I don't
 need to explain in length what it does - basically returns random sample
 of a table using a specified sampling method.


Tablesample, yay!

Sadly when the jsonb functions patch was committed a few oids where
used, so you should update the ones you are using. at least to make
the patch easier for testing.

The test added for this failed, attached is the diff. i didn't looked
up why it failed

Finally, i created a view with a tablesample clause. i used the view
and the tablesample worked, then dumped and restored and the
tablesample clause went away... actually pg_get_viewdef() didn't see
it at all.

will look at the patch more close tomorrow when my brain wake up ;)

-- 
Jaime Casanova www.2ndQuadrant.com
Professional PostgreSQL: Soporte 24x7 y capacitación
Phone: +593 4 5107566 Cell: +593 987171157


regression.diffs
Description: Binary data

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


Re: [HACKERS] Turning recovery.conf into GUCs

2014-11-24 Thread Jaime Casanova
On Fri, Nov 21, 2014 at 12:55 PM, Josh Berkus j...@agliodbs.com wrote:
 On 11/21/2014 09:35 AM, Alex Shulgin wrote:
 Hello,

 Here's an attempt to revive this patch.

 Yayy!  Thank you.

 People might not like me for the suggestion, but I think we should
 simply always include a 'recovery.conf' in $PGDATA
 unconditionally. That'd make this easier.
 Alternatively we could pass a filename to --write-recovery-conf.

 Well, the latest version of this patch fails to start when it sees
 'recovery.conf' in PGDATA:

   FATAL:  recovery.conf is not supported anymore as a recovery method
   DETAIL:  Refer to appropriate documentation about migration methods

 I've missed all the discussion behind this decision and after reading
 the ALTER SYSTEM/conf.d thread I'm even more confused so I'd like
 someone more knowledgeable to speak up on the status of this.

 The argument was that people wanted to be able to have an empty
 recovery.conf file as a we're in standby trigger so that they could
 preserve backwards compatibility with external tools.  I don't agree
 with this argument, but several people championed it.


well, my approach was that postgres just ignore the file completely. I
mean, recovery.conf will no longer mean anything special.
Then, every tool that create recovery.conf in $PGDATA only has to add
an ALTER SYSTEM to include it

 The docs use the term continuous recovery.

 Either way, from the code it is clear that we only stay in recovery if
 standby_mode is directly turned on.  This makes the whole check for a
 specially named file unnecessary, IMO: we should just check the value of
 standby_mode (which is off by default).


no. currently we enter in recovery mode when postgres see a
recovery.conf and stays in recovery mode when standby_mode is on or an
appropiate restore_command is provided.

which means recovery.conf has two uses:
1) start in recovery mode (not continuous)
2) provide parameters for recovery mode and for streaming

we still need a recovery trigger file that forces postgres to start
in recovery mode and acts accordingly to recovery GUCs

 So, what happens when someone does pg_ctl promote?  Somehow
 standby_mode needs to get set to off.  Maybe we write standby_mode =
 off to postgresql.auto.conf?


we need to delete or rename the recovery trigger file, all standby
GUCs are ignored (and recovery GUCs should be ignored too) unless
you're in recovery mode

 By the way, is there any use in setting standby_mode=on and any of the
 recovery_target* GUCs at the same time?

 See my thread on this topic from last week.  Short answer: No.


haven't read that thread, will do


  /* File path names (all relative to $PGDATA) */
 -#define RECOVERY_COMMAND_FILE  recovery.conf
 -#define RECOVERY_COMMAND_DONE  recovery.done
 +#define RECOVERY_ENABLE_FILE   standby.enabled

 Imo the file and variable names should stay coherent.

 Yes, once we settle on the name (and if we really need that extra
 trigger file.)


yes, we need it. but other names were suggested standby.enabled
transmit the wrong idea

 Personally, if we have three methods of promotion:

 1) pg_ctl promote
 2) edit postgresql.conf and reload
 3) ALTER SYSTEM SET and reload

 ... I don't honestly think we need a 4th method for promotion.


this is not for promotion, this is to force postgres to start in
recovery mode and read recovery configuration parameters.

 The main reason to want a we're in recovery file is for PITR rather
 than for replication, where it has a number of advantages as a method,
 the main one being that recovery.conf is unlikely to be overwritten by
 the contents of the backup.


only that you need to start in recovery mode to start replication

-- 
Jaime Casanova www.2ndQuadrant.com
Professional PostgreSQL: Soporte 24x7 y capacitación
Phone: +593 4 5107566 Cell: +593 987171157


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


Re: [HACKERS] Turning recovery.conf into GUCs

2014-11-24 Thread Jaime Casanova
On Mon, Nov 24, 2014 at 12:24 PM, Jaime Casanova ja...@2ndquadrant.com wrote:
 On Fri, Nov 21, 2014 at 12:55 PM, Josh Berkus j...@agliodbs.com wrote:
 On 11/21/2014 09:35 AM, Alex Shulgin wrote:
 Hello,

 Here's an attempt to revive this patch.

 Yayy!  Thank you.

 People might not like me for the suggestion, but I think we should
 simply always include a 'recovery.conf' in $PGDATA
 unconditionally. That'd make this easier.
 Alternatively we could pass a filename to --write-recovery-conf.

 Well, the latest version of this patch fails to start when it sees
 'recovery.conf' in PGDATA:

   FATAL:  recovery.conf is not supported anymore as a recovery method
   DETAIL:  Refer to appropriate documentation about migration methods

 I've missed all the discussion behind this decision and after reading
 the ALTER SYSTEM/conf.d thread I'm even more confused so I'd like
 someone more knowledgeable to speak up on the status of this.

 The argument was that people wanted to be able to have an empty
 recovery.conf file as a we're in standby trigger so that they could
 preserve backwards compatibility with external tools.  I don't agree
 with this argument, but several people championed it.


 well, my approach was that postgres just ignore the file completely. I
 mean, recovery.conf will no longer mean anything special.
 Then, every tool that create recovery.conf in $PGDATA only has to add
 an ALTER SYSTEM to include it


i mean ALTER SYSTEM in master before copying or just add the line in
postgresql.conf

but, as the patch shows agreement was to break backwards compatibility
and fail to start if the file is present

-- 
Jaime Casanova www.2ndQuadrant.com
Professional PostgreSQL: Soporte 24x7 y capacitación
Phone: +593 4 5107566 Cell: +593 987171157


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


Re: [HACKERS] Amazon Redshift

2014-11-05 Thread Jaime Casanova
On Wed, Nov 5, 2014 at 5:36 PM, philip taylor philta...@mail.com wrote:
 do you think we should implement some of the functions offered by
 Amazon Redshift?

 http://docs.aws.amazon.com/redshift/latest/dg/c_SQL_functions.html


 JSON Functions

 http://docs.aws.amazon.com/redshift/latest/dg/JSON_ARRAY_LENGTH.html
 http://docs.aws.amazon.com/redshift/latest/dg/JSON_EXTRACT_PATH_TEXT.html


We actually have the two above:
http://www.postgresql.org/docs/current/static/functions-json.html
Actually, if you look at the examples you will see they only copy ours
and changed the values a little.

-- 
Jaime Casanova www.2ndQuadrant.com
Professional PostgreSQL: Soporte 24x7 y capacitación
Phone: +593 4 5107566 Cell: +593 987171157


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


Re: [HACKERS] Let's drop two obsolete features which are bear-traps for novices

2014-11-02 Thread Jaime Casanova
El nov 2, 2014 7:54 AM, Andres Freund and...@2ndquadrant.com escribió:

 On 2014-11-01 16:59:35 -0700, Josh Berkus wrote:
  All,
 
  While there's argument about hash indexes, it looks like nobody minds if
  the MONEY type goes bye-bye.  So, time for a patch ...

 FWIW there have been somewhat recent patches for money and it was
 undeprecated not too long ago. So apparently there's users for it out
 there. I personally would never use money, but I doubt the gain is worth
 the cost of breaking existing setups without a warning period.


Not knowing how difficult it could be maybe a fair compromise is to move
MONEY datatype to a contrib. And documenting its limitations.

--
Jaime Casanova
2ndQuadrant Consultant
PostgreSQL Development, 24x7 Support, Training  Services


Re: [HACKERS] Shared Data Structure b/w clients

2014-07-22 Thread Jaime Casanova
On Tue, Jul 22, 2014 at 1:33 PM, Rohit Goyal rhtgyl...@gmail.com wrote:
 Hi Atri/All,

 I am very new in postgresql code. Can you please help in a bit detail ortel
 me how to create structure in shared memory(shared buffer).

 It would be really easy for me if you can give me a code snippet or any link
 to follow.


you can look at contrib/pg_stat_statements

-- 
Jaime Casanova www.2ndQuadrant.com
Professional PostgreSQL: Soporte 24x7 y capacitación
Phone: +593 4 5107566 Cell: +593 987171157


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


Re: [HACKERS] Minmax indexes

2014-07-10 Thread Jaime Casanova
On Thu, Jul 10, 2014 at 3:50 PM, Josh Berkus j...@agliodbs.com wrote:
 On 07/10/2014 12:20 PM, Alvaro Herrera wrote:
 So I guess the only thing left is to issue a NOTICE when said alter
  takes place (I don't see that on the patch, but maybe it's there?)
 That's not in the patch.  I don't think we have an appropriate place to
 emit such a notice.

 What do you mean by don't have an appropriate place?

 The suggestion is that when a user does:

 ALTER INDEX foo_minmax SET PAGES_PER_RANGE=100

 they should get a NOTICE:

 NOTICE: changes to pages per range will not take effect until the index
 is REINDEXed

 otherwise, we're going to get a lot of I Altered the pages per range,
 but performance didn't change emails.


How is this different from ALTER TABLE foo SET (FILLFACTOR=80);  or
from ALTER TABLE foo ALTER bar SET STORAGE EXTERNAL;  ?

we don't get a notice for these cases either

-- 
Jaime Casanova www.2ndQuadrant.com
Professional PostgreSQL: Soporte 24x7 y capacitación
Phone: +593 4 5107566 Cell: +593 987171157


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


Re: [HACKERS] view reloptions

2014-06-13 Thread Jaime Casanova
On Wed, Jun 11, 2014 at 2:46 PM, Alvaro Herrera
alvhe...@2ndquadrant.com wrote:
 I just noticed by chance that view relations are using StdRdOptions to
 allocate their reloptions.  I can't find any reason for this, other than
 someone failed to realize that they should instead have a struct defined
 of its own, just like (say) GIN indexes do.  Views using StdRdOptions is
 quite pointless, given that it's used for stuff like fillfactor and
 autovacuum, neither of which apply to views.

 9.2 added security_barrier to StdRdOptions, and 9.4 is now adding
 check_option_offset, which is a string reloption with some funny rules.

[...]
 2) it would mean that security_barrier would change for external code
 that expects StdRdOptions rather than, say, ViewOptions
 3) I don't have time to do the legwork before CF1 anyway

 If we don't change it now, I'm afraid we'll be stuck with using
 StdRdOptions for views for all eternity.

 (It's a pity I didn't become aware of this earlier in 9.4 cycle when I
 added the multixact freeze reloptions ... I could have promoted a patch
 back then.)


Attached is a patch moving the reloptions of views into its own structure.
i also created a view_reloptions() function in order to not use
heap_reloptions() for views, but maybe that was too much?

i haven't seen the check_option_offset thingy yet, but i hope to take
a look at that tomorrow.

-- 
Jaime Casanova www.2ndQuadrant.com
Professional PostgreSQL: Soporte 24x7 y capacitación
Phone: +593 4 5107566 Cell: +593 987171157
From 87ed78a4f276484a37917c417286a16082030f13 Mon Sep 17 00:00:00 2001
From: Jaime Casanova ja...@2ndquadrant.com
Date: Fri, 13 Jun 2014 01:10:24 -0500
Subject: [PATCH] Move reloptions from views into its own structure.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Per gripe from Álvaro Herrera.
---
 src/backend/access/common/reloptions.c |   42 ++-
 src/backend/commands/tablecmds.c   |9 +-
 src/include/access/reloptions.h|1 +
 src/include/utils/rel.h|   43 
 src/tools/pgindent/typedefs.list   |1 +
 5 files changed, 71 insertions(+), 25 deletions(-)

diff --git a/src/backend/access/common/reloptions.c b/src/backend/access/common/reloptions.c
index 522b671..c7ad6f9 100644
--- a/src/backend/access/common/reloptions.c
+++ b/src/backend/access/common/reloptions.c
@@ -834,10 +834,12 @@ extractRelOptions(HeapTuple tuple, TupleDesc tupdesc, Oid amoptions)
 	{
 		case RELKIND_RELATION:
 		case RELKIND_TOASTVALUE:
-		case RELKIND_VIEW:
 		case RELKIND_MATVIEW:
 			options = heap_reloptions(classForm-relkind, datum, false);
 			break;
+		case RELKIND_VIEW:
+			options = view_reloptions(datum, false);
+			break;
 		case RELKIND_INDEX:
 			options = index_reloptions(amoptions, datum, false);
 			break;
@@ -1200,10 +1202,6 @@ default_reloptions(Datum reloptions, bool validate, relopt_kind kind)
 		offsetof(StdRdOptions, autovacuum) +offsetof(AutoVacOpts, vacuum_scale_factor)},
 		{autovacuum_analyze_scale_factor, RELOPT_TYPE_REAL,
 		offsetof(StdRdOptions, autovacuum) +offsetof(AutoVacOpts, analyze_scale_factor)},
-		{security_barrier, RELOPT_TYPE_BOOL,
-		offsetof(StdRdOptions, security_barrier)},
-		{check_option, RELOPT_TYPE_STRING,
-		offsetof(StdRdOptions, check_option_offset)},
 		{user_catalog_table, RELOPT_TYPE_BOOL,
 		offsetof(StdRdOptions, user_catalog_table)}
 	};
@@ -1225,6 +1223,38 @@ default_reloptions(Datum reloptions, bool validate, relopt_kind kind)
 }
 
 /*
+ * Option parser for views
+ */
+bytea *
+view_reloptions(Datum reloptions, bool validate)
+{
+	relopt_value *options;
+	ViewOptions *vopts;
+	int			numoptions;
+	static const relopt_parse_elt tab[] = {
+		{security_barrier, RELOPT_TYPE_BOOL,
+		offsetof(ViewOptions, security_barrier)},
+		{check_option, RELOPT_TYPE_STRING,
+		offsetof(ViewOptions, check_option_offset)}
+	};
+
+	options = parseRelOptions(reloptions, validate, RELOPT_KIND_VIEW, numoptions);
+
+	/* if none set, we're done */
+	if (numoptions == 0)
+		return NULL;
+
+	vopts = allocateReloptStruct(sizeof(ViewOptions), options, numoptions);
+
+	fillRelOptions((void *) vopts, sizeof(ViewOptions), options, numoptions,
+   validate, tab, lengthof(tab));
+
+	pfree(options);
+
+	return (bytea *) vopts;
+}
+
+/*
  * Parse options for heaps, views and toast tables.
  */
 bytea *
@@ -1248,8 +1278,6 @@ heap_reloptions(char relkind, Datum reloptions, bool validate)
 		case RELKIND_RELATION:
 		case RELKIND_MATVIEW:
 			return default_reloptions(reloptions, validate, RELOPT_KIND_HEAP);
-		case RELKIND_VIEW:
-			return default_reloptions(reloptions, validate, RELOPT_KIND_VIEW);
 		default:
 			/* other relkinds are not supported */
 			return NULL;
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 341262b..fd27cdb 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c

Re: [HACKERS] Providing catalog view to pg_hba.conf file - Patch submission

2014-06-07 Thread Jaime Casanova
On Fri, Mar 14, 2014 at 12:30 AM, Prabakaran, Vaishnavi
vaishna...@fast.au.fujitsu.com wrote:
 Hi,

 In connection to my previous proposal about providing catalog view to
 pg_hba.conf file contents , I have developed the attached patch .

[...]

 [What this Patch does]

 Functionality of the attached patch is that it will provide a new view
 pg_hba_settings to admin users. Public access to the view is restricted.
 This view will display basic information about HBA setting details of
 postgresql cluster.  Information to be shown , is taken from parsed hba
 lines and not directly read from pg_hba.conf files. Documentation files are
 also updated to include details of this new view under Chapter 47.System
 Catalogs. Also , a new note is added in chapter 19.1 The pg_hba.conf File


A normal user can see all the info the view provide once you GRANT
permissions on it. How much info should a non-superuser see from this
view? currently a non-superuser can't see pg_hba info, now it can.

This function should be superuser only or only show info related for
current_user if it user is not superuser.

Also, i think you should use lowercase values just they are in
pg_hba.conf (ie: local not Local, host not Host, etc)

-- 
Jaime Casanova www.2ndQuadrant.com
Professional PostgreSQL: Soporte 24x7 y capacitación
Phone: +593 4 5107566 Cell: +593 987171157


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


Re: [HACKERS] SQL access to database attributes

2014-05-23 Thread Jaime Casanova
On Fri, May 23, 2014 at 10:53 PM, Vik Fearing vik.fear...@dalibo.com wrote:

 It was suggested to me that these options should either error out if
 there are existing connections or terminate said connections.  I don't
 agree with that because there is no harm in connecting to a template
 database (how else do you modify it?), and adding a reject rule in
 pg_hba.conf doesn't disconnect existing users so why should turning off
 ALLOW CONNECTIONS do it?


Which lead us to the question: you need to connect to the database to
modify it, don't you? then, how do you change ALLOW CONNECTIONS to
true?

-- 
Jaime Casanova www.2ndQuadrant.com
Professional PostgreSQL: Soporte 24x7 y capacitación
Phone: +593 4 5107566 Cell: +593 987171157


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


Re: [HACKERS] SQL access to database attributes

2014-05-23 Thread Jaime Casanova
On Fri, May 23, 2014 at 11:06 PM, Vik Fearing vik.fear...@dalibo.com wrote:
 On 05/24/2014 12:03 AM, Jaime Casanova wrote:
 On Fri, May 23, 2014 at 10:53 PM, Vik Fearing vik.fear...@dalibo.com wrote:
 It was suggested to me that these options should either error out if
 there are existing connections or terminate said connections.  I don't
 agree with that because there is no harm in connecting to a template
 database (how else do you modify it?), and adding a reject rule in
 pg_hba.conf doesn't disconnect existing users so why should turning off
 ALLOW CONNECTIONS do it?

 Which lead us to the question: you need to connect to the database to
 modify it, don't you? then, how do you change ALLOW CONNECTIONS to
 true?

 You can ALTER DATABASE from anywhere.


ah! doh! right!
don't know why i was convinced you need to connect to the database to
execute ALTER DATABASE

-- 
Jaime Casanova www.2ndQuadrant.com
Professional PostgreSQL: Soporte 24x7 y capacitación
Phone: +593 4 5107566 Cell: +593 987171157


-- 
Sent 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] showing index maintenance on EXPLAIN

2014-05-09 Thread Jaime Casanova
On Thu, May 8, 2014 at 10:44 PM, Amit Kapila amit.kapil...@gmail.com wrote:
 On Thu, May 8, 2014 at 12:01 PM, Jaime Casanova ja...@2ndquadrant.com wrote:
 On Wed, May 7, 2014 at 10:52 PM, Amit Kapila amit.kapil...@gmail.com wrote:

 Why to capture only for Index Insert/Update and not for Read; is it
 because Read will be always fast ot implementation complexity?

 EXPLAIN ANALYZE already shows that on any SELECT that uses an index in
 some way. Or are you thinking on something else?

[...]

 Are you referring actual time in above print?

 The actual time is node execution time which in above kind of cases will
 be: scanning the index + scanning the heap.  I think it is not same what
 you are planning to show for Insert/Update case.


ah! good point! my current case is because of write performance, but
will look at it a little

-- 
Jaime Casanova www.2ndQuadrant.com
Professional PostgreSQL: Soporte 24x7 y capacitación
Phone: +593 4 5107566 Cell: +593 987171157


-- 
Sent 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] showing index maintenance on EXPLAIN

2014-05-08 Thread Jaime Casanova
On Wed, May 7, 2014 at 10:52 PM, Amit Kapila amit.kapil...@gmail.com wrote:
 On Thu, May 8, 2014 at 5:30 AM, Jaime Casanova ja...@2ndquadrant.com wrote:
 Hi,

 This patch implements $subject only when ANALYZE and VERBOSE are on.
 I made it that way because for years nobody seemed interested in this
 info (at least no one did it) so i decided that maybe is to much
 information for most people (actually btree indexes are normally very
 fast).


 Why to capture only for Index Insert/Update and not for Read; is it
 because Read will be always fast ot implementation complexity?


EXPLAIN ANALYZE already shows that on any SELECT that uses an index in
some way. Or are you thinking on something else?

 Why not similar timings for heap?


well actual time shows us total time of the operation so just
deducting the time spent on triggers, indexes and planning seems like
a way to get heap modification time.

yes, maybe we still need some additional data. for example, i could
want to know how much time we spent extending a relation.

 Why can't we print when only Analyze is used with Explain, the
 execution time is printed with Analyze option?


i'm not sure the info is useful for everyone, i'm not opposed to show
it all the time though

 Could you please tell in what all kind of scenario's, do you expect it
 to be useful?
 One I could think is that if there are multiple indexes on a table and user
 wants to find out if any particular index is consuming more time.


exactly my use case. consider this plan (we spent 78% of the time
updating the index uniq_idx_on_text):

   QUERY PLAN

 Insert on public.t1 (actual time=0.540..0.540 rows=0 loops=1)
   -  Result (actual time=0.046..0.049 rows=1 loops=1)
 Output: some long data here
 Index uniq_idx_on_text on t1: time=0.421 rows=1
 Index t1_pkey on t1: time=0.027 rows=1
 Total runtime: 0.643 ms
(6 rows)

so i want to answer questions like, how much an index is hurting write
performance? once i know that i can look for alternative solutions.
In that vein, it was interesting to see how fastupdate affect
performance in a GIN index using gin_trgm_ops (5 times slower with
fastupdate=off)

(fastupdate=on) Index idx_ggin on t1: time=0.418 rows=1
(fastupdate=off) Index idx_ggin on t1: time=2.205 rows=1

this is not different to showing trigger time info, which we already do

-- 
Jaime Casanova www.2ndQuadrant.com
Professional PostgreSQL: Soporte 24x7 y capacitación
Phone: +593 4 5107566 Cell: +593 987171157


-- 
Sent 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] showing index maintenance on EXPLAIN

2014-05-08 Thread Jaime Casanova
On Thu, May 8, 2014 at 10:41 AM, Robert Haas robertmh...@gmail.com wrote:
 On Thu, May 8, 2014 at 2:31 AM, Jaime Casanova ja...@2ndquadrant.com wrote:
 On Wed, May 7, 2014 at 10:52 PM, Amit Kapila amit.kapil...@gmail.com wrote:
 On Thu, May 8, 2014 at 5:30 AM, Jaime Casanova ja...@2ndquadrant.com 
 wrote:
 Hi,

 This patch implements $subject only when ANALYZE and VERBOSE are on.
 I made it that way because for years nobody seemed interested in this
 info (at least no one did it) so i decided that maybe is to much
 information for most people (actually btree indexes are normally very
 fast).


 I would have expected the information about index maintenance times to
 be associated with the Insert node, not the plan overall.  IIUC, you
 could have more than one such node if, for example, there are
 writeable CTEs involved.


i followed how trigger info is showed in explain. I can try to change
it, if that is what most people prefer.

   QUERY PLAN
-
 Insert on pgbench_accounts (actual time=0.249..0.249 rows=0 loops=1)
   CTE results
 -  Insert on pgbench_accounts pgbench_accounts_1 (actual
time=0.152..0.159 rows=1 loops=1)
   -  Result (actual time=0.003..0.004 rows=1 loops=1)
   -  CTE Scan on results (actual time=0.165..0.174 rows=1 loops=1)
 Trigger trg1 on pgbench_accounts: time=0.033 calls=1
 Trigger trg1 on pgbench_accounts: time=0.059 calls=1
 Total runtime: 0.377 ms
(8 rows)


-- 
Jaime Casanova www.2ndQuadrant.com
Professional PostgreSQL: Soporte 24x7 y capacitación
Phone: +593 4 5107566 Cell: +593 987171157


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


[HACKERS] [WIP] showing index maintenance on EXPLAIN

2014-05-07 Thread Jaime Casanova
Hi,

This patch implements $subject only when ANALYZE and VERBOSE are on.
I made it that way because for years nobody seemed interested in this
info (at least no one did it) so i decided that maybe is to much
information for most people (actually btree indexes are normally very
fast).

But because we have GiST and GIN this became an interested piece of
data (there are other cases even when using btree).

Current patch doesn't have docs yet i will add them soon.

-- 
Jaime Casanova www.2ndQuadrant.com
Professional PostgreSQL: Soporte 24x7 y capacitación
Phone: +593 4 5107566 Cell: +593 987171157
From be4d94a7ca49b176d9f67476f2edde9e1f3d2a21 Mon Sep 17 00:00:00 2001
From: Jaime Casanova ja...@2ndquadrant.com
Date: Wed, 7 May 2014 18:28:46 -0500
Subject: [PATCH] Make EXPLAIN show measurements of updating indexes.

This adds a line in the output of EXPLAIN ANALYZE VERBOSE
showing the time that took to update indexes in INSERT/UPDATE
operations.

Also, add that info in auto_explain module using a new
variable auto_explain.log_indexes.
---
 contrib/auto_explain/auto_explain.c|   14 ++
 src/backend/catalog/indexing.c |2 +-
 src/backend/commands/copy.c|6 +--
 src/backend/commands/explain.c |   75 
 src/backend/executor/execUtils.c   |   19 +++-
 src/backend/executor/nodeModifyTable.c |6 +--
 src/include/commands/explain.h |1 +
 src/include/executor/executor.h|4 +-
 src/include/nodes/execnodes.h  |2 +
 9 files changed, 118 insertions(+), 11 deletions(-)

diff --git a/contrib/auto_explain/auto_explain.c b/contrib/auto_explain/auto_explain.c
index c8ca7c4..f2c11e5 100644
--- a/contrib/auto_explain/auto_explain.c
+++ b/contrib/auto_explain/auto_explain.c
@@ -25,6 +25,7 @@ static int	auto_explain_log_min_duration = -1; /* msec or -1 */
 static bool auto_explain_log_analyze = false;
 static bool auto_explain_log_verbose = false;
 static bool auto_explain_log_buffers = false;
+static bool auto_explain_log_indexes = false;
 static bool auto_explain_log_triggers = false;
 static bool auto_explain_log_timing = false;
 static int	auto_explain_log_format = EXPLAIN_FORMAT_TEXT;
@@ -114,6 +115,17 @@ _PG_init(void)
 			 NULL,
 			 NULL);
 
+	DefineCustomBoolVariable(auto_explain.log_indexes,
+			 Include index statistics in plans.,
+			 This has no effect unless log_analyze is also set.,
+			 auto_explain_log_indexes,
+			 false,
+			 PGC_SUSET,
+			 0,
+			 NULL,
+			 NULL,
+			 NULL);
+
 	DefineCustomBoolVariable(auto_explain.log_triggers,
 			 Include trigger statistics in plans.,
 			 This has no effect unless log_analyze is also set.,
@@ -307,6 +319,8 @@ explain_ExecutorEnd(QueryDesc *queryDesc)
 			ExplainBeginOutput(es);
 			ExplainQueryText(es, queryDesc);
 			ExplainPrintPlan(es, queryDesc);
+			if (es.analyze  auto_explain_log_indexes)
+ExplainPrintIndexes(es, queryDesc);
 			if (es.analyze  auto_explain_log_triggers)
 ExplainPrintTriggers(es, queryDesc);
 			ExplainEndOutput(es);
diff --git a/src/backend/catalog/indexing.c b/src/backend/catalog/indexing.c
index 4bf412f..b2d3a1e 100644
--- a/src/backend/catalog/indexing.c
+++ b/src/backend/catalog/indexing.c
@@ -46,7 +46,7 @@ CatalogOpenIndexes(Relation heapRel)
 	resultRelInfo-ri_RelationDesc = heapRel;
 	resultRelInfo-ri_TrigDesc = NULL;	/* we don't fire triggers */
 
-	ExecOpenIndices(resultRelInfo);
+	ExecOpenIndices(resultRelInfo, 0);
 
 	return resultRelInfo;
 }
diff --git a/src/backend/commands/copy.c b/src/backend/commands/copy.c
index 70ee7e5..fbb1f13 100644
--- a/src/backend/commands/copy.c
+++ b/src/backend/commands/copy.c
@@ -2179,7 +2179,7 @@ CopyFrom(CopyState cstate)
 	  1,		/* dummy rangetable index */
 	  0);
 
-	ExecOpenIndices(resultRelInfo);
+	ExecOpenIndices(resultRelInfo, 0);
 
 	estate-es_result_relations = resultRelInfo;
 	estate-es_num_result_relations = 1;
@@ -2333,7 +2333,7 @@ CopyFrom(CopyState cstate)
 
 if (resultRelInfo-ri_NumIndices  0)
 	recheckIndexes = ExecInsertIndexTuples(slot, (tuple-t_self),
-		   estate);
+		   estate, NULL);
 
 /* AFTER ROW INSERT Triggers */
 ExecARInsertTriggers(estate, resultRelInfo, tuple,
@@ -2440,7 +2440,7 @@ CopyFromInsertBatch(CopyState cstate, EState *estate, CommandId mycid,
 			ExecStoreTuple(bufferedTuples[i], myslot, InvalidBuffer, false);
 			recheckIndexes =
 ExecInsertIndexTuples(myslot, (bufferedTuples[i]-t_self),
-	  estate);
+	  estate, NULL);
 			ExecARInsertTriggers(estate, resultRelInfo,
  bufferedTuples[i],
  recheckIndexes);
diff --git a/src/backend/commands/explain.c b/src/backend/commands/explain.c
index 08f3167..8e2b10d 100644
--- a/src/backend/commands/explain.c
+++ b/src/backend/commands/explain.c
@@ -502,6 +502,10 @@ ExplainOnePlan(PlannedStmt *plannedstmt, IntoClause *into, ExplainState *es

Re: [HACKERS] [WIP] showing index maintenance on EXPLAIN

2014-05-07 Thread Jaime Casanova
On Wed, May 7, 2014 at 7:03 PM, Josh Berkus j...@agliodbs.com wrote:
 On 05/07/2014 05:00 PM, Jaime Casanova wrote:
 Hi,

 This patch implements $subject only when ANALYZE and VERBOSE are on.
 I made it that way because for years nobody seemed interested in this
 info (at least no one did it) so i decided that maybe is to much
 information for most people (actually btree indexes are normally very
 fast).

 What's index maintenance?


Maybe is not the right term... i mean: the time that take to update
indexes on an INSERT/UPDATE operation


-- 
Jaime Casanova www.2ndQuadrant.com
Professional PostgreSQL: Soporte 24x7 y capacitación
Phone: +593 4 5107566 Cell: +593 987171157


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


[HACKERS] server vs foreign server inconsistency

2014-04-14 Thread Jaime Casanova
Hi,

A few days ago i was wondering why we use CREATE/DROP SERVER but then
when we want to GRANT/REVOKE we need to use FOREIGN SERVER.

of course options are:

1) modify both to accept both forms
2) modify one of them to accept both forms and use that way in all our
examples in docs
3) do nothing at all

IMHO i think 3 is not acceptable because it's annoying

opinions?

-- 
Jaime Casanova www.2ndQuadrant.com
Professional PostgreSQL: Soporte 24x7 y capacitación
Phone: +593 4 5107566 Cell: +593 987171157


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


Re: [HACKERS] Patch for CREATE RULE sgml -- Was in: [DOCS]

2014-03-21 Thread Jaime Casanova
On Fri, Mar 21, 2014 at 8:15 PM, Michael Paquier
michael.paqu...@gmail.com wrote:

 On Sat, Mar 22, 2014 at 12:56 AM, Emanuel Calvo
 emanuel.ca...@2ndquadrant.com wrote:
  I realized that the output of the CREATE RULE has not a detailed
  output for the events parameter.
 
 The list of events possible is already listed in the section
 Parameters = event:

AFAIU, the synopsis is used to build the help command (\h) in psql.
Currently in that help the events doesn't appear.
btw, CREATE TRIGGER already looks this way:
http://www.postgresql.org/docs/current/static/sql-createtrigger.html

-- 
Jaime Casanova www.2ndQuadrant.com
Professional PostgreSQL: Soporte 24x7 y capacitación
Phone: +593 4 5107566 Cell: +593 987171157


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


Re: [HACKERS] db_user_namespace a temporary measure

2014-03-11 Thread Jaime Casanova
On Tue, Mar 11, 2014 at 10:06 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 But not sure how to define a unique
 index that allows (joe, db1) to coexist with (joe, db2) but not with
 (joe, 0).


and why you want that restriction? when you login you need to specify
the db, right? if you don't specify the db then is the global 'joe'
that want to login

-- 
Jaime Casanova www.2ndQuadrant.com
Professional PostgreSQL: Soporte 24x7 y capacitación
Phone: +593 4 5107566 Cell: +593 987171157


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


Re: [HACKERS] db_user_namespace a temporary measure

2014-03-11 Thread Jaime Casanova
On Tue, Mar 11, 2014 at 10:52 PM, Andrew Dunstan and...@dunslane.net wrote:

 On 03/11/2014 11:50 PM, Jaime Casanova wrote:

 On Tue, Mar 11, 2014 at 10:06 PM, Tom Lane t...@sss.pgh.pa.us wrote:

 But not sure how to define a unique
 index that allows (joe, db1) to coexist with (joe, db2) but not with
 (joe, 0).

 and why you want that restriction? when you login you need to specify
 the db, right? if you don't specify the db then is the global 'joe'
 that want to login


 You can't login without specifying a db. Every session is connected to a db.


then, what's the problem with global users?

-- 
Jaime Casanova www.2ndQuadrant.com
Professional PostgreSQL: Soporte 24x7 y capacitación
Phone: +593 4 5107566 Cell: +593 987171157


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


Re: [HACKERS] 9.2.1 index-only scans : abnormal heap fetches after VACUUM FULL

2014-01-24 Thread Jaime Casanova
On Tue, Dec 3, 2013 at 11:25 AM, Bruce Momjian br...@momjian.us wrote:

 Is everyone else OK with this approach?  Updated patch attached.


Hi,

I started to look at this patch and i found that it fails an assertion
as soon as you run a VACUUM FULL after a lazy VACUUM even if those are
on unrelated relations. For example in an assert-enabled build with
the regression database run:

VACUUM customer;
[... insert here whatever commands you like or nothing at all ...]
VACUUM FULL tenk1;

TRAP: FailedAssertion(!(InRecovery || ( ((void) ((bool) ((!
assert_enabled) || ! (!((heapBuf) = NBuffers  (heapBuf) =
-NLocBuffer)) || (ExceptionalCondition(!((heapBuf) = NBuffers 
(heapBuf) = -NLocBuffer), (FailedAssertion), visibilitymap.c,
260), 0, (heapBuf) != 0 )), File: visibilitymap.c, Line: 260)
LOG:  server process (PID 25842) was terminated by signal 6: Aborted
DETAIL:  Failed process was running: vacuum FULL customer;
LOG:  terminating any other active server processes

trace:
(gdb) bt
#0  0x7f9a3d00d475 in *__GI_raise (sig=optimized out) at
../nptl/sysdeps/unix/sysv/linux/raise.c:64
#1  0x7f9a3d0106f0 in *__GI_abort () at abort.c:92
#2  0x00777597 in ExceptionalCondition (
conditionName=conditionName@entry=0x7cd3b8 !(InRecovery || (
((void) ((bool) ((! assert_enabled) || ! (!((heapBuf) = NBuffers 
(heapBuf) = -NLocBuffer)) || (ExceptionalCondition(\!((heapBuf) =
NBuffers  (heapBuf) = -NLocBuffer)\, (\Fai...,
errorType=errorType@entry=0x7b0730 FailedAssertion,
fileName=fileName@entry=0x7cd105 visibilitymap.c,
lineNumber=lineNumber@entry=260) at assert.c:54
#3  0x004a7d99 in visibilitymap_set
(rel=rel@entry=0x7f9a3da56a00, heapBlk=heapBlk@entry=0,
heapBuf=heapBuf@entry=0, recptr=recptr@entry=0, vmBuf=220,
cutoff_xid=2) at visibilitymap.c:260
#4  0x004a33e5 in update_page_vm (relation=0x7f9a3da56a00,
page=page@entry=0x1868b18 , blkno=0) at rewriteheap.c:702
#5  0x004a3668 in raw_heap_insert
(state=state@entry=0x1849e98, tup=tup@entry=0x184f208) at
rewriteheap.c:641
#6  0x004a3b8b in rewrite_heap_tuple
(state=state@entry=0x1849e98, old_tuple=old_tuple@entry=0x1852a50,
new_tuple=new_tuple@entry=0x184f208)
at rewriteheap.c:433
#7  0x0055c373 in reform_and_rewrite_tuple
(tuple=tuple@entry=0x1852a50,
oldTupDesc=oldTupDesc@entry=0x7f9a3da4d350,
newTupDesc=newTupDesc@entry=0x7f9a3da599a8,
values=values@entry=0x1852f40, isnull=isnull@entry=0x184f920 ,
newRelHasOids=1 '\001',
rwstate=rwstate@entry=0x1849e98) at cluster.c:1670

-- 
Jaime Casanova www.2ndQuadrant.com
Professional PostgreSQL: Soporte 24x7 y capacitación
Phone: +593 4 5107566 Cell: +593 987171157


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


Re: [HACKERS] Trigger information for auto_explain.

2014-01-20 Thread Jaime Casanova
On Tue, Jan 14, 2014 at 4:25 AM, Kyotaro HORIGUCHI
horiguchi.kyot...@lab.ntt.co.jp wrote:

 This patch consists of two parts,

  0001_expose_explain_triggers_v1_20140114.patch

   Expose the code to print out trigger information currently
   hidden in ExplainOnePlan().

  0002_auto_explain_triggers_v1_20140114.patch

   Enable auto_explain to output trigger information.

 Documentation will be added later..


Hi,

I think documentation is the only thing that stops this patch to be
commitable... can you add it?

-- 
Jaime Casanova www.2ndQuadrant.com
Professional PostgreSQL: Soporte 24x7 y capacitación
Phone: +593 4 5107566 Cell: +593 987171157


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


Re: [HACKERS] Turning recovery.conf into GUCs

2014-01-14 Thread Jaime Casanova
On Wed, Jan 15, 2014 at 2:00 AM, Jaime Casanova ja...@2ndquadrant.com wrote:
 On Mon, Nov 18, 2013 at 12:27 PM, Andres Freund and...@2ndquadrant.com 
 wrote:

 * Maybe we should rename names like pause_at_recovery_target to
   recovery_pause_at_target? Since we already force everyone to bother
   changing their setup...

 i don't have a problem with this, anyone else? if no one speaks i will
 do what Andres suggests


Actually Michael had objected to this idea but i forgot about that...
so i will wait for some consensus (my personal opinion is that
Michael's argument is a good one)

-- 
Jaime Casanova www.2ndQuadrant.com
Professional PostgreSQL: Soporte 24x7 y capacitación
Phone: +593 4 5107566 Cell: +593 987171157


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


Re: [HACKERS] tracking commit timestamps

2013-12-01 Thread Jaime Casanova
On Tue, Oct 22, 2013 at 5:16 PM, Alvaro Herrera
alvhe...@2ndquadrant.com wrote:
 Hi,

 There has been some interest in keeping track of timestamp of
 transaction commits.  This patch implements that.


Hi,

Sorry for the delay on the review.

First, because of the recent fixes the patch doesn't apply cleanly
anymore but the changes seems to be easy.

=== performance ===

i expected a regression on performance with the module turned on
because of the new XLOG records and wrote of files in pg_committs but
the performance drop is excessive.

Master 437.835674 tps
Patch, guc off   436.4340982 tps
Patch, guc on   0.370524 tps

This is in a pgbench's db initialized with scale=50 and run with
pgbench -c 64 -j 16 -n -T 300 5 times (values above are the average
of runs)

postgresql changes:

shared_buffers = 1GB
full_page_writes = off
checkpoint_segments = 30
checkpoint_timeout = 15min
random_page_cost = 2.0

== functionality ==

I started the server with the module off, then after some work turned
the module on and restarted the server and run a few benchs then
turned it off again and restart the server and get a message like:


LOG:  database system was not properly shut down; automatic recovery in progress
LOG:  record with zero length at 0/3112AE58
LOG:  redo is not required
FATAL:  cannot make new WAL entries during recovery
LOG:  startup process (PID 24876) exited with exit code 1
LOG:  aborting startup due to startup process failure


-- 
Jaime Casanova www.2ndQuadrant.com
Professional PostgreSQL: Soporte 24x7 y capacitación
Phone: +593 4 5107566 Cell: +593 987171157


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


Re: [HACKERS] Turning recovery.conf into GUCs

2013-11-20 Thread Jaime Casanova
On Tue, Nov 19, 2013 at 3:32 PM, Andres Freund and...@2ndquadrant.com wrote:
 On 2013-11-19 22:09:48 +0900, Michael Paquier wrote:
 On Tue, Nov 19, 2013 at 2:27 AM, Andres Freund and...@2ndquadrant.com 
 wrote:

  * I am not sure I like recovery.trigger as a name. It seems to close
to what I've seen people use to trigger failover and too close to
trigger_file.

 This name was chosen and kept in accordance to the spec of this
 feature. Looks fine for me...

 I still think start_as_standby.trigger or such would be much clearer
 and far less likely to be confused with the promotion trigger file.


the function of the file is to inform the server it's in recovery and
it needs to consider recovery parameters, not to make the server a
standby. yes, i admit that is half the way to make the server a
standby. for example, if you are doing PITR and stopping the server
before some specific point (recovery_target_*) then
start_as_standby.trigger will has no meaning and could confuse
people


  * You made recovery_target_inclusive/pause_at_recovery_target PGC_SIGHUP
- did you review that actually works? Imo that should be changed in a
separate commit.

 Yep, I'd rather see those parameters as PGC_POSTMASTER... As of now,
 once recovery is started those parameter values do not change once
 readRecoveryCommandFile is kicked. Having them SIGHUP would mean that
 you could change them during recovery. Sounds kind of dangerous, no?

 I think it's desirable to make them changeable during recovery, but it's
 a separate patch.


uh! i read the patch and miss that. will change


  * Why did you change some of the recovery gucs to lowercase names, but
left out XLogRestoreCommand?

 This was part of the former patch, perhaps you are right and keeping
 the names as close as possible to the old ones would make sense to
 facilitate maintenance across versions.

 I think lowercase is slightly more consistent with the majority of the
 other GUCs, but if you change it you should change all the new GUC variables.


This one was my change, in the original patch is called
restore_command and i changed it because archive_command's parameter
is called XLogArchiveCommand so i wanted the name to follow the same
format.

i can return it to the original name if no one likes XLogRestoreCommand

-- 
Jaime Casanova www.2ndQuadrant.com
Professional PostgreSQL: Soporte 24x7 y capacitación
Phone: +593 4 5107566 Cell: +593 987171157


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


Re: [HACKERS] Turning recovery.conf into GUCs

2013-11-15 Thread Jaime Casanova
On Fri, Nov 15, 2013 at 9:28 AM, Peter Eisentraut pete...@gmx.net wrote:
 On 11/13/13, 12:17 AM, Jaime Casanova wrote:
 I have rebased Michael Paquier's patch and did a few changes:

 * changed standby.enabled filename to recovery.trigger
 * make archive_command a SIGHUP parameter again
 * make restore_command a SIGHUP parameter
 * rename restore_command variable to XLogRestoreCommand to match
 XLogArchiveCommand

 Please check for compiler warnings in pg_basebackup:

 pg_basebackup.c:1109:1: warning: ‘escapeConnectionParameter’ defined but not 
 used [-Wunused-function]
 pg_basebackup.c:1168:1: warning: ‘escape_quotes’ defined but not used 
 [-Wunused-function]


those are functions that are no longer used but Josh considered they
could become useful before release.
i can put them inside #ifdef _NOT_USED_ decorations or just remove
them now and if/when we find some use for them re add them

-- 
Jaime Casanova www.2ndQuadrant.com
Professional PostgreSQL: Soporte 24x7 y capacitación
Phone: +593 4 5107566 Cell: +593 987171157


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


Re: [HACKERS] Commitfest II CLosed

2013-10-22 Thread Jaime Casanova
On Tue, Oct 22, 2013 at 2:38 PM, Joshua D. Drake j...@commandprompt.com wrote:

 On 10/21/2013 08:11 AM, Robert Haas wrote:

 Supposedly, we have a policy that for each patch you submit, you ought
 to review a patch.  That right there ought to provide enough reviewers
 for all the patches, but clearly it didn't.  And I'm pretty sure that
 some people (like me) looked at a lot MORE patches than they
 themselves submitted.  I think auditing who is not contributing in
 that area and finding tactful ways to encourage them to contribute
 would be a very useful service to the project.


 What if as part of the patch submission process you had to pick the patch
 you were going to review? If there are no patches to review, then we
 obviously don't have a problem. If there are patches to review then we are
 all set.


if we are going to modify the CF app (not offering myself, and i'm not
trying to bind anyone also) i would prefer to see a flag stating if
number of reviews registered there are less than submitted patches.
This could be a column just after the author of a patch, so people can
give preference to patches of submitters that are also reviewing other
people's patches.

-- 
Jaime Casanova www.2ndQuadrant.com
Professional PostgreSQL: Soporte 24x7 y capacitación
Phone: +593 4 5107566 Cell: +593 987171157


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


Re: [HACKERS] Turning recovery.conf into GUCs

2013-10-21 Thread Jaime Casanova
On Mon, Oct 21, 2013 at 11:53 AM, Josh Berkus j...@agliodbs.com wrote:
 On 10/18/2013 02:36 PM, Andres Freund wrote:
  What will likely change first is Slony and Bucardo, who have a strong
  interest in dumping triggers and queues.
 But I don't understand what that has to do with recovery.conf and
 breakage around it.

 The simple thinking is this: if we announce and promote new replication,
 then our users who do upgrade are going to expect to upgrade their
 replication tools at the same time, even if they're not using the new
 replication.  That is people will look for a repmgr 2.0 / OmniPITR 1.5
 and update to it.

 Now, as a tool author, I know that supporting both models is going to be
 annoying.  But necessary.


AFAIU, even if we get in all about logical replication today that
won't affect tools that manage binary replication.

 And, as I said before, we need to do the GUC merger in the same release
 we introduce configuration directory (or after it).


you mean the ALTER SYSTEM syntax? anyway, why that restriction?

-- 
Jaime Casanova www.2ndQuadrant.com
Professional PostgreSQL: Soporte 24x7 y capacitación
Phone: +593 4 5107566 Cell: +593 987171157


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


Re: [HACKERS] Turning recovery.conf into GUCs

2013-10-21 Thread Jaime Casanova
On Mon, Oct 21, 2013 at 2:57 PM, Josh Berkus j...@agliodbs.com wrote:
 Jaime,

 And, as I said before, we need to do the GUC merger in the same release
 we introduce configuration directory (or after it).


 you mean the ALTER SYSTEM syntax? anyway, why that restriction?

 No, I'm referring to the proposal to have an automatically created and
 included conf.d directory for extra configuration files.


9.3 has include_dir and postgresql.conf has this setted:

#include_dir = 'conf.d' # include files ending in '.conf' from
# directory 'conf.d'


anything before this should be up to the packagers, no?

or, do you mean something else?

-- 
Jaime Casanova www.2ndQuadrant.com
Professional PostgreSQL: Soporte 24x7 y capacitación
Phone: +593 4 5107566 Cell: +593 987171157


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


Re: [HACKERS] Turning recovery.conf into GUCs

2013-10-18 Thread Jaime Casanova
On Fri, Oct 18, 2013 at 11:32 AM, Josh Berkus j...@agliodbs.com wrote:

 exactly as it is now, if it sees the recovery trigger file, then it
 starts ArchiveRecovery and after it finish delete the file (the only
 difference) and increment the timeline

 OK, so if I'm doing a PITR recovery, I just put the recovery variables
 into postgresql.conf, then?


create a recovery trigger file (called standby.enabled in current
patch) in $PGDATA and start the server

-- 
Jaime Casanova www.2ndQuadrant.com
Professional PostgreSQL: Soporte 24x7 y capacitación
Phone: +593 4 5107566 Cell: +593 987171157


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


Re: [HACKERS] Turning recovery.conf into GUCs

2013-10-18 Thread Jaime Casanova
On Fri, Oct 18, 2013 at 11:32 AM, Josh Berkus j...@agliodbs.com wrote:
 Jaime,

 well, after upgrade you should do checks. and even if it happens,
 after it happens once people will be aware of the change.
 now, some suggestions were made to avoid the problem. 1) read the file
 if exists last in the process of postgresql.conf, 2) add a GUC
 indicating if it should read it and include it (not using it as a
 trigger file). another one, 3) include in this release an
 include_if_exists directive and give a warning if it sees the file
 then include it, on next release remove the include_if_exists (at
 least that way people will be warned before breaking compatibility)

 I think all of these suggestions just make our code more complicated
 without improving the upgrade situation.


well #3 just add a line in postgresql.conf (an include_if_exists) and
current patch gives an error in case it finds the file (i'm suggesting
to make it a warning instead).
how does that makes our code more complicated?

 The reason given (and I think it's pretty good) for erroring on
 recovery.conf is that we don't want people to accidentally take a server
 out of replication because they didn't check which version of PostgreSQL
 they are on.


well, people will go out of replication also if they forgot the
recovery trigger file
even if they set the variables in postgresql.conf

it happens two me twice today ;)

-- 
Jaime Casanova www.2ndQuadrant.com
Professional PostgreSQL: Soporte 24x7 y capacitación
Phone: +593 4 5107566 Cell: +593 987171157


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


Re: [HACKERS] Turning recovery.conf into GUCs

2013-10-17 Thread Jaime Casanova
On Wed, Oct 16, 2013 at 1:34 PM, Josh Berkus j...@agliodbs.com wrote:
 Jaime,
 = Building =

 In pg_basebackup we have now 2 unused functions:
 escapeConnectionParameter and escape_quotes. the only use of these
 functions was when that tool created the recovery.conf file so they
 aren't needed anymore.

 Except that we'll want 9.4's -R to do something, probably create a file
 called conf.d/replication.conf.  Mind you, it won't need the same wonky
 quoting stuff.


Currently the patch uses -R to create the recovery trigger file

 1) the file to trigger recovery is now called standby.enabled. I know
 this is because we use the file to also make the node a standby.
 Now, reality is that the file doesn't make the node a standby but the
 combination of this file (which starts recovery) + standby_mode=on.
 so, i agree with the suggestion of naming the file: recovery.trigger

 2) This patch removes the dual existence of recovery.conf: as a state
 file and as a configuration file

 - the former is accomplished by changing the name of the file that
 triggers recovery (from recovery.conf to standby.enabled)
 - the latter is done by moving all parameters to postgresql.conf and
 *not reading* recovery.conf anymore

 so, after applying this patch postgres don't use recovery.conf for
 anything... except for complaining.
 it's completely fair to say we are no longer using that file and
 ignoring its existence, but why we should care if users want to use a
 file with that name for inclusion in postgresql.conf or where they put
 that hypotetic file?

 So this is a bit of a catch-22.  If we allow the user to use a file
 named recovery.conf in $PGDATA, then the user may be unaware that the
 *meaning* of the file has changed, which can result in unplanned
 promotion of replicas after upgrade.


well, after upgrade you should do checks. and even if it happens,
after it happens once people will be aware of the change.
now, some suggestions were made to avoid the problem. 1) read the file
if exists last in the process of postgresql.conf, 2) add a GUC
indicating if it should read it and include it (not using it as a
trigger file). another one, 3) include in this release an
include_if_exists directive and give a warning if it sees the file
then include it, on next release remove the include_if_exists (at
least that way people will be warned before breaking compatibility)

please, keep in mind none of these suggestions include make the
recovery.conf file act as a trigger for recovery

 *on the other hand*, if we prevent creation of a configuration file
 named recovery.conf, then we block efforts to write
 backwards-compatible management utilities.


and all tools and procedures that currently exists.

 AFAIK, there is no good solution for this, which is why it's taken so
 long for us to resolve the general issue.  Given that, I think it's
 better to go for the breakage and all the warnings.  If a user wants a
 file called recovery.conf, put it in the conf.d directory.


well, there should be good solutions... maybe we haven't thought them yet.
anyway, we can't postpone the decision forever. we need to make a
decision and stick with it otherwise this patch will be stalled N
releases for no good reason

 I don't remember, though: how does this patch handle PITR recovery?


exactly as it is now, if it sees the recovery trigger file, then it
starts ArchiveRecovery and after it finish delete the file (the only
difference) and increment the timeline

 = Code  functionality =

 +   {restore_command, PGC_POSTMASTER, WAL_ARCHIVE_RECOVERY,
 +   {archive_cleanup_command, PGC_POSTMASTER, WAL_ARCHIVE_RECOVERY,
 +   {recovery_end_command, PGC_POSTMASTER, WAL_ARCHIVE_RECOVERY,
 +   {recovery_target_xid, PGC_POSTMASTER, WAL_RECOVERY_TARGET,
 +   {recovery_target_name, PGC_POSTMASTER, WAL_RECOVERY_TARGET,
 +   {recovery_target_time, PGC_POSTMASTER, WAL_RECOVERY_TARGET,
 +   {trigger_file, PGC_POSTMASTER, REPLICATION_STANDBY,

 Not sure about these ones

 +   {recovery_target_timeline, PGC_POSTMASTER, WAL_RECOVERY_TARGET,
 +   {primary_conninfo, PGC_POSTMASTER, REPLICATION_STANDBY,

 It would be really nice to change these on the fly; it would help a lot
 of issues with minor changes to replication config.  I can understand,
 though, that the replication code might not be prepared for that.


well, archive_command can be changed right now with a SIGHUP so at
least that one shouldn't change... and i don't think most of these are
too different. even if we are not sure we can do this now and change
them as SIGHUP later

-- 
Jaime Casanova www.2ndQuadrant.com
Professional PostgreSQL: Soporte 24x7 y capacitación
Phone: +593 4 5107566 Cell: +593 987171157


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


[HACKERS] Turning recovery.conf into GUCs

2013-10-16 Thread Jaime Casanova
Hi everyone,

Seems that the latest patch in this series is:
http://www.postgresql.org/message-id/CAB7nPqRLWLH1b0YsvqYF-zOFjrv4FRiurQ6yqcP3wjBp=tj...@mail.gmail.com

= Applying =

Reading the threads it seems there is consensus in this happening, so
let's move in that direction.
The patch applies almost cleanly except for a minor change in xlog.c

= Building =

In pg_basebackup we have now 2 unused functions:
escapeConnectionParameter and escape_quotes. the only use of these
functions was when that tool created the recovery.conf file so they
aren't needed anymore.

= Functionality =

I have been testing functionality and it looks good so far, i still
need to test a few things but i don't expect anything bad.

I have 2 complaints though:

1) the file to trigger recovery is now called standby.enabled. I know
this is because we use the file to also make the node a standby.
Now, reality is that the file doesn't make the node a standby but the
combination of this file (which starts recovery) + standby_mode=on.
so, i agree with the suggestion of naming the file: recovery.trigger

2) This patch removes the dual existence of recovery.conf: as a state
file and as a configuration file

- the former is accomplished by changing the name of the file that
triggers recovery (from recovery.conf to standby.enabled)
- the latter is done by moving all parameters to postgresql.conf and
*not reading* recovery.conf anymore

so, after applying this patch postgres don't use recovery.conf for
anything... except for complaining.
it's completely fair to say we are no longer using that file and
ignoring its existence, but why we should care if users want to use a
file with that name for inclusion in postgresql.conf or where they put
that hypotetic file?

after this patch, recovery.conf will have no special meaning anymore
so let's the user put it whatever files he wants, wherever he wants.
if we really want to warn the user, use WARNING instead of FATAL

= Code  functionality =

why did you changed the context in which we can set archive_command?
please, let it as a SIGHUP parameter

-   {archive_command, PGC_SIGHUP, WAL_ARCHIVING,
+   {archive_command, PGC_POSTMASTER, WAL_ARCHIVING,


All parameters moved from recovery.conf has been marked as
PGC_POSTMASTER, but most of them are clearly PGC_SIGHUP candidates

+   {restore_command, PGC_POSTMASTER, WAL_ARCHIVE_RECOVERY,
+   {archive_cleanup_command, PGC_POSTMASTER, WAL_ARCHIVE_RECOVERY,
+   {recovery_end_command, PGC_POSTMASTER, WAL_ARCHIVE_RECOVERY,
+   {recovery_target_xid, PGC_POSTMASTER, WAL_RECOVERY_TARGET,
+   {recovery_target_name, PGC_POSTMASTER, WAL_RECOVERY_TARGET,
+   {recovery_target_time, PGC_POSTMASTER, WAL_RECOVERY_TARGET,
+   {trigger_file, PGC_POSTMASTER, REPLICATION_STANDBY,

Not sure about these ones

+   {recovery_target_timeline, PGC_POSTMASTER, WAL_RECOVERY_TARGET,
+   {primary_conninfo, PGC_POSTMASTER, REPLICATION_STANDBY,

This is the only one i agree, should be PGC_POSTMASTER only

+   {standby_mode, PGC_POSTMASTER, REPLICATION_STANDBY,

-- 
Jaime Casanova www.2ndQuadrant.com
Professional PostgreSQL: Soporte 24x7 y capacitación
Phone: +593 4 5107566 Cell: +593 987171157


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


Re: [HACKERS] [PoC] pgstattuple2: block sampling to reduce physical read

2013-10-10 Thread Jaime Casanova
On Thu, Oct 10, 2013 at 5:32 PM, Mark Kirkwood
mark.kirkw...@catalyst.net.nz wrote:

 Quietly replying to myself - looking at the code the sampler does 3000
 random page reads...

FWIW, something that bothers me is that there is 3000 random page
reads... i mean, why 3000? how do you get that number as absolute for
good accuracy in every relation? why not a percentage, maybe an
argument to the function?

also the name pgstattuple2, doesn't convince me... maybe you can use
pgstattuple() if you use a second argument (percentage of the sample)
to overload the function

-- 
Jaime Casanova www.2ndQuadrant.com
Professional PostgreSQL: Soporte 24x7 y capacitación
Phone: +593 4 5107566 Cell: +593 987171157


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


Re: [HACKERS] Assertions in PL/PgSQL

2013-09-21 Thread Jaime Casanova
On Fri, Sep 20, 2013 at 5:17 AM, Marko Tiikkaja ma...@joh.to wrote:
 On 9/20/13 12:09 PM, Amit Khandekar wrote:

 On 16 September 2013 03:43, Marko Tiikkaja ma...@joh.to wrote:

 I think it would be extremely surprising if a command like that got
 optimized away based on a GUC, so I don't think that would be a good
 idea.



 In pl_gram.y, in the rule stmt_raise, determine that this RAISE is for
 ASSERT, and then return NULL if plpgsql_curr_compile-enable_assertions is
 false. Isn't this possible ?


 Of course it's possible.  But I, as a PostgreSQL user writing PL/PgSQL code,
 would be extremely surprised if this new cool option to RAISE didn't work
 for some reason.  If we use ASSERT the situation is different; most people
 will realize it's a new command and works differently from RAISE.



What about just adding a clause WHEN to the RAISE statement and use
the level machinery (client_min_messages) to make it appear or not
of course, this has the disadvantage that an EXCEPTION level will
always happen... or you can make it a new loglevel that mean EXCEPTION
if asserts_enabled

-- 
Jaime Casanova www.2ndQuadrant.com
Professional PostgreSQL: Soporte 24x7 y capacitación
Phone: +593 4 5107566 Cell: +593 987171157


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


Re: [HACKERS] VMs for Reviewers Available

2013-09-21 Thread Jaime Casanova
El 21/09/2013 18:09, Andrew Dunstan and...@dunslane.net escribió:


 On 09/21/2013 06:48 PM, Peter Geoghegan wrote:

 On Sat, Sep 21, 2013 at 3:35 PM, Josh Berkus j...@agliodbs.com wrote:

 Windows VMs are also available, but I don't have the ability to
 preconfigure them with tools.

 Wasn't there an EC2 image doing the rounds that Magnus created, that
 took care of all of that for you?


http://blog.hagander.net/archives/151-Testing-PostgreSQL-patches-on-Windows-using-Amazon-EC2.html

 I'm not sure how current it is - was this before or after we started
 shipping our own WinFlex? Magnus?



 There is really no longer any need to use our WinFlex. Just use the flex
that comes with MSysGit - it works perfectly well, and you only need flex
if you're building from git anyway.


 If you're reviewing patches you're probably compiling from git.

those win machines come with msys (and flex)? not always our reviewers know
how to install those tools

--
Jaime Casanova
2ndQuadrant: Your PostgreSQL partner


Re: [HACKERS] Assertions in PL/PgSQL

2013-09-21 Thread Jaime Casanova
El 21/09/2013 17:16, Jaime Casanova ja...@2ndquadrant.com escribió:

 On Fri, Sep 20, 2013 at 5:17 AM, Marko Tiikkaja ma...@joh.to wrote:
  On 9/20/13 12:09 PM, Amit Khandekar wrote:
 
  On 16 September 2013 03:43, Marko Tiikkaja ma...@joh.to wrote:
 
  I think it would be extremely surprising if a command like that got
  optimized away based on a GUC, so I don't think that would be a good
  idea.
 
 
 
  In pl_gram.y, in the rule stmt_raise, determine that this RAISE is for
  ASSERT, and then return NULL if
plpgsql_curr_compile-enable_assertions is
  false. Isn't this possible ?
 
 
  Of course it's possible.  But I, as a PostgreSQL user writing PL/PgSQL
code,
  would be extremely surprised if this new cool option to RAISE didn't
work
  for some reason.  If we use ASSERT the situation is different; most
people
  will realize it's a new command and works differently from RAISE.
 
 

 What about just adding a clause WHEN to the RAISE statement and use
 the level machinery (client_min_messages) to make it appear or not
 of course, this has the disadvantage that an EXCEPTION level will
 always happen... or you can make it a new loglevel that mean EXCEPTION
 if asserts_enabled


meaning RAISE ASSERT of course

--
Jaime Casanova
2ndQuadrant: Your PostgreSQL partner


Re: [HACKERS] Minmax indexes

2013-09-18 Thread Jaime Casanova
On Tue, Sep 17, 2013 at 4:03 PM, Alvaro Herrera
alvhe...@2ndquadrant.com wrote:
 Thom Brown wrote:

 Thanks for testing.

 Thanks for the patch, but I seem to have immediately hit a snag:

 pgbench=# CREATE INDEX minmaxtest ON pgbench_accounts USING minmax (aid);
 PANIC:  invalid xlog record length 0

 Silly mistake I had already made in another patch.  Here's an
 incremental patch which fixes this bug.  Apply this on top of previous
 minmax-1.patch.

 I also renumbered the duplicate OID pointed out by Peter, and fixed the
 two compiler warnings reported by Jaime.

 Note you'll need to re-initdb in order to get the right catalog entries.


Hi,

Found another problem with the this steps:

create table t1 (i int);
create index idx_t1_i on t1 using minmax(i);
insert into t1 select generate_series(1, 200);
ERROR:  could not read block 1 in file base/12645/16397_vm: read
only 0 of 8192 bytes
STATEMENT:  insert into t1 select generate_series(1, 200);
ERROR:  could not read block 1 in file base/12645/16397_vm: read
only 0 of 8192 bytes

After that, i keep receiving these messages (when autovacuum tries to
vacuum this table):

ERROR:  could not truncate file base/12645/16397_vm to 2 blocks:
it's only 1 blocks now
CONTEXT:  automatic vacuum of table postgres.public.t1
ERROR:  could not truncate file base/12645/16397_vm to 2 blocks:
it's only 1 blocks now
CONTEXT:  automatic vacuum of table postgres.public.t1

-- 
Jaime Casanova www.2ndQuadrant.com
Professional PostgreSQL: Soporte 24x7 y capacitación
Phone: +593 4 5107566 Cell: +593 987171157


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


Re: [HACKERS] Minmax indexes

2013-09-17 Thread Jaime Casanova
On Mon, Sep 16, 2013 at 3:47 AM, Thom Brown t...@linux.com wrote:
 On 15 September 2013 01:14, Alvaro Herrera alvhe...@2ndquadrant.com wrote:

 Hi,

 Here's a reviewable version of what I've dubbed Minmax indexes.

 Thanks for the patch, but I seem to have immediately hit a snag:

 pgbench=# CREATE INDEX minmaxtest ON pgbench_accounts USING minmax (aid);
 PANIC:  invalid xlog record length 0


fwiw, this seems to be triggered by ANALYZE.
At least i can trigger it by executing ANALYZE on the table (attached
is a stacktrace of a backend exhibiting the failure)

Another thing is this messages i got when compiling:

mmxlog.c: In function ‘minmax_xlog_revmap_set’:
mmxlog.c:161:14: warning: unused variable ‘blkno’ [-Wunused-variable]
bufpage.c: In function ‘PageIndexDeleteNoCompact’:
bufpage.c:1066:18: warning: ‘lastused’ may be used uninitialized in
this function [-Wmaybe-uninitialized]


-- 
Jaime Casanova www.2ndQuadrant.com
Professional PostgreSQL: Soporte 24x7 y capacitación
Phone: +593 4 5107566 Cell: +593 987171157
Program received signal SIGABRT, Aborted.
0x7f4428819475 in *__GI_raise (sig=optimized out) at 
../nptl/sysdeps/unix/sysv/linux/raise.c:64
64  ../nptl/sysdeps/unix/sysv/linux/raise.c: No existe el fichero o el 
directorio.
(gdb) bt
#0  0x7f4428819475 in *__GI_raise (sig=optimized out) at 
../nptl/sysdeps/unix/sysv/linux/raise.c:64
#1  0x7f442881c6f0 in *__GI_abort () at abort.c:92
#2  0x0076d6ac in errfinish (dummy=dummy@entry=0) at elog.c:546
#3  0x0076fdca in elog_finish (elevel=elevel@entry=22, 
fmt=fmt@entry=0x7d2aa7 invalid xlog record length %u) at elog.c:1304
#4  0x004e1eba in XLogInsert (rmid=rmid@entry=17 '\021', 
info=info@entry=48 '0', rdata=rdata@entry=0x7fffc836ea10) at xlog.c:966
#5  0x004a9bb9 in mmSetHeapBlockItemptr 
(rmAccess=rmAccess@entry=0x1b3af38, heapBlk=heapBlk@entry=0, 
blkno=blkno@entry=6, offno=offno@entry=1)
at mmrevmap.c:169
#6  0x004a73e8 in mm_doinsert (idxrel=0x7f4429054c88, 
rmAccess=0x1b3af38, buffer=buffer@entry=0x7f441f7e718c, heapblkno=0, 
tup=tup@entry=0x7f441f84cff8, 
itemsz=16) at minmax.c:1410
#7  0x004a9464 in rerun_summarization (numnonsummarized=22, 
nonsummarized=0x1b3a408, rmAccess=0x1b3af38, heapRel=0x7f4429052e68, 
idxRel=0x7f4429054c88)
at minmax.c:1205
#8  mmvacuumcleanup (fcinfo=optimized out) at minmax.c:1268
#9  0x0077388f in FunctionCall2Coll 
(flinfo=flinfo@entry=0x7fffc836f0a0, collation=collation@entry=0, 
arg1=arg1@entry=140736552432368, arg2=arg2@entry=0)
at fmgr.c:1326
#10 0x004a6c2d in index_vacuum_cleanup (info=info@entry=0x7fffc836f2f0, 
stats=stats@entry=0x0) at indexam.c:715
#11 0x005570d1 in do_analyze_rel (onerel=onerel@entry=0x7f4429052e68, 
acquirefunc=0x556020 acquire_sample_rows, relpages=45, inh=inh@entry=0 
'\000', 
elevel=elevel@entry=13, vacstmt=error reading variable: Unhandled dwarf 
expression opcode 0xfa, 
vacstmt=error reading variable: Unhandled dwarf expression opcode 0xfa) 
at analyze.c:634
#12 0x00557fef in analyze_rel (relid=relid@entry=16384, 
vacstmt=vacstmt@entry=0x1af5678, bstrategy=optimized out) at analyze.c:267
---Type return to continue, or q return to quit---
#13 0x005aa224 in vacuum (vacstmt=vacstmt@entry=0x1af5678, 
relid=relid@entry=0, do_toast=do_toast@entry=1 '\001', bstrategy=optimized 
out, 
bstrategy@entry=0x0, for_wraparound=for_wraparound@entry=0 '\000', 
isTopLevel=isTopLevel@entry=1 '\001') at vacuum.c:249
#14 0x0069f417 in standard_ProcessUtility (parsetree=0x1af5678, 
queryString=optimized out, context=optimized out, params=0x0, 
dest=optimized out, 
completionTag=optimized out) at utility.c:682
#15 0x0069c587 in PortalRunUtility (portal=0x1b33198, 
utilityStmt=0x1af5678, isTopLevel=1 '\001', dest=0x1af5a00, 
completionTag=0x7fffc836f920 )
at pquery.c:1187
#16 0x0069d299 in PortalRunMulti (portal=portal@entry=0x1b33198, 
isTopLevel=isTopLevel@entry=1 '\001', dest=dest@entry=0x1af5a00, 
altdest=altdest@entry=0x1af5a00, 
completionTag=completionTag@entry=0x7fffc836f920 ) at pquery.c:1318
#17 0x0069df32 in PortalRun (portal=portal@entry=0x1b33198, 
count=count@entry=9223372036854775807, isTopLevel=isTopLevel@entry=1 '\001', 
dest=dest@entry=0x1af5a00, altdest=altdest@entry=0x1af5a00, 
completionTag=completionTag@entry=0x7fffc836f920 ) at pquery.c:816
#18 0x0069afdb in exec_simple_query (query_string=0x1af4c18 analyze 
t1;) at postgres.c:1048
#19 PostgresMain (argc=optimized out, argv=argv@entry=0x1ab0a70, 
dbname=0x1ab08f0 postgres, username=optimized out) at postgres.c:3992
#20 0x0046559e in BackendRun (port=0x1ab2770) at postmaster.c:4083
#21 BackendStartup (port=0x1ab2770) at postmaster.c:3772
#22 ServerLoop () at postmaster.c:1583
#23 0x0065230e in PostmasterMain (argc=argc@entry=3, 
argv=argv@entry=0x1a8df00) at postmaster.c:1239
#24 0x00465ec5 in main (argc=3

Re: [HACKERS] Minmax indexes

2013-09-17 Thread Jaime Casanova
On Tue, Sep 17, 2013 at 3:30 AM, Thom Brown t...@linux.com wrote:
 On 17 September 2013 07:20, Jaime Casanova ja...@2ndquadrant.com wrote:

 On Mon, Sep 16, 2013 at 3:47 AM, Thom Brown t...@linux.com wrote:
  On 15 September 2013 01:14, Alvaro Herrera alvhe...@2ndquadrant.com
  wrote:
 
  Hi,
 
  Here's a reviewable version of what I've dubbed Minmax indexes.
 
  Thanks for the patch, but I seem to have immediately hit a snag:
 
  pgbench=# CREATE INDEX minmaxtest ON pgbench_accounts USING minmax
  (aid);
  PANIC:  invalid xlog record length 0
 

 fwiw, this seems to be triggered by ANALYZE.
 At least i can trigger it by executing ANALYZE on the table (attached
 is a stacktrace of a backend exhibiting the failure)


 I'm able to run ANALYSE manually without it dying:


try inserting some data before the ANALYZE, that will force a
resumarization which is mentioned in the stack trace of the failure

-- 
Jaime Casanova www.2ndQuadrant.com
Professional PostgreSQL: Soporte 24x7 y capacitación
Phone: +593 4 5107566 Cell: +593 987171157


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


Re: [HACKERS] Minmax indexes

2013-09-17 Thread Jaime Casanova
On Tue, Sep 17, 2013 at 8:43 AM, Thom Brown t...@linux.com wrote:
 On 17 September 2013 14:37, Jaime Casanova ja...@2ndquadrant.com wrote:

 On Tue, Sep 17, 2013 at 3:30 AM, Thom Brown t...@linux.com wrote:
  On 17 September 2013 07:20, Jaime Casanova ja...@2ndquadrant.com
  wrote:
 
  On Mon, Sep 16, 2013 at 3:47 AM, Thom Brown t...@linux.com wrote:
   On 15 September 2013 01:14, Alvaro Herrera alvhe...@2ndquadrant.com
   wrote:
  
   Hi,
  
   Here's a reviewable version of what I've dubbed Minmax indexes.
  
   Thanks for the patch, but I seem to have immediately hit a snag:
  
   pgbench=# CREATE INDEX minmaxtest ON pgbench_accounts USING minmax
   (aid);
   PANIC:  invalid xlog record length 0
  
 
  fwiw, this seems to be triggered by ANALYZE.
  At least i can trigger it by executing ANALYZE on the table (attached
  is a stacktrace of a backend exhibiting the failure)
 
 
  I'm able to run ANALYSE manually without it dying:
 

 try inserting some data before the ANALYZE, that will force a
 resumarization which is mentioned in the stack trace of the failure


 I've tried inserting 1 row then ANALYSE and 10,000 rows then ANALYSE, and in
 both cases there's no error.  But then trying to create the index again
 results in my original error.


Ok

So, please confirm if this is the pattern you are following:

CREATE TABLE t1(i int);
INSERT INTO t1 SELECT generate_series(1, 1);
CREATE INDEX idx1 ON t1 USING minmax (i);

if that, then the attached stack trace (index_failure_thom.txt) should
correspond to the failure you are looking.

My test was slightly different:

CREATE TABLE t1(i int);
CREATE INDEX idx1 ON t1 USING minmax (i);
INSERT INTO t1 SELECT generate_series(1, 1);
ANALYZE t1;

and the failure happened in a different time, in resumarization
(attached index_failure_jcm.txt)

but in the end, both failures seems to happen for the same reason: a
record of length 0... at XLogInsert time

#4  XLogInsert at xlog.c:966
#5  mmSetHeapBlockItemptr at mmrevmap.c:169
#6  mm_doinsert at minmax.c:1410

actually, if you create a temp table both tests works fine

-- 
Jaime Casanova www.2ndQuadrant.com
Professional PostgreSQL: Soporte 24x7 y capacitación
Phone: +593 4 5107566 Cell: +593 987171157
Program received signal SIGABRT, Aborted.
0x7f4428819475 in *__GI_raise (sig=optimized out) at 
../nptl/sysdeps/unix/sysv/linux/raise.c:64
64  ../nptl/sysdeps/unix/sysv/linux/raise.c: No existe el fichero o el 
directorio.
(gdb) bt
#0  0x7f4428819475 in *__GI_raise (sig=optimized out) at 
../nptl/sysdeps/unix/sysv/linux/raise.c:64
#1  0x7f442881c6f0 in *__GI_abort () at abort.c:92
#2  0x0076d6ac in errfinish (dummy=dummy@entry=0) at elog.c:546
#3  0x0076fdca in elog_finish (elevel=elevel@entry=22, 
fmt=fmt@entry=0x7d2aa7 invalid xlog record length %u) at elog.c:1304
#4  0x004e1eba in XLogInsert (rmid=rmid@entry=17 '\021', 
info=info@entry=48 '0', rdata=rdata@entry=0x7fffc836ea10) at xlog.c:966
#5  0x004a9bb9 in mmSetHeapBlockItemptr 
(rmAccess=rmAccess@entry=0x1b3af38, heapBlk=heapBlk@entry=0, 
blkno=blkno@entry=6, offno=offno@entry=1)
at mmrevmap.c:169
#6  0x004a73e8 in mm_doinsert (idxrel=0x7f4429054c88, 
rmAccess=0x1b3af38, buffer=buffer@entry=0x7f441f7e718c, heapblkno=0, 
tup=tup@entry=0x7f441f84cff8, 
itemsz=16) at minmax.c:1410
#7  0x004a9464 in rerun_summarization (numnonsummarized=22, 
nonsummarized=0x1b3a408, rmAccess=0x1b3af38, heapRel=0x7f4429052e68, 
idxRel=0x7f4429054c88)
at minmax.c:1205
#8  mmvacuumcleanup (fcinfo=optimized out) at minmax.c:1268
#9  0x0077388f in FunctionCall2Coll 
(flinfo=flinfo@entry=0x7fffc836f0a0, collation=collation@entry=0, 
arg1=arg1@entry=140736552432368, arg2=arg2@entry=0)
at fmgr.c:1326
#10 0x004a6c2d in index_vacuum_cleanup (info=info@entry=0x7fffc836f2f0, 
stats=stats@entry=0x0) at indexam.c:715
#11 0x005570d1 in do_analyze_rel (onerel=onerel@entry=0x7f4429052e68, 
acquirefunc=0x556020 acquire_sample_rows, relpages=45, inh=inh@entry=0 
'\000', 
elevel=elevel@entry=13, vacstmt=error reading variable: Unhandled dwarf 
expression opcode 0xfa, 
vacstmt=error reading variable: Unhandled dwarf expression opcode 0xfa) 
at analyze.c:634
#12 0x00557fef in analyze_rel (relid=relid@entry=16384, 
vacstmt=vacstmt@entry=0x1af5678, bstrategy=optimized out) at analyze.c:267
---Type return to continue, or q return to quit---
#13 0x005aa224 in vacuum (vacstmt=vacstmt@entry=0x1af5678, 
relid=relid@entry=0, do_toast=do_toast@entry=1 '\001', bstrategy=optimized 
out, 
bstrategy@entry=0x0, for_wraparound=for_wraparound@entry=0 '\000', 
isTopLevel=isTopLevel@entry=1 '\001') at vacuum.c:249
#14 0x0069f417 in standard_ProcessUtility (parsetree=0x1af5678, 
queryString=optimized out, context=optimized out, params=0x0, 
dest=optimized out, 
completionTag=optimized out) at utility.c:682
#15 0x0069c587 in PortalRunUtility (portal

Re: [HACKERS] Assertions in PL/PgSQL

2013-09-15 Thread Jaime Casanova
El 14/09/2013 15:25, Pavel Stehule pavel.steh...@gmail.com escribió:

 Hello

 There is a significant issue - new reserved keyword. There is high
probability so lot of users has a functions named assert.

 I like this functionality, but I dislike any compatibility break for
feature, that can be implemented as extension without any lost of
compatibility or lost of functionality.

 So can you redesign this without new keyword?


Hi,

If using ASSERT as keyword is not acceptable, not that i agree but in case.
What about using RAISE EXCEPTION WHEN (condition)

--
Jaime Casanova
2ndQuadrant: Your PostgreSQL partner


Re: [HACKERS] Assertions in PL/PgSQL

2013-09-15 Thread Jaime Casanova
El 15/09/2013 17:13, Marko Tiikkaja ma...@joh.to escribió:

 On 2013-09-15 23:23, Jaime Casanova wrote:

 If using ASSERT as keyword is not acceptable, not that i agree but in
case.
 What about using RAISE EXCEPTION WHEN (condition)


 I think it would be extremely surprising if a command like that got
optimized away based on a GUC, so I don't think that would be a good idea.



Ah! good point, didn't think on that

--
Jaime Casanova
2ndQuadrant: Your PostgreSQL partner


Re: [HACKERS] [PATCH] bitmap indexes

2013-09-15 Thread Jaime Casanova
On Sat, Sep 14, 2013 at 1:14 PM, Abhijit Menon-Sen a...@2ndquadrant.com wrote:
 Hi.

 This is a cleaned-up and rebased version of the bitmap index patch from
 Gavin Sherry, later revised by Gianni Ciolli and Gabriele Bartolini, and
 others including Daniel Bausch.


Hi Abhijit,

Please, in the next update consider this messages i'm getting when
compiling with your patch.

bitmapxlog.c: In function ‘bitmap_xlog_cleanup’:
bitmapxlog.c:658:32: warning: ‘reln’ may be used uninitialized in this
function [-Wuninitialized]
selfuncs.c: In function ‘bmcostestimate’:
selfuncs.c:7327:13: warning: unused variable ‘indexCorrelation’
[-Wunused-variable]
selfuncs.c:7326:15: warning: unused variable ‘indexSelectivity’
[-Wunused-variable]
selfuncs.c:7325:11: warning: unused variable ‘indexTotalCost’
[-Wunused-variable]
selfuncs.c:7324:11: warning: unused variable ‘indexStartupCost’
[-Wunused-variable]


Also, there are 2 regression tests failing (attached regression.diffs)

And this error, when trying to generate docs

openjade:bitmap.sgml:123:85:X: reference to non-existent ID
SQL-CREATEINDEX-TITLE


And finally, i was excercising the feature in some ways and got a
crash when creating an index concurrently (attached
index_failure.txt), it wasn't just a crash i couldn't start up the
server again after it

-- 
Jaime Casanova www.2ndQuadrant.com
Professional PostgreSQL: Soporte 24x7 y capacitación
Phone: +593 4 5107566 Cell: +593 987171157
postgres=# create index concurrently  idx_t1_i on t1 using bitmap (i);
TRAP: FailedAssertion(!(GetTopTransactionIdIfAny() == ((TransactionId) 0)), 
File: index.c, Line: 3011)
La conexión al servidor se ha perdido. Intentando reiniciar: LOG:  server 
process (PID 20955) was terminated by signal 6: Aborted
DETAIL:  Failed process was running: create index concurrently  idx_t1_i on t1 
using bitmap (i);
LOG:  terminating any other active server processes
WARNING:  terminating connection because of crash of another server process
DETAIL:  The postmaster has commanded this server process to roll back the 
current transaction and exit, because another server process exited abnormally 
and possibly corrupted shared memory.
HINT:  In a moment you should be able to reconnect to the database and repeat 
your command.
FATAL:  the database system is in recovery mode
falló.
Duración: 239,212 ms
! LOG:  all server processes terminated; reinitializing
LOG:  database system was interrupted; last known up at 2013-09-15 23:21:15 ECT
LOG:  database system was not properly shut down; automatic recovery in progress
LOG:  redo starts at 0/AA9C4E8
PANIC:  _bitmap_xlog_insert_last_bitmapwords: VMI block not found: 5
CONTEXT:  xlog redo insert words in a not-last bitmap page: rel 1663/12937/70025
LOG:  startup process (PID 24399) was terminated by signal 6: Aborted
LOG:  aborting startup due to startup process failure



regression.diffs
Description: Binary data

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


Re: [HACKERS] Assertions in PL/PgSQL

2013-09-14 Thread Jaime Casanova
On Sat, Sep 14, 2013 at 1:52 PM, Marko Tiikkaja ma...@joh.to wrote:
 On 14/09/2013 20:47, I wrote:

 These are
 similar to the Assert() backend macro: they can be disabled during
 compile time, but when enabled, abort execution if the passed expression
 is not true.


Hi,

That's very good, thanks.


 And by compile time here, I mean at the time when the PL/PgSQL function
is
 compiled, not the postgres server binary.


and compile time means when the function is created or replaced? or the
first time is used?

if the second. Why not have a plpgsql.enable_assert variable?
A directive you can use per function

then i can keep the ASSERT and activate them by replacing the function

-- 
Jaime Casanova www.2ndQuadrant.com
Professional PostgreSQL: Soporte 24x7 y capacitación
Phone: +593 4 5107566 Cell: +593 987171157


Re: [HACKERS] Assertions in PL/PgSQL

2013-09-14 Thread Jaime Casanova
El 14/09/2013 15:18, Marko Tiikkaja ma...@joh.to escribió:

 On 2013-09-14 21:55, Jaime Casanova wrote:

 On Sat, Sep 14, 2013 at 1:52 PM, Marko Tiikkaja ma...@joh.to wrote:

 And by compile time here, I mean at the time when the PL/PgSQL
function

 is

 compiled, not the postgres server binary.


 and compile time means when the function is created or replaced? or the
 first time is used?


 Uhh..  I have to admit, I'm not sure.  I think this would be when you
CREATE the function for the backend that created the function, and on the
first call in other backends.


 if the second. Why not have a plpgsql.enable_assert variable?
 A directive you can use per function

 then i can keep the ASSERT and activate them by replacing the function


 The patch supports a compiler option to override the global option and
enable it per function:

 create function foof()
 returns void
 as $$
 #enable_assertions
 begin
 -- failure
 assert 1  2;
 end
 $$ language plpgsql;


 Does this address your wishes?



That's exactly what i wanted. thanks

--
Jaime Casanova
2ndQuadrant: Your PostgreSQL partner


Re: [HACKERS] Differences in WHERE clause of SELECT

2013-07-10 Thread Jaime Casanova
On Wed, Jul 10, 2013 at 8:42 PM, Prabakaran, Vaishnavi
vaishna...@fast.au.fujitsu.com wrote:
 Hi Berkus,

 Thanks for your time and response.

 I do understand that there is no LIKE operator support for integers and it 
 would be great if you could help me understand the reason why is it not 
 supported.

 My intention is to know whether this is not supported because of any 
 technical limitation or is it against any Postgresql/SQL standards.


the latter

 My use cases are like below ones :
 Integer LIKE pattern [ESCAPE escape-character]
 1. List all the customers who are having negative balance:
 SELECT * from Customer where balance LIKE ‘-%’


this is not cleaner implemented this way?
SELECT * FROM customer WHERE balance  0;

 2. List all the customers whose id starts with 1:
 SELECT * from Customer where cust_id LIKE ‘1%’


there is any real use for that query? i understand if you ask
for all customers whose names begins with 'A' but that the code
begins with '1'?

--
Jaime Casanova www.2ndQuadrant.com
Professional PostgreSQL: Soporte 24x7 y capacitación
Phone: +593 4 5107566 Cell: +593 987171157


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


Re: [HACKERS] in-catalog Extension Scripts and Control parameters (templates?)

2013-07-07 Thread Jaime Casanova
On Sat, Jul 6, 2013 at 2:25 PM, Dimitri Fontaine dimi...@2ndquadrant.fr wrote:
 Jaime Casanova ja...@2ndquadrant.com writes:
 not sure if you're wrong. but at the very least, you miss a
 heap_freetuple(oldtup) there, because get_catalog_object_by_oid()

 Well, oldtup can be either a syscache copy or a heap tuple. I've been
 looking at other call sites and they don't free their tuple either. So
 I'm leaving it at that for now.

 no, that code is not unchanged because function
 get_ext_ver_list_from_catalog() comes from your patch.

 Yes. Here's the relevant hunk:


No. That's get_ext_ver_list_from_files() and that function is
unchanged (except for
the name). I'm talking about get_ext_ver_list_from_catalog() which is
a different
function.

--
Jaime Casanova www.2ndQuadrant.com
Professional PostgreSQL: Soporte 24x7 y capacitación
Phone: +593 4 5107566 Cell: +593 987171157


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


Re: [HACKERS] in-catalog Extension Scripts and Control parameters (templates?)

2013-07-04 Thread Jaime Casanova
On Mon, Jun 24, 2013 at 6:20 AM, Dimitri Fontaine
dimi...@2ndquadrant.fr wrote:
 Jaime Casanova ja...@2ndquadrant.com writes:
 just tried to build this one, but it doesn't apply cleanly anymore...
 specially the ColId_or_Sconst contruct in gram.y

 Please find attached a new version of the patch, v7, rebased to current
 master tree and with some more cleanup. I've been using the new grammar
 entry NonReservedWord_or_Sconst, I'm not sure about that.


Hi,

code review (haven't read all the code)


- The error message in aclchk.c:5175 isn't very clear, i mean the user
should  see something better than uptmpl

if (!HeapTupleIsValid(tuple))
   ereport(ERROR,
(errcode(ERRCODE_UNDEFINED_OBJECT),
 errmsg(extension uptmpl with OID %u does not exist,
   ext_uptmpl_oid)));


- In alter.c you made AlterObjectRename_internal non static and
replaced a SearchSysCache1 call with a get_catalog_object_by_oid one.
Now, in its comment that function says that is for simple cases. And
because of the things you're doing it seems to me this isn't a simple
case. Maybe instead of modifying it you should create other function
RenameExtensionTemplateInternal, just like tablecmd.c does?

btw, get_catalog_object_by_oid will execute a SearchSysCacheCopy1 so
should be calling heap_freetuple(oldtuple)

- This is a typo i guess: AtlerExtensionTemplateRename

- In event_triggers.c, it seems the intention was to keep the
event_trigger_support array in order, any reason to for not doing it?

- extension.c: In function ‘get_ext_ver_list_from_catalog’:
extension.c:1150:25: warning: variable ‘evi’ set but not used
[-Wunused-but-set-variable]


Functionality
===

i tried this:

create template for extension test version 'abc' with (nosuperuser) as $$
  create function f1(i int) returns int as $_$ select 1; $_$
language sql;
$$;
CREATE TEMPLATE FOR EXTENSION

create template for extension test from 'abc' to 'xyz' with (nosuperuser) as $$
  create function f2(i int) returns int as $_$ select 1; $_$
language sql;
$$;
CREATE TEMPLATE FOR EXTENSION

create template for extension test from 'xyz' to '123' with (nosuperuser) as $$
  create function f3(i int) returns int as $_$ select 1; $_$
language sql;
$$;
CREATE TEMPLATE FOR EXTENSION

create extension test version '123';
CREATE EXTENSION

postgres=# \df
   List of functions
 Schema | Name | Result data type | Argument data types | Type
+--+--+-+--
(0 rows)

Actually, what this did was to create an 123 schema and it puts the
functions there.

But that schema is inaccesible and undroppable:

select * from 123.f1(1);
ERROR:  schema 123 does not exist

drop schema 123;
ERROR:  schema 123 does not exist

--

postgres=# create template for extension test2 version '1.0' with
(nosuperuser) as $$
  create function f1(i int) returns int as $_$ select 1; $_$
language sql;
$$;
CREATE TEMPLATE FOR EXTENSION

postgres=# create template for extension test2 from '1.0' to '1.1'
with (nosuperuser) as $$
  create function f2(i int) returns int as $_$ select 1; $_$
language sql;
$$;
CREATE TEMPLATE FOR EXTENSION

postgres=# create template for extension test2 from '1.0' to '2.1'
with (nosuperuser) as $$
  create function f3(i int) returns int as $_$ select 1; $_$
language sql;
$$;
CREATE TEMPLATE FOR EXTENSION

postgres=# create extension test2 version '2.1';
CREATE EXTENSION


In this case only f1() and f3() exists, because the extension is going
from 1.0 to 2.1. is this expected?
and, yes... the functions are in the schema 2.1

--
Jaime Casanova www.2ndQuadrant.com
Professional PostgreSQL: Soporte 24x7 y capacitación
Phone: +593 4 5107566 Cell: +593 987171157


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


Re: [HACKERS] in-catalog Extension Scripts and Control parameters (templates?)

2013-07-04 Thread Jaime Casanova
On Thu, Jul 4, 2013 at 2:42 AM, Jaime Casanova ja...@2ndquadrant.com wrote:

 create extension test version '123';
 CREATE EXTENSION

 postgres=# \df
List of functions
  Schema | Name | Result data type | Argument data types | Type
 +--+--+-+--
 (0 rows)

 Actually, what this did was to create an 123 schema and it puts the
 functions there.

 But that schema is inaccesible and undroppable:


and dropping the extension let the schema around

--
Jaime Casanova www.2ndQuadrant.com
Professional PostgreSQL: Soporte 24x7 y capacitación
Phone: +593 4 5107566 Cell: +593 987171157


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


Re: [HACKERS] in-catalog Extension Scripts and Control parameters (templates?)

2013-07-04 Thread Jaime Casanova
On Thu, Jul 4, 2013 at 7:32 AM, Dimitri Fontaine dimi...@2ndquadrant.fr wrote:

 - In alter.c you made AlterObjectRename_internal non static and
 replaced a SearchSysCache1 call with a get_catalog_object_by_oid one.
 Now, in its comment that function says that is for simple cases. And
 because of the things you're doing it seems to me this isn't a simple
 case. Maybe instead of modifying it you should create other function
 RenameExtensionTemplateInternal, just like tablecmd.c does?

 The get_catalog_object_by_oid() is doing a SearchSysCache1 when that's
 relevant and possible, so I don't think I changed enough things around
 to warrant a different API. I'm open to hearing about why I'm wrong if
 that's the case, though.


not sure if you're wrong. but at the very least, you miss a
heap_freetuple(oldtup) there, because get_catalog_object_by_oid()


 - extension.c: In function ‘get_ext_ver_list_from_catalog’:
 extension.c:1150:25: warning: variable ‘evi’ set but not used
 [-Wunused-but-set-variable]

 I don't have the warning here, and that code is unchanged from master's
 branch, only the name of the function did change. Do you have the same
 warning with master? which version of gcc are you using?


no, that code is not unchanged because function
get_ext_ver_list_from_catalog() comes from your patch.
it's seems the thing is that function get_ext_ver_info() is append a
new ExtensionVersionInfo which is then returned and assigned to an
*evi pointer that is never used.

i'm sure that evi in line 1150 is only because you need to receive the
returned value. Maybe you could use (void) get_ext_ver_info() (or it
should be (void *)?) to avoid that assignment and keep compiler quiet

PS: i'm on gcc (Debian 4.7.2-5) 4.7.2

--
Jaime Casanova www.2ndQuadrant.com
Professional PostgreSQL: Soporte 24x7 y capacitación
Phone: +593 4 5107566 Cell: +593 987171157


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


Re: [HACKERS] in-catalog Extension Scripts and Control parameters (templates?)

2013-06-23 Thread Jaime Casanova
On Wed, Apr 3, 2013 at 8:29 AM, Dimitri Fontaine dimi...@2ndquadrant.fr wrote:
 Hi,

 Dimitri Fontaine dimi...@2ndquadrant.fr writes:
 Documentation doesn't build, multiple errors. In addition to the reference
 pages, there should be a section in the main docs about these templates.

 I'm now working on that, setting up the documentation tool set.

 Fixed in the attached version 6 of the patch.


just tried to build this one, but it doesn't apply cleanly anymore...
specially the ColId_or_Sconst contruct in gram.y

--
Jaime Casanova www.2ndQuadrant.com
Professional PostgreSQL: Soporte 24x7 y capacitación
Phone: +593 4 5107566 Cell: +593 987171157


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


Re: [HACKERS] problem with commitfest redirection

2013-06-21 Thread Jaime Casanova
On Fri, Jun 21, 2013 at 8:56 PM, Martín Marqués mar...@2ndquadrant.com wrote:
 When ever I try to see the patch from this commit it never loads:

 https://commitfest.postgresql.org/action/patch_view?id=1129

 Some problem there? I can see other patches, from other commits.


Yes, the URL is wrong. right URL is
http://www.postgresql.org/message-id/CAFjNrYuh=4Vwnv=2n7cj0jjuwc4hool1epxsoflj6s19u02...@mail.gmail.com


--
Jaime Casanova www.2ndQuadrant.com
Professional PostgreSQL: Soporte 24x7 y capacitación
Phone: +593 4 5107566 Cell: +593 987171157


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


Re: [HACKERS] Hard limit on WAL space used (because PANIC sucks)

2013-06-06 Thread Jaime Casanova
On Thu, Jun 6, 2013 at 4:28 PM, Christian Ullrich ch...@chrullrich.net wrote:
 * Heikki Linnakangas wrote:

 The current situation is that if you run out of disk space while writing
 WAL, you get a PANIC, and the server shuts down. That's awful. We can


 So we need to somehow stop new WAL insertions from happening, before
 it's too late.


 A naive idea is to check if there's enough preallocated WAL space, just
 before inserting the WAL record. However, it's too late to check that in


 There is a database engine, Microsoft's Jet Blue aka the Extensible
 Storage Engine, that just keeps some preallocated log files around,
 specifically so it can get consistent and halt cleanly if it runs out of
 disk space.


fwiw, informix (at least until IDS 2000, not sure after that) had the
same thing. only this was a parameter to set, and bad things happened
if you forgot about it :D

--
Jaime Casanova www.2ndQuadrant.com
Professional PostgreSQL: Soporte 24x7 y capacitación
Phone: +593 4 5107566 Cell: +593 987171157


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


Re: [HACKERS] Extent Locks

2013-05-28 Thread Jaime Casanova
On Tue, May 28, 2013 at 7:36 AM, Stephen Frost sfr...@snowman.net wrote:

 On the other hand, I do feel like people are worried about
 over-extending a relation and wasting disk space- but with the way that
 vacuum can clean up pages at the end, that would only be a temporary
 situation anyway.


Hi,

Maybe i'm wrong but this should be easily solved by an
autovacuum_no_truncate_empty_pages or an autovacuum_empty_pages_limit
GUC/reloption.
Just to clarify the second one autovacuum will allow until that limit
of empty pages, and will remove excess from there

We can also think in GUC/reloption for next_extend_blocks so formula
is needed, or of course the automated calculation that has been
proposed

--
Jaime Casanova www.2ndQuadrant.com
Professional PostgreSQL: Soporte 24x7 y capacitación
Phone: +593 4 5107566 Cell: +593 987171157


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


Re: [HACKERS] Extent Locks

2013-05-28 Thread Jaime Casanova
On Tue, May 28, 2013 at 8:38 AM, Jaime Casanova ja...@2ndquadrant.com wrote:

 We can also think in GUC/reloption for next_extend_blocks so formula
 is needed, or of course the automated calculation that has been
 proposed


s/so formula is needed/so *no* formula is needed

btw, we can also use a next_extend_blocks GUC/reloption as a limit for
autovacuum so it will allow that empty pages at the end of the table

--
Jaime Casanova www.2ndQuadrant.com
Professional PostgreSQL: Soporte 24x7 y capacitación
Phone: +593 4 5107566 Cell: +593 987171157


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


Re: [HACKERS] Extent Locks

2013-05-28 Thread Jaime Casanova
On Tue, May 28, 2013 at 10:53 AM, Andres Freund and...@2ndquadrant.com wrote:

 But I agree. This needs to work without much manual intervention. I
 think we just need to make autovacuum truncate only if it finds more
 free space than whatever amount we might have added at that relation
 size plus some slop.


And how do you decide the amount of that slop?

--
Jaime Casanova www.2ndQuadrant.com
Professional PostgreSQL: Soporte 24x7 y capacitación
Phone: +593 4 5107566 Cell: +593 987171157


-- 
Sent 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 incompatibilities for Postgres 10.0

2013-05-28 Thread Jaime Casanova
On Tue, May 28, 2013 at 4:26 PM, Joshua D. Drake j...@commandprompt.com wrote:

 On 05/28/2013 02:18 PM, Bruce Momjian wrote:

 I would like to see the ability to define if a query is read only at
 the protocol level, so that load balances that speak libpq can know
 what to do with the query without parsing it.


 Sounds nice, but how would we do that?  That would require libpq to know
 it, right?  Do we pass anything back after parsing but before execution?
   Could it be optional?  What about functions that modify the database
 --- isn't that only known at execution time?


 I can't speak to the actual C code that would be required but from a user
 space, I could see something like this:

 con = psycopg2.connect(database='testdb', user='test', transaction-type='r')

 Thus when the connection is made, before anything else is done, we know it
 is a read only connection and therefore any load balancer speaking libpq
 would also know it is a read only. The default of course would be r/w and
 you would use a different connection handler for r/w or w queries.


you can do that today already, kind-of

create an entry in pgbouncer that connect to
host=read-only.servers.dns and make read-only.servers.dns to point to
more than 1 ip.
then when the application wants to do load balancing, just connect to
the entry that points to read-only.servers.dns and let the magic
happens

which would be great is this to happen transparently to the application

 The other option would be to do it on query execute but that doesn't seem as
 efficient as it would have to be parsed each time. Although it would still
 be better than reading the actual SQL.


another idea, as someone else mentioned, and i think has been
discussed bedore is a function that says if the query is r-o or not...
maybe even exporting the plan so we don't need to replan again...

Not sure if that is possible, just hand waving...


--
Jaime Casanova www.2ndQuadrant.com
Professional PostgreSQL: Soporte 24x7 y capacitación
Phone: +593 4 5107566 Cell: +593 987171157


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

2013-05-27 Thread Jaime Casanova
On Mon, May 27, 2013 at 10:32 AM, Magnus Hagander mag...@hagander.net wrote:
 It's been brought to my attention that I forgot to email hackers about us
 adding new committers to the project, as I planned to do when I wrote my
 blog post about it.

 This is the same people as were announced during the pgcon closing session -
 Jeff Davis, Stephen frost, fujii masao and Noah misch.


Congratulations for you guys...

--
Jaime Casanova www.2ndQuadrant.com
Professional PostgreSQL: Soporte 24x7 y capacitación
Phone: +593 4 5107566 Cell: +593 987171157


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


  1   2   3   4   5   6   7   8   9   >