Re: [HACKERS] snapbuild woes

[Noah Misch]

On Thu, Apr 27, 2017 at 09:42:58PM -0700, Andres Freund wrote:
> 
> 
> On April 27, 2017 9:34:44 PM PDT, Noah Misch  wrote:
> >On Fri, Apr 21, 2017 at 10:36:21PM -0700, Andres Freund wrote:
> >> On 2017-04-17 21:16:57 -0700, Andres Freund wrote:
> >> > I've since the previous update reviewed Petr's patch, which he
> >since has
> >> > updated over the weekend.  I'll do another round tomorrow, and will
> >see
> >> > how it looks.  I think we might need some more tests for this to be
> >> > committable, so it might not become committable tomorrow.  I hope
> >we'll
> >> > have something in tree by end of this week, if not I'll send an
> >update.
> >> 
> >> I was less productive this week than I'd hoped, and creating a
> >testsuite
> >> was more work than I'd anticipated, so I'm slightly lagging behind. 
> >I
> >> hope to have a patchset tomorrow, aiming to commit something
> >> Monday/Tuesday.
> >
> >This PostgreSQL 10 open item is past due for your status update. 
> >Kindly send
> >a status update within 24 hours, and include a date for your subsequent
> >status
> >update.  Refer to the policy on open item ownership:
> >https://www.postgresql.org/message-id/20170404140717.GA2675809%40tornado.leadboat.com
> 
> I committed part of the series today, plan to continue doing so over the next 
> few days.  Changes require careful review & testing, this is easy to get 
> wrong...

This PostgreSQL 10 open item is past due for your status update.  Kindly send
a status update within 24 hours, and include a date for your subsequent status
update.

Also, this open item has been alive for three weeks, well above guideline.  I
understand it's a tricky bug, but I'm worried this isn't on track to end.
What is missing to make it end?

Refer to the policy on open item ownership:
https://www.postgresql.org/message-id/20170404140717.GA2675809%40tornado.leadboat.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] [POC] hash partitioning

[Jeff Davis]

On Tue, May 2, 2017 at 7:01 PM, Robert Haas  wrote:
> On Tue, May 2, 2017 at 9:01 PM, Jeff Davis  wrote:
>> 1. Consider a partition-wise join of two hash-partitioned tables. If
>> that's a hash join, and we just use the hash opclass, we immediately
>> lose some useful bits of the hash function. Same for hash aggregation
>> where the grouping key is the partition key.
>
> Hmm, that could be a problem in some cases.  I think there's probably
> much less of a problem if the modulus isn't a power of two?

That's true, but it's awkward to describe that to users. And I think
most people would be inclined to use a power-of-two number of
partitions, perhaps coming from other systems.

>> To fix this, I think we need to include a salt in the hash API. Each
>> level of hashing can choose a random salt.
>
> Do you mean that we'd salt partitioning hashing differently from
> grouping hashing which would be salted different from aggregation
> hashing which, I suppose, would be salted differently from hash index
> hashing?

Yes. The way I think about it is that choosing a new random salt is an
easy way to get a new hash function.

> Or do you mean that you'd have to specify a salt when
> creating a hash-partitioned table, and make sure it's the same across
> all compatibly partitioned tables you might want to hash-join?  That
> latter sounds unappealing.

I don't see a reason to expose the salt to users. If we found a reason
in the future, we could, but it would create all of the problems you
are thinking about.

>> 2. Consider a partition-wise join where the join keys are varchar(10)
>> and char(10). We can't do that join if we just use the existing hash
>> strategy, because 'foo' = 'foo   ' should match, but those values
>> have different hashes when using the standard hash opclass.

...

> You're basically describing what a hash opfamily already does, except
> that we don't have a single opfamily that covers both varchar(10) and
> char(10), nor do we have one that covers both int and numeric.  We
> have one that covers int2, int4, and int8, though.  If somebody wanted
> to make the ones you're suggesting, there's nothing preventing it,
> although I'm not sure exactly how we'd encourage people to start using
> the new one and deprecating the old one.  We don't seem to have a good
> infrastructure for that.

OK. I will propose new hash opfamilies for varchar/bpchar/text,
int2/4/8/numeric, and timestamptz/date.

One approach is to promote the narrower type to the wider type, and
then hash. The problem is that would substantially slow down the
hashing of integers, so then we'd need to use one hash opfamily for
partitioning and one for hashjoin, and it gets messy.

The other approach is to check if the wider type is within the domain
of the narrower type, and if so, *demote* the value and then hash. For
instance, '4.2'::numeric would hash the same as it does today, but
'4'::numeric would hash as an int2. I prefer this approach, and int8
already does something resembling it.

For timestamptz/date, it's not nearly as important.

>> My opinion is that we should work on this hashing infrastructure
>> first, and then support the DDL. If we get the hash functions right,
>> that frees us up to create better plans, with better push-downs, which
>> will be good for parallel query.
>
> I am opposed to linking the fate of this patch to multiple
> independent, possibly large, possibly difficult, possibly
> controversial enhancements to the hashing mechanism.

It's a little early in the v11 cycle to be having this argument.
Really what I'm saying is that a small effort now may save us a lot of
headache later.

Regards,
 Jeff Davis


-- 
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] WITH clause in CREATE STATISTICS

[Simon Riggs]

On 3 May 2017 at 23:31, Andrew Dunstan  wrote:

>> It also seems like we don't need to have *both* fully-reserved keywords
>> introducing each clause *and* parentheses around the lists.  Maybe
>> dropping the parens around the stats-types list and the column-names
>> list would help to declutter?  (But I'd keep parens around the WITH
>> options, for consistency with other statements.)

+1

>> One other point is that as long as we've got reserved keywords introducing
>> each clause, there isn't actually an implementation reason why we couldn't
>> accept the clauses in any order.  Not sure I want to document it that way,
>> but it might not be a bad thing if the grammar was forgiving about whether
>> you write the USING or ON part first ...
>
> +1 for allowing arbitrary order of clauses.

+1

> I would document it with the
> USING clause at the end, and have that be what psql supports and pg_dump
> produces. Since there are no WITH options now we should leave that out
> until it's required.

Let's record the target syntax in parser comments so we can just slot
things in when needed later, without rediscussion.

-- 
Simon Riggshttp://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] PROVE_FLAGS

[Stephen Frost]

* Tom Lane (t...@sss.pgh.pa.us) wrote:
> Andrew Dunstan  writes:
> > Can someone please explain to me why we have this in Makefile.global.in?
> > (from commit e9c81b60 )
> > PROVE_FLAGS =
> 
> Before that commit it was like
> 
>   PROVE_FLAGS = --verbose

right.

> which had some value.  I agree that now we'd be better off to not
> set it at all, especially since the convention now appears to be that
> automatically-supplied prove options should be set in PG_PROVE_FLAGS.

Good point.

> I'd suggest that the comment just above be replaced by something like
> 
> # User-supplied prove flags can be provided in PROVE_FLAGS.

Works for me.

Thanks!

Stephen


signature.asc
Description: Digital signature

Re: [HACKERS] GCC 7 warnings

[Tom Lane]

Peter Eisentraut  writes:
> On 4/10/17 11:03, Peter Eisentraut wrote:
>> The release of GCC 7 is approaching [0], and the number of warnings in
>> PostgreSQL has gone up since we last looked [1].  Output attached.  (My
>> version is 7.0.1 20170408.)

> GCC 7 has been released.

> Should we backpatch these warning fixes?  The commit in question is
> 6275f5d28a1577563f53f2171689d4f890a46881.  (I haven't actually checked
> whether backpatches well.)

Seems like that patch represents generally better coding practice,
and applying it would reduce cross-branch differences that would be
hazards for future patches, so I'm mostly +1 for this.

But I'd suggest waiting till after next week's releases.  If there
are any problems induced by this, we'd be more likely to find them
with another three months' time before it hits the wild.

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] pg_dump emits ALTER TABLE ONLY partitioned_table

[Stephen Frost]

Amit,

* Amit Langote (amitlangot...@gmail.com) wrote:
> On Wed, May 3, 2017 at 12:05 PM, Stephen Frost  wrote:
> > * Amit Langote (langote_amit...@lab.ntt.co.jp) wrote:
> >> Attached updated patches.
> >
> > Please find an updated version which corrects the issue with
> > binary-upgrade of partitioned tables having partitions in other schemas,
> > along with a few other minor improvements.
> >
> > If you could take a look at it, I'd appreciate it.  We already had a
> > test case in the pg_dump TAP tests for partitions existing in a schema
> > different from the partitioned table, but we weren't checking the
> > binary-upgrade case, so I've added a check to do that now.  I'm sure
> > additional tests would be good to add and will take a look at doing that
> > tomorrow, but this hopefully closes at least the latest issue.
> >
> > Assuming this looks good to you, I'll push it tomorrow, possibly with
> > other minor adjustments and perhaps a few more tests.
> 
> Your latest patch looks good to me.

Ok, great, thanks for taking a look at it.  Too late for me to push it
tonight, so I'll do so tomorrow.

Thanks again!

Stephen


signature.asc
Description: Digital signature

Re: [HACKERS] pg_dump emits ALTER TABLE ONLY partitioned_table

[Amit Langote]

Hi Stephen,

On Wed, May 3, 2017 at 12:05 PM, Stephen Frost  wrote:
> Amit,
>
> * Amit Langote (langote_amit...@lab.ntt.co.jp) wrote:
>> Attached updated patches.
>
> Please find an updated version which corrects the issue with
> binary-upgrade of partitioned tables having partitions in other schemas,
> along with a few other minor improvements.
>
> If you could take a look at it, I'd appreciate it.  We already had a
> test case in the pg_dump TAP tests for partitions existing in a schema
> different from the partitioned table, but we weren't checking the
> binary-upgrade case, so I've added a check to do that now.  I'm sure
> additional tests would be good to add and will take a look at doing that
> tomorrow, but this hopefully closes at least the latest issue.
>
> Assuming this looks good to you, I'll push it tomorrow, possibly with
> other minor adjustments and perhaps a few more tests.

Your latest patch looks good to me.

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

[HACKERS] Reducing runtime of stats regression test

[Tom Lane]

On a reasonably fast development machine, one of the biggest time sinks
while running the core regression tests is the long "sleep" calls in the
stats.sql regression test.  I took a closer look at these, and I think
we could basically get rid of them.

First up is this bit at the beginning of that test script:

-- wait to let any prior tests finish dumping out stats;
-- else our messages might get lost due to contention
SELECT pg_sleep_for('2 seconds');

The stated concern isn't really all that plausible, since even if we
launch a batch of test scripts at once, they don't all finish at once,
so there's unlikely to be a big pileup of traffic to the stats collector.
But we don't have to take that on faith: in assert-enabled builds,
pgstat_send() logs any failure to send a stats message.

I have grepped the buildfarm logs for "could not send to statistics
collector" log messages during the "make check" stage (a total of 754313
runs dating back ten years).  What I find is that members mereswine and
gull occasionally report "Network is down", and a few times currawong and
thrips have complained "Invalid argument", and there are exactly no other
such messages.  In particular there are no EAGAIN or EWOULDBLOCK failures
that would suggest congestion on the stats collector's input.  So this
is basically not something that happens at all in the regression tests,
let alone during startup of the stats test in particular.

Now, another failure mechanism that could conceivably be ameliorated
by this initial wait is if one of the immediately preceding runs has
performed a scan on tenk2 but doesn't manage to transmit that info before
stats.sql creates its initial copy of the stats counts.  Then when that
count does get sent, it could look like one triggered by stats.sql itself,
fooling the test.  But that seems rather unlikely, because tenk2 isn't
touched by very many test scripts.  And even if it did happen that would
not cause an observed test failure; at worst it would obscure a failure
that we should have detected.  I'm doubtful that this is worth losing any
sleep over.

In short, I think we could just drop the above wait altogether,
and be no worse off.  The only useful thing it's doing for us is
exercising pg_sleep_for(), which is otherwise untested ... but we
can transfer that responsibility into wait_for_stats().

The other significant delay in stats.sql is

-- force the rate-limiting logic in pgstat_report_stat() to time out
-- and send a message
SELECT pg_sleep(1.0);

Now, we do seem to need a delay there, because the rate-limiting logic
is unlikely to have permitted the count from the immediately preceding
statement to have gotten sent right then, and the count won't get
sent at all while we're inside wait_for_stats (since backends only
send stats just before going idle).  But there's more than one way
to skin this cat.  We can just start a new connection with \c, and
let wait_for_stats wait for the old one to send its stats before quitting.
Even on my oldest and slowest buildfarm machines, starting a new session
takes well under one second.

In short then, I propose the attached patch, which reduces the
runtime of stats.sql by a shade under 3 seconds.  Given that the
runtime of "make installcheck-parallel" is circa 15-17 seconds on
typical current hardware, that's a nice gain.

I'm not sure about backpatching.  I think developers mostly only
care about regression tests on HEAD, and the savings is relatively
less exciting on the buildfarm, since it doesn't get any bigger on
slower machines.  Thoughts?

regards, tom lane

diff --git a/src/test/regress/expected/stats.out b/src/test/regress/expected/stats.out
index a811265..91c061b 100644
*** a/src/test/regress/expected/stats.out
--- b/src/test/regress/expected/stats.out
*** SET enable_seqscan TO on;
*** 16,31 
  SET enable_indexscan TO on;
  -- for the moment, we don't want index-only scans here
  SET enable_indexonlyscan TO off;
- -- wait to let any prior tests finish dumping out stats;
- -- else our messages might get lost due to contention
- SELECT pg_sleep_for('2 seconds');
-  pg_sleep_for 
- --
-  
- (1 row)
- 
  -- save counters
! CREATE TEMP TABLE prevstats AS
  SELECT t.seq_scan, t.seq_tup_read, t.idx_scan, t.idx_tup_fetch,
 (b.heap_blks_read + b.heap_blks_hit) AS heap_blks,
 (b.idx_blks_read + b.idx_blks_hit) AS idx_blks,
--- 16,23 
  SET enable_indexscan TO on;
  -- for the moment, we don't want index-only scans here
  SET enable_indexonlyscan TO off;
  -- save counters
! CREATE TABLE prevstats AS
  SELECT t.seq_scan, t.seq_tup_read, t.idx_scan, t.idx_tup_fetch,
 (b.heap_blks_read + b.heap_blks_hit) AS heap_blks,
 (b.idx_blks_read + b.idx_blks_hit) AS idx_blks,
*** begin
*** 66,72 
  exit when updated1 and updated2 and updated3;
  
  -- wait a little
! perform pg_sleep(0.1);
  
  -- reset stats snapshot so we can test again
  

Re: [HACKERS] [BUGS] Concurrent ALTER SEQUENCE RESTART Regression

[Peter Eisentraut]

On 4/30/17 04:05, Noah Misch wrote:
> The above-described topic is currently a PostgreSQL 10 open item.  Peter,
> since you committed the patch believed to have created it, you own this open
> item.  If some other commit is more relevant or if this does not belong as a
> v10 open item, please let us know.  Otherwise, please observe the policy on
> open item ownership[1] and send a status update within three calendar days of
> this message.  Include a date for your subsequent status update.  Testers may
> discover new open items at any time, and I want to plan to get them all fixed
> well in advance of shipping v10.  Consequently, I will appreciate your efforts
> toward speedy resolution.  Thanks.

I'm working on this and will report on Friday.

-- 
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] statement_timeout is not working as expected with postgres_fdw

[Robert Haas]

On Wed, May 3, 2017 at 3:49 PM, Robert Haas  wrote:
> It seems pretty clear to me that we are not going to fix all of these
> issues in one patch.  Here's a sketch of an idea for how to start
> making things better:

Patch attached.

> - Add an in_abort_cleanup flag to ConnCacheEntry.

Ended up renaming this to abort_cleanup_incomplete.

> - Before pgfdw_xact_callback() or pgfdw_subxact_callback() begin abort
> cleanup, check whether the flag is set.  If so, slam the connection
> shut unless that's already been done; furthermore, if the flag is set
> and we're in pgfdw_xact_callback (i.e. this is a toplevel abort),
> forget about the connection entry entirely.  On the other hand, if the
> flag is not set, set it flag and attempt abort cleanup.  If we
> succeed, clear the flag.

Did approximately this.  It turned out not to be necessary to add any
new calls to PQfinish(); the existing one was adequate.

> - Before pgfdw_xact_callback() or pgfdw_subxact_callback() begin
> pre-commit processing, check whether the flag is set.  If so, throw an
> ERROR, so that we switch over to abort processing.

Did this.

> - Change uses of PQexec() in the abort path to use pgfdw_exec_query()
> instead.  If we exit pgfdw_exec_query() with an error, we'll re-enter
> the abort path, but now in_abort_cleanup will be set, so we'll just
> drop the connection (and force any outer transaction levels to abort
> as well).

Created a new function pgfdw_exec_cleanup_query() for this, also
incorporating a timeout.

Also fixed things so that after issuing PQcancel() we actually throw
away the pending result from the cancelled query, and added some error
recursion protection.

Review would be appreciated.

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


improve-pgfdw-abort-behavior-v1.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] GCC 7 warnings

[Peter Eisentraut]

On 4/10/17 11:03, Peter Eisentraut wrote:
> The release of GCC 7 is approaching [0], and the number of warnings in
> PostgreSQL has gone up since we last looked [1].  Output attached.  (My
> version is 7.0.1 20170408.)

GCC 7 has been released.

Should we backpatch these warning fixes?  The commit in question is
6275f5d28a1577563f53f2171689d4f890a46881.  (I haven't actually checked
whether backpatches well.)

PG 9.2 and newer compile warning-free with GCC 6, so there would be some
value in preserving this with GCC 7.

-- 
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] Re: [BUGS] BUG #14634: On Windows pg_basebackup should write tar to stdout in binary mode

[Haribabu Kommi]

On Wed, May 3, 2017 at 10:44 PM, Ashutosh Sharma 
wrote:

> Hi Craig,
>
> On Wed, May 3, 2017 at 10:50 AM, Craig Ringer 
> wrote:
> > On 3 May 2017 at 12:32, Haribabu Kommi  wrote:
> >> [Adding -hackers mailing list]
> >>
> >> On Fri, Apr 28, 2017 at 6:28 PM,  wrote:
> >>>
> >>> The following bug has been logged on the website:
> >>>
> >>> Bug reference:  14634
> >>> Logged by:  Henry Boehlert
> >>> Email address:  henry_boehl...@agilent.com
> >>> PostgreSQL version: 9.6.2
> >>> Operating system:   Windows Server 2012 R2 6.3.9600
> >>> Description:
> >>>
> >>> Executing command pg_basebackup -D -F t writes its output to stdout,
> which
> >>> is open in text mode, causing LF to be converted to CR LF thus
> corrupting
> >>> the resulting archive.
> >>>
> >>> To write the tar to stdout, on Windows stdout's mode should be
> temporarily
> >>> switched to binary.
> >>>
> >>> https://msdn.microsoft.com/en-us/library/tw4k6df8.aspx
> >>
> >>
> >> Thanks for reporting the issue.
> >> With the attached patch, I was able to extract the tar file that gets
> >> generated when the tar file is written into stdout. I tested the
> >> the compressed tar also.
> >>
> >> This bug needs to be fixed in back branches also.
> >
> > We should do the same for pg_dump in -Fc mode.
>
> Did you meant -Fp mode. I think we are already setting stdout file to
> binary mode if the output format is custom. Please refer to the
> following code in parseArchiveFormat() and _allocAH() respectively
>
> static ArchiveFormat
> parseArchiveFormat(const char *format, ArchiveMode *mode)
> {
> ...
> ...
> else if (pg_strcasecmp(format, "c") == 0)
> archiveFormat = archCustom;
> else if (pg_strcasecmp(format, "custom") == 0)
> archiveFormat = archCustom;
>
> else if (pg_strcasecmp(format, "p") == 0)
> archiveFormat = archNull;
> else if (pg_strcasecmp(format, "plain") == 0)
> archiveFormat = archNull;
> ...
> ...
> }
>
> static ArchiveHandle *
> _allocAH(const char *FileSpec, const ArchiveFormat fmt,
>  const int compression, bool dosync, ArchiveMode mode,
>  SetupWorkerPtrType setupWorkerPtr)
> {
>
> ...
> ...
> #ifdef WIN32
> if (fmt != archNull &&
> (AH->fSpec == NULL || strcmp(AH->fSpec, "") == 0))
> {
> if (mode == archModeWrite)
> setmode(fileno(stdout), O_BINARY);
> else
> setmode(fileno(stdin), O_BINARY);
> }
> #endif
> ..
> ..
> }
>
> Please confirm.
>

I also think it is the plain text mode. There is no problem with text
mode file that contains the CR LF characters.

Meanwhile, I have unit tested the patch submitted by Hari upthread on
> postgresql v10 and it works fine. Below are the steps that i have
> followed to test Hari's patch.
>
> Without patch:
> ==
> C:\Users\ashu\postgresql\TMP\test\bin>.\pg_basebackup.exe -D - -F t -X
> none > base.tar
> NOTICE:  WAL archiving is not enabled; you must ensure that all required
> WAL seg
> ments are copied through other means to complete the backup
>
> C:\Users\ashu\postgresql\TMP\test\bin>tar -xf base.tar
> tar: Skipping to next header
> tar: Exiting with failure status due to previous errors
>
>
> With patch:
> ===
> C:\Users\ashu\git-clone-postgresql\postgresql\TMP\
> test\bin>.\pg_basebackup.exe
> -D - -F t -X none > base.tar
> NOTICE:  WAL archiving is not enabled; you must ensure that all required
> WAL seg
> ments are copied through other means to complete the backup
>
> C:\Users\ashu\postgresql\TMP\test\bin>cp base.tar ..\basebakup
>
> C:\Users\ashu\postgresql\TMP\test\basebakup>tar -xf base.tar
>
> C:\Users\ashu\postgresql\TMP\test\basebakup>ls
> PG_VERSIONpg_commit_ts   pg_multixact  pg_stat  pg_wal
> backup_label  pg_dynshmempg_notify pg_stat_tmp  pg_xact
> base  pg_hba.confpg_replslot   pg_subtrans
> postgresql.auto.conf
> base.tar  pg_ident.conf  pg_serial pg_tblspcpostgresql.conf
> globalpg_logical pg_snapshots  pg_twophase  tablespace_map
>
>
Thanks for the tests to verify the patch.


Regards,
Hari Babu
Fujitsu Australia

Re: [HACKERS] password_encryption, default and 'plain' support

[Michael Paquier]

On Wed, May 3, 2017 at 9:57 PM, Magnus Hagander  wrote:
>
>
> On Wed, May 3, 2017 at 2:25 PM, Michael Paquier 
> wrote:
>>
>> On Wed, May 3, 2017 at 8:38 PM, Magnus Hagander 
>> wrote:
>> > On Wed, May 3, 2017 at 1:31 PM, Heikki Linnakangas 
>> > wrote:
>> >> In various threads on SCRAM, we've skirted around the question of
>> >> whether
>> >> we should still allow storing passwords in plaintext. I've avoided
>> >> discussing that in those other threads, because it's been an orthogonal
>> >> question, but it's a good question and we should discuss it.
>> >>
>> >> So, I propose that we remove support for password_encryption='plain' in
>> >> PostgreSQL 10. If you try to do that, you'll get an error.
>> >
>> > Is there any usecase at all for it today?
>>
>> For developers running applications on top of Postgres?
>
>
> I don't get it. How does password_encryption=plain help them?

Sanity checks at development stage of web applications to make sure
that the password strength automatically generated by the application
at first login is strong enough. I personally found that helpful for
this purpose.
-- 
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] How huge does mvtest_huge need to be?

[Kevin Grittner]

On Wed, May 3, 2017 at 4:37 PM, Tom Lane  wrote:

> At this point I'm inclined to just delete the whole test.

ok

-- 
Kevin Grittner
VMware vCenter Server
https://www.vmware.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] WITH clause in CREATE STATISTICS

[Tomas Vondra]

On 5/3/17 11:36 PM, Alvaro Herrera wrote:

Andrew Dunstan wrote:


On 05/03/2017 04:42 PM, Tom Lane wrote:



One other point is that as long as we've got reserved keywords introducing
each clause, there isn't actually an implementation reason why we couldn't
accept the clauses in any order.  Not sure I want to document it that way,
but it might not be a bad thing if the grammar was forgiving about whether
you write the USING or ON part first ...


+1 for allowing arbitrary order of clauses. I would document it with the
USING clause at the end, and have that be what psql supports and pg_dump
produces. Since there are no WITH options now we should leave that out
until it's required.


Ok, sounds good to me.  Unless there are objections I'm going to have a
shot at implementing this.  Thanks for the discussion.



Works for me. Do you also plan to remove the parentheses for the USING 
clause?


regards

--
Tomas Vondra  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] How huge does mvtest_huge need to be?

[Tom Lane]

I wrote:
> Kevin Grittner  writes:
>> Hm.  This seems like a particularly useless size.  It would test a
>> possibly useful corner case if it was over 10MB so that it was over
>> vacuum's truncation threshold, but that would obviously be even
>> slower.  It doesn't seem justified.  How about 500 so it at least
>> goes to a second page which is then truncated to 1 page.

> Yeah, that aspect occurred to me after a bit too.  I'll make it so.

Umm... but wait.  I stuck some "select pg_relation_size()" calls into
the test sequence to verify what page counts I was getting, and realized
that actually the REFRESH MATERIALIZED VIEW step is leaving the matview
with physical size zero.  So there's nothing for VACUUM to do anyway,
and it doesn't matter what size the matview had been before the DELETE
and REFRESH.  Maybe we could devise a test that allows VACUUM to be
responsible for actually truncating some pages from a matview, but this
test case ain't it.

I now remember that this test case was intended to exercise the hack
we'd had at the time whereby nonzero physical size signified whether
the matview was populated or not.  We got rid of that on the grounds that
it was too fragile, in favor of adding a pg_class.relispopulated column.
No released version of PG has ever had the capacity to have the type of
bug this test is meant to find.

At this point I'm inclined to just delete the whole test.  The code
that b69ec7cc9 added is long gone, and I don't think the test deserves
to be memorialized either.

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] WITH clause in CREATE STATISTICS

[Alvaro Herrera]

Andrew Dunstan wrote:

> On 05/03/2017 04:42 PM, Tom Lane wrote:

> > One other point is that as long as we've got reserved keywords introducing
> > each clause, there isn't actually an implementation reason why we couldn't
> > accept the clauses in any order.  Not sure I want to document it that way,
> > but it might not be a bad thing if the grammar was forgiving about whether
> > you write the USING or ON part first ...
> 
> +1 for allowing arbitrary order of clauses. I would document it with the
> USING clause at the end, and have that be what psql supports and pg_dump
> produces. Since there are no WITH options now we should leave that out
> until it's required.

Ok, sounds good to me.  Unless there are objections I'm going to have a
shot at implementing this.  Thanks for the discussion.

-- 
Á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] CTE inlining

[Andrew Dunstan]

On 05/03/2017 05:24 PM, Merlin Moncure wrote:
> On Wed, May 3, 2017 at 12:33 PM, Alvaro Herrera
>  wrote:
>> David Fetter wrote:
>>
>>> When we add a "temporary" GUC, we're taking on a gigantic burden.
>>> Either we support it forever somehow, or we put it on a deprecation
>>> schedule immediately and expect to be answering questions about it for
>>> years after it's been removed.
>>>
>>> -1 for the GUC.
>> Absolutely.
>>
>> So ISTM we have three choices:
>>
>> 1) we switch unmarked CTEs as inlineable by default in pg11.  What seems
>> likely to happen for a user that upgrades to pg11 is that 5 out of 10
>> CTE-using queries are going to become faster than with pg10, and they
>> are going to be happy; 4 out of five are going to see no difference, but
>> they didn't have to do anything about it; and the remaining query is
>> going to become slower, either indistinguishably so (in which case they
>> don't care and they remain happy because of the other improvements) or
>> notably so, in which case they can easily figure where to add the
>> MATERIALIZED option and regain the original performance.
> +1 for option 1.  This change will be welcome for a large number of
> queries, but forced materialization is a real need and I use it often.
> This comes off as a very reasonable compromise in my opinion unless it
> requires major coding gymnastics to implement.
>


The only thing I am totally dead set against is making people go back to
using OFFSET 0. It's ugly and completely non-intuitive.

cheers

andrew

-- 
Andrew Dunstanhttps://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] CTE inlining

[Gavin Flower]

On 04/05/17 05:33, Alvaro Herrera wrote:

David Fetter wrote:


When we add a "temporary" GUC, we're taking on a gigantic burden.
Either we support it forever somehow, or we put it on a deprecation
schedule immediately and expect to be answering questions about it for
years after it's been removed.

-1 for the GUC.

Absolutely.

So ISTM we have three choices:

1) we switch unmarked CTEs as inlineable by default in pg11.  What seems
likely to happen for a user that upgrades to pg11 is that 5 out of 10
CTE-using queries are going to become faster than with pg10, and they
are going to be happy; 4 out of five are going to see no difference, but
they didn't have to do anything about it; and the remaining query is
going to become slower, either indistinguishably so (in which case they
don't care and they remain happy because of the other improvements) or
notably so, in which case they can easily figure where to add the
MATERIALIZED option and regain the original performance.

+1

I've watched a colleague spend hours trying to optimise a complicated 
query with nested views, then find that this 'optimisation fence' was 
the heart of the problem.



Cheers,
Gavin



--
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] WITH clause in CREATE STATISTICS

[Andrew Dunstan]

On 05/03/2017 04:42 PM, Tom Lane wrote:
> Alvaro Herrera  writes:
>> Yawn.  So, we have not achieved the stated goal which was to get rid of
>> the ugly clause in the middle of the command; moreover we have gained
>> *yet another* clause in the middle of the command.  Is this really an
>> improvement?  We're trading this
>> CREATE STATISTICS s1 WITH (dependencies, ndistinct, options) ON (a, b) 
>> FROM t1 WHERE partial-stuff;
>> for this:
>> CREATE STATISTICS s1 USING (dependencies, ndistinct) WITH (options) ON 
>> (a, b) FROM t1 WHERE partial-stuff;
>> Can we decide by a show of hands, please, whether we're really on board
>> with this change?
> That seems totally messy :-(
>
> I can't see any good reason why "WITH (options)" can't be at the end of
> the query.  WITH is a fully reserved word, there is not going to be any
> danger of a parse problem from having it follow the FROM or WHERE clauses.
> And the end is where we have other instances of "WITH (options)".
>
> It also seems like we don't need to have *both* fully-reserved keywords
> introducing each clause *and* parentheses around the lists.  Maybe
> dropping the parens around the stats-types list and the column-names
> list would help to declutter?  (But I'd keep parens around the WITH
> options, for consistency with other statements.)
>
> One other point is that as long as we've got reserved keywords introducing
> each clause, there isn't actually an implementation reason why we couldn't
> accept the clauses in any order.  Not sure I want to document it that way,
> but it might not be a bad thing if the grammar was forgiving about whether
> you write the USING or ON part first ...
>
>   


+1 for allowing arbitrary order of clauses. I would document it with the
USING clause at the end, and have that be what psql supports and pg_dump
produces. Since there are no WITH options now we should leave that out
until it's required.

cheers

andrew

-- 
Andrew Dunstanhttps://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] CTE inlining

[Merlin Moncure]

On Wed, May 3, 2017 at 12:33 PM, Alvaro Herrera
 wrote:
> David Fetter wrote:
>
>> When we add a "temporary" GUC, we're taking on a gigantic burden.
>> Either we support it forever somehow, or we put it on a deprecation
>> schedule immediately and expect to be answering questions about it for
>> years after it's been removed.
>>
>> -1 for the GUC.
>
> Absolutely.
>
> So ISTM we have three choices:
>
> 1) we switch unmarked CTEs as inlineable by default in pg11.  What seems
> likely to happen for a user that upgrades to pg11 is that 5 out of 10
> CTE-using queries are going to become faster than with pg10, and they
> are going to be happy; 4 out of five are going to see no difference, but
> they didn't have to do anything about it; and the remaining query is
> going to become slower, either indistinguishably so (in which case they
> don't care and they remain happy because of the other improvements) or
> notably so, in which case they can easily figure where to add the
> MATERIALIZED option and regain the original performance.

+1 for option 1.  This change will be welcome for a large number of
queries, but forced materialization is a real need and I use it often.
This comes off as a very reasonable compromise in my opinion unless it
requires major coding gymnastics to implement.

merlin


-- 
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] CTE inlining

[Tomas Vondra]

On 5/3/17 9:54 PM, Andreas Karlsson wrote:

On 05/03/2017 07:33 PM, Alvaro Herrera wrote:

1) we switch unmarked CTEs as inlineable by default in pg11.  What seems
likely to happen for a user that upgrades to pg11 is that 5 out of 10
CTE-using queries are going to become faster than with pg10, and they
are going to be happy; 4 out of five are going to see no difference, but
they didn't have to do anything about it; and the remaining query is
going to become slower, either indistinguishably so (in which case they
don't care and they remain happy because of the other improvements) or
notably so, in which case they can easily figure where to add the
MATERIALIZED option and regain the original performance.


2) unmarked CTEs continue to be an optimization barrier, but we add
"WITH INLINED" so that they're inlineable.  Some users may wonder about
it and waste a lot of time trying to figure out which CTEs to add it to.
They see a benefit in half the queries, which makes them happy, but they
are angry that they had to waste all that time on the other queries.


3) We don't do anything, because we all agree that GUCs are not
suitable.  No progress.  No anger, but nobody is happy either.


+1 for option 1. And while I would not like if we had to combine it with 
a backwards compatibility GUC which enables the old behavior to get it 
merged I still personally would prefer that over option 2 and 3.



> Andreas
>

+1 to what Andreas says

--
Tomas Vondra  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] PROVE_FLAGS

[Tom Lane]

Andrew Dunstan  writes:
> Can someone please explain to me why we have this in Makefile.global.in?
> (from commit e9c81b60 )
> PROVE_FLAGS =

Before that commit it was like

PROVE_FLAGS = --verbose

which had some value.  I agree that now we'd be better off to not
set it at all, especially since the convention now appears to be that
automatically-supplied prove options should be set in PG_PROVE_FLAGS.

I'd suggest that the comment just above be replaced by something like

# User-supplied prove flags can be provided in PROVE_FLAGS.

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] [PostgreSQL 10] default of hot_standby should be "on"?

[Magnus Hagander]

On Wed, May 3, 2017 at 4:18 PM, Michael Banck 
wrote:

> Hi,
>
> On Tue, May 02, 2017 at 11:13:58AM +0200, Magnus Hagander wrote:
> > Looks good to me as well. Applied, with only a minor further docs
> addition
> > saying that this is the default also on the high availability page.
>
> I understand this is late, but a colleague alerted me to the following
> behaviour change: If you recover a server with default settings, it is
> our understanding that pg_isready will now return true immediately after
> the consistent state is reached and possibly well before recovery had
> actually ended (depending on the amount of outstanding wal). As hot
> standby works with log shipping, this is independent of the
> recovery.conf settings, i.e. even if standby_mode and primary_conninfo
> are not set. So if one was monitoring recovery like that before and
> expects pg_isready to only return true once the recovery is fully
> complete, this will now have to be adjusted. Also, if the recovered
> server is to be used for transactions, there will now be a window where
> the server accepts connections, but is in read-only mode.
>
> Before, one had the make the concious choice to set hot_standby to get
> the behaviour, now it might be surprising to users, or maybe I'm
> overthinking this?
>
> If that is indeed the case, maybe it should be mentioned more
> prominently in the documentation and/or get highlighted in the release
> notes?
>

Hmm. That's an interesting usecase.

I don't think it's a big enough one to revert this change, but it
definitely makes sense to mention it under incompatible changes.

I wonder if what we really want here, at least long-term, is a flag for
pg_isready that makes it wait for a server to actually go out of
recovery?`Seems that a tool like the one mentioned here would have to do
that -- it can be done now by doing pg_isready first and then psql to check
the status, but it seems like it could be a worthwhile addition?


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

Re: [HACKERS] WITH clause in CREATE STATISTICS

[Tom Lane]

Alvaro Herrera  writes:
> Yawn.  So, we have not achieved the stated goal which was to get rid of
> the ugly clause in the middle of the command; moreover we have gained
> *yet another* clause in the middle of the command.  Is this really an
> improvement?  We're trading this
> CREATE STATISTICS s1 WITH (dependencies, ndistinct, options) ON (a, b) 
> FROM t1 WHERE partial-stuff;
> for this:
> CREATE STATISTICS s1 USING (dependencies, ndistinct) WITH (options) ON 
> (a, b) FROM t1 WHERE partial-stuff;

> Can we decide by a show of hands, please, whether we're really on board
> with this change?

That seems totally messy :-(

I can't see any good reason why "WITH (options)" can't be at the end of
the query.  WITH is a fully reserved word, there is not going to be any
danger of a parse problem from having it follow the FROM or WHERE clauses.
And the end is where we have other instances of "WITH (options)".

It also seems like we don't need to have *both* fully-reserved keywords
introducing each clause *and* parentheses around the lists.  Maybe
dropping the parens around the stats-types list and the column-names
list would help to declutter?  (But I'd keep parens around the WITH
options, for consistency with other statements.)

One other point is that as long as we've got reserved keywords introducing
each clause, there isn't actually an implementation reason why we couldn't
accept the clauses in any order.  Not sure I want to document it that way,
but it might not be a bad thing if the grammar was forgiving about whether
you write the USING or ON part first ...

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] How huge does mvtest_huge need to be?

[Tom Lane]

Kevin Grittner  writes:
> On Wed, May 3, 2017 at 12:08 PM, Tom Lane  wrote:
>> So ... is there a good reason to be using a large table here, and
>> if so what is it, and how big does the table really need to be
>> to provide useful test coverage?

> Hm.  This seems like a particularly useless size.  It would test a
> possibly useful corner case if it was over 10MB so that it was over
> vacuum's truncation threshold, but that would obviously be even
> slower.  It doesn't seem justified.  How about 500 so it at least
> goes to a second page which is then truncated to 1 page.

Yeah, that aspect occurred to me after a bit too.  I'll make it so.

> The "huge" in the object names then seems odd, of course.

Right ... will pick some other name.

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] CTE inlining

[Andreas Karlsson]

On 05/03/2017 07:33 PM, Alvaro Herrera wrote:

1) we switch unmarked CTEs as inlineable by default in pg11.  What seems
likely to happen for a user that upgrades to pg11 is that 5 out of 10
CTE-using queries are going to become faster than with pg10, and they
are going to be happy; 4 out of five are going to see no difference, but
they didn't have to do anything about it; and the remaining query is
going to become slower, either indistinguishably so (in which case they
don't care and they remain happy because of the other improvements) or
notably so, in which case they can easily figure where to add the
MATERIALIZED option and regain the original performance.


2) unmarked CTEs continue to be an optimization barrier, but we add
"WITH INLINED" so that they're inlineable.  Some users may wonder about
it and waste a lot of time trying to figure out which CTEs to add it to.
They see a benefit in half the queries, which makes them happy, but they
are angry that they had to waste all that time on the other queries.


3) We don't do anything, because we all agree that GUCs are not
suitable.  No progress.  No anger, but nobody is happy either.


+1 for option 1. And while I would not like if we had to combine it with 
a backwards compatibility GUC which enables the old behavior to get it 
merged I still personally would prefer that over option 2 and 3.


Andreas


--
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] statement_timeout is not working as expected with postgres_fdw

[Robert Haas]

On Thu, Apr 20, 2017 at 10:27 AM, Ashutosh Bapat
 wrote:
> The logs above show that 34 seconds elapsed between starting to abort
> the transaction and knowing that the foreign server is unreachable. It
> looks like it took that much time for the local server to realise that
> the foreign server is not reachable. Looking at PQcancel code, it
> seems to be trying to connect to the foreign server to cancel the
> query. But somehow it doesn't seem to honor connect_timeout setting.
> Is that expected?

Well, there's no code to do anything else.  Regular connections go
through connectDBComplete() which uses non-blocking mode and timed
waits to respect connection_timeout.  PQcancel() has no such handling.
internal_cancel just opens a socket and, without setting any options
on it, calls connect().  No loop, no timed waits, nada.  So it's going
to take as long as it takes for the operating system to notice.

> Irrespective of what PQcancel does, it looks like postgres_fdw should
> just slam the connection if query is being aborted because of
> statement_timeout. But then pgfdw_xact_callback() doesn't seem to have
> a way to know whether this ABORT is because of user's request to
> cancel the query, statement timeout, an abort because of some other
> error or a user requested abort. Except statement timeout (may be
> user's request to cancel the query?), it should try to keep the
> connection around to avoid any future reconnection. But I am not able
> to see how can we provide that information to pgfdw_xact_callback().

I don't think we can.  In general, PostgreSQL has no facility for
telling error cleanup handlers why the abort happened, and it wouldn't
really be the right thing anyway.  The fact that statement_timeout
fired doesn't necessarily mean that the connection is dead; it could
equally well mean that the query ran for a long time.  I think we
basically have two choices.  One is to bound the amount of time we
spend performing error cleanup, and the other is just to always drop
the connection.  A problem with the latter is that it might do the
wrong thing if we're aborting a subtransaction but not the whole
transaction.  In that case, we need to undo changes since the relevant
savepoint, but not the ones made before that; closing the connection
amounts to a full rollback.

Therefore, I think the patch you proposed in
https://www.postgresql.org/message-id/CAFjFpRdcWw4h0a-zrL-EiaekkPj8O0GR2M1FwZ1useSRfRm3-g%40mail.gmail.com
isn't correct, because if the cancel fails it will slam the connection
shut regardless of whether we're in pgfdw_xact_callback or
pgfdw_subxact_callback.

It looks to me like there's a fairly lengthy list of conceptually
separate but practically related problems here:

1. PQcancel() ignores the keepalive parameters, because it makes no
attempt to set the relevant socket options before connecting.

2. PQcancel() ignores connection_timeout, because it doesn't use
non-blocking mode and has no wait loop.

3. There is no asynchronous equivalent of PQcancel(), so we can't use
a loop to allow responding to interrupts while the cancel is
outstanding (see pgfdw_get_result for an example).

4. pgfdw_xact_callback() and pgfdw_subxact_callback() use PQexec()
rather than the asynchronous interfaces for sending queries and
checking for results, so the SQL commands they send are not
interruptible.

5. pgfdw_xact_callback() and pgfdw_subxact_callback() try to get the
connection back to a good state by using ABORT TRANSACTION for the
former and ROLLBACK TO SAVEPOINT and RELEASE SAVEPOINT for the latter.
But if it doesn't work, then they just emit a WARNING and continue on
as if they'd succeeded.  That seems highly likely to make the next use
of that connection fail in unpredictable ways.

6. postgres_fdw doesn't use PQsetnonblocking() anywhere, so even if we
converted pgfdw_xact_callback() and pgfdw_subxact_callback() to use
pgfdw_exec_query() or something like it rather than PQexec() to submit
queries, they might still block if we fail to send the query, as the
comments for pgfdw_exec_query() explain.  This is not possibly but not
particularly likely to happen for queries being sent out of error
handling paths, because odds are good that the connection was sent
just before.  However, if the user is pressing ^C because the remote
server isn't responding, it's quite probable that we'll run into this
exact issue.

7. postgres_fdw never considers slamming the connection shut as a
response to trouble.  It seems pretty clear that this is only a safe
response if we're aborting the toplevel transaction.  If we did it for
a subtransaction, we'd end up reconnecting if the server were accessed
again, which would at the very least change the snapshot (note that we
use at least REPEATABLE READ on the remote side regardless of the
local isolation level) and might discard changes made on the remote
side at outer transaction levels.  Even for a top-level transaction,
it might not always 

Re: [HACKERS] PROVE_FLAGS

[Andrew Dunstan]

On 05/03/2017 03:21 PM, Andres Freund wrote:
> On 2017-05-03 15:14:27 -0400, Andrew Dunstan wrote:
>> Can someone please explain to me why we have this in Makefile.global.in?
>> (from commit e9c81b60 )
>>
>>
>> PROVE_FLAGS =
>>
>>
>> ISTM it's unnecessary, and prevents us from using the same named value
>> in the environment. I want to be able to use the environment in
>> vcregress.pl, and I'd like the Make files to work the same way.
> Wouldn't it be better to append the environment to the flags here,
> that'd allow us to modify flags from both places?
>



The Makefile already has:

$(PROVE) $(PG_PROVE_FLAGS) $(PROVE_FLAGS) ...

It doesn't set PROVE_FLAGS anywhere.

cheers

andrew

-- 
Andrew Dunstanhttps://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] PROVE_FLAGS

[Andres Freund]

On 2017-05-03 15:14:27 -0400, Andrew Dunstan wrote:
> 
> Can someone please explain to me why we have this in Makefile.global.in?
> (from commit e9c81b60 )
> 
> 
> PROVE_FLAGS =
> 
> 
> ISTM it's unnecessary, and prevents us from using the same named value
> in the environment. I want to be able to use the environment in
> vcregress.pl, and I'd like the Make files to work the same way.

Wouldn't it be better to append the environment to the flags here,
that'd allow us to modify flags from both places?

Andres


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

[HACKERS] PROVE_FLAGS

[Andrew Dunstan]

Can someone please explain to me why we have this in Makefile.global.in?
(from commit e9c81b60 )


PROVE_FLAGS =


ISTM it's unnecessary, and prevents us from using the same named value
in the environment. I want to be able to use the environment in
vcregress.pl, and I'd like the Make files to work the same way.


cheers


andrew


-- 
Andrew Dunstanhttps://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] WITH clause in CREATE STATISTICS

[Alvaro Herrera]

Stephen Frost wrote:

> > Here I add one, which seems to work for me.

Pushed it -- I also added another one which specifies options, to test
WITH handling in ruleutils.

> > Considering that Stephen missed a terminating semicolon for test with
> > create_order 96 (the last one prior to my commit) in commit
> > 31a8b77a9244, I propose that we change whatever is concatenating those
> > strings append a terminating semicolon.  (Surely we don't care about two
> > tests stanzas writing a single SQL command by omitting the semicolon
> > terminator.)
> 
> Whoops, sorry about that.  Yes, we could pretty easily add that.  The
> create SQL is built up at the bottom of 002_pg_dump.pl:
> 
> $create_sql .= $tests{$test}->{create_sql};

Okay, I added it.

-- 
Á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] password_encryption, default and 'plain' support

[Magnus Hagander]

On Wed, May 3, 2017 at 5:52 PM, Robert Haas  wrote:

> On Wed, May 3, 2017 at 7:31 AM, Heikki Linnakangas 
> wrote:
> > In various threads on SCRAM, we've skirted around the question of
> whether we
> > should still allow storing passwords in plaintext. I've avoided
> discussing
> > that in those other threads, because it's been an orthogonal question,
> but
> > it's a good question and we should discuss it.
> >
> > So, I propose that we remove support for password_encryption='plain' in
> > PostgreSQL 10. If you try to do that, you'll get an error.
>
> I have no idea how widely used that option is.
>
> > Another question that's been touched upon but not explicitly discussed,
> is
> > whether we should change the default to "scram-sha-256". I propose that
> we
> > do that as well. If you need to stick to md5, e.g. because you use
> drivers
> > that don't support SCRAM yet, you can change it in postgresql.conf, but
> the
> > majority of installations that use modern clients will be more secure by
> > default.
>
> I think that we should investigate how many connectors have support
> for SCRAM or are likely to do so by the time v10 is released.  A *lot*
> of people are using connectors that are not based on libpq, especially
> JDBC but I think many of the others as well.  If most of those are
> going to support SCRAM by the time v10 comes out, cool, but if not,
> maybe it's wise to hold off for a release before flipping the default.
> Not sure.
>


>From the traffic on the list it sounds like the JDBC people are working on
it already -- hopefully they will have something in time.

It might make sense to ping other "major drivers" people as well -- such as
maybe npgsql. What else?

A good approach might be to change the default now, before beta. Then if
drivers don't change, or if we get a lot of pushback from beta testers, we
change it back before release. But if we don't change it, we will not know
how big the impact would be...

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

Re: [HACKERS] WITH clause in CREATE STATISTICS

[Alvaro Herrera]

Alvaro Herrera wrote:
> Simon Riggs wrote:
> 
> > 2.
> > USING keyword, no brackets
> > CREATE STATISTICS s1 USING (dependencies, ndistinct) ON (a, b) FROM t1
> > WHERE partial-stuff;
> > 
> > and if there are options, use the WITH for the optional parameters like this
> > CREATE STATISTICS s1 USING (dependencies, ndistinct) WITH (options) ON
> > (a, b) FROM t1 WHERE partial-stuff;
> > 
> > 
> > I think I like (2)
> 
> OK, sounds sensible.

Yawn.  So, we have not achieved the stated goal which was to get rid of
the ugly clause in the middle of the command; moreover we have gained
*yet another* clause in the middle of the command.  Is this really an
improvement?  We're trading this
CREATE STATISTICS s1 WITH (dependencies, ndistinct, options) ON (a, b) FROM 
t1 WHERE partial-stuff;
for this:
CREATE STATISTICS s1 USING (dependencies, ndistinct) WITH (options) ON (a, 
b) FROM t1 WHERE partial-stuff;

Can we decide by a show of hands, please, whether we're really on board
with this change?

-- 
Á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] CTE inlining

[Kenneth Marshall]

On Wed, May 03, 2017 at 02:33:05PM -0300, Alvaro Herrera wrote:
> David Fetter wrote:
> 
> > When we add a "temporary" GUC, we're taking on a gigantic burden.
> > Either we support it forever somehow, or we put it on a deprecation
> > schedule immediately and expect to be answering questions about it for
> > years after it's been removed.
> > 
> > -1 for the GUC.
> 
> Absolutely.
> 
> So ISTM we have three choices:
> 
> 1) we switch unmarked CTEs as inlineable by default in pg11.  What seems
> likely to happen for a user that upgrades to pg11 is that 5 out of 10
> CTE-using queries are going to become faster than with pg10, and they
> are going to be happy; 4 out of five are going to see no difference, but
> they didn't have to do anything about it; and the remaining query is
> going to become slower, either indistinguishably so (in which case they
> don't care and they remain happy because of the other improvements) or
> notably so, in which case they can easily figure where to add the
> MATERIALIZED option and regain the original performance.
> 
> 
> 2) unmarked CTEs continue to be an optimization barrier, but we add
> "WITH INLINED" so that they're inlineable.  Some users may wonder about
> it and waste a lot of time trying to figure out which CTEs to add it to.
> They see a benefit in half the queries, which makes them happy, but they
> are angry that they had to waste all that time on the other queries.
> 
> 
> 3) We don't do anything, because we all agree that GUCs are not
> suitable.  No progress.  No anger, but nobody is happy either.
> 

+1 for option 1. I just finished rewriting a well written CTE query to
avoid the optimization fence and get reasonable performance.

Regards,
Ken


-- 
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] renaming "transaction log"

[Alvaro Herrera]

David Fetter wrote:
> On Wed, May 03, 2017 at 10:33:32AM -0700, David Fetter wrote:
> > On Wed, May 03, 2017 at 10:57:06AM -0300, Alvaro Herrera wrote:
> > > Peter Eisentraut wrote:
> > > > Most documentation and error messages still uses the term "transaction
> > > > log" to refer to the write-ahead log.  Here is a patch to rename that,
> > > > which I think should be done, to match the xlog -> wal renaming in APIs.
> > > 
> > > +1 for the idea.  I suggest grepping for "transaction$" too -- there are
> > > a few cases in SGML that have the phrase "transaction log" split across
> > > lines.
> > 
> > In general, you can use ag a.k.a. "The Silver Searcher" for phrase
> > searches as follows:

> ag $(echo "the phrase to be searched" | sed -e 's/ /\\s+/g')

Nice, thanks.  So after applying Peter's patch, this grep returns the
following (after removing the *.po files from the result):

src/backend/access/transam/transam.c:4: * postgres transaction log 
interface routines

Probably change to "commit log"; this one is not related to WAL.

doc/src/sgml/release-old.sgml:631:Fix race condition in 
transaction log management
doc/src/sgml/release-old.sgml:2245:Prevent possible compressed 
transaction log loss (Tom)
doc/src/sgml/release-old.sgml:2282:Fix for compressed 
transaction log id wraparound (Tom)
doc/src/sgml/release-8.2.sgml:5163:   Allow a forced switch to a new 
transaction log file (Simon, Tom)
doc/src/sgml/release-8.2.sgml:5168:   in sync with the master.  Transaction 
log file switching now also happens
doc/src/sgml/release-8.2.sgml:5172:   transaction log files needed for 
recovery can be archived immediately.
doc/src/sgml/release-8.2.sgml:5182:   Add functions for interrogating the 
current transaction log insertion
doc/src/sgml/release-8.2.sgml:5222:   to force transaction log file 
switches at a given interval (Simon)
doc/src/sgml/release-8.0.sgml:2582:Fix race condition in 
transaction log management
doc/src/sgml/release-9.6.sgml:3180:to expose the current transaction 
log flush location (Tomas Vondra)
doc/src/sgml/release-7.4.sgml:1950:Fix race condition in 
transaction log management

Leave old release notes untouched.

doc/src/sgml/release-10.sgml:2362:   2016-11-09 [41124a91e] pgbench: Allow 
the transaction log file prefix to be cha
doc/src/sgml/release-10.sgml:2520:   2016-10-23 [56c7d8d45] Allow 
pg_basebackup to stream transaction log in tar mod

Comments, ignore.  (First one is unrelated anyway.)

doc/src/sgml/high-availability.sgml:141:   Transaction Log Shipping
doc/src/sgml/high-availability.sgml:292: Transaction Log 
Shipping

I think it'd make sense to flip these two to Write-Ahead Log Shipping
too.  (Why aren't there index entries for the term?  Maybe add two
entries, one for "transaction log" and another one for WAL).

doc/src/sgml/wal.sgml:241:transaction log

Leave this one alone, add another indexentry for the WAL terminology.

doc/src/sgml/ref/pg_receivewal.sgml:35:   from a running 
PostgreSQL cluster. The transaction
doc/src/sgml/ref/pg_receivewal.sgml:36:   log is streamed using the streaming 
replication protocol, and is written

Peter missed this one.

doc/src/sgml/ref/pg_receivewal.sgml:43:   
pg_receivewal streams the transaction
doc/src/sgml/ref/pg_receivewal.sgml:44:   log in real time as it's being 
generated on the server, and does not wait

Ditto.

doc/src/sgml/ref/pg_basebackup.sgml:181:the cluster has no 
additional tablespaces and transaction
doc/src/sgml/ref/pg_basebackup.sgml:182:log streaming is not used.

Ditto.

doc/src/sgml/ref/pgbench.sgml:1126:  Per-Transaction Logging

Unrelated, don't change.

-- 
Á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] How huge does mvtest_huge need to be?

[Kevin Grittner]

On Wed, May 3, 2017 at 12:08 PM, Tom Lane  wrote:

> So ... is there a good reason to be using a large table here, and
> if so what is it, and how big does the table really need to be
> to provide useful test coverage?

Hm.  This seems like a particularly useless size.  It would test a
possibly useful corner case if it was over 10MB so that it was over
vacuum's truncation threshold, but that would obviously be even
slower.  It doesn't seem justified.  How about 500 so it at least
goes to a second page which is then truncated to 1 page.

The "huge" in the object names then seems odd, of course.

--
Kevin Grittner
VMware vCenter Server
https://www.vmware.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] CTE inlining

[Pavel Stehule]

2017-05-03 19:33 GMT+02:00 Alvaro Herrera :

> David Fetter wrote:
>
> > When we add a "temporary" GUC, we're taking on a gigantic burden.
> > Either we support it forever somehow, or we put it on a deprecation
> > schedule immediately and expect to be answering questions about it for
> > years after it's been removed.
> >
> > -1 for the GUC.
>
> Absolutely.
>
> So ISTM we have three choices:
>
> 1) we switch unmarked CTEs as inlineable by default in pg11.  What seems
> likely to happen for a user that upgrades to pg11 is that 5 out of 10
> CTE-using queries are going to become faster than with pg10, and they
> are going to be happy; 4 out of five are going to see no difference, but
> they didn't have to do anything about it; and the remaining query is
> going to become slower, either indistinguishably so (in which case they
> don't care and they remain happy because of the other improvements) or
> notably so, in which case they can easily figure where to add the
> MATERIALIZED option and regain the original performance.
>
>
> 2) unmarked CTEs continue to be an optimization barrier, but we add
> "WITH INLINED" so that they're inlineable.  Some users may wonder about
> it and waste a lot of time trying to figure out which CTEs to add it to.
> They see a benefit in half the queries, which makes them happy, but they
> are angry that they had to waste all that time on the other queries.
>
>
> 3) We don't do anything, because we all agree that GUCs are not
> suitable.  No progress.  No anger, but nobody is happy either.
>

yes, these variants are all.

@2 is only little bit better than bad @3 - is not nice to have default
behave different than any other and than what 90% developers expects.

Regards

Pavel


>
> --
> Á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] WITH clause in CREATE STATISTICS

[Stephen Frost]

Alvaro,

* Alvaro Herrera (alvhe...@2ndquadrant.com) wrote:
> Alvaro Herrera wrote:
> 
> > In the meantime, I noticed that pg_dump support for extstats is not
> > covered, which I'll go fix next.
> 
> Here I add one, which seems to work for me.
> 
> Considering that Stephen missed a terminating semicolon for test with
> create_order 96 (the last one prior to my commit) in commit
> 31a8b77a9244, I propose that we change whatever is concatenating those
> strings append a terminating semicolon.  (Surely we don't care about two
> tests stanzas writing a single SQL command by omitting the semicolon
> terminator.)

Whoops, sorry about that.  Yes, we could pretty easily add that.  The
create SQL is built up at the bottom of 002_pg_dump.pl:

$create_sql .= $tests{$test}->{create_sql};

> I wonder if there's any rationale to the create_order numbers.  Surely
> we only care for objects that depend on others.

Yes, it was just a way to manage those dependencies.  If there's value
in doing something more complicated then we could certainly do that, but
I'm not sure why it would be necessary to add that complexity.

Thanks!

Stephen


signature.asc
Description: Digital signature

Re: [HACKERS] password_encryption, default and 'plain' support

[Tom Lane]

Heikki Linnakangas  writes:
> On 05/03/2017 07:14 PM, Tom Lane wrote:
>> Is it possible that there are still client libraries that don't support
>> password encryption at all?  If so, are we willing to break them?
>> I'd say "yes" but it's worth thinking about.

> That doesn't make sense. The client doesn't even know what 
> password_encryption is set to. I think you're confusing 
> password_encryption='plain' with the plaintext "password" authentication 
> method.

Ah, you're right.

> If the server has an MD5 hash stored in pg_authid, the server will ask 
> the client to do MD5 authentication. If the server has a SCRAM verifier 
> in pg_authid, it will ask the client to do SCRAM authentication. If the 
> server has a plaintext password in pg_authid, it will also ask the 
> client to do SCRAM authentication (it could ask for MD5 authentication, 
> but as the code stands, it will ask for SCRAM).

Um.  That would be a backwards compatibility break ... but it doesn't
matter if we get rid of the option to store in plaintext.

The other question I can think to ask is what will happen during
pg_upgrade, given an existing installation with one or more passwords
stored plain.  If the answer is "silently convert to MD5", I'd be
good with that.

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] renaming "transaction log"

[David Fetter]

On Wed, May 03, 2017 at 10:33:32AM -0700, David Fetter wrote:
> On Wed, May 03, 2017 at 10:57:06AM -0300, Alvaro Herrera wrote:
> > Peter Eisentraut wrote:
> > > Most documentation and error messages still uses the term "transaction
> > > log" to refer to the write-ahead log.  Here is a patch to rename that,
> > > which I think should be done, to match the xlog -> wal renaming in APIs.
> > 
> > +1 for the idea.  I suggest grepping for "transaction$" too -- there are
> > a few cases in SGML that have the phrase "transaction log" split across
> > lines.
> 
> In general, you can use ag a.k.a. "The Silver Searcher" for phrase
> searches as follows:
> 
> ag $(echo "the phrase to be searched" | sed -e 's/ /\\s/g')

Oops.  That should read:

ag $(echo "the phrase to be searched" | sed -e 's/ /\\s+/g')

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] password_encryption, default and 'plain' support

[Heikki Linnakangas]

On 05/03/2017 07:14 PM, Tom Lane wrote:

Robert Haas  writes:

On Wed, May 3, 2017 at 7:31 AM, Heikki Linnakangas  wrote:

So, I propose that we remove support for password_encryption='plain' in
PostgreSQL 10. If you try to do that, you'll get an error.



I have no idea how widely used that option is.


Is it possible that there are still client libraries that don't support
password encryption at all?  If so, are we willing to break them?
I'd say "yes" but it's worth thinking about.


That doesn't make sense. The client doesn't even know what 
password_encryption is set to. I think you're confusing 
password_encryption='plain' with the plaintext "password" authentication 
method.


If the server has an MD5 hash stored in pg_authid, the server will ask 
the client to do MD5 authentication. If the server has a SCRAM verifier 
in pg_authid, it will ask the client to do SCRAM authentication. If the 
server has a plaintext password in pg_authid, it will also ask the 
client to do SCRAM authentication (it could ask for MD5 authentication, 
but as the code stands, it will ask for SCRAM).


The server will only ask the client to do plaintext password 
authentication, if you put "password" as the authentication method in 
pg_hba.conf. But that works regardless of what password_encryption is 
set to.


No, I don't think there's any valid reason to store passwords in 
plaintext anymore. In theory, you could use either MD5 or SCRAM 
authentication with a plaintext password, which would be an advantage, 
but we don't provide an option for that.


- 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] renaming "transaction log"

[David Fetter]

On Wed, May 03, 2017 at 10:57:06AM -0300, Alvaro Herrera wrote:
> Peter Eisentraut wrote:
> > Most documentation and error messages still uses the term "transaction
> > log" to refer to the write-ahead log.  Here is a patch to rename that,
> > which I think should be done, to match the xlog -> wal renaming in APIs.
> 
> +1 for the idea.  I suggest grepping for "transaction$" too -- there are
> a few cases in SGML that have the phrase "transaction log" split across
> lines.

In general, you can use ag a.k.a. "The Silver Searcher" for phrase
searches as follows:

ag $(echo "the phrase to be searched" | sed -e 's/ /\\s/g')

Thanks to Dagfinn Ilmari Mannsåker for the tip :)

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] CTE inlining

[Alvaro Herrera]

David Fetter wrote:

> When we add a "temporary" GUC, we're taking on a gigantic burden.
> Either we support it forever somehow, or we put it on a deprecation
> schedule immediately and expect to be answering questions about it for
> years after it's been removed.
> 
> -1 for the GUC.

Absolutely.

So ISTM we have three choices:

1) we switch unmarked CTEs as inlineable by default in pg11.  What seems
likely to happen for a user that upgrades to pg11 is that 5 out of 10
CTE-using queries are going to become faster than with pg10, and they
are going to be happy; 4 out of five are going to see no difference, but
they didn't have to do anything about it; and the remaining query is
going to become slower, either indistinguishably so (in which case they
don't care and they remain happy because of the other improvements) or
notably so, in which case they can easily figure where to add the
MATERIALIZED option and regain the original performance.


2) unmarked CTEs continue to be an optimization barrier, but we add
"WITH INLINED" so that they're inlineable.  Some users may wonder about
it and waste a lot of time trying to figure out which CTEs to add it to.
They see a benefit in half the queries, which makes them happy, but they
are angry that they had to waste all that time on the other queries.


3) We don't do anything, because we all agree that GUCs are not
suitable.  No progress.  No anger, but nobody is happy either.

-- 
Á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] WITH clause in CREATE STATISTICS

[Alvaro Herrera]

Alvaro Herrera wrote:

> In the meantime, I noticed that pg_dump support for extstats is not
> covered, which I'll go fix next.

Here I add one, which seems to work for me.

Considering that Stephen missed a terminating semicolon for test with
create_order 96 (the last one prior to my commit) in commit
31a8b77a9244, I propose that we change whatever is concatenating those
strings append a terminating semicolon.  (Surely we don't care about two
tests stanzas writing a single SQL command by omitting the semicolon
terminator.)

I wonder if there's any rationale to the create_order numbers.  Surely
we only care for objects that depend on others.

-- 
Álvaro Herrerahttps://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
>From 1dcb16de656d48c1004d4f4a12475438618a5c72 Mon Sep 17 00:00:00 2001
From: Alvaro Herrera 
Date: Wed, 3 May 2017 14:18:22 -0300
Subject: [PATCH] Add pg_dump tests for CREATE STATISTICS

---
 src/bin/pg_dump/t/002_pg_dump.pl | 36 +++-
 1 file changed, 35 insertions(+), 1 deletion(-)

diff --git a/src/bin/pg_dump/t/002_pg_dump.pl b/src/bin/pg_dump/t/002_pg_dump.pl
index ccd0ed6a53..79108cd331 100644
--- a/src/bin/pg_dump/t/002_pg_dump.pl
+++ b/src/bin/pg_dump/t/002_pg_dump.pl
@@ -1053,7 +1053,7 @@ my %tests = (
all_runs  => 1,
catch_all => 'ALTER TABLE ... commands',
create_order => 96,
-   create_sql => 'ALTER TABLE dump_test.test_table CLUSTER ON 
test_table_pkey',
+   create_sql => 'ALTER TABLE dump_test.test_table CLUSTER ON 
test_table_pkey;',
regexp=> qr/^
\QALTER TABLE test_table CLUSTER ON test_table_pkey;\E\n
/xm,
@@ -4920,6 +4920,40 @@ qr/CREATE TRANSFORM FOR integer LANGUAGE sql \(FROM SQL 
WITH FUNCTION pg_catalog
role => 1,
section_post_data=> 1, }, },
 
+   'CREATE STATISTICS extended_stats_no_using' => {
+   all_runs => 1,
+   catch_all=> 'CREATE ... commands',
+   create_order => 97,
+   create_sql   => 'CREATE STATISTICS 
dump_test.test_ext_stats_no_using
+   ON (col1, col2) FROM 
dump_test.test_fifth_table;',
+   regexp => qr/^
+   \QCREATE STATISTICS dump_test.test_ext_stats_no_using 
ON (col1, col2) FROM test_fifth_table;\E
+   /xms,
+   like => {
+   binary_upgrade  => 1,
+   clean   => 1,
+   clean_if_exists => 1,
+   createdb=> 1,
+   defaults=> 1,
+   exclude_test_table  => 1,
+   exclude_test_table_data => 1,
+   no_blobs=> 1,
+   no_privs=> 1,
+   no_owner=> 1,
+   only_dump_test_schema   => 1,
+   pg_dumpall_dbprivs  => 1,
+   schema_only => 1,
+   section_post_data   => 1,
+   test_schema_plus_blobs  => 1,
+   with_oids   => 1, },
+   unlike => {
+   exclude_dump_test_schema => 1,
+   only_dump_test_table => 1,
+   pg_dumpall_globals   => 1,
+   pg_dumpall_globals_clean => 1,
+   role => 1,
+   section_pre_data => 1, }, },
+
'CREATE SEQUENCE test_table_col1_seq' => {
all_runs  => 1,
catch_all => 'CREATE ... commands',
-- 
2.11.0


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

[HACKERS] How huge does mvtest_huge need to be?

[Tom Lane]

Continuing to investigate possible speedups of the regression tests,
I noticed that some of the slower individual statements are those
dealing with mvtest_huge and mvtest_hugeview in matview.sql.
Cutting the size of mvtest_huge from 100K rows to 10K rows is enough
to halve the overall runtime of matview.sql, at least on the relatively
slow buildfarm animal I was checking this on.

I was going to propose doing that, but then looking at commit b69ec7cc9
which introduced these tables, I began to wonder why they're large at all.
Even a one-row matview would have been enough to test for the bug that
that commit fixed.

So ... is there a good reason to be using a large table here, and
if so what is it, and how big does the table really need to be
to provide useful test coverage?

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] CTE inlining

[Pavel Stehule]

2017-05-03 18:54 GMT+02:00 David Fetter :

> On Wed, May 03, 2017 at 01:27:38PM -0300, Claudio Freire wrote:
> > On Wed, May 3, 2017 at 11:31 AM, David Fetter  wrote:
> > > Are you aware of such an ORM which both supports WITH and doesn't
> > > also closely track PostgreSQL development?  I'm not.
> > >
> > > Even assuming that such a thing exists, it's not at all obvious to
> > > me that we should be stalling and/or putting in what will turn out
> > > to be misfeatures to accommodate it.
> >
> > I know SQLAlchemy does support CTEs, and lags quite considerably in
> > its support of the latest syntactic elements.
> >
> > For instance, it took them 8 months to support the "skip locked"
> > option.
>
> That is pretty strictly their problem.
>
> > Not sure whether that qualifies as "closely tracking" postgres for
> > you. Clearly they do track it, but that doesn't mean they're fast or
> > as fast as one would like/need.
>
> We can NOT make their tardiness a driver of our development.
>
> > Sure, that might not be enough to warrant the GUC. I would think so,
> > those are my 2 cents. YMMV.
>
> When we add a "temporary" GUC, we're taking on a gigantic burden.
> Either we support it forever somehow, or we put it on a deprecation
> schedule immediately and expect to be answering questions about it for
> years after it's been removed.
>
> -1 for the GUC.
>

Is possible to find consensus without GUC? I understand well, why GUC is
wrong, but I don't see any possible solution how to change current behave
and don't break lot of applications.

Regards

Pavel


> 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] CTE inlining

[David Fetter]

On Wed, May 03, 2017 at 01:27:38PM -0300, Claudio Freire wrote:
> On Wed, May 3, 2017 at 11:31 AM, David Fetter  wrote:
> > Are you aware of such an ORM which both supports WITH and doesn't
> > also closely track PostgreSQL development?  I'm not.
> >
> > Even assuming that such a thing exists, it's not at all obvious to
> > me that we should be stalling and/or putting in what will turn out
> > to be misfeatures to accommodate it.
> 
> I know SQLAlchemy does support CTEs, and lags quite considerably in
> its support of the latest syntactic elements.
> 
> For instance, it took them 8 months to support the "skip locked"
> option.

That is pretty strictly their problem.

> Not sure whether that qualifies as "closely tracking" postgres for
> you. Clearly they do track it, but that doesn't mean they're fast or
> as fast as one would like/need.

We can NOT make their tardiness a driver of our development.

> Sure, that might not be enough to warrant the GUC. I would think so,
> those are my 2 cents. YMMV.

When we add a "temporary" GUC, we're taking on a gigantic burden.
Either we support it forever somehow, or we put it on a deprecation
schedule immediately and expect to be answering questions about it for
years after it's been removed.

-1 for the GUC.

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] Cost of src/test/recovery and .../subscription tests

[Tom Lane]

Robert Haas  writes:
> On Wed, Apr 19, 2017 at 1:28 PM, Tom Lane  wrote:
>> I am going to say flat out that that's unacceptably long for
>> a test script that will be run dozens of times a day by the
>> buildfarm.  There isn't any other test script that takes more
>> than circa 90 seconds on that machine, and I don't think this
>> one should either.

> I think that's bunk.  If there are tests that are part of those test
> suites that are taking a long time to run and not providing meaningful
> coverage, then that's something that can be improved.  However, I
> reject the argument that a test running for a long time is in itself
> bad.  I'd rather have tests that run for a long time (and thus get run
> less often) than have no tests.

That's fine, the problem is that these are now in the buildfarm schedule,
meaning they are no longer in the "get run less often" group.

I've been poking at this further, and it seems like a lot of the problem
is not so much excessive coverage as badly implemented "wait" logic
causing the test to run a lot longer than it needs to.  I already fixed
one such issue in 7834d20b5, and there seem to be more, both in the test
scaffolding and the code-under-test.  Don't know how reasonable it is
to try to fix all this stuff at this point in the development cycle, but
I'm continuing to look into 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] CTE inlining

[Claudio Freire]

On Wed, May 3, 2017 at 11:31 AM, David Fetter  wrote:
> On Wed, May 03, 2017 at 11:26:27AM -0300, Claudio Freire wrote:
>> On Wed, May 3, 2017 at 2:19 AM, Craig Ringer  wrote:
>> >> Or we will choose WITH MATERIALIZE, and then the users aware of
>> >> the fencing (and using the CTEs for that purpose) will have to
>> >> modify the queries. But does adding MATERIALIZE quality as major
>> >> query rewrite?
>> >
>> > Hardly.
>> >
>> >> Perhaps combining this with a GUC would be a solution. I mean, a
>> >> GUC specifying the default behavior, and then INLINE /
>> >> MATERIALIZE for individual CTEs in a query?
>> >
>> > It'd be nice if we could do that for a couple of releases as an
>> > interim measure, but people will then get locked into relying on
>> > it, and we'll never be able to remove it.
>>
>> The proposed guc seems like a good idea, without which ORMs that
>> support CTEs would be at a loss.
>
> Are you aware of such an ORM which both supports WITH and doesn't also
> closely track PostgreSQL development?  I'm not.
>
> Even assuming that such a thing exists, it's not at all obvious to me
> that we should be stalling and/or putting in what will turn out to be
> misfeatures to accommodate it.

I know SQLAlchemy does support CTEs, and lags quite considerably in
its support of the latest syntactic elements.

For instance, it took them 8 months to support the "skip locked" option.

Not sure whether that qualifies as "closely tracking" postgres for
you. Clearly they do track it, but that doesn't mean they're fast or
as fast as one would like/need.

Sure, that might not be enough to warrant the GUC. I would think so,
those are my 2 cents. YMMV.


-- 
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] Cost of src/test/recovery and .../subscription tests

[Robert Haas]

On Wed, Apr 19, 2017 at 1:28 PM, Tom Lane  wrote:
> So I updated longfin to the new release of the buildfarm client,
> and was quite dismayed by the fact that its cycle time went
> from 16 minutes to 24.  Some of that might be random effects like
> the state of the kernel disk caches, but a large chunk of it
> --- over 5 minutes --- evidently is from src/test/recovery/,
> which the buildfarm script didn't run before and now does.
>
> I am going to say flat out that that's unacceptably long for
> a test script that will be run dozens of times a day by the
> buildfarm.  There isn't any other test script that takes more
> than circa 90 seconds on that machine, and I don't think this
> one should either.

I think that's bunk.  If there are tests that are part of those test
suites that are taking a long time to run and not providing meaningful
coverage, then that's something that can be improved.  However, I
reject the argument that a test running for a long time is in itself
bad.  I'd rather have tests that run for a long time (and thus get run
less often) than have no tests.

Mind you, I'm not entirely sanguine about the large increase in time
that it takes to run make check-world on my local machine vs. a few
years ago.  It's annoying, and optimizing it would be good.  I think
the TAP test framework encourages people to add possibly-lengthy tests
pretty freely, because you can stick the test inside of a loop and run
it in many slightly different configurations, so people do, maybe
without giving enough thought to the value of those tests.  But
outlawing long-running tests is not the right solution to that
problem.

-- 
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] transition table behavior with inheritance appears broken (was: Declarative partitioning - another take)

[David Fetter]

On Wed, May 03, 2017 at 11:47:04AM -0400, Robert Haas wrote:
> On Tue, May 2, 2017 at 9:44 PM, Thomas Munro
>  wrote:
> > I think that we should only capture transition tuples captured from
> > the explicitly named relation, since we only fire AFTER STATEMENT
> > triggers on that relation.  I see no inconsistency with the policy of
> > rejecting transition tables on partitioned tables (as I proposed and
> > Kevin accepted[1]), because partitioned tables can't have any data so
> > there would be no point.  In contrast, every parent table in an
> > inheritance hierarchy is also a regular table and can hold data, so I
> > think we should allow transition tables on them, and capture
> > transition tuples from that table only when you modify it directly.
> 
> I suspect that most users would find it more useful to capture all of
> the rows that the statement actually touched, regardless of whether
> they hit the named table or an inheritance child.  I just don't know
> if it's practical to make that work.  (And, of course, I don't know if
> other people agree with my assessment of what is useful ... but
> generally there seems to be support for making partitioned tables, at
> least, look more like a single table that happens to have partitions
> and less like a bunch of separate tables attached to each other with
> duct tape.)

+1 on the not-duct-tape view of partitioned tables.

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] transition table behavior with inheritance appears broken (was: Declarative partitioning - another take)

[Robert Haas]

On Wed, May 3, 2017 at 12:02 PM, Alvaro Herrera
 wrote:
> Robert Haas wrote:
>> I suspect that most users would find it more useful to capture all of
>> the rows that the statement actually touched, regardless of whether
>> they hit the named table or an inheritance child.
>
> Yes, agreed.  For the plain inheritance cases each row would need to
> have an indicator of which relation it comes from (tableoid); I'm not
> sure if such a thing would be useful in the partitioning case.

I think it would be about equally useful.

-- 
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] password_encryption, default and 'plain' support

[Tom Lane]

Robert Haas  writes:
> On Wed, May 3, 2017 at 7:31 AM, Heikki Linnakangas  wrote:
>> So, I propose that we remove support for password_encryption='plain' in
>> PostgreSQL 10. If you try to do that, you'll get an error.

> I have no idea how widely used that option is.

Is it possible that there are still client libraries that don't support
password encryption at all?  If so, are we willing to break them?
I'd say "yes" but it's worth thinking about.

>> Another question that's been touched upon but not explicitly discussed, is
>> whether we should change the default to "scram-sha-256". I propose that we
>> do that as well. If you need to stick to md5, e.g. because you use drivers
>> that don't support SCRAM yet, you can change it in postgresql.conf, but the
>> majority of installations that use modern clients will be more secure by
>> default.

> I think that we should investigate how many connectors have support
> for SCRAM or are likely to do so by the time v10 is released.  A *lot*
> of people are using connectors that are not based on libpq, especially
> JDBC but I think many of the others as well.

Yes, there's an awful lot out there besides libpq.  I do not think it is
reasonable at all to change this default in v10.  Maybe v11, depending on
how fast JDBC et al move.  If we try to force it in v10, we are going to
get a heck of a lot of complaints about "I changed my password and now
I can't get in at all using ", where  is also going to include
back-rev psql.  (And I'm not even considering the possibility of nasty
bugs in the SCRAM code.)  Making SCRAM the default in the first version
where it's even available is moving way too fast IMO.

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] transition table behavior with inheritance appears broken (was: Declarative partitioning - another take)

[Alvaro Herrera]

Robert Haas wrote:

> I suspect that most users would find it more useful to capture all of
> the rows that the statement actually touched, regardless of whether
> they hit the named table or an inheritance child.

Yes, agreed.  For the plain inheritance cases each row would need to
have an indicator of which relation it comes from (tableoid); I'm not
sure if such a thing would be useful in the partitioning case.

-- 
Á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] password_encryption, default and 'plain' support

[Robert Haas]

On Wed, May 3, 2017 at 7:31 AM, Heikki Linnakangas  wrote:
> In various threads on SCRAM, we've skirted around the question of whether we
> should still allow storing passwords in plaintext. I've avoided discussing
> that in those other threads, because it's been an orthogonal question, but
> it's a good question and we should discuss it.
>
> So, I propose that we remove support for password_encryption='plain' in
> PostgreSQL 10. If you try to do that, you'll get an error.

I have no idea how widely used that option is.

> Another question that's been touched upon but not explicitly discussed, is
> whether we should change the default to "scram-sha-256". I propose that we
> do that as well. If you need to stick to md5, e.g. because you use drivers
> that don't support SCRAM yet, you can change it in postgresql.conf, but the
> majority of installations that use modern clients will be more secure by
> default.

I think that we should investigate how many connectors have support
for SCRAM or are likely to do so by the time v10 is released.  A *lot*
of people are using connectors that are not based on libpq, especially
JDBC but I think many of the others as well.  If most of those are
going to support SCRAM by the time v10 comes out, cool, but if not,
maybe it's wise to hold off for a release before flipping the default.
Not sure.

-- 
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] CTE inlining

[Thomas Kellerer]

> I could tolerate telling people to use OFFSET 0 (and documenting it!)
> as a workaround if we can't get something more friendly in.

I agree with that. 

> If we go with WITH INLINE then we're likely not solving anything, because
> most people will simply use WITH just like now, and will be subject to the
> fencing without realizing it.

I agree - the default behaviour should be change to match what everybody
expects. The current behaviour should be the exception. 

> Yes, and we're missing the opportunity to confirm with what other
> systems do, and the spirit of the SQL language's declare what I want,
> not how to do it, model.

Essentially *all* other systems optimize CTEs the same way they optimize
derived tables. I think even MySQL does it like that in the upcoming 8.0
release. 

I have never heard anyone saying that the Postgres implementation is an
advantage and that they would hate to see this disappear. I usually hear
"Why is Postgres doing that? Can't they change that?" 

Maybe I have a limited view on this, but from where I stand, simply changing
it would help everybody I know and would not break anything. I don't even
think a replacement for the old behaviour would be necessary.





--
View this message in context: 
http://www.postgresql-archive.org/CTE-inlining-tp5958992p5959509.html
Sent from the PostgreSQL - hackers mailing list archive at Nabble.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] transition table behavior with inheritance appears broken (was: Declarative partitioning - another take)

[Robert Haas]

On Tue, May 2, 2017 at 9:44 PM, Thomas Munro
 wrote:
> I think that we should only capture transition tuples captured from
> the explicitly named relation, since we only fire AFTER STATEMENT
> triggers on that relation.  I see no inconsistency with the policy of
> rejecting transition tables on partitioned tables (as I proposed and
> Kevin accepted[1]), because partitioned tables can't have any data so
> there would be no point.  In contrast, every parent table in an
> inheritance hierarchy is also a regular table and can hold data, so I
> think we should allow transition tables on them, and capture
> transition tuples from that table only when you modify it directly.

I suspect that most users would find it more useful to capture all of
the rows that the statement actually touched, regardless of whether
they hit the named table or an inheritance child.  I just don't know
if it's practical to make that work.  (And, of course, I don't know if
other people agree with my assessment of what is useful ... but
generally there seems to be support for making partitioned tables, at
least, look more like a single table that happens to have partitions
and less like a bunch of separate tables attached to each other with
duct tape.)

-- 
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] WITH clause in CREATE STATISTICS

[Alvaro Herrera]

Simon Riggs wrote:

> 2.
> USING keyword, no brackets
> CREATE STATISTICS s1 USING (dependencies, ndistinct) ON (a, b) FROM t1
> WHERE partial-stuff;
> 
> and if there are options, use the WITH for the optional parameters like this
> CREATE STATISTICS s1 USING (dependencies, ndistinct) WITH (options) ON
> (a, b) FROM t1 WHERE partial-stuff;
> 
> 
> I think I like (2)

OK, sounds sensible.

Note that the USING list is also optional -- if you don't specify it, we
default to creating all stat types.  Also note that we currently don't
have any option other than stat types, so the WITH would always be empty
-- in other words we should remove it until we implement some option.

(I can readily see two possible options to implement for pg11, so the
omission of the WITH clause would be temporary:
 1. sample size to use instead of the per-column values
 2. whether to forcibly collect stats for all column for this stat
 object even if the column has gotten a SET STATISTICS 0
Surely there can be others.)

Patch attached that adds the USING clause replacing the WITH clause,
which is also optional and only accepts statistic types (it doesn't
accept "foo = OFF" anymore, as it seems pointless, but I'm open to
accepting it if people care about it.)

(This patch removes WITH, but I verified that bison accepts having both.
The second attached reversed patch is what I used for removal.)

In the meantime, I noticed that pg_dump support for extstats is not
covered, which I'll go fix next.

-- 
Álvaro Herrerahttps://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
diff --git a/doc/src/sgml/perform.sgml b/doc/src/sgml/perform.sgml
index b10b734b90..3406b7a1cd 100644
--- a/doc/src/sgml/perform.sgml
+++ b/doc/src/sgml/perform.sgml
@@ -1132,7 +1132,7 @@ WHERE tablename = 'road';
  To inspect functional dependencies on a statistics
  stts, you may do this:
 
-CREATE STATISTICS stts WITH (dependencies)
+CREATE STATISTICS stts USING (dependencies)
ON (zip, city) FROM zipcodes;
 ANALYZE zipcodes;
 SELECT stxname, stxkeys, stxdependencies
@@ -1219,7 +1219,7 @@ EXPLAIN (ANALYZE, TIMING OFF) SELECT * FROM t WHERE a = 1 
AND b = 10;
  Continuing the above example, the n-distinct coefficients in a ZIP
  code table may look like the following:
 
-CREATE STATISTICS stts2 WITH (ndistinct)
+CREATE STATISTICS stts2 USING (ndistinct)
ON (zip, state, city) FROM zipcodes;
 ANALYZE zipcodes;
 SELECT stxkeys AS k, stxndistinct AS nd
diff --git a/doc/src/sgml/planstats.sgml b/doc/src/sgml/planstats.sgml
index f4430eb23c..16c433c3a2 100644
--- a/doc/src/sgml/planstats.sgml
+++ b/doc/src/sgml/planstats.sgml
@@ -526,7 +526,7 @@ EXPLAIN (ANALYZE, TIMING OFF) SELECT * FROM t WHERE a = 1 
AND b = 1;
 multivariate statistics on the two columns:
 
 
-CREATE STATISTICS stts WITH (dependencies) ON (a, b) FROM t;
+CREATE STATISTICS stts USING (dependencies) ON (a, b) FROM t;
 ANALYZE t;
 EXPLAIN (ANALYZE, TIMING OFF) SELECT * FROM t WHERE a = 1 AND b = 1;
   QUERY PLAN   
@@ -569,7 +569,7 @@ EXPLAIN (ANALYZE, TIMING OFF) SELECT COUNT(*) FROM t GROUP 
BY a, b;
 calculation, the estimate is much improved:
 
 DROP STATISTICS stts;
-CREATE STATISTICS stts WITH (dependencies, ndistinct) ON (a, b) FROM t;
+CREATE STATISTICS stts USING (dependencies, ndistinct) ON (a, b) FROM t;
 ANALYZE t;
 EXPLAIN (ANALYZE, TIMING OFF) SELECT COUNT(*) FROM t GROUP BY a, b;
QUERY PLAN  
  
diff --git a/doc/src/sgml/ref/create_statistics.sgml 
b/doc/src/sgml/ref/create_statistics.sgml
index edbcf5840b..ff6ed0668f 100644
--- a/doc/src/sgml/ref/create_statistics.sgml
+++ b/doc/src/sgml/ref/create_statistics.sgml
@@ -22,7 +22,7 @@ PostgreSQL documentation
  
 
 CREATE STATISTICS [ IF NOT EXISTS ] statistics_name
-WITH ( option [= value] [, ... ] )
+USING ( statistic_type [, ... 
] )
 ON ( column_name, 
column_name [, ...])
 FROM table_name
 
@@ -103,14 +103,14 @@ CREATE STATISTICS [ IF NOT EXISTS ] statistics_na
  
 

-The WITH clause can specify options
-for the statistics. Available options are listed below.
+The USING clause can specify types of statistics
+to be enabled. Available types are listed below.

 

 

-dependencies (boolean)
+dependencies
 
  
   Enables functional dependencies for the statistics.
@@ -119,7 +119,7 @@ CREATE STATISTICS [ IF NOT EXISTS ] statistics_na

 

-ndistinct (boolean)
+ndistinct
 
  
   Enables ndistinct coefficients for the statistics.
diff --git a/src/backend/commands/statscmds.c b/src/backend/commands/statscmds.c
index 0b9c33e30a..f4d1712091 100644
--- a/src/backend/commands/statscmds.c
+++ b/src/backend/commands/statscmds.c
@@ -194,23 +194,22 @@ CreateStatistics(CreateStatsStmt *stmt)
stxkeys = 

[HACKERS] WIP Patch: Precalculate stable functions, infrastructure v1

[Marina Polyakova]

Hello everyone again!

This is the continuation of my previous patch on the same topic; here 
there are changes made thanks to Tom Lane comments (see thread here 
[1]). To not send big patch I have split it (that's why version starts 
with the first again) and here I send infrastructure patch which 
includes:

- creation of CachedExpr node
- usual node functions for it
- mutator to replace nonovolatile functions' and operators' expressions 
by appropriate cached expressions.


Any suggestions are welcome!

[1] 
https://www.postgresql.org/message-id/flat/98c77534fa51aa4bf84a5b39931c42ea%40postgrespro.ru#98c77534fa51aa4bf84a5b39931c4...@postgrespro.ru


--
Marina Polyakova
Postgres Professional: http://www.postgrespro.com
The Russian Postgres CompanyFrom d7871c9aaf64210130b591a93c13b18c74ebb2b4 Mon Sep 17 00:00:00 2001
From: Marina Polyakova 
Date: Wed, 3 May 2017 18:09:16 +0300
Subject: [PATCH] Precalculate stable functions, infrastructure v1

Now in Postgresql only immutable functions are precalculated; stable functions
are calculated for every row so in fact they don't differ from volatile
functions.

This patch includes:
- creation of CachedExpr node
- usual node functions for it
- mutator to replace nonovolatile functions' and operators' expressions by
appropriate cached expressions.
---
 src/backend/nodes/copyfuncs.c|  22 +++
 src/backend/nodes/equalfuncs.c   |  22 +++
 src/backend/nodes/nodeFuncs.c| 121 ++
 src/backend/nodes/outfuncs.c |  32 +
 src/backend/nodes/readfuncs.c|  33 ++
 src/backend/optimizer/plan/planner.c | 124 +++
 src/include/nodes/nodeFuncs.h|   2 +
 src/include/nodes/nodes.h|   1 +
 src/include/nodes/primnodes.h|  32 +
 9 files changed, 389 insertions(+)

diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c
index 35a237a..1a16e3a 100644
--- a/src/backend/nodes/copyfuncs.c
+++ b/src/backend/nodes/copyfuncs.c
@@ -1529,6 +1529,25 @@ _copyNullIfExpr(const NullIfExpr *from)
 	return newnode;
 }
 
+static CachedExpr *
+_copyCachedExpr(const CachedExpr *from)
+{
+	CachedExpr *newnode = makeNode(CachedExpr);
+
+	COPY_SCALAR_FIELD(subexprtype);
+	switch(from->subexprtype)
+	{
+		case CACHED_FUNCEXPR:
+			COPY_NODE_FIELD(subexpr.funcexpr);
+			break;
+		case CACHED_OPEXPR:
+			COPY_NODE_FIELD(subexpr.opexpr);
+			break;
+	}
+ 
+ 	return newnode;
+}
+
 /*
  * _copyScalarArrayOpExpr
  */
@@ -4869,6 +4888,9 @@ copyObjectImpl(const void *from)
 		case T_NullIfExpr:
 			retval = _copyNullIfExpr(from);
 			break;
+		case T_CachedExpr:
+			retval = _copyCachedExpr(from);
+			break;
 		case T_ScalarArrayOpExpr:
 			retval = _copyScalarArrayOpExpr(from);
 			break;
diff --git a/src/backend/nodes/equalfuncs.c b/src/backend/nodes/equalfuncs.c
index 21dfbb0..5a0929a 100644
--- a/src/backend/nodes/equalfuncs.c
+++ b/src/backend/nodes/equalfuncs.c
@@ -384,6 +384,25 @@ _equalNullIfExpr(const NullIfExpr *a, const NullIfExpr *b)
 }
 
 static bool
+_equalCachedExpr(const CachedExpr *a, const CachedExpr *b)
+{
+	COMPARE_SCALAR_FIELD(subexprtype);
+
+	/* the same subexprtype for b because we have already compared it */
+	switch(a->subexprtype)
+	{
+		case CACHED_FUNCEXPR:
+			COMPARE_NODE_FIELD(subexpr.funcexpr);
+			break;
+		case CACHED_OPEXPR:
+			COMPARE_NODE_FIELD(subexpr.opexpr);
+			break;
+	}
+ 
+ 	return true;
+ }
+
+static bool
 _equalScalarArrayOpExpr(const ScalarArrayOpExpr *a, const ScalarArrayOpExpr *b)
 {
 	COMPARE_SCALAR_FIELD(opno);
@@ -3031,6 +3050,9 @@ equal(const void *a, const void *b)
 		case T_NullIfExpr:
 			retval = _equalNullIfExpr(a, b);
 			break;
+		case T_CachedExpr:
+			retval = _equalCachedExpr(a, b);
+			break;
 		case T_ScalarArrayOpExpr:
 			retval = _equalScalarArrayOpExpr(a, b);
 			break;
diff --git a/src/backend/nodes/nodeFuncs.c b/src/backend/nodes/nodeFuncs.c
index 3e8189c..9621511 100644
--- a/src/backend/nodes/nodeFuncs.c
+++ b/src/backend/nodes/nodeFuncs.c
@@ -32,6 +32,7 @@ static bool planstate_walk_subplans(List *plans, bool (*walker) (),
 void *context);
 static bool planstate_walk_members(List *plans, PlanState **planstates,
 	   bool (*walker) (), void *context);
+static const Node *get_const_subexpr(const CachedExpr *cachedexpr);
 
 
 /*
@@ -92,6 +93,9 @@ exprType(const Node *expr)
 		case T_NullIfExpr:
 			type = ((const NullIfExpr *) expr)->opresulttype;
 			break;
+		case T_CachedExpr:
+			type = exprType(get_const_subexpr((const CachedExpr *) expr));
+			break;
 		case T_ScalarArrayOpExpr:
 			type = BOOLOID;
 			break;
@@ -311,6 +315,8 @@ exprTypmod(const Node *expr)
 return exprTypmod((Node *) linitial(nexpr->args));
 			}
 			break;
+		case T_CachedExpr:
+			return exprTypmod(get_const_subexpr((const CachedExpr *) expr));
 		case T_SubLink:
 			{
 const SubLink *sublink = (const SubLink *) expr;
@@ -573,6 +579,10 @@ 

Re: [HACKERS] logical replication syntax (was DROP SUBSCRIPTION, query cancellations and slot handling)

[Petr Jelinek]

On 02/05/17 16:14, Petr Jelinek wrote:
> On 02/05/17 15:31, Tom Lane wrote:
>> Petr Jelinek  writes:
>>> Let me expand, if we don't drop the slot by default when dropping
>>> subscription, we'll have a lot of users with dead slots laying around
>>> holding back WAL/catalog_xmin, that's really bad. If we do drop by
>>> default like now, we need option to not do that, neither RESTRICT, nor
>>> CASCADE fit that.
>>
>> I'm not sure which part of "you can't have an option in DROP" isn't
>> clear to you.  Whatever the default behavior is always has to work,
>> because that is what's going to happen during DROP OWNED BY, or
>> DROP DATABASE. 
> 
> You are saying it like there was some guarantee that those commands
> can't fail because of other objects. AFAIK for example drop database can
> trivially fail just because there is replication slot in it before PG10
> (IIRC Craig managed to fix that in 10 for unrelated reasons).
> 

Btw looks like I already forgot how this stuff behaves. Existence of
subscription currently also prevents DROP DATABASE (for same reason
active backends do).

DROP OWNED BY ignores SUBSCRIPTION too now, although I think it might
not be strictly necessary to stay that way.

But if we keep this behavior then the point about these two commands
cascading to subscriptions is largely moot.

-- 
  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] CTE inlining

[David Fetter]

On Wed, May 03, 2017 at 11:26:27AM -0300, Claudio Freire wrote:
> On Wed, May 3, 2017 at 2:19 AM, Craig Ringer  wrote:
> >> Or we will choose WITH MATERIALIZE, and then the users aware of
> >> the fencing (and using the CTEs for that purpose) will have to
> >> modify the queries. But does adding MATERIALIZE quality as major
> >> query rewrite?
> >
> > Hardly.
> >
> >> Perhaps combining this with a GUC would be a solution. I mean, a
> >> GUC specifying the default behavior, and then INLINE /
> >> MATERIALIZE for individual CTEs in a query?
> >
> > It'd be nice if we could do that for a couple of releases as an
> > interim measure, but people will then get locked into relying on
> > it, and we'll never be able to remove it.
> 
> The proposed guc seems like a good idea, without which ORMs that
> support CTEs would be at a loss.

Are you aware of such an ORM which both supports WITH and doesn't also
closely track PostgreSQL development?  I'm not. 

Even assuming that such a thing exists, it's not at all obvious to me
that we should be stalling and/or putting in what will turn out to be
misfeatures to accommodate it.

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] check with serial

[Andrew Dunstan]

On 05/03/2017 10:12 AM, Tom Lane wrote:
> Andrew Dunstan  writes:
>> On 05/02/2017 10:13 PM, Vaishnavi Prabakaran wrote:
>>> And, now after your patch, do we still need "installcheck-parallel"
>>> command? It is redundant IMO, just give a thought.
>> I'd be quite happy to remove the target in favor of this more general
>> solution.
> -1 --- that will break a lot of existing habits to no purpose, not to
> mention creating complications for scripts used to test multiple branches.
> We have intentionally maintained backwards compatibility in these testing
> targets for a very long time, even though that's left us with
> inconsistencies like installcheck defaulting to serial while check
> defaults to parallel.  Adding a new capability is not an excuse for
> breaking the historical test targets.
>
>   



OK

cheers

andrew

-- 
Andrew Dunstanhttps://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] CTE inlining

[Claudio Freire]

On Wed, May 3, 2017 at 2:19 AM, Craig Ringer  wrote:
>> Or we will choose WITH MATERIALIZE, and then the users aware of the fencing
>> (and using the CTEs for that purpose) will have to modify the queries. But
>> does adding MATERIALIZE quality as major query rewrite?
>
> Hardly.
>
>> Perhaps combining this with a GUC would be a solution. I mean, a GUC
>> specifying the default behavior, and then INLINE / MATERIALIZE for
>> individual CTEs in a query?
>
> It'd be nice if we could do that for a couple of releases as an
> interim measure, but people will then get locked into relying on it,
> and we'll never be able to remove it.

The proposed guc seems like a good idea, without which ORMs that
support CTEs would be at a loss. People using those ORMs that need
materialized behavior would have to wait for the ORM to catch up with
postgres syntax before upgrading, and that wouldn't be a nice thing.

It's not about requiring testing before upgrading, of course users
should/will do that. But if said testing says inlined CTEs perform
horribly, and the ORM has no support for the materialized keyword, the
only option is to not upgrade. With the CTE, people can upgrade,
changing the default behavior back to what it was.

That seems to me a useful thing to have.


-- 
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] [PostgreSQL 10] default of hot_standby should be "on"?

[Michael Banck]

Hi,

On Tue, May 02, 2017 at 11:13:58AM +0200, Magnus Hagander wrote:
> Looks good to me as well. Applied, with only a minor further docs addition
> saying that this is the default also on the high availability page.

I understand this is late, but a colleague alerted me to the following
behaviour change: If you recover a server with default settings, it is
our understanding that pg_isready will now return true immediately after
the consistent state is reached and possibly well before recovery had
actually ended (depending on the amount of outstanding wal). As hot
standby works with log shipping, this is independent of the
recovery.conf settings, i.e. even if standby_mode and primary_conninfo
are not set. So if one was monitoring recovery like that before and
expects pg_isready to only return true once the recovery is fully
complete, this will now have to be adjusted. Also, if the recovered
server is to be used for transactions, there will now be a window where
the server accepts connections, but is in read-only mode.

Before, one had the make the concious choice to set hot_standby to get
the behaviour, now it might be surprising to users, or maybe I'm
overthinking this?

If that is indeed the case, maybe it should be mentioned more
prominently in the documentation and/or get highlighted in the release
notes?


Michael

-- 
Michael Banck
Projektleiter / Senior Berater
Tel.: +49 2166 9901-171
Fax:  +49 2166 9901-100
Email: michael.ba...@credativ.de

credativ GmbH, HRB Mönchengladbach 12080
USt-ID-Nummer: DE204566209
Trompeterallee 108, 41189 Mönchengladbach
Geschäftsführung: Dr. Michael Meskes, Jörg Folz, Sascha Heuer


-- 
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] check with serial

[Tom Lane]

Andrew Dunstan  writes:
> On 05/02/2017 10:13 PM, Vaishnavi Prabakaran wrote:
>> And, now after your patch, do we still need "installcheck-parallel"
>> command? It is redundant IMO, just give a thought.

> I'd be quite happy to remove the target in favor of this more general
> solution.

-1 --- that will break a lot of existing habits to no purpose, not to
mention creating complications for scripts used to test multiple branches.
We have intentionally maintained backwards compatibility in these testing
targets for a very long time, even though that's left us with
inconsistencies like installcheck defaulting to serial while check
defaults to parallel.  Adding a new capability is not an excuse for
breaking the historical test targets.

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] check with serial

[Andrew Dunstan]

On 05/02/2017 10:13 PM, Vaishnavi Prabakaran wrote:
>
>
> On Tue, May 2, 2017 at 11:30 PM, Andrew Dunstan
>  > wrote:
>
>
> Here's a simple patch that does what I had in mind. It allows
> providing
> for an arbitrary schedule file in both the check and installcheck
> recipes. The historic behaviour is preserved.
>
>
> Hmm, installcheck command with SCHEDULE set as "Parallel" does not
> honor "MAXCONNOPT" settings in the attached patch.


good point.

>  
> And, now after your patch, do we still need "installcheck-parallel"
> command? It is redundant IMO, just give a thought.


I'd be quite happy to remove the target in favor of this more general
solution.

Thoughts?

>
> Documentation changes("Running the Tests") are also required as the
> behavior documented is now changed in this patch.


Yes, agreed. Thanks for your comments.


cheers

andrew

-- 
Andrew Dunstanhttps://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] renaming "transaction log"

[Alvaro Herrera]

Peter Eisentraut wrote:
> Most documentation and error messages still uses the term "transaction
> log" to refer to the write-ahead log.  Here is a patch to rename that,
> which I think should be done, to match the xlog -> wal renaming in APIs.

+1 for the idea.  I suggest grepping for "transaction$" too -- there are
a few cases in SGML that have the phrase "transaction log" split across
lines.

-- 
Á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] CTE inlining

[Andrew Dunstan]

On 05/02/2017 07:00 PM, Tomas Vondra wrote:
>
>
>
> I think we agree that:
>
> * Just removing the optimization fence and telling users to use OFFSET
> 0 instead is a no-go, just like removing the fencing and not providing
> any sensible replacement.
>
> * GUC is not the solution.


yes

>
> Which leaves us with either WITH INLINE or WITH MATERIALIZE, or
> something along those lines.
>
> If we go with WITH INLINE then we're likely not solving anything,
> because most people will simply use WITH just like now, and will be
> subject to the fencing without realizing it.


In many cases it won't actually matter much.

We're going to penalize some group of users no matter what we do. It
just seems a pity that it might be the group who actually took us at our
word. It's no skin off my nose - I will happily spend time finding
places where this will make things worse for customers.


>
> Or we will choose WITH MATERIALIZE, and then the users aware of the
> fencing (and using the CTEs for that purpose) will have to modify the
> queries. But does adding MATERIALIZE quality as major query rewrite?


It's not a major rewrite. But I can think of at least one former
customer who will have to go through a heck of a lot of code finding
where to add that one word.


>
> Perhaps combining this with a GUC would be a solution. I mean, a GUC
> specifying the default behavior, and then INLINE / MATERIALIZE for
> individual CTEs in a query?
>
> If you have an application intentionally using CTEs as a fence, just do
>
> ALTER DATABASE x SET enable_guc_fencing = on
>
> and you don't have to rewrite the queries.
>


ITYM enable_cte_fencing.

I'm not sure it will help all that much unless we provide both decorator
variants, so people can remediate their code one query at a time, and
this guc would just govern the default.


cheers

andrew

-- 
Andrew Dunstanhttps://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] Error message on missing SCRAM authentication with older clients

[Aleksander Alekseev]

Hi Magnus,

> +1, even though it's not strictly speaking a bugfix to go in a backpatch, I
> think it will help enough users that it's worth doing. And I can't see how
> it could possibly be unsafe...

Well, strictly speaking there could be applications that parse error
messages using regular expressions or something like this. But I don't
think it's something we should really bother about.

-- 
Best regards,
Aleksander Alekseev


signature.asc
Description: PGP signature

Re: [HACKERS] renaming "transaction log"

[David Steele]

On 5/2/17 9:09 PM, Peter Eisentraut wrote:
> Most documentation and error messages still uses the term "transaction
> log" to refer to the write-ahead log.  Here is a patch to rename that,
> which I think should be done, to match the xlog -> wal renaming in APIs.

+1 for the idea.

The documentation changes look good to me, but the error message changes
are a random mix of "WAL" and "write-ahead log", even when referring to
the same thing.  For example:

- errmsg("could not open transaction log directory \"%s\": %m",
+ errmsg("could not open write-ahead log directory \"%s\": %m",

and:

- fprintf(stderr, _("%s: failed to remove transaction log directory\n"),
+ fprintf(stderr, _("%s: failed to remove WAL directory\n"),

It seems like they should be one or the other.  I think "write-ahead
log" is good in the documentation since it is more explanatory, but the
error messages should either be all "write-ahead log" or all "WAL".

Personally I favor "WAL" for brevity in the error messages, but I would
be OK with "write-ahead log", too.

-- 
-David
da...@pgmasters.net


-- 
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] [POC] hash partitioning

[amul sul]

On Thu, Apr 27, 2017 at 1:42 AM, Robert Haas  wrote:

>I spent some time today looking at these patches.  It seems like there
>is some more work still needed here to produce something committable
>regardless of which way we go, but I am inclined to think that Amul's
>patch is a better basis for work going forward than Nagata-san's
>patch. Here are some general comments on the two patches:

Thanks for your time.

[...]

> - Neither patch contains any documentation updates, which is bad.

Fixed in the attached version.

>
> Nagata-san's patch also contains no regression tests.  Amul's patch
> does, but they need to be rebased, since they no longer apply, and I
> think some other improvements are possible as well.  It's probably not
> necessary to re-test things like whether temp and non-temp tables can
> be mixed within a partitioning hierarchy, but there should be tests
> that tuple routing actually works.  The case where it fails because no
> matching partition exists should be tested as well.  Also, the tests
> should validate not only that FOR VALUES isn't accept when creating a
> hash partition (which they do) but also that WITH (...) isn't accepted
> for a range or list partition (which they do not).
>

Fixed in the attached version.

[...]
> - Amul's patch should perhaps update tab completion support:  create
> table foo1 partition of foo  completes with "for values", but now
> "with" will be another option.
>

Fixed in the attached version.

>
> - Amul's patch probably needs to validate the WITH () clause more
> thoroughly.  I bet you get a not-very-great error message if you leave
> out "modulus" and no error at all if you leave out "remainder".
>

Thats not true, there will be syntax error if you leave modulus or
remainder, see this:

postgres=# CREATE TABLE hpart_2 PARTITION OF hash_parted  WITH(modulus 4);
ERROR:  syntax error at or near ")"
LINE 1: ...hpart_2 PARTITION OF hash_parted WITH(modulus 4);

>
> This is not yet a detailed review - I may be missing things, and
> review and commentary from others is welcome.  If there is no major
> disagreement with the idea of moving forward using Amul's patch as a
> base, then I will do a more detailed review of that patch (or,
> hopefully, an updated version that addresses the above comments).
>

I have made a smaller change in earlier proposed syntax to create
partition to be aligned with current range and list partition syntax,
new syntax will be as follow:

CREATE TABLE p1 PARTITION OF hash_parted FOR VALUES WITH (modulus 10,
remainder 1);

Regards,
Amul


hash-partitioning_another_design-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] password_encryption, default and 'plain' support

[Magnus Hagander]

On Wed, May 3, 2017 at 2:25 PM, Michael Paquier 
wrote:

> On Wed, May 3, 2017 at 8:38 PM, Magnus Hagander 
> wrote:
> > On Wed, May 3, 2017 at 1:31 PM, Heikki Linnakangas 
> wrote:
> >> In various threads on SCRAM, we've skirted around the question of
> whether
> >> we should still allow storing passwords in plaintext. I've avoided
> >> discussing that in those other threads, because it's been an orthogonal
> >> question, but it's a good question and we should discuss it.
> >>
> >> So, I propose that we remove support for password_encryption='plain' in
> >> PostgreSQL 10. If you try to do that, you'll get an error.
> >
> > Is there any usecase at all for it today?
>
> For developers running applications on top of Postgres?
>

I don't get it. How does password_encryption=plain help them?


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

Re: [HACKERS] Re: [BUGS] BUG #14634: On Windows pg_basebackup should write tar to stdout in binary mode

[Ashutosh Sharma]

Hi Craig,

On Wed, May 3, 2017 at 10:50 AM, Craig Ringer  wrote:
> On 3 May 2017 at 12:32, Haribabu Kommi  wrote:
>> [Adding -hackers mailing list]
>>
>> On Fri, Apr 28, 2017 at 6:28 PM,  wrote:
>>>
>>> The following bug has been logged on the website:
>>>
>>> Bug reference:  14634
>>> Logged by:  Henry Boehlert
>>> Email address:  henry_boehl...@agilent.com
>>> PostgreSQL version: 9.6.2
>>> Operating system:   Windows Server 2012 R2 6.3.9600
>>> Description:
>>>
>>> Executing command pg_basebackup -D -F t writes its output to stdout, which
>>> is open in text mode, causing LF to be converted to CR LF thus corrupting
>>> the resulting archive.
>>>
>>> To write the tar to stdout, on Windows stdout's mode should be temporarily
>>> switched to binary.
>>>
>>> https://msdn.microsoft.com/en-us/library/tw4k6df8.aspx
>>
>>
>> Thanks for reporting the issue.
>> With the attached patch, I was able to extract the tar file that gets
>> generated when the tar file is written into stdout. I tested the
>> the compressed tar also.
>>
>> This bug needs to be fixed in back branches also.
>
> We should do the same for pg_dump in -Fc mode.

Did you meant -Fp mode. I think we are already setting stdout file to
binary mode if the output format is custom. Please refer to the
following code in parseArchiveFormat() and _allocAH() respectively

static ArchiveFormat
parseArchiveFormat(const char *format, ArchiveMode *mode)
{
...
...
else if (pg_strcasecmp(format, "c") == 0)
archiveFormat = archCustom;
else if (pg_strcasecmp(format, "custom") == 0)
archiveFormat = archCustom;

else if (pg_strcasecmp(format, "p") == 0)
archiveFormat = archNull;
else if (pg_strcasecmp(format, "plain") == 0)
archiveFormat = archNull;
...
...
}

static ArchiveHandle *
_allocAH(const char *FileSpec, const ArchiveFormat fmt,
 const int compression, bool dosync, ArchiveMode mode,
 SetupWorkerPtrType setupWorkerPtr)
{

...
...
#ifdef WIN32
if (fmt != archNull &&
(AH->fSpec == NULL || strcmp(AH->fSpec, "") == 0))
{
if (mode == archModeWrite)
setmode(fileno(stdout), O_BINARY);
else
setmode(fileno(stdin), O_BINARY);
}
#endif
..
..
}

Please confirm.

Meanwhile, I have unit tested the patch submitted by Hari upthread on
postgresql v10 and it works fine. Below are the steps that i have
followed to test Hari's patch.

Without patch:
==
C:\Users\ashu\postgresql\TMP\test\bin>.\pg_basebackup.exe -D - -F t -X
none > base.tar
NOTICE:  WAL archiving is not enabled; you must ensure that all required WAL seg
ments are copied through other means to complete the backup

C:\Users\ashu\postgresql\TMP\test\bin>tar -xf base.tar
tar: Skipping to next header
tar: Exiting with failure status due to previous errors


With patch:
===
C:\Users\ashu\git-clone-postgresql\postgresql\TMP\test\bin>.\pg_basebackup.exe
-D - -F t -X none > base.tar
NOTICE:  WAL archiving is not enabled; you must ensure that all required WAL seg
ments are copied through other means to complete the backup

C:\Users\ashu\postgresql\TMP\test\bin>cp base.tar ..\basebakup

C:\Users\ashu\postgresql\TMP\test\basebakup>tar -xf base.tar

C:\Users\ashu\postgresql\TMP\test\basebakup>ls
PG_VERSIONpg_commit_ts   pg_multixact  pg_stat  pg_wal
backup_label  pg_dynshmempg_notify pg_stat_tmp  pg_xact
base  pg_hba.confpg_replslot   pg_subtrans  postgresql.auto.conf
base.tar  pg_ident.conf  pg_serial pg_tblspcpostgresql.conf
globalpg_logical pg_snapshots  pg_twophase  tablespace_map

--
With Regards,
Ashutosh Sharma
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] scram and \password

[Michael Paquier]

On Wed, May 3, 2017 at 5:26 PM, Heikki Linnakangas  wrote:
> Ok, gotcha. I disagree, I think we should provide a default. Libpq is in a
> better position to make a good choice than most applications.
>
> I've committed the new PQencryptPasswordConn function, with the default
> behavior of doing "show password_encryption", and the changes to use it in
> psql and createuser. This closes the open issue with \password.

Well, there is always the counter argument that applications can check
for password_encryption by themselves and complete
PQencryptPasswordConn and that would be a couple of extra lines in any
applications. But honestly, people will appreciate a way to rely on
what the backend uses automatically.

> On 04/27/2017 07:03 AM, Michael Paquier wrote:
>> I think that it is a mistake to move SASLprep out of
>> scram_build_verifier, because pre-processing the password is not
>> necessary, it is normally mandatory. The BE/FE versions that you are
>> adding also duplicate the calls to pg_saslprep().
>
> I played with that a little bit, but decided to keep pg_saslprep() out of
> scram_build_verifier() after all. It would seem asymmetric to have
> scram_build_verifier() call pg_saslprep(), but require callers of
> scram_SaltedPassword() to call it. So for consistency, I think
> scram_SaltedPassword() should also call pg_saslprep(). That would
> complicated the scram_SaltedPassword() function, however. It would need to
> report an OOM error somehow, for starters. Not an insurmountable issue, of
> course, but it felt cleaner this way, after all, despite the duplication.

Okay, I won't fight on that further.
-- 
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] Re: [BUGS] BUG #14634: On Windows pg_basebackup should write tar to stdout in binary mode

[Haribabu Kommi]

On Wed, May 3, 2017 at 3:20 PM, Craig Ringer  wrote:

> On 3 May 2017 at 12:32, Haribabu Kommi  wrote:
> > [Adding -hackers mailing list]
> >
> > On Fri, Apr 28, 2017 at 6:28 PM,  wrote:
> >>
> >> The following bug has been logged on the website:
> >>
> >> Bug reference:  14634
> >> Logged by:  Henry Boehlert
> >> Email address:  henry_boehl...@agilent.com
> >> PostgreSQL version: 9.6.2
> >> Operating system:   Windows Server 2012 R2 6.3.9600
> >> Description:
> >>
> >> Executing command pg_basebackup -D -F t writes its output to stdout,
> which
> >> is open in text mode, causing LF to be converted to CR LF thus
> corrupting
> >> the resulting archive.
> >>
> >> To write the tar to stdout, on Windows stdout's mode should be
> temporarily
> >> switched to binary.
> >>
> >> https://msdn.microsoft.com/en-us/library/tw4k6df8.aspx
> >
> >
> > Thanks for reporting the issue.
> > With the attached patch, I was able to extract the tar file that gets
> > generated when the tar file is written into stdout. I tested the
> > the compressed tar also.
> >
> > This bug needs to be fixed in back branches also.
>
> We should do the same for pg_dump in -Fc mode.


There are no "CR LF" characters that are present in the dump file
that is created with custom format.

what is the problem do you see in custom format that needs similar
handling like pg_basebackup?

Regards,
Hari Babu
Fujitsu Australia

Re: [HACKERS] password_encryption, default and 'plain' support

[Michael Paquier]

On Wed, May 3, 2017 at 8:38 PM, Magnus Hagander  wrote:
> On Wed, May 3, 2017 at 1:31 PM, Heikki Linnakangas  wrote:
>> In various threads on SCRAM, we've skirted around the question of whether
>> we should still allow storing passwords in plaintext. I've avoided
>> discussing that in those other threads, because it's been an orthogonal
>> question, but it's a good question and we should discuss it.
>>
>> So, I propose that we remove support for password_encryption='plain' in
>> PostgreSQL 10. If you try to do that, you'll get an error.
>
> Is there any usecase at all for it today?

For developers running applications on top of Postgres?

>> Another question that's been touched upon but not explicitly discussed, is
>> whether we should change the default to "scram-sha-256". I propose that we
>> do that as well. If you need to stick to md5, e.g. because you use drivers
>> that don't support SCRAM yet, you can change it in postgresql.conf, but the
>> majority of installations that use modern clients will be more secure by
>> default.
>
> Much as that's going to cause issues for some people, I think it's worth
> doing. We should probably put something specific in the release notes
> mentioning the error message you get in libpq, and possibly some of the
> other most common drivers.

My original view on the matter was, and is still, to wait for one or
two releases before switching the default to scram.
-- 
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] Error message on missing SCRAM authentication with older clients

[Magnus Hagander]

On Wed, May 3, 2017 at 10:58 AM, Heikki Linnakangas  wrote:

> Currently, if you use 9.6 libpq to connect to a v10 server that requires
> SCRAM authentication, you get an error:
>
> psql: authentication method 10 not supported
>
> I'd like to apply this small patch to all the stable branches, to give a
> nicer error message:
>
> psql: SCRAM authentication not supported by this version of libpq
>
> It won't help unless you upgrade to the latest minor version, of course,
> but it's better than nothing. Any objections?


+1, even though it's not strictly speaking a bugfix to go in a backpatch, I
think it will help enough users that it's worth doing. And I can't see how
it could possibly be unsafe...

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

Re: [HACKERS] Error message on missing SCRAM authentication with older clients

[Aleksander Alekseev]

Hi Heikki,

> psql: SCRAM authentication not supported by this version of libpq

Maybe it would be better to specify a minimum required version?

-- 
Best regards,
Aleksander Alekseev


signature.asc
Description: PGP signature

[HACKERS] Error message on missing SCRAM authentication with older clients

[Heikki Linnakangas]

Currently, if you use 9.6 libpq to connect to a v10 server that requires 
SCRAM authentication, you get an error:


psql: authentication method 10 not supported

I'd like to apply this small patch to all the stable branches, to give a 
nicer error message:


psql: SCRAM authentication not supported by this version of libpq

It won't help unless you upgrade to the latest minor version, of course, 
but it's better than nothing. Any objections?


- Heikki


backport-nicer-error-on-scram.patch
Description: invalid/octet-stream

-- 
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] password_encryption, default and 'plain' support

[Magnus Hagander]

On Wed, May 3, 2017 at 1:31 PM, Heikki Linnakangas  wrote:

> Hi,
>
> In various threads on SCRAM, we've skirted around the question of whether
> we should still allow storing passwords in plaintext. I've avoided
> discussing that in those other threads, because it's been an orthogonal
> question, but it's a good question and we should discuss it.
>
> So, I propose that we remove support for password_encryption='plain' in
> PostgreSQL 10. If you try to do that, you'll get an error.
>

Is there any usecase at all for it today?

+1 for getting rid of it :)



> Another question that's been touched upon but not explicitly discussed, is
> whether we should change the default to "scram-sha-256". I propose that we
> do that as well. If you need to stick to md5, e.g. because you use drivers
> that don't support SCRAM yet, you can change it in postgresql.conf, but the
> majority of installations that use modern clients will be more secure by
> default.


Much as that's going to cause issues for some people, I think it's worth
doing. We should probably put something specific in the release notes
mentioning the error message you get in libpq, and possibly some of the
other most common drivers.

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

[HACKERS] password_encryption, default and 'plain' support

[Heikki Linnakangas]

Hi,

In various threads on SCRAM, we've skirted around the question of 
whether we should still allow storing passwords in plaintext. I've 
avoided discussing that in those other threads, because it's been an 
orthogonal question, but it's a good question and we should discuss it.


So, I propose that we remove support for password_encryption='plain' in 
PostgreSQL 10. If you try to do that, you'll get an error.


Another question that's been touched upon but not explicitly discussed, 
is whether we should change the default to "scram-sha-256". I propose 
that we do that as well. If you need to stick to md5, e.g. because you 
use drivers that don't support SCRAM yet, you can change it in 
postgresql.conf, but the majority of installations that use modern 
clients will be more secure by default.


- 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] Logical replication - TRAP: FailedAssertion in pgstat.c

[Erik Rijkers]

On 2017-05-03 08:17, Petr Jelinek wrote:

On 02/05/17 20:43, Robert Haas wrote:

On Thu, Apr 20, 2017 at 2:58 PM, Peter Eisentraut



code path that calls CommitTransactionCommand() should have one, no?


Is there anything left to be committed here?



Afaics the fix was not committed. Peter wanted more comprehensive fix
which didn't happen. I think something like attached should do the job.


I'm running my pgbench-over-logical-replication test in chunk of 15 
minutes, wth different pgbench -c (num clients) and -s (scale) values.


With this patch (and nothing else)  on top of master (8f8b9be51fd7 to be 
precise):



fix-statistics-reporting-in-logical-replication-work.patch


logical replication is still often failing (as expected, I suppose; it 
seems because of "inital snapshot too large") but indeed I do not see 
the 'TRAP: FailedAssertion in pgstat.c' anymore.


(If there is any other configuration of patches worth testing please let 
me know)


thanks

Erik Rijkers



--
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] Logical replication ApplyContext bloat

[Stas Kelvich]

> On 20 Apr 2017, at 17:01, Dilip Kumar  wrote:
> 
> On Thu, Apr 20, 2017 at 7:04 PM, Stas Kelvich  
> wrote:
>> Thanks for noting.
>> 
>> Added short description of ApplyContext and ApplyMessageContext to README.
> 
> Typo
> 
> /analysys/analysis
> 

Fixed, thanks.

Added this to open items list.



apply_contexts_3.patch
Description: Binary data


Stas Kelvich
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company

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

[HACKERS] Error message on missing SCRAM authentication with older clients

[Heikki Linnakangas]

Currently, if you use 9.6 libpq to connect to a v10 server that requires 
SCRAM authentication, you get an error:


psql: authentication method 10 not supported

I'd like to apply this small patch to all the stable branches, to give a 
nicer error message:


psql: SCRAM authentication not supported by this version of libpq

It won't help unless you upgrade to the latest minor version, of course, 
but it's better than nothing. Any objections?


- Heikki



backport-nicer-error-on-scram.patch
Description: invalid/octet-stream

-- 
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 and \password

[Magnus Hagander]

On Wed, May 3, 2017 at 10:26 AM, Heikki Linnakangas  wrote:

> On 05/02/2017 07:47 PM, Robert Haas wrote:
>
>> On Tue, May 2, 2017 at 3:42 AM, Heikki Linnakangas 
>> wrote:
>>
>>> There's going to be a default, one way or another. The default is going
>>> to
>>> come from password_encryption, or it's going to be a hard-coded value or
>>> logic based on server-version in PQencryptPasswordConn(). Or it's going
>>> to
>>> be a hard-coded value or logic implemented in every application that uses
>>> PQencryptPasswordConn(). I think looking at password_encryption makes the
>>> most sense. The application is not in a good position to make the
>>> decision,
>>> and forcing the end-user to choose every time they change a password is
>>> too
>>> onerous.
>>>
>>
>> I think there should be no default, and the caller should have to pass
>> the algorithm explicitly.  If they want to determine what default to
>> pass by running 'SHOW password_encryption', that's their choice.
>>
>
> Ok, gotcha. I disagree, I think we should provide a default. Libpq is in a
> better position to make a good choice than most applications.
>
> I've committed the new PQencryptPasswordConn function, with the default
> behavior of doing "show password_encryption", and the changes to use it in
> psql and createuser. This closes the open issue with \password.


If we're basically just telling people to call SHOW manually, we might as
well do it in the default case. I think the wording you put into the docs
there is good, as it tells people exactly what happens and how to reproduce
it locally.

For the security perspective, perhaps we should have a link to the part of
the docs that discusses the different algorithms?

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

Re: [HACKERS] scram and \password

[Heikki Linnakangas]

On 05/02/2017 07:47 PM, Robert Haas wrote:

On Tue, May 2, 2017 at 3:42 AM, Heikki Linnakangas  wrote:

There's going to be a default, one way or another. The default is going to
come from password_encryption, or it's going to be a hard-coded value or
logic based on server-version in PQencryptPasswordConn(). Or it's going to
be a hard-coded value or logic implemented in every application that uses
PQencryptPasswordConn(). I think looking at password_encryption makes the
most sense. The application is not in a good position to make the decision,
and forcing the end-user to choose every time they change a password is too
onerous.


I think there should be no default, and the caller should have to pass
the algorithm explicitly.  If they want to determine what default to
pass by running 'SHOW password_encryption', that's their choice.


Ok, gotcha. I disagree, I think we should provide a default. Libpq is in 
a better position to make a good choice than most applications.


I've committed the new PQencryptPasswordConn function, with the default 
behavior of doing "show password_encryption", and the changes to use it 
in psql and createuser. This closes the open issue with \password.


On 04/27/2017 07:03 AM, Michael Paquier wrote:

I think that it is a mistake to move SASLprep out of
scram_build_verifier, because pre-processing the password is not
necessary, it is normally mandatory. The BE/FE versions that you are
adding also duplicate the calls to pg_saslprep().


I played with that a little bit, but decided to keep pg_saslprep() out 
of scram_build_verifier() after all. It would seem asymmetric to have 
scram_build_verifier() call pg_saslprep(), but require callers of 
scram_SaltedPassword() to call it. So for consistency, I think 
scram_SaltedPassword() should also call pg_saslprep(). That would 
complicated the scram_SaltedPassword() function, however. It would need 
to report an OOM error somehow, for starters. Not an insurmountable 
issue, of course, but it felt cleaner this way, after all, despite the 
duplication.



Using "encrypt" instead of "hash" in the function name :(


Yeah. For better or worse, I've kept the "encrypt" nomenclature 
everywhere, for consistency.


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] proposal psql \gdesc

[Pavel Stehule]

2017-04-28 6:08 GMT+02:00 Pavel Stehule :

> Hi
>
> Sometimes I have to solve the result types of some query. It is invisible
> in psql. You have to materialize table or you have to create view. Now,
> when we can enhance \g command, we can introduce query describing
>
> some like
>
> select a, b from foo
> \gdesc
>
>  |   type | length | collation | 
> 
>  a  | varchar  | 30  |
>  b  | numeric |  20 |
>
>
here is the patch. It is based on PQdescribePrepared result.

postgres=# select * from pg_proc \gdesc
┌─┬──┐
│  Name   │ Type │
╞═╪══╡
│ proname │ name │
│ pronamespace│ oid  │
│ proowner│ oid  │
│ prolang │ oid  │
│ procost │ real │
│ prorows │ real │
│ provariadic │ oid  │
│ protransform│ regproc  │
│ proisagg│ boolean  │
│ proiswindow │ boolean  │
│ prosecdef   │ boolean  │
│ proleakproof│ boolean  │
│ proisstrict │ boolean  │
│ proretset   │ boolean  │
│ provolatile │ "char"   │
│ proparallel │ "char"   │
│ pronargs│ smallint │
│ pronargdefaults │ smallint │
│ prorettype  │ oid  │
│ proargtypes │ oidvector│
│ proallargtypes  │ oid[]│
│ proargmodes │ "char"[] │
│ proargnames │ text[]   │
│ proargdefaults  │ pg_node_tree │
│ protrftypes │ oid[]│
│ prosrc  │ text │
│ probin  │ text │
│ proconfig   │ text[]   │
│ proacl  │ aclitem[]│
└─┴──┘
(29 rows)



> What do you think about this idea?
>
> Regards
>
> Pavel
>
diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml
index 3b86612..03cba87 100644
--- a/doc/src/sgml/ref/psql-ref.sgml
+++ b/doc/src/sgml/ref/psql-ref.sgml
@@ -1957,6 +1957,16 @@ Tue Oct 26 21:40:57 CEST 1999
 
 
   
+\gdesc
+
+
+Create unnamed prepared statement from current buffer and get and
+show description of this prepared statement.
+
+
+  
+
+  
 \gexec
 
 
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index 859ded7..282e2e5 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -88,6 +88,7 @@ static backslashResult exec_command_errverbose(PsqlScanState scan_state, bool ac
 static backslashResult exec_command_f(PsqlScanState scan_state, bool active_branch);
 static backslashResult exec_command_g(PsqlScanState scan_state, bool active_branch,
 			   const char *cmd);
+static backslashResult exec_command_gdesc(PsqlScanState scan_state, bool active_branch);
 static backslashResult exec_command_gexec(PsqlScanState scan_state, bool active_branch);
 static backslashResult exec_command_gset(PsqlScanState scan_state, bool active_branch);
 static backslashResult exec_command_help(PsqlScanState scan_state, bool active_branch);
@@ -337,6 +338,8 @@ exec_command(const char *cmd,
 		status = exec_command_f(scan_state, active_branch);
 	else if (strcmp(cmd, "g") == 0 || strcmp(cmd, "gx") == 0)
 		status = exec_command_g(scan_state, active_branch, cmd);
+	else if (strcmp(cmd, "gdesc") == 0)
+		status = exec_command_gdesc(scan_state, active_branch);
 	else if (strcmp(cmd, "gexec") == 0)
 		status = exec_command_gexec(scan_state, active_branch);
 	else if (strcmp(cmd, "gset") == 0)
@@ -1328,6 +1331,25 @@ exec_command_g(PsqlScanState scan_state, bool active_branch, const char *cmd)
 }
 
 /*
+ * \gdesc -- describe query result
+ */
+static backslashResult
+exec_command_gdesc(PsqlScanState scan_state, bool active_branch)
+{
+	backslashResult status = PSQL_CMD_SKIP_LINE;
+
+	if (active_branch)
+	{
+		pset.gdesc_flag = true;
+		status = PSQL_CMD_SEND;
+	}
+	else
+		ignore_slash_filepipe(scan_state);
+
+	return status;
+}
+
+/*
  * \gexec -- send query and execute each field of result
  */
 static backslashResult
diff --git a/src/bin/psql/common.c b/src/bin/psql/common.c
index a2f1259..8a16744 100644
--- a/src/bin/psql/common.c
+++ b/src/bin/psql/common.c
@@ -1323,7 +1323,62 @@ SendQuery(const char *query)
 		}
 	}
 
-	if (pset.fetch_count <= 0 || pset.gexec_flag ||
+	if (pset.gdesc_flag)
+	{
+		results = PQprepare(pset.db, "", query, 0, NULL);
+		OK = ProcessResult();
+
+		if (OK && results)
+		{
+			results = PQdescribePrepared(pset.db, "");
+			OK = ProcessResult();
+
+			if (OK && results)
+			{
+PQExpBufferData		buf;
+int		i;
+
+initPQExpBuffer();
+printfPQExpBuffer(,
+	  "SELECT name AS \"Name\", pg_catalog.format_type(tp, tpm) AS \"Type\" FROM\n"
+		  " (VALUES ");
+
+for(i = 0; i< PQnfields(results); i++)
+{
+	char	*name;
+	char	*escname;
+	size_t		name_length;
+
+	if (i > 0)
+		appendPQExpBufferStr(, ",");
+
+	name = 

Re: [HACKERS] Time based lag tracking for logical replication

[Petr Jelinek]

On 03/05/17 08:28, Simon Riggs wrote:
> On 23 April 2017 at 01:10, Petr Jelinek  wrote:
>> Hi,
>>
>> The time based lag tracking commit [1] added interface for logging
>> progress of replication so that we can report lag as time interval
>> instead of just bytes. But the patch didn't contain patch for the
>> builtin logical replication.
>>
>> So I wrote something that implements this. I didn't like all that much
>> the API layering in terms of exporting the walsender's LagTrackerWrite()
>> for use by plugin directly. Normally output plugin does not have to care
>> if it's running under walsender or not, it uses abstracted write
>> interface for that which can be implemented in various ways (that's how
>> we implement SQL interface to logical decoding after all). So I decided
>> to add another function to the logical decoding write api called
>> update_progress and call that one from the output plugin. The walsender
>> then implements that new API to call the LagTrackerWrite() while the SQL
>> interface just does not implement it at all. This seems like cleaner way
>> of doing it.
>>
>> Thoughts?
> 
> Agree cleaner.
> 
> I don't see any pacing or comments about it, nor handling of
> intermediate messages while we process a large transaction.

Agreed, pacing is good idea because on busy server storing info for
every commit could get expensive.

Don't understand what you mean by "handling of intermediate messages
while we process a large transaction". Logical replication is
transaction based so far, it does not stream like physical replication
so it seems like there is limited usefulness in doing this outside of
commit no?

-- 
  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] Time based lag tracking for logical replication

[Thomas Munro]

On Wed, May 3, 2017 at 6:28 PM, Simon Riggs  wrote:
> On 23 April 2017 at 01:10, Petr Jelinek  wrote:
>> Hi,
>>
>> The time based lag tracking commit [1] added interface for logging
>> progress of replication so that we can report lag as time interval
>> instead of just bytes. But the patch didn't contain patch for the
>> builtin logical replication.
>>
>> So I wrote something that implements this. I didn't like all that much
>> the API layering in terms of exporting the walsender's LagTrackerWrite()
>> for use by plugin directly. Normally output plugin does not have to care
>> if it's running under walsender or not, it uses abstracted write
>> interface for that which can be implemented in various ways (that's how
>> we implement SQL interface to logical decoding after all). So I decided
>> to add another function to the logical decoding write api called
>> update_progress and call that one from the output plugin. The walsender
>> then implements that new API to call the LagTrackerWrite() while the SQL
>> interface just does not implement it at all. This seems like cleaner way
>> of doing it.
>>
>> Thoughts?
>
> Agree cleaner.

+1

> I don't see any pacing or comments about it, nor handling of
> intermediate messages while we process a large transaction.
>
> I'll look at adding some pacing code in WalSndUpdateProgress()

By the way, I have a small improvement to the interpolation to
propose.  Right now, after a period of idleness it can report a silly
large number based on an ancient time, but you won't usually see it
because it's quickly replaced by a sensible number.  I think this
thinko will affect logical rep with Petr's patch more.  I had been
meaning to post the improvement but got sidetracked by that recovery
test failure problem.  I'll post that in the next few days.

-- 
Thomas Munro
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] Time based lag tracking for logical replication

[Simon Riggs]

On 23 April 2017 at 01:10, Petr Jelinek  wrote:
> Hi,
>
> The time based lag tracking commit [1] added interface for logging
> progress of replication so that we can report lag as time interval
> instead of just bytes. But the patch didn't contain patch for the
> builtin logical replication.
>
> So I wrote something that implements this. I didn't like all that much
> the API layering in terms of exporting the walsender's LagTrackerWrite()
> for use by plugin directly. Normally output plugin does not have to care
> if it's running under walsender or not, it uses abstracted write
> interface for that which can be implemented in various ways (that's how
> we implement SQL interface to logical decoding after all). So I decided
> to add another function to the logical decoding write api called
> update_progress and call that one from the output plugin. The walsender
> then implements that new API to call the LagTrackerWrite() while the SQL
> interface just does not implement it at all. This seems like cleaner way
> of doing it.
>
> Thoughts?

Agree cleaner.

I don't see any pacing or comments about it, nor handling of
intermediate messages while we process a large transaction.

I'll look at adding some pacing code in WalSndUpdateProgress()

-- 
Simon Riggshttp://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] Logical replication - TRAP: FailedAssertion in pgstat.c

[Petr Jelinek]

On 02/05/17 20:43, Robert Haas wrote:
> On Thu, Apr 20, 2017 at 2:58 PM, Peter Eisentraut
>  wrote:
>> On 4/16/17 16:11, Petr Jelinek wrote:
>>> Yeah it is, it needs to be fenced to happen only after commit, which is
>>> not guaranteed at the point of code, we probably need to put the
>>> pgstat_report_stat() inside the if above after the
>>> CommitTransactionCommand() (that will make it report stats for changes
>>> apply did to pg_subscription_rel after next transaction though)
>>
>> I think to avoid the latter, we should add more pgstat_report_stat()
>> calls, such as in process_syncing_tables_for_apply().  Basically every
>> code path that calls CommitTransactionCommand() should have one, no?
> 
> Is there anything left to be committed here?
> 

Afaics the fix was not committed. Peter wanted more comprehensive fix
which didn't happen. I think something like attached should do the job.

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


fix-statistics-reporting-in-logical-replication-work.patch
Description: binary/octet-stream

-- 
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] Inefficient shutdown of pg_basebackup

[Simon Riggs]

On 27 April 2017 at 05:31, Tom Lane  wrote:

> The attached draft patch fixes this by expanding the StreamCtl API
> with a socket that the low-level wait routine should check for input.
> For me, this consistently knocks about 10 seconds off the runtime of
> 001_stream_rep.pl.

That is good. I noticed that delay many times.

> It could be argued that this isn't too significant in the real world
> because pg_basebackup would always run far longer than 10 seconds
> anyway for non-toy data.  But it still looks like a bug to me.

Not sure its a bug, but if it causes people to avoid running tests
then it is clearly a reliability issue.

I don't see anything to gain by waiting a year to apply this, so +1 to
move on it now.

-- 
Simon Riggshttp://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