[HACKERS] Consolidate 'unique array values' logic into a reusable function?

[Thomas Munro]

Hi,

Looking at commits f10eab73d and c50d192c, I wondered why we don't
have a reusable in-place unique function.  It may be trivial, but we
seem to have a lot of copies and variations in the tree.

Here's a sketch patch that creates a function array_unique which takes
the same arguments as qsort or qsort_arg and returns the new length.
The patch replaces all the specialised unique functions and open coded
versions that I could find with simple greps, but there are probably
more.

My compiler seems to inline the comparator function and memcpy well,
so I can't measure any speed difference between array_unique(array,
size, sizeof(int), compare_int) and a hand-crafted loop using == for
comparison and = for assignment, for a billion items.

If no one objects I'll post a version of this to a commitfest, along
with some other trivial code duplication refactoring work I posted a
while back that consolidates popcount and ffs/fls implementations.  I
don't like code duplication :-)

-- 
Thomas Munro
http://www.enterprisedb.com


array-unique.patch
Description: Binary data

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

[HACKERS] Draft release notes for next week's back-branch releases

[Tom Lane]

I've put up draft notes for 9.5.4 at

https://git.postgresql.org/gitweb/?p=postgresql.git;a=commitdiff;h=3676631c687009c2fadcb35e7d312e9eb8a98182

The website should have them after guaibasaurus's next run, due a couple
hours from now.

As usual, the older branches will have subsets of these notes (but there
is one item about pg_ctl that applies only to 9.1).  Please review in the
next 24 hours.

regards, tom lane


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

Re: [HACKERS] Surprising behaviour of \set AUTOCOMMIT ON

[Matt Kelly]

Its worth noting that the JDBC's behavior when you switch back to
autocommit is to immediately commit the open transaction.

Personally, I think committing immediately or erroring are unsurprising
behaviors.  The current behavior is surprising and obviously wrong.
Rolling back without an error would be very surprising (no other database
API I know of does that) and would take forever to debug issues around the
behavior.  And committing after the next statement is definitely the most
surprising behavior suggested.

IMHO, I think committing immediately and erroring are both valid.  I think
I'd prefer the error in principle, and per the law of bad code I'm sure,
although no one has ever intended to use this behavior, there is probably
some code out there that is relying on this behavior for "correctness".  I
think a hard failure and making the dev add an explicit commit is least
likely to cause people serious issues.  As for the other options, consider
me opposed.

- Matt K.

Re: [HACKERS] Oddity with NOT IN

[Corey Huinker]

On Sat, Aug 6, 2016 at 2:13 PM, Jim Nasby  wrote:

> On 8/6/16 12:57 PM, Andrew Gierth wrote:
>
>> The easy to catch case, I think, is when the targetlist of the IN or NOT
>> IN subquery contains vars of the outer query level but no vars of the
>> inner one and no volatile functions. This can be checked for with a
>> handful of lines in the parser or a couple of dozen lines in a plugin
>> module (though one would have to invent an error code, none of the
>> existing WARNING sqlstates would do).
>>
>
> I would still like to warn on any outer vars show up in the target list
> (other than as function params), because it's still very likely to be a
> bug. But I agree that what you describe is even more certain to be one.
>
> Maybe David Fetter's suggested module for catching missing WHERE clauses
>> could be expanded into a more general SQL-'Lint' module?
>>
>
> Possibly, though I hadn't really considered treating this condition as an
> error.
>
> Also, there's some other common gotchas that we could better warn users
> about, some of which involve DDL. One example is accidentally defining
> duplicate indexes.
> --
> Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX
> Experts in Analytics, Data Architecture and PostgreSQL
> Data in Trouble? Get it in Treble! http://BlueTreble.com
> 855-TREBLE2 (855-873-2532)   mobile: 512-569-9461
>
>
> --
> Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers
>

If we are contemplating a setting wherein we issue debug/notice/warning
messages for potentially erroneous SQL, I would suggest a simple test would
be any reference to a column without the a corresponding table/alias.

This is fine:
SELECT a.x, b.y FROM table_that_has_x a JOIN table_that_has_y b ON a.id
= b.foreign_id
This gives the notice/warning:
SELECT x, b.y FROM table_that_has_x a JOIN table_that_has_y b ON a.id =
b.foreign_id

We'd have to suppress the warning in cases where no tables are mentioned
(no table to alias, i.e. "SELECT 'a_constant' as config_var"), and I could
see a reason for suppressing it where only one table is mentioned, though I
often urge table aliasing and full references because it makes it easier
when you modify the query to add another table.

Some setting name suggestions:

notify_vague_column_reference = (on,off)
pedantic_column_identifiers = (off,debug,notice,warn,error)

Re: [HACKERS] Parallel tuplesort (for parallel B-Tree index creation)

[Peter Geoghegan]

On Sat, Aug 6, 2016 at 6:46 AM, Amit Kapila  wrote:
> I think here some of the factors like how many workers will be used
> for merge phase might impact the performance.   Having too many
> workers can lead to more communication cost and having too few workers
> might not yield best results for merge.  One thing, I have noticed
> that in general for sorting, some of the other databases uses range
> partitioning [1], now that might not be what is good for us.

I don't disagree with anything you say here. I acknowledged that
partitioning will probably be important for sorting in my introductory
e-mail, after all.

> I see
> you mentioned above that why it is not good [2], but I don't
> understand why you think it is a risky assumption to assume good
> partition boundaries for parallelizing sort.

Well, apparently there are numerous problems with partitioning in
systems like SQL Server and Oracle in the worst case. For one thing,
in the event of a misestimation (or failure of the dynamic sampling
that I presume can sometimes be used), workers can be completely
starved of work for the entire duration of the sort. And for CREATE
INDEX to get much of any benefit, all workers must write their part of
the index independently, too. This can affect the physical structure
of the final index. SQL Server also has a caveat in its documentation
about this resulting in an unbalanced final index, which I imagine
could be quite bad in the worst case.

I believe that it's going to be hard to get any version of this that
writes the index simultaneously in each worker accepted for these
reasons. This patch I came up with isn't very different from the
serial case at all. Any index built in parallel by the patch ought to
have relfilenode files on the filesystem that are 100% identical to
those produced by the serial case, in fact (since CREATE INDEX does
not set LSNs in the new index pages). I've actually developed a simple
way of "fingerprinting" indexes during testing of this patch, knowing
that hashing the files on disk ought to produce a perfect match
compared to a master branch serial sort case.

At the same time, any information that I've seen about how much
parallel CREATE INDEX speeds things up in these other systems
indicates that the benefits are very similar. It tends to be in the 2x
- 3x range, with the same reduction in throughput seen at about 16
workers, after we peak at about 8 workers. So, I think that the
benefits of partitioning are not really seen with CREATE INDEX (I
think of partitioning as more of a parallel query thing). Obviously,
any benefit that might still exist for CREATE INDEX in particular,
when weighed against the costs, makes partitioning look pretty
unattractive as a next step.

I think that during the merge phase of parallel CREATE INDEX as
implemented, the system generally still isn't that far from being I/O
bound. Whereas, with parallel query, partitioning makes each worker
able to return one tuple from its own separated range very quickly,
not just one worker (presumably, each worker merges non-overlapping
"ranges" from runs initially sorted in each worker. Each worker
subsequently merges after a partition-wise redistribution of the
initial fully sorted runs, allowing for dynamic sampling to optimize
the actual range used for load balancing.). The workers can then do
more CPU-bound processing in whatever node is fed by each worker's
ranged merge; everything is kept busy. That's the approach that I
personally had in mind for partitioning, at least. It's really nice
for parallel query to be able to totally separate workers after the
point of redistribution. CREATE INDEX is not far from being I/O bound
anyway, though, so it benefits far less. (Consider how fast the merge
phase still is at writing out the index in *absolute* terms.)

Look at figure 9 in this paper: http://www.vldb.org/pvldb/vol7/p85-balkesen.pdf

Even in good cases for "independent sorting", there is only a benefit
seen at 8 cores. At the same time, I can only get about 6x scaling
with 8 workers, just for the initial generation of runs.

All of these factors are why I believe I'm able to compete well with
other systems with this relatively straightforward, evolutionary
approach. I have a completely open mind about partitioning, but my
approach makes sense in this context.

-- 
Peter Geoghegan


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

Re: [HACKERS] Oddity with NOT IN

[Jim Nasby]

On 8/6/16 12:57 PM, Andrew Gierth wrote:

The easy to catch case, I think, is when the targetlist of the IN or NOT
IN subquery contains vars of the outer query level but no vars of the
inner one and no volatile functions. This can be checked for with a
handful of lines in the parser or a couple of dozen lines in a plugin
module (though one would have to invent an error code, none of the
existing WARNING sqlstates would do).


I would still like to warn on any outer vars show up in the target list 
(other than as function params), because it's still very likely to be a 
bug. But I agree that what you describe is even more certain to be one.



Maybe David Fetter's suggested module for catching missing WHERE clauses
could be expanded into a more general SQL-'Lint' module?


Possibly, though I hadn't really considered treating this condition as 
an error.


Also, there's some other common gotchas that we could better warn users 
about, some of which involve DDL. One example is accidentally defining 
duplicate indexes.

--
Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX
Experts in Analytics, Data Architecture and PostgreSQL
Data in Trouble? Get it in Treble! http://BlueTreble.com
855-TREBLE2 (855-873-2532)   mobile: 512-569-9461


--
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] Oddity with NOT IN

[Andrew Gierth]

> "Andrew" == Andrew Gierth  writes:

 Andrew> The easy to catch case, I think, is when the targetlist of the
 Andrew> IN or NOT IN subquery contains vars of the outer query level
 Andrew> but no vars of the inner one and no volatile functions. This
 Andrew> can be checked for with a handful of lines in the parser or a
 Andrew> couple of dozen lines in a plugin module (though one would have
 Andrew> to invent an error code, none of the existing WARNING sqlstates
 Andrew> would do).

Actually thinking about this, if you did it in a module, you'd probably
want to make it an ERROR not a WARNING, because you'd want to actually
stop queries like

delete from t1 where x in (select x from table_with_no_column_x);

rather than let them run.

-- 
Andrew (irc:RhodiumToad)


-- 
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] Oddity with NOT IN

[Pavel Stehule]

2016-08-06 20:01 GMT+02:00 Jim Nasby :

> On 8/6/16 12:03 PM, Pavel Stehule wrote:
>
>> It would be very useful if we had some way to warn users about stuff
>> like this. Emitting a NOTICE comes to mind.
>>
>>
>> This can be valid query
>>
>
> Right, but in my experience it's an extremely uncommon pattern and much
> more likely to be a mistake (that ends up being very time consuming to
> debug). That's why I think something like a NOTICE or even a WARNING would
> be useful. The only thing I don't like about that idea is if you ever did
> actually want this behavior you'd have to do something to squash the
> ereport. Though, that's a problem we already have in some places, so
> perhaps not worth worrying about.


I worked for company where they generated sets of SQL queries as result of
transformation from multidimensional query language. Some similar queries
are possible there.

I don't thing so using NOTICE or WARNING for any valid query is good idea.

I like the idea of some special extension than can block or raises warning
for some strange plans like this or with Cartesian product ...

Regards

Pavel


>
> --
> Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX
> Experts in Analytics, Data Architecture and PostgreSQL
> Data in Trouble? Get it in Treble! http://BlueTreble.com
> 855-TREBLE2 (855-873-2532)   mobile: 512-569-9461
>

Re: [HACKERS] Oddity with NOT IN

[Jim Nasby]

On 8/6/16 12:03 PM, Pavel Stehule wrote:

It would be very useful if we had some way to warn users about stuff
like this. Emitting a NOTICE comes to mind.


This can be valid query


Right, but in my experience it's an extremely uncommon pattern and much 
more likely to be a mistake (that ends up being very time consuming to 
debug). That's why I think something like a NOTICE or even a WARNING 
would be useful. The only thing I don't like about that idea is if you 
ever did actually want this behavior you'd have to do something to 
squash the ereport. Though, that's a problem we already have in some 
places, so perhaps not worth worrying about.

--
Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX
Experts in Analytics, Data Architecture and PostgreSQL
Data in Trouble? Get it in Treble! http://BlueTreble.com
855-TREBLE2 (855-873-2532)   mobile: 512-569-9461


--
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] Oddity with NOT IN

[Andrew Gierth]

> "Pavel" == Pavel Stehule  writes:

 >> Well now I feel dumb...
 >> 
 >> It would be very useful if we had some way to warn users about stuff
 >> like this. Emitting a NOTICE comes to mind.

 Pavel> This can be valid query

It can be, but it essentially never is. The cases where you genuinely
want a correlated IN query are rare, but even then there would be
something in the targetlist that referenced the inner query.

The easy to catch case, I think, is when the targetlist of the IN or NOT
IN subquery contains vars of the outer query level but no vars of the
inner one and no volatile functions. This can be checked for with a
handful of lines in the parser or a couple of dozen lines in a plugin
module (though one would have to invent an error code, none of the
existing WARNING sqlstates would do).

Maybe David Fetter's suggested module for catching missing WHERE clauses
could be expanded into a more general SQL-'Lint' module?

-- 
Andrew (irc:RhodiumToad)


-- 
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] No longer possible to query catalogs for index capabilities?

[Jim Nasby]

On 8/6/16 10:13 AM, Bruce Momjian wrote:

On Sat, Aug  6, 2016 at 04:04:41PM +0100, Andrew Gierth wrote:

"Bruce" == Bruce Momjian  writes:


 Bruce> Would it be helpful to output an array of strings representing
 Bruce> the index definition?

 >> Why would that help, if the point is to enable programmatic access
 >> to information?

 Bruce> I was thinking an array of strings would avoid problems in
 Bruce> having to re-scan the output for tokens.

OK, but that still leaves the issue of how to interpret each string,
yes?


Yes, you still have to parse it, just not scan/tokenize it.


That's an improvement. For some scenarios maybe it's enough. But I also 
don't see what's wrong with having the ability to probe for specific 
capabilities. I've needed to do this for unit tests, and for some items 
it's a real bear.


Trigger definitions are an example. I've done this in the past in unit 
tests to ensure that a trigger was defined in a particular way. Some 
data is available directly in the catalog, but getting at other info 
would have required hard-coding knowledge of specific bit patterns into 
the query. Instead of doing that I elected to parse 
pg_get_triggerdef[1], but that's hardly satisfying either.


1: https://github.com/decibel/cat_tools/blob/master/sql/cat_tools.sql#L339
--
Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX
Experts in Analytics, Data Architecture and PostgreSQL
Data in Trouble? Get it in Treble! http://BlueTreble.com
855-TREBLE2 (855-873-2532)   mobile: 512-569-9461


--
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] Oddity with NOT IN

[Pavel Stehule]

2016-08-06 18:53 GMT+02:00 Jim Nasby :

> On 8/4/16 4:53 PM, Marko Tiikkaja wrote:
>
>> On 2016-08-04 11:23 PM, Jim Nasby wrote:
>>
>>> I've got a customer that discovered something odd...
>>>
>>> SELECT f1 FROM v1 WHERE f2 not in (SELECT bad FROM v2 WHERE f3 = 1);
>>>
>>> does not error, even though bad doesn't exist, but
>>>
>>
>> I'm guessing there's a v1.bad?
>>
>> This is a common mistake, and also why I recommend always table
>> qualifying column references when there's more than one table in scope.
>>
>
> Well now I feel dumb...
>
> It would be very useful if we had some way to warn users about stuff like
> this. Emitting a NOTICE comes to mind.
>

This can be valid query

Regards

Pavel


> --
> Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX
> Experts in Analytics, Data Architecture and PostgreSQL
> Data in Trouble? Get it in Treble! http://BlueTreble.com
> 855-TREBLE2 (855-873-2532)   mobile: 512-569-9461
>
>
> --
> 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] Oddity with NOT IN

[Jim Nasby]

On 8/4/16 4:53 PM, Marko Tiikkaja wrote:

On 2016-08-04 11:23 PM, Jim Nasby wrote:

I've got a customer that discovered something odd...

SELECT f1 FROM v1 WHERE f2 not in (SELECT bad FROM v2 WHERE f3 = 1);

does not error, even though bad doesn't exist, but


I'm guessing there's a v1.bad?

This is a common mistake, and also why I recommend always table
qualifying column references when there's more than one table in scope.


Well now I feel dumb...

It would be very useful if we had some way to warn users about stuff 
like this. Emitting a NOTICE comes to mind.

--
Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX
Experts in Analytics, Data Architecture and PostgreSQL
Data in Trouble? Get it in Treble! http://BlueTreble.com
855-TREBLE2 (855-873-2532)   mobile: 512-569-9461


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

Re: [HACKERS] Re: GiST optimizing memmoves in gistplacetopage for fixed-size updates [PoC]

[Andrew Borodin]

Anastasia , thank you for your attentive code examine.

2016-08-05 21:19 GMT+05:00 Anastasia Lubennikova :
> First of all, shouldn't we use MAXALIGN(oldsize) instead of oldsize?
> Although, I'm quite sure that it was already aligned somewhere before.
> I doubt that the check (size_diff != MAXALIGN(size_diff)) is necessary.
> I'd rather add  the check: (offset+size_diff < pd_lower).
Actually, that's a tricky question. There may be different code expectations.
1. If we expect that not-maxaligned tuple may be placed by any other
routine, we should remove check (size_diff != MAXALIGN(size_diff)),
since it will fail for not-maxaligned tuple.
2. If we expect that noone should use PageIndexTupleOverwrite with
not-maxaligned tuples, than current checks are OK: we will break
execution if we find non-maxaligned tuples. With an almost correct
message.
I suggest that this check may be debug-only assertion: in a production
code routine will work with not-maxaligned tuples if they already
reside on the page, in a dev build it will inform dev that something
is going wrong. Is this behavior Postgres-style compliant?


I agree that pd_lower check makes sense.

> BTW, I'm very surprised that it improves performance so much.
> And also size is reduced significantly. 89MB against 289MB without patch.
> Could you explain in details, why does it happen?
Size reduction is unexpected for me.
There might be 4 plausible explanations. I'll list them ordered by
descending of probability:
1. Before this patch every update was throwing recently updated tuple
to the end of a page. Thus, in some-how ordered data, recent best-fit
will be discovered last. This can cause increase of MBB's overlap in
spatial index and slightly higher tree. But 3 times size decrease is
unlikely.
How did you obtained those results? Can I look at a test case?
2. Bug in PageIndexTupleDelete causing unused space emersion. I've
searched for it, unsuccessfully.
3. Bug in PageIndexTupleOVerwrite. I cannot imagine nature of such a
bug. May be we are not placing something not very important and big on
a page?
4. Magic.
I do not see what one should do with the R-tree to change it's size by
a factor of 3. First three explanations are not better that forth,
actually.
Those 89 MB, they do not include WAL, right?

Thank you for the review.


Best regards, Andrey Borodin, Octonica & Ural Federal University.


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

Re: [HACKERS] Possible duplicate release of buffer lock.

[Tom Lane]

Amit Kapila  writes:
> Isn't the problem here is that due to some reason (like concurrent
> split), the code in question (loop --
> while (P_ISDELETED(opaque) || opaque->btpo_next != target)) releases
> the lock on target buffer and the caller again tries to release the
> lock on target buffer when false is returned.

Yeah.  I do not think there is anything wrong with the loop-to-find-
current-leftsib logic.  The problem is lack of care about what
resources are held on failure return.  The sole caller thinks it
should do "_bt_relbuf(rel, buf)" --- that is, drop lock and pin on
what _bt_unlink_halfdead_page calls the leafbuf.  But that function
already dropped the lock (line 1582 in 9.4), and won't have reacquired
it unless target != leafblkno.  Another problem is that if the target
is different from leafblkno, we'll hold a pin on the target page, which
is leaked entirely in this code path.

The caller knows nothing of the "target" block so it can't reasonably
deal with all these cases.  I'm inclined to change the function's API
spec to say that on failure return, it's responsible for dropping
lock and pin on the passed buffer.  We could alternatively reacquire
lock before returning, but that seems pointless.

Another thing that looks fishy is at line 1611:

leftsib = opaque->btpo_prev;

At this point we already released lock on leafbuf so it seems pretty
unsafe to fetch its left-link.  And it's unnecessary cause we already
did; the line should be just

leftsib = leafleftsib;

regards, tom lane


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

Re: [HACKERS] No longer possible to query catalogs for index capabilities?

[Bruce Momjian]

On Sat, Aug  6, 2016 at 04:04:41PM +0100, Andrew Gierth wrote:
> > "Bruce" == Bruce Momjian  writes:
> 
>  Bruce> Would it be helpful to output an array of strings representing
>  Bruce> the index definition?
> 
>  >> Why would that help, if the point is to enable programmatic access
>  >> to information?
> 
>  Bruce> I was thinking an array of strings would avoid problems in
>  Bruce> having to re-scan the output for tokens.
> 
> OK, but that still leaves the issue of how to interpret each string,
> yes?

Yes, you still have to parse it, just not scan/tokenize it.

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

+ As you are, so once was I. As I am, so you will be. +
+ Ancient Roman grave inscription +


-- 
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] No longer possible to query catalogs for index capabilities?

[Andrew Gierth]

> "Bruce" == Bruce Momjian  writes:

 Bruce> Would it be helpful to output an array of strings representing
 Bruce> the index definition?

 >> Why would that help, if the point is to enable programmatic access
 >> to information?

 Bruce> I was thinking an array of strings would avoid problems in
 Bruce> having to re-scan the output for tokens.

OK, but that still leaves the issue of how to interpret each string,
yes?

-- 
Andrew (irc:RhodiumToad)


-- 
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] No longer possible to query catalogs for index capabilities?

[Bruce Momjian]

On Sat, Aug  6, 2016 at 01:00:15PM +0100, Andrew Gierth wrote:
> > "Bruce" == Bruce Momjian  writes:
> 
>  >> As far as I understood Andrew's use case, he was specifically *not*
>  >> interested in a complete representation of an index definition, but
>  >> rather about whether it had certain properties that would be of
>  >> interest to query-constructing applications.
> 
> Well, I wouldn't limit it to query-constructing applications.
> 
> I'll give another random example that I thought of. Suppose an
> administrative GUI (I have no idea if any of the existing GUIs do this)
> has an option to do CLUSTER on a table; how should it know which indexes
> to offer the user to cluster on, without access to amclusterable?
> 
>  Bruce> Would it be helpful to output an array of strings representing
>  Bruce> the index definition?
> 
> Why would that help, if the point is to enable programmatic access to
> information?

I was thinking an array of strings would avoid problems in having to
re-scan the output for tokens.

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

+ As you are, so once was I. As I am, so you will be. +
+ Ancient Roman grave inscription +


-- 
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] Heap WARM Tuples - Design Draft

[Bruce Momjian]

On Sat, Aug  6, 2016 at 10:08:47AM +0530, Pavan Deolasee wrote:
> So, we are only checking the index if the WARM chain was pruned, and we
> can bail out if there is only one index changed.  This is looking more
> doable.
> 
> 
> The duplicate tuples problem that we are focusing on, happens when an index
> already has two or index tuples pointing to the same root tuple/lp. When it's
> time to insert third index tuple, we must not insert a duplicate (key, CTID)
> tuple. I've a design where we can track which columns (we are interested only
> in the columns on which indexes use) were ever changed in the WARM chain. We
> allow one change for every index column, but the second change will require a
> duplicate lookup. This is still quite an improvement, the cold updates may
> reduce by at least more than 50% already, but someone can argue that this does
> not handle the case where same index column is repeatedly updated.

Yes, I was thinking of that too.  We can use LP_REDIRECT ctid's lp_len,
which is normally all zeros and has 15 unused bits, to record which of
the first 15 columns have been had changed tuples pruned.  A simpler
idea would be to use just one bit to record if WARM tuples have been
pruned, because if we prune HOT tuples, we can still traverse the chain.

A more serious concern is index creation.  If we already have a WARM
chain and create an index, we can't assume that all indexes will have
entries for change in the WARM chain.  One fix would be for index
creation to walk warm chains and and add index entries for all changed
columns.  We don't need to that now because we assume the index will
only be used by new transactions that can't see the old chain.  Another
idea would be to look at the index xmin and determine if the index was
recording tuples in the chain.

(FYI, in your idea above, I don't think you can track which _indexes_
had changes because index creation will need to know information on
column changes that weren't recorded at WARM chain creation time, hence
I am tracking column changes, not index changes.)

Another issue is that while index walking is usually fast, there are
pathological cases.  For example, if a column value is 80% of all rows,
the optimizer would use statistics to avoid using the index for that
constant, but if someone changes a WARM chain to use that value, we
would need to read 80% of the index to find if the key/ctid exists, and
in most cases it will not, so we have to read the entire thing.

We do have the fallback to create a non-HOT, non-WARM tuple for this
case for single-index changes.

> If we need to find an efficient way to convert WARM chains back to HOT, which
> will happen soon when the old index tuple retires, the system can attain a
> stable state, not for all but many use cases.

I don't see how that is possible, except perhaps by vacuum.

OK, now I am less certain.

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

+ As you are, so once was I. As I am, so you will be. +
+ Ancient Roman grave inscription +


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

Re: [HACKERS] Parallel tuplesort (for parallel B-Tree index creation)

[Amit Kapila]

On Sat, Aug 6, 2016 at 2:16 AM, Peter Geoghegan  wrote:
> On Fri, Aug 5, 2016 at 9:06 AM, Robert Haas  wrote:
>> There are some somewhat outdated and perhaps naive ideas about this
>> that we wrote up here:
>>
>> https://wiki.postgresql.org/wiki/Parallel_Sort
>
> I'm familiar with that effort. I think that when research topics like
> sorting, it can sometimes be a mistake to not look at an approach
> specifically recommended by the database research community. A lot of
> the techniques we've benefited from within tuplesort.c have been a
> matter of addressing memory latency as a bottleneck; techniques that
> are fairly simple and not worth writing a general interest paper on.
> Also, things like abbreviated keys are beneficial in large part
> because people tend to follow the first normal form, and therefore an
> abbreviated key can contain a fair amount of entropy most of the time.
> Similarly, Radix sort seems really cool, but our requirements around
> generality seem to make it impractical.
>
>> Anyway, you're proposing an algorithm that can't be fully
>> parallelized.  Maybe that's OK.  But I'm a little worried about it.
>> I'd feel more confident if we knew that the merge could be done in
>> parallel and were just leaving that to a later development stage; or
>> if we picked an algorithm like the one above that doesn't leave a
>> major chunk of the work unparallelizable.
>
> I might be able to resurrect the parallel merge stuff, just to guide
> reviewer intuition on how much that can help or hurt.
>

I think here some of the factors like how many workers will be used
for merge phase might impact the performance.   Having too many
workers can lead to more communication cost and having too few workers
might not yield best results for merge.  One thing, I have noticed
that in general for sorting, some of the other databases uses range
partitioning [1], now that might not be what is good for us.  I see
you mentioned above that why it is not good [2], but I don't
understand why you think it is a risky assumption to assume good
partition boundaries for parallelizing sort.


[1] -
https://docs.oracle.com/cd/E11882_01/server.112/e25523/parallel002.htm
Refer Producer or Consumer Operations section.

[2] -
"That approach would not suit CREATE INDEX, because the approach's
great strength is that the workers can run in parallel for the entire
duration, since there is no merge bottleneck (this assumes good
partition boundaries, which is of a bit risky assumption)"


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


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

Re: [HACKERS] [sqlsmith] Crash in GetOldestSnapshot()

[Andrew Gierth]

> "Amit" == Amit Kapila  writes:

 Amit> Sure, that is the reason of crash, but even if we do that it will
 Amit> lead to an error "no known snapshots".  Here, what is going on is
 Amit> that we initialized toast snapshot when there is no active
 Amit> snapshot in the backend, so GetOldestSnapshot() won't return any
 Amit> snapshot.

Hmm.

So this happens because RETURNING queries run to completion immediately
and populate a tuplestore with the results, and the portal then fetches
from the tuplestore to send to the destination. The assumption is that
the tuplestore output can be processed without needing a snapshot, which
obviously is not true now if it contains toasted data.

In a similar case in the past involving holdable cursors, the solution
was to detoast _before_ storing in the tuplestore (see
PersistHoldablePortal). I guess the question now is, under what
circumstances is it now allowable to detoast a datum with no active
snapshot? (Wouldn't it be necessary in such a case to know what the
oldest snapshot ever used in the transaction was?)

 Amit> I think for such situations, we need to initialize the lsn and
 Amit> whenTaken of ToastSnapshot as we do in GetSnapshotData() [1].

Would that not give a too-recent LSN, resulting in possibly failing to
fetch the toast rows?

-- 
Andrew (irc:RhodiumToad)


-- 
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] [sqlsmith] Crash in GetOldestSnapshot()

[Amit Kapila]

On Sat, Aug 6, 2016 at 5:51 PM, Andrew Gierth
 wrote:
>> "Andreas" == Andreas Seltenreich  writes:
>
> 418   if (OldestActiveSnapshot != NULL)
> 419   ActiveLSN = OldestActiveSnapshot->as_snap->lsn;
> 420
> 421   if (XLogRecPtrIsInvalid(RegisteredLSN) || RegisteredLSN > ActiveLSN)
> 422   return OldestActiveSnapshot->as_snap;
>
> This second conditional should clearly be inside the first one...
>

Sure, that is the reason of crash, but even if we do that it will lead
to an error "no known snapshots".  Here, what is going on is that we
initialized toast snapshot when there is no active snapshot in the
backend, so GetOldestSnapshot() won't return any snapshot.  I think
for such situations, we need to initialize the lsn and whenTaken of
ToastSnapshot as we do in GetSnapshotData() [1].  We need to do this
when snapshot returned by GetOldestSnapshot() is NULL.

Thoughts?


[1]
In below code
if (old_snapshot_threshold < 0)
{
..
}
else
{
/*
* Capture the current time and WAL stream location in case this
* snapshot becomes old enough to need to fall back on the special
* "old snapshot" logic.
*/
snapshot->lsn = GetXLogInsertRecPtr();
snapshot->whenTaken = GetSnapshotCurrentTimestamp();
MaintainOldSnapshotTimeMapping(snapshot->whenTaken, xmin);
}


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


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

Re: [HACKERS] [sqlsmith] Crash in GetOldestSnapshot()

[Andrew Gierth]

> "Andreas" == Andreas Seltenreich  writes:

418   if (OldestActiveSnapshot != NULL)
419   ActiveLSN = OldestActiveSnapshot->as_snap->lsn;
420
421   if (XLogRecPtrIsInvalid(RegisteredLSN) || RegisteredLSN > ActiveLSN)
422   return OldestActiveSnapshot->as_snap;

This second conditional should clearly be inside the first one...

-- 
Andrew (irc:RhodiumToad)


-- 
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] [sqlsmith] Crash in GetOldestSnapshot()

[Michael Paquier]

On Sat, Aug 6, 2016 at 6:32 PM, Andreas Seltenreich  wrote:
> since updating master from c93d873..fc509cd, I see crashes in
> GetOldestSnapshot() on update/delete returning statements.
>
> I reduced the triggering statements down to this:
>
> update clstr_tst set d = d returning d;
>
> Backtrace below.

3e2f3c2e is likely to blame here.. I have moved the open item
"old_snapshot_threshold allows heap:toast disagreement" back to the
list of open items.
-- 
Michael


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

Re: [HACKERS] No longer possible to query catalogs for index capabilities?

[Andrew Gierth]

> "Bruce" == Bruce Momjian  writes:

 >> As far as I understood Andrew's use case, he was specifically *not*
 >> interested in a complete representation of an index definition, but
 >> rather about whether it had certain properties that would be of
 >> interest to query-constructing applications.

Well, I wouldn't limit it to query-constructing applications.

I'll give another random example that I thought of. Suppose an
administrative GUI (I have no idea if any of the existing GUIs do this)
has an option to do CLUSTER on a table; how should it know which indexes
to offer the user to cluster on, without access to amclusterable?

 Bruce> Would it be helpful to output an array of strings representing
 Bruce> the index definition?

Why would that help, if the point is to enable programmatic access to
information?

Anyway, what I haven't seen in this thread is any implementable
counter-proposal other than the "just hardcode the name 'btree'"
response that was given in the JDBC thread, which I don't consider
acceptable in any sense. Is 9.6 going to go out like this or is action
going to be taken before rc1?

-- 
Andrew (irc:RhodiumToad)


-- 
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] Quorum commit for multiple synchronous replication.

[Petr Jelinek]

On 04/08/16 06:40, Masahiko Sawada wrote:

On Wed, Aug 3, 2016 at 3:05 PM, Michael Paquier
 wrote:

On Wed, Aug 3, 2016 at 2:52 PM, Masahiko Sawada  wrote:

I was thinking that the syntax for quorum method would use '[ ... ]'
but it will be confused with '( ... )' priority method used.
001 patch adds 'Any N ( ... )' style syntax but I know that we still
might need to discuss about better syntax, discussion is very welcome.
Attached draft patch, please give me feedback.


I am +1 for using either "{}" or "[]" to define a quorum set, and -1
for the addition of a keyword in front of the integer defining for how
many nodes server need to wait for.


Thank you for reply.
"{}" or "[]" are not bad but because these are not intuitive, I
thought that it will be hard for uses to use different method for each
purpose.



I think the "any" keyword is more explicit and understandable, also 
closer to SQL. So I would be in favor of doing that.


--
  Petr Jelinek  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] [sqlsmith] FailedAssertion("!(XLogCtl->Insert.exclusiveBackup)", File: "xlog.c", Line: 10200)

[Andreas Seltenreich]

Michael Paquier writes:

> Andreas, with the patch attached is the assertion still triggered?
> [2. text/x-diff; base-backup-crash-v2.patch]

I didn't observe the crashes since applying this patch.  There should
have been about five by the amount of fuzzing done.

regards
Andreas


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

[HACKERS] [sqlsmith] Crash in GetOldestSnapshot()

[Andreas Seltenreich]

Hi,

since updating master from c93d873..fc509cd, I see crashes in
GetOldestSnapshot() on update/delete returning statements.

I reduced the triggering statements down to this:

update clstr_tst set d = d returning d;

Backtrace below.

regards,
Andreas

Program received signal SIGSEGV, Segmentation fault.
(gdb) bt
#0  GetOldestSnapshot () at snapmgr.c:422
#1  0x004b8279 in init_toast_snapshot (toast_snapshot=0x7ffcd824b010) 
at tuptoaster.c:2314
#2  0x004b83bc in toast_fetch_datum (attr=) at 
tuptoaster.c:1869
#3  0x004b9ab5 in heap_tuple_untoast_attr (attr=0x18226c8) at 
tuptoaster.c:179
#4  0x007f71ad in pg_detoast_datum_packed (datum=) at 
fmgr.c:2266
#5  0x007cfc12 in text_to_cstring (t=0x18226c8) at varlena.c:186
#6  0x007f5735 in FunctionCall1Coll (flinfo=flinfo@entry=0x18221c0, 
collation=collation@entry=0, arg1=arg1@entry=25306824) at fmgr.c:1297
#7  0x007f68ee in OutputFunctionCall (flinfo=0x18221c0, val=25306824) 
at fmgr.c:1946
#8  0x00478bc1 in printtup (slot=0x1821f80, self=0x181ce48) at 
printtup.c:359
#9  0x006f9c8e in RunFromStore (portal=portal@entry=0x177cbf8, 
direction=direction@entry=ForwardScanDirection, count=count@entry=0, 
dest=0x181ce48) at pquery.c:1117
#10 0x006f9d52 in PortalRunSelect (portal=portal@entry=0x177cbf8, 
forward=forward@entry=1 '\001', count=0, count@entry=9223372036854775807, 
dest=dest@entry=0x181ce48) at pquery.c:942
#11 0x006fb41e in PortalRun (portal=portal@entry=0x177cbf8, 
count=count@entry=9223372036854775807, isTopLevel=isTopLevel@entry=1 '\001', 
dest=dest@entry=0x181ce48, altdest=altdest@entry=0x181ce48, 
completionTag=completionTag@entry=0x7ffcd824b920 "") at pquery.c:787
#12 0x006f822b in exec_simple_query (query_string=0x17db878 "update 
clstr_tst set d = d returning d;") at postgres.c:1094
#13 PostgresMain (argc=, argv=argv@entry=0x1781ce0, 
dbname=0x1781b40 "regression", username=) at postgres.c:4074
#14 0x0046c9bd in BackendRun (port=0x1786920) at postmaster.c:4262
#15 BackendStartup (port=0x1786920) at postmaster.c:3936
#16 ServerLoop () at postmaster.c:1693
#17 0x00693044 in PostmasterMain (argc=argc@entry=3, 
argv=argv@entry=0x175d5f0) at postmaster.c:1301
#18 0x0046dd26 in main (argc=3, argv=0x175d5f0) at main.c:228
(gdb) list
417
418 if (OldestActiveSnapshot != NULL)
419 ActiveLSN = OldestActiveSnapshot->as_snap->lsn;
420
421 if (XLogRecPtrIsInvalid(RegisteredLSN) || RegisteredLSN > 
ActiveLSN)
422 return OldestActiveSnapshot->as_snap;
423
424 return OldestRegisteredSnapshot;
425 }
426
(gdb) bt full
#0  GetOldestSnapshot () at snapmgr.c:422
OldestRegisteredSnapshot = 
RegisteredLSN = 
ActiveLSN = 
#1  0x004b8279 in init_toast_snapshot (toast_snapshot=0x7ffcd824b010) 
at tuptoaster.c:2314
snapshot = 
#2  0x004b83bc in toast_fetch_datum (attr=) at 
tuptoaster.c:1869
toastrel = 0x7f8b4ca88920
toastidxs = 0x18447c8
toastkey = {
  sk_flags = 0,
  sk_attno = 1,
  sk_strategy = 3,
  sk_subtype = 0,
  sk_collation = 100,
  sk_func = {
fn_addr = 0x77c490 ,
fn_oid = 184,
fn_nargs = 2,
fn_strict = 1 '\001',
fn_retset = 0 '\000',
fn_stats = 2 '\002',
fn_extra = 0x0,
fn_mcxt = 0x18282a8,
fn_expr = 0x0
  },
  sk_argument = 34491
}
toastscan = 
ttup = 
toasttupDesc = 0x7f8b4ca88c50
result = 0x18422d8
toast_pointer = 
ressize = 5735
residx = 
nextidx = 0
numchunks = 3
chunk = 
isnull = 
chunkdata = 
chunksize = 
num_indexes = 1
validIndex = 0
SnapshotToast = {
  satisfies = 0x112,
  xmin = 3626283536,
  xmax = 32764,
  xip = 0xf8ac628,
  xcnt = 5221870,
  subxip = 0x0,
  subxcnt = 0,
  suboverflowed = 0 '\000',
  takenDuringRecovery = 0 '\000',
  copied = 0 '\000',
  curcid = 14,
  speculativeToken = 0,
  active_count = 260753304,
  regd_count = 0,
  ph_node = {
first_child = 0xf8ac680,
next_sibling = 0xa400112,
prev_or_parent = 0x0
  },
  whenTaken = 274,
  lsn = 0
}
__func__ = "toast_fetch_datum"
#3  0x004b9ab5 in heap_tuple_untoast_attr (attr=0x18226c8) at 
tuptoaster.c:179
No locals.
#4  0x007f71ad in pg_detoast_datum_packed (datum=) at 
fmgr.c:2266
No locals.
#5  0x007cfc12 in text_to_cstring (t=0x18226c8) at varlena.c:186
tunpacked = 
result = 
#6  0x007f5735 in FunctionCall1Coll (flinfo=flinfo@entry=0x18221c0, 

Re: [HACKERS] [Patch] Temporary tables that do not bloat pg_catalog (a.k.a fast temp tables)

[Dmitry Dolgov]

​Hi

I tried to dig into this patch (which seems pretty interesting) to help
bring
it in good shape. Here are few random notes, I hope they can be helpful:

> I think we actually need a real API here.
Definitely, there are plenty places in the new code with the same pattern:
 * figure out if it's an action related to the fast temporary tables based
on
   `ItemPointer`/relation OID/etc
 * if it is, add some extra logic or skip something in original
implementation

in `heapam.c`, `indexam.c`, `xact.c`, `dependency.c`. I believe it's
possible to
make it more generic (although it will contain almost the same logic), e.g.
separate regular and fasttable implementations completely, and decide which
one
we should choose in that particular case.

Btw, I'm wondering about the `heap_*` functions in `heapam.c` - some of
them are
wrapped and never used directly, although they're contains in
`INTERFACE_ROUTINES` (like `simple_heap_delete` -> `heap_delete`), some of
them
aren't. It looks like inconsistency in terms of function names, probably it
should be unified?

> What is needed is an overview of the approach, so that the reviewers can
read
> that first,
I feel lack of such information even in new version of this patch (but, I'm
probably a noob in these matters).  I noted that the `fasttab.c` file
contains some
general overview, but in terms of "what are we doing", and "why are we doing
this". I think general overview of "how are we doing this" also may be
useful.
And there are several slightly obvious commentaries like:

```
+ /* Is it a virtual TID? */
+ if (IsFasttabItemPointer(tid))
```

but I believe an introduction of a new API (see the previous note) will
solve
this eventually.

> Why do we need the new relpersistence value? ISTM we could easily got with
> just RELPERSISTENCE_TEMP, which would got right away of many chances as
the
> steps are exactly the same.
I agree, it looks like `RELPERSISTENCE_FAST_TEMP` hasn't any influence on
the
code.

> For example, suppose I create a fast temporary table and then I create a
> functional index on the fast temporary table that uses some SQL function
> defined in pg_proc.
Just to clarify, did you mean something like this?
```
create fast temp table fasttab(x int, s text);
create or replace function test_function_for_index(t text) returns text as
$$
begin
return lower(t);
end;
$$ language plpgsql immutable;
create index fasttab_s_idx on fasttab (test_function_for_index(s));
drop function test_function_for_index(t text);
```
As far as I understand dependencies should protect in case of fasttable too,
because everything is visible as in regular case, isn't it?

And finally one more question, why items of `FasttabIndexMethodsTable[]`
like
this one:
```
+ /* 2187, non-unique */
+ {InheritsParentIndexId, 1,
+ {Anum_pg_inherits_inhparent, 0, 0},
+ {CompareOid, CompareInvalid, CompareInvalid}
+ },
```
have this commentary before them? I assume it's an id and an extra
information,
and I'm concerned that they can easily become outdated inside commentary
block.

Re: [HACKERS] Surprising behaviour of \set AUTOCOMMIT ON

[Brendan Jurd]

As an extra data point, if you try this in Python (psycopg2) you get an
exception:

psycopg2.ProgrammingError: autocommit cannot be used inside a transaction

I think this exception is a legitimate response.  If the user switches on
autocommit mode inside a transaction, it was most likely not on purpose.
Chances are, they didn't realise autocommit was off in the first place.

Even if we assume that it was done deliberately, it's difficult to know
exactly what the user intended.  It seems to hinge on a subtlety of what
the user understands autocommit mode to mean -- either "issue an implicit
COMMIT after each statement", or "ensure there is never an open
transaction".

I feel that raising an error is a sane move here -- it is reasonable to
insist that the user make their intention unambiguous.

Cheers,
BJ



On Sat, 6 Aug 2016 at 15:30 Amit Kapila  wrote:

> On Thu, Aug 4, 2016 at 7:46 PM, Pavel Stehule 
> wrote:
>
> > 2016-08-04 15:37 GMT+02:00 Amit Kapila :
> >>
> >> > I dislike automatic commit or rollback here.
> >> >
> >>
> >> What problem you see with it, if we do so and may be mention the same
> >> in docs as well.  Anyway, I think we should make the behaviour of both
> >> ecpg and psql same.
> >
> >
> > Implicit COMMIT can be dangerous
> >
>
> Not, when user has specifically requested for autocommit mode as 'on'.
> I think here what would be more meaningful is that after "Set
> AutoCommit On", when the first command is committed, it should commit
> previous non-pending committed commands as well.
>
> >>
> >> Not sure what benefit we will get by raising warning.  I think it is
> >> better to choose one behaviour (automatic commit or leave the
> >> transaction open as is currently being done in psql) and make it
> >> consistent across all clients.
> >
> >
> > I am not sure about value of ecpg for this case. It is used by 0.0001%
> > users. Probably nobody in Czech Republic knows this client.
> >
>
> Sure, but that doesn't give us the license for being inconsistent in
> behaviour across different clients.
>
> > Warnings enforce the user do some decision
> >
>
> They could be annoying as well, especially if that happens in scripts.
>
>
> --
> With Regards,
> Amit Kapila.
> EnterpriseDB: http://www.enterprisedb.com
>
>
> --
> Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers
>

Re: [HACKERS] Possible duplicate release of buffer lock.

[Amit Kapila]

On Fri, Aug 5, 2016 at 11:27 AM, Kyotaro HORIGUCHI
 wrote:
> Hello,
>
> If concurrent deletion (marking half-dead, specifically) can't
> happen, P_ISDELETED() won't be seen in the loop but we may
> consider it as a skippable condition to continue VACUUM. But
> still I think we shouldn't release the lock on the target page
> there. But it doesn't harm except that VACUUM stops there so I
> don't mind it will be as it is. So I'd like to have opinions of
> others about this.
>

Isn't the problem here is that due to some reason (like concurrent
split), the code in question (loop --
while (P_ISDELETED(opaque) || opaque->btpo_next != target)) releases
the lock on target buffer and the caller again tries to release the
lock on target buffer when false is returned.  So, rather than
changing the loop in a way suggested by you, why can't we ensure that
we don't release the lock on target buffer in that loop or we can
check in the caller such that if target buffer is valid (for a failure
case), then release it.

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


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

Re: [HACKERS] Surprising behaviour of \set AUTOCOMMIT ON

[Pavel Stehule]

2016-08-06 7:29 GMT+02:00 Amit Kapila :

> On Thu, Aug 4, 2016 at 7:46 PM, Pavel Stehule 
> wrote:
>
> > 2016-08-04 15:37 GMT+02:00 Amit Kapila :
> >>
> >> > I dislike automatic commit or rollback here.
> >> >
> >>
> >> What problem you see with it, if we do so and may be mention the same
> >> in docs as well.  Anyway, I think we should make the behaviour of both
> >> ecpg and psql same.
> >
> >
> > Implicit COMMIT can be dangerous
> >
>
> Not, when user has specifically requested for autocommit mode as 'on'.
> I think here what would be more meaningful is that after "Set
> AutoCommit On", when the first command is committed, it should commit
> previous non-pending committed commands as well.
>

This is place when safety and and user friendly interface going against -
the most safe behave is raising rollback there. But it can be in contrast
with user's expectation.

>
> >>
> >> Not sure what benefit we will get by raising warning.  I think it is
> >> better to choose one behaviour (automatic commit or leave the
> >> transaction open as is currently being done in psql) and make it
> >> consistent across all clients.
> >
> >
> > I am not sure about value of ecpg for this case. It is used by 0.0001%
> > users. Probably nobody in Czech Republic knows this client.
> >
>
> Sure, but that doesn't give us the license for being inconsistent in
> behaviour across different clients.
>

This is question. ecpg was designed years ago - and some details can be
designed wrong.

Next question is design for interactive and non interactive usage.


>
> > Warnings enforce the user do some decision
> >
>
> They could be annoying as well, especially if that happens in scripts.
>

in script - probably rollback is correct - script can be executed more time
and user can fix it.

I am not sure if we can solve this issue as isolated problem. The first
question should be - who, why and when does switching from autocommit off
to on? How often this operation is? And we should be safe or we should not?


Regards

Pavel

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