Re: [HACKERS] [ODBC] getting rid of SnapshotNow

2013-07-19 Thread Hiroshi Inoue

(2013/07/19 22:03), Andres Freund wrote:

On 2013-07-19 08:57:01 +0900, Inoue, Hiroshi wrote:

I had the idea they were used for a client-side implementation of WHERE
CURRENT OF.  Perhaps that's dead code and could be removed entirely?


It's been reported that ODBC still uses them.


Though PostgreSQL's TID is similar to Orale's ROWID, it is transient
and changed after update operations unfortunately. I implemented
the currtid_xx functions to supplement the difference. For example

currtid(relname, original tid)

(hopefully) returns the current tid of the original row when it is
updated.


That is only guaranteed to work though when you're in a transaction old
enough to prevent removal of the old or intermediate row versions. E.g.


Yes it's what I meant by (hopefully).
At the time when I implemented currtid(), I was able to use TIDs in
combination with OIDs.

regards,
Hiroshi Inoue


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


Re: [HACKERS] [ODBC] getting rid of SnapshotNow

2013-07-19 Thread Andres Freund
On 2013-07-19 08:57:01 +0900, Inoue, Hiroshi wrote:
> >>I had the idea they were used for a client-side implementation of WHERE
> >>CURRENT OF.  Perhaps that's dead code and could be removed entirely?
> >
> >It's been reported that ODBC still uses them.
> 
> Though PostgreSQL's TID is similar to Orale's ROWID, it is transient
> and changed after update operations unfortunately. I implemented
> the currtid_xx functions to supplement the difference. For example
> 
>   currtid(relname, original tid)
> 
> (hopefully) returns the current tid of the original row when it is
> updated.

That is only guaranteed to work though when you're in a transaction old
enough to prevent removal of the old or intermediate row versions. E.g.
BEGIN;
INSERT INTO foo...; -- last tid (0, 1)
COMMIT;
BEGIN;
SELECT currtid(foo, '(0, 1'));
COMMIT;

can basically return no or even an arbitrarily different row. Same with
an update...

Greetings,

Andres Freund

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


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


Re: [HACKERS] [ODBC] getting rid of SnapshotNow

2013-07-18 Thread Inoue, Hiroshi

(2013/07/18 23:54), Robert Haas wrote:

On Thu, Jul 18, 2013 at 10:34 AM, Tom Lane  wrote:

Robert Haas  writes:

1. snapshot-error-v1.patch introduces a new special snapshot, called
SnapshotError.  In the cases where we set SnapshotNow as a sort of
default snapshot, this patch changes the code to use SnapshotError
instead.  This affects scan->xs_snapshot in genam.c and
estate->es_snapshot in execUtils.c.  This passes make check-world, so
apparently there is no code in the core distribution that does this.
However, this is safer for third-party code, which will ERROR instead
of seg faulting.  The alternative approach would be to use
InvalidSnapshot, which I think would be OK too if people dislike this
approach.


FWIW, I think using InvalidSnapshot would be preferable to introducing
a new concept for what's pretty much the same thing.


Andres voted the other way on the previous thread.  I'll wait and see
if there are any other opinions.  The SnapshotError concept seemed
attractive to me initially, but I'm not as excited about it after
seeing how it turned out, so maybe it's best to do it as you suggest.


With that done, the only remaining uses of SnapshotNow in our code
base will be in currtid_byreloid() and currtid_byrelname().  So far no
one on this list has been able to understand clearly what the purpose
of those functions is, so I'm copying this email to pgsql-odbc in case
someone there can provide more insight.


I had the idea they were used for a client-side implementation of WHERE
CURRENT OF.  Perhaps that's dead code and could be removed entirely?


It's been reported that ODBC still uses them.


Though PostgreSQL's TID is similar to Orale's ROWID, it is transient
and changed after update operations unfortunately. I implemented
the currtid_xx functions to supplement the difference. For example

currtid(relname, original tid)

(hopefully) returns the current tid of the original row when it is
updated.

regards,
Hiroshi Inoue



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


Re: [HACKERS] [ODBC] getting rid of SnapshotNow

2013-07-18 Thread Robert Haas
On Thu, Jul 18, 2013 at 12:25 PM, Alvaro Herrera
 wrote:
> Ah, yeah, that does show up.  I had grepped for 'currtid_'.  Sorry.
> They're all in positioned_load() in results.c.

Well, in that case, we'll have to keep it around.  I still wish we
could get a clear answer to the question of how it's being used.

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


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


Re: [HACKERS] [ODBC] getting rid of SnapshotNow

2013-07-18 Thread Alvaro Herrera
Andres Freund escribió:
> On 2013-07-18 12:01:39 -0400, Robert Haas wrote:
> > On Thu, Jul 18, 2013 at 11:54 AM, Alvaro Herrera
> >  wrote:
> > > They don't show up in a quick grep of psqlodbc's source code, FWIW.
> > 
> > Hmm.  Maybe we should just remove them and see if anyone complains.
> > We could always put them back (or make them available via contrib) if
> > it's functionality someone actually needs.  The last discussion of
> > those functions was in 2007 and nobody seemed too sure back then
> > either, so maybe the rumor that anyone is actually using this is no
> > more than rumor.
> 
> I am pretty sure they are still used. A quick grep on a not too old
> checkout prooves that... Note that the sql accessible functions are
> named currtid and currtid2 (yes, really)...

Ah, yeah, that does show up.  I had grepped for 'currtid_'.  Sorry.
They're all in positioned_load() in results.c.

-- 
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services


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


Re: [HACKERS] [ODBC] getting rid of SnapshotNow

2013-07-18 Thread Andres Freund
On 2013-07-18 12:01:39 -0400, Robert Haas wrote:
> On Thu, Jul 18, 2013 at 11:54 AM, Alvaro Herrera
>  wrote:
> > They don't show up in a quick grep of psqlodbc's source code, FWIW.
> 
> Hmm.  Maybe we should just remove them and see if anyone complains.
> We could always put them back (or make them available via contrib) if
> it's functionality someone actually needs.  The last discussion of
> those functions was in 2007 and nobody seemed too sure back then
> either, so maybe the rumor that anyone is actually using this is no
> more than rumor.

I am pretty sure they are still used. A quick grep on a not too old
checkout prooves that... Note that the sql accessible functions are
named currtid and currtid2 (yes, really)...

Greetings,

Andres Freund

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


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


Re: [HACKERS] [ODBC] getting rid of SnapshotNow

2013-07-18 Thread Robert Haas
On Thu, Jul 18, 2013 at 11:54 AM, Alvaro Herrera
 wrote:
> They don't show up in a quick grep of psqlodbc's source code, FWIW.

Hmm.  Maybe we should just remove them and see if anyone complains.
We could always put them back (or make them available via contrib) if
it's functionality someone actually needs.  The last discussion of
those functions was in 2007 and nobody seemed too sure back then
either, so maybe the rumor that anyone is actually using this is no
more than rumor.

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


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


Re: [HACKERS] [ODBC] getting rid of SnapshotNow

2013-07-18 Thread Alvaro Herrera
Robert Haas escribió:
> On Thu, Jul 18, 2013 at 10:34 AM, Tom Lane  wrote:
> > Robert Haas  writes:
> >> 1. snapshot-error-v1.patch introduces a new special snapshot, called
> >> SnapshotError.  In the cases where we set SnapshotNow as a sort of
> >> default snapshot, this patch changes the code to use SnapshotError
> >> instead.  This affects scan->xs_snapshot in genam.c and
> >> estate->es_snapshot in execUtils.c.  This passes make check-world, so
> >> apparently there is no code in the core distribution that does this.
> >> However, this is safer for third-party code, which will ERROR instead
> >> of seg faulting.  The alternative approach would be to use
> >> InvalidSnapshot, which I think would be OK too if people dislike this
> >> approach.
> >
> > FWIW, I think using InvalidSnapshot would be preferable to introducing
> > a new concept for what's pretty much the same thing.
> 
> Andres voted the other way on the previous thread.  I'll wait and see
> if there are any other opinions.  The SnapshotError concept seemed
> attractive to me initially, but I'm not as excited about it after
> seeing how it turned out, so maybe it's best to do it as you suggest.

Yeah ... SnapshotError is a way to ensure the server doesn't crash if an
extension hasn't been fixed in order not to cause a crash if it doesn't
use the APIs correctly.  However, there's many other ways for a
C-language extension to cause crashes, so I don't think this is buying
us much.

> >> With that done, the only remaining uses of SnapshotNow in our code
> >> base will be in currtid_byreloid() and currtid_byrelname().  So far no
> >> one on this list has been able to understand clearly what the purpose
> >> of those functions is, so I'm copying this email to pgsql-odbc in case
> >> someone there can provide more insight.
> >
> > I had the idea they were used for a client-side implementation of WHERE
> > CURRENT OF.  Perhaps that's dead code and could be removed entirely?
> 
> It's been reported that ODBC still uses them.

They don't show up in a quick grep of psqlodbc's source code, FWIW.

-- 
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services


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


Re: [HACKERS] [ODBC] getting rid of SnapshotNow

2013-07-18 Thread Robert Haas
On Thu, Jul 18, 2013 at 10:34 AM, Tom Lane  wrote:
> Robert Haas  writes:
>> 1. snapshot-error-v1.patch introduces a new special snapshot, called
>> SnapshotError.  In the cases where we set SnapshotNow as a sort of
>> default snapshot, this patch changes the code to use SnapshotError
>> instead.  This affects scan->xs_snapshot in genam.c and
>> estate->es_snapshot in execUtils.c.  This passes make check-world, so
>> apparently there is no code in the core distribution that does this.
>> However, this is safer for third-party code, which will ERROR instead
>> of seg faulting.  The alternative approach would be to use
>> InvalidSnapshot, which I think would be OK too if people dislike this
>> approach.
>
> FWIW, I think using InvalidSnapshot would be preferable to introducing
> a new concept for what's pretty much the same thing.

Andres voted the other way on the previous thread.  I'll wait and see
if there are any other opinions.  The SnapshotError concept seemed
attractive to me initially, but I'm not as excited about it after
seeing how it turned out, so maybe it's best to do it as you suggest.

>> With that done, the only remaining uses of SnapshotNow in our code
>> base will be in currtid_byreloid() and currtid_byrelname().  So far no
>> one on this list has been able to understand clearly what the purpose
>> of those functions is, so I'm copying this email to pgsql-odbc in case
>> someone there can provide more insight.
>
> I had the idea they were used for a client-side implementation of WHERE
> CURRENT OF.  Perhaps that's dead code and could be removed entirely?

It's been reported that ODBC still uses them.

>> If we don't want to risk any change to the semantics, we can (1) grit
>> our teeth and keep SnapshotNow around or (2) use an instantaneous MVCC
>> snapshot there, and accept that people who have very large connection
>> counts and extremely heavy use of those functions may see a
>> performance regression.
>
> Of those I'd go for (2); it's unlikely that an extra snapshot would be
> visible compared to the query roundtrip overhead.  But another attractive
> possibility is to make these functions use GetActiveSnapshot(), which
> would avoid taking any new snapshot.

You could probably construct a case where the overhead is visible, if
you ran the functions many times in a single query, but arguably no
one does that.  Also, Andres's test case that involves running BEGIN;
SELECT txid_current(); very short sleep; COMMIT; in several hundred
sessions at once is pretty brutal on PGXACT and makes the overhead of
taking extra snapshots a lot more visible.

I'm not too familiar with GetActiveSnapshot(), but wouldn't that
change the user-visible semantics?  If, for example, someone's using
that function to test whether the row has been updated since their
snapshot was taken, that use case would get broken.  SnapshotSelf
would be change from the current behavior in many fewer cases than
using an older snapshot.

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


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


Re: [HACKERS] [ODBC] getting rid of SnapshotNow

2013-07-18 Thread Tom Lane
Robert Haas  writes:
> 1. snapshot-error-v1.patch introduces a new special snapshot, called
> SnapshotError.  In the cases where we set SnapshotNow as a sort of
> default snapshot, this patch changes the code to use SnapshotError
> instead.  This affects scan->xs_snapshot in genam.c and
> estate->es_snapshot in execUtils.c.  This passes make check-world, so
> apparently there is no code in the core distribution that does this.
> However, this is safer for third-party code, which will ERROR instead
> of seg faulting.  The alternative approach would be to use
> InvalidSnapshot, which I think would be OK too if people dislike this
> approach.

FWIW, I think using InvalidSnapshot would be preferable to introducing
a new concept for what's pretty much the same thing.

> With that done, the only remaining uses of SnapshotNow in our code
> base will be in currtid_byreloid() and currtid_byrelname().  So far no
> one on this list has been able to understand clearly what the purpose
> of those functions is, so I'm copying this email to pgsql-odbc in case
> someone there can provide more insight.

I had the idea they were used for a client-side implementation of WHERE
CURRENT OF.  Perhaps that's dead code and could be removed entirely?

> If we don't want to risk any change to the semantics, we can (1) grit
> our teeth and keep SnapshotNow around or (2) use an instantaneous MVCC
> snapshot there, and accept that people who have very large connection
> counts and extremely heavy use of those functions may see a
> performance regression.

Of those I'd go for (2); it's unlikely that an extra snapshot would be
visible compared to the query roundtrip overhead.  But another attractive
possibility is to make these functions use GetActiveSnapshot(), which
would avoid taking any new snapshot.

regards, tom lane


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