Re: [DOC] Document concurrent index builds waiting on each other

[David Johnston]

The following review has been posted through the commitfest application:
make installcheck-world:  not tested
Implements feature:   not tested
Spec compliant:   not tested
Documentation:tested, passed

James,

I'm on board with the point of pointing out explicitly the "concurrent index 
builds on multiple tables at the same time will not return on any one table 
until all have completed", with back-patching.  I do not believe the new 
paragraph is necessary though.  I'd suggest trying to weave it into the 
existing paragraph ending "Even then, however, the index may not be immediately 
usable for queries: in the worst case, it cannot be used as long as 
transactions exist that predate the start of the index build."  Adding 
"Notably, " in front of the existing sentence fragment above and tacking it 
onto the end probably suffices.

I don't actually don't whether this is true behavior though.  Is it something 
our tests do, or could, demonstrate?

It is sorta weird to say "one will not return until all have completed, though, 
since usually people think return means completed".  That whole paragraph is a 
bit unclear for the inexperienced DBA, in particular marked ready to use but 
isn't usable.

That isn't really on this patch to fix though, and the clarity around 
concurrent CIC seems worthwhile to add, even if imprecise - IMO it doesn't make 
that whole section any less clear and points out what seems to be a unique 
dynamic.  IOW I would send the simple fix (inline, not a new paragraph) to a 
committer.  The bigger doc reworking or actual behavioral improvements 
shouldn't hold up such a simple improvement.

David J.

The new status of this patch is: Waiting on Author

Re: psql FETCH_COUNT feature does not work with combined queries

[David Johnston]

The following review has been posted through the commitfest application:
make installcheck-world:  not tested
Implements feature:   not tested
Spec compliant:   not tested
Documentation:tested, passed

Fabien,

There is a minor typo (works -> work) but overall I'm not a fan of the wording.

Instead of a note maybe add a paragraph stating:

"This setting is ignored when multiple statements are sent to the server as a 
single command (i.e., via the -c command line option or the \; meta-command).  
Additionally, it is possible for certain combinations of statements sent in 
that manner to instead return no results, or even be ignored, instead of 
returning the result set of the last query.  In short, when FETCH_COUNT is 
non-zero, and you send a multi-statement command to the server, the results are 
undefined. This combination in presently allowed for backward compatibility."

I would suggest, however, adding some tests in src/test/psql.sql demonstrating 
the broken behavior.  A potentially useful test setup would be:
create temp table testtbl (idx int, div int);
insert into testtbl (1,1), (2,1),(3,1),(4,0),(5,1);
and combine that with FETCH_COUNT 3

Some other things I tried, with and without FETCH_COUNT:

begin \; select 2 \; commit \; select 1 / div from (select div from testtbl 
order by idx) as src;
select 1/0 \; select 1 / div from (select div from testtbl where div <> 0 order 
by idx) as src;
begin \; select 2 \; select 1 \; commit;
begin \; select 2 \; select 1;
commit;

I'm not sure how to go about getting a equivalent result of "select pg_sleep(2) 
\; select 1;" not sleeping.  If I put select 1/0 instead of pg_sleep I get an 
error and any DML seems to end up working just fine (minus actual batch 
fetching)
update testtbl set val = 2 \; select 1; --result (1)

David J.

The new status of this patch is: Waiting on Author