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

2014-12-08 Thread Anssi Kääriäinen
On Fri, 2014-12-05 at 10:00 -0800, Josh Berkus wrote:
 I thought the point of INSERT ... ON CONFLICT update was so that you
 didn't have to care if it was a new row or not?
 
 If you do care, it seems like it makes more sense to do your own INSERTs
 and UPDATEs, as Django currently does.

Django tries to update the object if it already exists in the database.
If it doesn't, then Django does an insert. This is suboptimal from
concurrency standpoint, and does two round trips to the database instead
of just one.

For Django, both insert and update are OK when saving an object to the
database, but Django needs to know which one was done.

I too agree that this doesn't need to be handled in the first version of
the patch.

 - Anssi



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


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

2014-12-05 Thread Anssi Kääriäinen
On Fri, 2014-12-05 at 00:21 -0800, Peter Geoghegan wrote:
 On Thu, Dec 4, 2014 at 10:27 PM, Anssi Kääriäinen
 anssi.kaariai...@thl.fi wrote:
  For Django's use case this is a requirement. We must inform the user if
  the save() action created a new row or if it modified an existing one.
 
 Can you explain that in more detail, please?

Django has a post_save signal. The signal provide information of the
save operation. One piece of information is a created boolean flag. When
set to True the operation was an insert, on False it was an update.
See
https://docs.djangoproject.com/en/1.7/ref/signals/#django.db.models.signals.post_save
for details.

The created flag is typically used to perform some related action. An
example is User and UserProfile models. Each user must have an
UserProfile, so post_save can be used to create an empty userprofile on
creation of user.

If Django is going to use the INSERT ... ON CONFLICT UPDATE variant in
Django for the existing save() method, then it needs to know if the
result was an UPDATE or INSERT. If we are going to use this for other
operations (for example bulk merge of rows to the database), it would be
very convenient to have per-row updated/created information available so
that we can fire the post_save signals for the rows. If we don't have
that information available, it means we can't fire signals, and no
signals means we can't use the bulk merge operation internally as we
have to fire the signals where that happened before.

Outside of Django there are likely similar reasons to want to know if
the result of an operation was a creation of a new row. The reason could
be creation of related row, doing some action in application layer, or
just UI message telling object created successfully vs object updated
successfully.

 - Anssi



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


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

2014-12-04 Thread Anssi Kääriäinen
On Wed, 2014-11-26 at 16:59 -0800, Peter Geoghegan wrote:
 On Mon, Nov 24, 2014 at 1:03 PM, Peter Geoghegan p...@heroku.com wrote:
  Looks like the consensus is that we should have RETURNING project
  updated tuples too, then.
 
 Attached revision, v1.5, establishes this behavior (as always, there
 is a variant for each approach to value locking). There is a new
 commit with a commit message describing the new RETURNING/command tag
 behavior in detail, so no need to repeat it here. The documentation
 has been updated in these areas, too.

It seems there isn't any way to distinguish between insert and update of
given row. Maybe a pseudo-column can be added so that it can be used in
the returning statement:

  insert into foobar(id, other_col) values(2, '2') on conflict (id) update set 
other_col=excluded.other_col returning id, pseudo.was_updated;

This would ensure that users could check for each primary key value if
the row was updated or inserted.

Of course, the pseudo.was_updated name should be replaced by something
better.

It would be nice to be able to skip updates of rows that were not
changed:

   insert into foobar values(2, '2') on conflict (id) update set 
other_col=excluded.other_col where target is distinct from excluded;

 - Anssi Kääriäinen




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


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

2014-12-04 Thread Anssi Kääriäinen
On Thu, 2014-12-04 at 10:27 -0800, Peter Geoghegan wrote:

 I think that the standard for adding a new system attribute ought to
 be enormous. The only case where a new one was added post-Postgres95
 was tableoid. I'm pretty sure that others aren't going to want to do
 it that way. Besides, I'm not entirely convinced that this is actually
 an important distinction to expose.

For Django's use case this is a requirement. We must inform the user if
the save() action created a new row or if it modified an existing one.

Another way to do this would be to expose the excluded alias in the
returning clause. All columns of the excluded alias would be null in
the case of insert (especially the primary key column), and thus if a
query
insert into foobar values(2, '2') on conflict (id) update set 
other_col=excluded.other_col returning excluded.id
returns a non-null value, then it was an update.

 - Anssi




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


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

2014-11-20 Thread Anssi Kääriäinen
On Thu, 2014-11-20 at 13:42 -0800, Peter Geoghegan wrote:
  I am a developer of the Django ORM. Django reports to the user whether a
  row was inserted or updated. It is possible to know which rows were
  inserted by returning the primary key value. If something is returned,
  then it was an insert. If Django implements updated vs inserted checking
  this way, then if PostgreSQL adds RETURNING for update case later on,
  that would be a breaking change for Django.
 
 How does that actually work at the moment? Do you use RETURNING, or
 look at the command tag? Would you be happy to just know that certain
 rows were either inserted or updated in the context of an UPSERT (and
 not cancelled by a BEFORE ROW INSERT or UPDATE trigger returning
 NULL), or do you want to specifically know if there was an insert or
 an update in respect of each row/slot processed?

Django uses the command tag currently to check if a row was updated. We
also use RETURNING to get SERIAL values back from the database on
insert.

The most likely place to use this functionality in Django is
Model.save(). This method is best defined as make sure this object's
state is either inserted or updated to the database by the primary key
of the object. The Model.save() method needs to also report if the
model was created or updated. The command tag is sufficient for this
case.

So, the proposed feature now has everything Django needs for
Model.save().

Django might add a bulk_merge(objs) command later on. This is defined as
make sure each obj in objs is persisted to the database using the
fastest way available. The INSERT ON CONFLICT UPDATE command looks
excellent for this case. In this case it will be more problematic to
check which rows were inserted, which update, as we need information for
each primary key value separately for this case.

When I think of this feature outside of Django, it seems it is
completely reasonable to return database computed values on UPSERT. This
requires two queries with the proposed API. My opinion is that RETURNING
for the update case is better than not having it.

 - Anssi



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


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

2014-11-19 Thread Anssi Kääriäinen
On Wed, 2014-11-19 at 16:52 -0800, Peter Geoghegan wrote:
 Someone mentioned to me privately that they weren't sure that the
 question of whether or not RETURNING only projected actually inserted
 tuples was the right one. Also, I think someone else mentioned this a
 few months back. I'd like to address this question directly sooner
 rather than later, and so I've added a note on the Wiki page in
 relation to this [1]. It's a possible area of concern at this point.

I think the biggest problem with the current approach is that there is
no way to know if a row was skipped by the where clause when using
INSERT ON CONFLICT UPDATE ... WHERE.

I am a developer of the Django ORM. Django reports to the user whether a
row was inserted or updated. It is possible to know which rows were
inserted by returning the primary key value. If something is returned,
then it was an insert. If Django implements updated vs inserted checking
this way, then if PostgreSQL adds RETURNING for update case later on,
that would be a breaking change for Django.

So, if it is not too hard to implement RETURNING for the update case
then I think it should be done. A pseudo column informing if the result
was an update or insert would then be a requirement for Django. Changing
the returning behavior in later releases might cause problems due to
backwards compatibility.

 - Anssi




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

2014-11-09 Thread Anssi Kääriäinen
On Sun, 2014-11-09 at 11:57 -0500, Steve Singer wrote:

 The reason why Jim and myself are asking for the LSN and not just the 
 timestamp is that I want to be able to order the transactions. Jim 
 pointed out earlier in the thread that just ordering on timestamp allows 
 for multiple transactions with the same timestamp.
 
 Maybe we don't need the entire LSN to solve that.  If you already have 
 the commit timestamp maybe you only need another byte or two of 
 granularity to order transactions that are within the same microsecond.

There is no guarantee that a commit with later LSN has a later
timestamp. There are cases where the clock could move significantly
backwards.

A robust solution to storing transaction commit information (including
commit order) in a way that can be referenced from other tables, can be
loaded to another cluster, and survives crashes would be a great
feature. But this feature doesn't have those properties.

 - Anssi




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

2014-11-05 Thread Anssi Kääriäinen
On Tue, 2014-11-04 at 23:43 -0600, Jim Nasby wrote:

 I'm worried about 2 commits in the same microsecond on the same
 system, not on 2 different systems. Or, put another way, if we're
 going to expose this I think it should also provide a guaranteed
 unique commit ordering for a single cluster. Presumably, this
 shouldn't be that hard since we do know the exact order in which
 things committed.

Addition of LSN when the timestamps for two transactions are exactly
same isn't enough. There isn't any guarantee that a later commit gets a
later timestamp than an earlier commit.

In addition, I wonder if this feature would be misused. Record
transaction ids to a table to find out commit order (use case could be
storing historical row versions for example). Do a dump and restore on
another cluster, and all the transaction ids are completely meaningless
to the system.

Having the ability to record commit order into an audit table would be
extremely welcome, but as is, this feature doesn't provide it.

 - Anssi




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


Re: [HACKERS] Promise index tuples for UPSERT

2014-10-08 Thread Anssi Kääriäinen
On Tue, 2014-10-07 at 13:33 +0100, Simon Riggs wrote:
 Is there a way of detecting that we are updating a unique constraint
 column and then applying the HW locking only in that case? Or can we
 only apply locking when we have multiple unique constraints on a
 table?

What is the use case of doing an UPSERT into a table with multiple
unique constraints?

Consider table user with unique columns name and email and a non-unique
column age. If it has data

Jack | j...@example.com |33 
Tom | t...@example.com | 35

And the user does UPSERT values (Jack, t...@example.com, 34). The
proposed specification will pick random unique index (either name or
email index) and do an update of that row.

First, this will cause unique violation, second, doing the UPSERT on
random index seems confusing.

The MySQL documentation says that you should try to avoid using an ON
DUPLICATE KEY UPDATE clause on tables with multiple unique indexes[1].
The proposed feature's documentation has the same suggestion[2]. Still,
the feature defaults to this behavior. Why is the default something the
documentation says you shouldn't do?

Going a bit further, I am wondering what is the use case of doing an
UPSERT against multiple unique indexes? If multiple unique indexes
UPSERT could be dropped that might allow for faster or cleaner index
locking techniques.

 - Anssi

1: http://dev.mysql.com/doc/refman/5.6/en/insert-on-duplicate.html
2: 
http://postgres-benchmarks.s3-website-us-east-1.amazonaws.com/on-conflict-docs/sql-insert.html



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


Re: [HACKERS] Promise index tuples for UPSERT

2014-10-08 Thread Anssi Kääriäinen
On Wed, 2014-10-08 at 02:22 -0700, Peter Geoghegan wrote:
 On Wed, Oct 8, 2014 at 1:25 AM, Heikki Linnakangas
 hlinnakan...@vmware.com wrote:
  Instead of naming the index, you should name the columns, and the system can
  look up the index or indexes that match those columns.
 
 It's not totally clear that we need *any* WITHIN clause, BTW. I'm not
 dead set on it. It was something I mainly added at Kevin's request. I
 do see the risks, though.

To be usable in Django ORM's .save() method there must be an option to
use the primary key index, and only the primary key index for conflict
resolution.

Assume an author table with id SERIAL PRIMARY KEY, email TEXT UNIQUE,
age INTEGER, then when saving an object, Django must update the row with
matching primary key value, or otherwise do an insert. Doing an update
of matching email column isn't allowed. So, Django can't use the query:

   INSERT INTO author values(1, 't...@example.com', 35)
   ON CONFLICT UPDATE
   SET email = conflicting(email), age = conflicting(age);

If the table contains data (id=2, email='t...@example.com', age=34), the
query would update tom's age to 35 when it should have resulted in
unique constraint violation.

Other ORMs have similar save/persist implementations, that is objects
are persisted on primary key identity alone.

 - Anssi



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


Re: [HACKERS] Promise index tuples for UPSERT

2014-10-08 Thread Anssi Kääriäinen
On Wed, 2014-10-08 at 01:10 -0700, Peter Geoghegan wrote:
 On Wed, Oct 8, 2014 at 12:41 AM, Anssi Kääriäinen
 anssi.kaariai...@thl.fi wrote:
  The MySQL documentation says that you should try to avoid using an ON
  DUPLICATE KEY UPDATE clause on tables with multiple unique indexes[1].
  The proposed feature's documentation has the same suggestion[2]. Still,
  the feature defaults to this behavior. Why is the default something the
  documentation says you shouldn't do?

 As we all know, naming a unique index in DML is ugly, and has poor
 support in ORMs. It seems likely that we're better off making it
 optional - it hasn't been much of a problem with the existing subxact
 looping pattern.

The subxact approach is a bit different than the proposed UPSERT
command. It loops:

   try:
   INSERT INTO author VALUE('Jack', 't...@example.com', 34)
   except UniqueConstraintViolation:
   UPDATE author SET ... WHERE name = 'Jack'

while the UPSERT command does something like:

   try:
   INSERT INTO author VALUE('Jack', 't...@example.com', 34)
   except UniqueConstaintViolation:
   UPDATE author SET ... WHERE name = 'Jack' OR email = 't...@example.com' 
LIMIT 1;

 - Anssi



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


Re: [HACKERS] temporal support patch

2012-08-21 Thread Anssi Kääriäinen
I have written one approach to audit tables, available from 
https://github.com/akaariai/pgsql_shadow_tables


The approach is that every table is backed by a similar audit table + 
some meta information. The tables and triggers to update the audit 
tables are managed by plpgsql procedures.


While the approach isn't likely that interesting itself there is one 
interesting aspects. Views similar to the original tables are created 
automatically in the shadow schema. The views use a session variable for 
wanted snapshot time. The reason is that one can use this to query the 
database at wanted time:


set search_path = 'shadow_public, public';
set test_session_variable.view_time = 'wanted view timestamp'; -- for 
example '2012-05-06 22:08:00'


And now you can use exactly the same queries you use normally to 
retrieve data from wanted view timestamp. This is very useful if you 
happen to use an ORM.


In addition the known limitations mentioned in the README are likely 
something the temporal support patch needs to tackle.


 - Anssi


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


[HACKERS] Feature proposal: list role members in psql

2012-03-28 Thread Anssi Kääriäinen
It seems there is no way to get role members using psql. \du and \dg 
give you this role is a member of information, but no information 
about who have been granted the role.


I would like to write a patch implementing this feature. Some questions:
  - Is this wanted?
  - Should there be a new flag for this: \du[m+] where m list the role 
memebers. Or could this be added to the '+' flag?
  - Any pointers for patches implementing a similar addition to psql? 
That would help me a lot, as I do not know the code base.
  - Should the roles be fetched recursively, or should it be just the 
direct role members? My opinion is that direct role members is what is 
wanted.


The output would be something like:

 Role name |  Attributes  |   Member of| Role members
---+--++---
 hot2_dba  | Cannot login | {hot2_user,common_dba} | {akaariai}


 - Anssi Kääriäinen

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


Re: [HACKERS] So, is COUNT(*) fast now?

2011-10-31 Thread Anssi Kääriäinen

On 10/31/2011 02:44 PM, Robert Haas wrote:

What I think you're probably measuring here (oprofile would tell us
for sure) is that once the size of the table goes beyond about half a
gigabyte, it will have more than one page in the visibility map.  The
index-only scan code keeps the most recently used visibility map page
pinned to save on overhead, but if you're bouncing back and forth
between data in the first ~500MB of the table and data in the last
~100MB, each switch will result in dropping the current pin and
getting a new one, which figures to be fairly expensive.  With the
table is only a little over 500GB, you're probably only changing VM
pages every couple of tuples, but with a 6GB table just about every
tuple will switch to a new VM page.

Now, maybe you're right and the CPU caches are the more significant
effect.  But I wouldn't like to bet on it without seeing how much the
drop-and-get-new-pin operations are costing us.


Maybe I should have left the analysis part out of the post,
I don't know the internals, so my analysis is likely to be wrong.
Now that I think of it, claiming that the cache effect is 50%
of the runtime is likely a little wrong...

However the part about clustering being important is still correct.
According to the test, you can get 50% overhead because of
random access to the VM.

Stupid question, but why not keep the whole VM pinned?

 - Anssi

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


Re: [HACKERS] index-only scans

2011-08-16 Thread Anssi Kääriäinen

On 08/14/2011 12:31 AM, Heikki Linnakangas wrote:

The same idea could of course be used to calculate the effective cache hit 
ratio for each table. Cache hit ratio would have the problem of feedback loops, 
though.

Yeah, I'm not excited about making the planner and statistics more
dynamic. Feedback loops and plan instability are not fun.
I might be a little out of my league here... But I was thinking about 
the cache hit ratio and feedback loops. I understand automatic tuning 
would be hard. But making automatic tuning easier (by using pg_tune for 
example) would be a big plus for most use cases.


To make it easier to tune the page read costs automatically, it would be 
nice if there would be four variables instead of the current two:
  - random_page_cost is the cost of reading a random page from storage. 
Currently it is not, it is the cost of accessing a random page, taking 
in account it might be in memory.

  - seq_page_cost is the cost of reading pages sequentially from storage
  - memory_page_cost is the cost of reading a page in memory
  - cache_hit_ratio is the expected cache hit ratio

memory_page_cost would be server global, random and seq page costs 
tablespace specific, and cache_hit_ratio relation specific. You would 
get the current behavior by tuning *_page_costs realistically, and 
setting cache_hit_ratio globally so that the expected random_page_cost / 
seq_page_cost stays the same as now.


The biggest advantage of this would be that the correct values are much 
easier to detect automatically compared to current situation. This can 
be done using pg_statio_* views and IO speed testing. They should not be 
tuned automatically by PostgreSQL, at least not the cache_hit_ratio, as 
that leads to the possibility of feedback loops and plan instability. 
The variables would also be much easier to understand.


There is the question if one should be allowed to tune the *_page_costs 
at all. If I am not missing something, it is possible to detect the 
correct values programmatically and they do not change if you do not 
change the hardware. Cache hit ratio is the real reason why they are 
currently so important for tuning.


An example why the current random_page_cost and seq_page_cost tuning is 
not adequate is that you can only set random_page_cost per tablespace. 
That makes perfect sense if random_page_cost would be the cost of 
accessing a page in storage. But it is not, it is a combination of that 
and caching effects, so that it actually varies per relation (and over 
time). How do you set it correctly for a query where one relation is 
fully cached and another one not?


Another problem is that if you use random_page_cost == seq_page_cost, 
you are effectively saying that everything is in cache. But if 
everything is in cache, the cost of page access relative to cpu_*_costs 
is way off. The more random_page_cost and seq_page_cost are different, 
the more they mean the storage access costs. When they are the same, 
they mean the memory page cost. There can be an order of magnitude in 
difference of a storage page cost and a memory page cost. So it is hard 
to tune the cpu_*_costs realistically for cases where sometimes data is 
in cache and sometimes not.


Ok, enough hand waving for one post :) Sorry if this all is obvious / 
discussed before. My googling didn't turn out anything directly related, 
although these have some similarity:
 - Per-table random_page_cost for tables that we know are always cached 
[http://archives.postgresql.org/pgsql-hackers/2008-04/msg01503.php]

 - Script to compute random page cost
[http://archives.postgresql.org/pgsql-hackers/2002-09/msg00503.php]
-  The science of optimization in practical terms?
[http://archives.postgresql.org/pgsql-hackers/2009-02/msg00718.php], 
getting really interesting starting from here:

[http://archives.postgresql.org/pgsql-hackers/2009-02/msg00787.php]

 - Anssi


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


Re: [HACKERS] Transient plans versus the SPI API

2011-08-08 Thread Anssi Kääriäinen

On 08/07/2011 12:25 PM, Hannu Krosing wrote:

On Sun, 2011-08-07 at 11:15 +0200, Hannu Krosing wrote:

On Wed, 2011-08-03 at 15:19 -0400, Tom Lane wrote:

Hm, you mean reverse-engineering the parameterization of the query?

Yes, basically re-generate the query after (or while) parsing, replacing
constants and arguments with another set of generated arguments and
printing the list of these arguments at the end. It may be easiest to do
This in parallel with parsing.


Interesting thought, but I really don't see a way to make it practical.

Another place where this could be really useful is logging  monitoring

If there were an option to log the above queries as

SELECT a, b FROM foo WHERE c = $1, (123)
SELECT a, b FROM foo WHERE c = $1, (97)
SELECT a, b FROM foo WHERE c = $1, (236)

The main monitoring use_case would be pg_stat_statements,
http://developer.postgresql.org/pgdocs/postgres/pgstatstatements.html
which is currently pretty useless for queries without parameters


I was trying to implement something similar for pgpool-II. The user 
could configure queries for which cached plans are wanted. The 
configuration would have been a file containing lines in format SELECT 
* FROM foo WHERE id = ?. I did not get anything implemented, as there 
were some problems. The problems were mainly with DEALLOCATE ALL called 
without pgpool-II knowing it, issues with search_path and the amount of 
work needed to implement parse tree matching.


It would be interesting if pg_stat_statements would be globally 
available with queries using generic arguments. First, there would be an 
obvious heuristic for when to cache the plan: If the average runtime of 
the query is much larger than the average planning time, there is no 
point in caching the plan. This would also give one option for cache hit 
estimation. The hit_percent is directly available. On the other hand 
pg_stat_statements could easily become a choke-point.


I would love to work on this, but I lack the needed skills. Maybe I 
could take another try for writing a proof-of-concept parse tree 
transformer and matcher, but I doubt I can produce anything useful.


 - Anssi

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


Re: [HACKERS] Transient plans versus the SPI API

2011-08-08 Thread Anssi Kääriäinen

On 08/08/2011 01:07 PM, Hannu Krosing wrote:

That is why I think it is best done in the main parser - it has to parse
and analyse the query anyway and likely knows which constants are
arguments to the query
As far as I understand the problem, the parsing must transform table 
references to schema-qualified references. The table_foobar in SELECT * 
FROM table_foobar WHERE id = ? is not enough to identify a table. Using 
search_path, query_str as a key is one possibility, but the search_path 
is likely to be different for each user, and this could result in the 
same query being cached multiple times.


By the way, I checked current Git HEAD and pg_stat_statements seems to 
not handle search_path correctly. For pg_stat_statements this is not 
critical, but if the raw query string is used as plan cache key things 
will obviously break...


 - Anssi

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


Re: [HACKERS] pg_terminate_backend and pg_cancel_backend by not administrator user

2011-02-14 Thread Anssi Kääriäinen

On 02/14/2011 02:10 PM, Torello Querci wrote:

I suppose that give the right to the owner db user to terminate or
cancel other session connected to the database which it is owner is a
good thing.
I not see any security problem because this user can cancel or
terminate only the session related with the own database,
but if you think that this is a problem, a configuration parameter can be used.
For what it's worth, a big +1 from me. We have pretty much the same use 
case.


It would be good if you could also terminate your own connections.

 - Anssi

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


Re: [HACKERS] ALTER EXTENSION UPGRADE, v3

2011-02-11 Thread Anssi Kääriäinen

On 02/11/2011 05:05 AM, Tom Lane wrote:

Actually, I was having second thoughts about that while at dinner.  What
is the value of separating the bootstrap-an-extension-from-old-objects
operation into two steps?  It's certainly not convenient for users, and
I don't see that the intermediate state with an empty extension has any
redeeming social value for developers either.  (If you need such a thing,
just make an empty creation script.)

So: let's forget the concept of a special null version altogether, at
least from the user's-eye viewpoint.  Instead, the way to bootstrap from
loose objects is something like

CREATE EXTENSION foo [ VERSION '1.0' ] [ FROM OLD ]
The above command assumes there is only one unpackaged version from 
which users might update from. Is that what is wanted? I am wondering if 
FROM OLD should be FROM OLD VERSION version (or better: FROM UNPACKAGED 
VERSION version). This would also solve how to name the old version(s). 
Author decides.


 - Anssi

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


Re: [HACKERS] ALTER EXTENSION UPGRADE, v3

2011-02-03 Thread Anssi Kääriäinen

On 02/02/2011 08:22 PM, Dimitri Fontaine wrote:

Either one line in the Makefile or a new file with the \i equivalent
lines, that would maybe look like:

   SELECT pg_execute_sql_file('upgrade.v14.sql');
   SELECT pg_execute_sql_file('upgrade.v15.sql');

So well… I don't see how you've made it less gross here.
Chaining the upgrade files should be relatively easy, if something like 
pg_execute_sql_file would be available (actually it would need to be 
pg_execute_extension_file so that @extschema@ would be substituted 
correctly).


Example:

upgrade_from_1_0 = '1.0 = upgrade_from_1.0.sql'
upgrade_from_2_0 = '2.0 = upgrade_from_2.0.sql'
upgrade_from_3_0 = '3.0 = upgrade_from_3.0.sql'

upgrade_from_1.0.sql contents:
alter table foobar add column id2 integer;
pg_execute_extension_file('upgrade_from_2.0.sql');

upgrade_from_2.0.sql contents:
alter table foobar add column id3 integer;
pg_execute_extension_file('upgrade_from_3.0.sql');

...

So, when creating a new version you would need to update the main .sql 
file, create a new upgrade file, and alter the 
upgrade_from_previous_version.sql to include the new upgrade file. This 
should be relatively easy to maintain. Also, this would give you the 
freedom to not chain the files when that is not appropriate.


By the way, I saw that the character '.' is not allowed in the xxx part 
of upgrade_from_xxx and this is not documented in the patch. What can be 
in the xxx part, and is this documented somewhere else?


 - Anssi



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


Re: [HACKERS] ALTER EXTENSION UPGRADE, v3

2011-02-03 Thread Anssi Kääriäinen

On 02/03/2011 12:23 AM, Robert Haas wrote:

[..]
-- unconditional stuff

[..6]
-- stuff to do if coming from pre-7

[..]
-- some more unconditional stuff

[6..12]
-- stuff to do if coming from between 6 and 12

[..]
-- a few more unconditional things
This might be a stupid idea, but how about introducing another 
placeholder variable in addition to @extschema@ when upgrading, named 
maybe @fromversion@. Combined with do blocks and 
pg_execute_extension_file this would allow doing the above stuff:


upgrade.sql contents:
[..]
-- uncoditional stuff
[..6]
DO $$
begin
if @fromversion@ matches [..6] then
pg_execute_extension_file('stuff to do if coming from pre-7-file');
end if;
end;
$$
language 'plpgsql';
...

 - Anssi

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


Re: [HACKERS] SSI and Hot Standby

2011-01-21 Thread Anssi Kääriäinen

On 01/21/2011 03:25 AM, Florian Pflug wrote:

The COMMIT order in the actual, concurrent, schedule doesn't not necessarily
represent the order of the transaction in an equivalent serial schedule. Here's
an example

T1: BEGIN SERIALIZABLE; -- (Assume snapshot is set here)
T1: UPDATE D1 ... ;
T2: BEGIN SERIALIZABLE; -- (Assume snapshot is set here)
T2: SELECT * FROM D1 ... ;
T2: UPDATE D2 ... ;
T1: COMMIT;
T3: SELECT * FROM D1, D2;
T2: COMMIT;

Now, the COMMIT order is T1, T3, T2. Lets check if there is a equivalent
serial schedule. In any such schedule

T2 must run before T1 because T2 didn't see T1's changes to D1
T3 must run after T1 because T3 did see T1's changes to D1
T3 must run before T2 because T3 didn't see T2's changes to D2

This is obviously impossible - if T3 runs before T2 and T2 runs before T1
then T3 runs before T1, contradicting the second requirement. There is thus
no equivalent serial schedule and we must abort of these transactions with
a serialization error.

Note that aborting T3 is sufficient, even though T3 is READ ONLY!. With T3 gone,
an equivalent serial schedule is T2,T1!
Sorry for bothering all of you, but I just don't get this. What if T2 
rolls back instead of committing? Then the snapshot of T3 would have 
been valid, right? Now, for the snapshot of T3 it doesn't matter if T2 
commits or if it doesn't, because it can't see the changes of T2 in any 
case. Thus, it would seem that the snapshot is valid. On the other hand 
I can't see anything wrong in the logic in your post. What am I missing? 
I am feeling stupid...


At least for dumps I don't see how T2 can matter (assuming T3 is the 
pg_dump's snapshot). Because if you reload from the dump, T2 never 
happened in that dump. In the reloaded database it just did not exist at 
all.


 - Anssi

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


Re: [HACKERS] SSI and Hot Standby

2011-01-21 Thread Anssi Kääriäinen

On 01/21/2011 02:21 PM, Florian Pflug wrote:

Still, the would dump reflects a database state that *logically* never existed 
(i.e. not in any serial schedule). If you dump for disaster recovery, you might 
not care. If you dump to copy the data onto some reporting server you might.

best regards,
Florian Pflug


I am beginning to understand the problem. If you don't mind, here is a 
complete example if somebody else is having troubles understanding this.


Let's say we have tables D1 and D2. Both contain a single column, id, 
and a single row. The data in the beginning is as follows:


D1:  id = 1
D2:  id = 1

The constrains: D1.id can only be incremented. Whenever D2.id is 
updated, it must be updated to D1.id + 1.


The transactions:
T1: begin; update D1 set id = id + 1;
T2: begin; update D2 set id = (select id+1 from D1);
T1: commit;
T3: begin; select id from D1; select id from D2; commit; Data seen: (2, 
1) -- this is a possible state

T2: commit;
T4: begin; select id from D1; select id from D2; commit; Data seen: (2, 2)
This is again a possible state. But if we compare this to the state seen 
by T3 this is not valid. From state (2, 1) we can not get to state (2, 
2) without breaking one of the constraints. Thus, the state of T3 is not 
valid in the database.


So, I finally got it! :-) I hope this example will help somebody else 
understand the problem. The problem I had understanding this was that 
the state in T3 is in fact perfectly valid. I though that there must be 
some problem with that state alone. There isn't, unless you compare it 
to the state after T2 has committed.


Thanks to all explaining this to me,
 - Anssi

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


Re: [HACKERS] REVIEW: Extensions support for pg_dump

2011-01-18 Thread Anssi Kääriäinen

On 01/17/2011 07:58 PM, Kääriäinen Anssi wrote:

The issue I saw was this: assume you have an extension foo, containing one 
function, test().

CREATE EXTENSION foo;
DROP FUNCTION test();
-- restricted due to dependency

ALTER FUNCTION test() RENAME TO test2;
DROP FUNCTION test2();
-- not restricted!

The same can be done using CREATE OR REPLACE.

I hope this is not an error on my part. It is possible because I had a lot of 
schemas and my search_path might have been wrong...
The rename is an error on my part, sorry for that. Renaming can be done, 
but dropping is not possible even after rename. But a function in a 
package can be CREATE OR REPLACEd, and after that the function can be 
dropped. COR should be restricted in the same way as DROP is. I will 
check if this is still a problem with the latest patch.


Another problem is that you can ALTER FUNCTION  test() SET SCHEMA = 
something_else, and you can alter the functions search_path, which could 
be a problem for non-relocatable extensions. Even if the schema is 
changed, dropping extension / altering extension will work as expected. 
The function is just in different schema than the extension. But, both 
of these IMO fall in the category don't do that.


 - Anssi

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


Re: [HACKERS] REVIEW: Extensions support for pg_dump

2011-01-18 Thread Anssi Kääriäinen

On 01/17/2011 06:53 PM, Dimitri Fontaine wrote:


Usability review:

The patch implements a way to create extensions. While the patch is labeled
extensions support for pg_dump, it actually implements more. It implements a
new way to package and install extension, and changes contrib extensions to
use the new way.

Well, all there's in the patch is infrastructure to be able to issue
those single lines in your dump :)
Is this supposed to be used mainly by contrib and PGXN extensions? When 
I saw the documentation, I immediately thought that this is a nice way 
to package my application's stored procedures. If this is not one of the 
intended usages, it should be documented. I can see that this could be 
problematic when updating PostgreSQL and when recovering from backups.


When recovering from backup, you need to have the locally created 
extension available. But it might be that the extension files are lost 
when the system went down in flames. Now, the backup is unusable 
(right?) until extension files can be recovered from source control or 
where ever they might be stored. This is why I suggested having multiple 
locations for the extensions. It would be easy to backup locally created 
extensions if those were in a single directory. All in all, I have a 
nervous feeling that somebody someday will have an unusable dump because 
they used this feature, but do not have the extension files available...


Also, this part of documentation:

The goal of using extensions is so that applicationpg_dump/ knows
not to dump all the object definitions into your backups, but rather
issue a single xref linkend=SQL-CREATEEXTENSION command.

From that, it is not entirely clear to me why this is actually wanted 
in PostgreSQL. I suppose this will make dump/restore easier to use. But 
from that paragraph, I get the feeling the only advantage is that your 
dump will be smaller. And I still have a feeling this implements more. 
Not that it is a bad thing at all.

It used to work this way with \i, obviously.  Should the extension patch
care about that and how?  Do we want to RESET search_path or to manually
manage it like we do for the client_min_messages and log_min_messages?
It was unintuitive to me to have search_path changed by SQL command as a 
side effect. When using \i, not so much.

When trying to load extensions which contain identical signatured functions,
if the loading will succeed is dependent on the search path. If there is a
conflicting function in search path (first extension loaded to schema
public), then the second extension load will fail. But if the order is
reversed (first extension loaded to schema foo which is not in search path),
then the second extension can be loaded to the public schema.

Well I think that's an expected limitation here.  In the future we might
want to add suport for inter-extension dependencies and conflicts, but
we're certainly not there yet.
OK with this as is. It is just a bit surprising that you can create the 
extensions in one order but not in another.

There is no validation for the extension names in share/contrib/. It is
possible to have extensions control files named .control, .name.control,
name''.control etc. While it is stupid to name them so, it might be better
to give an explicit warning / error in these cases. It is of course possible
to use these extensions.

I don't have a strong opinion here, will wait for some votes.
Yeah, this is not a big problem in practice. Just don't name them like 
that. And if you do, you will find out soon enough that you made a 
mistake. By the way, in my comment above It is of course possible to 
use these extensions. - It is of course NOT possible 

I haven't had time to review the pg_dump part of the patch yet, will do that
next (tomorrow). I hope it is OK to post a partial review...

 From here, it's very good!  Will you continue from the git repository,
where the fixes are available, or do you want a v26 already?
It is easy for me to continue from the Git repo. I will next continue 
doing the pg_dump part of the review. I hope I have time to complete 
that today.


 - Anssi


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


Re: [HACKERS] REVIEW: Extensions support for pg_dump

2011-01-18 Thread Anssi Kääriäinen

On 01/18/2011 11:42 AM, Dimitri Fontaine wrote:

I've fixed the case by having the code remember the function's extension
if any, and restore it along with the other dependencies.
The only question here is should CREATE OR REPLACE be allowed. I just 
realized this could present a new problem. If I am not mistaken, when 
loading from dump, you suddenly get the extension's version back, not 
the one you defined in CREATE OR REPLACE. If this is the case, this 
should NOT be allowed. And by the same reasoning, ALTER FUNCTION 
[anything] should not be allowed either. Or at least then the 
function/(or any object for that matter) should be restored somehow from 
the backup, not from the extension files.


I still haven't had the time to start pg_dump reviewing, so I haven't 
verified if this is really a problem. But I suspect so...


 - Anssi

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


Re: [HACKERS] REVIEW: Extensions support for pg_dump

2011-01-18 Thread Anssi Kääriäinen

On 01/18/2011 12:11 PM, Anssi Kääriäinen wrote:

The only question here is should CREATE OR REPLACE be allowed. I just
realized this could present a new problem. If I am not mistaken, when
loading from dump, you suddenly get the extension's version back, not
the one you defined in CREATE OR REPLACE. If this is the case, this
should NOT be allowed. And by the same reasoning, ALTER FUNCTION
[anything] should not be allowed either. Or at least then the
function/(or any object for that matter) should be restored somehow from
the backup, not from the extension files.

I still haven't had the time to start pg_dump reviewing, so I haven't
verified if this is really a problem. But I suspect so...
Ok, verified at least for CREATE OR REPLACE, ALTER FUNCTION RENAME and 
ALTER FUNCTION SET search_path. You will get the extensions version back 
when restoring from plain sql dump, not the CORed function, rename is 
lost and same for search_path. I suspect this is a problem for any 
object type supported in extensions. But unfortunately I do not have 
time to verify that.


One more problem with pg_dump. If you have CREATE EXTENSION in you 
extensions .sql file, you will have problems restoring. I know it is 
stupid to do so, but still CREATE EXTENSION inside CREATE EXTENSION 
should be disallowed, as it is possible you find out too late that this 
is stupid thing to do. Also, the functions created in the recursive 
CREATE EXTENSION will be dumped, and the dependencies are not created 
correctly.


Unfortunately I have used up all the time I have for reviewing this 
patch. I can arrange some more time, maybe late this week, maybe a bit 
later. So, I do not have time to do the pg_dump part review in full 
detail right now. Still, I hope the work I have done is helpful.


Should I write up a post that contains all the current outstanding 
issues in one post, or is it enough to just link this thread in the 
CommitFest application?


 - Anssi

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


Re: [HACKERS] REVIEW: Extensions support for pg_dump

2011-01-18 Thread Anssi Kääriäinen

On 01/18/2011 01:03 PM, Dimitri Fontaine wrote:

I'd appreciate a list of yet-to-fix items.  What I have is the
search_path issue where CREATE EXTENSION foo; can leave it changed for
the current session, I intend to fix that later today.

Other than that, I have no further already agreed on code fix to make.
What's your list?
There is only documentation fixes, and I am not sure if even those are 
agreed to be necessary. It might be good if the documentation contained:


 - A warning that you need to have the files for your extensions 
readily available to be able to restore from a dump. This might be 
obvious, but better safe than sorry...
 - A warning that you will be restored to the extension's version if 
you ALTER or CREATE OR REPLACE a function.


From the current documentation, it is maybe too easy to miss these 
risks. I am seeing this from non-experienced user's angle, and thus see 
these as potential foot guns.


Other than that, I don't think there is anything more. I am a little 
nervous of restoring to extension's version of a function when the 
function has been CREATE OR REPLACEd, but that might be just me over 
thinking this. Also, from the previous posts, there is just the control 
file naming issue, and the issue of load order if two extensions contain 
similarly named and signatured functions. But these were agreed to be 
issues not needing any further work.


Now, I need help what to do next. Should I leave the status as Needs 
Review as the pg_dump part is almost completely non-reviewed? And then 
attach this thread as a comment? Or as a review?


 - Anssi

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


[HACKERS] REVIEW: Extensions support for pg_dump

2011-01-17 Thread Anssi Kääriäinen
I used the patch from CommitFest application and applied the following 
commit to fix a known issue:


http://git.postgresql.org/gitweb?p=postgresql-extension.git;a=commitdiff;h=d4991d35283ae0ceeb7f9e4203cf6a9dfb5d128d

Is the patch in context diff format?
Yes.

Does the patch apply cleanly?
No:
patching file src/include/commands/defrem.h
Hunk #2 succeeded at 107 with fuzz 2 (offset 27 lines).
Hunk #3 FAILED at 105.
1 out of 5 hunks FAILED -- saving rejects to file 
src/include/commands/defrem.h.rej


I have used the git head of

http://git.postgresql.org/gitweb?p=postgresql-extension.git branch extensions

to do the rest of reviewing. There is one compiler warning:

gcc -O2 -Wall -Wmissing-prototypes -Wpointer-arith

-Wdeclaration-after-statement -Wendif-labels -fno-strict-aliasing
-fwrapv -g -I../../../src/interfaces/libpq -I../../../src/include -D_GNU_
SOURCE   -c -o pg_dump.o pg_dump.c
pg_dump.c: In function ‘getTables’:
pg_dump.c:3748: warning: too many arguments for format


And, make check gives me the following errors:
test sanity_check ... FAILED
test rules... FAILED

Does it include reasonable tests, necessary doc patches, etc?

There is enough documentation, but I think the documentation needs some 
polishing. I am not a native English speaker, so it might be it is my 
English that is failing. The data is there, but the representation might 
be a little off.


I didn't spot any tests. This could be that I don't know what to look for...

Usability review:

The patch implements a way to create extensions. While the patch is 
labeled extensions support for pg_dump, it actually implements more. It 
implements a new way to package and install extension, and changes 
contrib extensions to use the new way.


I do think we want these features, and that we do not have those 
already. As far as I understand, there is nothing in the standard 
regarding this feature.


I wonder if the structure of having all the extensions in share/contrib/ 
is a good idea. It might be better if one could separate the contrib 
extensions in one place, and user created extensions in another place. 
This could make it easy to see what user created extensions is installed 
in (or installable to) the cluster. I think this might be helpful to 
DBAs upgrading PostgreSQL.


Overall, the system seems intuitive to use. It is relatively easy to 
create extension using this feature, and loading contrib extensions is 
really easy. I haven't tested how easy it is to create C-language 
extensions using this. If I am not mistaken it just adds the compile  
install the C-functions before installing the extension.


Feature test:

The packaging feature works as advertised, expect for the following bugs 
and inconsistencies.


When using the plain CREATE EXTENSION foobar command without schema 
qualifier, the extension is created in schema public (by name) without 
regard to the current search path. This is inconsistent with create 
table, and if the public schema is renamed, the command gives error:


ERROR: schema public does not exist

This makes the name public have a special meaning, and I suspect that 
is not wanted.


When issuing CREATE EXTENSION foo SCHEMA bar, and the extension foo is 
not relocatable and it's control file uses the SET search_path TO 
@extschema@, the search_path is set to bar for the session. That is, it 
is not changed to the original search_path after the command has completed.


When trying to load extensions which contain identical signatured 
functions, if the loading will succeed is dependent on the search path. 
If there is a conflicting function in search path (first extension 
loaded to schema public), then the second extension load will fail. But 
if the order is reversed (first extension loaded to schema foo which is 
not in search path), then the second extension can be loaded to the 
public schema.


While it is not possible to drop functions in extensions, it is possible 
to rename a function, and also to CREATE OR REPLACE a function in an 
extension. After renaming or CORing a function, it is possible to drop 
the function. I also wonder if alter function ... set parameter should 
be allowed?


There is no validation for the extension names in share/contrib/. It is 
possible to have extensions control files named .control, 
.name.control, name''.control etc. While it is stupid to name them 
so, it might be better to give an explicit warning / error in these 
cases. It is of course possible to use these extensions.


If there is no comment for a given extension in the .control file, then 
\dx will not show the extension installed even if it is installed.


I was able to make it crash:

postgres=# alter extension foo.bar set schema baz;
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.

Log contains this:
TRAP: 

Re: [HACKERS] SSI patch version 12

2011-01-16 Thread Anssi Kääriäinen
While I haven't tried this patch, I tried to break the version 11 of the 
patch (some of the work was against earlier versions). In total I have 
used a full work day just trying to break things, but haven't been able 
to find anything after version 8. I can verify that the partial index 
issue is fixed, and the count(*) performance is a lot better now.


One thing I have been thinking about is how does predicate locking 
indexes work when using functional indexes and functions marked as 
immutable but which really aren't. I don't know how predicate locking 
indexes works, so it might be that this is a non-issue. I haven't been 
able to break anything in this way, and even if I could, this is 
probably something that doesn't need anything else than a warning that 
if you label your index functions immutable when they aren't, 
serializable transactions might not work.


On 01/15/2011 01:54 AM, Kevin Grittner wrote:

The index types other than btree don't have fine-grained support,
which I don't think is a fatal defect, but it would be nice to
implement.  I may be able to get GiST working again this weekend in
addition to the documentation work.  The others might not get done
for 9.1 unless someone who knows their way around the guts of those
AMs can give us some advice soon
I wonder if there are people using GiST and GIN indexes and serializable 
transactions. When upgrading to 9.1 and if there is no backwards 
compatibility GUC this could be a problem... The amount of users in this 
category is probably very low anyways, so maybe this is not an issue 
worth worrying about.


 - Anssi

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


Re: [HACKERS] SSI patch version 8

2011-01-14 Thread Anssi Kääriäinen

On 01/14/2011 02:21 AM, Kevin Grittner wrote:

I hope you have no objection to having the code you wrote included
in the test suite which is part of the patch.  Well, if you do, I'll
pull it back out and invent something similar...  ;-)

No objection.

 - Anssi

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


Re: [HACKERS] SSI patch version 8

2011-01-11 Thread Anssi Kääriäinen

On 01/10/2011 06:03 PM, Kevin Grittner wrote:

Due to popular request (Hey, David's popular, right?), I'm posting a
patch for Serializable Snapshot Isolation (SSI), although I don't
yet have everything in it that I was planning on submitting before
the CF.  I will probably be submitting another version before the
deadline with the following, but there should be plenty here for
people to test and benchmark.  We're done with the major refactoring
needed to address concerns raised in earlier reviews, and I don't
expect the remaining work to destabilize what's there or to have a
significant impact on performance.

I think I found a problem. This is using SSI v8. The table definition:

create table test_t (id integer, val1 text, val2 integer);

create index test_idx on test_t(id) where val2 = 1;

The data:

insert into test_t (select generate_series(0, 1), 'a', 2);
insert into test_t (select generate_series(0, 10), 'a', 1);

The transactions:
T1:
hot2= begin transaction isolation level serializable;
BEGIN
hot2= select * from test_t where val2 = 1;
 id | val1 | val2
+--+--
  0 | a|1
  1 | a|1
  2 | a|1
  3 | a|1
  4 | a|1
  5 | a|1
  6 | a|1
  7 | a|1
  8 | a|1
  9 | a|1
 10 | a|1
(11 rows)

hot2= update test_t set val2 = 2 where val2 = 1 and id = 10;
UPDATE 1
-- The concurrent transaction:
T2:
hot2= begin transaction isolation level serializable;
BEGIN
hot2= select * from test_t where val2 = 1;
 id | val1 | val2
+--+--
  0 | a|1
  1 | a|1
  2 | a|1
  3 | a|1
  4 | a|1
  5 | a|1
  6 | a|1
  7 | a|1
  8 | a|1
  9 | a|1
 10 | a|1
(11 rows)

hot2= update test_t set val2 = 2 where val2 = 1 and id = 9;
UPDATE 1
hot2= commit;
COMMIT
-- Now, t1 can commit, too. Even though there is a serialization anomaly
T1:
hot2= commit;
COMMIT

If the test_idx is changed:
(outside any transaction...)
hot2= drop index test_idx;
DROP INDEX
hot2= create index test_idx on test_t(id, val2);
CREATE INDEX


T1:
hot2= begin transaction isolation level serializable;
BEGIN
hot2= select * from test_t where val2 = 1;
 id | val1 | val2
+--+--
  0 | a|1
  1 | a|1
  2 | a|1
  3 | a|1
  4 | a|1
  5 | a|1
  6 | a|1
  7 | a|1
  8 | a|1
(9 rows)

hot2= update test_t set val2 = 2 where val2 = 1 and id = 8;
UPDATE 1

T2:
hot2= select * from test_t where val2 = 1;
 id | val1 | val2
+--+--
  0 | a|1
  1 | a|1
  2 | a|1
  3 | a|1
  4 | a|1
  5 | a|1
  6 | a|1
  7 | a|1
  8 | a|1
(9 rows)

hot2= update test_t set val2 = 2 where val2 = 1 and id = 7;
UPDATE 1
hot2= commit;
ERROR:  could not serialize access due to read/write dependencies among 
transactions

HINT:  The transaction might succeed if retried.
T1:
hot2= commit;
COMMIT

So, something seems to be broken when using partial indexes.

 - Anssi


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


Re: [HACKERS] SSI patch version 8

2011-01-11 Thread Anssi Kääriäinen

On 01/10/2011 06:03 PM, Kevin Grittner wrote:

Due to popular request (Hey, David's popular, right?), I'm posting a
patch for Serializable Snapshot Isolation (SSI), although I don't
yet have everything in it that I was planning on submitting before
the CF.  I will probably be submitting another version before the
deadline with the following, but there should be plenty here for
people to test and benchmark.  We're done with the major refactoring
needed to address concerns raised in earlier reviews, and I don't
expect the remaining work to destabilize what's there or to have a
significant impact on performance.

A speed test showing a significant drop in performance when using SSI:

hot2= create table test_t2 as (select generate_series(0, 100));
hot2= \timing
begin transaction isolation level repeatable read;
Time: 0.185 ms
hot2= select count(*) from test_t2;
Time: 134.521 ms
hot2= select count(*) from test_t2;
Time: 142.132 ms
hot2= select count(*) from test_t2;
Time: 147.469 ms
hot2= commit;
hot2= begin transaction isolation level serializable;
Time: 0.165 ms
hot2= select count(*) from test_t2;
Time: 349.219 ms
hot2= select count(*) from test_t2;
Time: 354.010 ms
hot2= select count(*) from test_t2;
Time: 355.960 ms
hot2= commit;

So, count(*) queries are more than twice as slow compared to the old 
serializable transaction isolation level. The relative speed difference 
stays the same if testing with more rows. Also, the same speed 
difference is there if testing with more columns:


create table test_t4 as (select g g1, g g2, g g3, g g4, g g5, g g6 from 
(select generate_series as g from generate_series(0, 100)) as t1);


repeatable read times:
140.113 ms 134.628 ms 140.113 ms 156.166 ms

serializable:
353.257 ms 366.558 ms 373.914 ms 359.682 ms

 - Anssi

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


Re: [HACKERS] SSI patch version 8

2011-01-11 Thread Anssi Kääriäinen

On 01/11/2011 04:53 PM, Kevin Grittner wrote:

Thanks much for testing.  You're managing to exercise some code
paths I didn't think to test, which is great!  I guess this is the
up side of having posted yesterday.  :-)


Glad that I can help. This feature is something that is very important
to our usage of PostgreSQL.

Just to add some positive feedback: I tried this patch with our in-house
in development EAV database. Everything was working perfectly, and time
to import current and history data for 7000 employees (total of 95000
attributes) to the DB using stored procedures went from 4 minutes 20
seconds to 4 minutes 30 seconds (the procedures are doing a lot of
validation...). So in this case the real-world performance difference
was barely noticeable. SSI was also able to block serialization
anomalies and I did not have any false rollbacks in my (very limited)
testing. So, thank you for the work on this feature. And, of course, I
will try to break it some more.:-)

 - Anssi



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


Re: [HACKERS] Row-level Locks SERIALIZABLE transactions, postgres vs. Oracle

2010-05-13 Thread Anssi Kääriäinen

On 05/14/2010 03:37 AM, Greg Stark wrote:
 On Thu, May 13, 2010 at 10:25 PM, Florian Pflugf...@phlo.org  wrote:
 C1: BEGIN
 C1: SELECT * FROM t WHERE id = 1 FOR UPDATE
 C2: BEGIN
 C2: SET TRANSACTION ISOLATION LEVEL SERIALIZABLE
 C2: SELECT * FROM t -- Take snapshot before C1 commits
 C1: COMMIT
 C2: DELETE FROM t WHERE id = 1
 C2: COMMIT


 Can you give an actual realistic example -- ie, not doing a select for
 update and then never updating the row or with an explanation of what
 the programmer is attempting to accomplish with such an unusual
 sequence? The rest of the post talks about FKs but I don't see any
 here...


Doing a select for update and then never updating the row is a realistic 
example.


I am currently designing a database where this is an issue. The 
simplified schema to illustrate the problem:


create table object (
   id integer primary key
);

insert into object values(1);

create table attribute (
   object_id integer not null references object,
   attr_type integer not null, -- references attr_types
   value text not null,
   valid_from timestamp not null,
   valid_until timestamp
);

Now, I want to make sure there are no pairs of (object_id, attr_type) 
where the valid_from, valid_until times overlap.


A problematic sequence for this schema, both transactions in isolation 
level serializable:



C1: begin;
C1: select * from object where id = 1 for update;
-- check for conflicting attr_type, realistically where condition should 
have overlapping check, but left out for simplicity...

C1: select * from attribute where object_id = 1 and attr_type = 1;
-- Ok, nothing overlapping, I am able to insert.
C1: insert into attribute values (1, 1, 'Anssi', now(), null);
C2: begin;
-- This blocks.
C2: select * from object where id = 1 for update;
C1: commit;
-- Check for conflicts. This select won't see the insert C1 did.
C2: select * from attribute where object_id = 1 and attr_type = 1;
-- C2 doesn't see anything conflicting
C2: insert into attribute values (1, 1, 'Matti', now(), null);
C2: commit;
-- Inconsistency.

Now, that same sequence does work for read committed isolation level (C2 
sees the insert of C1), and that is my solution for now: require 
applications to use read committed isolation level. This could also be 
solved by issuing update object set id = id where id = 1 instead of 
using select for update. This would result in serialization error.


I know that for this particular example the upcoming exclusion 
constraints would solve the problem. But if I would want to ensure that 
if attr_value for attr_type 1 is 'Anssi' then attr_value for attr_type 2 
is 'Kääriäinen', then exclusion constraints could not be used.


--
Anssi Kääriäinen

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