Re: \if, \elseif, \else, \endif (was Re: [HACKERS] PSQL commands: \quit_if, \quit_unless)

[Fabien COELHO]

Hello Corey,


In cases where there was a boolean parsing failure, and ON_ERROR_STOP is
on, the error message no longer speak of a future which the session does
not have. I could left the ParseVariableBool() message as the only one, but
wasn't sure that that was enough of an error message on its own.
Added the test case to the existing tap tests. Incidentally, the tap tests
aren't presently fooled into thinking they're interactive.


Yes.


Revised cumulative patch attached for those playing along at home.


Nearly there...

It seems that ON_ERROR_STOP is mostly ignored by design when in 
interactive mode, probably because it is nicer not to disconnect the user 
who is actually typing things on a terminal.


"""
  ON_ERROR_STOP

By default, command processing continues after an error. When this 
variable is set to on, processing will instead stop immediately. In 
interactive mode, psql will return to the command prompt; otherwise, psql 
will exit, returning error code 3 to distinguish this case from fatal 
error conditions, which are reported using error code 1.

"""

So, you must check for interactive as well when shortening the message, 
and adapting it accordingly, otherwise on gets the wrong message in 
interactive mode:


   bash> ./psql -v ON_ERROR_STOP=1
   psql (10devel, server 9.6.1)
   Type "help" for help.

   calvin=# \if error
   unrecognized value "error" for "\if ": boolean expected
   new \if is invalid.
   calvin=# -- not stopped, but the stack has been cleaned up, I think

Basically it seems that there are 4 cases and 2 behaviors:

 - on_error_stop && scripting:
 actually exit on error

 - on_error_stop && interactive, !on_error_stop whether scripting or not:
 keep going, possibly with nesting checks?

The problem is that currently interactive behavior cannot be tested 
automatically.


--
Fabien.


--
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] IF [NOT] EXISTS for replication slots

[Petr Jelinek]

On 07/02/17 01:00, Michael Paquier wrote:
> On Tue, Feb 7, 2017 at 2:07 AM, Petr Jelinek
>  wrote:
>> On 06/02/17 05:15, Michael Paquier wrote:
>>> Hi all,
>>>
>>> Lately I have bumped into a case where it would have been useful to
>>> make the difference between a failure because of a slot already
>>> dropped and an internal failure of Postgres. Is there any interest for
>>> support of IE and INE for CREATE and DROP_REPLICATION_SLOT?
>>> My use case involved only the SQL-callable interface, but I would
>>> think that it is useful to make this difference as well with the
>>> replication protocol. For the function we could just add a boolean
>>> argument to control the switch, and for the replication commands a
>>> dedicated keyword.
>>>
>>> Any thoughts?
>>
>> My thought is, how would this handle the snapshot creation that logical
>> slot does when it's created?
> 
> In what is that related to IF NOT EXISTS? If the slot does not get
> created you could just return NULL to the caller to let him know that
> there is something already around. 

Well, the current behavior and the common use-case for creating logical
slot via walsender is to get the snapshot which corresponds to LSN, the
above would break that behavior for some variants of the command which I
find rather confusing from user perspective.

> The use-case I have found INE
> useful involved physical slots actually.

I am sure others would find it useful for logical ones as well.

-- 
  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] Proposal : For Auto-Prewarm.

[Amit Kapila]

On Tue, Feb 7, 2017 at 10:44 AM, Mithun Cy  wrote:
>
> ==
> One problem now I have kept it open is multiple "auto pg_prewarm dump"
> can be launched even if already a dump/load worker is running by
> calling launch_pg_prewarm_dump. I can avoid this by dropping a
> lock-file before starting the bgworkers. But, if there is an another
> method to avoid launching bgworker on a simple method I can do same.
>

How about keeping a variable in PROC_HDR structure to indicate if
already one dump worker is running, then don't allow to start a new
one?

-- 
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] Proposal : For Auto-Prewarm.

[Amit Kapila]

On Tue, Feb 7, 2017 at 11:53 AM, Beena Emerson  wrote:
> Hello,
>
> Thank you for the updated patch.
>
> On Tue, Feb 7, 2017 at 10:44 AM, Mithun Cy 
> wrote:
>>
>> Hi all,
>> Here is the new patch which fixes all of above comments, I changed the
>> design a bit now as below
>>
>> What is it?
>> ===
>> A pair of bgwrokers one which automatically dumps buffer pool's block
>> info at a given interval and another which loads those block into
>> buffer pool when
>> the server restarts.
>
>
> Are 2 workers required?
>

I think in the new design there is a provision of launching the worker
dynamically to dump the buffers, so there seems to be a need of
separate workers for loading and dumping the buffers.  However, there
is no explanation in the patch or otherwise when and why this needs a
pair of workers.  Also, if the dump interval is greater than zero,
then do we really need to separately register a dynamic worker?


-- 
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] Proposal : For Auto-Prewarm.

[Beena Emerson]

Hello,

Thank you for the updated patch.

On Tue, Feb 7, 2017 at 10:44 AM, Mithun Cy 
wrote:

> Hi all,
> Here is the new patch which fixes all of above comments, I changed the
> design a bit now as below
>
> What is it?
> ===
> A pair of bgwrokers one which automatically dumps buffer pool's block
> info at a given interval and another which loads those block into
> buffer pool when
> the server restarts.
>

Are 2 workers required? This would reduce the number of workers to be
launched by other  applications. Also with max_worker_processes = 2 and
restart, the system crashes when the 2nd worker is not launched:
2017-02-07 11:36:39.132 IST [20573] LOG:  auto pg_prewarm load : number of
buffers actually tried to load 64
2017-02-07 11:36:39.143 IST [18014] LOG:  worker process: auto pg_prewarm
load (PID 20573) was terminated by signal 11: Segmentation fault

I think the document should also mention that an appropriate
max_worker_processes should be set else the dump worker will not be
launched at all.


-- 
Thank you,

Beena Emerson

Have a Great Day!

[HACKERS] pg_stat_wal_write statistics view

[Haribabu Kommi]

Hi Hackers,

I just want to discuss adding of a new statistics view that provides
the information of wal writing details as follows

postgres=# \d pg_stat_wal_writer
View "pg_catalog.pg_stat_wal_writer"
Column |   Type   | Collation | Nullable |
Default
---+--+---+--+-
 num_backend_writes   | bigint   |   |
 |
 num_total_writes  | bigint   |   |  |
 num_blocks  | bigint   |   |  |
 total_write_time   | bigint|   |  |
 stats_reset   | timestamp with time zone |   |  |

The columns of the view are
1. Total number of xlog writes that are called from the backend.
2. Total number of xlog writes that are called from both backend
 and background workers. (This column can be changed to just
 display on the background writes).
3. The number of the blocks that are written.
4. Total write_time of the IO operation it took, this variable data is
filled only when the track_io_timing GUC is enabled.
5. Last time when the stats are reset.

I feel this view information may be useful in finding out how much
time does backend may spend in writing the xlog, based on this
information, it may be possible to tune wal_writer_delay GUC.

Or it is possible to integrate the new columns into the existing
pg_stat_bgwriter view also.

Opinions?

Regards,
Hari Babu
Fujitsu Australia

Re: [HACKERS] GSoC 2017

[Amit Langote]

On 2017/02/06 20:51, Ruben Buchatskiy wrote:
> Also we have seen in the mailing list that Kumar Rajeev had been
> investigating this idea too, and he reported that the results were
> impressive (unfortunately, without specifying more details):
> 
> https://www.postgresql.org/message-id/BF2827DCCE55594C8D7A8F7FFD3AB77159A9B904%40szxeml521-mbs.china.huawei.com

You might also want to take a look at some of the ongoing work in this area:

WIP: Faster Expression Processing and Tuple Deforming (including JIT)
https://www.postgresql.org/message-id/flat/20161206034955.bh33paeralxbtluv%40alap3.anarazel.de

Thanks,
Amit




-- 
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] One-shot expanded output in psql using \gx

[David Fetter]

On Mon, Feb 06, 2017 at 08:54:13PM +0100, Christoph Berg wrote:
> The majority of voices here was in favor of using \gx, so here is
> another version of the same patch which implements that.

Patch is useful, and works as documented.

Maybe it could get a test or two in src/test/regress/*/psql.*

Best,
David.
-- 
David Fetter  http://fetter.org/
Phone: +1 415 235 3778  AIM: dfetter666  Yahoo!: dfetter
Skype: davidfetter  XMPP: david(dot)fetter(at)gmail(dot)com

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate


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

Re: [HACKERS] Proposal : For Auto-Prewarm.

[Mithun Cy]

Hi all,
Here is the new patch which fixes all of above comments, I changed the
design a bit now as below

What is it?
===
A pair of bgwrokers one which automatically dumps buffer pool's block
info at a given interval and another which loads those block into
buffer pool when
the server restarts.

How does it work?
=
When the shared library pg_prewarm is preloaded during server startup.
A bgworker "auto pg_prewarm load" is launched immediately after the
server is started. The bgworker will start loading blocks obtained
from block info entry
 in
$PGDATA/AUTO_PG_PREWARM_FILE, until there is a free buffer in the
buffer pool. This way we do not replace any new blocks which were
loaded either by the recovery process or the querying clients.

Once the "auto pg_prewarm load" bgworker has completed its job, it
will register a dynamic bgworker "auto pg_prewarm dump" which has to
be launched
when the server reaches a consistent state. The new bgworker will
periodically scan the buffer pool and then dump the meta info of
blocks
which are currently in the buffer pool. The GUC
pg_prewarm.dump_interval if set > 0 indicates the minimum time
interval between two dumps. If
pg_prewarm.dump_interval is set to AT_PWARM_DUMP_AT_SHUTDOWN_ONLY the
bgworker will only dump at the time of server shutdown. If it is set
to AT_PWARM_LOAD_ONLY we do not want the bgworker to dump anymore, so
it stops there.

To relaunch a stopped "auto pg_prewarm dump" bgworker we can use the
utility function "launch_pg_prewarm_dump".

==
One problem now I have kept it open is multiple "auto pg_prewarm dump"
can be launched even if already a dump/load worker is running by
calling launch_pg_prewarm_dump. I can avoid this by dropping a
lock-file before starting the bgworkers. But, if there is an another
method to avoid launching bgworker on a simple method I can do same.
Any suggestion on this will be very helpful.

-- 
Thanks and Regards
Mithun C Y
EnterpriseDB: http://www.enterprisedb.com


pg_auto_prewarm_03.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

Re: [HACKERS] Passing query string to workers

[Rafia Sabih]

On Mon, Jan 23, 2017 at 2:46 PM, Kuntal Ghosh
 wrote:
> I've looked into the patch. I've some comments regarding that.
>
> +#define PARALLEL_KEY_QUERY_TEXTUINT64CONST(0xE010)
> It should be UINT64CONST(0xE00A)
>
> +   query_len = strlen(query_data) + 1;
> +   shm_toc_estimate_chunk(>estimator, query_len);
> +   shm_toc_estimate_keys(>estimator, 1);
> +
> Adding a comment before this will be helpful. You should maintain the
> same order for estimating and storing the query string. For example,
> as you've estimated the space for query string after estimation of dsa
> space, you should store the same after creating dsa. Besides, I think
> the appropriate place for this would be before planned statement.
Fixed.

> The regression test for select_parallel is failing after applying the
> patch. You can reproduce the scenario by the following sql statements:
>
> CREATE TABLE t1(a int);
> INSERT INTO t1 VALUES (generate_series(1,10));
> SET parallel_setup_cost=0;
> SET parallel_tuple_cost=0;
> SET min_parallel_relation_size=0;
> SET max_parallel_workers_per_gather=4;
> SELECT count(*) FROM t1;
>
> It is showing the following warning.
> WARNING:  problem in alloc set ExecutorState: detected write past
> chunk end in block 0x14f5310, chunk 0x14f6c50
Fixed.

Thanks a lot Kuntal for the review, please find attached patch for the
revised version.
-- 
Regards,
Rafia Sabih
EnterpriseDB: http://www.enterprisedb.com/


pass_queryText_to_workers_v3.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

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

[Peter Geoghegan]

On Mon, Jan 30, 2017 at 9:15 PM, Peter Geoghegan  wrote:
>> IIUC worker_wait() is only being used to keep the worker around so its
>> files aren't deleted.  Once buffile cleanup is changed to be
>> ref-counted (in an on_dsm_detach hook?) then workers might as well
>> exit sooner, freeing up a worker slot... do I have that right?
>
> Yes. Or at least I think it's very likely that that will end up happening.

I've looked into this, and have a version of the patch where clean-up
occurs when the last backend with a reference to the BufFile goes
away. It seems robust; all of my private tests pass, which includes
things that parallel CREATE INDEX won't use, and yet is added as
infrastructure (e.g., randomAccess recycling of blocks by leader from
workers).

As Thomas anticipated, worker_wait() now only makes workers wait until
the leader comes along to take a reference to their files, at which
point the worker processes can go away. In effect, the worker
processes go away as soon as possible, just as the leader begins its
final on-the-fly merge. At that point, they could be reused by some
other process, of course.

However, there are some specific implementation issues with this that
I didn't quite anticipate. I would like to get feedback on these
issues now, from both Thomas and Robert. The issues relate to how much
the patch can or should "buy into resource management". You might
guess that this new resource management code is something that should
live in fd.c, alongside the guts of temp file resource management,
within the function FileClose(). That way, it would be called by every
possible path that might delete a temp file, including
ResourceOwnerReleaseInternal(). That's not what I've done, though.
Instead, refcount management is limited to a few higher level routines
in buffile.c. Initially, resource management in FileClose() is made to
assume that it must delete the file. Then, if and when directed to by
BufFileClose()/refcount, a backend may determine that it is not its
job to do the deletion -- it will not be the one that must "turn out
the lights", and so indicates to FileClose() that it should not delete
the file after all (it should just release vFDs, close(), and so on).
Otherwise, when refcount reaches zero, temp files are deleted by
FileClose() in more or less the conventional manner.

The fact that there could, in general, be any error that causes us to
attempt a double-deletion (deletion of a temp file from more than one
backend) for a time is less of a problem than you might think. This is
because there is a risk of this only for as long as two backends hold
open the file at the same time. In the case of parallel CREATE INDEX,
this is now the shortest possible period of time, since workers close
their files using BufFileClose() immediately after the leader wakes
them up from a quiescent state. And, if that were to actually happen,
say due to some random OOM error during that small window, the
consequence is no worse than an annoying log message: "could not
unlink file..." (this would come from the second backend that
attempted an unlink()). You would not see this when a worker raised an
error due to a duplicate violation, or any other routine problem, so
it should really be almost impossible.

That having been said, this probably *is* a problematic restriction in
cases where a temp file's ownership is not immediately handed over
without concurrent sharing. What happens to be a small window for the
parallel CREATE INDEX patch probably wouldn't be a small window for
parallel hash join.   :-(

It's not hard to see why I would like to do things this way. Just look
at ResourceOwnerReleaseInternal(). Any release of a file happens
during RESOURCE_RELEASE_AFTER_LOCKS, whereas the release of dynamic
shared memory segments happens earlier, during
RESOURCE_RELEASE_BEFORE_LOCKS. ISTM that the only sensible way to
implement a refcount is using dynamic shared memory, and that seems
hard. There are additional reasons why I suggest we go this way, such
as the fact that all the relevant state belongs to BufFile, which is
implemented a layer above all of the guts of resource management of
temp files within fd.c. I'd have to replicate almost all state in fd.c
to make it all work, which seems like a big modularity violation.

Does anyone have any suggestions on how to tackle this?

-- 
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] asynchronous execution

[Amit Langote]

Horiguchi-san,

On 2017/01/31 12:45, Kyotaro HORIGUCHI wrote:
> I noticed that this patch is conflicting with 665d1fa (Logical
> replication) so I rebased this. Only executor/Makefile
> conflicted.

With the latest set of patches, I observe a crash due to an Assert failure:

#0  0x003969632625 in *__GI_raise (sig=6) at
../nptl/sysdeps/unix/sysv/linux/raise.c:64
#1  0x003969633e05 in *__GI_abort () at abort.c:92
#2  0x0098b22c in ExceptionalCondition (conditionName=0xb30e02
"!(added)", errorType=0xb30d77 "FailedAssertion", fileName=0xb30d50
"execAsync.c",
lineNumber=345) at assert.c:54
#3  0x006883ed in ExecAsyncEventWait (estate=0x13c01b8,
timeout=-1) at execAsync.c:345
#4  0x00687ed5 in ExecAsyncEventLoop (estate=0x13c01b8,
requestor=0x13c1640, timeout=-1) at execAsync.c:186
#5  0x006a5170 in ExecAppend (node=0x13c1640) at nodeAppend.c:257
#6  0x00692b9b in ExecProcNode (node=0x13c1640) at execProcnode.c:411
#7  0x006bf4d7 in ExecResult (node=0x13c1170) at nodeResult.c:113
#8  0x00692b5c in ExecProcNode (node=0x13c1170) at execProcnode.c:399
#9  0x006a596b in fetch_input_tuple (aggstate=0x13c06a0) at
nodeAgg.c:587
#10 0x006a8530 in agg_fill_hash_table (aggstate=0x13c06a0) at
nodeAgg.c:2272
#11 0x006a7e76 in ExecAgg (node=0x13c06a0) at nodeAgg.c:1910
#12 0x00692d69 in ExecProcNode (node=0x13c06a0) at execProcnode.c:514
#13 0x006c1a42 in ExecSort (node=0x13c03d0) at nodeSort.c:103
#14 0x00692d3f in ExecProcNode (node=0x13c03d0) at execProcnode.c:506
#15 0x0068e733 in ExecutePlan (estate=0x13c01b8,
planstate=0x13c03d0, use_parallel_mode=0 '\000', operation=CMD_SELECT,
sendTuples=1 '\001',
numberTuples=0, direction=ForwardScanDirection, dest=0x7fa368ee1da8)
at execMain.c:1609
#16 0x0068c751 in standard_ExecutorRun (queryDesc=0x135c568,
direction=ForwardScanDirection, count=0) at execMain.c:341
#17 0x0068c5dc in ExecutorRun (queryDesc=0x135c568,


I was running a query whose plan looked like:

explain (costs off) select tableoid::regclass, a, min(b), max(b) from ptab
group by 1,2 order by 1;
  QUERY PLAN
--
 Sort
   Sort Key: ((ptab.tableoid)::regclass)
   ->  HashAggregate
 Group Key: (ptab.tableoid)::regclass, ptab.a
 ->  Result
   ->  Append
 ->  Foreign Scan on ptab_1
 ->  Foreign Scan on ptab_2
 ->  Foreign Scan on ptab_3
 ->  Foreign Scan on ptab_4
 ->  Foreign Scan on ptab_5
 ->  Foreign Scan on ptab_6
 ->  Foreign Scan on ptab_7
 ->  Foreign Scan on ptab_8
 ->  Foreign Scan on ptab_9
 ->  Foreign Scan on ptab_00010
   

The snipped part contains Foreign Scans on 90 more foreign partitions (in
fact, I could see the crash even with 10 foreign table partitions for the
same query).

There is a crash in one more case, which seems related to how WaitEventSet
objects are manipulated during resource-owner-mediated cleanup of a failed
query, such as after the FDW returned an error like below:

ERROR:  relation "public.ptab_00010" does not exist
CONTEXT:  Remote SQL command: SELECT a, b FROM public.ptab_00010

The backtrace in this looks like below:

Program terminated with signal SIGSEGV, Segmentation fault.
#0  0x009c4c35 in ResourceArrayRemove (resarr=0x7f7f7f7f7f7f80bf,
value=20645152) at resowner.c:301
301 lastidx = resarr->lastidx;
(gdb)
(gdb) bt
#0  0x009c4c35 in ResourceArrayRemove (resarr=0x7f7f7f7f7f7f80bf,
value=20645152) at resowner.c:301
#1  0x009c6578 in ResourceOwnerForgetWES
(owner=0x7f7f7f7f7f7f7f7f, events=0x13b0520) at resowner.c:1317
#2  0x00806098 in FreeWaitEventSet (set=0x13b0520) at latch.c:600
#3  0x009c5338 in ResourceOwnerReleaseInternal (owner=0x12de768,
phase=RESOURCE_RELEASE_BEFORE_LOCKS, isCommit=0 '\000', isTopLevel=1 '\001')
at resowner.c:566
#4  0x009c5155 in ResourceOwnerRelease (owner=0x12de768,
phase=RESOURCE_RELEASE_BEFORE_LOCKS, isCommit=0 '\000', isTopLevel=1
'\001') at resowner.c:485
#5  0x00524172 in AbortTransaction () at xact.c:2588
#6  0x00524854 in AbortCurrentTransaction () at xact.c:3016
#7  0x00836aa6 in PostgresMain (argc=1, argv=0x12d7b08,
dbname=0x12d7968 "postgres", username=0x12d7948 "amit") at postgres.c:3860
#8  0x007a49d8 in BackendRun (port=0x12cdf00) at postmaster.c:4310
#9  0x007a4151 in BackendStartup (port=0x12cdf00) at postmaster.c:3982
#10 0x007a0885 in ServerLoop () at postmaster.c:1722
#11 0x0079febf in PostmasterMain (argc=3, argv=0x12aacc0) at
postmaster.c:1330
#12 0x006e7549 in main (argc=3, argv=0x12aacc0) at 

Re: [HACKERS] Possible spelling fixes

[Josh Soref]

diff --git a/src/backend/access/transam/xlog.c 
b/src/backend/access/transam/xlog.c
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -6083,7 +6083,7 @@ StartupXLOG(void)
ereport(LOG,
(errmsg("database system was shut down in 
recovery at %s",
str_time(ControlFile->time;
-   else if (ControlFile->state == DB_SHUTDOWNING)
+   else if (ControlFile->state == DB_SHUTTINGDOWN)
ereport(LOG,
(errmsg("database system shutdown was 
interrupted; last known up at %s",
str_time(ControlFile->time;
@@ -8398,7 +8398,7 @@ CreateCheckPoint(int flags)
if (shutdown)
{
LWLockAcquire(ControlFileLock, LW_EXCLUSIVE);
-   ControlFile->state = DB_SHUTDOWNING;
+   ControlFile->state = DB_SHUTTINGDOWN;
ControlFile->time = (pg_time_t) time(NULL);
UpdateControlFile();
LWLockRelease(ControlFileLock);
diff --git a/src/backend/executor/nodeLockRows.c 
b/src/backend/executor/nodeLockRows.c
--- a/src/backend/executor/nodeLockRows.c
+++ b/src/backend/executor/nodeLockRows.c
@@ -349,7 +349,7 @@ ExecInitLockRows(LockRows *node, EState 
 {
LockRowsState *lrstate;
Plan   *outerPlan = outerPlan(node);
-   List   *epq_arowmarks;
+   List   *epq_arowMarks;
ListCell   *lc;
 
/* check for unsupported flags */
@@ -398,7 +398,7 @@ ExecInitLockRows(LockRows *node, EState 
 * built the global list of ExecRowMarks.)
 */
lrstate->lr_arowMarks = NIL;
-   epq_arowmarks = NIL;
+   epq_arowMarks = NIL;
foreach(lc, node->rowMarks)
{
PlanRowMark *rc = castNode(PlanRowMark, lfirst(lc));
@@ -425,12 +425,12 @@ ExecInitLockRows(LockRows *node, EState 
if (RowMarkRequiresRowShareLock(erm->markType))
lrstate->lr_arowMarks = lappend(lrstate->lr_arowMarks, 
aerm);
else
-   epq_arowmarks = lappend(epq_arowmarks, aerm);
+   epq_arowMarks = lappend(epq_arowMarks, aerm);
}
 
/* Now we have the info needed to set up EPQ state */
EvalPlanQualInit(>lr_epqstate, estate,
-outerPlan, epq_arowmarks, 
node->epqParam);
+outerPlan, epq_arowMarks, 
node->epqParam);
 
return lrstate;
 }
diff --git a/src/backend/executor/nodeModifyTable.c 
b/src/backend/executor/nodeModifyTable.c
--- a/src/backend/executor/nodeModifyTable.c
+++ b/src/backend/executor/nodeModifyTable.c
@@ -1471,7 +1471,7 @@ ExecModifyTable(ModifyTableState *node)
junkfilter = resultRelInfo->ri_junkFilter;
estate->es_result_relation_info = resultRelInfo;
EvalPlanQualSetPlan(>mt_epqstate, 
subplanstate->plan,
-   
node->mt_arowmarks[node->mt_whichplan]);
+   
node->mt_arowMarks[node->mt_whichplan]);
continue;
}
else
@@ -1653,7 +1653,7 @@ ExecInitModifyTable(ModifyTable *node, E
 
mtstate->mt_plans = (PlanState **) palloc0(sizeof(PlanState *) * 
nplans);
mtstate->resultRelInfo = estate->es_result_relations + 
node->resultRelIndex;
-   mtstate->mt_arowmarks = (List **) palloc0(sizeof(List *) * nplans);
+   mtstate->mt_arowMarks = (List **) palloc0(sizeof(List *) * nplans);
mtstate->mt_nplans = nplans;
mtstate->mt_onconflict = node->onConflictAction;
mtstate->mt_arbiterindexes = node->arbiterIndexes;
@@ -1975,7 +1975,7 @@ ExecInitModifyTable(ModifyTable *node, E
 
subplan = mtstate->mt_plans[i]->plan;
aerm = ExecBuildAuxRowMark(erm, subplan->targetlist);
-   mtstate->mt_arowmarks[i] = 
lappend(mtstate->mt_arowmarks[i], aerm);
+   mtstate->mt_arowMarks[i] = 
lappend(mtstate->mt_arowMarks[i], aerm);
}
}
 
@@ -1983,7 +1983,7 @@ ExecInitModifyTable(ModifyTable *node, E
mtstate->mt_whichplan = 0;
subplan = (Plan *) linitial(node->plans);
EvalPlanQualSetPlan(>mt_epqstate, subplan,
-   mtstate->mt_arowmarks[0]);
+   mtstate->mt_arowMarks[0]);
 
/*
 * Initialize the junk filter(s) if needed.  INSERT queries need a 
filter
diff --git a/src/bin/pg_controldata/pg_controldata.c 
b/src/bin/pg_controldata/pg_controldata.c
--- a/src/bin/pg_controldata/pg_controldata.c
+++ 

Re: [HACKERS] Possible spelling fixes

[Josh Soref]

diff --git a/src/test/regress/sql/plpgsql.sql b/src/test/regress/sql/plpgsql.sql
--- a/src/test/regress/sql/plpgsql.sql
+++ b/src/test/regress/sql/plpgsql.sql
@@ -2219,15 +2219,15 @@ drop type eitype cascade;
 -- SQLSTATE and SQLERRM test
 --
 
-create function excpt_test1() returns void as $$
+create function except_test1() returns void as $$
 begin
 raise notice '% %', sqlstate, sqlerrm;
 end; $$ language plpgsql;
 -- should fail: SQLSTATE and SQLERRM are only in defined EXCEPTION
 -- blocks
-select excpt_test1();
-
-create function excpt_test2() returns void as $$
+select except_test1();
+
+create function except_test2() returns void as $$
 begin
 begin
 begin
@@ -2236,9 +2236,9 @@ begin
 end;
 end; $$ language plpgsql;
 -- should fail
-select excpt_test2();
-
-create function excpt_test3() returns void as $$
+select except_test2();
+
+create function except_test3() returns void as $$
 begin
 begin
 raise exception 'user exception';
@@ -2257,19 +2257,19 @@ begin
raise notice '% %', sqlstate, sqlerrm;
 end;
 end; $$ language plpgsql;
-select excpt_test3();
-
-create function excpt_test4() returns text as $$
+select except_test3();
+
+create function except_test4() returns text as $$
 begin
begin perform 1/0;
exception when others then return sqlerrm; end;
 end; $$ language plpgsql;
-select excpt_test4();
-
-drop function excpt_test1();
-drop function excpt_test2();
-drop function excpt_test3();
-drop function excpt_test4();
+select except_test4();
+
+drop function except_test1();
+drop function except_test2();
+drop function except_test3();
+drop function except_test4();
 
 -- parameters of raise stmt can be expressions
 create function raise_exprs() returns void as $$

-- 
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 spelling fixes

[Josh Soref]

diff --git a/src/backend/access/transam/multixact.c 
b/src/backend/access/transam/multixact.c
--- a/src/backend/access/transam/multixact.c
+++ b/src/backend/access/transam/multixact.c
@@ -3342,7 +3342,7 @@ pg_get_multixact_members(PG_FUNCTION_ARG
} mxact;
MultiXactId mxid = PG_GETARG_UINT32(0);
mxact  *multi;
-   FuncCallContext *funccxt;
+   FuncCallContext *funcctx;
 
if (mxid < FirstMultiXactId)
ereport(ERROR,
@@ -3354,8 +3354,8 @@ pg_get_multixact_members(PG_FUNCTION_ARG
MemoryContext oldcxt;
TupleDesc   tupdesc;
 
-   funccxt = SRF_FIRSTCALL_INIT();
-   oldcxt = MemoryContextSwitchTo(funccxt->multi_call_memory_ctx);
+   funcctx = SRF_FIRSTCALL_INIT();
+   oldcxt = MemoryContextSwitchTo(funcctx->multi_call_memory_ctx);
 
multi = palloc(sizeof(mxact));
/* no need to allow for old values here */
@@ -3369,14 +3369,14 @@ pg_get_multixact_members(PG_FUNCTION_ARG
TupleDescInitEntry(tupdesc, (AttrNumber) 2, "mode",
   TEXTOID, -1, 0);
 
-   funccxt->attinmeta = TupleDescGetAttInMetadata(tupdesc);
-   funccxt->user_fctx = multi;
+   funcctx->attinmeta = TupleDescGetAttInMetadata(tupdesc);
+   funcctx->user_fctx = multi;
 
MemoryContextSwitchTo(oldcxt);
}
 
-   funccxt = SRF_PERCALL_SETUP();
-   multi = (mxact *) funccxt->user_fctx;
+   funcctx = SRF_PERCALL_SETUP();
+   multi = (mxact *) funcctx->user_fctx;
 
while (multi->iter < multi->nmembers)
{
@@ -3386,16 +3386,16 @@ pg_get_multixact_members(PG_FUNCTION_ARG
values[0] = psprintf("%u", multi->members[multi->iter].xid);
values[1] = 
mxstatus_to_string(multi->members[multi->iter].status);
 
-   tuple = BuildTupleFromCStrings(funccxt->attinmeta, values);
+   tuple = BuildTupleFromCStrings(funcctx->attinmeta, values);
 
multi->iter++;
pfree(values[0]);
-   SRF_RETURN_NEXT(funccxt, HeapTupleGetDatum(tuple));
+   SRF_RETURN_NEXT(funcctx, HeapTupleGetDatum(tuple));
}
 
if (multi->nmembers > 0)
pfree(multi->members);
pfree(multi);
 
-   SRF_RETURN_DONE(funccxt);
+   SRF_RETURN_DONE(funcctx);
 }
diff --git a/src/backend/access/transam/xlog.c 
b/src/backend/access/transam/xlog.c
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -851,7 +851,7 @@ static bool InstallXLogFileSegment(XLogS
   bool find_free, XLogSegNo max_segno,
   bool use_lock);
 static int XLogFileRead(XLogSegNo segno, int emode, TimeLineID tli,
-int source, bool notexistOk);
+int source, bool notfoundOk);
 static int XLogFileReadAnyTLI(XLogSegNo segno, int emode, int source);
 static int XLogPageRead(XLogReaderState *xlogreader, XLogRecPtr targetPagePtr,
 int reqLen, XLogRecPtr targetRecPtr, char *readBuf,
diff --git a/src/backend/executor/nodeAgg.c b/src/backend/executor/nodeAgg.c
--- a/src/backend/executor/nodeAgg.c
+++ b/src/backend/executor/nodeAgg.c
@@ -486,7 +486,7 @@ static void agg_fill_hash_table(AggState
 static TupleTableSlot *agg_retrieve_hash_table(AggState *aggstate);
 static Datum GetAggInitVal(Datum textInitVal, Oid transtype);
 static void build_pertrans_for_aggref(AggStatePerTrans pertrans,
- AggState *aggsate, EState 
*estate,
+ AggState *aggstate, EState 
*estate,
  Aggref *aggref, Oid 
aggtransfn, Oid aggtranstype,
  Oid aggserialfn, Oid 
aggdeserialfn,
  Datum initValue, bool 
initValueIsNull,
diff --git a/src/backend/replication/walreceiverfuncs.c 
b/src/backend/replication/walreceiverfuncs.c
--- a/src/backend/replication/walreceiverfuncs.c
+++ b/src/backend/replication/walreceiverfuncs.c
@@ -322,7 +322,7 @@ GetReplicationApplyDelay(void)
longsecs;
int usecs;
 
-   TimestampTz chunckReplayStartTime;
+   TimestampTz chunkReplayStartTime;
 
SpinLockAcquire(>mutex);
receivePtr = walrcv->receivedUpto;
@@ -333,12 +333,12 @@ GetReplicationApplyDelay(void)
if (receivePtr == replayPtr)
return 0;
 
-   chunckReplayStartTime = GetCurrentChunkReplayStartTime();
+   chunkReplayStartTime = GetCurrentChunkReplayStartTime();
 
-   if (chunckReplayStartTime == 0)
+   if (chunkReplayStartTime == 0)
return -1;
 
-   

Re: [HACKERS] Possible spelling fixes

[Josh Soref]

diff --git a/contrib/ltree/ltxtquery_io.c b/contrib/ltree/ltxtquery_io.c
--- a/contrib/ltree/ltxtquery_io.c
+++ b/contrib/ltree/ltxtquery_io.c
@@ -96,7 +96,7 @@ gettoken_query(QPRS_STATE *state, int32 
if (*flag)
ereport(ERROR,

(errcode(ERRCODE_SYNTAX_ERROR),
-
errmsg("modificators syntax error")));
+
errmsg("modifiers syntax error")));
*lenval += charlen;
}
else if (charlen == 1 && t_iseq(state->buf, 
'%'))
diff --git a/doc/src/sgml/biblio.sgml b/doc/src/sgml/biblio.sgml
--- a/doc/src/sgml/biblio.sgml
+++ b/doc/src/sgml/biblio.sgml
@@ -295,7 +295,7 @@ ssimk...@ag.or.at
 April, 1990
 
  University  of  California
- Berkely, California
+ Berkeley, California
 


diff --git a/doc/src/sgml/monitoring.sgml b/doc/src/sgml/monitoring.sgml
--- a/doc/src/sgml/monitoring.sgml
+++ b/doc/src/sgml/monitoring.sgml
@@ -1225,7 +1225,7 @@ postgres   27093  0.0  0.0  30096  2752 
 
 
  MessageQueuePutMessage
- Waiting to write a protoocol message to a shared message 
queue.
+ Waiting to write a protocol message to a shared message 
queue.
 
 
  MessageQueueReceive
diff --git a/doc/src/sgml/protocol.sgml b/doc/src/sgml/protocol.sgml
--- a/doc/src/sgml/protocol.sgml
+++ b/doc/src/sgml/protocol.sgml
@@ -5882,7 +5882,7 @@ TupleData
 
 
 
-Idenfifies the data as NULL value.
+Identifies the data as NULL value.
 
 
 
@@ -5895,7 +5895,7 @@ TupleData
 
 
 
-Idenfifies unchanged TOASTed value (the actual value is not
+Identifies unchanged TOASTed value (the actual value is not
 sent).
 
 
@@ -5909,7 +5909,7 @@ TupleData
 
 
 
-Idenfifies the data as text formatted value.
+Identifies the data as text formatted value.
 
 
 
diff --git a/doc/src/sgml/ref/create_subscription.sgml 
b/doc/src/sgml/ref/create_subscription.sgml
--- a/doc/src/sgml/ref/create_subscription.sgml
+++ b/doc/src/sgml/ref/create_subscription.sgml
@@ -131,7 +131,7 @@ CREATE SUBSCRIPTION  option -g
-to specify role membership (Chistopher Browne)
+to specify role membership (Christopher Browne)

   
 
diff --git a/doc/src/sgml/release-9.6.sgml b/doc/src/sgml/release-9.6.sgml
--- a/doc/src/sgml/release-9.6.sgml
+++ b/doc/src/sgml/release-9.6.sgml
@@ -2783,7 +2783,7 @@ 2015-12-30 [e84290823] Avoid useless tru
   
 

 Improve full-text search to support
@@ -4612,7 +4612,7 @@ 2016-02-16 [fc1ae7d2e] Change ecpg lexer
 
   
 

 Add a --strict-names option
@@ -5609,7 +5609,7 @@ 2016-03-16 [f576b17cd] Add word_similari
 
   
 

diff --git a/doc/src/sgml/release-old.sgml b/doc/src/sgml/release-old.sgml
--- a/doc/src/sgml/release-old.sgml
+++ b/doc/src/sgml/release-old.sgml
@@ -5737,8 +5737,8 @@ fix rtree for use in inner scan (Vadim)
 fix gist for use in inner scan, cleanups (Vadim, Andrea)
 avoid unnecessary local buffers allocation (Vadim, Massimo)
 fix local buffers leak in transaction aborts (Vadim)
-fix file manager memmory leaks, cleanups (Vadim, Massimo)
-fix storage manager memmory leaks (Vadim)
+fix file manager memory leaks, cleanups (Vadim, Massimo)
+fix storage manager memory leaks (Vadim)
 fix btree duplicates handling (Vadim)
 fix deleted rows reincarnation caused by vacuum (Vadim)
 fix SELECT varchar()/char() INTO TABLE made zero-length fields(Bruce)
@@ -5904,7 +5904,7 @@ European date format now set when postma
 Execute lowercase function names if not found with exact case
 Fixes for aggregate/GROUP processing, allow 'select sum(func(x),sum(x+y) from 
z'
 Gist now included in the distribution(Marc)
-Idend authentication of local users(Bryan)
+Ident authentication of local users(Bryan)
 Implement BETWEEN qualifier(Bruce)
 Implement IN qualifier(Bruce)
 libpq has PQgetisnull()(Bruce)
diff --git a/src/backend/optimizer/geqo/geqo_erx.c 
b/src/backend/optimizer/geqo/geqo_erx.c
--- a/src/backend/optimizer/geqo/geqo_erx.c
+++ b/src/backend/optimizer/geqo/geqo_erx.c
@@ -458,7 +458,7 @@ edge_failure(PlannerInfo *root, Gene *ge
if (edge_table[i].unused_edges >= 0)
return (Gene) i;
 
-   elog(LOG, "no edge found via looking for the last ununsed 
point");
+   elog(LOG, "no edge found via looking for the last unused 
point");
}
 
 
diff --git a/src/backend/port/dynloader/linux.c 
b/src/backend/port/dynloader/linux.c
--- a/src/backend/port/dynloader/linux.c
+++ b/src/backend/port/dynloader/linux.c
@@ -124,7 +124,7 @@ char *
 

Re: [HACKERS] Add pgstathashindex() to get hash index table statistics.

[Robert Haas]

On Mon, Feb 6, 2017 at 10:40 PM, Amit Kapila  wrote:
>> Maybe we should call them "unused pages".
>
> +1.  If we consider some more names for that column then probably one
> alternative could be "empty pages".

Yeah, but I think "unused" might be better.  Because a page could be
in use (as an overflow page or primary bucket page) and still be
empty.

-- 
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] Add pgstathashindex() to get hash index table statistics.

[Amit Kapila]

On Tue, Feb 7, 2017 at 2:04 AM, Robert Haas  wrote:
> On Sun, Feb 5, 2017 at 11:36 PM, Ashutosh Sharma  
> wrote:
>>> Committed with those changes.
>>
>> Thanks for above corrections and commit. But, There are couple of
>> things that we might have to change once the patch for 'WAL in Hash
>> Indexes' gets checked-in.
>>
>> 1) The test-case result needs to be changed because there won't be any
>> WARNING message : "WARNING:  hash indexes are not WAL-logged and their
>> use is discouraged".
>>
>> 2) From WAL patch for Hash Indexes onwards, we won't have any zero
>> pages in Hash Indexes so I don't think we need to have column showing
>> zero pages (zero_pages). When working on WAL in hash indexes, we found
>> that WAL routine 'XLogReadBufferExtended' does not expect a page to be
>> completely zero page else it returns Invalid Buffer. To fix this, we
>> started initializing freed overflow page and newly allocated bucket
>> pages using _hash_pageinit() hence I don't think there will be any
>> zero pages from here onwards.
>
> Maybe we should call them "unused pages".
>

+1.  If we consider some more names for that column then probably one
alternative could be "empty pages".

-- 
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] Index corruption with CREATE INDEX CONCURRENTLY

[Amit Kapila]

On Mon, Feb 6, 2017 at 10:28 PM, Tom Lane  wrote:
> Amit Kapila  writes:
>> Hmm.  Consider that the first time relcahe invalidation occurs while
>> computing id_attrs, so now the retry logic will compute the correct
>> set of attrs (considering two indexes, if we take the example given by
>> Alvaro above.).  However, the attrs computed for hot_* and key_* will
>> be using only one index, so this will lead to a situation where two of
>> the attrs (hot_attrs and key_attrs) are computed with one index and
>> id_attrs is computed with two indexes. I think this can lead to
>> Assertion in HeapSatisfiesHOTandKeyUpdate().
>
> That seems like something we'd be best off to fix by changing the
> assertion.
>

The above-mentioned problem doesn't exist in your version of the patch
(which is now committed) because the index attrs are cached after
invalidation and I have mentioned the same in my yesterday's followup
mail.   The guarantee of safety is that we don't handle invalidation
between two consecutive calls to RelationGetIndexAttrBitmap() in
heap_update, but if we do handle due to any reason which is not
apparent to me, then I think there is some chance of hitting the
assertion.

>  I doubt it's really going to be practical to guarantee that
> those bitmapsets are always 100% consistent throughout a transaction.
>

Agreed.  As the code stands, I think it is only expected to be
guaranteed across three consecutive calls to
RelationGetIndexAttrBitmap() in heap_update. Now, if the assertion in
HeapSatisfiesHOTandKeyUpdate() is useless and we can remove it, then
probably we don't need a guarantee to be consistent in three
consecutive calls as well.

>> Okay, please find attached patch which is an extension of Tom's and
>> Andres's patch and I think it fixes the above problem noted by me.
>
> I don't like this patch at all.  It only fixes the above issue if the
> relcache inval arrives while RelationGetIndexAttrBitmap is executing.
> If the inval arrives between two such calls, you still have the problem
> of the second one delivering a bitmapset that might be inconsistent
> with the first one.
>

I think it won't happen between two consecutive calls to
RelationGetIndexAttrBitmap in heap_update.  It can happen during
multi-row update, but that should be fine.  I think this version of
patch only defers the creation of fresh bitmapsets where ever
possible.

-- 
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] Index corruption with CREATE INDEX CONCURRENTLY

[Pavan Deolasee]

On Mon, Feb 6, 2017 at 11:54 PM, Tom Lane  wrote:

> After some discussion among the release team, we've concluded that the
> best thing to do is to push Pavan's/my patch into today's releases.
> This does not close the matter by any means: we should continue to
> study whether there are related bugs or whether there's a more principled
> way of fixing this bug.  But that patch clearly makes things better,
> and we shouldn't let worries about whether there are more bugs stop
> us from providing some kind of fix to users.
>
> I've made the push, and barring negative reports from the buildfarm,
> it will be in today's releases.
>

Thank you for taking care of it. Buildfarm is looking green until now.

Thanks,
Pavan

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

Re: [HACKERS] SCRAM authentication, take three

[Michael Paquier]

On Mon, Feb 6, 2017 at 9:55 PM, Heikki Linnakangas  wrote:
> I rebased the SCRAM authentication patches over current master. Here you
> are.

Thanks! Nice to see you around.

> So, if you haven't paid attention on this for a while, now would be a good
> time to have another look at the patch. I believe all the basic
> functionality, documentation, and tests are there, and there are no known
> bugs. Please review! I'll start reading through these myself again tomorrow.

To all: this wiki page is up to date with all the items that remain:
https://wiki.postgresql.org/wiki/SCRAM_authentication
I am keeping the list there up to date with issues noticed on the way.

> One thing that's missing, that we need to address before the release, is the
> use of SASLPrep to "normalize" the password. We discussed that in the
> previous thread, and I think we have a good path forward on it. I'd be happy
> to leave that for a follow-up commit, after these other patches have been
> committed, so we can discuss that work separately.

Yes, I am actively working on this one now. I am trying to come up
first with something in the shape of an extension to begin with, and
get a patch out of it. That will be more simple for testing. For now
the work that really remains in the patches attached on this thread is
to get the internal work done, all the UTF8-related routines being
already present in scram-common.c to work on the strings.
-- 
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] SCRAM authentication, take three

[Michael Paquier]

On Tue, Feb 7, 2017 at 3:12 AM, Aleksander Alekseev
 wrote:
> No, I'm afraid `make distclean` doesn't help. I've re-checked twice.

Hm. I can see the failure on macos and python2 builds as well with the
set of patches applied. And the master branch is working properly.
This needs some investigation.
-- 
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] multivariate statistics (v19)

[Alvaro Herrera]

Still about 0003.  dependencies.c comment at the top of the file should
contain some details about what is it implementing and a general
description of the algorithm and data structures.  As before, it's best
to have the main entry point build_mv_dependencies at the top, the other
public functions, keeping the internal routines at the bottom of the
file.  That eases code study for future readers.  (Minimizing number of
function prototypes is not a goal.)

What is MVSTAT_DEPS_TYPE_BASIC?  Is "functional dependencies" really
BASIC?  I wonder if it should be TYPE_FUNCTIONAL_DEPS or something.

As with pg_ndistinct_out, there's no need to pstrdup(str.data), as it's
already palloc'ed in the right context.

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


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

Re: [HACKERS] asynchronous execution

[Corey Huinker]

On Fri, Feb 3, 2017 at 5:04 AM, Antonin Houska  wrote:

> Kyotaro HORIGUCHI  wrote:
>
> > I noticed that this patch is conflicting with 665d1fa (Logical
> > replication) so I rebased this. Only executor/Makefile
> > conflicted.
>
> I was lucky enough to see an infinite loop when using this patch, which I
> fixed by this change:
>
> diff --git a/src/backend/executor/execAsync.c
> b/src/backend/executor/execAsync.c
> new file mode 100644
> index 588ba18..9b87fbd
> *** a/src/backend/executor/execAsync.c
> --- b/src/backend/executor/execAsync.c
> *** ExecAsyncEventWait(EState *estate, long
> *** 364,369 
> --- 364,370 
>
> if ((w->events & WL_LATCH_SET) != 0)
> {
> +   ResetLatch(MyLatch);
> process_latch_set = true;
> continue;
> }



Hi, I've been testing this patch because seemed like it would help a use
case of mine, but can't tell if it's currently working for cases other than
a local parent table that has many child partitions which happen to be
foreign tables. Is it? I was hoping to use it for a case like:

select x, sum(y) from one_remote_table
union all
select x, sum(y) from another_remote_table
union all
select x, sum(y) from a_third_remote_table


but while aggregates do appear to be pushed down, it seems that the remote
tables are being queried in sequence. Am I doing something wrong?

Re: [HACKERS] IF [NOT] EXISTS for replication slots

[Michael Paquier]

On Tue, Feb 7, 2017 at 2:07 AM, Petr Jelinek
 wrote:
> On 06/02/17 05:15, Michael Paquier wrote:
>> Hi all,
>>
>> Lately I have bumped into a case where it would have been useful to
>> make the difference between a failure because of a slot already
>> dropped and an internal failure of Postgres. Is there any interest for
>> support of IE and INE for CREATE and DROP_REPLICATION_SLOT?
>> My use case involved only the SQL-callable interface, but I would
>> think that it is useful to make this difference as well with the
>> replication protocol. For the function we could just add a boolean
>> argument to control the switch, and for the replication commands a
>> dedicated keyword.
>>
>> Any thoughts?
>
> My thought is, how would this handle the snapshot creation that logical
> slot does when it's created?

In what is that related to IF NOT EXISTS? If the slot does not get
created you could just return NULL to the caller to let him know that
there is something already around. The use-case I have found INE
useful involved physical slots actually.
-- 
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: \if, \elseif, \else, \endif (was Re: [HACKERS] PSQL commands: \quit_if, \quit_unless)

[Corey Huinker]

On Mon, Feb 6, 2017 at 3:43 PM, Corey Huinker 
wrote:

> I did some more tests. I found a subtlety that I missed before: when
>> running under ON_ERROR_STOP, messages are not fully consistent:
>>
>>  sh> cat test.sql
>>  \set ON_ERROR_STOP on
>>  \if error
>>\echo NO
>>  \endif
>>  \echo NO
>>
>>  sh> ./psql < test.sql
>>  SET
>>  # ok
>>  unrecognized value "error" for "\if ": boolean expected
>>  # ok
>
>
> That's straight from ParseVariableBool, and we can keep that or suppress
> it. Whatever we do, we should do it with the notion that more complex
> expressions will eventually be allowed, but they'll still have to resolve
> to something that's a text boolean.
>
>
>>
>>  new \if is invalid, ignoring commands until next \endif
>>  # hmmm... but it does not, it is really stopping immediately...
>
>  found EOF before closing \endif(s)
>>  # no, it has just stopped before EOF because of the error...
>>
>
> Yeah, chattiness caught up to us here. Both of these messages can be
> suppressed, I think.
>
>
>>
>> Also I'm not quite sure why psql decided that it is in interactive mode
>> above, its stdin is a file, but why not.
>>
>> The issue is made more explicit with -f:
>>
>>  sh> ./psql -f test.sql
>>  SET
>>  psql:test.sql:2: unrecognized value "error" for "\if ": boolean
>> expected
>>  psql:test.sql:2: new \if is invalid, ignoring commands until next \endif
>>  psql:test.sql:2: found EOF before closing \endif(s)
>>
>> It stopped on line 2, which is expected, but it was not on EOF.
>>
>> I think that the message when stopping should be ", stopping as required
>> by ON_ERROR_STOP" or something like that instead of ", ignoring...", and
>> the EOF message should not be printed at all in this case.
>
>
> I agree, and will look into making that happen. Thanks for the test case.
>
>

I suppressed the endif-balance checking in cases where we're in an
already-failed situation.
In cases where there was a boolean parsing failure, and ON_ERROR_STOP is
on, the error message no longer speak of a future which the session does
not have. I could left the ParseVariableBool() message as the only one, but
wasn't sure that that was enough of an error message on its own.
Added the test case to the existing tap tests. Incidentally, the tap tests
aren't presently fooled into thinking they're interactive.

$ cat test2.sql
\if error
\echo NO
\endif
\echo NOPE
$ psql test < test2.sql -v ON_ERROR_STOP=0
unrecognized value "error" for "\if ": boolean expected
new \if is invalid, ignoring commands until next \endif
NOPE
$ psql test < test2.sql -v ON_ERROR_STOP=1
unrecognized value "error" for "\if ": boolean expected
new \if is invalid.
$ psql test -f test2.sql -v ON_ERROR_STOP=0
psql:test2.sql:1: unrecognized value "error" for "\if ": boolean
expected
psql:test2.sql:1: new \if is invalid, ignoring commands until next \endif
NOPE
$ psql test -f test2.sql -v ON_ERROR_STOP=1
psql:test2.sql:1: unrecognized value "error" for "\if ": boolean
expected
psql:test2.sql:1: new \if is invalid.


Revised cumulative patch attached for those playing along at home.
diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml
index ae58708..c0ba4c4 100644
--- a/doc/src/sgml/ref/psql-ref.sgml
+++ b/doc/src/sgml/ref/psql-ref.sgml
@@ -2035,6 +2035,67 @@ hello 10
 
   
 
+  
+\if expr
+\elif expr
+\else
+\endif
+
+
+This group of commands implements nestable conditional blocks, like
+this:
+
+
+SELECT
+EXISTS(SELECT 1 FROM customer WHERE customer_id = 123) as is_customer,
+EXISTS(SELECT 1 FROM employee WHERE employee_id = 456) as is_employee
+\gset
+\if :is_customer
+SELECT * FROM customer WHERE customer_id = 123;
+\elif :is_employee
+\echo 'is not a customer but is an employee'
+SELECT * FROM employee WHERE employee_id = 456;
+\else
+\if yes
+\echo 'not a customer or employee'
+\else
+\echo 'this should never print'
+\endif
+\endif
+
+
+Conditional blocks must begin with a \if and end
+with an \endif, and the pairs must be found in
+the same source file. If an EOF is reached on the main file or an
+\include-ed file before all local
+\if-\endif are matched, then
+psql will raise an error.
+
+
+A conditional block can have any number of 
+\elif clauses, which may optionally be followed by a
+single \else clause.
+
+
+The \if and \elif commands
+read the rest of the line and evaluate it as a boolean expression.
+Currently, expressions are limited to a single unquoted string
+which are evaluated like other on/off options, so the valid values
+are any unambiguous case insensitive matches for one of:
+true, false, 
1,
+0, on, off,
+yes, no.  So 
+t, T, and tR
+will all match true.
+

Re: [HACKERS] multivariate statistics (v19)

[Alvaro Herrera]

Looking at 0003, I notice that gram.y is changed to add a WITH ( .. )
clause.  If it's not specified, an error is raised.  If you create
stats with (ndistinct) then you can't alter it later to add
"dependencies" or whatever; unless I misunderstand, you have to drop the
statistics and create another one.  Probably in a forthcoming patch we
should have ALTER support to add a stats type.

Also, why isn't the default to build everything, rather than nothing?

BTW, almost everything in the backend could be inside "utils/", so let's
not do that -- let's just create src/backend/statistics/ for all your
code.

Here a few notes while reading README.dependencies -- some typos, two
questions.

diff --git a/src/backend/utils/mvstats/README.dependencies 
b/src/backend/utils/mvstats/README.dependencies
index 908f094..7f3ed3d 100644
--- a/src/backend/utils/mvstats/README.dependencies
+++ b/src/backend/utils/mvstats/README.dependencies
@@ -36,7 +36,7 @@ design choice to model the dataset in denormalized way, 
either because of
 performance or to make querying easier.
 
 
-soft dependencies
+Soft dependencies
 -
 
 Real-world data sets often contain data errors, either because of data entry
@@ -48,7 +48,7 @@ rendering the approach mostly useless even for slightly noisy 
data sets, or
 result in sudden changes in behavior depending on minor differences between
 samples provided to ANALYZE.
 
-For this reason the statistics implementes "soft" functional dependencies,
+For this reason the statistics implements "soft" functional dependencies,
 associating each functional dependency with a degree of validity (a number
 number between 0 and 1). This degree is then used to combine selectivities
 in a smooth manner.
@@ -75,6 +75,7 @@ The algorithm also requires a minimum size of the group to 
consider it
 consistent (currently 3 rows in the sample). Small groups make it less likely
 to break the consistency.
 
+## What is it that we store in the catalog?
 
 Clause reduction (planner/optimizer)
 
@@ -95,12 +96,12 @@ example for (a,b,c) we first use (a,b=>c) to break the 
computation into
 and then apply (a=>b) the same way on P(a=?,b=?).
 
 
-Consistecy of clauses
+Consistency of clauses
 -
 
 Functional dependencies only express general dependencies between columns,
 without referencing particular values. This assumes that the equality clauses
-are in fact consistent with the functinal dependency, i.e. that given a
+are in fact consistent with the functional dependency, i.e. that given a
 dependency (a=>b), the value in (b=?) clause is the value determined by (a=?).
 If that's not the case, the clauses are "inconsistent" with the functional
 dependency and the result will be over-estimation.
@@ -111,6 +112,7 @@ set will be empty, but we'll estimate the selectivity using 
the ZIP condition.
 
 In this case the default estimation based on AVIA principle happens to work
 better, but mostly by chance.
+## what is AVIA principle?
 
 This issue is the price for the simplicity of functional dependencies. If the
 application frequently constructs queries with clauses inconsistent with

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


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

Re: [HACKERS] Provide list of subscriptions and publications in psql's completion

[Peter Eisentraut]

On 2/6/17 10:54 AM, Fujii Masao wrote:
> Yes, that's an option. And, if we add dbid to pg_stat_subscription,
> I'm tempted to add all the pg_subscription's columns except subconninfo
> into pg_stat_subscription. Since subconninfo may contain security-sensitive
> information, it should be excluded. But it seems useful to expose other
> columns. Then, if we expose all the columns except subconninfo, maybe
> it's better to just revoke subconninfo column on pg_subscription instead of
> all columns. Thought?

I think previous practice is to provide a view such as pg_subscriptions
that contains all the nonprivileged information.

-- 
Peter Eisentraut  http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


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

Re: [HACKERS] Provide list of subscriptions and publications in psql's completion

[Peter Eisentraut]

On 2/5/17 11:52 PM, Michael Paquier wrote:
>> In "CREATE SUBSCRIPTION ... PUBLICATION" and "ALTER SUBSCRIPTION ... SET
>> PUBLICATION" cases, the publication defined in the publisher side should be
>> specified. But, with this patch, the tab-completion for PUBLICATION shows
>> the publications defined in local server (ie, subscriber side in those 
>> cases).
>> This might be confusing.
> Which would mean that psql tab completion should try to connect the
> remote server using the connection string defined with the
> subscription to get this information, which looks unsafe to me. To be
> honest, I'd rather see a list of local objects defined than nothing..

But it's the wrong list.

-- 
Peter Eisentraut  http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


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

Re: [HACKERS] Enabling replication connections by default in pg_hba.conf

[Peter Eisentraut]

On 2/2/17 2:59 PM, Simon Riggs wrote:
> We currently have REPLICATION to mean "physical replication".

Well, it doesn't really mean that.  The same facilities are used to
control access to both logical and physical replication.

> I think if we have another capability for logical replication then we
> would be able to easily allow one or the other, or both, so I don't
> see a problem here that forces us to keep pg_hba.conf the way it is.

I think change is possible and possibly desirable.  One issue is that we
might need to adjust dump/restore so that it keeps existing usages
working.  Details depend on the actual changes, of course.

-- 
Peter Eisentraut  http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


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

Re: [HACKERS] multivariate statistics (v19)

[Alvaro Herrera]

Tomas Vondra wrote:
> On 02/01/2017 11:52 PM, Alvaro Herrera wrote:

> > Nearby, some auxiliary functions such as n_choose_k and
> > num_combinations are not documented. What it is that they do? I'd
> > move these at the end of the file, keeping the important entry points
> > at the top of the file.
> 
> I'd say n-choose-k is pretty widely known term from combinatorics. The
> comment would essentially say just 'this is n-choose-k' which seems rather
> pointless. So as much as I dislike the self-documenting code, this actually
> seems like a good case of that.

Actually, we do have such comments all over the place.  I knew this as
"n sobre k", so the english name doesn't immediately ring a bell with me
until I look it up; I think the function comment could just say
"n_choose_k -- this function returns the binomial coefficient".

> > I see this patch has a estimate_ndistinct() which claims to be a re-
> > implementation of code already in analyze.c, but it is actually a lot
> > simpler than what analyze.c does.  I've been wondering if it'd be a good
> > idea to use some of this code so that some routines are moved out of
> > analyze.c; good implementations of statistics-related functions would
> > live in src/backend/statistics/ where they can be used both by analyze.c
> > and your new mvstats stuff.  (More generally I am beginning to wonder if
> > the new directory should be just src/backend/statistics.)
> 
> I'll look into that. I have to check if I ignored some assumptions or corner
> cases the analyze.c deals with.

Maybe it's not terribly important to refactor analyze.c from the get go,
but let's give the subdir a more general name.  Hence my vote for having
the subdir be "statistics" instead of "mvstats".

> > In another subthread you seem to have surrendered to the opinion that
> > the new catalog should be called pg_statistics_ext, just in case in the
> > future we come up with additional things to put on it.  However, given
> > its schema, with a "starelid / stakeys", is it sensible to think that
> > we're going to get anything other than something that involves multiple
> > variables?  Maybe it should just be "pg_statistics_multivar" and if
> > something else comes along we create another catalog with an appropriate
> > schema.  Heck, how does this catalog serve the purpose of cross-table
> > statistics in the first place, given that it has room to record a single
> > relid only?  Are you thinking that in the future you'd change starelid
> > into an oidvector column?
> 
> Yes, I think the starelid will turn into OID vector. The reason why I
> haven't done that in the current version of the catalog is to keep it
> simple.

OK -- as long as we know what the way forward is, I'm good.  Still, my
main point was that even if we have multiple rels, this catalog will be
about having multivariate statistics, and not different kinds of
statistical data.  I would keep pg_mv_statistics, really.

> > The comment in gram.y about the CREATE STATISTICS is at odds with what
> > is actually allowed by the grammar.
> 
> Which comment?

This one:
 *  CREATE STATISTICS stats_name ON relname (columns) WITH (options)
the production actually says:
  CREATE STATISTICS any_name ON '(' columnList ')' FROM qualified_name

> > I think the name of a statistics is only useful to DROP/ALTER it, right?
> > I wonder why it's useful that statistics belongs in a schema.  Perhaps
> > it should be a global object?  I suppose the name collisions would
> > become bothersome if you have many mvstats.
> 
> I think it shouldn't be a global object. I consider them to be a part of a
> schema (just like indexes, for example). Imagine you have a multi-tenant
> database, with using exactly the same (tables/indexes) schema, but keept in
> different schemas. Why shouldn't it be possible to also use the same set of
> statistics for each tenant?

True.  Suggestion withdrawn.

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


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

Re: [HACKERS] Active zombies at AIX

[Andres Freund]

On 2017-02-06 16:06:25 -0500, Tom Lane wrote:
> Andres Freund  writes:
> > On 2017-02-06 15:39:10 -0500, Peter Eisentraut wrote:
> >> On 2/6/17 6:28 AM, Konstantin Knizhnik wrote:
> >>> I wonder why do we prohibit now configuration of Postgres without mmap?
> 
> >> It's not really prohibited, but it's not something that people generally
> >> need, and we want to keep the number of configuration variations low.
> 
> > I think that was a fairly bad call. Making it hard to use anything but
> > mmap (on mmap supporting platforms) caused a fair bit of trouble and
> > performance regressions on several platforms by now (freebsd reported it
> > fairly quickly, and now aix), all to avoid a trivial amount of code and
> > one guc.
> 
> > FWIW, there's a patch somewhere in the archive making it configurable.
> 
> Clearly we should do something, but I'm not sure that a GUC is the right
> answer; far too few people would set it correctly.  I think it might be
> better to have the per-platform "template" files decide whether to set
> USE_ANONYMOUS_SHMEM or not.

Well, sysv shmem will be less "comfortable" to use on those platforms
too. And you'll usually only hit the performance problems on bigger
installations. I don't think it'll be an improvement if after an upgrade
postgres doesn't work anymore because people have gotten used to not
having to configure sys shmem.

I suspect a better solution would be to have a list GUC with a platform
dependant default (i.e. sysv, anonymous on freebsd/aix; the other way
round on linux). At startup we'd then try those in order.

Regards,

Andres


-- 
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] Active zombies at AIX

[Tom Lane]

Andres Freund  writes:
> On 2017-02-06 15:39:10 -0500, Peter Eisentraut wrote:
>> On 2/6/17 6:28 AM, Konstantin Knizhnik wrote:
>>> I wonder why do we prohibit now configuration of Postgres without mmap?

>> It's not really prohibited, but it's not something that people generally
>> need, and we want to keep the number of configuration variations low.

> I think that was a fairly bad call. Making it hard to use anything but
> mmap (on mmap supporting platforms) caused a fair bit of trouble and
> performance regressions on several platforms by now (freebsd reported it
> fairly quickly, and now aix), all to avoid a trivial amount of code and
> one guc.

> FWIW, there's a patch somewhere in the archive making it configurable.

Clearly we should do something, but I'm not sure that a GUC is the right
answer; far too few people would set it correctly.  I think it might be
better to have the per-platform "template" files decide whether to set
USE_ANONYMOUS_SHMEM or not.

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] proposal: session server side variables

[Pavel Stehule]

2017-02-06 21:36 GMT+01:00 Fabien COELHO :

>
> Hello,
>
> I'll work on my proposal in v11 time. Maybe in this time Postgres will
>> support autonomous transactions.
>>
>
> Maybe.
>
> The variables syntax should be better integrated to core - it should be
>> implemented without getter/setter functions.
>>
>
> Yes, a nicer syntax would be great.
>
> Note that setter/getter could be useful for some use case, eg with queries
> built dynamically?


There is not any problem for usage in dynamic sql. Some generic access  is
done already.


>
>
> I am not sure If statement SET can be enhanced to allows the work with
>> session variables without some conflicts, but we will see.
>>
>
> If so, maybe some kind of prefix could provide a workaround.


any other database objects has not prefix. But we can identify a ambiguous
situation and in this case we can require qualified identifier.

Regards

Pavel


>
>
> --
> Fabien.
>

Re: [HACKERS] LWLock optimization for multicore Power machines

[Bernd Helmle]

On Mon, 2017-02-06 at 22:44 +0300, Alexander Korotkov wrote:
> Results looks strange for me.  I wonder why there is difference
> between
> lwlock-power-1.patch and lwlock-power-3.patch?  From my intuition, it
> shouldn't be there because it's not much difference between them. 
> Thus, I
> have following questions.
> 
> 

Yeah, i've realized that as well.

>    1. Have you warm up database?  I.e. could you do "SELECT sum(x.x)
> FROM
>    (SELECT pg_prewarm(oid) AS x FROM pg_class WHERE relkind IN ('i',
> 'r')
>    ORDER BY oid) x;" before each run?
>    2. Also could you run each test longer: 3-5 mins, and run them
> with
>    variety of clients count?

The results i've posted were the last 3 run of 9 in summary. I hoped
that should be enough to prewarm the system. I'm going to repeat the
tests with the changes you've requested, though.



-- 
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] Active zombies at AIX

[Andres Freund]

On 2017-02-06 15:39:10 -0500, Peter Eisentraut wrote:
> On 2/6/17 6:28 AM, Konstantin Knizhnik wrote:
> > I wonder why do we prohibit now configuration of Postgres without mmap?
> 
> It's not really prohibited, but it's not something that people generally
> need, and we want to keep the number of configuration variations low.

I think that was a fairly bad call. Making it hard to use anything but
mmap (on mmap supporting platforms) caused a fair bit of trouble and
performance regressions on several platforms by now (freebsd reported it
fairly quickly, and now aix), all to avoid a trivial amount of code and
one guc.

FWIW, there's a patch somewhere in the archive making it configurable.

- Andres


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

Re: \if, \elseif, \else, \endif (was Re: [HACKERS] PSQL commands: \quit_if, \quit_unless)

[Corey Huinker]

>
> I did some more tests. I found a subtlety that I missed before: when
> running under ON_ERROR_STOP, messages are not fully consistent:
>
>  sh> cat test.sql
>  \set ON_ERROR_STOP on
>  \if error
>\echo NO
>  \endif
>  \echo NO
>
>  sh> ./psql < test.sql
>  SET
>  # ok
>  unrecognized value "error" for "\if ": boolean expected
>  # ok


That's straight from ParseVariableBool, and we can keep that or suppress
it. Whatever we do, we should do it with the notion that more complex
expressions will eventually be allowed, but they'll still have to resolve
to something that's a text boolean.


>
>  new \if is invalid, ignoring commands until next \endif
>  # hmmm... but it does not, it is really stopping immediately...

 found EOF before closing \endif(s)
>  # no, it has just stopped before EOF because of the error...
>

Yeah, chattiness caught up to us here. Both of these messages can be
suppressed, I think.


>
> Also I'm not quite sure why psql decided that it is in interactive mode
> above, its stdin is a file, but why not.
>
> The issue is made more explicit with -f:
>
>  sh> ./psql -f test.sql
>  SET
>  psql:test.sql:2: unrecognized value "error" for "\if ": boolean
> expected
>  psql:test.sql:2: new \if is invalid, ignoring commands until next \endif
>  psql:test.sql:2: found EOF before closing \endif(s)
>
> It stopped on line 2, which is expected, but it was not on EOF.
>
> I think that the message when stopping should be ", stopping as required
> by ON_ERROR_STOP" or something like that instead of ", ignoring...", and
> the EOF message should not be printed at all in this case.


I agree, and will look into making that happen. Thanks for the test case.

Re: [HACKERS] Active zombies at AIX

[Peter Eisentraut]

On 2/6/17 6:28 AM, Konstantin Knizhnik wrote:
> I wonder why do we prohibit now configuration of Postgres without mmap?

It's not really prohibited, but it's not something that people generally
need, and we want to keep the number of configuration variations low.

-- 
Peter Eisentraut  http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


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

Re: [HACKERS] proposal: session server side variables

[Fabien COELHO]

Hello,

I'll work on my proposal in v11 time. Maybe in this time Postgres will 
support autonomous transactions.


Maybe.


The variables syntax should be better integrated to core - it should be
implemented without getter/setter functions.


Yes, a nicer syntax would be great.

Note that setter/getter could be useful for some use case, eg with queries 
built dynamically?


I am not sure If statement SET can be enhanced to allows the work with 
session variables without some conflicts, but we will see.


If so, maybe some kind of prefix could provide a workaround.

--
Fabien.


--
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] Add pgstathashindex() to get hash index table statistics.

[Robert Haas]

On Sun, Feb 5, 2017 at 11:36 PM, Ashutosh Sharma  wrote:
>> Committed with those changes.
>
> Thanks for above corrections and commit. But, There are couple of
> things that we might have to change once the patch for 'WAL in Hash
> Indexes' gets checked-in.
>
> 1) The test-case result needs to be changed because there won't be any
> WARNING message : "WARNING:  hash indexes are not WAL-logged and their
> use is discouraged".
>
> 2) From WAL patch for Hash Indexes onwards, we won't have any zero
> pages in Hash Indexes so I don't think we need to have column showing
> zero pages (zero_pages). When working on WAL in hash indexes, we found
> that WAL routine 'XLogReadBufferExtended' does not expect a page to be
> completely zero page else it returns Invalid Buffer. To fix this, we
> started initializing freed overflow page and newly allocated bucket
> pages using _hash_pageinit() hence I don't think there will be any
> zero pages from here onwards.

Maybe we should call them "unused pages".  Those will presumably still exist.

-- 
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] pg_sequences bug ?

[Peter Eisentraut]

On 2/4/17 7:30 AM, Michael Paquier wrote:
> We could perhaps just use has_sequence_privilege() and return NULL if
> the caller of pg_sequences does not have select and usage access to a
> given sequence? Please see the patch attached.

Committed with documentation updates.  Thanks.

-- 
Peter Eisentraut  http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


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

Re: \if, \elseif, \else, \endif (was Re: [HACKERS] PSQL commands: \quit_if, \quit_unless)

[Fabien COELHO]

Consolidated Fabien's TAP test additions with v7, in case anyone else wants
to be reviewing.


Patch applies (with "patch"), make check ok, psql tap test ok.

I did some more tests. I found a subtlety that I missed before: when 
running under ON_ERROR_STOP, messages are not fully consistent:


 sh> cat test.sql
 \set ON_ERROR_STOP on
 \if error
   \echo NO
 \endif
 \echo NO

 sh> ./psql < test.sql
 SET
 # ok
 unrecognized value "error" for "\if ": boolean expected
 # ok
 new \if is invalid, ignoring commands until next \endif
 # hmmm... but it does not, it is really stopping immediately...
 found EOF before closing \endif(s)
 # no, it has just stopped before EOF because of the error...

Also I'm not quite sure why psql decided that it is in interactive mode 
above, its stdin is a file, but why not.


The issue is made more explicit with -f:

 sh> ./psql -f test.sql
 SET
 psql:test.sql:2: unrecognized value "error" for "\if ": boolean expected
 psql:test.sql:2: new \if is invalid, ignoring commands until next \endif
 psql:test.sql:2: found EOF before closing \endif(s)

It stopped on line 2, which is expected, but it was not on EOF.

I think that the message when stopping should be ", stopping as required 
by ON_ERROR_STOP" or something like that instead of ", ignoring...", and 
the EOF message should not be printed at all in this case.


--
Fabien.


--
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] One-shot expanded output in psql using \gx

[Christoph Berg]

The majority of voices here was in favor of using \gx, so here is
another version of the same patch which implements that.

Christoph
diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml
new file mode 100644
index ae58708..e0302ea
*** a/doc/src/sgml/ref/psql-ref.sgml
--- b/doc/src/sgml/ref/psql-ref.sgml
*** Tue Oct 26 21:40:57 CEST 1999
*** 1891,1896 
--- 1891,1908 
  
  

+ \gx [ filename ]
+ \gx [ |command ]
+ 
+ 
+ \gx is equivalent to \g, but
+ forces expanded output mode for this query.
+ 
+ 
+   
+ 
+ 
+   
  \gexec
  
  
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
new file mode 100644
index f17f610..6e140fe
*** a/src/bin/psql/command.c
--- b/src/bin/psql/command.c
*** exec_command(const char *cmd,
*** 910,917 
  		free(fname);
  	}
  
! 	/* \g [filename] -- send query, optionally with output to file/pipe */
! 	else if (strcmp(cmd, "g") == 0)
  	{
  		char	   *fname = psql_scan_slash_option(scan_state,
     OT_FILEPIPE, NULL, false);
--- 910,920 
  		free(fname);
  	}
  
! 	/*
! 	 * \g [filename] -- send query, optionally with output to file/pipe
! 	 * \gx [filename] -- same as \g, with expanded mode forced
! 	 */
! 	else if (strcmp(cmd, "g") == 0 || strcmp(cmd, "gx") == 0)
  	{
  		char	   *fname = psql_scan_slash_option(scan_state,
     OT_FILEPIPE, NULL, false);
*** exec_command(const char *cmd,
*** 924,929 
--- 927,934 
  			pset.gfname = pg_strdup(fname);
  		}
  		free(fname);
+ 		if (strcmp(cmd, "gx") == 0)
+ 			pset.g_expanded = true;
  		status = PSQL_CMD_SEND;
  	}
  
diff --git a/src/bin/psql/common.c b/src/bin/psql/common.c
new file mode 100644
index 5349c39..4534bd9
*** a/src/bin/psql/common.c
--- b/src/bin/psql/common.c
*** PrintQueryTuples(const PGresult *results
*** 770,775 
--- 770,779 
  {
  	printQueryOpt my_popt = pset.popt;
  
+ 	/* one-shot expanded output requested via \gx */
+ 	if (pset.g_expanded)
+ 		my_popt.topt.expanded = true;
+ 
  	/* write output to \g argument, if any */
  	if (pset.gfname)
  	{
*** sendquery_cleanup:
*** 1410,1415 
--- 1414,1422 
  		pset.gfname = NULL;
  	}
  
+ 	/* reset \gx's expanded-mode flag */
+ 	pset.g_expanded = false;
+ 
  	/* reset \gset trigger */
  	if (pset.gset_prefix)
  	{
diff --git a/src/bin/psql/help.c b/src/bin/psql/help.c
new file mode 100644
index 3e3cab4..2aece7e
*** a/src/bin/psql/help.c
--- b/src/bin/psql/help.c
*** slashUsage(unsigned short int pager)
*** 174,179 
--- 174,180 
  	fprintf(output, _("  \\copyright show PostgreSQL usage and distribution terms\n"));
  	fprintf(output, _("  \\errverboseshow most recent error message at maximum verbosity\n"));
  	fprintf(output, _("  \\g [FILE] or ; execute query (and send results to file or |pipe)\n"));
+ 	fprintf(output, _("  \\gx [FILE] as \\g, but force expanded output\n"));
  	fprintf(output, _("  \\gexec execute query, then execute each value in its result\n"));
  	fprintf(output, _("  \\gset [PREFIX] execute query and store results in psql variables\n"));
  	fprintf(output, _("  \\q quit psql\n"));
diff --git a/src/bin/psql/settings.h b/src/bin/psql/settings.h
new file mode 100644
index 195f5a1..70ff181
*** a/src/bin/psql/settings.h
--- b/src/bin/psql/settings.h
*** typedef struct _psqlSettings
*** 91,96 
--- 91,97 
  	printQueryOpt popt;
  
  	char	   *gfname;			/* one-shot file output argument for \g */
+ 	bool		g_expanded;		/* one-shot expanded output requested via \gx */
  	char	   *gset_prefix;	/* one-shot prefix argument for \gset */
  	bool		gexec_flag;		/* one-shot flag to execute query's results */
  	bool		crosstab_flag;	/* one-shot request to crosstab results */
diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
new file mode 100644
index 6e759d0..0bd2ae3
*** a/src/bin/psql/tab-complete.c
--- b/src/bin/psql/tab-complete.c
*** psql_completion(const char *text, int st
*** 1338,1344 
  		"\\dm", "\\dn", "\\do", "\\dO", "\\dp", "\\drds", "\\ds", "\\dS",
  		"\\dt", "\\dT", "\\dv", "\\du", "\\dx", "\\dy",
  		"\\e", "\\echo", "\\ef", "\\encoding", "\\errverbose", "\\ev",
! 		"\\f", "\\g", "\\gexec", "\\gset", "\\h", "\\help", "\\H", "\\i", "\\ir", "\\l",
  		"\\lo_import", "\\lo_export", "\\lo_list", "\\lo_unlink",
  		"\\o", "\\p", "\\password", "\\prompt", "\\pset", "\\q", "\\qecho", "\\r",
  		"\\s", "\\set", "\\setenv", "\\sf", "\\sv", "\\t", "\\T",
--- 1338,1344 
  		"\\dm", "\\dn", "\\do", "\\dO", "\\dp", "\\drds", "\\ds", "\\dS",
  		"\\dt", "\\dT", "\\dv", "\\du", "\\dx", "\\dy",
  		"\\e", "\\echo", "\\ef", "\\encoding", "\\errverbose", "\\ev",
! 		"\\f", "\\g", "\\gx", "\\gexec", "\\gset", "\\h", "\\help", "\\H", "\\i", "\\ir", "\\l",
  	

Re: [HACKERS] LWLock optimization for multicore Power machines

[Andres Freund]

Hi,

On 2017-02-03 20:01:03 +0300, Alexander Korotkov wrote:
> Using assembly in lwlock.c looks rough.  This is why I refactored it by
> introducing new atomic operation pg_atomic_fetch_mask_add_u32 (see
> lwlock-power-2.patch).  It checks that all masked bits are clear and then
> adds to variable.  This atomic have special assembly implementation for
> Power, and generic implementation for other platforms with loop of CAS.
> Probably we would have other implementations for other architectures in
> future.  This level of abstraction is the best I managed to invent.

I think that's a reasonable approach.  And I think it might be worth
experimenting with a more efficient implementation on x86 too, using
hardware lock elision / HLE and/or tsx.

Andres


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

Re: \if, \elseif, \else, \endif (was Re: [HACKERS] PSQL commands: \quit_if, \quit_unless)

[Corey Huinker]

On Mon, Feb 6, 2017 at 2:32 PM, Fabien COELHO  wrote:

>
> Do you think the TAP tests would benefit from having the input described
>> in a q(...) block rather than a string?
>>
>
> My 0.02€: Not really, so I would not bother. It breaks perl indentation
> and logic for a limited benefit. Maybe add comments if you feel that a test
> case is unclear.
>
> --
> Fabien.


Consolidated Fabien's TAP test additions with v7, in case anyone else wants
to be reviewing.
diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml
index ae58708..c0ba4c4 100644
--- a/doc/src/sgml/ref/psql-ref.sgml
+++ b/doc/src/sgml/ref/psql-ref.sgml
@@ -2035,6 +2035,67 @@ hello 10
 
   
 
+  
+\if expr
+\elif expr
+\else
+\endif
+
+
+This group of commands implements nestable conditional blocks, like
+this:
+
+
+SELECT
+EXISTS(SELECT 1 FROM customer WHERE customer_id = 123) as is_customer,
+EXISTS(SELECT 1 FROM employee WHERE employee_id = 456) as is_employee
+\gset
+\if :is_customer
+SELECT * FROM customer WHERE customer_id = 123;
+\elif :is_employee
+\echo 'is not a customer but is an employee'
+SELECT * FROM employee WHERE employee_id = 456;
+\else
+\if yes
+\echo 'not a customer or employee'
+\else
+\echo 'this should never print'
+\endif
+\endif
+
+
+Conditional blocks must begin with a \if and end
+with an \endif, and the pairs must be found in
+the same source file. If an EOF is reached on the main file or an
+\include-ed file before all local
+\if-\endif are matched, then
+psql will raise an error.
+
+
+A conditional block can have any number of 
+\elif clauses, which may optionally be followed by a
+single \else clause.
+
+
+The \if and \elif commands
+read the rest of the line and evaluate it as a boolean expression.
+Currently, expressions are limited to a single unquoted string
+which are evaluated like other on/off options, so the valid values
+are any unambiguous case insensitive matches for one of:
+true, false, 
1,
+0, on, off,
+yes, no.  So 
+t, T, and tR
+will all match true.
+
+
+Lines within false branches are not evaluated in any way: queries are
+not sent to the server, non-conditional commands are not evaluated but
+bluntly ignored, nested if-expressions in such branches are also not
+evaluated but are tallied to check for proper nesting.
+
+
+  
 
   
 \ir or \include_relative 
filename
diff --git a/src/bin/psql/Makefile b/src/bin/psql/Makefile
index c53733f..7a418c6 100644
--- a/src/bin/psql/Makefile
+++ b/src/bin/psql/Makefile
@@ -61,8 +61,16 @@ uninstall:
 
 clean distclean:
rm -f psql$(X) $(OBJS) lex.backup
+   rm -rf tmp_check
 
 # files removed here are supposed to be in the distribution tarball,
 # so do not clean them in the clean/distclean rules
 maintainer-clean: distclean
rm -f sql_help.h sql_help.c psqlscanslash.c
+
+
+check:
+   $(prove_check)
+
+installcheck:
+   $(prove_installcheck)
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index f17f610..4a3e471 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -49,6 +49,7 @@
 #include "psqlscanslash.h"
 #include "settings.h"
 #include "variables.h"
+#include "fe_utils/psqlscan_int.h"
 
 /*
  * Editable database object types.
@@ -132,7 +133,7 @@ HandleSlashCmds(PsqlScanState scan_state,
status = PSQL_CMD_ERROR;
}
 
-   if (status != PSQL_CMD_ERROR)
+   if (status != PSQL_CMD_ERROR && pset.active_branch)
{
/* eat any remaining arguments after a valid command */
/* note we suppress evaluation of backticks here */
@@ -194,6 +195,30 @@ read_connect_arg(PsqlScanState scan_state)
return result;
 }
 
+/*
+ * Read and interpret argument as a boolean expression.
+ * Return true if a boolean value was successfully parsed.
+ */
+static bool
+read_boolean_expression(PsqlScanState scan_state, char *action,
+   bool *result)
+{
+   char*value = psql_scan_slash_option(scan_state,
+   
OT_NORMAL, NULL, false);
+   boolsuccess = ParseVariableBool(value, action, result);
+   free(value);
+   return success;
+}
+
+/*
+ * Return true if the command given is a branching command.
+ */
+static bool
+is_branching_command(const char *cmd)
+{
+   return (strcmp(cmd, "if") == 0 || strcmp(cmd, "elif") == 0
+   || strcmp(cmd, "else") == 0 || strcmp(cmd, "endif") == 
0);
+}
 
 /*
  * Subroutine to actually try to execute a backslash 

Re: [HACKERS] LWLock optimization for multicore Power machines

[Alexander Korotkov]

On Mon, Feb 6, 2017 at 8:28 PM, Bernd Helmle  wrote:

> Am Montag, den 06.02.2017, 16:45 +0300 schrieb Alexander Korotkov:
> > I tried lwlock-power-2.patch on multicore Power machine we have in
> > PostgresPro.
> > I realized that using labels in assembly isn't safe.  Thus, I removed
> > labels and use relative jumps instead (lwlock-power-2.patch).
> > Unfortunately, I didn't manage to make any reasonable benchmarks.
> > This
> > machine runs AIX, and there are a lot of problems which prevents
> > PostgreSQL
> > to show high TPS.  Installing Linux there is not an option too,
> > because
> > that machine is used for tries to make Postgres work properly on AIX.
> > So, benchmarking help is very relevant.  I would very appreciate
> > that.
>
> Okay, so here are some results. The bench runs against
> current PostgreSQL master, 24 GByte shared_buffers configured (128
> GByte physical RAM), max_wal_size=8GB and effective_cache_size=100GB.
>

Thank you very much for testing!

Results looks strange for me.  I wonder why there is difference between
lwlock-power-1.patch and lwlock-power-3.patch?  From my intuition, it
shouldn't be there because it's not much difference between them.  Thus, I
have following questions.


   1. Have you warm up database?  I.e. could you do "SELECT sum(x.x) FROM
   (SELECT pg_prewarm(oid) AS x FROM pg_class WHERE relkind IN ('i', 'r')
   ORDER BY oid) x;" before each run?
   2. Also could you run each test longer: 3-5 mins, and run them with
   variety of clients count?


--
Alexander Korotkov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company

Re: \if, \elseif, \else, \endif (was Re: [HACKERS] PSQL commands: \quit_if, \quit_unless)

[Fabien COELHO]

Do you think the TAP tests would benefit from having the input described 
in a q(...) block rather than a string?


My 0.02€: Not really, so I would not bother. It breaks perl indentation 
and logic for a limited benefit. Maybe add comments if you feel that a 
test case is unclear.


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

Re: [HACKERS] [PROPOSAL] Temporal query processing with range types

[Peter Eisentraut]

On 2/2/17 12:43 PM, Peter Moser wrote:
> Hereby, we used the following commands to create both patches:
> git diff --no-prefix origin/master -- src/ ':!src/test/*' >
> tpg_primitives_out_v4.patch
> 
> git diff --no-prefix origin/master -- src/test/ >
> tpg_primitives_out_tests_v2.patch
> 
> We have also tested our patches on the current HEAD with the command:
> patch -p0 < patch-file
> 
> Both worked without problems or warnings on our Linux machine.
> Could you please explain, which problems occurred while you tried to
> apply our patches?

Your patches apply OK for me.

In the future, just use git diff without any options or git
format-patch, and put both the code and the tests in one patch.

-- 
Peter Eisentraut  http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


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

Re: [HACKERS] proposal: session server side variables

[Pavel Stehule]

2017-02-03 11:18 GMT+01:00 Fabien COELHO :

>
> We can implement XA support for variables, ale I don't think so default
>> should be XA.
>>
>
> I was answering your question, which is what you can do about the
> feedback: take the one hard/strong point into account in your proposal.
>
> You do not want to do that. Too bad.
>
> The argument that you keep on repeating about "other software do it like
> that so it is the good way" do not work because these software (Oracle,
> DB2, ...) have features unavailable to postgres which mitigate the issue
> I'm raising, and there is no such mitigation in postgres.
>
> Note that you can proceed and simply ignore my negative opinion, which
> will stay negative till these "secure" variables are transactional by
> default, or till nested/autonomous transactions are provided by postgres.


I'll work on my proposal in v11 time. Maybe in this time Postgres will
support autonomous transactions.

The variables syntax should be better integrated to core - it should be
implemented without getter/setter functions. I am not sure If statement SET
can be enhanced to allows the work with session variables without some
conflicts, but we will see.

Regards

Pavel




>
>
> --
> Fabien.
>

Re: \if, \elseif, \else, \endif (was Re: [HACKERS] PSQL commands: \quit_if, \quit_unless)

[Corey Huinker]

On Mon, Feb 6, 2017 at 11:21 AM, Corey Huinker 
wrote:

>
>> Find attached a small patch to improve tap tests, which also checks that
>>> psql really exited by checking that nothing is printed afterwards.
>>>
>>
>>
> Do you think the TAP tests would benefit from having the input described
> in a q(...) block rather than a string?
>
> q(\if false
> \echo a
> \elif invalid
> \echo b
> \endif
> \echo c
> )
>
>
> It's a lot more lines, obviously, but it might make what is being tested
> clearer.
>
>
It occurred to me that the part of this patch most important to casual
users would be the printed messages at various states. I've enumerated
those below, along with the circumstances under which the user would see
them.

The following messages are for interactive and script users. They are also
errors which respect ON_ERROR_STOP.
---

\if statement which had an invalid boolean expression:

new \if is invalid, ignoring commands until next \endif


\elif was in a proper \if block, and not after the true block, but boolean
expression was invalid:

\elif is invalid, ignoring commands until next \endif


\elif statement after an \else

encountered \elif after \else


\elif statement outside of an \if block [*]

encountered un-matched \elif


\else outside of an \if

encountered un-matched \else


\else after an \else

encountered \else after \else


\endif statement outside of an \if block

encountered un-matched \endif


Input file ends with unresolved \if blocks

found EOF before closing \endif(s)


The following are interactive-only non-error informational messages.
-

\if statement which parsed to true:

new \if is true, executing commands


\if statement which parsed to false:

new \if is false, ignoring commands until next \elif, \else, or \endif

\if statement while already in a false/invalid block:

new \if is inside ignored block, ignoring commands until next \endif


\elif statement immediately after the true \if or \elif

\elif is after true block, ignoring commands until next \endif


\elif statement within a false block or subsequent elif after the first
ignored elif

\elif is inside ignored block, ignoring commands until next \endif


\elif was evaluated, was true

\elif is true, executing commands


\elif was evaluated, was false

\elif is false, ignoring commands until next \elif, \else, or \endif


\else statement in an ignored block or after the true block was found:

\else after true condition or in ignored block, ignoring commands until
next \endif


\else statement and all previous blocks were false

\else after all previous conditions false, executing commands


\endif statement ending only \if on the stack

exited \if, executing commands


\endif statement where last block was false but parent block is also false:

exited \\if to false parent branch, ignoring commands until next \endif


\endif statement where last block was true and parent is true

exited \\if to true parent branch, continuing executing commands


\endif statement where last block was false but parent is true

exited \\if to true parent branch, resuming executing commands


Script is currently in a false (or invalid) branch, and user entered a
command other than if/elif/endif:

inside inactive branch, command ignored.


Script currently in a false branch, and user entered a query:

inside inactive branch, query ignored. use \endif to exit current branch.


User in an \if branch and pressed ^C, with no more branches remaining:

escaped \\if, executing commands


User in an \if branch and pressed ^C, but parent branch was false:

escaped \\if to false parent branch, ignoring commands until next \endif


User in a true \if branch and pressed ^C, parent branch true

escaped \\if to true parent branch, continuing executing commands


User in a false \if branch and pressed ^C, parent branch true

escaped \if to true parent branch, resuming executing commands



Notes:


The text for ignored commands vs ignored queries is different.

The text for all the Ctrl-C messages re-uses the \endif messages, but are
"escaped" instead of "exited".

Re: [HACKERS] Index corruption with CREATE INDEX CONCURRENTLY

[Tom Lane]

After some discussion among the release team, we've concluded that the
best thing to do is to push Pavan's/my patch into today's releases.
This does not close the matter by any means: we should continue to
study whether there are related bugs or whether there's a more principled
way of fixing this bug.  But that patch clearly makes things better,
and we shouldn't let worries about whether there are more bugs stop
us from providing some kind of fix to users.

I've made the push, and barring negative reports from the buildfarm,
it will be in today's releases.

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] SCRAM authentication, take three

[Aleksander Alekseev]

No, I'm afraid `make distclean` doesn't help. I've re-checked twice.

On Mon, Feb 06, 2017 at 12:52:11PM -0300, Alvaro Herrera wrote:
> Aleksander Alekseev wrote:
> > Hello.
> > 
> > Good to know that the work on this great feature continues!
> > 
> > However this set of patches does not pass `make check-world` on my
> > laptop.
> > 
> > Here is how I build and test PostgreSQL:
> > 
> > https://github.com/afiskon/pgscripts/blob/master/full-build.sh
> 
> I think you need "make distclean" instead of "make clean", unless you
> also add --enable-depend to configure.
> 
> -- 
> Álvaro Herrerahttps://www.2ndQuadrant.com/
> PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

-- 
Best regards,
Aleksander Alekseev


signature.asc
Description: PGP signature

Re: [HACKERS] Index corruption with CREATE INDEX CONCURRENTLY

[Robert Treat]

On Sun, Feb 5, 2017 at 9:42 PM, Pavan Deolasee  wrote:
> On Mon, Feb 6, 2017 at 5:41 AM, Peter Geoghegan  wrote:
>> On Sun, Feb 5, 2017 at 4:09 PM, Robert Haas  wrote:
>> > I don't think this kind of black-and-white thinking is very helpful.
>> > Obviously, data corruption is bad.  However, this bug has (from what
>> > one can tell from this thread) been with us for over a decade; it must
>> > necessarily be either low-probability or low-severity, or somebody
>> > would've found it and fixed it before now.  Indeed, the discovery of
>> > this bug was driven by new feature development, not a user report.  It
>> > seems pretty clear that if we try to patch this and get it wrong, the
>> > effects of our mistake could easily be a lot more serious than the
>> > original bug.
>>
>> +1. The fact that it wasn't driven by a user report convinces me that
>> this is the way to go.
>>
>
> I'm not sure that just because the bug wasn't reported by a user, makes it
> any less critical. As Tomas pointed down thread, the nature of the bug is
> such that the users may not discover it very easily, but that doesn't mean
> it couldn't be affecting them all the time. We can now correlate many past
> reports of index corruption to this bug, but we don't have evidence to prove
> that. Lack of any good tool or built-in checks probably makes it even
> harder.
>
> The reason why I discovered this with WARM is because it now has a built-in
> recheck logic, which was discarding some tuples returned by the index scan.
> It took me lots of code review and inspection to finally conclude that this
> must be an existing bug. Even then for lack of any utility, I could not
> detect this easily with master. That doesn't mean I wasn't able to
> reproduce, but detection was a problem.
>
> If you look at the reproduction setup, one in every 10, if not 5, CREATE
> INDEX CONCURRENTLY ends up creating a corrupt index. That's a good 10%
> probability. And this is on a low end laptop, with just 5-10 concurrent
> clients running. Probability of hitting the problem will be much higher on a
> bigger machine, with many users (on a decent AWS machine, I would find every
> third CIC to be corrupt). Moreover the setup is not doing anything
> extraordinary. Just concurrent updates which change between HOT to non-HOT
> and a CIC.
>

Not that I am advocating that we should do a release just for this;
having a fix we believe in is certainly as important a factor, but
that the idea that the bug has been around a long time means it is
less of an issue does seem wrong. We've certainly seen plenty of cases
over the years where bugs have existed in the code in seldom used code
paths, only to be exposed by new features or other code changes over
time. In general, we should be less worried about the age of a bug vs
our expectations that users are likely to hit that bug now, which does
seem high based on the above numbers.

In the meantime, it's certainly worth warning users, providing help on
how to determine if this is a likely problem for them, and possibly
rolling a patch ahead of upstream in cases where that's feasible.

Robert Treat
play: xzilla.net
work: omniti.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] increasing the default WAL segment size

[Beena Emerson]

Hello,
PFA the updated patches.

On Fri, Jan 27, 2017 at 2:17 PM, Beena Emerson 
wrote:

> Hello Andres,
>
> Thank you for your review.
>
> On Fri, Jan 27, 2017 at 12:39 AM, Andres Freund 
> wrote:
>
>> Hi,
>>
>> On 2017-01-23 11:35:11 +0530, Beena Emerson wrote:
>> > Please find attached an updated WIP patch. I have incorporated almost
>> all
>> > comments. This is to be applied over Robert's patches. I will post
>> > performance results later on.
>> >
>> > 1. shift (>>) and AND (&) operations: The assign hook of
>> wal_segment_size
>> > sets the WalModMask and WalShiftBit. All the modulo and division
>> operations
>> > using XLogSegSize has been replaced with these. However, there are many
>> > preprocessors which divide with XLogSegSize in xlog_internal.h. I have
>> not
>> > changed these because it would mean I will have to reassign the
>> WalShiftBit
>> > along with XLogSegSize in all the modules which use these macros. That
>> does
>> > not seem to be a good idea. Also, this means shift operator can be used
>> > only in couple of places.
>>
>> I think it'd be better not to have XLogSegSize anymore. Silently
>> changing a macros behaviour from being a compile time constant to
>> something runtime configurable is a bad idea.
>>
>
> I dont think I understood u clearly. You mean convert the macros using
> XLogSegSize to functions?
>

I have moved the ModMask related changes to a separate patch
 01-add-XLogSegmentOffset-macro.patch. This creates the macro
XLogSegmentOffset and replaces all "% XLogSegSize" and "% XLOG_SEG_SIZE"
with this macro.
I have not included the shift operator because the changes only apply to
about 4 lines did not give any performance boost or such.

Hm. Are GUC hooks a good way to compute the masks?  Interdependent GUCs
>> are unfortunately not working well, and several GUCs might end up
>> depending on these.  I think it might be better to assign the variables
>> somewhere early in StartupXLOG() or such.
>>
>
> I am not sure about these interdependent GUCs. I need to study this better
> and make changes as required.
>
>
The process flow is such thatthe Control File which sets the XLogSegSIze is
read after the GUC options are initialized. StartupXLOG is called by
StartupProcessMain() which restores the XLOG and then exits. Hence he value
initialised here are not persistent throughout the postgres run. It throws
error during pg_ctl stop.
The XLogSegSize adjustment in assign hooks have been removed and a new
macro ConvertToXSegs is used to convert the min and max wal_size to the
segment count when required. wal_segment_size set from ReadControlFile also
affects the Checkpointsegment value and hence the assign_wal_segment_size
calls CalculateCheckpointSegments.

Documentation is updated


Performance Tests:

I ran pgbench tests for different wal segment size on database of scale
factor 300 with shared_buffers of 8GB. Each of the tests ran for 10 min and
a median of 3 readings were considered. The following table shows the
performance of the patch wrt the HEAD for different client count for
various wal-segsize value. We could say that there is not performance
difference.

16 32 64 128





8MB -1.36 0.02 0.43 -0.24
16MB -0.38 0.18 -0.09 0.4
32MB -0.52 0.29 0.39 0.59
64MB -0.15 0.04 0.52 0.38



-- 
Thank you,

Beena Emerson

Have a Great Day!


01-add-XLogSegmentOffset-macro.patch
Description: Binary data


02-initdb-walsegsize-v2.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

Re: [HACKERS] Index corruption with CREATE INDEX CONCURRENTLY

[Tom Lane]

Alvaro Herrera  writes:
> Tom Lane wrote:
>> Better to fix the callers so that they don't have the assumption you
>> refer to.  Or maybe we could adjust the API of RelationGetIndexAttrBitmap
>> so that it returns all the sets needed by a given calling module at
>> once, which would allow us to guarantee they're consistent.

> Note that my "interesting attrs" patch does away with these independent
> bitmaps (which was last posted by Pavan as part of his WARM set).  I
> think we should fix just this bug now, and for the future look at that
> other approach.

BTW, if there is a risk of the assertion failure that Amit posits,
it seems like it should have happened in the tests that Pavan was doing
originally.  I'd sort of like to see a demonstration that it can actually
happen before we spend any great amount of time fixing it.

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] [PATCH] Rename pg_switch_xlog to pg_switch_wal

[Magnus Hagander]

On Mon, Feb 6, 2017 at 5:24 AM, Michael Paquier 
wrote:

> On Sat, Feb 4, 2017 at 6:39 AM, Robert Haas  wrote:
> > On Fri, Feb 3, 2017 at 5:21 AM, Stephen Frost 
> wrote:
> >> Daniel,
> >>
> >> * Daniel Verite (dan...@manitou-mail.org) wrote:
> >>> What if we look at the change from the pessimistic angle?
> >>> An example of confusion that the change would create:
> >>> a lot of users currently choose pg_wal for the destination
> >>> directory of their archive command. Less-informed users
> >>> that set up archiving and/or log shipping in PG10 based on
> >>> advice online from previous versions will be fairly
> >>> confused about the missing pg_xlog, and the fact that the
> >>> pg_wal directory they're supposed to create already exists.
> >>
> >> One would hope that they would realize that's not going to work
> >> when they set up PG10.  If they aren't paying attention sufficient
> >> to realize that then it seems entirely likely that they would feel
> >> equally safe removing the contents of a directory named 'pg_xlog'.
> >
> > So... somebody want to tally up the votes here?
>
> Here is what I have, 6 votes clearly stated:
> 1. Rename nothing: Daniel,
> 2. Rename directory only: Andres
> 3. Rename everything: Stephen, Vladimir, David S, Michael P (with
> aliases for functions, I could live without at this point...)
>

Put my vote down for 2.



> > And... was this discussed at the FOSDEM developer meeting?
> >
> > (Please say yes.)
>
> Looking only at the minutes, the answer is no:
> https://wiki.postgresql.org/wiki/FOSDEM/PGDay_2017_Developer_Meeting



We discussed discussing it :) And came to the conclusion that we did not
have enough of a quorum to actually make any decision on it complete, so we
figured it's better if everybody just chime in individually.


-- 
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/

Re: [HACKERS] LWLock optimization for multicore Power machines

[Bernd Helmle]

Am Montag, den 06.02.2017, 16:45 +0300 schrieb Alexander Korotkov:
> I tried lwlock-power-2.patch on multicore Power machine we have in
> PostgresPro.
> I realized that using labels in assembly isn't safe.  Thus, I removed
> labels and use relative jumps instead (lwlock-power-2.patch).
> Unfortunately, I didn't manage to make any reasonable benchmarks. 
> This
> machine runs AIX, and there are a lot of problems which prevents
> PostgreSQL
> to show high TPS.  Installing Linux there is not an option too,
> because
> that machine is used for tries to make Postgres work properly on AIX.
> So, benchmarking help is very relevant.  I would very appreciate
> that.

Okay, so here are some results. The bench runs against
current PostgreSQL master, 24 GByte shared_buffers configured (128
GByte physical RAM), max_wal_size=8GB and effective_cache_size=100GB.

I've just discovered that max_connections was accidently set to 601,
normally i'd have set something near 110 or so...



transaction type: 
scaling factor: 1000
query mode: prepared
number of clients: 100
number of threads: 100
duration: 30 s
number of transactions actually processed: 16910687
latency average = 0.177 ms
tps = 563654.968585 (including connections establishing)
tps = 563991.459659 (excluding connections establishing)
transaction type: 
scaling factor: 1000
query mode: prepared
number of clients: 100
number of threads: 100
duration: 30 s
number of transactions actually processed: 16523247
latency average = 0.182 ms
tps = 550744.748084 (including connections establishing)
tps = 552069.267389 (excluding connections establishing)
transaction type: 
scaling factor: 1000
query mode: prepared
number of clients: 100
number of threads: 100
duration: 30 s
number of transactions actually processed: 16796056
latency average = 0.179 ms
tps = 559830.986738 (including connections establishing)
tps = 560333.682010 (excluding connections establishing)



transaction type: 
scaling factor: 1000
query mode: prepared
number of clients: 100
number of threads: 100
duration: 30 s
number of transactions actually processed: 14563500
latency average = 0.206 ms
tps = 485420.764515 (including connections establishing)
tps = 485720.606371 (excluding connections establishing)
transaction type: 
scaling factor: 1000
query mode: prepared
number of clients: 100
number of threads: 100
duration: 30 s
number of transactions actually processed: 14618457
latency average = 0.205 ms
tps = 487246.817758 (including connections establishing)
tps = 488117.718816 (excluding connections establishing)
transaction type: 
scaling factor: 1000
query mode: prepared
number of clients: 100
number of threads: 100
duration: 30 s
number of transactions actually processed: 14522462
latency average = 0.207 ms
tps = 484052.194063 (including connections establishing)
tps = 485434.771590 (excluding connections establishing)



transaction type: 
scaling factor: 1000
query mode: prepared
number of clients: 100
number of threads: 100
duration: 30 s
number of transactions actually processed: 17946058
latency average = 0.167 ms
tps = 598164.841490 (including connections establishing)
tps = 598582.503248 (excluding connections establishing)
transaction type: 
scaling factor: 1000
query mode: prepared
number of clients: 100
number of threads: 100
duration: 30 s
number of transactions actually processed: 17719648
latency average = 0.169 ms
tps = 590621.671588 (including connections establishing)
tps = 591093.333153 (excluding connections establishing)
transaction type: 
scaling factor: 1000
query mode: prepared
number of clients: 100
number of threads: 100
duration: 30 s
number of transactions actually processed: 17722941
latency average = 0.169 ms
tps = 590728.715465 (including connections establishing)
tps = 591619.817043 (excluding connections establishing)


-- 
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 corruption with CREATE INDEX CONCURRENTLY

[Alvaro Herrera]

Tom Lane wrote:

> Better to fix the callers so that they don't have the assumption you
> refer to.  Or maybe we could adjust the API of RelationGetIndexAttrBitmap
> so that it returns all the sets needed by a given calling module at
> once, which would allow us to guarantee they're consistent.

Note that my "interesting attrs" patch does away with these independent
bitmaps (which was last posted by Pavan as part of his WARM set).  I
think we should fix just this bug now, and for the future look at that
other approach.

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


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

Re: [HACKERS] Cannot shutdown subscriber after DROP SUBSCRIPTION

[Petr Jelinek]

On 06/02/17 17:33, Fujii Masao wrote:
> On Sun, Feb 5, 2017 at 5:11 AM, Petr Jelinek
>  wrote:
>> On 03/02/17 19:38, Fujii Masao wrote:
>>> On Sat, Feb 4, 2017 at 12:49 AM, Fujii Masao  wrote:
 On Fri, Feb 3, 2017 at 10:56 AM, Kyotaro HORIGUCHI
  wrote:
> At Fri, 3 Feb 2017 01:02:47 +0900, Fujii Masao  
> wrote in 
> 

Re: [HACKERS] IF [NOT] EXISTS for replication slots

[Petr Jelinek]

On 06/02/17 05:15, Michael Paquier wrote:
> Hi all,
> 
> Lately I have bumped into a case where it would have been useful to
> make the difference between a failure because of a slot already
> dropped and an internal failure of Postgres. Is there any interest for
> support of IE and INE for CREATE and DROP_REPLICATION_SLOT?
> My use case involved only the SQL-callable interface, but I would
> think that it is useful to make this difference as well with the
> replication protocol. For the function we could just add a boolean
> argument to control the switch, and for the replication commands a
> dedicated keyword.
> 
> Any thoughts?
> 

My thought is, how would this handle the snapshot creation that logical
slot does when it's created?

-- 
  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] Index corruption with CREATE INDEX CONCURRENTLY

[Tom Lane]

Amit Kapila  writes:
> Hmm.  Consider that the first time relcahe invalidation occurs while
> computing id_attrs, so now the retry logic will compute the correct
> set of attrs (considering two indexes, if we take the example given by
> Alvaro above.).  However, the attrs computed for hot_* and key_* will
> be using only one index, so this will lead to a situation where two of
> the attrs (hot_attrs and key_attrs) are computed with one index and
> id_attrs is computed with two indexes. I think this can lead to
> Assertion in HeapSatisfiesHOTandKeyUpdate().

That seems like something we'd be best off to fix by changing the
assertion.  I doubt it's really going to be practical to guarantee that
those bitmapsets are always 100% consistent throughout a transaction.

> Okay, please find attached patch which is an extension of Tom's and
> Andres's patch and I think it fixes the above problem noted by me.

I don't like this patch at all.  It only fixes the above issue if the
relcache inval arrives while RelationGetIndexAttrBitmap is executing.
If the inval arrives between two such calls, you still have the problem
of the second one delivering a bitmapset that might be inconsistent
with the first one.

To go in this direction, I think we would have to hot-wire
RelationGetIndexAttrBitmap so that once any bitmapset has been requested
in a transaction, we intentionally ignore all index set updates for the
remainder of the transaction.  And that would very likely create more
problems than it solves (consider locally initiated DDL within the
transaction, for example).

Better to fix the callers so that they don't have the assumption you
refer to.  Or maybe we could adjust the API of RelationGetIndexAttrBitmap
so that it returns all the sets needed by a given calling module at
once, which would allow us to guarantee they're consistent.

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] Cannot shutdown subscriber after DROP SUBSCRIPTION

[Fujii Masao]

On Sun, Feb 5, 2017 at 5:11 AM, Petr Jelinek
 wrote:
> On 03/02/17 19:38, Fujii Masao wrote:
>> On Sat, Feb 4, 2017 at 12:49 AM, Fujii Masao  wrote:
>>> On Fri, Feb 3, 2017 at 10:56 AM, Kyotaro HORIGUCHI
>>>  wrote:
 At Fri, 3 Feb 2017 01:02:47 +0900, Fujii Masao  
 wrote in 
 

Re: \if, \elseif, \else, \endif (was Re: [HACKERS] PSQL commands: \quit_if, \quit_unless)

[Corey Huinker]

>
>
> Find attached a small patch to improve tap tests, which also checks that
>> psql really exited by checking that nothing is printed afterwards.
>>
>
>
Do you think the TAP tests would benefit from having the input described in
a q(...) block rather than a string?

q(\if false
\echo a
\elif invalid
\echo b
\endif
\echo c
)


It's a lot more lines, obviously, but it might make what is being tested
clearer.

Re: [HACKERS] Draft release notes for next week's releases are up for review

[Tom Lane]

Tobias Bussmann  writes:
> another typo taken over from commit log:
> s/Tobias Bussman/Tobias Bussmann/

Will fix, thanks!

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] [RFC] Should "SHOW huge_pages" display the effective value "off" when the huge page is unavailable?

[Tom Lane]

Fujii Masao  writes:
> On Tue, Feb 7, 2017 at 12:36 AM, Tom Lane  wrote:
>> If the proposal is to have SHOW report something other than the setting
>> of the variable, that's not a great plan either.  It's generally important
>> that the output of SHOW be something that's acceptable to SET, as not
>> having that equivalence will break assorted client-side code.

> I was thinking that Tunakawa-san's proposal is this, i.e., use GUC show-hook
> to show "off" if the server fails to use huge-page and "on" otherwise.

Well, then you wouldn't know whether the true setting was "try" or not,
which is important information because of the crash/restart possibility.
If we went this direction, I think the SHOW output would have to read
something like "try (off)" or "try (on)", which is why I was concerned
about it not being acceptable SET input.

>> I think this desire would be better addressed by some kind of specialized
>> inquiry function, which would also be able to return more information than
>> just a naked "on/off" bit.  People might for instance wish to know what
>> hugepage size is in use.

> +1

But it's moot anyway if we're agreed that a separate function is better.

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] IF (NOT) EXISTS in psql-completion

[Pavel Stehule]

Hi

2017-02-03 9:17 GMT+01:00 Kyotaro HORIGUCHI :

> Hello. This is the new version of this patch.
>
> - Rebased to the current master (555494d)
>   PUBLICATION/SUBSCRIPTION stuff conflicted.
>
> - Fix a bug of CREATE INDEX(0012-Simplify-completion-for-CREATE-INDEX.
> patch).
>   CREATE INDEX ON no longer gets a suggestion of "ON".
>
> - Added logging feature (0018-Debug-output-of-psql-completion.patch)
>
>   This might be suitable to be a separate patch. psql completion
>   code is defficult to debug when it is uncertain what line did a
>   suggestion. This patch allows completion logs to psql log,
>   which is activated by -L option.
>
>   psql -L  
>
>   And the logs like the following will be printed.
>
>   | completion performed at tab-complete.c:1146 for "do"
>
> - OR REPLACE suggestion (0019-Add-suggestion-of-OR-REPLACE.patch)
>
> At Wed, 1 Feb 2017 09:42:54 +0100, Pavel Stehule 
> wrote in  jgbu2trrs...@mail.gmail.com>
> > 2017-02-01 9:37 GMT+01:00 Kyotaro HORIGUCHI <
> horiguchi.kyot...@lab.ntt.co.jp
> > the content of my last mail is a copy my mail from end of December.
> > Probably lot of changes there.
>
> Thanks for reposting.
>
> > > > 2. tab complete doesn't work well if I am manually writing "create
> index
> > > > on" - second "ON" is appended - it is a regression
> > >
> > > I'll fix it in the version.
> > >
> > > > I didn't find any other issues -
> > > >
> > > > note: not necessary to implement (nice to have) - I miss a support
> for OR
> > > > REPLACE flag - it is related to LANGUAGE, TRANSFORMATION,  FUNCTION
> and
> > > > RULE.
>
> Hmm. This patch perhaps should not 'add a feature' (how about the
> logging..). Anyway the last 19th patch does that.  The word
> removal framework works well for this case.
>
> After all, this patch is so large that I'd like to attach them as
> one compressed file. The content of the file is the following.
>
>
> 0001-Refactoring-tab-complete-to-make-psql_completion-cod.patch
>   - Just a refactoring of psql_completion
>
> 0002-Make-keywords-case-follow-to-input.patch
>   - The letter case of additional suggestions for
> COMPLETION_WITH_XX follows input.
>
> 0003-Introduce-word-shift-and-removal-feature-to-psql-com.patch
>   - A feature to ignore preceding words. And a feature to remove
> intermediate words.
>
> 0004-Add-README-for-tab-completion.patch
>   - README
>
> 0005-Make-SET-RESET-SHOW-varialble-follow-input-letter-ca.patch
> 0006-Allow-complete-schema-elements-in-more-natural-way.patch
> 0007-Allow-CREATE-RULE-to-use-command-completion-recursiv.patch
> 0008-Allow-completing-the-body-of-EXPLAIN.patch
> 0009-Simpilfy-ALTER-TABLE-ALTER-COLUMN-completion.patch
> 0010-Simplify-completion-for-CLUSTER-VERBOSE.patch
> 0011-Simplify-completion-for-COPY.patch
> 0012-Simplify-completion-for-CREATE-INDEX.patch
> 0013-Simplify-completion-for-CREATE-SEQUENCE.patch
> 0014-Simplify-completion-for-DROP-INDEX.patch
> 0015-Add-CURRENT_USER-to-some-completions-of-role.patch
> 0016-Refactor-completion-for-ALTER-DEFAULT-PRIVILEGES.patch
> 0017-Add-suggestions-of-IF-NOT-EXISTS.patch
>   - A kind of sample refctoring (or augmenting) suggestion code
> based on the new infrastructure.
>
> 0018-Debug-output-of-psql-completion.patch
>   - Debug logging for psql_completion (described above)
>
> 0019-Add-suggestion-of-OR-REPLACE.patch
>   - Suggestion of CREATE OR REPLACE.
>
>
> # I hear the footsteps of another conflict..
>
>
The patch 0018 was not be applied.

Few other notes from testing - probably these notes should not be related
to your patch set

1. When we have set of keywords, then the upper or lower chars should to
follow previous keyword. Is it possible? It should to have impact only on
keywords.

2. the list of possible functions after EXECUTE PROCEDURE in CREATE TRIGGER
statement should be reduced to trigger returns function only.

CREATE OR REPLACE FUNCTIONS works great, thank you!

Regards

Pavel








> regards,
>
> --
> Kyotaro Horiguchi
> NTT Open Source Software Center
>

Re: [HACKERS] [RFC] Should "SHOW huge_pages" display the effective value "off" when the huge page is unavailable?

[Fujii Masao]

On Tue, Feb 7, 2017 at 12:36 AM, Tom Lane  wrote:
> Fujii Masao  writes:
>> On Mon, Feb 6, 2017 at 4:01 PM, Tsunakawa, Takayuki
>>  wrote:
>>> I don't have a strong opinion on that, but I think a bit that it would be 
>>> better to reflect the effective setting, i.e. SHOW displays huge_pages as 
>>> off, not try.
>
>> Not sure if this is best way to do that, but I agree that it's helpful if
>> we can see whether the server actually uses huge page or not in
>> huge_page=try case.
>
> If the proposal is to actually change the stored value of huge_pages,
> I would say "absolutely not".  Suppose that you change "try" to "on",
> and there's a backend crash and restart so that the postmaster needs
> to reallocate shared memory, and this time it's unable to obtain
> huge pages for some reason.  Taking the database down would be entirely
> the wrong thing.  Also, how would you handle postgresql.conf reload
> situations?
>
> If the proposal is to have SHOW report something other than the setting
> of the variable, that's not a great plan either.  It's generally important
> that the output of SHOW be something that's acceptable to SET, as not
> having that equivalence will break assorted client-side code.

I was thinking that Tunakawa-san's proposal is this, i.e., use GUC show-hook
to show "off" if the server fails to use huge-page and "on" otherwise.

> I think this desire would be better addressed by some kind of specialized
> inquiry function, which would also be able to return more information than
> just a naked "on/off" bit.  People might for instance wish to know what
> hugepage size is in use.

+1

Regards,

-- 
Fujii Masao


-- 
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] Provide list of subscriptions and publications in psql's completion

[Fujii Masao]

On Mon, Feb 6, 2017 at 1:52 PM, Michael Paquier
 wrote:
> On Sun, Feb 5, 2017 at 5:04 AM, Fujii Masao  wrote:
>> With this patch, when normal users type TAB after SUBSCRIPTION,
>> they got "permission denied" error. I don't think that's acceptable.
>
> Right, I can see that now. pg_stat_get_subscription() does not offer
> as well a way to filter by databases... Perhaps we could add it? it is
> stored as LogicalRepWorker->dbid so making it visible is sort of easy.

Yes, that's an option. And, if we add dbid to pg_stat_subscription,
I'm tempted to add all the pg_subscription's columns except subconninfo
into pg_stat_subscription. Since subconninfo may contain security-sensitive
information, it should be excluded. But it seems useful to expose other
columns. Then, if we expose all the columns except subconninfo, maybe
it's better to just revoke subconninfo column on pg_subscription instead of
all columns. Thought?

BTW, I found that psql's \dRs command has the same problem;
the permission denied error occurs when normal user runs \dRs.
This issue should be fixed in the same way as tab-completion one.

Also I found that tab-completion for psql's meta-commands doesn't
show \dRp and \dRs.

>> In "CREATE SUBSCRIPTION ... PUBLICATION" and "ALTER SUBSCRIPTION ... SET
>> PUBLICATION" cases, the publication defined in the publisher side should be
>> specified. But, with this patch, the tab-completion for PUBLICATION shows
>> the publications defined in local server (ie, subscriber side in those 
>> cases).
>> This might be confusing.
>
> Which would mean that psql tab completion should try to connect the
> remote server using the connection string defined with the
> subscription to get this information, which looks unsafe to me. To be
> honest, I'd rather see a list of local objects defined than nothing..

IMO showing nothing is better. But many people think that even local
objects should be showed in that case, I can live with that.

Regards,

-- 
Fujii Masao


-- 
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 spelling fixes

[Piotr Stefaniak]

On 2017-02-06 10:40, Heikki Linnakangas wrote:
> On 02/06/2017 04:50 AM, Josh Soref wrote:
>> NUL-terminated -> NULL-terminated
> 
> When we're talking about NUL-terminated strings, NUL refers to the NUL
> ASCII character. NULL usually refers to a NULL pointer. We're probably
> not consistent about this, but in this context, NUL-terminated isn't
> wrong, so let's leave them as they are.

The C standard talks about how "a byte with all bits set to 0, called
the null character" is used to "terminate a character string"; it
mentions '\0' as "commonly used to represent the null character"; and it
also talks about when snprintf() produces "null-terminated output".

It never mentions ASCII in this context; quite intentionally it avoids
assuming ASCII at all, so that a standard-compliant C implementation may
co-exist with other encodings (like EBCDIC).



-- 
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] SCRAM authentication, take three

[Alvaro Herrera]

Aleksander Alekseev wrote:
> Hello.
> 
> Good to know that the work on this great feature continues!
> 
> However this set of patches does not pass `make check-world` on my
> laptop.
> 
> Here is how I build and test PostgreSQL:
> 
> https://github.com/afiskon/pgscripts/blob/master/full-build.sh

I think you need "make distclean" instead of "make clean", unless you
also add --enable-depend to configure.

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


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

Re: [HACKERS] [RFC] Should "SHOW huge_pages" display the effective value "off" when the huge page is unavailable?

[Tom Lane]

Fujii Masao  writes:
> On Mon, Feb 6, 2017 at 4:01 PM, Tsunakawa, Takayuki
>  wrote:
>> I don't have a strong opinion on that, but I think a bit that it would be 
>> better to reflect the effective setting, i.e. SHOW displays huge_pages as 
>> off, not try.

> Not sure if this is best way to do that, but I agree that it's helpful if
> we can see whether the server actually uses huge page or not in
> huge_page=try case.

If the proposal is to actually change the stored value of huge_pages,
I would say "absolutely not".  Suppose that you change "try" to "on",
and there's a backend crash and restart so that the postmaster needs
to reallocate shared memory, and this time it's unable to obtain
huge pages for some reason.  Taking the database down would be entirely
the wrong thing.  Also, how would you handle postgresql.conf reload
situations?

If the proposal is to have SHOW report something other than the setting
of the variable, that's not a great plan either.  It's generally important
that the output of SHOW be something that's acceptable to SET, as not
having that equivalence will break assorted client-side code.

I think this desire would be better addressed by some kind of specialized
inquiry function, which would also be able to return more information than
just a naked "on/off" bit.  People might for instance wish to know what
hugepage size is in use.

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] [RFC] Should "SHOW huge_pages" display the effective value "off" when the huge page is unavailable?

[Fujii Masao]

On Mon, Feb 6, 2017 at 4:01 PM, Tsunakawa, Takayuki
 wrote:
> Hello, all
>
> Could you give me your opinions on whether the SHOW command should display 
> the effective value or the specified value for huge_pages?  During the review 
> of "Supporting huge_pages on Windows", which is now shifted to CommitFest 
> 2017-3, Magnus gave me a comment that the huge_page variable should retain 
> the value "try" when the huge page is not available on the machine and the 
> server falls back to huge_page=off.  The Linux version does so.
>
> I don't have a strong opinion on that, but I think a bit that it would be 
> better to reflect the effective setting, i.e. SHOW displays huge_pages as 
> off, not try.

Not sure if this is best way to do that, but I agree that it's helpful if
we can see whether the server actually uses huge page or not in
huge_page=try case.

Regards,

-- 
Fujii Masao


-- 
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] SCRAM authentication, take three

[Aleksander Alekseev]

Hello.

Good to know that the work on this great feature continues!

However this set of patches does not pass `make check-world` on my
laptop.

Here is how I build and test PostgreSQL:

https://github.com/afiskon/pgscripts/blob/master/full-build.sh

Error message:

http://afiskon.ru/s/0b/258c6e4f14_123.txt

regression.diffs:

http://afiskon.ru/s/a0/4993102c32_regression.diffs.txt

regression.out:

http://afiskon.ru/s/4b/acd5a58374_regression.out.txt

Backtrace:

http://afiskon.ru/s/00/c0b32b45a6_bt.txt

I'm using Arch Linux with latest updates, Python version is 3.6.0. I
have no idea what SCRAM has to do with Python, but this issue reproduced
two times of two I did `make check-world`. There is no such issue in
current master branch.

On Mon, Feb 06, 2017 at 02:55:08PM +0200, Heikki Linnakangas wrote:
> I rebased the SCRAM authentication patches over current master. Here you
> are.
> 
> I'm trying to whack this into the final shape that it could actually be
> committed. The previous thread on SCRAM authentication has grown
> ridiculously long and meandered into all kinds of details, so I thought it's
> best to start afresh with a new thread.
> 
> So, if you haven't paid attention on this for a while, now would be a good
> time to have another look at the patch. I believe all the basic
> functionality, documentation, and tests are there, and there are no known
> bugs. Please review! I'll start reading through these myself again tomorrow.
> 
> One thing that's missing, that we need to address before the release, is the
> use of SASLPrep to "normalize" the password. We discussed that in the
> previous thread, and I think we have a good path forward on it. I'd be happy
> to leave that for a follow-up commit, after these other patches have been
> committed, so we can discuss that work separately.
> 
> These are also available on Michael's github repository, at
> https://github.com/michaelpq/postgres/tree/scram.
> 
> - Heikki
> 

-- 
Best regards,
Aleksander Alekseev


signature.asc
Description: PGP signature

Re: [HACKERS] [PATCH] Rename pg_switch_xlog to pg_switch_wal

[Fujii Masao]

On Mon, Feb 6, 2017 at 1:24 PM, Michael Paquier
 wrote:
> On Sat, Feb 4, 2017 at 6:39 AM, Robert Haas  wrote:
>> On Fri, Feb 3, 2017 at 5:21 AM, Stephen Frost  wrote:
>>> Daniel,
>>>
>>> * Daniel Verite (dan...@manitou-mail.org) wrote:
 What if we look at the change from the pessimistic angle?
 An example of confusion that the change would create:
 a lot of users currently choose pg_wal for the destination
 directory of their archive command. Less-informed users
 that set up archiving and/or log shipping in PG10 based on
 advice online from previous versions will be fairly
 confused about the missing pg_xlog, and the fact that the
 pg_wal directory they're supposed to create already exists.
>>>
>>> One would hope that they would realize that's not going to work
>>> when they set up PG10.  If they aren't paying attention sufficient
>>> to realize that then it seems entirely likely that they would feel
>>> equally safe removing the contents of a directory named 'pg_xlog'.
>>
>> So... somebody want to tally up the votes here?
>
> Here is what I have, 6 votes clearly stated:
> 1. Rename nothing: Daniel,
> 2. Rename directory only: Andres
> 3. Rename everything: Stephen, Vladimir, David S, Michael P (with
> aliases for functions, I could live without at this point...)

I vote for 1.
I still wonder how much the renaming of pg_xlog actually helps very careless
people who remove pg_xlog becase its name includes "log". I'm afraid that
they would make another serious mistake (e.g., remove pg_wal because it has
many files and it occupies large amount of disk space) even after renaming
to pg_wal. The crazy idea, making initdb create the empty file with the name
"DONT_REMOVE_pg_xlog_IF_YOU_DONT_WANT_TO_LOSE_YOUR_IMPORTANT_DATA"
in $PGDATA seems more helpful. Anyway I'm afraid that the renaming would
cause more pain than gain.

Regards,

-- 
Fujii Masao


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

Re: [HACKERS] [PATCH] Rename pg_switch_xlog to pg_switch_wal

[Kevin Grittner]

On Sun, Feb 5, 2017 at 10:24 PM, Michael Paquier
 wrote:

>> So... somebody want to tally up the votes here?
>
> Here is what I have, 6 votes clearly stated:
> 1. Rename nothing: Daniel,
> 2. Rename directory only: Andres
> 3. Rename everything: Stephen, Vladimir, David S, Michael P (with
> aliases for functions, I could live without at this point...)

I vote for 3 as well, with 1 as the only sane alternative.

-- 
Kevin Grittner


-- 
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] Transactions involving multiple postgres foreign servers

[Masahiko Sawada]

On Wed, Feb 1, 2017 at 8:25 PM, Robert Haas  wrote:
> On Mon, Jan 30, 2017 at 2:30 AM, Masahiko Sawada  
> wrote:
>> "txn" can be used for abbreviation of "Transaction", so for example
>> pg_fdw_txn_resolver?
>> I'm also fine to change the module and function name.
>
> If we're judging the relative clarity of various ways of abbreviating
> the word "transaction", "txn" surely beats "x".
>
> To repeat my usual refrain, is there any merit to abbreviating at all?
>  Could we call it, say, "fdw_transaction_resolver" or
> "fdw_transaction_manager"?
>

Almost modules in contrib are name with "pg_" prefix but I prefer
"fdw_transcation_resolver" if we don't need  "pg_" prefix.

Regards,

--
Masahiko Sawada
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center


-- 
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] LWLock optimization for multicore Power machines

[Alexander Korotkov]

On Fri, Feb 3, 2017 at 11:31 PM, Bernd Helmle  wrote:

> > UPD: It appears that Postgres Pro have access to big Power machine
> > now.
> > So, I can do testing/benchmarking myself.
>
> We currently also have access to a LPAR on an E850 machine with 4
> sockets POWER8 running a Ubuntu 16.04 LTS Server ppc64el OS. I can do
> some tests next week, if you need to verify your findings.
>

Very good, thank you!

I tried lwlock-power-2.patch on multicore Power machine we have in
PostgresPro.
I realized that using labels in assembly isn't safe.  Thus, I removed
labels and use relative jumps instead (lwlock-power-2.patch).
Unfortunately, I didn't manage to make any reasonable benchmarks.  This
machine runs AIX, and there are a lot of problems which prevents PostgreSQL
to show high TPS.  Installing Linux there is not an option too, because
that machine is used for tries to make Postgres work properly on AIX.
So, benchmarking help is very relevant.  I would very appreciate that.

--
Alexander Korotkov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company


lwlock-power-3.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

Re: [HACKERS] Possible spelling fixes

[Alvaro Herrera]

Andres Freund wrote:

> On 2017-02-05 21:05:50 -0500, Josh Soref wrote:

> > A complete diff would be roughly 130k. I've recently tried to submit a
> > similarly sized patch to another project and it was stuck in
> > moderation (typically mailing lists limit attachments to around 40k).
> 
> IIRC 130k shouln't get you stuck in filters yet - if you're concerned
> you can gzip the individual patches.

Our limit for pgsql-hackers is 1 MB.

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


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

[HACKERS] SCRAM authentication, take three

[Heikki Linnakangas]

I rebased the SCRAM authentication patches over current master. Here you 
are.


I'm trying to whack this into the final shape that it could actually be 
committed. The previous thread on SCRAM authentication has grown 
ridiculously long and meandered into all kinds of details, so I thought 
it's best to start afresh with a new thread.


So, if you haven't paid attention on this for a while, now would be a 
good time to have another look at the patch. I believe all the basic 
functionality, documentation, and tests are there, and there are no 
known bugs. Please review! I'll start reading through these myself again 
tomorrow.


One thing that's missing, that we need to address before the release, is 
the use of SASLPrep to "normalize" the password. We discussed that in 
the previous thread, and I think we have a good path forward on it. I'd 
be happy to leave that for a follow-up commit, after these other patches 
have been committed, so we can discuss that work separately.


These are also available on Michael's github repository, at 
https://github.com/michaelpq/postgres/tree/scram.


- Heikki



0001-Refactor-SHA2-functions-and-move-them-to-src-common.patch.gz
Description: application/gzip


0002-Add-encoding-routines-for-base64-without-whitespace-.patch.gz
Description: application/gzip


0003-Add-clause-PASSWORD-val-USING-protocol-to-CREATE-ALT.patch.gz
Description: application/gzip


0004-Support-for-SCRAM-SHA-256-authentication-RFC-5802-an.patch.gz
Description: application/gzip


0005-Add-regression-tests-for-passwords.patch.gz
Description: application/gzip


0006-Add-TAP-tests-for-authentication-methods.patch.gz
Description: application/gzip

-- 
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 corruption with CREATE INDEX CONCURRENTLY

[Alvaro Herrera]

Andres Freund wrote:

> To show what I mean here's an *unpolished* and *barely tested* patch
> implementing the first of my suggestions.
> 
> Alvaro, Pavan, I think should address the issue as well?

Hmm, interesting idea.  Maybe a patch along these lines is a good way to
make index-list cache less brittle going forward.  However, I'm against
putting it in the stable branches -- and we should definitely not stall
an immediate fix in order to get this patch ready.  IMO we should get
Tom's patch in tree for all branches very soon, and then after the
release you can finalize this one and put it to master.

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


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

Re: [HACKERS] Index corruption with CREATE INDEX CONCURRENTLY

[Alvaro Herrera]

Tom Lane wrote:

> Pavan Deolasee  writes:

> > 2. In the second patch, we tried to recompute attribute lists if a relcache
> > flush happens in between and index list is invalidated. We've seen problems
> > with that, especially it getting into an infinite loop with
> > CACHE_CLOBBER_ALWAYS. Not clear why it happens, but apparently relcache
> > flushes keep happening.
> 
> Well, yeah: the point of CLOBBER_CACHE_ALWAYS is to make a relcache flush
> happen whenever it possibly could.  The way to deal with that without
> looping is to test whether the index set *actually* changed, not whether
> it just might have changed.

Ah, that's a nice and simple way to fix that patch!  I see that Pavan
confirmed that it fixes the tests we saw failing too.  It seems to me
that we should go with this one, and it looks unlikely that this causes
other problems.

BTW, while neiter Pavan nor I sent this patch to -hackers, this
implementation is pretty much the same thing we did.  Pavan deserves
credit as co-author in this commit.

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


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

Re: [HACKERS] GSoC 2017

[Ruben Buchatskiy]

2017-01-10 12:53 GMT+03:00 Alexander Korotkov :

>
> 1. What project ideas we have?
>
>
Hi!

We would like to propose a project on rewriting PostgreSQL executor from

traditional Volcano-style [1] to so-called push-based architecture as
implemented in

Hyper [2][3] and VitesseDB [4]. The idea is to reverse the direction of
data flow

control: instead of pulling up tuples one-by-one with ExecProcNode(), we
suggest

pushing them from below to top until blocking operator (e.g. Aggregation) is

encountered. There’s a good example and more detailed explanation for this
approach in [2].

The advantages of this approach:

 * It allows to completely avoid the need of loading/storing the internal
state of the bottommost

   (scanning) nodes, which will significantly reduce overhead. With current
pull-based model,

   we call functions like heapgettup_pagemode() (and many others)
number-of-tuples-to-retrieve

   times, while in push-based model we will call them only once. Currently,
we have

   implemented a prototype for SeqScan node and achieved 2x speedup on query

   “select * from lineitem”;

 * The number of memory accesses is minimized; generally better code and
data locality,

cache is used more effectively;

 * Switching to push model also makes a good base for building effective
JIT-compiler.

   Currently we have working LLVM-based JIT compiler for expressions [5],
as well as whole query

   JIT-compiler [6], which speeds up TPC-H queries up to 4-5 times, but the
latter took manually

   re-implementing the executor logic with LLVM API using push model to get
this speedup. JIT-compiling

   from original Postgres C code didn't give significant improvement
because of Volcano-style model

   inherent inefficiency. After making a switch to push-model we expect to
achieve speedup comparable

   to stand-alone JIT, but using the same code for both JIT and the
interpreter.

Also, while working on this project, we are likely be revealing and fixing
other

weak places of the current query executor. Volcano-style model is known to
have

inadequate performance characteristics [7][8], e.g. function call overhead,

and we should deal with it anyway. We also plan to make relatively small
patches,

which will optimize the redundant reload of the internal state in the
current pull-model.

Many DB systems with support of full query compilation (e.g. LegoBase [9],
Hekaton [10]) implement it in push-based manner.

Also we have seen in the mailing list that Kumar Rajeev had been
investigating this idea too, and he reported that the results were
impressive (unfortunately, without specifying more details):

https://www.postgresql.org/message-id/BF2827DCCE55594C8D7A8F7FFD3AB77159A9B904%40szxeml521-mbs.china.huawei.com

References

[1]  Graefe G.. Volcano — an extensible and parallel query evaluation
system. IEEE Trans. Knowl. Data Eng.,6(1): 120–135, 1994.

[2] Efficiently Compiling Efficient Query Plans for Modern Hardware,

   http://www.vldb.org/pvldb/vol4/p539-neumann.pdf

[3] Compiling Database Queries into Machine Code,

   http://sites.computer.org/debull/A14mar/p3.pdf

[4]
https://docs.google.com/presentation/d/1R0po7_Wa9fym5U9Y5qHXGlUi77nSda2LlZXPuAxtd-M/pub?slide=id.g9b338944f_4_131

[5] PostgreSQL with JIT compiler for expressions,

   https://github.com/ispras/postgres

[6] LLVM Cauldron, slides,

   http://llvm.org/devmtg/2016-09/slides/Melnik-PostgreSQLLLVM.pdf

[7] MonetDB/X100: Hyper-Pipelining Query Execution

   http://cidrdb.org/cidr2005/papers/P19.pdf

[8] Vectorization vs. Compilation in Query Execution,


https://pdfs.semanticscholar.org/dcee/b1e11d3b078b0157325872a581b51402ff66.pdf

[9] http://www.vldb.org/pvldb/vol7/p853-klonatos.pdf
[10]
https://www.microsoft.com/en-us/research/wp-content/uploads/2013/06/Hekaton-Sigmod2013-final.pdf


-- 

*Best Regards,**Ruben.* 
ISP RAS.
Project title: Implementing push-based query executor

Project Description
Currently, PostgreSQL uses traditional Volcano-style [1] query execution model.
While it is a simple and flexible model, it behaves poorly on modern superscalar
CPUs [2][3] due to lack of locality and frequent instruction mispredictions.
It becomes a major issue for complex OLAP queries with CPU-heavy workloads.
We propose to implement so-called push-based query executor model
as described in [4][5], which improves code and data locality and cache usage
itself; also push-based executor can serve as a platform
for efficient JIT query compilation.

See [6] for more details.

Skills needed
  The ability to understand and modify PostgresSQL executor code;
  The ability to run careful in-memory benchmarks to demonstrate the result;
  The ability to profile Postgres in order to find slow code;
  Understanding modern processors features (pipelining, superscalar CPUs,
branch prediction, etc) would be very helpful.

Difficulty Level
Moderate-level; however, microoptimizations might be hard.
Probably it will also be 

Re: [HACKERS] [PATCH] kNN for SP-GiST

[Nikita Glukhov]

Attached v02 version (rebased to HEAD).

--
Nikita Glukhov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company
diff --git a/src/backend/access/gist/gistutil.c b/src/backend/access/gist/gistutil.c
index f92baed..1dfaba2 100644
--- a/src/backend/access/gist/gistutil.c
+++ b/src/backend/access/gist/gistutil.c
@@ -23,6 +23,7 @@
 #include "storage/lmgr.h"
 #include "utils/builtins.h"
 #include "utils/syscache.h"
+#include "utils/lsyscache.h"
 
 
 /*
@@ -851,12 +852,6 @@ gistproperty(Oid index_oid, int attno,
 			 IndexAMProperty prop, const char *propname,
 			 bool *res, bool *isnull)
 {
-	HeapTuple	tuple;
-	Form_pg_index rd_index PG_USED_FOR_ASSERTS_ONLY;
-	Form_pg_opclass rd_opclass;
-	Datum		datum;
-	bool		disnull;
-	oidvector  *indclass;
 	Oid			opclass,
 opfamily,
 opcintype;
@@ -890,49 +885,28 @@ gistproperty(Oid index_oid, int attno,
 	}
 
 	/* First we need to know the column's opclass. */
-
-	tuple = SearchSysCache1(INDEXRELID, ObjectIdGetDatum(index_oid));
-	if (!HeapTupleIsValid(tuple))
+	opclass = get_index_column_opclass(index_oid, attno);
+	if (!OidIsValid(opclass))
 	{
 		*isnull = true;
 		return true;
 	}
-	rd_index = (Form_pg_index) GETSTRUCT(tuple);
-
-	/* caller is supposed to guarantee this */
-	Assert(attno > 0 && attno <= rd_index->indnatts);
-
-	datum = SysCacheGetAttr(INDEXRELID, tuple,
-			Anum_pg_index_indclass, );
-	Assert(!disnull);
-
-	indclass = ((oidvector *) DatumGetPointer(datum));
-	opclass = indclass->values[attno - 1];
-
-	ReleaseSysCache(tuple);
 
 	/* Now look up the opclass family and input datatype. */
-
-	tuple = SearchSysCache1(CLAOID, ObjectIdGetDatum(opclass));
-	if (!HeapTupleIsValid(tuple))
+	if (!get_opclass_opfamily_and_input_type(opclass, , ))
 	{
 		*isnull = true;
 		return true;
 	}
-	rd_opclass = (Form_pg_opclass) GETSTRUCT(tuple);
-
-	opfamily = rd_opclass->opcfamily;
-	opcintype = rd_opclass->opcintype;
-
-	ReleaseSysCache(tuple);
 
 	/* And now we can check whether the function is provided. */
-
 	*res = SearchSysCacheExists4(AMPROCNUM,
  ObjectIdGetDatum(opfamily),
  ObjectIdGetDatum(opcintype),
  ObjectIdGetDatum(opcintype),
  Int16GetDatum(procno));
+	*isnull = false;
+
 	return true;
 }
 
diff --git a/src/backend/utils/cache/lsyscache.c b/src/backend/utils/cache/lsyscache.c
index 1b04c09..1d479fe 100644
--- a/src/backend/utils/cache/lsyscache.c
+++ b/src/backend/utils/cache/lsyscache.c
@@ -1050,6 +1050,32 @@ get_opclass_input_type(Oid opclass)
 	return result;
 }
 
+/*
+ * get_opclass_family_and_input_type
+ *
+ *		Returns the OID of the operator family the opclass belongs to,
+ *the OID of the datatype the opclass indexes
+ */
+bool
+get_opclass_opfamily_and_input_type(Oid opclass, Oid *opfamily, Oid *opcintype)
+{
+	HeapTuple	tp;
+	Form_pg_opclass cla_tup;
+
+	tp = SearchSysCache1(CLAOID, ObjectIdGetDatum(opclass));
+	if (!HeapTupleIsValid(tp))
+		return false;
+
+	cla_tup = (Form_pg_opclass) GETSTRUCT(tp);
+
+	*opfamily = cla_tup->opcfamily;
+	*opcintype = cla_tup->opcintype;
+
+	ReleaseSysCache(tp);
+
+	return true;
+}
+
 /*-- OPERATOR CACHE --	 */
 
 /*
@@ -3061,3 +3087,45 @@ get_range_subtype(Oid rangeOid)
 	else
 		return InvalidOid;
 }
+
+/*-- PG_INDEX CACHE -- */
+
+/*
+ * get_index_column_opclass
+ *
+ *		Given the index OID and column number,
+ *		return opclass of the index column
+ *			or InvalidOid if the index was not found.
+ */
+Oid
+get_index_column_opclass(Oid index_oid, int attno)
+{
+	HeapTuple	tuple;
+	Form_pg_index rd_index PG_USED_FOR_ASSERTS_ONLY;
+	Datum		datum;
+	bool		isnull;
+	oidvector  *indclass;
+	Oid			opclass;
+
+	/* First we need to know the column's opclass. */
+
+	tuple = SearchSysCache1(INDEXRELID, ObjectIdGetDatum(index_oid));
+	if (!HeapTupleIsValid(tuple))
+		return InvalidOid;
+
+	rd_index = (Form_pg_index) GETSTRUCT(tuple);
+
+	/* caller is supposed to guarantee this */
+	Assert(attno > 0 && attno <= rd_index->indnatts);
+
+	datum = SysCacheGetAttr(INDEXRELID, tuple,
+			Anum_pg_index_indclass, );
+	Assert(!isnull);
+
+	indclass = ((oidvector *) DatumGetPointer(datum));
+	opclass = indclass->values[attno - 1];
+
+	ReleaseSysCache(tuple);
+
+	return opclass;
+}
diff --git a/src/include/utils/lsyscache.h b/src/include/utils/lsyscache.h
index b6d1fca..618c4e8 100644
--- a/src/include/utils/lsyscache.h
+++ b/src/include/utils/lsyscache.h
@@ -73,6 +73,8 @@ extern char *get_constraint_name(Oid conoid);
 extern char *get_language_name(Oid langoid, bool missing_ok);
 extern Oid	get_opclass_family(Oid opclass);
 extern Oid	get_opclass_input_type(Oid opclass);
+extern bool get_opclass_opfamily_and_input_type(Oid opclass,
+	Oid *opfamily, Oid *opcintype);
 extern RegProcedure get_opcode(Oid opno);
 extern char *get_opname(Oid opno);
 extern Oid	get_op_rettype(Oid opno);
@@ -159,6 +161,7 @@ extern void free_attstatsslot(Oid atttype,
 extern char *get_namespace_name(Oid nspid);
 extern 

Re: [HACKERS] Active zombies at AIX

[Konstantin Knizhnik]

I tried to rebuild Postgres without mmap and the problem disappears 
(pgbench with -C doesn't cause connection limit exhaustion any more).
Unfortunately there is no any convenient way to configure PostgreSQL not 
to use mmap.

I have to edit sysv_shmem.c file, commenting

#ifndef EXEC_BACKEND
#define USE_ANONYMOUS_SHMEM
#endif

I wonder why do we prohibit now configuration of Postgres without mmap?


On 06.02.2017 12:47, Konstantin Knizhnik wrote:

Last update on the problem.
Using kdb tool (thank's to Tony Reix for advice and help) we get the 
following trace of Poastgres backend in existing stack trace:


pvthread+073000 STACK:
[005E1958]slock+000578 (005E1958, 80001032 [??])
[9558].simple_lock+58 ()
[00651DBC]vm_relalias+00019C (??, ??, ??, ??, ??)
[006544AC]vm_map_entry_delete+00074C (??, ??, ??)
[00659C30]vm_map_delete+000150 (??, ??, ??, ??)
[00659D88]vm_map_deallocate+48 (??, ??)
[0011C588]kexitx+001408 (??)
[000BB08C]kexit+8C ()
___ Recovery (FFF9290) ___
WARNING: Eyecatcher/version mismatch in RWA


So there seems to be lock contention while unmapping memory segments.
My assumption was that Postgres is detaching all attached segments 
before exit (in shmem_exit callback or earlier).
I have added logging around proc_exit_prepare function (which is 
called by atexit callback) and check that it completes immediately.
So I thought that this vm_map_deallocate can be related with 
deallocation of normal (malloced) memory, because in Linux memory 
allocator may use mmap.

But in AIX it is not true.
Below is report of Bergamini Demien (once again a lot of thanks for 
help with investigation the problem):


The memory allocator in AIX libc does not use mmap and vm_relalias() 
is only called for shared memory mappings.


I talked with the AIX VMM expert at IBM and he said that what you hit 
is one of the most common performance bottlenecks in AIX memory 
management.


He also said that SysV Shared Memory (shmget/shmat) perform better on 
AIX than mmap.


Some improvements have been made in AIX 6.1 (see “perf suffers when 
procs sharing the same segs all exit at once”: 
http://www-01.ibm.com/support/docview.wss?uid=isg1IZ83819) but it does 
not help in your case.


In src/backend/port/sysv_shmem.c, it says that PostgreSQL 9.3 switched 
from using SysV Shared Memory to using mmap.


Maybe you could try to switch back to using SysV Shared Memory on AIX 
to see if it helps performance-wise.


Also, the good news is that there are some restricted tunables in AIX 
that can be tweaked to help different workloads which may have 
different demands.


One of them is relalias_percentage which works with force_relalias_lite:

# vmo -h relalias_percentage

Help for tunable relalias_percentage:

Purpose:

If force_relalias_lite is set to 0, then this specifies the factor 
used in the heuristic to decide whether to avoid locking the source 
mmapped segment or not.


Values:

Default: 0
Range: 0 - 32767
Type: Dynamic
Unit:

Tuning:

This is used when tearing down an mmapped region and is a scalability 
statement, where avoiding the lock may help system throughput, but, in 
some cases, at the cost of more compute time used. If the number of 
pages being unmapped is less than this value divided by 100 and 
multiplied by the total number of pages in memory in the source 
mmapped segment, then the source lock will be avoided. A value of 0 
for relalias_percentage, with force_relalias_lite also set to 0, will 
cause the source segment lock to always be taken. Effective values for 
relalias_percentage will vary by workload, however, a suggested value 
is: 200.



You may also try to play with the munmap_npages vmo tunable.
Your vmo settings for lgpg_size, lgpg_regions and v_pinshm already 
seem correct.



On 24.01.2017 18:08, Konstantin Knizhnik wrote:

Hi hackers,

Yet another story about AIX. For some reasons AIX very slowly 
cleaning zombie processes.
If we launch pgbench with -C parameter then very soon limit for 
maximal number of connections is exhausted.
If maximal number of connection is set to 1000, then after ten 
seconds of pgbench activity we get about 900 zombie processes and it 
takes about 100 seconds (!)

before all of them are terminated.

proctree shows a lot of defunt processes:

[14:44:41]root@postgres:~ # proctree 26084446
26084446 /opt/postgresql/xlc/9.6/bin/postgres -D /postg_fs/postgresql/xlc
4784362 
4980786 
11403448 
11468930 
11993176 
12189710 
12517390 
13238374 
13565974 
13893826 postgres: wal writer process
14024716 
15401000 
...
25691556 

But ps shows that status of process is 

[14:46:02]root@postgres:~ # ps -elk | grep 25691556

  * A - 25691556 - - - - - 


Breakpoint set in reaper() function in postmaster shows that each 
invocation of this functions (called by SIGCHLD handler) proceed 5-10 
PIDS per invocation.
So there are two hypothesis: either AIX is very slowly delivering 
SIGCHLD to parent, either exit of 

Re: [HACKERS] Draft release notes for next week's releases are up for review

[Tobias Bussmann]

Am 04.02.2017 um 18:57 schrieb Tom Lane :
> Right now the question is whether individual items are
> correctly/adequately documented.

> Allow statements prepared with PREPARE to be given parallel plans (Amit 
> Kapila, Tobias Bussman)

another typo taken over from commit log:

s/Tobias Bussman/Tobias Bussmann/

thanks
Tobias 



-- 
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 spelling fixes

[Heikki Linnakangas]

On 02/06/2017 12:52 PM, Josh Soref wrote:

Did you want me to submit emails for the remaining portions from
https://github.com/jsoref/postgres/commits/spelling


Ah, yes please. Post them over and I'll have a look at those as well.

- Heikki



--
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] Partition-wise join for join between (declaratively) partitioned tables

[Ashutosh Bapat]

sent the previous mail before completing my reply. Sorry for that.
Here's the rest of the reply.

>>
>> +   if (bms_num_members(outer_relids) > 1)
>>
>> Seems like bms_get_singleton_member could be used.
>>
>> +* Partitioning scheme in join relation indicates a possibilty that 
>> the
>>
>> Spelling.

Will take care of this.

>>
>> There seems to be no reason for create_partition_plan to be separated
>> from create_plan_recurse.  You can just add another case for the new
>> path type.

Will take care of this.

>>
>> Why does create_partition_join_path need to be separate from
>> create_partition_join_path_with_pathkeys?  Couldn't that be combined
>> into a single function with a pathkeys argument that might sometimes
>> be NIL?  I assume most of the logic is common.

Agreed. will take care of this.

>>
>> From a sort of theoretical standpoint, the biggest danger of this
>> patch seems to be that by deferring path creation until a later stage
>> than normal, we could miss some important processing.
>> subquery_planner() does a lot of stuff after
>> expand_inherited_tables(); if any of those things, especially the ones
>> that happen AFTER path generation, have an effect on the paths, then
>> this code needs to compensate for those changes somehow.  It seems
>> like having the planning of unsampled children get deferred until
>> create_plan() time is awfully surprising; here we are creating the
>> plan and suddenly what used to be a straightforward path->plan
>> translation is running around doing major planning work.  I can't
>> entirely justify it, but I somehow have a feeling that work ought to
>> be moved earlier.  Not sure exactly where.

I agree with this. Probably we should add a path tree mutator before
SS_identify_outer_params() to replace any Partition*Paths with
Merge/Append paths. The mutator will create paths for child-joins
within temporary memory context, copy the relevant paths and create
Merge/Append paths. There are two problems there 1. We have to write
code to copy paths; most of the paths would be flat copy but custom
scan paths might have some unexpected problems. 2. There will be many
surviving PartitionPaths, and all the corresponding child paths would
need copying and consume memory. In order to reduce that consumption,
we have run this mutator after set_cheapest() in subquery_planner();
but then nothing interesting happens between that and create_plan().
Expanding PartitionPaths during create_plan() does not need any path
copying and we expand only the PartitionPaths which will be converted
to plans. That does save a lot of memory; the reason why we defer
creating paths for child-joins.

>>
>> This is not really a full review, mostly because I can't easily figure
>> out the motivation for all of the changes the patch makes.  It makes a
>> lot of changes in a lot of places, and it's not really very easy to
>> understand why those changes are necessary.  My comments above about
>> splitting the patch into a series of patches that can potentially be
>> reviewed and applied independently, with the main patch being the last
>> in the series, are a suggestion as to how to tackle that.  There might
>> be some work that needs to or could be done on the comments, too.  For
>> example, the patch splits out add_paths_to_append_rel from
>> set_append_rel_pathlist, but the comments don't say anything helpful
>> like "we need to do X after Y, because Z".  They just say that we do
>> it.  To some extent I think the comments in the optimizer have that
>> problem generally, so it's not entirely the fault of this patch;
>> still, the lack of those explanations makes the code reorganization
>> harder to follow, and might confuse future patch authors, too.

Specifically about add_paths_to_append_rel(), what do you expect the
comment to say? It would be obvious why we split that functionality
into a separate function: in fact, we don't necessarily explain why
certain code resides in a separate function in the comments. I think,
that particular comment (or for that matter other such comments in the
optimizer) can be removed altogether, since it just writes the
function names as an "English" sentence. I sometimes find those
comments useful, because I can read just those comments and forget
about the code, making comprehension easy. If highlighting is ON, your
brain habitually ignores the non-comment portions when required. I am
open to suggestions.

-- 
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database 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] Possible spelling fixes

[Josh Soref]

Heikki wrote:
‎> I pushed most of these. Except for the below:
> optimisation -> optimization et al.

> Most of our code is written with the American spelling,
> but the British spelling isn't wrong,
> so I don't want to go around changing them all.

Sure

As you'll see, my approach is to aim for consistency. If you used en-GB 99% of 
the time, I'd have offered a change to enforce that. I have a personal 
preference, but there's no obligation, and I understand the potential costs 
churn entails (I see you backported to branches). 

> NUL-terminated -> NULL-terminated

> When we're talking about NUL-terminated strings,
> NUL refers to the NUL ASCII character. NULL usually refers to a NULL pointer.

This wasn't even in my original set (i.e. The dictionary I'm using didn't 
consider NUL to be misspelled). I ran across it while splitting comments out 
per Andres and figured I'd offer it as well. 

> We're probably not consistent about this,

Hrm, I was going to say "that's correct, you aren't", but rereading, I realize 
that I'd have to review every instance in order to prove to myself that 
statement. I could make the weaker argument that "nul-terminated" should be 
changed to either NUL-terminated or null-terminated . My general approach is to 
only make changes when I can detect an inconsistency. And I generally tend 
toward "majority rule".

Here, I think the corpus has 4 spellings, and it sounds like it should only 
have two, but probably (NUL- and null-) not the two I picked (NULL- and null-). 

> but in this context, NUL-terminated isn't wrong, so let's leave them as they 
> are.

But that's OK. My goal in posting these is to encourage people to consider 
their code. 

>> Ooops -> Oops

> "Oops" is more idiomatic, but this doesn't really seem worth changing. 

Technically oops is in dictionaries whereas the other isn't, but I understood 
the intent. 

> Maybe "Ooops" indicates a slightly bigger mistake than "oops" :-)

That seemed like the intent. It's certainly not unreasonable to retain it. It's 
also why I generally offer a queue, so people can reject families of changes. 

>> re-entrancy -> reentrancy

> Googling around, I can see both spellings being used.

Both are used, but reentrancy is in most dictionaries (and encyclopedias) and 
is the form that's used in instruction (certainly it was when I studied in 
university, and it isn't likely to regress). It's akin to email vs e-mail. Once 
the dashless form becomes accepted (within a domain [1]), it's the correct 
form, and the other was merely transitional. 

> "Re-entrancy" actually feels more natural to me, although I'm not sure which 
> is more correct.
> Let's leave them as they are.

Sure 

>> passthru -> passthrough

> "Passthrough" is clearly the correct spelling (or "pass-through"?),

The former is also present in the codebase. (I didn't look for the latter, for 
the same reason as the previous note.)

> but "passthru" seems OK in the context, as an informal shorthand.

My goal is consistency. If you always spell a concept a single way, then 
grepping for that concept is easier and more reliable. 

I personally recognize quite a few flavors, because they're usable for talking 
to Coverity / Purify. 

>> - * Temporay we use TSLexeme.flags for inner use...
>> + * Temporary we use TSLexeme.flags for inner use...

> Looking at the code real quick, I couldn't understand the original 
meaning of this. Is it:
> * DT_USEASIS is a temporary value we use for something. For what?
> * DT_USEASIS is used temporarily for something.
> Does this mean, "temporarily" until we get around to write the code 
> differently, or does 
> it happen temporarily at runtime, or what?

> Just fixing the typo doesn't help much here,
> and I'm not sure if it should be "temporary" or "temporarily" anyway.

Apparently I didn't look at this one much at all. I believe temporarily is the 
intended word (fwiw, I originally mis-corrected directly as directory, that I 
did spot before submitting). And probably as a runtime concept. 

But I'm not volunteering to fix all comments in the project ;-). After spelling 
fixes, I'm more likely to try actual bugs / usability issues. I have a specific 
bug which bit me, but fixing that would require more effort than a spelling 
pass and more cooperation. I tend to do a spelling pass to determine if the 
more expensive activity is viable. So far, the project is welcoming :-) so, 
perhaps I'll manage to write the real fix...

> I wasn't sure if this changes the meaning of the comment slightly.
> An "UPDATE" in all-caps refers to an UPDATE statement,
> is that what's meant here? Or just updating a tuple,
> i.e. should this rather be "skip updating of the tuple" or "skip update of 
> tuple"?

I'm not certain. I do understand that capital UPDATE is special. This one 
people more familiar with the project will have to resolve. 

Fwiw, if it's the former, you could omit the "of".

> This "postsql" refers to the SQL dialect of PostgreSQL,

I had 

Re: [HACKERS] Index corruption with CREATE INDEX CONCURRENTLY

[Martín Marqués]

El 05/02/17 a las 21:57, Tomas Vondra escribió:
> 
> +1 to not rushing fixes into releases. While I think we now finally
> understand the mechanics of this bug, the fact that we came up with
> three different fixes in this thread, only to discover issues with each
> of them, warrants some caution.

I agree also with Robert on not rushing the patch. My point was if we
had to rush the release.

> OTOH I disagree with the notion that bugs that are not driven by user
> reports are somehow less severe. Some data corruption bugs cause quite
> visible breakage - segfaults, immediate crashes, etc. Those are pretty
> clear bugs, and are reported by users.
> 
> Other data corruption bugs are much more subtle - for example this bug
> may lead to incorrect results to some queries, failing to detect values
> violating UNIQUE constraints, issues with foreign keys, etc.

I was recalling just yesterday after sending the mail a logical
replication setup we did on a 9.3 server of a customer which brought up
data inconsistencies on the primary key of one of the tables. The table
had duplicate values.

As Tomas says, it's subtle and hard to find unless you constantly run
index checks (query a sample of the data from the heap and from the
index and check they match). In our case, the customer was not aware of
the dups until we found them.

Regards,

-- 
Martín Marquéshttp://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] Partition-wise join for join between (declaratively) partitioned tables

[Ashutosh Bapat]

On Thu, Feb 2, 2017 at 2:41 AM, Robert Haas  wrote:
> On Mon, Jan 2, 2017 at 7:32 AM, Ashutosh Bapat
>  wrote:
>> PFA the patch (pg_dp_join_v6.patch) with some bugs fixed and rebased
>> on the latest code.
>
> Maybe not surprisingly given how fast things are moving around here
> these days, this needs a rebase.
>
> Apart from that, my overall comment on this patch is that it's huge:
>
>  37 files changed, 7993 insertions(+), 287 deletions(-)
>
> Now, more than half of that is regression test cases and their output,
> which you will certainly be asked to pare down in any version of this
> intended for commit.

Yes. I will work on that once the design and implementation is in
acceptable state. I have already toned down testcases compared to the
previous patch.

> But even excluding those, it's still a fairly
> patch:
>
>  30 files changed, 2783 insertions(+), 272 deletions(-)
>
> I think the reason this is so large is because there's a fair amount
> of refactoring work that has been done as a precondition of the actual
> meat of the patch, and no attempt has been made to separate the
> refactoring work from the main body of the patch.  I think that's
> something that needs to be done.  If you look at the way Amit Langote
> submitted the partitioning patches and the follow-up bug fixes, he had
> a series of patches 0001-blah, 0002-quux, etc. generated using
> format-patch.  Each patch had its own commit message written by him
> explaining the purpose of that patch, links to relevant discussion,
> etc.  If you can separate this into more digestible chunks it will be
> easier to get committed.

I will try to break down the patch into smaller, easy-to-review,
logically cohesive patches.

>
> Other questions/comments:
>
> Why does find_partition_scheme need to copy the partition bound
> information instead of just pointing to it?  Amit went to some trouble
> to make sure that this can't change under us while we hold a lock on
> the relation, and we'd better hold a lock on the relation if we're
> planning a query against it.

PartitionScheme is shared across multiple relations, join or base,
partitioned similarly. Obviously it can't and does not need to point
partition bound informations (which should all be same) of all those
base relations. O the the face of it, it looks weird that it points to
only one of them, mostly the one which it encounters first. But, since
it's going to be the same partition bound information, it doesn't
matter which one. So, I think, we can point of any one of those. Do
you agree?

>
> I think the PartitionScheme stuff should live in the optimizer rather
> that src/backend/catalog/partition.c.  Maybe plancat.c?  Perhaps we
> eventually need a new file in the optimizer just for partitioning
> stuff, but I'm not sure about that yet.

I placed PartitionScheme stuff in partition.c because most of the
functions and structures in partition.c are not visible outside that
file. But I will try again to locate PartitionScheme to optimizer.

>
> The fact that set_append_rel_size needs to reopen the relation to
> extract a few more bits of information is not desirable.  You need to
> fish this information through in some other way; for example, you
> could have get_relation_info() stash the needed bits in the
> RelOptInfo.

I considered this option and discarded it, since not all partitioned
relations will have OIDs for partitions e.g. partitioned joins will
not have OIDs for their partitions. But now that I think of it, we
should probably store those OIDs just for the base relation and leave
them unused for non-base relations just like other base relation
specific fields in RelOptInfo.

>
> +* For two partitioned tables with the same
> partitioning scheme, it is
> +* assumed that the Oids of matching partitions from
> both the tables
> +* are placed at the same position in the array of
> partition oids in
>
> Rather than saying that we assume this, you should say why it has to
> be true.  (If it doesn't have to be true, we shouldn't assume it.)

Will take care of this.

>
> +* join relations. Partition tables should have same
> layout as the
> +* parent table and hence should not need any
> translation. But rest of
>
> The same attributes have to be present with the same types, but they
> can be rearranged.  This comment seems to imply the contrary.

Hmm, will take care of this.

>
> FRACTION_PARTS_TO_PLAN seems like it should be a GUC.

+1. Will take care of this. Does "representative_partitions_fraction"
or "sample_partition_fraction" look like a good GUC name? Any other
suggestions?

>
> +   /*
> +* Add this relation to the list of samples ordered by
> the increasing
> +* number of rows at appropriate place.
> +*/
> +   foreach (lc, ordered_child_nos)
> +   {
> +

Re: [HACKERS] Active zombies at AIX

[Konstantin Knizhnik]

Last update on the problem.
Using kdb tool (thank's to Tony Reix for advice and help) we get the 
following trace of Poastgres backend in existing stack trace:


pvthread+073000 STACK:
[005E1958]slock+000578 (005E1958, 80001032 [??])
[9558].simple_lock+58 ()
[00651DBC]vm_relalias+00019C (??, ??, ??, ??, ??)
[006544AC]vm_map_entry_delete+00074C (??, ??, ??)
[00659C30]vm_map_delete+000150 (??, ??, ??, ??)
[00659D88]vm_map_deallocate+48 (??, ??)
[0011C588]kexitx+001408 (??)
[000BB08C]kexit+8C ()
___ Recovery (FFF9290) ___
WARNING: Eyecatcher/version mismatch in RWA


So there seems to be lock contention while unmapping memory segments.
My assumption was that Postgres is detaching all attached segments 
before exit (in shmem_exit callback or earlier).
I have added logging around proc_exit_prepare function (which is called 
by atexit callback) and check that it completes immediately.
So I thought that this vm_map_deallocate can be related with 
deallocation of normal (malloced) memory, because in Linux memory 
allocator may use mmap.

But in AIX it is not true.
Below is report of Bergamini Demien (once again a lot of thanks  for 
help with investigation the problem):


The memory allocator in AIX libc does not use mmap and vm_relalias() is 
only called for shared memory mappings.


I talked with the AIX VMM expert at IBM and he said that what you hit is 
one of the most common performance bottlenecks in AIX memory management.


He also said that SysV Shared Memory (shmget/shmat) perform better on 
AIX than mmap.


Some improvements have been made in AIX 6.1 (see “perf suffers when 
procs sharing the same segs all exit at once”: 
http://www-01.ibm.com/support/docview.wss?uid=isg1IZ83819) but it does 
not help in your case.


In src/backend/port/sysv_shmem.c, it says that PostgreSQL 9.3 switched 
from using SysV Shared Memory to using mmap.


Maybe you could try to switch back to using SysV Shared Memory on AIX to 
see if it helps performance-wise.


Also, the good news is that there are some restricted tunables in AIX 
that can be tweaked to help different workloads which may have different 
demands.


One of them is relalias_percentage which works with force_relalias_lite:

# vmo -h relalias_percentage

Help for tunable relalias_percentage:

Purpose:

If force_relalias_lite is set to 0, then this specifies the factor used 
in the heuristic to decide whether to avoid locking the source mmapped 
segment or not.


Values:

Default: 0
Range: 0 - 32767
Type: Dynamic
Unit:

Tuning:

This is used when tearing down an mmapped region and is a scalability 
statement, where avoiding the lock may help system throughput, but, in 
some cases, at the cost of more compute time used. If the number of 
pages being unmapped is less than this value divided by 100 and 
multiplied by the total number of pages in memory in the source mmapped 
segment, then the source lock will be avoided. A value of 0 for 
relalias_percentage, with force_relalias_lite also set to 0, will cause 
the source segment lock to always be taken. Effective values for 
relalias_percentage will vary by workload, however, a suggested value 
is: 200.



You may also try to play with the munmap_npages vmo tunable.
Your vmo settings for lgpg_size, lgpg_regions and v_pinshm already seem 
correct.



On 24.01.2017 18:08, Konstantin Knizhnik wrote:

Hi hackers,

Yet another story about AIX. For some reasons AIX very slowly cleaning 
zombie processes.
If we launch pgbench with -C parameter then very soon limit for 
maximal number of connections is exhausted.
If maximal number of connection is set to 1000, then after ten seconds 
of pgbench activity we get about 900 zombie processes and it takes 
about 100 seconds (!)

before all of them are terminated.

proctree shows a lot of defunt processes:

[14:44:41]root@postgres:~ # proctree 26084446
26084446 /opt/postgresql/xlc/9.6/bin/postgres -D /postg_fs/postgresql/xlc
4784362 
4980786 
11403448 
11468930 
11993176 
12189710 
12517390 
13238374 
13565974 
13893826 postgres: wal writer process
14024716 
15401000 
...
25691556 

But ps shows that status of process is 

[14:46:02]root@postgres:~ # ps -elk | grep 25691556

  * A - 25691556 - - - - - 


Breakpoint set in reaper() function in postmaster shows that each 
invocation of this functions (called by SIGCHLD handler) proceed 5-10 
PIDS per invocation.
So there are two hypothesis: either AIX is very slowly delivering 
SIGCHLD to parent, either exit of process takes too much time.


The fact the backends are in exiting state makes second hypothesis 
more reliable.
We have tried different Postgres configurations with local and TCP 
sockets, with different amount of shared buffers and built both with 
gcc and xlc.

In all cases behavior is similar: zombies do not want to die.
As far as it is not possible to attach debugger to defunct process, it 
is not clear how to understand what's going on.


I 

Re: [HACKERS] Variable name typo in launcher.c

[Heikki Linnakangas]

On 02/05/2017 01:05 AM, Masahiko Sawada wrote:

I think "laucher" should be "launcher". Attached patch fixes it.


Fixed, thanks!

- Heikki



--
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 spelling fixes

[Heikki Linnakangas]

On 02/06/2017 04:50 AM, Josh Soref wrote:

It's now split more or less to your suggestion:
https://github.com/jsoref/postgres/commits/spelling


Thanks!

I pushed most of these. Except for the below:


optimisation -> optimization et al.


Most of our code is written with the American spelling, but the British 
spelling isn't wrong, so I don't want to go around changing them all.



NUL-terminated -> NULL-terminated


When we're talking about NUL-terminated strings, NUL refers to the NUL 
ASCII character. NULL usually refers to a NULL pointer. We're probably 
not consistent about this, but in this context, NUL-terminated isn't 
wrong, so let's leave them as they are.



Ooops -> Oops


"Oops" is more idiomatic, but this doesn't really seem worth changing. 
Maybe "Ooops" indicates a slightly bigger mistake than "oops" :-)



re-entrancy -> reentrancy


Googling around, I can see both spellings being used. "Re-entrancy" 
actually feels more natural to me, although I'm not sure which is more 
correct. Let's leave them as they are.



passthru -> passthrough


"Passthrough" is clearly the correct spelling (or "pass-through"?), but 
"passthru" seems OK in the context, as an informal shorthand.



--- a/src/backend/tsearch/dict_thesaurus.c
+++ b/src/backend/tsearch/dict_thesaurus.c
@@ -23,7 +23,7 @@


 /*
- * Temporay we use TSLexeme.flags for inner use...
+ * Temporary we use TSLexeme.flags for inner use...
  */
 #define DT_USEASIS 0x1000


Looking at the code real quick, I couldn't understand the original 
meaning of this. Is it:


* DT_USEASIS is a temporary value we use for something. For what?

* DT_USEASIS is used temporarily for something. Does this mean, 
"temporarily" until we get around to write the code differently, or does 
it happen temporarily at runtime, or what?


Just fixing the typo doesn't help much here, and I'm not sure if it 
should be "temporary" or "temporarily" anyway.



--- a/contrib/spi/timetravel.c
+++ b/contrib/spi/timetravel.c
@@ -51,7 +51,7 @@ static EPlan *find_plan(char *ident, EPlan **eplan, int 
*nplans);
  * and stop_date eq INFINITY [ and update_user eq current 
user ]
  * and all other column values as in new tuple, and insert 
tuple
  * with old data and stop_date eq current date
- * ELSE - skip updation of tuple.
+ * ELSE - skip UPDATE of tuple.
  * 2.  IF a delete affects tuple with stop_date eq INFINITY
  * then insert the same tuple with stop_date eq current 
date
  * [ and delete_user eq current user ]


I wasn't sure if this changes the meaning of the comment slightly. An 
"UPDATE" in all-caps refers to an UPDATE statement, is that what's meant 
here? Or just updating a tuple, i.e. should this rather be "skip 
updating of the tuple" or "skip update of tuple"?



--- a/src/test/regress/sql/errors.sql
+++ b/src/test/regress/sql/errors.sql
@@ -2,7 +2,7 @@
 -- ERRORS
 --

--- bad in postquel, but ok in postsql
+-- bad in postquel, but ok in PostgreSQL
 select 1;


This "postsql" refers to the SQL dialect of PostgreSQL, rather than 
PostgreSQL the project. I don't remember seeing it called "postsql" 
anywhere else, though. We hardly care about what was an error in 
postqual anyway, though, so perhaps this should be rewritten into 
something else entirely, like "This is not allowed by the SQL standard, 
but ok on PostgreSQL" (assuming that's correct, I'm not 100% sure). Or 
just leave it alone.


Thanks for the fixes! I was particularly impressed that you caught the 
typo in Marcel Kornacker's surname.


- Heikki


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

Re: \if, \elseif, \else, \endif (was Re: [HACKERS] PSQL commands: \quit_if, \quit_unless)

[Fabien COELHO]

Find attached a small patch to improve tap tests, which also checks that psql 
really exited by checking that nothing is printed afterwards.


. It is better with the attachement.

--
Fabien.diff --git a/src/bin/psql/t/001_if.pl b/src/bin/psql/t/001_if.pl
index 68c9b15..a703cab 100644
--- a/src/bin/psql/t/001_if.pl
+++ b/src/bin/psql/t/001_if.pl
@@ -4,7 +4,7 @@ use warnings;
 use Config;
 use PostgresNode;
 use TestLib;
-use Test::More tests => 15;
+use Test::More tests => 18;
 
 #
 # test invalid \if respects ON_ERROR_STOP
@@ -14,12 +14,14 @@ $node->init;
 $node->start;
 
 my $tests = [
-	[ "\\if invalid_expression\n\\endif\n", '', 'boolean expected', 'syntax error' ],
+# syntax errors
+	[ "\\if invalid\n\\echo NO\n\\endif\n\\echo NO\n", '', 'boolean expected', 'syntax error' ],
+	[ "\\if false\n\\elif invalid\n\\echo NO\n\\endif\n\\echo NO\n", '', 'boolean expected', 'syntax error' ],
 	# unmatched checks
-	[ "\\if true\\n", '', 'found EOF before closing.*endif', 'unmatched \if' ],
-	[ "\\elif true\\n", '', 'encountered un-matched.*elif', 'unmatched \elif' ],
-	[ "\\else\\n", '', 'encountered un-matched.*else', 'unmatched \else' ],
-	[ "\\endif\\n", '', 'encountered un-matched.*endif', 'unmatched \endif' ],
+	[ "\\if true\n", '', 'found EOF before closing.*endif', 'unmatched \if' ],
+	[ "\\elif true\n\\echo NO\n", '', 'encountered un-matched.*elif', 'unmatched \elif' ],
+	[ "\\else\n\\echo NO\n", '', 'encountered un-matched.*else', 'unmatched \else' ],
+	[ "\\endif\n\\echo NO\n", '', 'encountered un-matched.*endif', 'unmatched \endif' ],
 ];
 
 # 3 checks per tests
@@ -29,7 +31,7 @@ for my $test (@$tests) {
   my $retcode = $node->psql('postgres', $script,
 		stdout => \$stdout, stderr => \$stderr,
 		on_error_stop => 1);
-  is($retcode,'3',"$name test respects ON_ERROR_STOP");
+  is($retcode,'3',"$name test ON_ERROR_STOP");
   is($stdout, $stdout_expect, "$name test STDOUT");
   like($stderr, qr/$stderr_re/, "$name test STDERR");
 }

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

Re: \if, \elseif, \else, \endif (was Re: [HACKERS] PSQL commands: \quit_if, \quit_unless)

[Fabien COELHO]

  # elif error
  "\\if false\n\\elif error\n\\endif\n"

  # ignore commands on error (stdout must be empty)
  "\\if error\n\\echo NO\n\\else\n\\echo NO\n\\endif\n"


Those are already in the regression (around line 2763 of
expected/psql.out), are you saying we should have them in TAP as well?
Should we only do TAP tests?


Ok. so, maybe just the first one. The idea would be to cover more cases of 
on error stop and check that it indeed stopped.


Find attached a small patch to improve tap tests, which also checks that 
psql really exited by checking that nothing is printed afterwards.


Also, for some reason there were \\n instead of \n in some place, it was 
working because the first command induced the error.



Anyway, here's the Ctrl-C behavior:


Ok. Basically it moves up each time Ctrl-C is called. Fine.

The future improvement would be to do that if the current input line was 
empty, otherwise only the current input line would be cleaned up.



Ctrl-C exits do the same before/after state checks that \endif does, the
lone difference being that it "escaped" the \if rather than "exited" the
\if. Thanks to Daniel for pointing out where it should be handled, because
I wasn't going to figure that out on my own.

v7's only major difference from v6 is the Ctrl-C branch escaping.


Ok. Bar from minor tests improvements, this looks pretty much ok to me.

--
Fabien.


--
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] contrib modules and relkind check

[Amit Langote]

On 2017/01/24 15:35, Amit Langote wrote:
> On 2017/01/24 15:11, Michael Paquier wrote:
>> On Tue, Jan 24, 2017 at 2:14 PM, Amit Langote
>>  wrote:
>>> Some contrib functions fail to fail sooner when relations of unsupported
>>> relkinds are passed, resulting in error message like one below:
>>>
>>> create table foo (a int);
>>> create view foov as select * from foo;
>>> select pg_visibility('foov', 0);
>>> ERROR:  could not open file "base/13123/16488": No such file or directory
>>>
>>> Attached patch fixes that for all such functions I could find in contrib.
>>>
>>> It also installs RELKIND_PARTITIONED_TABLE as unsupported in a couple of
>>> places (in pageinspect and pgstattuple).
>>
>> I have spent some time looking at your patch, and did not find any
>> issues with it, nor did I notice code paths that were not treated or
>> any other contrib modules sufferring from the same deficiencies that
>> you may have missed. Nice work.
> 
> Thanks for the review, Michael!

Added to the next CF, just so someone can decide to pick it up later.

https://commitfest.postgresql.org/13/988/

Thanks,
Amit




-- 
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 corruption with CREATE INDEX CONCURRENTLY

[Vladimir Borodin]

> 6 февр. 2017 г., в 4:57, Peter Geoghegan  написал(а):
> 
> I meant that I find the fact that there were no user reports in all
> these years to be a good reason to not proceed for now in this
> instance.

Well, we had some strange situations with indexes (see below for example) but I 
couldn’t even think that CIC is the problem.

And it is really difficult to give diagnostics for problems of such kind. 
Because 1. you may see the problem several days after last major change in the 
database and 2. you don’t even know how to start investigating the problem.

xdb314g/maildb M # show enable_indexscan ;
 enable_indexscan
--
 off
(1 row)

Time: 0.120 ms
xdb314g/maildb M #  select * from mail.folders where uid=448300241 and fid=1;
-[ RECORD 1 ]---+--
uid | 448300241
fid | 1
<...>

Time: 30398.637 ms
xdb314g/maildb M # set enable_indexscan to true;
SET
Time: 0.111 ms
xdb314g/maildb M #  select * from mail.folders where uid=448300241 and fid=1;
(0 rows)

Time: 0.312 ms

xdb314g/maildb M #

The row of course hasn’t been deleted.

--
May the force be with you…
https://simply.name