Re: Sparse bit set data structure

[Heikki Linnakangas]

On 09/04/2019 03:45, Alvaro Herrera wrote:

On 2019-Apr-08, Bruce Momjian wrote:


Uh, should this be applied?


Yes, it's a pretty obvious typo methinks.


Pushed, thanks, and sorry for the delay.

- Heikki

Re: "could not reattach to shared memory" on buildfarm member dory

[Noah Misch]

On Sun, Apr 07, 2019 at 12:43:23AM -0400, Tom Lane wrote:
> Noah Misch  writes:
> > On Tue, Apr 02, 2019 at 10:09:00AM -0400, Tom Lane wrote:
> >> I worry that your proposed fix is unstable, in particular this assumption
> >> seems shaky:
> >>> + * ... The idea is that, if the allocator handed out
> >>> + * REGION1 pages before REGION2 pages at one occasion, it will do so 
> >>> whenever
> >>> + * both regions are free.
> 
> > True.  If Windows changes to prefer allocating from the most recently freed
> > region, shmem-protective-region-v1.patch would cease to help.  It's not
> > impossible.
> 
> >> I wonder whether it's possible to address this by configuring the "default
> >> thread pool" to have only one thread?  It seems like the extra threads are
> >> just adding backend startup overhead to no benefit, since we won't use 'em.
> 
> > I didn't find a way to configure the pool's size.
> 
> Seems odd ...

I think you're expected to create your own thread pool if you're picky about
the size.  The default thread pool is for non-picky callers and for Windows
libraries to use internally.  The fact that the pool starts before main() also
narrows the use case for configuring it.  If there were an API for "block
until all threads of the pool are alive", that would meet our needs nicely.
Needless to say, I didn't find that, either.

> > Another option is to reattach shared memory earlier, before the default 
> > thread
> > pool starts.  A Windows application using only the unavoidable DLLs
> > (kernel32.dll, ntdll.dll, kernelbase.dll) doesn't get a default thread pool;
> > the pool starts when one loads ws2_32.dll, ucrtbased.dll, etc.  Hence, the
> > DllMain() function of a DLL that loads early could avoid the problem.  
> > (Cygwin
> > fork() uses that route to remap shared memory, though it also retries failed
> > forks.)  If we proceed that way, we'd add a tiny pg_shmem.dll that appears
> > early in the load order, just after the unavoidable DLLs.  It would extract
> > applicable parameters from the command line and reattach shared memory.  
> > When
> > it fails, it would set a flag so the usual code can raise an error.  Does 
> > that
> > sound more or less attractive than shmem-protective-region-v1.patch?
> 
> Well, it definitely sounds like more work.  Let's not do such work
> until we have to.  Your patch has the advantages of being (a) small
> and (b) done, and there's much to be said for that.

Works for me.  Pushed.

Re: hyrax vs. RelationBuildPartitionDesc

[Amit Langote]

On 2019/03/16 6:41, Robert Haas wrote:
> On Fri, Mar 15, 2019 at 3:45 PM Tom Lane  wrote:
>> I agree that copying data isn't great.  What I don't agree is that this
>> solution is better.  In particular, copying data out of the relcache
>> rather than expecting the relcache to hold still over long periods
>> is the way we've done things everywhere else, cf RelationGetIndexList,
>> RelationGetStatExtList, RelationGetIndexExpressions,
>> RelationGetIndexPredicate, RelationGetIndexAttrBitmap,
>> RelationGetExclusionInfo, GetRelationPublicationActions.  I don't care
>> for a patch randomly deciding to do things differently on the basis of an
>> unsupported-by-evidence argument that it might cost too much to copy the
>> data.  If we're going to make a push to reduce the amount of copying of
>> that sort that we do, it should be a separately (and carefully) designed
>> thing that applies to all the relcache substructures that have the issue,
>> not one-off hacks that haven't been reviewed thoroughly.
> 
> That's not an unreasonable argument.  On the other hand, if you never
> try new stuff, you lose opportunities to explore things that might
> turn out to be better and worth adopting more widely.
> 
> I am not very convinced that it makes sense to lump something like
> RelationGetIndexAttrBitmap in with something like
> RelationGetPartitionDesc.  RelationGetIndexAttrBitmap is returning a
> single Bitmapset, whereas the data structure RelationGetPartitionDesc
> is vastly larger and more complex.  You can't say "well, if it's best
> to copy 32 bytes of data out of the relcache every time we need it, it
> must also be right to copy 10k or 100k of data out of the relcache
> every time we need it."
> 
> There is another difference as well: there's a good chance that
> somebody is going to want to mutate a Bitmapset, whereas they had
> BETTER NOT think that they can mutate the PartitionDesc.  So returning
> an uncopied Bitmapset is kinda risky in a way that returning an
> uncopied PartitionDesc is not.
> 
> If we want an at-least-somewhat unified solution here, I think we need
> to bite the bullet and make the planner hold a reference to the
> relcache throughout planning.  (The executor already keeps it open, I
> believe.) Otherwise, how is the relcache supposed to know when it can
> throw stuff away and when it can't?  The only alternative seems to be
> to have each subsystem hold its own reference count, as I did with the
> PartitionDirectory stuff, which is not something we'd want to scale
> out.

Fwiw, I'd like to vote for planner holding the relcache reference open
throughout planning.  The planner could then reference the various
substructures directly (using a non-copying accessor), except those that
something in the planner might want to modify, in which case use the
copying accessor.

Thanks,
Amit

Re: [PATCH v20] GSSAPI encryption support

[Tom Lane]

Peter Eisentraut  writes:
> On 2019-04-05 23:37, Stephen Frost wrote:
>> I've also reached out to some colleagues about having one of them test
>> with MacOS.  What version are you on..? 

> macOS 10.14.14 it says.

I tried to replicate this on my own laptop (macOS 10.14.4 ... I do not
think there is or ever will be a 10.14.14).  I can't, because the
kerberos test fails immediately:

1..4
# setting up Kerberos
# Running: krb5-config --version
# Running: kdb5_util create -s -P secret0
Can't exec "kdb5_util": No such file or directory at 
/Users/tgl/pgsql/src/test/kerberos/../../../src/test/perl/TestLib.pm line 190.
Bail out!  system kdb5_util failed

and indeed, there's no kdb5_util in /usr/bin/ or anywhere else that
I can find.  So I speculate that Peter is running some weird hodgepodge
of Apple and Homebrew code, making the question not so much "why does
it fail" as "how did it ever work".

I also notice that the build spews out a bunch of deprecation warnings,
because just as with openSSL, Apple has stuck deprecation attributes
on everything in gssapi/gssapi.h.  They want you to use their GSS
"framework" instead.

So I'm not convinced we should spend a lot of effort on fooling with
the test scripts for this.  This platform has got much more fundamental
problems than that.

regards, tom lane

Re: 2019-03 Starts Tomorrow

[Pavel Stehule]

út 9. 4. 2019 v 6:04 odesílatel Michael Paquier 
napsal:

> On Sat, Apr 06, 2019 at 04:52:42PM -0400, David Steele wrote:
> > OK, these have all been pushed now.
>
> Please note that the commit fest has been closed as a result of the
> feature freeze, any remaining items have been moved to the next CF if
> they still were in "Needs Review" state and patches waiting on author
> have been marked as returned with feedback.  Bug fixes have all been
> moved to the next CF.
>
> Here is the final score:
> Committed: 100.
> Moved to next CF: 82.
> Withdrawn: 5.
> Rejected: 3.
> Returned with Feedback: 17.
> Total: 207.
>

lot of work is done,

good work

Regards

Pavel


> --
> Michael
>

Re: reloption to prevent VACUUM from truncating empty pages at the end of relation

[Kyotaro HORIGUCHI]

Hello.

At Mon, 8 Apr 2019 19:22:04 +0900, Fujii Masao  wrote in 

> > "TRUNCATE" option for vacuum command should be added to the open items?
> 
> Yes, I think.
> Attached is the patch which adds TRUNCATE option into VACUUM.

By the way, this might have been discussed upthread, but boolean
options of VACUUM command is defaulted to true. So, it seems to
me that the name is better (or convenient to me) to be
SKIP_TRUNCATE. The name of the reloption seems good to me.

regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center

Re: 2019-03 Starts Tomorrow

[Michael Paquier]

On Sat, Apr 06, 2019 at 04:52:42PM -0400, David Steele wrote:
> OK, these have all been pushed now.

Please note that the commit fest has been closed as a result of the
feature freeze, any remaining items have been moved to the next CF if
they still were in "Needs Review" state and patches waiting on author
have been marked as returned with feedback.  Bug fixes have all been
moved to the next CF.

Here is the final score:
Committed: 100.
Moved to next CF: 82.
Withdrawn: 5.
Rejected: 3.
Returned with Feedback: 17.
Total: 207. 
--
Michael


signature.asc
Description: PGP signature

Re: hyrax vs. RelationBuildPartitionDesc

[Amit Langote]

On 2019/04/08 22:59, Tom Lane wrote:
> Amit Langote  writes:
>> Should I add this patch to Older Bugs [1]?
> 
> Yeah, it's an open issue IMO.  I think we've been focusing on getting
> as many feature patches done as we could during the CF, but now it's
> time to start mopping up problems like this one.

Thanks, done.

Regards,
Amit

Re: COLLATE: Hash partition vs UPDATE

[Amit Langote]

Hi Jesper,

On 2019/04/09 1:33, Jesper Pedersen wrote:
> Hi,
> 
> The following case
> 
> -- test.sql --
> CREATE TABLE test (a text PRIMARY KEY, b text) PARTITION BY HASH (a);
> CREATE TABLE test_p0 PARTITION OF test FOR VALUES WITH (MODULUS 2,
> REMAINDER 0);
> CREATE TABLE test_p1 PARTITION OF test FOR VALUES WITH (MODULUS 2,
> REMAINDER 1);
> -- CREATE INDEX idx_test_b ON test USING HASH (b);
> 
> INSERT INTO test VALUES ('', '');
> 
> -- Regression
> UPDATE test SET b = '' WHERE a = '';
> -- test.sql --
> 
> fails on master, which includes [1], with
> 
> 
> psql:test.sql:9: ERROR:  could not determine which collation to use for
> string hashing
> HINT:  Use the COLLATE clause to set the collation explicitly.
> 
> 
> It passes on 11.x.

Thanks for the report.

This seems to broken since the following commit (I see you already cc'd
Peter):

commit 5e1963fb764e9cc092e0f7b58b28985c311431d9
Author: Peter Eisentraut 
Date:   Fri Mar 22 12:09:32 2019 +0100

Collations with nondeterministic comparison


As of this commit, hashing functions hashtext() and hashtextextended()
require a valid collation to be passed in.  ISTM,
satisfies_hash_partition() that's called by hash partition constraint
checking should have been changed to use FunctionCall2Coll() interface to
account for the requirements of the above commit.  I see that it did that
for compute_partition_hash_value(), which is used by hash partition tuple
routing.  That also seems to be covered by regression tests, but there are
no tests that cover satisfies_hash_partition().

Attached patch is an attempt to fix this.  I've also added Amul Sul who
can maybe comment on the satisfies_hash_partition() changes.

BTW, it seems we don't need to back-patch this to PG 11 which introduced
hash partitioning, because text hashing functions don't need collation
there, right?

Thanks,
Amit
diff --git a/src/backend/partitioning/partbounds.c 
b/src/backend/partitioning/partbounds.c
index c8770ccfee..331dab3954 100644
--- a/src/backend/partitioning/partbounds.c
+++ b/src/backend/partitioning/partbounds.c
@@ -2778,6 +2778,7 @@ satisfies_hash_partition(PG_FUNCTION_ARGS)
boolvariadic_typbyval;
charvariadic_typalign;
FmgrInfopartsupfunc[PARTITION_MAX_KEYS];
+   Oid partcollid[PARTITION_MAX_KEYS];
} ColumnsHashData;
Oid parentId;
int modulus;
@@ -2865,6 +2866,7 @@ satisfies_hash_partition(PG_FUNCTION_ARGS)
fmgr_info_copy(_extra->partsupfunc[j],
   >partsupfunc[j],
   
fcinfo->flinfo->fn_mcxt);
+   my_extra->partcollid[j] = key->partcollation[j];
}
 
}
@@ -2888,6 +2890,7 @@ satisfies_hash_partition(PG_FUNCTION_ARGS)
 
/* check argument types */
for (j = 0; j < key->partnatts; ++j)
+   {
if (key->parttypid[j] != 
my_extra->variadic_type)
ereport(ERROR,

(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
@@ -2895,6 +2898,8 @@ satisfies_hash_partition(PG_FUNCTION_ARGS)
j + 1,

format_type_be(key->parttypid[j]),

format_type_be(my_extra->variadic_type;
+   my_extra->partcollid[j] = key->partcollation[j];
+   }
 
fmgr_info_copy(_extra->partsupfunc[0],
   >partsupfunc[0],
@@ -2928,9 +2933,10 @@ satisfies_hash_partition(PG_FUNCTION_ARGS)
 
Assert(OidIsValid(my_extra->partsupfunc[i].fn_oid));
 
-   hash = FunctionCall2(_extra->partsupfunc[i],
-
PG_GETARG_DATUM(argno),
-seed);
+   hash = FunctionCall2Coll(_extra->partsupfunc[i],
+
my_extra->partcollid[i],
+
PG_GETARG_DATUM(argno),
+seed);
 
/* Form a single 64-bit hash value */
rowHash = hash_combine64(rowHash, DatumGetUInt64(hash));
diff --git a/src/test/regress/expected/hash_part.out 
b/src/test/regress/expected/hash_part.out
index 731d26fc3d..b536b6955c 100644
--- 

Re: [PATCH v20] GSSAPI encryption support

[Stephen Frost]

Greetings,

* Peter Eisentraut (peter.eisentr...@2ndquadrant.com) wrote:
> On 2019-04-05 23:37, Stephen Frost wrote:
> > I wonder if somehow the keytab file that the server is using isn't
> > getting destroyed between the two test runs and so you're ending up with
> > the server using the key from the old KDC, while the user is using the
> > new one..?  Or something is equally going wrong in the tests.
> > 
> > Could you try doing something like removing the 001_auth.pl, moving the
> > 002_enc.pl to 001_enc.pl, double-check that everything is cleaned up and
> > that there aren't any old PG servers running, et al, and re-try that
> > way?
> 
> Running just 002_enc.pl by itself passes the tests!

Great!  I think what I'll do is work to incorporate the two tests back
into one script, to avoid whatever the race condition or other confusion
is happening on macOS here.

Thanks so much for testing it!

Stephen


signature.asc
Description: PGP signature

Re: [HACKERS] PATCH: multivariate histograms and MCV lists

[Michael Paquier]

On Sat, Mar 30, 2019 at 09:13:01PM +0100, Tomas Vondra wrote:
> Hmmm, what's the right status in the CF app when a part of a patch was
> committed and the rest should be moved to the next CF? Committed, Moved
> to next CF, or something else?

This stuff has been around for nine commit fests, and you have been
able to finish the basic work.  So I think that committed is most
appropriate so as you can start later on with a new concept, new patch
sets, perhaps a new thread, and surely a new CF entry.
--
Michael


signature.asc
Description: PGP signature

Re: Zedstore - compressed in-core columnar storage

[Ashwin Agrawal]

On Mon, Apr 8, 2019 at 6:04 PM Andres Freund  wrote:

> Hi,
>
> On 2019-04-08 17:27:05 -0700, Ashwin Agrawal wrote:
> > Heikki and I have been hacking recently for few weeks to implement
> > in-core columnar storage for PostgreSQL. Here's the design and initial
> > implementation of Zedstore, compressed in-core columnar storage (table
> > access method).
>
> That's very cool.
>
>
> > Motivations / Objectives
> >
> > * Performance improvement for queries selecting subset of columns
> >   (reduced IO).
> > * Reduced on-disk footprint compared to heap table. Shorter tuple
> >   headers and also leveraging compression of similar type data
> > * Be first-class citizen in the Postgres architecture (tables data can
> >   just independently live in columnar storage)
> > * Fully MVCC compliant
> > * All Indexes supported
> > * Hybrid row-column store, where some columns are stored together, and
> >   others separately. Provide flexibility of granularity on how to
> >   divide the columns. Columns accessed together can be stored
> >   together.
> > * Provide better control over bloat (similar to zheap)
> > * Eliminate need for separate toast tables
> > * Faster add / drop column or changing data type of column by avoiding
> >   full rewrite of the table.
>
> Is storage going through the bufmgr.c or separately?
>

Yes, below access method its pretty much same as heap. All reads and writes
flow via buffer cache. The implementation sits nicely in between, just
modifying the access method code changing how just how data is stored in
pages, above AM and below AM is basically all behaves similar to heap code.


> > In uncompressed form, the page can be arbitrarily large. But after
> > compression, it must fit into a physical 8k block. If on insert or
> > update of a tuple, the page cannot be compressed below 8k anymore, the
> > page is split. Note that because TIDs are logical rather than physical
> > identifiers, we can freely move tuples from one physical page to
> > another during page split. A tuple's TID never changes.
>
> When does compression happen? After each modifcation of the expanded
>
"page"? Are repeated expansions prevented somehow, e.g. if I
> insert/delete rows into the same page one-by-one?
>

Compression is performed with new data is only if page becomes full, till
then uncompressed data is added to the page. If even after compression
cannot add data to the page then page split is performed. Already
compressed data is not compressed again on next insert. New compressed
block is created for newly added uncompressed items.

The line of thought we have for delete is will not free the space as soon
as delete is performed, but instead delay and reuse the space deleted on
next insertion to the page.


> A metapage at block 0, has links to the roots of the B-trees. Leaf
> > pages look the same, but instead of storing the whole tuple, stores
> > just a single attribute. To reconstruct a row with given TID, scan
> > descends down the B-trees for all the columns using that TID, and
> > fetches all attributes. Likewise, a sequential scan walks all the
> > B-trees in lockstep.
>
> Does the size of the metapage limit the number of column [groups]? Or is
> there some overflow / tree of trees / whatnot happening?
>

In design it doesn't limit the number of columns, as can have chain of
meta-pages to store the required meta-data, page 0 still being start of the
chain.


> > Insert:
> > Inserting a new row, splits the row into datums. Then for first column
> > decide which block to insert the same to, and pick a TID for it, and
> > write undo record for the same. Rest of the columns are inserted using
> > that same TID and point to same undo position.
>
> Is there some buffering? Without that it seems like retail inserts are
> going to be pretty slow?
>

Yes, regular buffer cache.



> > Property is added to Table AM to convey if column projection is
> > leveraged by AM for scans. While scanning tables with AM leveraging
> > this property, executor parses the plan. Leverages the target list and
> > quals to find the required columns for query. This list is passed down
> > to AM on beginscan. Zedstore uses this column projection list to only
> > pull data from selected columns. Virtual tuple table slot is used to
> > pass back the datums for subset of columns.
> >
> > Current table am API requires enhancement here to pass down column
> > projection to AM. The patch showcases two different ways for the same.
> >
> > * For sequential scans added new beginscan_with_column_projection()
> >   API. Executor checks AM property and if it leverages column
> >   projection uses this new API else normal beginscan() API.
> >
> > * For index scans instead of modifying the begin scan API, added new
> >   API to specifically pass column projection list after calling begin
> >   scan to populate the scan descriptor but before fetching the tuples.
>
> FWIW, I don't quite think this is the right approach.  I've only a vague
> sketch of 

Re: Status of the table access method work

[Haribabu Kommi]

On Sat, Apr 6, 2019 at 7:25 AM Andres Freund  wrote:

> Hi,
>
> In this email I want to give a brief status update of the table access
> method work - I assume that most of you sensibly haven't followed it
> into all nooks and crannies.
>
> I want to thank Haribabu, Alvaro, Alexander, David, Dmitry and all the
> others that collaborated on making tableam happen. It was/is a huge
> project.


A big thank you Andres for your enormous efforts in this patch.
Without your involvement, this patch couldn't have been made into v12.


With regards to storing the rows themselves, the second biggest
> limitation is a limitation that is not actually a part of tableam
> itself: WAL. Many tableam's would want to use WAL, but we only have
> extensible WAL as part of generic_xlog.h. While that's useful to allow
> prototyping etc, it's imo not efficient enough to build a competitive
> storage engine for OLTP (OLAP probably much less of a problem).  I don't
> know what the best approach here is - allowing "well known" extensions
> to register rmgr entries would be the easiest solution, but it's
> certainly a bit crummy.
>

I got the same doubt when i looked into some of the UNDO patches
where it tries to modify the core code to add UNDO specific WAL types.
Different AM's may need different set of operations to be WAL logged,
so it may be better for the AM's to register their own types?

Regards,
Haribabu Kommi
Fujitsu Australia

Re: pg_rewind vs superuser

[Michael Paquier]

On Mon, Apr 08, 2019 at 10:03:48AM +0200, Peter Eisentraut wrote:
> How about some tests to show that this is actually true?

Sure.  With something like the attached?  I don't think that there is
much point to complicate the test code with multiple roles if the
default is a superuser.
--
Michael
diff --git a/src/bin/pg_rewind/t/RewindTest.pm b/src/bin/pg_rewind/t/RewindTest.pm
index 900d452d8b..618de85161 100644
--- a/src/bin/pg_rewind/t/RewindTest.pm
+++ b/src/bin/pg_rewind/t/RewindTest.pm
@@ -144,6 +144,20 @@ sub start_master
 {
 	$node_master->start;
 
+	# Create a custom role which will be used to run pg_rewind.  This has
+	# minimal permissions to make pg_rewind able to work with an online
+	# source.
+	$node_master->psql('postgres', "
+		CREATE ROLE rewind_user LOGIN;
+		GRANT EXECUTE ON function pg_catalog.pg_ls_dir(text, boolean, boolean)
+		  TO rewind_user;
+		GRANT EXECUTE ON function pg_catalog.pg_stat_file(text, boolean)
+		  TO rewind_user;
+		GRANT EXECUTE ON function pg_catalog.pg_read_binary_file(text)
+		  TO rewind_user;
+		GRANT EXECUTE ON function pg_catalog.pg_read_binary_file(text, bigint, bigint, boolean)
+		  TO rewind_user;");
+
 	 Now run the test-specific parts to initialize the master before setting
 	# up standby
 
@@ -207,6 +221,9 @@ sub run_pg_rewind
 	my $standby_connstr = $node_standby->connstr('postgres');
 	my $tmp_folder  = TestLib::tempdir;
 
+	# Append the rewind role to the connection string.
+	$standby_connstr = "$standby_connstr user=rewind_user";
+
 	# Stop the master and be ready to perform the rewind
 	$node_master->stop;
 


signature.asc
Description: PGP signature

Re: Problem with default partition pruning

[Kyotaro HORIGUCHI]

Sigh..

At Tue, 09 Apr 2019 10:28:48 +0900 (Tokyo Standard Time), Kyotaro HORIGUCHI 
 wrote in 
<20190409.102848.252476604.horiguchi.kyot...@lab.ntt.co.jp>
> As the second thought. Partition constraint is not constraint
> expression so that's fair to apply partqual ignoring
> constraint_exclusion. The variable is set false to skip useless
> expression evaluation on all partitions, but partqual should be
> evaluated just once.  Sorry for my confusion.
> 
> So still it is wrong that the new code is added in
> gen_partprune_steps_internal.

So still it is wrong that the new code is added at the beginning
of the loop on clauses in gen_partprune_steps_internal.

>   If partqual results true and the
> clause is long, the partqual is evaluated uselessly at every
> recursion.
> 
> Maybe we should do that when we find that the current clause
> doesn't match part attributes. Specifically just after the for
> loop "for (i = 0 ; i < part_scheme->partnattrs; i++)".

-- 
Kyotaro Horiguchi
NTT Open Source Software Center

Re: Problem with default partition pruning

[Kyotaro HORIGUCHI]

At Mon, 8 Apr 2019 16:57:35 +0900, "Yuzuko Hosoya" 
 wrote in 
<00c101d4ede0$babd4390$3037cab0$@lab.ntt.co.jp>
> > BTW, now I'm a bit puzzled between whether this case should be fixed by 
> > hacking on partprune.c like
> > this patch does or whether to work on getting the other patch committed and 
> > expect users to set
> > constraint_exclusion = on for this to behave as expected.  The original 
> > intention of setting
> > partition_qual in set_relation_partition_info() was for partprune.c to use 
> > it to remove useless
> > arguments of OR clauses which otherwise would cause the failure to 
> > correctly prune the default partitions
> > of sub-partitioned tables.  As shown by the examples in this thread, the 
> > original effort was
> > insufficient, which this patch aims to improve.  But, it also expands the 
> > scope of partprune.c's usage
> > of partition_qual, which is to effectively perform full-blown constraint 
> > exclusion without being
> > controllable by constraint_exclusion GUC, which may be seen as being good 
> > or bad.  The fact that it
> > helps in getting partition pruning working correctly in more obscure cases 
> > like those discussed in
> > this thread means it's good maybe.
> > 
> Umm, even though this modification might be overhead, I think this problem 
> should be solved
> without setting constraint_exclusion GUC. But I'm not sure.

As the second thought. Partition constraint is not constraint
expression so that's fair to apply partqual ignoring
constraint_exclusion. The variable is set false to skip useless
expression evaluation on all partitions, but partqual should be
evaluated just once.  Sorry for my confusion.

So still it is wrong that the new code is added in
gen_partprune_steps_internal. If partqual results true and the
clause is long, the partqual is evaluated uselessly at every
recursion.

Maybe we should do that when we find that the current clause
doesn't match part attributes. Specifically just after the for
loop "for (i = 0 ; i < part_scheme->partnattrs; i++)".

regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center

Re: Zedstore - compressed in-core columnar storage

[Andres Freund]

Hi,

On 2019-04-08 17:27:05 -0700, Ashwin Agrawal wrote:
> Heikki and I have been hacking recently for few weeks to implement
> in-core columnar storage for PostgreSQL. Here's the design and initial
> implementation of Zedstore, compressed in-core columnar storage (table
> access method).

That's very cool.


> Motivations / Objectives
> 
> * Performance improvement for queries selecting subset of columns
>   (reduced IO).
> * Reduced on-disk footprint compared to heap table. Shorter tuple
>   headers and also leveraging compression of similar type data
> * Be first-class citizen in the Postgres architecture (tables data can
>   just independently live in columnar storage)
> * Fully MVCC compliant
> * All Indexes supported
> * Hybrid row-column store, where some columns are stored together, and
>   others separately. Provide flexibility of granularity on how to
>   divide the columns. Columns accessed together can be stored
>   together.
> * Provide better control over bloat (similar to zheap)
> * Eliminate need for separate toast tables
> * Faster add / drop column or changing data type of column by avoiding
>   full rewrite of the table.

Is storage going through the bufmgr.c or separately?



> In uncompressed form, the page can be arbitrarily large. But after
> compression, it must fit into a physical 8k block. If on insert or
> update of a tuple, the page cannot be compressed below 8k anymore, the
> page is split. Note that because TIDs are logical rather than physical
> identifiers, we can freely move tuples from one physical page to
> another during page split. A tuple's TID never changes.

When does compression happen? After each modifcation of the expanded
"page"? Are repeated expansions prevented somehow, e.g. if I
insert/delete rows into the same page one-by-one?


> A metapage at block 0, has links to the roots of the B-trees. Leaf
> pages look the same, but instead of storing the whole tuple, stores
> just a single attribute. To reconstruct a row with given TID, scan
> descends down the B-trees for all the columns using that TID, and
> fetches all attributes. Likewise, a sequential scan walks all the
> B-trees in lockstep.

Does the size of the metapage limit the number of column [groups]? Or is
there some overflow / tree of trees / whatnot happening?


> Insert:
> Inserting a new row, splits the row into datums. Then for first column
> decide which block to insert the same to, and pick a TID for it, and
> write undo record for the same. Rest of the columns are inserted using
> that same TID and point to same undo position.

Is there some buffering? Without that it seems like retail inserts are
going to be pretty slow?



> Property is added to Table AM to convey if column projection is
> leveraged by AM for scans. While scanning tables with AM leveraging
> this property, executor parses the plan. Leverages the target list and
> quals to find the required columns for query. This list is passed down
> to AM on beginscan. Zedstore uses this column projection list to only
> pull data from selected columns. Virtual tuple table slot is used to
> pass back the datums for subset of columns.
> 
> Current table am API requires enhancement here to pass down column
> projection to AM. The patch showcases two different ways for the same.
> 
> * For sequential scans added new beginscan_with_column_projection()
>   API. Executor checks AM property and if it leverages column
>   projection uses this new API else normal beginscan() API.
> 
> * For index scans instead of modifying the begin scan API, added new
>   API to specifically pass column projection list after calling begin
>   scan to populate the scan descriptor but before fetching the tuples.

FWIW, I don't quite think this is the right approach.  I've only a vague
sketch of this in my head, but I think we should want a general API to
pass that down to *any* scan. Even for heap, not deforming leading
columns that a uninteresting, but precede relevant columns, would be
quite a noticable performance win.  I don't think the projection list is
the right approach for that.


> Extremely basic UNDO logging has be implemented just for MVCC
> perspective. MVCC is missing tuple lock right now. Plus, doesn't
> actually perform any undo yet. No WAL logging exist currently hence
> its not crash safe either.

Have you looked at the undo APIs developed for zheap, as discussed on
the list? Seems important that they're suitable for this too.


> Test mentioned in thread "Column lookup in a row performance" [6],
> good example query for zedstore locally on laptop using lz4 shows
> 
> postgres=# SELECT AVG(i199) FROM (select i199 from layout offset 0) x; --
> heap
>  avg
> -
>  50.5000
> (1 row)
> 
> Time: 4679.026 ms (00:04.679)
> 
> postgres=# SELECT AVG(i199) FROM (select i199 from zlayout offset 0) x; --
> zedstore
>  avg
> -
>  50.5000
> (1 row)
> 
> Time: 379.710 ms

Well, I'm not 

Re: [Sender Address Forgery]Re: [HACKERS] generated columns

[Amit Langote]

On 2019/04/08 20:50, Peter Eisentraut wrote:
> On 2019-04-04 16:37, Amit Langote wrote:
>> OK,  thanks for explaining.  We do allow DEFAULT to be specified on
>> foreign tables, although locally-defined defaults have little meaning
>> if the FDW doesn't allow inserts.  Maybe same thing applies to
>> GENERATED AS columns.
>>
>> Would it make sense to clarify this on CREATE FOREIGN TABLE page?
> 
> done

Thanks.

Regards,
Amit

Re: Sparse bit set data structure

[Alvaro Herrera]

On 2019-Apr-08, Bruce Momjian wrote:

> Uh, should this be applied?

Yes, it's a pretty obvious typo methinks.

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

Re: change password_encryption default to scram-sha-256?

[Tatsuo Ishii]

>> I am not sure all third party programs concerning scram-sha-256 are
>> listed on this. There are some programs that talk to PostgreSQL using
>> frontend/backend protocol, but not based on libpq or other native
>> drivers (for example Pgpool-II). I guess PgBouncer is in the same
>> category too.
>>
> ... and pgbouncer doesn't support scram-sha-256 authentication method.
> There is a bit-rot PR but the discussion died a while ago. It is
> widely used and it would be really sad to turn on SCRAM on v13 without
> pgbouncer SCRAM support.

I don't how hard it would be for pgbouncer to support scram-sha-256,
but it was pretty hard for Pgpool-II to support scram-sha-256. In case
of Pgpool-II (it starts to support it since 4.0), it needed to keep
clients' password lists.

http://www.pgpool.net/docs/latest/en/html/auth-methods.html#AUTH-SCRAM

Best regards,
--
Tatsuo Ishii
SRA OSS, Inc. Japan
English: http://www.sraoss.co.jp/index_en.php
Japanese:http://www.sraoss.co.jp

RE: reloption to prevent VACUUM from truncating empty pages at the end of relation

[Tsunakawa, Takayuki]

From: Fujii Masao [mailto:masao.fu...@gmail.com]
> Thanks for the info, so I marked the patch as committed.

Thanks a lot for your hard work!  This felt relatively tough despite the 
simplicity of the patch.  I'm starting to feel the difficulty and fatigue in 
developing in the community...


Regards
Takayuki Tsunakawa

Re: Adding a concept of TEMPORARY TABLESPACE for the use in temp_tablespaces

[Bruce Momjian]

On Thu, Mar 14, 2019 at 12:53:02AM -0700, Mitar wrote:
> Hi!
> 
> On Fri, Jan 25, 2019 at 2:32 PM Bruce Momjian  wrote:
> > I wrote a blog entry about this:
> >
> > https://momjian.us/main/blogs/pgblog/2017.html#June_2_2017
> >
> > This is certainly an area we can improve, but it would require changes
> > in several parts of the system to handle cases where the tablespace
> > disappears.
> 
> Yes, I read the discussion thread you point at the end of your blog
> post. [1] This is why I posted an e-mail to the mailing list because
> some statements from that thread do not hold anymore. For example, in
> the thread it is stated:
> 
> "Just pointing the tablespace to non'restart'safe storage will get you
> an installation that fails to boot after a restart, since there's a
> tree structure that is expected to survive, and when it's not found,
> postgres just fails to boot."
> 
> This does not seem to be true (anymore?) based on my testing. You get
> noise in logs, but installation boots without a problem.
> 
> So maybe we are closer to this than we realize?

Interesting.  What happens when you references objects that were in the
tablespace?  What would we want to happen?

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

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

Re: Problem with default partition pruning

[Kyotaro HORIGUCHI]

At Mon, 08 Apr 2019 20:42:51 +0900 (Tokyo Standard Time), Kyotaro HORIGUCHI 
 wrote in 
<20190408.204251.143128146.horiguchi.kyot...@lab.ntt.co.jp>
> At Mon, 8 Apr 2019 16:57:35 +0900, "Yuzuko Hosoya" 
>  wrote in 
> <00c101d4ede0$babd4390$3037cab0$@lab.ntt.co.jp>
> > > BTW, now I'm a bit puzzled between whether this case should be fixed by 
> > > hacking on partprune.c like
> > > this patch does or whether to work on getting the other patch committed 
> > > and expect users to set
> > > constraint_exclusion = on for this to behave as expected.  The original 
> > > intention of setting
> > > partition_qual in set_relation_partition_info() was for partprune.c to 
> > > use it to remove useless
> > > arguments of OR clauses which otherwise would cause the failure to 
> > > correctly prune the default partitions
> > > of sub-partitioned tables.  As shown by the examples in this thread, the 
> > > original effort was
> > > insufficient, which this patch aims to improve.  But, it also expands the 
> > > scope of partprune.c's usage
> > > of partition_qual, which is to effectively perform full-blown constraint 
> > > exclusion without being
> > > controllable by constraint_exclusion GUC, which may be seen as being good 
> > > or bad.  The fact that it
> > > helps in getting partition pruning working correctly in more obscure 
> > > cases like those discussed in
> > > this thread means it's good maybe.
> > > 
> > Umm, even though this modification might be overhead, I think this problem 
> > should be solved
> > without setting constraint_exclusion GUC. But I'm not sure.
> 
> Partition pruning and constraint exclusion are orthogonal
> functions. Note that the default value of the variable is
> CONSTRAINT_EXCLUSION_PARTITION and the behavior is not a perfect
> bug.  So I think we can reasonably ignore constraints when
> constraint_exclusion is intentionally turned off.

> As the result I propose to move the "if(partconstr)" block in the
> latest patches after the constant-false block, changing the
> condition as "if (partconstr && constraint_exclusion !=
> CONSTRAINT_EXCLUSION_OFF)".

Mmm. Something is wrong. I should have been sleeping at the
time. In my opinion, what we should there is:

- Try partition pruning first.

- If the partition was not pruned, and constraint is set, check
  for constant false.

- if constraint_exclusion is turned on and constraint is set,
  examine the constraint.

Sorry for the stupidity.

regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center

Re: change password_encryption default to scram-sha-256?

[Euler Taveira]

Em seg, 8 de abr de 2019 às 19:43, Tatsuo Ishii  escreveu:
>
> I am not sure all third party programs concerning scram-sha-256 are
> listed on this. There are some programs that talk to PostgreSQL using
> frontend/backend protocol, but not based on libpq or other native
> drivers (for example Pgpool-II). I guess PgBouncer is in the same
> category too.
>
... and pgbouncer doesn't support scram-sha-256 authentication method.
There is a bit-rot PR but the discussion died a while ago. It is
widely used and it would be really sad to turn on SCRAM on v13 without
pgbouncer SCRAM support.


-- 
   Euler Taveira   Timbira -
http://www.timbira.com.br/
   PostgreSQL: Consultoria, Desenvolvimento, Suporte 24x7 e Treinamento

Re: Sparse bit set data structure

[Bruce Momjian]

Uh, should this be applied?

---

On Thu, Mar 28, 2019 at 03:46:03PM +0100, Adrien NAYRAT wrote:
> Hello,
> 
> According to the draw and simple8b_mode struct comment, it seems there is a
> typo:
> 
> > *  20-bit integer   20-bit integer   20-bit integer
> >  * 1101 00010010 00110010 00010100
> >  * ^
> >  * selector
> >  *
> >  * The selector 1101 is 13 in decimal.  From the modes table below, we see
> >  * that it means that the codeword encodes three 12-bit integers.  In 
> > decimal,
> >  * those integers are 18, 50 and 20.  Because we encode deltas rather 
> > than
> >  * absolute values, the actual values that they represent are 18, 500018 and
> >  * 500038.
> [...]
> > {20, 3},/* mode 13: three 20-bit integers */
> 
> 
> The comment should be "the codeword encodes three *20-bit* integers" ?
> 
> Patch attached.
> 
> Regards,

> diff --git a/src/backend/lib/integerset.c b/src/backend/lib/integerset.c
> index 28b4a38609..9984fd55e8 100644
> --- a/src/backend/lib/integerset.c
> +++ b/src/backend/lib/integerset.c
> @@ -805,7 +805,7 @@ intset_binsrch_leaf(uint64 item, leaf_item *arr, int 
> arr_elems, bool nextkey)
>   * selector
>   *
>   * The selector 1101 is 13 in decimal.  From the modes table below, we see
> - * that it means that the codeword encodes three 12-bit integers.  In 
> decimal,
> + * that it means that the codeword encodes three 20-bit integers.  In 
> decimal,
>   * those integers are 18, 50 and 20.  Because we encode deltas rather 
> than
>   * absolute values, the actual values that they represent are 18, 500018 and
>   * 500038.


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

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

Re: Fix doc bug in logical replication.

[Euler Taveira]

Em seg, 8 de abr de 2019 às 19:38, Robert Treat  escreveu:
>
> I noticed that the docs currently state "A different order of columns
> in the target table is allowed, but the column types have to match."
> This is untrue, as you can replicate between any two data types as
> long as the data can be coerced into the right format on the
> subscriber. Attached is a patch that attempts to clarify this, and
> provides some additional wordsmithing of that section. Patch is
> against head but the nature of the patch would apply to the docs for
> 11 and 10, which both have the incorrect information as well, even if
> the patch itself does not.
>
I would say it is inaccurate because the actual instruction works. I
agree that your words are an improvement but it lacks a comment on the
slot_[store|modify]_cstrings.


-- 
   Euler Taveira   Timbira -
http://www.timbira.com.br/
   PostgreSQL: Consultoria, Desenvolvimento, Suporte 24x7 e Treinamento

Re: change password_encryption default to scram-sha-256?

[Tatsuo Ishii]

> On Sun, Apr 07, 2019 at 12:59:05PM -0400, Tom Lane wrote:
>> Peter Eisentraut  writes:
>> > Should we change the default of the password_encryption setting to
>> > 'scram-sha-256' in PG12?
>> 
>> I thought we were going to wait a bit longer --- that just got added
>> last year, no?  What do we know about the state of support in client
>> libraries?
> 
> Great idea!  Does it make sense to test all, or at least some
> significant fraction of the connectors listed in
> https://wiki.postgresql.org/wiki/Client_Libraries by default?

I am not sure all third party programs concerning scram-sha-256 are
listed on this. There are some programs that talk to PostgreSQL using
frontend/backend protocol, but not based on libpq or other native
drivers (for example Pgpool-II). I guess PgBouncer is in the same
category too.

Best regards,
--
Tatsuo Ishii
SRA OSS, Inc. Japan
English: http://www.sraoss.co.jp/index_en.php
Japanese:http://www.sraoss.co.jp

Fix doc bug in logical replication.

[Robert Treat]

Howdy folks,

I noticed that the docs currently state "A different order of columns
in the target table is allowed, but the column types have to match."
This is untrue, as you can replicate between any two data types as
long as the data can be coerced into the right format on the
subscriber. Attached is a patch that attempts to clarify this, and
provides some additional wordsmithing of that section. Patch is
against head but the nature of the patch would apply to the docs for
11 and 10, which both have the incorrect information as well, even if
the patch itself does not.


Robert Treat
https://xzilla.net


logical_replication_type_mismatch.patch
Description: Binary data

Re: Trailing whitespaces in various documentations

[Julien Rouhaud]

Le lun. 8 avr. 2019 à 22:43, Peter Eisentraut <
peter.eisentr...@2ndquadrant.com> a écrit :

> On 2019-04-07 18:51, Julien Rouhaud wrote:
> > While working on unrelated documentation change, I noticed some
> > trailing whitespaces in various documentation files.  PFA a simple
> > patch to get rid of them (I didn't removed the one corresponding to
> > psql output), if that helps.
>
> committed, thanks


Thanks!

Re: change password_encryption default to scram-sha-256?

[Jonathan S. Katz]

On 4/8/19 4:20 PM, Alvaro Herrera wrote:
> On 2019-Apr-08, Jonathan S. Katz wrote:
> 
>> On 4/8/19 4:10 PM, Alvaro Herrera wrote:
> 
>>> I wonder why we have two pages
>>> https://wiki.postgresql.org/wiki/Client_Libraries
>>> https://wiki.postgresql.org/wiki/List_of_drivers
>>
>> No clue, but it appears that first one is the newer of the two[1][2]
>>
>> I'd be happy to consolidate them and provide a forwarding reference from
>> Client Libraries to List of Drivers, given I think we reference "List of
>> Drivers" in other places.
> 
> There are two links to List of drivers, and one of them is in Client
> Libraries :-)
> https://wiki.postgresql.org/wiki/Special:WhatLinksHere/Client_Libraries
> 
> +1 for consolidation and setting up a redirect.

OK, so trying to not be too off topic, I did update the original page as so:

https://wiki.postgresql.org/wiki/List_of_drivers

When determining what to add, I tried to keep it one-abstraction level
deep, i.e., a driver is implemented on top of libpq, implemented the PG
protocol on its own, or did some driver-like extensions on top of the
base language driver. I steered clear of ORMs or other abstraction
layers unless they met the above criteria.

(There are a lot of handy ORM-ish abstraction layers as well, but I
don't want to go down that path on that page, at least not today).

I also added a deprecation warning on top of the "Client Libraries"
page. If we're feeling satisfied with the consolidation, I'll wipe the
content and indicate where the maintained content is and end the
split-brain situation.

(One thing that I will say is this is one of those sections that may be
worth moving to pgweb, to give it some semi-permanence. Separate
discussion.)

The good news: while going through the added drivers, most of the
non-libpq ones I've added do support SCRAM :)

That said, I am still in favor of the PG13 plan, and without objection I
would like to reach out to the driver authors in the "no" category,
reference this thread, and that this is at least discussed, if not
decided upon, and they should considering adding support for SCRAM to
allow adequate testing time as well as time for their drivers to make it
into appropriate packaging systems.

Jonathan



signature.asc
Description: OpenPGP digital signature

Re: PageGetFreeSpace() isn't quite the right thing for some of its callers

[Peter Geoghegan]

On Mon, Apr 8, 2019 at 2:10 PM Andres Freund  wrote:
> I'm not sure I understand what the problem is. We got to get the
> information for the fsm from somewhere? Are you arguing we should
> instead have it included as an explicit xlog record payload?

No. I am simply pointing out that PageGetFreeSpace() "should usually
only be used on index pages" according to its own comments. And yet
it's called for other stuff.

Maybe it's not that important in that one instance, but I find it
pretty distracting that PageGetFreeSpace() is intended for index AMs
that use conventional line pointers.

-- 
Peter Geoghegan

Re: PageGetFreeSpace() isn't quite the right thing for some of its callers

[Andres Freund]

Hi,

On 2019-04-08 14:05:02 -0700, Peter Geoghegan wrote:
> However, now that I take a closer look I also notice that there is
> core code that calls PageGetFreeSpace() when it probably shouldn't,
> either. For example, what business does heap_xlog_visible() have
> calling PageGetFreeSpace()?

I'm not sure I understand what the problem is. We got to get the
information for the fsm from somewhere? Are you arguing we should
instead have it included as an explicit xlog record payload? Or just
that it should use PageGetExactFreeSpace()? I assume the former based on
your "what business" language, but that'd not make terribly much sense
to me.  I don't think precision terribly matters here...

Greetings,

Andres Freund

Re: change password_encryption default to scram-sha-256?

[Tom Lane]

Dave Cramer  writes:
> That said 42.2.0 was released in January 2018, so by PG13 it's going to be
> 4 years old.

Huh?  13 should come out in the fall of 2020.

regards, tom lane

Re: [PATCH] Implement uuid_version()

[Andres Freund]

Hi,

On 2019-04-08 11:06:57 -0400, Robert Haas wrote:
> That's not really a compelling reason, though, because anybody who
> needs UUIDs can always install the extension.  And on the other hand,
> if we moved UUID support into core, then we'd be adding a hard compile
> dependency on one of the UUID facilities, which might annoy some
> developers.  We could possibly work around that by implementing our
> own UUID facilities in core, but I'm not volunteering to do the work,
> and I'm not sure that the work has enough benefit to justify the
> labor.

The randomness based UUID generators don't really have dependencies, now
that we have a dependency on strong randomness.  I kinda thing the
dependency argument actually works *against* uuid-ossp - precisely
because of its dependencies (which also vary by OS) it's not a proper
replacement for a type of facility a very sizable fraction of our users
need.

Greetings,

Andres Freund

PageGetFreeSpace() isn't quite the right thing for some of its callers

[Peter Geoghegan]

I notice that a number of contrib modules call PageGetFreeSpace()
where they should really call PageGetExactFreeSpace() instead.
PageGetFreeSpace() assumes that the overhead for one line pointer
should be pro-actively subtracted, which is handy for plenty of nbtree
code, but doesn't quite make sense for stuff like pageinspect's
bt_page_stats() function.

I was thinking about fixing one or two of these buglets, without
expecting that to be complicated in any way. However, now that I take
a closer look I also notice that there is core code that calls
PageGetFreeSpace() when it probably shouldn't, either. For example,
what business does heap_xlog_visible() have calling
PageGetFreeSpace()? I also doubt that terminate_brin_buildstate()
should call it -- doesn't BRIN avoid using conventional item pointers?
Doesn't GIN's entryIsEnoughSpace() function double-count the item
pointer overhead?

I wonder if we should add an assertion based on the pd_special offset
of the page to PageGetFreeSpace(). That would make it harder to make
mistakes like this in the future. Maybe it would be better to revise
the whole API instead, though.
-- 
Peter Geoghegan

Re: [PATCH] Implement uuid_version()

[Jose Luis Tallon]

On 8/4/19 17:06, Robert Haas wrote:

On Sun, Apr 7, 2019 at 10:15 AM David Fetter  wrote:

I see some.

UUIDs turn out to be super useful in distributed systems to give good
guarantees of uniqueness without coordinating with a particular node.
Such systems have become a good bit more common since the most recent
time this was discussed.

That's not really a compelling reason, though, because anybody who
needs UUIDs can always install the extension.  And on the other hand,
if we moved UUID support into core, then we'd be adding a hard compile
dependency on one of the UUID facilities, which might annoy some
developers.  We could possibly work around that by implementing our
own UUID facilities in core,


Yup. My proposal basically revolves around implementing v3 / v4 / v5 
(most used/useful versions for the aforementioned use cases) in core, 
using the already existing md5 and sha1 facilities (which are already 
being linked from the current uuid-ossp extension as fallback with 
certain configurations) ... and leaving the remaining functionality in 
the extension, just as it is now.


This way, we guarantee backwards compatibility: Those already using the 
extension wouldn't have to change anything, and new users won't need to 
load any extension to benefit from this (base) functionality.



  but I'm not volunteering to do the work,

Of course, I'd take care of that :)

and I'm not sure that the work has enough benefit to justify the
labor.


With this "encouragement", I'll write the code and submit the patches to 
a future commitfest. Then the normal procedure will take care of judging 
whether it's worth being included or not :$



My biggest gripe about uuid-ossp is that the name is stupid.  I wish
we could see our way clear to renaming that extension to just 'uuid',
because as J.L. says, virtually nobody's actually compiling against
the OSSP library any more.  The trick there is how to do that without
annoying exiting users.  Maybe we could leave behind an "upgrade"
script for the uuid-ossp extension that does CREATE EXTENSION uuid,
then alters all objects owned by the current extension to be owned by
the new extension, and maybe even drops itself.


I believe my proposal above mostly solves the issue: new users with 
"standard" needs won't need to load any extension (better than current), 
old users will get the same functionality as they have today (only part 
in core and part in the extension)...


 ...and a relatively simple "alias" (think Linux kernel modules) 
facility would make the transition fully transparent: rename extension 
to "uuid" ---possibly dropping the dependency on uuid-ossp in the 
process--- and expose an "uuid-ossp" alias for backwards compatibility.



Thanks,

    J.L.

Re: change password_encryption default to scram-sha-256?

[Dave Cramer]

On Mon, 8 Apr 2019 at 16:38, Tom Lane  wrote:

> Dave Cramer  writes:
> >> If someone installs a postgres RPM/DEB from postgresql.org, they could
> >> also install postgresql-jdbc, right ?
>
> > I would guess there might be some distro specific java apps that might
> > actually use what is on the machine but as mentioned any reasonably
> complex
> > Java app is going to ensure it has the correct versions for their app
> using
> > Maven.
>
> I'm not really sure if that makes things better or worse.  If some app
> thinks that it needs version N of the driver, but SCRAM support was
> added in version N-plus-something, how tough is it going to be to get
> it updated?  And are you going to have to go through that dance for
> each app separately?
>
>

I see the problem you are contemplating, but even installing a newer
version of the driver has it's perils (we have been known to break some
expectations in the name of the spec).
So I could see a situation where there is a legacy app that wants to use
SCRAM. They update the JDBC jar on the system and due to the "new and
improved" version their app breaks.
Honestly I don't have a solution to this.

That said 42.2.0 was released in January 2018, so by PG13 it's going to be
4 years old.

Dave

Re: Trailing whitespaces in various documentations

[Peter Eisentraut]

On 2019-04-07 18:51, Julien Rouhaud wrote:
> While working on unrelated documentation change, I noticed some
> trailing whitespaces in various documentation files.  PFA a simple
> patch to get rid of them (I didn't removed the one corresponding to
> psql output), if that helps.

committed, thanks

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

Re: change password_encryption default to scram-sha-256?

[Tom Lane]

Dave Cramer  writes:
>> If someone installs a postgres RPM/DEB from postgresql.org, they could
>> also install postgresql-jdbc, right ?

> I would guess there might be some distro specific java apps that might
> actually use what is on the machine but as mentioned any reasonably complex
> Java app is going to ensure it has the correct versions for their app using
> Maven.

I'm not really sure if that makes things better or worse.  If some app
thinks that it needs version N of the driver, but SCRAM support was
added in version N-plus-something, how tough is it going to be to get
it updated?  And are you going to have to go through that dance for
each app separately?

regards, tom lane

Re: change password_encryption default to scram-sha-256?

[Dave Cramer]

>
>
>
> > The scenario that worries me here is somebody using a bleeding-edge PGDG
> > server package in an environment where the rest of the Postgres ecosystem
> > is much less bleeding-edge.
>
> If someone installs a postgres RPM/DEB from postgresql.org, they could
> also
> install postgresql-jdbc, right ?
>
>
No, this is not how the majority of people use Java at all. They would use
Maven to pull down the JDBC driver of choice.

I would guess there might be some distro specific java apps that might
actually use what is on the machine but as mentioned any reasonably complex
Java app is going to ensure it has the correct versions for their app using
Maven.

Dave Cramer

da...@postgresintl.com
www.postgresintl.com


>

Re: change password_encryption default to scram-sha-256?

[Alvaro Herrera]

On 2019-Apr-08, Tom Lane wrote:

> I'm particularly concerned about the idea that they won't see a problem
> during initial testing, only to have things fall over after they enter
> production and do a "routine" password change.

This is a fair objection.

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

RE: minimizing pg_stat_statements performance overhead

[legrand legrand]

CF entry created 
https://commitfest.postgresql.org/23/2092/

Regards
PAscal



--
Sent from: http://www.postgresql-archive.org/PostgreSQL-hackers-f1928748.html

Re: change password_encryption default to scram-sha-256?

[Alvaro Herrera]

On 2019-Apr-08, Jonathan S. Katz wrote:

> On 4/8/19 4:10 PM, Alvaro Herrera wrote:

> > I wonder why we have two pages
> > https://wiki.postgresql.org/wiki/Client_Libraries
> > https://wiki.postgresql.org/wiki/List_of_drivers
> 
> No clue, but it appears that first one is the newer of the two[1][2]
> 
> I'd be happy to consolidate them and provide a forwarding reference from
> Client Libraries to List of Drivers, given I think we reference "List of
> Drivers" in other places.

There are two links to List of drivers, and one of them is in Client
Libraries :-)
https://wiki.postgresql.org/wiki/Special:WhatLinksHere/Client_Libraries

+1 for consolidation and setting up a redirect.

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

Re: change password_encryption default to scram-sha-256?

[Tom Lane]

Justin Pryzby  writes:
> On Mon, Apr 08, 2019 at 02:28:30PM -0400, Tom Lane wrote:
>> The scenario that worries me here is somebody using a bleeding-edge PGDG
>> server package in an environment where the rest of the Postgres ecosystem
>> is much less bleeding-edge.

> If someone installs a postgres RPM/DEB from postgresql.org, they could also
> install postgresql-jdbc, right ?

The client software is very possibly not on the same machine as the server,
and may indeed not be under the server admin's control.  That sort of
complex interdependency is why we need to move slowly on changes that
require client updates.

regards, tom lane

Re: change password_encryption default to scram-sha-256?

[Jonathan S. Katz]

On 4/8/19 4:10 PM, Alvaro Herrera wrote:
> On 2019-Apr-08, Dave Cramer wrote:
> 
>> On Mon, 8 Apr 2019 at 16:07, Alvaro Herrera 
>> wrote:
> 
>>> I meant an exception to the common situation that SCRAM-SHA-256 is
>>> supported and shipped in stable releases of each driver.  The wiki here
>>> still says it's unsupported on JDBC:
>>> https://wiki.postgresql.org/wiki/List_of_drivers
>>> For once I'm happy to learn that the wiki is outdated :-)
>>
>> Way too many places to update :)
> 
> Yeah.  Actually, it's up to date (it says "yes from 42.2")... I just
> misread it.
> 
> I wonder why we have two pages
> https://wiki.postgresql.org/wiki/Client_Libraries
> https://wiki.postgresql.org/wiki/List_of_drivers

No clue, but it appears that first one is the newer of the two[1][2]

I'd be happy to consolidate them and provide a forwarding reference from
Client Libraries to List of Drivers, given I think we reference "List of
Drivers" in other places.

Jonathan

[1]
https://wiki.postgresql.org/index.php?title=Client_Libraries=history
[2]
https://wiki.postgresql.org/index.php?title=List_of_drivers=history



signature.asc
Description: OpenPGP digital signature

Re: change password_encryption default to scram-sha-256?

[Alvaro Herrera]

On 2019-Apr-08, Dave Cramer wrote:

> On Mon, 8 Apr 2019 at 16:07, Alvaro Herrera 
> wrote:

> > I meant an exception to the common situation that SCRAM-SHA-256 is
> > supported and shipped in stable releases of each driver.  The wiki here
> > still says it's unsupported on JDBC:
> > https://wiki.postgresql.org/wiki/List_of_drivers
> > For once I'm happy to learn that the wiki is outdated :-)
> 
> Way too many places to update :)

Yeah.  Actually, it's up to date (it says "yes from 42.2")... I just
misread it.

I wonder why we have two pages
https://wiki.postgresql.org/wiki/Client_Libraries
https://wiki.postgresql.org/wiki/List_of_drivers

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

Re: change password_encryption default to scram-sha-256?

[Dave Cramer]

On Mon, 8 Apr 2019 at 16:07, Alvaro Herrera 
wrote:

> On 2019-Apr-08, Dave Cramer wrote:
>
> > > IIUC the vast majority of clients already support SCRAM auth.  So the
> > > vast majority of PG users can take advantage of the additional
> security.
> > > I think the only massive-adoption exception is JDBC, and apparently
> they
> > > already have working patches for SCRAM.
> >
> > We have more than patches this is already in the driver.
> >
> > What do you mean by "massive-adoption exception"
>
> I meant an exception to the common situation that SCRAM-SHA-256 is
> supported and shipped in stable releases of each driver.  The wiki here
> still says it's unsupported on JDBC:
> https://wiki.postgresql.org/wiki/List_of_drivers
> For once I'm happy to learn that the wiki is outdated :-)
>


Way too many places to update :)


Dave Cramer

da...@postgresintl.com
www.postgresintl.com


>
>

Re: change password_encryption default to scram-sha-256?

[Alvaro Herrera]

On 2019-Apr-08, Dave Cramer wrote:

> > IIUC the vast majority of clients already support SCRAM auth.  So the
> > vast majority of PG users can take advantage of the additional security.
> > I think the only massive-adoption exception is JDBC, and apparently they
> > already have working patches for SCRAM.
> 
> We have more than patches this is already in the driver.
> 
> What do you mean by "massive-adoption exception"

I meant an exception to the common situation that SCRAM-SHA-256 is
supported and shipped in stable releases of each driver.  The wiki here
still says it's unsupported on JDBC:
https://wiki.postgresql.org/wiki/List_of_drivers
For once I'm happy to learn that the wiki is outdated :-)

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

Re: change password_encryption default to scram-sha-256?

[Dave Cramer]

On Mon, 8 Apr 2019 at 15:18, Jonathan S. Katz  wrote:

> On 4/8/19 2:28 PM, Tom Lane wrote:
> > Andres Freund  writes:
> >> On 2019-04-08 13:34:12 -0400, Alvaro Herrera wrote:
> >>> I'm not sure I understand all this talk about deferring changing the
> >>> default to pg13.  AFAICS only a few fringe drivers are missing support;
> >>> not changing in pg12 means we're going to leave *all* users, even those
> >>> whose clients have support, without the additional security for 18 more
> >>> months.
> >
> >> Imo making such changes after feature freeze is somewhat poor
> >> form.
> >
> > Yeah.
>
> Yeah, that's fair.
>
> >
> >> If jdbc didn't support scram, it'd be an absolutely clear no-go imo. A
> >> pretty large fraction of users use jdbc to access postgres. But it seems
> >> to me that support has been merged for a while:
> >> https://github.com/pgjdbc/pgjdbc/pull/1014
> >
> > "Merged to upstream" is a whole lot different from "readily available in
> > the field".  What's the actual status in common Linux distros, for
> > example?
>
> Did some limited research just to get a sense.
>
> Well, if it's RHEL7, it's PostgreSQL 9.2 so, unless they're using our
> RPM, that definitely does not have it :)
>
> (While researching this, I noticed on the main RHEL8 beta page[1] that
> PostgreSQL is actually featured, which is kind of neat. I could not
> quickly find which version of the JDBC driver it is shipping with, though)
>
> On Ubuntu, 18.04 LTS ships PG10, but the version of JDBC does not
> include SCRAM support. 18.10 ships JDBC w/SCRAM support.
>
> On Debian, stretch is on 9.4. buster has 11 packaged, and JDBC is
> shipping with SCRAM support.
>
>

Honestly what JDBC driver XYZ distro ships with is a red herring. Any
reasonably complex java program is going to use maven and pull it's
dependencies.

That said from a driver developer, I support pushing this decision off to
PG13

Dave Cramer

da...@postgresintl.com
www.postgresintl.com


>
>

Re: change password_encryption default to scram-sha-256?

[Dave Cramer]

Alvaro,

On Mon, 8 Apr 2019 at 13:34, Alvaro Herrera 
wrote:

> I'm not sure I understand all this talk about deferring changing the
> default to pg13.  AFAICS only a few fringe drivers are missing support;
> not changing in pg12 means we're going to leave *all* users, even those
> whose clients have support, without the additional security for 18 more
> months.
>
> IIUC the vast majority of clients already support SCRAM auth.  So the
> vast majority of PG users can take advantage of the additional security.
> I think the only massive-adoption exception is JDBC, and apparently they
> already have working patches for SCRAM.
>


We have more than patches this is already in the driver.

What do you mean by "massive-adoption exception"

Dave Cramer

da...@postgresintl.com
www.postgresintl.com



>
>

Re: change password_encryption default to scram-sha-256?

[Justin Pryzby]

On Mon, Apr 08, 2019 at 02:28:30PM -0400, Tom Lane wrote:
>On Mon, Apr 08, 2019 at 10:41:07AM -0700, Andres Freund wrote:
>> If jdbc didn't support scram, it'd be an absolutely clear no-go imo. A
>> pretty large fraction of users use jdbc to access postgres. But it seems
>> to me that support has been merged for a while:
>> https://github.com/pgjdbc/pgjdbc/pull/1014
> 
> "Merged to upstream" is a whole lot different from "readily available in
> the field".  What's the actual status in common Linux distros, for
> example?

I found:

https://jdbc.postgresql.org/documentation/changelog.html#version_42.2.1
Version 42.2.0 (2018-01-17)
Added
Support SCRAM-SHA-256 for PostgreSQL 10 in the JDBC 4.2 version (Java 8+) using 
the Ongres SCRAM library. PR 842

I see that's in ubuntu, but not any LTS release:
https://packages.ubuntu.com/search?keywords=libpostgresql-jdbc-java

And in Debian testing, but no released version:
https://packages.debian.org/search?keywords=libpostgresql-jdbc-java

For centos6/7, OS packages would not have scram support:

$ yum list --showdupl postgresql-jdbc
Available Packages
postgresql-jdbc.noarch  
 9.2.1002-6.el7_5   
 base
postgresql-jdbc.noarch  
 42.2.5-1.rhel7.1   
 pgdg11

$ yum list --showdupl postgresql-jdbc
Available Packages
postgresql-jdbc.noarch  
 8.4.704-2.el6  
 base
postgresql-jdbc.noarch  
 42.2.5-1.rhel6.1   
 pgdg11

> The scenario that worries me here is somebody using a bleeding-edge PGDG
> server package in an environment where the rest of the Postgres ecosystem
> is much less bleeding-edge.

If someone installs a postgres RPM/DEB from postgresql.org, they could also
install postgresql-jdbc, right ?

I realize that doesn't mean that people will consistently know to and actually
do that.

If the default were changed, possibly the PGDG package could define something
like (I haven't done packaging in a long time):
Conflicts: postgresql-jdbc<42.2.0

On Mon, Apr 08, 2019 at 03:18:42PM -0400, Jonathan S. Katz wrote:
> Well, if it's RHEL7, it's PostgreSQL 9.2 so, unless they're using our
> RPM, that definitely does not have it :)

Re: change password_encryption default to scram-sha-256?

[Jonathan S. Katz]

On 4/8/19 2:28 PM, Tom Lane wrote:
> Andres Freund  writes:
>> On 2019-04-08 13:34:12 -0400, Alvaro Herrera wrote:
>>> I'm not sure I understand all this talk about deferring changing the
>>> default to pg13.  AFAICS only a few fringe drivers are missing support;
>>> not changing in pg12 means we're going to leave *all* users, even those
>>> whose clients have support, without the additional security for 18 more
>>> months.
> 
>> Imo making such changes after feature freeze is somewhat poor
>> form.
> 
> Yeah.

Yeah, that's fair.

> 
>> If jdbc didn't support scram, it'd be an absolutely clear no-go imo. A
>> pretty large fraction of users use jdbc to access postgres. But it seems
>> to me that support has been merged for a while:
>> https://github.com/pgjdbc/pgjdbc/pull/1014
> 
> "Merged to upstream" is a whole lot different from "readily available in
> the field".  What's the actual status in common Linux distros, for
> example?

Did some limited research just to get a sense.

Well, if it's RHEL7, it's PostgreSQL 9.2 so, unless they're using our
RPM, that definitely does not have it :)

(While researching this, I noticed on the main RHEL8 beta page[1] that
PostgreSQL is actually featured, which is kind of neat. I could not
quickly find which version of the JDBC driver it is shipping with, though)

On Ubuntu, 18.04 LTS ships PG10, but the version of JDBC does not
include SCRAM support. 18.10 ships JDBC w/SCRAM support.

On Debian, stretch is on 9.4. buster has 11 packaged, and JDBC is
shipping with SCRAM support.

> The scenario that worries me here is somebody using a bleeding-edge PGDG
> server package in an environment where the rest of the Postgres ecosystem
> is much less bleeding-edge.  The last time that situation would have
> caused them can't-connect problems was, um, probably when we introduced
> MD5 password encryption.  So they won't be expecting to get blindsided by
> something like this.
> 
> I'm particularly concerned about the idea that they won't see a problem
> during initial testing, only to have things fall over after they enter
> production and do a "routine" password change.

Yeah, I think all of the above is fair. It's been awhile since md5 was
added :)

So I think based on that and a quick look at the different distros
indicate that changing the default to PG12 has too much risk of
breakage, but PG13 would be a fair target as long as we start making
noise sooner (now?).

Jonathan

[1] https://developers.redhat.com/rhel8/



signature.asc
Description: OpenPGP digital signature

Re: change password_encryption default to scram-sha-256?

[Tom Lane]

Andres Freund  writes:
> On 2019-04-08 13:34:12 -0400, Alvaro Herrera wrote:
>> I'm not sure I understand all this talk about deferring changing the
>> default to pg13.  AFAICS only a few fringe drivers are missing support;
>> not changing in pg12 means we're going to leave *all* users, even those
>> whose clients have support, without the additional security for 18 more
>> months.

> Imo making such changes after feature freeze is somewhat poor
> form.

Yeah.

> If jdbc didn't support scram, it'd be an absolutely clear no-go imo. A
> pretty large fraction of users use jdbc to access postgres. But it seems
> to me that support has been merged for a while:
> https://github.com/pgjdbc/pgjdbc/pull/1014

"Merged to upstream" is a whole lot different from "readily available in
the field".  What's the actual status in common Linux distros, for
example?

The scenario that worries me here is somebody using a bleeding-edge PGDG
server package in an environment where the rest of the Postgres ecosystem
is much less bleeding-edge.  The last time that situation would have
caused them can't-connect problems was, um, probably when we introduced
MD5 password encryption.  So they won't be expecting to get blindsided by
something like this.

I'm particularly concerned about the idea that they won't see a problem
during initial testing, only to have things fall over after they enter
production and do a "routine" password change.

regards, tom lane

Re: ECPG regression with DECLARE STATEMENT support

[Bruce Momjian]

On Wed, Mar 13, 2019 at 04:35:48AM +, Matsumura, Ryo wrote:
> Hi Kurokawa-san
> 
> I reviewd it. It's ok.
> I also confirm there is no same bug.

FYI, this was applied a few weeks ago:

Author: Michael Meskes 
Date:   Fri Mar 15 22:35:24 2019 +0100

Use correct connection name variable in ecpglib.

Fixed-by: Kuroda-san 

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

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

Re: change password_encryption default to scram-sha-256?

[Andres Freund]

Hi,

On 2019-04-08 13:34:12 -0400, Alvaro Herrera wrote:
> I'm not sure I understand all this talk about deferring changing the
> default to pg13.  AFAICS only a few fringe drivers are missing support;
> not changing in pg12 means we're going to leave *all* users, even those
> whose clients have support, without the additional security for 18 more
> months.

Imo making such changes after feature freeze is somewhat poor
form. These arguments would have made a ton more sense at the
*beginning* of the v12 development cycle, because that'd have given all
these driver authors a lot more heads up.


> IIUC the vast majority of clients already support SCRAM auth.  So the
> vast majority of PG users can take advantage of the additional security.
> I think the only massive-adoption exception is JDBC, and apparently they
> already have working patches for SCRAM.

If jdbc didn't support scram, it'd be an absolutely clear no-go imo. A
pretty large fraction of users use jdbc to access postgres. But it seems
to me that support has been merged for a while:
https://github.com/pgjdbc/pgjdbc/pull/1014

Greetings,

Andres Freund

Re: ToDo: show size of partitioned table

[Alvaro Herrera]

On 2019-Apr-08, Amit Langote wrote:

> I noticed that the size shown in the output of both \dP+ and \dPt+ does
> include toast relation sizes of (leaf) partitions, because
> pg_table_size(), which is used by all the queries that
> listPartitionedTables sends to the server, does:
> 
> /*
>  * heap size, including FSM and VM
>  */
> for (forkNum = 0; forkNum <= MAX_FORKNUM; forkNum++)
> size += calculate_relation_size(&(rel->rd_node), rel->rd_backend,
> forkNum);
> 
> /*
>  * Size of toast relation
>  */
> if (OidIsValid(rel->rd_rel->reltoastrelid))
> size += calculate_toast_table_size(rel->rd_rel->reltoastrelid);

Sigh.  I must have mixed up whether pg_table_size did include toast size
or not.  Anyway, I think \dP should report the same thing as \d, which I
think it's doing, so we're good there.

> >> Also, I think the new \dP should gain a new flag (maybe "l") to make it
> >> list leaf tables/indexes too with their local sizes, and remove those
> >> from the standard \d listing.
> > 
> > +1
> 
> +1 to the idea.  Perhaps we'd need to post this in a separate thread to
> highlight what may be a substantive change to \d's output?

Aye aye.

Not in a position to work on that right now; I might get to that in a
few days if nobody beats me to it.  (But then, I might not.)

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

Re: Pluggable Storage - Andres's take

[Andres Freund]

Hi,

On 2019-04-08 15:34:46 +0300, Heikki Linnakangas wrote:
> There were a bunch of typos in the comments in tableam.h, see attached. Some
> of the comments could use more copy-editing and clarification, I think, but
> I stuck to fixing just typos and such for now.

I pushed these after adding three boring changes by pgindent. Thanks for
those!

I'd greatly welcome more feedback on the comments - I've been pretty
deep in this for so long that I don't see all of the issues anymore. And
a mild dyslexia doesn't help...


> index_update_stats() calls RelationGetNumberOfBlocks(). If the AM
> doesn't use normal data files, that won't work. I bumped into that with my
> toy implementation, which wouldn't need to create any data files, if it
> wasn't for this.

Hm. That should be fixed. I've been burning the candle on both ends for
too long, so I'll not get to it today. But I think we should fix it
soon.  I'll create an open item for it.


> The comments for relation_set_new_relfilenode() callback say that the AM can
> set *freezeXid and *minmulti to invalid. But when I did that, VACUUM hits
> this assertion:
> 
> TRAP: FailedAssertion("!(((classForm->relfrozenxid) >= ((TransactionId)
> 3)))", File: "vacuum.c", Line: 1323)

Hm. That needs to be fixed - IIRC it previously worked, because zheap
doesn't have relfrozenxid either. Probably broke it when trying to
winnow down the tableam patches. I'm planning to rebase zheap onto the
newest version soon, so I'll re-encounter this.


> There's a little bug in index-only scan executor node, where it mixes up the
> slots to hold a tuple from the index, and from the table. That doesn't cause
> any ill effects if the AM uses TTSOpsHeapTuple, but with my toy AM, which
> uses a virtual slot, it caused warnings like this from index-only scans:

Hm. That's another one that I think I had fixed previously :(, and then
concluded that it's not actually necessary for some reason. Your fix
looks correct to me.  Do you want to commit it? Otherwise I'll look at
it after rebasing zheap, and checking it with that.


> Attached is a patch with the toy implementation I used to test this. I'm not
> suggesting we should commit that - although feel free to do that if you
> think it's useful - but it shows how I bumped into these issues.

Hm, probably not a bad idea to include something like it. It seems like
we kinda would need non-stub implementation of more functions for it to
test much / and to serve as an example.  I'm mildy inclined to just do
it via zheap / externally, but I'm not quite sure that's good enough.


> +static Size
> +toyam_parallelscan_estimate(Relation rel)
> +{
> + ereport(ERROR,
> + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
> +  errmsg("function %s not implemented yet", __func__)));
> +}

The other stubbed functions seem like we should require them, but I
wonder if we should make the parallel stuff optional?


Greetings,

Andres Freund

Re: change password_encryption default to scram-sha-256?

[Alvaro Herrera]

I'm not sure I understand all this talk about deferring changing the
default to pg13.  AFAICS only a few fringe drivers are missing support;
not changing in pg12 means we're going to leave *all* users, even those
whose clients have support, without the additional security for 18 more
months.

IIUC the vast majority of clients already support SCRAM auth.  So the
vast majority of PG users can take advantage of the additional security.
I think the only massive-adoption exception is JDBC, and apparently they
already have working patches for SCRAM.

Like many other configuration parameters, setting the default for this
one is a trade-off: give the most benefit to most users, causing the
least possible pain to users for whom the default is not good.  Users
that require opening connections from clients that have not updated
should just set password_encryption to md5.  It's not like things will
suddenly blow up in their faces.

IMO we don't need to wait until every single client in existence has
updated to support SCRAM.  After all, they've already had two years.

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

pgbench - add \aset to store results of a combined query

[Fabien COELHO]

Hello devs,

A long time ago I submitted a pgbench \into command to store results of 
queries into variables independently of the query being processed, which 
got turn into \gset (;) and \cset (\;), which got committed, then \cset 
was removed because it was not "up to standard", as it could not work with 
empty query (the underlying issue is that pg silently skips empty queries, 
so that "\; SELECT 1 \; \; SELECT 3," returns 2 results instead of 4, a 
misplaced optimisation from my point of view).


Now there is a pgbench \gset which allows to extract the results of 
variables of the last query, but as it does both setting and ending a 
query at the same time, there is no way to set variables out of a combined 
(\;) query but the last, which is the kind of non orthogonal behavior that 
I dislike much.


This annoys me because testing the performance of combined queries cannot 
be tested if the script needs to extract variables.


To make the feature somehow accessible to combined queries, the attached 
patch adds the "\aset" (all set) command to store all results of queries 
which return just one row into variables, i.e.:


  SELECT 1 AS one \;
  SELECT 2 AS two UNION SELECT 2 \;
  SELECT 3 AS three \aset

will set both "one" and "three", while "two" is not set because there were 
two rows. It is a kind of more permissive \gset.


Because it does it for all queries, there is no need for synchronizing 
with the underlying queries, which made the code for \cset both awkward 
and with limitations. Hopefully this version might be "up to standard".

I'll see. I'm in no hurry:-)

--
Fabien.diff --git a/doc/src/sgml/ref/pgbench.sgml b/doc/src/sgml/ref/pgbench.sgml
index ee2501be55..83eb7f5f4a 100644
--- a/doc/src/sgml/ref/pgbench.sgml
+++ b/doc/src/sgml/ref/pgbench.sgml
@@ -966,18 +966,27 @@ pgbench  options  d

 
  \gset [prefix]
+ \aset [prefix]
 
 
 
  
-  This command may be used to end SQL queries, taking the place of the
+  These commands may be used to end SQL queries, taking the place of the
   terminating semicolon (;).
  
 
  
-  When this command is used, the preceding SQL query is expected to
-  return one row, the columns of which are stored into variables named after
-  column names, and prefixed with prefix if provided.
+  When the \gset command is used, the preceding SQL query is
+  expected to return one row, the columns of which are stored into variables
+  named after column names, and prefixed with prefix
+  if provided.
+ 
+
+ 
+  When the \aset command is used, all combined SQL queries
+  (separated by \;) which return just one row see their
+  columns stored into variables named after column names, and prefixed with
+  prefix if provided. Other queries are ignored.
  
 
  
@@ -986,6 +995,8 @@ pgbench  options  d
   p_two and p_three
   with integers from the third query.
   The result of the second query is discarded.
+  The result of the two last combined queries are stored in variables
+  four and five.
 
 UPDATE pgbench_accounts
   SET abalance = abalance + :delta
@@ -994,6 +1005,7 @@ UPDATE pgbench_accounts
 -- compound of two queries
 SELECT 1 \;
 SELECT 2 AS two, 3 AS three \gset p_
+SELECT 4 AS four \; SELECT 5 AS five \aset
 
  
 
diff --git a/src/bin/pgbench/pgbench.c b/src/bin/pgbench/pgbench.c
index 99529de52a..9bae707368 100644
--- a/src/bin/pgbench/pgbench.c
+++ b/src/bin/pgbench/pgbench.c
@@ -461,6 +461,7 @@ typedef enum MetaCommand
 	META_SHELL,	/* \shell */
 	META_SLEEP,	/* \sleep */
 	META_GSET,	/* \gset */
+	META_ASET,	/* \aset */
 	META_IF,	/* \if */
 	META_ELIF,	/* \elif */
 	META_ELSE,	/* \else */
@@ -493,6 +494,7 @@ static const char *QUERYMODE[] = {"simple", "extended", "prepared"};
  * varprefix 	SQL commands terminated with \gset have this set
  *to a non NULL value.  If nonempty, it's used to prefix the
  *variable name that receives the value.
+ * aset			do gset on all possible queries of a combined query (\;).
  * expr			Parsed expression, if needed.
  * stats		Time spent in this command.
  */
@@ -505,6 +507,7 @@ typedef struct Command
 	int			argc;
 	char	   *argv[MAX_ARGS];
 	char	   *varprefix;
+	bool		aset;
 	PgBenchExpr *expr;
 	SimpleStats stats;
 } Command;
@@ -2493,6 +2496,8 @@ getMetaCommand(const char *cmd)
 		mc = META_ENDIF;
 	else if (pg_strcasecmp(cmd, "gset") == 0)
 		mc = META_GSET;
+	else if (pg_strcasecmp(cmd, "aset") == 0)
+		mc = META_ASET;
 	else
 		mc = META_NONE;
 	return mc;
@@ -2729,7 +2734,7 @@ sendCommand(CState *st, Command *command)
  * Returns true if everything is A-OK, false if any error occurs.
  */
 static bool
-readCommandResponse(CState *st, char *varprefix)
+readCommandResponse(CState *st, char *varprefix, bool is_aset)
 {
 	PGresult   *res;
 	int			qrynum = 0;
@@ -2746,7 +2751,7 @@ readCommandResponse(CState *st, char *varprefix)
 		{
 			case 

Re: lazy_scan_heap() forgets to mark buffer dirty when setting all frozen?

[Andres Freund]

Hi,

On 2019-04-08 10:59:32 -0400, Robert Haas wrote:
> On Mon, Apr 8, 2019 at 12:55 AM Andres Freund  wrote:
> > > Apparently the initial commit a892234f830e had MarkBufferDirty, but it
> > > was removed one week later by 77a1d1e79892.
> >
> > Good catch. Kinda looks like it could have been an accidental removal?
> > Robert?
>
> So you're talking about this hunk?
>
> -/* Page is marked all-visible but should be all-frozen */
> -PageSetAllFrozen(page);
> -MarkBufferDirty(buf);
> -
>
> I don't remember exactly, but I am pretty sure that I assumed from the
> way that hunk looked that the MarkBufferDirty() was only needed there
> because of the call to PageSetAllFrozen().  Perhaps I should've
> figured it out from the "NB: If the heap page is all-visible..."
> comment, but I unfortunately don't find that comment to be very clear
> -- it basically says we don't need to do it, and then immediately
> contradicts itself by saying we sometimes do need to do it "because it
> may be logged."  But that's hardly an explanation, because why should
> the fact that the page is going to be logged require that it be
> dirtied?  We could improve the comment, but before we go there...

> Why the heck does visibilitymap_set() require callers to do
> MarkBufferDirty() instead of doing it itself?  Or at least, if it's
> got to work that way, can't it Assert() something?  It seems crazy to
> me that it calls PageSetLSN() without calling MarkBufferDirty() or
> asserting that the buffer is dirty or having a header comment that
> says that the buffer must be dirty. Ugh.

I think the visibilitymap_set() has incrementally gotten worse, to the
point that it should just be redone. Initially, before you made it
crashsafe, it indeed didn't do any I/O (like the header claims), and
didn't touch the heap. To make it crashsafe it started to call back into
heap. Then to support checksums, most of its callers had to take be
adapted around marking buffers dirty. And then the all-frozen stuff
complicated it a bit further.

I don't quite know what the right answer is, but I think
visibilitymap_set (or whatever it's successor is), shouldn't do any WAL
logging of its own, and it shouldn't call into heap - we want to be able
to reuse the vm for things like zheap after all. It's also just an
extremely confusing interface, especially because say
visibilitymap_clear() doesn't do WAL logging.

I think we should have a heap_visibilitymap_set() that does the WAL
logging, which internally calls into visibilitymap.c.

Perhaps it could look something roughly like:

static void
heap_visibilitymap_set(...)
{
Assert(LWLockIsHeld(BufferDescriptorGetIOLock(heapBuf))

Assert(PageIsAllVisible(heapPage));
/* other checks */

START_CRIT_SECTION();

/* if change required, this locks VM buffer */
if (visibilitmap_start_set(rel, heapBlk, ))
{
XLogRecPtr recptr;

MarkBufferDirty(heapBuf)

if (RelationNeedsWAL(rel))
{
/* inlined body of log_heap_visible */
recptr = XLogInsert(XLOG_HEAP2_VISIBLE);

/*
 * If data checksums are enabled (or wal_log_hints=on), we
 * need to protect the heap page from being torn.
 */
if (XLogHintBitIsNeeded())
{
PageheapPage = BufferGetPage(heapBuf);

/* caller is expected to set PD_ALL_VISIBLE first */
Assert(PageIsAllVisible(heapPage));
PageSetLSN(heapPage, recptr);
}
}


/* actually change bits, set page LSN, release vmbuf lock */
visibilitmap_finish_set(rel, heapBlk, vmbuf, recptr);
}

END_CRIT_SECTION();
}

The replay routines would then not use heap_visibilitymap_set (they
right now call visibilitymap_set with a valid XLogRecPtr), but instead
visibilitmap_redo_set() or such, which can check the LSNs etc.

Greetings,

Andres Freund

Re: Status of the table access method work

[Andres Freund]

Hi

On 2019-04-08 14:53:53 +0300, Heikki Linnakangas wrote:
> On 05/04/2019 23:25, Andres Freund wrote:
> > - the (optional) bitmap heap scan API - that's fairly intrinsically
> >block based. An AM could just internally subdivide TIDs in a different
> >way, but I don't think a bitmap scan like we have would e.g. make a
> >lot of sense for an index oriented table without any sort of stable
> >tid.
> 
> If an AM doesn't implement the bitmap heap scan API, what happens? Bitmap
> scans are disabled?

Yea, the planner doesn't consider them. It just masks the index's
amhasgetbitmap.  Seems to be the most reasonable thing to do?


> Even if an AM isn't block-oriented, the bitmap heap scan API still makes
> sense as long as there's some correlation between TIDs and physical
> location.

Yea, it could be a non-linear mapping. But I'm honestly not sure how
many non-block oriented AMs with such a correlation there are - I mean
you're not going to have that in say an IOT. And it'd be trivial to just
"fake" a block mapping for an in-memory AM.


> The only really broken thing about that currently is the
> prefetching: nodeBitmapHeapScan.c calls PrefetchBuffer() directly with the
> TID's block numbers. It would be pretty straightforward to wrap that in a
> callback, so that the AM could do something different.

That, and the VM_ALL_VISIBLE() checks both in nodeBitmapHeapscan.c and
nodeIndexonlyscan.c.


> Or move even more of the logic to the AM, so that the AM would get the whole
> TIDBitmap in table_beginscan_bm(). It could then implement the fetching and
> prefetching as it sees fit.
> 
> I don't think it's urgent, though. We can cross that bridge when we get
> there, with the first AM that needs that flexibility.

Yea, it seemed nontrivial (not in really hard, just not obvious), and
the implicated code duplication scared me away.


> > The most constraining factor for storage, I think, is that currently the
> > API relies on ItemPointerData style TIDs in a number of places (i.e. a 6
> > byte tuple identifier).
> 
> I think 48 bits would be just about enough

I don't think that's really true. Consider e.g. implementing an index
oriented table - there's no way you can efficiently implement one with
that small a key. You basically need a helper index just to have
efficient and small enough tids.  And given that we're also going to
need wider tids for global indexes, I suspect we're just going to have
to bite into the sour apple and make tids variable width.


> , but it's even more limited than
> you might at the moment. There are a few places that assume that the
> offsetnumber <= MaxHeapTuplesPerPage. See ginpostinglist.c, and
> MAX_TUPLES_PER_PAGE in tidbitmap.c. Also, offsetnumber can't be 0, because
> that makes the ItemPointer invalid, which is inconvenient if you tried to
> use ItemPointer as just an arbitrary 48-bit integer.

Good point.

Thanks for looking (and playing, in the other thread)!

Greetings,

Andres Freund

COLLATE: Hash partition vs UPDATE

[Jesper Pedersen]

Hi,

The following case

-- test.sql --
CREATE TABLE test (a text PRIMARY KEY, b text) PARTITION BY HASH (a);
CREATE TABLE test_p0 PARTITION OF test FOR VALUES WITH (MODULUS 2, 
REMAINDER 0);
CREATE TABLE test_p1 PARTITION OF test FOR VALUES WITH (MODULUS 2, 
REMAINDER 1);

-- CREATE INDEX idx_test_b ON test USING HASH (b);

INSERT INTO test VALUES ('', '');

-- Regression
UPDATE test SET b = '' WHERE a = '';
-- test.sql --

fails on master, which includes [1], with


psql:test.sql:9: ERROR:  could not determine which collation to use for 
string hashing

HINT:  Use the COLLATE clause to set the collation explicitly.


It passes on 11.x.

I'll add it to the open items list.

[1] 
https://git.postgresql.org/gitweb/?p=postgresql.git;a=commitdiff;h=5e1963fb764e9cc092e0f7b58b28985c311431d9


Best regards,
 Jesper

pgbench - implement strict TPC-B benchmark

[Fabien COELHO]

Hello devs,

The attached patch does $SUBJECT, as a showcase for recently added 
features, including advanced expressions (CASE...), \if, \gset, ending SQL 
commands at ";"...


There is also a small fix to the doc which describes the tpcb-like 
implementation but gets one variable name wrong: balance -> delta.


--
Fabien.diff --git a/doc/src/sgml/ref/pgbench.sgml b/doc/src/sgml/ref/pgbench.sgml
index ee2501be55..29f4168e76 100644
--- a/doc/src/sgml/ref/pgbench.sgml
+++ b/doc/src/sgml/ref/pgbench.sgml
@@ -347,7 +347,8 @@ pgbench  options  d
 An optional integer weight after @ allows to adjust the
 probability of drawing the script.  If not specified, it is set to 1.
 Available built-in scripts are: tpcb-like,
-simple-update and select-only.
+simple-update, select-only and
+strict-tpcb.
 Unambiguous prefixes of built-in names are accepted.
 With special name list, show the list of built-in scripts
 and exit immediately.
@@ -843,7 +844,7 @@ pgbench  options  d
   
The default built-in transaction script (also invoked with -b tpcb-like)
issues seven commands per transaction over randomly chosen aid,
-   tid, bid and balance.
+   tid, bid and delta.
The scenario is inspired by the TPC-B benchmark, but is not actually TPC-B,
hence the name.
   
@@ -869,6 +870,14 @@ pgbench  options  d
If you select the select-only built-in (also -S),
only the SELECT is issued.
   
+
+  
+   If you select the strict-tpcb built-in, the SQL commands are similar
+   to tpcb-like, but the delta amount is 6 digits,
+   the account branch has a 15% probability to be in the same branch as the teller (unless
+   there is only one branch in which case it is always the same), and the final account
+   balance is actually extracted by the script.
+  
  
 
  
diff --git a/src/bin/pgbench/pgbench.c b/src/bin/pgbench/pgbench.c
index 99529de52a..039dc32393 100644
--- a/src/bin/pgbench/pgbench.c
+++ b/src/bin/pgbench/pgbench.c
@@ -566,6 +566,47 @@ static const BuiltinScript builtin_script[] =
 		"",
 		"\\set aid random(1, " CppAsString2(naccounts) " * :scale)\n"
 		"SELECT abalance FROM pgbench_accounts WHERE aid = :aid;\n"
+	},
+	{
+		/*---
+		 * Standard conforming TPC-B benchmark script:
+		 *
+		 * The account branch has a 15% probability to be the same as the
+		 * teller branch.
+		 * The amount is in [-99, 99].
+		 * The final account balance is actually extracted by the driver.
+		 */
+		"strict-tpcb",
+		"",
+		/* choose teller and compute its branch */
+		"\\set tid random(1, " CppAsString2(ntellers) " * :scale)\n"
+		"\\set btid 1 + (:tid - 1) / (" CppAsString2(ntellers) " / " CppAsString2(nbranches) ")\n"
+		/* choose account branch: 85% different from teller unless there is only one branch */
+		"\\if :scale = 1 or random(1, 100) > 85\n"
+		"\\set bid :btid\n"
+		"\\else\n"
+		"\\set bid random(1, " CppAsString2(nbranches) " * :scale - 1)\n"
+		"\\set bid :bid + case when :bid >= :btid then 1 else 0 end\n"
+		"\\endif\n"
+		/* choose account within branch */
+		"\\set lid random(1, 10)\n"
+		"\\set aid (:bid - 1) * " CppAsString2(naccounts) "/" CppAsString2(nbranches) " + :lid\n"
+		"\\set delta random(-99, 99)\n"
+		/* it should probably be combined, but that breaks -M prepared */
+		"BEGIN;\n"
+		"INSERT INTO pgbench_history (tid, bid, aid, delta, mtime)\n"
+		"  VALUES (:tid, :bid, :aid, :delta, CURRENT_TIMESTAMP);\n"
+		"UPDATE pgbench_accounts\n"
+		"  SET abalance = abalance + :delta\n"
+		"  WHERE aid = :aid\n"
+		"  RETURNING abalance\\gset\n"
+		"UPDATE pgbench_tellers\n"
+		"  SET tbalance = tbalance + :delta\n"
+		"  WHERE tid = :tid;\n"
+		"UPDATE pgbench_branches\n"
+		"  SET bbalance = bbalance + :delta\n"
+		"  WHERE bid = :bid;\n"
+		"END;\n"
 	}
 };
 

pgbench - add option to show actual builtin script code

[Fabien COELHO]

Hello devs,

The minor attached patch $SUBJECT, so that it can be inspected easily, 
instead of having to look at the source code or whatever.


  sh> pgbench --list select-only
  -- select-only: 
  \set aid random(1, 10 * :scale)
  SELECT abalance FROM pgbench_accounts WHERE aid = :aid;

The builtin list output is also slightly improved:

  sh> pgbench -b list
  Available builtin scripts:
tpcb-like: 
simple-update: 
  select-only: 

--
Fabien.diff --git a/doc/src/sgml/ref/pgbench.sgml b/doc/src/sgml/ref/pgbench.sgml
index ee2501be55..4b7368b05e 100644
--- a/doc/src/sgml/ref/pgbench.sgml
+++ b/doc/src/sgml/ref/pgbench.sgml
@@ -355,7 +355,6 @@ pgbench  options  d
   
  
 
-
  
   -c clients
   --client=clients
@@ -456,6 +455,16 @@ pgbench  options  d

  
 
+ 
+  --listscriptname
+  
+   
+Show the actual code of builtin script scriptname
+on stderr, and exit immediately.
+   
+  
+ 
+
  
   -M querymode
   --protocol=querymode
diff --git a/src/bin/pgbench/pgbench.c b/src/bin/pgbench/pgbench.c
index 99529de52a..9f085508d9 100644
--- a/src/bin/pgbench/pgbench.c
+++ b/src/bin/pgbench/pgbench.c
@@ -651,6 +651,7 @@ usage(void)
 		   "  --progress-timestamp use Unix epoch timestamps for progress\n"
 		   "  --random-seed=SEED   set random seed (\"time\", \"rand\", integer)\n"
 		   "  --sampling-rate=NUM  fraction of transactions to log (e.g., 0.01 for 1%%)\n"
+		   "  --list=NAME  show builtin script code, then exit\n"
 		   "\nCommon options:\n"
 		   "  -d, --debug  print debugging output\n"
 		   "  -h, --host=HOSTNAME  database server host or socket directory\n"
@@ -4658,7 +4659,7 @@ listAvailableScripts(void)
 
 	fprintf(stderr, "Available builtin scripts:\n");
 	for (i = 0; i < lengthof(builtin_script); i++)
-		fprintf(stderr, "\t%s\n", builtin_script[i].name);
+		fprintf(stderr, "  %13s: %s\n", builtin_script[i].name, builtin_script[i].desc);
 	fprintf(stderr, "\n");
 }
 
@@ -5095,6 +5096,7 @@ main(int argc, char **argv)
 		{"log-prefix", required_argument, NULL, 7},
 		{"foreign-keys", no_argument, NULL, 8},
 		{"random-seed", required_argument, NULL, 9},
+		{"list", required_argument, NULL, 10},
 		{NULL, 0, NULL, 0}
 	};
 
@@ -5447,6 +5449,13 @@ main(int argc, char **argv)
 	exit(1);
 }
 break;
+			case 10:			/* list */
+{
+	const BuiltinScript   *s = findBuiltin(optarg);
+	fprintf(stderr, "-- %s: %s\n%s\n", s->name, s->desc, s->script);
+	exit(0);
+}
+break;
 			default:
 fprintf(stderr, _("Try \"%s --help\" for more information.\n"), progname);
 exit(1);
diff --git a/src/bin/pgbench/t/002_pgbench_no_server.pl b/src/bin/pgbench/t/002_pgbench_no_server.pl
index 69a6d03bb3..d9cf01233c 100644
--- a/src/bin/pgbench/t/002_pgbench_no_server.pl
+++ b/src/bin/pgbench/t/002_pgbench_no_server.pl
@@ -218,6 +218,15 @@ pgbench(
 	],
 	'pgbench builtin list');
 
+# builtin listing
+pgbench(
+	'--list se',
+	0,
+	[qr{^$}],
+	[ qr{select-only: }, qr{SELECT abalance FROM pgbench_accounts WHERE},
+	  qr{(?!UPDATE)}, qr{(?!INSERT)} ],
+	'pgbench builtin listing');
+
 my @script_tests = (
 
 	# name, err, { file => contents }

Re: Pluggable Storage - Andres's take

[Fabrízio de Royes Mello]

On Mon, Apr 8, 2019 at 9:34 AM Heikki Linnakangas  wrote:
>
> I wrote a little toy implementation that just returns constant data to
> play with this a little. Looks good overall.
>
> There were a bunch of typos in the comments in tableam.h, see attached.
> Some of the comments could use more copy-editing and clarification, I
> think, but I stuck to fixing just typos and such for now.
>
> index_update_stats() calls RelationGetNumberOfBlocks(). If the AM
> doesn't use normal data files, that won't work. I bumped into that with
> my toy implementation, which wouldn't need to create any data files, if
> it wasn't for this.
>
> The comments for relation_set_new_relfilenode() callback say that the AM
> can set *freezeXid and *minmulti to invalid. But when I did that, VACUUM
> hits this assertion:
>
> TRAP: FailedAssertion("!(((classForm->relfrozenxid) >= ((TransactionId)
> 3)))", File: "vacuum.c", Line: 1323)
>
> There's a little bug in index-only scan executor node, where it mixes up
> the slots to hold a tuple from the index, and from the table. That
> doesn't cause any ill effects if the AM uses TTSOpsHeapTuple, but with
> my toy AM, which uses a virtual slot, it caused warnings like this from
> index-only scans:
>
> WARNING:  problem in alloc set ExecutorState: detected write past chunk
> end in block 0x56419b0f88e8, chunk 0x56419b0f8f90
>
> Attached is a patch with the toy implementation I used to test this.
> I'm not suggesting we should commit that - although feel free to do that
> if you think it's useful - but it shows how I bumped into these issues.
> The second patch fixes the index-only-scan slot confusion (untested,
> except with my toy AM).
>

Awesome... it's built and ran tests cleanly, but I got assertion running
VACUUM:

fabrizio=# vacuum toytab ;
TRAP: FailedAssertion("!(((classForm->relfrozenxid) >= ((TransactionId)
3)))", File: "vacuum.c", Line: 1323)
psql: server closed the connection unexpectedly
This probably means the server terminated abnormally
before or while processing the request.
The connection to the server was lost. Attempting reset: 2019-04-08
12:29:16.204 -03 [20844] LOG:  server process (PID 24457) was terminated by
signal 6: Aborted
2019-04-08 12:29:16.204 -03 [20844] DETAIL:  Failed process was running:
vacuum toytab ;
2019-04-08 12:29:16.204 -03 [20844] LOG:  terminating any other active
server processes
2019-04-08 12:29:16.205 -03 [24458] WARNING:  terminating connection
because of crash of another server process

And backtrace is:

(gdb) bt
#0  0x7f813779f428 in __GI_raise (sig=sig@entry=6) at
../sysdeps/unix/sysv/linux/raise.c:54
#1  0x7f81377a102a in __GI_abort () at abort.c:89
#2  0x00ec0de9 in ExceptionalCondition (conditionName=0x10e3bb8
"!(((classForm->relfrozenxid) >= ((TransactionId) 3)))",
errorType=0x10e33f3 "FailedAssertion", fileName=0x10e345a "vacuum.c",
lineNumber=1323) at assert.c:54
#3  0x00893646 in vac_update_datfrozenxid () at vacuum.c:1323
#4  0x0089127a in vacuum (relations=0x26c4390,
params=0x7ffeb1a3fb30, bstrategy=0x26c4218, isTopLevel=true) at vacuum.c:452
#5  0x008906ae in ExecVacuum (pstate=0x26145b8, vacstmt=0x25f46f0,
isTopLevel=true) at vacuum.c:196
#6  0x00c3a883 in standard_ProcessUtility (pstmt=0x25f4a50,
queryString=0x25f3be8 "vacuum toytab ;", context=PROCESS_UTILITY_TOPLEVEL,
params=0x0, queryEnv=0x0, dest=0x25f4b48, completionTag=0x7ffeb1a3ffb0 "")
at utility.c:670
#7  0x00c3977a in ProcessUtility (pstmt=0x25f4a50,
queryString=0x25f3be8 "vacuum toytab ;", context=PROCESS_UTILITY_TOPLEVEL,
params=0x0, queryEnv=0x0, dest=0x25f4b48, completionTag=0x7ffeb1a3ffb0 "")
at utility.c:360
#8  0x00c3793e in PortalRunUtility (portal=0x265ba28,
pstmt=0x25f4a50, isTopLevel=true, setHoldSnapshot=false, dest=0x25f4b48,
completionTag=0x7ffeb1a3ffb0 "") at pquery.c:1175
#9  0x00c37d7f in PortalRunMulti (portal=0x265ba28,
isTopLevel=true, setHoldSnapshot=false, dest=0x25f4b48, altdest=0x25f4b48,
completionTag=0x7ffeb1a3ffb0 "") at pquery.c:1321
#10 0x00c36899 in PortalRun (portal=0x265ba28,
count=9223372036854775807, isTopLevel=true, run_once=true, dest=0x25f4b48,
altdest=0x25f4b48, completionTag=0x7ffeb1a3ffb0 "") at pquery.c:796
#11 0x00c2a40e in exec_simple_query (query_string=0x25f3be8 "vacuum
toytab ;") at postgres.c:1215
#12 0x00c332a3 in PostgresMain (argc=1, argv=0x261fe68,
dbname=0x261fca8 "fabrizio", username=0x261fc80 "fabrizio") at
postgres.c:4249
#13 0x00b051fc in BackendRun (port=0x2616d20) at postmaster.c:4429
#14 0x00b042c3 in BackendStartup (port=0x2616d20) at
postmaster.c:4120
#15 0x00afc70a in ServerLoop () at postmaster.c:1703
#16 0x00afb94e in PostmasterMain (argc=3, argv=0x25ed850) at
postmaster.c:1376
#17 0x00977de8 in main (argc=3, argv=0x25ed850) at main.c:228


Isn't better raise an exception as you did in other functions??

static void
toyam_relation_vacuum(Relation onerel,

Re: [PATCH] Implement uuid_version()

[Tom Lane]

Robert Haas  writes:
> My biggest gripe about uuid-ossp is that the name is stupid.  I wish
> we could see our way clear to renaming that extension to just 'uuid',
> because as J.L. says, virtually nobody's actually compiling against
> the OSSP library any more.

+1

There's no ALTER EXTENSION RENAME, and I suppose there can't be because
it would require editing/rewriting on-disk files that the server might
not even have write permissions for.  But your idea of an "update"
script that effectively moves everything over into a new extension
(that's physically installed but not present in current database)
might work.

Another way to approach it would be to have a script that belongs
to the new extension and what you do is
CREATE EXTENSION uuid FROM "uuid_ossp";
to perform the migration of the SQL objects.

Either way, we'd be looking at providing two .so's for some period
of time, but fortunately they're small.

regards, tom lane

Re: [PATCH] Implement uuid_version()

[Robert Haas]

On Sun, Apr 7, 2019 at 10:15 AM David Fetter  wrote:
> I see some.
>
> UUIDs turn out to be super useful in distributed systems to give good
> guarantees of uniqueness without coordinating with a particular node.
> Such systems have become a good bit more common since the most recent
> time this was discussed.

That's not really a compelling reason, though, because anybody who
needs UUIDs can always install the extension.  And on the other hand,
if we moved UUID support into core, then we'd be adding a hard compile
dependency on one of the UUID facilities, which might annoy some
developers.  We could possibly work around that by implementing our
own UUID facilities in core, but I'm not volunteering to do the work,
and I'm not sure that the work has enough benefit to justify the
labor.

My biggest gripe about uuid-ossp is that the name is stupid.  I wish
we could see our way clear to renaming that extension to just 'uuid',
because as J.L. says, virtually nobody's actually compiling against
the OSSP library any more.  The trick there is how to do that without
annoying exiting users.  Maybe we could leave behind an "upgrade"
script for the uuid-ossp extension that does CREATE EXTENSION uuid,
then alters all objects owned by the current extension to be owned by
the new extension, and maybe even drops itself.

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

Re: lazy_scan_heap() forgets to mark buffer dirty when setting all frozen?

[Robert Haas]

On Mon, Apr 8, 2019 at 12:55 AM Andres Freund  wrote:
> > Apparently the initial commit a892234f830e had MarkBufferDirty, but it
> > was removed one week later by 77a1d1e79892.
>
> Good catch. Kinda looks like it could have been an accidental removal?
> Robert?

So you're talking about this hunk?

-/* Page is marked all-visible but should be all-frozen */
-PageSetAllFrozen(page);
-MarkBufferDirty(buf);
-

I don't remember exactly, but I am pretty sure that I assumed from the
way that hunk looked that the MarkBufferDirty() was only needed there
because of the call to PageSetAllFrozen().  Perhaps I should've
figured it out from the "NB: If the heap page is all-visible..."
comment, but I unfortunately don't find that comment to be very clear
-- it basically says we don't need to do it, and then immediately
contradicts itself by saying we sometimes do need to do it "because it
may be logged."  But that's hardly an explanation, because why should
the fact that the page is going to be logged require that it be
dirtied?  We could improve the comment, but before we go there...

Why the heck does visibilitymap_set() require callers to do
MarkBufferDirty() instead of doing it itself?  Or at least, if it's
got to work that way, can't it Assert() something?  It seems crazy to
me that it calls PageSetLSN() without calling MarkBufferDirty() or
asserting that the buffer is dirty or having a header comment that
says that the buffer must be dirty. Ugh.

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

Re: [HACKERS] Weaker shmem interlock w/o postmaster.pid

[Noah Misch]

On Mon, Apr 08, 2019 at 08:07:28PM +1200, Thomas Munro wrote:
> On Mon, Apr 8, 2019 at 6:42 PM Noah Misch  wrote:
> > - lorikeet's FailedAssertion("!(vmq->mq_sender == ((void *)0))" looked
> >   suspicious, but this happened six other times in the past year[2], always 
> > on
> >   v10 lorikeet.
> 
> It happens on v11 too:
> 
> https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=lorikeet=2018-09-25%2010%3A06%3A31
> 
> The text changed slightly because we dropped an unnecessary extra
> pointer-to-volatile:
> 
> FailedAssertion("!(mq->mq_sender == ((void *)0))"
> 
> So either two workers started with the same parallel worker number, or
> something unexpectedly overwrote the shm_mq struct?

Ahh.  In that case, it's a duplicate of
https://postgr.es/m/flat/136712b0-0619-5619-4634-0f0286acaef7%402ndQuadrant.com

Re: Translation updates for zh_CN.po (Chinese Simplified)

[Tom Lane]

"Zhang, Jie"  writes:
> zh_CN.po has not been updated for three years.
> The source has changed a lot.
> I want to do something for postgresql.
> I think I can update the zh_CN.po file.

That would be great, but we don't use the pgsql-hackers mailing list
to coordinate translation work.  Please join pgsql-translators for that.

regards, tom lane

Re: hyrax vs. RelationBuildPartitionDesc

[Robert Haas]

On Mon, Apr 8, 2019 at 9:59 AM Tom Lane  wrote:
> Amit Langote  writes:
> > Should I add this patch to Older Bugs [1]?
>
> Yeah, it's an open issue IMO.  I think we've been focusing on getting
> as many feature patches done as we could during the CF, but now it's
> time to start mopping up problems like this one.

Do you have any further thoughts based on my last response?

Does anyone else wish to offer opinions?

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

Re: change password_encryption default to scram-sha-256?

[Jonathan S. Katz]

On 4/8/19 10:08 AM, Tom Lane wrote:
> "Jonathan S. Katz"  writes:
>> On 4/8/19 8:49 AM, Magnus Hagander wrote:
>>> I think the real question is, is it OK to give them basically 5months
>>> warning, by right now saying if you don't have a release out in 6
>>> months, things will break.
> 
>> Given the supported libraries all have open pull requests or issues, it
>> should be fairly easy to inquire if they would be able to support it for
>> PG12 vs PG13. If this sounds like a reasonable plan, I'm happy to reach
>> out and see.
> 
> I think that the right course here is to notify these developers that
> we will change the default in PG13, and it'd be good if they put out
> stable releases with SCRAM support well before that.

+1; I'm happy to reach out with that messaging, referencing this thread.

> This discussion
> seems to be talking as though it's okay if we allow zero daylight
> between availability of fixed drivers and release of a PG version that
> defaults to using SCRAM.  That'd be totally unfair to packagers and
> users.  There needs to be a pretty fair-size window for those fixed
> drivers to propagate into the wild.  A year is not too much; IMO it's
> barely enough.

I agree in principle, esp. related to testing + packaging (and I think
packaging would be my biggest concern), but IMV this primarily would
affect new applications, which is why I thought to provide reasoning for
a more aggressive timeline. You typically keep you pg.conf settings
consistent between version upgrades (with exceptions, e.g. based on
upgrade method). That could also inadvertently block people from
upgrading, too, but the bigger risk would be new application development
on PG12.

Looking at the uncovered user base too, it's not the largest portion of
our users, though accessing PostgreSQL via Go is certainly increasingly
rapidly so I'm very sympathetic that we don't break their accessibility
(and I've personally used asyncpg and would not want my apps to break
either :).

Anyway, I primarily wanted to see if an aggressive timeline to update
our default password approach would make sense esp. given we've had this
feature around for some time, and, again, it is far superior to the
other password based methods. I'm fine with being cautious, just wanted
to ensure we're not being too cautious about getting our users to
utilize a feature with better security.

Jonathan



signature.asc
Description: OpenPGP digital signature

Re: clean up docs for v12

[Justin Pryzby]

Find attached updated patches for v12 docs.

Note that Alvaro applied an early patch for log_statement_sample_rate, but
unfortunately I hadn't sent a v2 patch with additional change from myon, so
there's one remaining hunk included here.

If needed I can split up differently for review, or resend a couple on separate
threads, or resend inline.

Patches are currently optimized for review, but maybe should be squished into
one and/or reindented before merging.

Justin
>From da9c81526918c49c3c30fdb6838323d2f66cba69 Mon Sep 17 00:00:00 2001
From: Justin Pryzby 
Date: Thu, 28 Mar 2019 08:53:26 -0500
Subject: [PATCH v2 01/12] Clean up docs for log_statement_sample_rate..

..which was added at commit 88bdbd3f746049834ae3cc972e6e650586ec3c9d

And this one little bit was missed here:
https://www.postgresql.org/message-id/20190403215938.GA26375%40alvherre.pgsql

Author: Christoph Berg 
---
 doc/src/sgml/config.sgml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 804406a..ad5a620 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -5887,7 +5887,7 @@ local0.*/var/log/postgresql
 
  Determines the fraction of statements that exceed
   to be logged.
- The default is 1, meaning log all such
+ The default is 1.0, meaning log all such
  statements.
  Setting this to zero disables logging by duration, same as setting
  log_min_duration_statement to
-- 
2.1.4

>From 4c78b873d7db73ddc1e2f7aa94b6a2e816746d3b Mon Sep 17 00:00:00 2001
From: Justin Pryzby 
Date: Thu, 28 Mar 2019 18:50:03 -0500
Subject: [PATCH v2 02/12] review docs for pg12dev

---
 doc/src/sgml/bloom.sgml|  2 +-
 doc/src/sgml/catalogs.sgml |  4 ++--
 doc/src/sgml/config.sgml   | 13 ++-
 doc/src/sgml/ecpg.sgml | 18 +++---
 doc/src/sgml/libpq.sgml|  2 +-
 doc/src/sgml/monitoring.sgml   | 34 +--
 doc/src/sgml/perform.sgml  |  8 +++
 doc/src/sgml/ref/alter_table.sgml  |  4 ++--
 doc/src/sgml/ref/create_index.sgml |  4 ++--
 doc/src/sgml/ref/pg_rewind.sgml| 17 +++---
 doc/src/sgml/ref/pgbench.sgml  |  6 ++---
 doc/src/sgml/ref/reindex.sgml  |  2 +-
 doc/src/sgml/runtime.sgml  |  7 +++---
 doc/src/sgml/sources.sgml  | 48 +++---
 doc/src/sgml/xfunc.sgml| 12 +-
 src/bin/pg_upgrade/check.c |  6 ++---
 16 files changed, 92 insertions(+), 95 deletions(-)

diff --git a/doc/src/sgml/bloom.sgml b/doc/src/sgml/bloom.sgml
index 6eeadde..c341b65 100644
--- a/doc/src/sgml/bloom.sgml
+++ b/doc/src/sgml/bloom.sgml
@@ -65,7 +65,7 @@
  
   Number of bits generated for each index column. Each parameter's name
   refers to the number of the index column that it controls.  The default
-  is 2 bits and maximum is 4095.  Parameters for
+  is 2 bits and the maximum is 4095.  Parameters for
   index columns not actually used are ignored.
  
 
diff --git a/doc/src/sgml/catalogs.sgml b/doc/src/sgml/catalogs.sgml
index 1701863..40ddec4 100644
--- a/doc/src/sgml/catalogs.sgml
+++ b/doc/src/sgml/catalogs.sgml
@@ -3052,7 +3052,7 @@ SCRAM-SHA-256$iteration count:
simplifies ATTACH/DETACH PARTITION operations:
the partition dependencies need only be added or removed.
Example: a child partitioned index is made partition-dependent
-   on both the partition table it is on and the parent partitioned
+   on both the table partition and the parent partitioned
index, so that it goes away if either of those is dropped, but
not otherwise.  The dependency on the parent index is primary,
so that if the user tries to drop the child partitioned index,
@@ -3115,7 +3115,7 @@ SCRAM-SHA-256$iteration count:
Note that it's quite possible for two objects to be linked by more than
one pg_depend entry.  For example, a child
partitioned index would have both a partition-type dependency on its
-   associated partition table, and an auto dependency on each column of
+   associated table partition, and an auto dependency on each column of
that table that it indexes.  This sort of situation expresses the union
of multiple dependency semantics.  A dependent object can be dropped
without CASCADE if any of its dependencies satisfies
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index ad5a620..e45a321 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -112,7 +112,7 @@
For example, 30.1 GB will be converted
to 30822 MB not 32319628902 B.
If the parameter is of integer type, a final rounding to integer
-   occurs after any units conversion.
+   occurs after any unit conversion.
   
  
 
@@ -3433,7 +3433,7 @@ restore_command = 'copy "C:\\server\\archivedir\\%f" "%p"'  # Windows
  

Re: change password_encryption default to scram-sha-256?

[Tom Lane]

"Jonathan S. Katz"  writes:
> On 4/8/19 8:49 AM, Magnus Hagander wrote:
>> I think the real question is, is it OK to give them basically 5months
>> warning, by right now saying if you don't have a release out in 6
>> months, things will break.

> Given the supported libraries all have open pull requests or issues, it
> should be fairly easy to inquire if they would be able to support it for
> PG12 vs PG13. If this sounds like a reasonable plan, I'm happy to reach
> out and see.

I think that the right course here is to notify these developers that
we will change the default in PG13, and it'd be good if they put out
stable releases with SCRAM support well before that.  This discussion
seems to be talking as though it's okay if we allow zero daylight
between availability of fixed drivers and release of a PG version that
defaults to using SCRAM.  That'd be totally unfair to packagers and
users.  There needs to be a pretty fair-size window for those fixed
drivers to propagate into the wild.  A year is not too much; IMO it's
barely enough.

regards, tom lane

Re: hyrax vs. RelationBuildPartitionDesc

[Tom Lane]

Amit Langote  writes:
> Should I add this patch to Older Bugs [1]?

Yeah, it's an open issue IMO.  I think we've been focusing on getting
as many feature patches done as we could during the CF, but now it's
time to start mopping up problems like this one.

regards, tom lane

Re: change password_encryption default to scram-sha-256?

[Jonathan S. Katz]

On 4/8/19 8:49 AM, Magnus Hagander wrote:
> On Mon, Apr 8, 2019 at 2:38 PM Jonathan S. Katz  > wrote:

> Counter-argument: SCRAM has been available for 2 years since 10 feature
> freeze, there has been a lot of time already given to implement support
> for it. Given is at least 5 months until PG12 comes out, and each of the
> popular drivers already has patches in place, we could default it for 12
> and let them know this is a reality.
> 
> 
> You can't really count feature freeze, you have to count release I
> think. And basically we're saying they had 2 years. Which in itself
> would've been perfectly reasonable, *if we told them*. But we didn't.
> 
> I think the real question is, is it OK to give them basically 5months
> warning, by right now saying if you don't have a release out in 6
> months, things will break.

Yeah, that's a good and fair question.

> That said, that would be an aggressive approach, so I would not object
> to changing the default for PG13 and giving 17 months vs. 5, but we do
> let md5 persist that much longer.
> 
> 
> I think we definitely should not make it *later* than 13.

+1

> Maybe we should simply reach out to those driver developers, it's not
> that many of them after all, and *ask* if they would think it's a
> problem if we change it in 12.

It wouldn't hurt. I went through the list again[1] to see which ones
don't have it and updated:

- pgsql (Erlang) - this webpage doesn't load, maybe we should remove? It
may have been replaced by this one[2]?

- erlang-pgsql-driver (Erlang) - on the page it says it's unsupported,
so we should definitely remove it from the wiki and from consideration

- node-postgres (JavaScript) - they added SCRAM in 7.9.0 so I've updated
the wiki

- pq (Go) - No; as mentioned there are 3 separate patches in consideration

- crystal-pg (Ruby) No; open issue, not patch

- asyncpg (Python) No; open issue, suggestion on how to implement but no
patch

Let me also add:

- pgx (Go)[3] - another popular Go driver, there is an open patch for
SCRAM support

So IMV it's pq, crystal-pg, asyncpg, & pgx we have to reach out to,
pending resolution on Erlang libs.

Given the supported libraries all have open pull requests or issues, it
should be fairly easy to inquire if they would be able to support it for
PG12 vs PG13. If this sounds like a reasonable plan, I'm happy to reach
out and see.

Jonathan

[1] https://wiki.postgresql.org/wiki/List_of_drivers
[2] https://github.com/semiocast/pgsql
[3] https://github.com/jackc/pgx



signature.asc
Description: OpenPGP digital signature

Re: change password_encryption default to scram-sha-256?

[Magnus Hagander]

On Mon, Apr 8, 2019 at 2:38 PM Jonathan S. Katz 
wrote:

> On 4/8/19 8:19 AM, Peter Eisentraut wrote:
> > On 2019-04-08 13:52, Andrew Dunstan wrote:
> >> Yeah, if we're not going to do it now we should announce that we will
> >> do it in the next release.
> >
> > Targeting PG13 seems reasonable.
>

Yeah, that would be fairly consistent with how we usually do htings

Counter-argument: SCRAM has been available for 2 years since 10 feature
> freeze, there has been a lot of time already given to implement support
> for it. Given is at least 5 months until PG12 comes out, and each of the
> popular drivers already has patches in place, we could default it for 12
> and let them know this is a reality.
>

You can't really count feature freeze, you have to count release I think.
And basically we're saying they had 2 years. Which in itself would've been
perfectly reasonable, *if we told them*. But we didn't.

I think the real question is, is it OK to give them basically 5months
warning, by right now saying if you don't have a release out in 6 months,
things will break.



Given it's superior to the existing methods, it'd be better to encourage
> the drivers to get this in place sooner. Given what I know about md5,
> I've tried to avoid building apps with drivers that don't support SCRAM.
>
> That said, that would be an aggressive approach, so I would not object
> to changing the default for PG13 and giving 17 months vs. 5, but we do
> let md5 persist that much longer.
>

I think we definitely should not make it *later* than 13.

Maybe we should simply reach out to those driver developers, it's not that
many of them after all, and *ask* if they would think it's a problem if we
change it in 12.

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

Re: change password_encryption default to scram-sha-256?

[Jonathan S. Katz]

On 4/8/19 8:19 AM, Peter Eisentraut wrote:
> On 2019-04-08 13:52, Andrew Dunstan wrote:
>> Yeah, if we're not going to do it now we should announce that we will
>> do it in the next release.
> 
> Targeting PG13 seems reasonable.

Counter-argument: SCRAM has been available for 2 years since 10 feature
freeze, there has been a lot of time already given to implement support
for it. Given is at least 5 months until PG12 comes out, and each of the
popular drivers already has patches in place, we could default it for 12
and let them know this is a reality.

Given it's superior to the existing methods, it'd be better to encourage
the drivers to get this in place sooner. Given what I know about md5,
I've tried to avoid building apps with drivers that don't support SCRAM.

That said, that would be an aggressive approach, so I would not object
to changing the default for PG13 and giving 17 months vs. 5, but we do
let md5 persist that much longer.

Jonathan



signature.asc
Description: OpenPGP digital signature

Re: Pluggable Storage - Andres's take

[Heikki Linnakangas]

I wrote a little toy implementation that just returns constant data to 
play with this a little. Looks good overall.


There were a bunch of typos in the comments in tableam.h, see attached. 
Some of the comments could use more copy-editing and clarification, I 
think, but I stuck to fixing just typos and such for now.


index_update_stats() calls RelationGetNumberOfBlocks(). If the AM 
doesn't use normal data files, that won't work. I bumped into that with 
my toy implementation, which wouldn't need to create any data files, if 
it wasn't for this.


The comments for relation_set_new_relfilenode() callback say that the AM 
can set *freezeXid and *minmulti to invalid. But when I did that, VACUUM 
hits this assertion:


TRAP: FailedAssertion("!(((classForm->relfrozenxid) >= ((TransactionId) 
3)))", File: "vacuum.c", Line: 1323)


There's a little bug in index-only scan executor node, where it mixes up 
the slots to hold a tuple from the index, and from the table. That 
doesn't cause any ill effects if the AM uses TTSOpsHeapTuple, but with 
my toy AM, which uses a virtual slot, it caused warnings like this from 
index-only scans:


WARNING:  problem in alloc set ExecutorState: detected write past chunk 
end in block 0x56419b0f88e8, chunk 0x56419b0f8f90


Attached is a patch with the toy implementation I used to test this. 
I'm not suggesting we should commit that - although feel free to do that 
if you think it's useful - but it shows how I bumped into these issues. 
The second patch fixes the index-only-scan slot confusion (untested, 
except with my toy AM).


- Heikki
>From 97e0eea6a3fb123845ac5650f1aaa1802bf56694 Mon Sep 17 00:00:00 2001
From: Heikki Linnakangas 
Date: Mon, 8 Apr 2019 15:16:53 +0300
Subject: [PATCH 1/3] Add a toy table AM implementation to play with.

It returns a constant data set. No insert/update/delete. But you can
create indexes.
---
 src/test/modules/toytable/Makefile|  25 +
 .../modules/toytable/expected/toytable.out|  41 ++
 src/test/modules/toytable/sql/toytable.sql|  17 +
 src/test/modules/toytable/toytable--1.0.sql   |  12 +
 src/test/modules/toytable/toytable.control|   4 +
 src/test/modules/toytable/toytableam.c| 612 ++
 6 files changed, 711 insertions(+)
 create mode 100644 src/test/modules/toytable/Makefile
 create mode 100644 src/test/modules/toytable/expected/toytable.out
 create mode 100644 src/test/modules/toytable/sql/toytable.sql
 create mode 100644 src/test/modules/toytable/toytable--1.0.sql
 create mode 100644 src/test/modules/toytable/toytable.control
 create mode 100644 src/test/modules/toytable/toytableam.c

diff --git a/src/test/modules/toytable/Makefile b/src/test/modules/toytable/Makefile
new file mode 100644
index 000..142ef2d23e6
--- /dev/null
+++ b/src/test/modules/toytable/Makefile
@@ -0,0 +1,25 @@
+# src/test/modules/toytable/Makefile
+
+MODULE_big = toytable
+OBJS = toytableam.o $(WIN32RES)
+PGFILEDESC = "A dummy implementantation of the table AM API"
+
+EXTENSION = toytable
+DATA = toytable--1.0.sql
+
+REGRESS = toytable
+
+ifdef USE_PGXS
+PG_CONFIG = pg_config
+PGXS := $(shell $(PG_CONFIG) --pgxs)
+include $(PGXS)
+else
+subdir = src/test/modules/toytable
+top_builddir = ../../../..
+include $(top_builddir)/src/Makefile.global
+include $(top_srcdir)/contrib/contrib-global.mk
+endif
+
+OBJS = toytableam.o
+
+include $(top_srcdir)/src/backend/common.mk
diff --git a/src/test/modules/toytable/expected/toytable.out b/src/test/modules/toytable/expected/toytable.out
new file mode 100644
index 000..3e8598e284c
--- /dev/null
+++ b/src/test/modules/toytable/expected/toytable.out
@@ -0,0 +1,41 @@
+CREATE EXTENSION toytable;
+create table toytab (i int4, j int4, k int4) using toytable;
+select * from toytab;
+ i  | j  | k  
+++
+  1 |  1 |  1
+  2 |  2 |  2
+  3 |  3 |  3
+  4 |  4 |  4
+  5 |  5 |  5
+  6 |  6 |  6
+  7 |  7 |  7
+  8 |  8 |  8
+  9 |  9 |  9
+ 10 | 10 | 10
+(10 rows)
+
+create index toyidx on toytab(i);
+-- test index scan
+set enable_seqscan=off;
+set enable_indexscan=on;
+select i, j from toytab where i = 4;
+ i | j 
+---+---
+ 4 | 4
+(1 row)
+
+-- index only scan
+explain (costs off) select i from toytab where i = 4;
+   QUERY PLAN   
+
+ Index Only Scan using toyidx on toytab
+   Index Cond: (i = 4)
+(2 rows)
+
+select i from toytab where i = 4 ;
+ i 
+---
+ 4
+(1 row)
+
diff --git a/src/test/modules/toytable/sql/toytable.sql b/src/test/modules/toytable/sql/toytable.sql
new file mode 100644
index 000..8d9bac41bbf
--- /dev/null
+++ b/src/test/modules/toytable/sql/toytable.sql
@@ -0,0 +1,17 @@
+CREATE EXTENSION toytable;
+
+create table toytab (i int4, j int4, k int4) using toytable;
+
+select * from toytab;
+
+create index toyidx on toytab(i);
+
+-- test index scan
+set enable_seqscan=off;
+set enable_indexscan=on;
+
+select i, j from toytab where i = 4;
+
+-- index only scan
+explain (costs off) 

Re: change password_encryption default to scram-sha-256?

[Peter Eisentraut]

On 2019-04-08 13:52, Andrew Dunstan wrote:
> Yeah, if we're not going to do it now we should announce that we will
> do it in the next release.

Targeting PG13 seems reasonable.

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

Re: Speed up transaction completion faster after many relations are accessed in a transaction

[Peter Eisentraut]

On 2019-04-08 05:46, Tsunakawa, Takayuki wrote:
> I'm registering you as another author and me as a reviewer, and marking this 
> ready for committer.

Moved to next commit fest.

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

Re: [PATCH v20] GSSAPI encryption support

[Peter Eisentraut]

On 2019-04-05 23:37, Stephen Frost wrote:
> I wonder if somehow the keytab file that the server is using isn't
> getting destroyed between the two test runs and so you're ending up with
> the server using the key from the old KDC, while the user is using the
> new one..?  Or something is equally going wrong in the tests.
> 
> Could you try doing something like removing the 001_auth.pl, moving the
> 002_enc.pl to 001_enc.pl, double-check that everything is cleaned up and
> that there aren't any old PG servers running, et al, and re-try that
> way?

Running just 002_enc.pl by itself passes the tests!

> I've also reached out to some colleagues about having one of them test
> with MacOS.  What version are you on..? 

macOS 10.14.14 it says.

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

Re: Status of the table access method work

[Heikki Linnakangas]

On 05/04/2019 23:25, Andres Freund wrote:

I think what's in v12 - I don't know of any non-cleanup / bugfix work
pending for 12 - is a pretty reasonable initial set of features.


Hooray!


- the (optional) bitmap heap scan API - that's fairly intrinsically
   block based. An AM could just internally subdivide TIDs in a different
   way, but I don't think a bitmap scan like we have would e.g. make a
   lot of sense for an index oriented table without any sort of stable
   tid.


If an AM doesn't implement the bitmap heap scan API, what happens? 
Bitmap scans are disabled?


Even if an AM isn't block-oriented, the bitmap heap scan API still makes 
sense as long as there's some correlation between TIDs and physical 
location. The only really broken thing about that currently is the 
prefetching: nodeBitmapHeapScan.c calls PrefetchBuffer() directly with 
the TID's block numbers. It would be pretty straightforward to wrap that 
in a callback, so that the AM could do something different.


Or move even more of the logic to the AM, so that the AM would get the 
whole TIDBitmap in table_beginscan_bm(). It could then implement the 
fetching and prefetching as it sees fit.


I don't think it's urgent, though. We can cross that bridge when we get 
there, with the first AM that needs that flexibility.



The most constraining factor for storage, I think, is that currently the
API relies on ItemPointerData style TIDs in a number of places (i.e. a 6
byte tuple identifier).


I think 48 bits would be just about enough, but it's even more limited 
than you might at the moment. There are a few places that assume that 
the offsetnumber <= MaxHeapTuplesPerPage. See ginpostinglist.c, and 
MAX_TUPLES_PER_PAGE in tidbitmap.c. Also, offsetnumber can't be 0, 
because that makes the ItemPointer invalid, which is inconvenient if you 
tried to use ItemPointer as just an arbitrary 48-bit integer.


- Heikki

Re: reloption to prevent VACUUM from truncating empty pages at the end of relation

[Julien Rouhaud]

On Mon, Apr 8, 2019 at 10:24 AM Fujii Masao  wrote:
>
> On Mon, Apr 8, 2019 at 5:20 PM Julien Rouhaud  wrote:
> >
> > On Mon, Apr 8, 2019 at 10:15 AM Fujii Masao  wrote:
> > >
> > > But it has not been actually pushed into the community's git
> > > repository yet.That's maybe because it's been a while since
> > > my last commit and my commit bit is temporarily limited?
> > > Anyway the patch has been pushed before April 8th 0:00 in AoE.
> >
> > Indeed, there's no mail yet, but I can see the commit at
> > https://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=119dcfad988d5b5d9f52b256087869997670aa36
>
> Thanks for the info, so I marked the patch as committed.

FTR I just received the notification email, and it's also up at
https://www.postgresql.org/message-id/E1hDOzB-pO-ED%40gemulon.postgresql.org!

Re: change password_encryption default to scram-sha-256?

[Andrew Dunstan]

On Mon, Apr 8, 2019 at 2:38 AM Michael Paquier  wrote:
>
> On Mon, Apr 08, 2019 at 09:08:05AM +0300, Heikki Linnakangas wrote:
> > I wouldn't hold my breath. That's the third PR to add SCRAM support already,
> > see also https://github.com/lib/pq/pull/788 and
> > https://github.com/lib/pq/pull/608. The project seems to lack the committer
> > manpower or round tuits to review and push these.
>
> I am wondering on the contrary if switching the default on Postgres
> side would make things move faster on their side though.


Yeah, if we're not going to do it now we should announce that we will
do it in the next release.

cheers

andrew


-- 
Andrew Dunstanhttps://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Re: [HACKERS] generated columns

[Peter Eisentraut]

On 2019-04-04 16:37, Amit Langote wrote:
> OK,  thanks for explaining.  We do allow DEFAULT to be specified on
> foreign tables, although locally-defined defaults have little meaning
> if the FDW doesn't allow inserts.  Maybe same thing applies to
> GENERATED AS columns.
> 
> Would it make sense to clarify this on CREATE FOREIGN TABLE page?

done

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

Translation updates for zh_CN.po (Chinese Simplified)

[Zhang, Jie]

Hi all

zh_CN.po has not been updated for three years.
The source has changed a lot.
I want to do something for postgresql.
I think I can update the zh_CN.po file.

I plan to translate and update the following zh_CN.po
postgresql/src/bin/initdb/po/zh_CN.po  [Patch has been completed]   
postgresql/src/bin/pg_archivecleanup/po/zh_CN.po
postgresql/src/bin/pg_basebackup/po/zh_CN.po
postgresql/src/bin/pg_checksums/po/zh_CN.po
postgresql/src/bin/pg_config/po/zh_CN.po
postgresql/src/bin/pg_controldata/po/zh_CN.po
postgresql/src/bin/pg_ctl/po/zh_CN.po
postgresql/src/bin/pg_dump/po/zh_CN.po
postgresql/src/bin/pg_resetwal/po/zh_CN.po
postgresql/src/bin/pg_rewind/po/zh_CN.po
postgresql/src/bin/pg_test_fsync/po/zh_CN.po
postgresql/src/bin/pg_test_timing/po/zh_CN.po
postgresql/src/bin/pg_upgrade/po/zh_CN.po
postgresql/src/bin/pg_waldump/po/zh_CN.po
postgresql/src/bin/psql/zh_CN.po
postgresql/src/bin/scripts/po/zh_CN.po
postgresql/src/backend/po/zh_CN.po
postgresql/src/pl/plpgsql/src/po/zh_CN.po
postgresql/src/interfaces/ecpg/preproc/po/zh_CN.po
postgresql/src/interfaces/ecpg/ecpglib/po/zh_CN.po
postgresql/src/interfaces/libpq/po/zh_CN.po

Here is a patch for postgresql/src/bin/initdb/po/zh_CN.po






initdb_po.patch
Description: initdb_po.patch

Re: Problem with default partition pruning

[Kyotaro HORIGUCHI]

At Mon, 8 Apr 2019 16:57:35 +0900, "Yuzuko Hosoya" 
 wrote in 
<00c101d4ede0$babd4390$3037cab0$@lab.ntt.co.jp>
> > BTW, now I'm a bit puzzled between whether this case should be fixed by 
> > hacking on partprune.c like
> > this patch does or whether to work on getting the other patch committed and 
> > expect users to set
> > constraint_exclusion = on for this to behave as expected.  The original 
> > intention of setting
> > partition_qual in set_relation_partition_info() was for partprune.c to use 
> > it to remove useless
> > arguments of OR clauses which otherwise would cause the failure to 
> > correctly prune the default partitions
> > of sub-partitioned tables.  As shown by the examples in this thread, the 
> > original effort was
> > insufficient, which this patch aims to improve.  But, it also expands the 
> > scope of partprune.c's usage
> > of partition_qual, which is to effectively perform full-blown constraint 
> > exclusion without being
> > controllable by constraint_exclusion GUC, which may be seen as being good 
> > or bad.  The fact that it
> > helps in getting partition pruning working correctly in more obscure cases 
> > like those discussed in
> > this thread means it's good maybe.
> > 
> Umm, even though this modification might be overhead, I think this problem 
> should be solved
> without setting constraint_exclusion GUC. But I'm not sure.

Partition pruning and constraint exclusion are orthogonal
functions. Note that the default value of the variable is
CONSTRAINT_EXCLUSION_PARTITION and the behavior is not a perfect
bug.  So I think we can reasonably ignore constraints when
constraint_exclusion is intentionally turned off.

As the result I propose to move the "if(partconstr)" block in the
latest patches after the constant-false block, changing the
condition as "if (partconstr && constraint_exclusion !=
CONSTRAINT_EXCLUSION_OFF)".

This make partprune reacts to constraint_exclusion the consistent
way with the old-fashioned partitioning.

regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center

Re: reloption to prevent VACUUM from truncating empty pages at the end of relation

[Julien Rouhaud]

On Mon, Apr 8, 2019 at 12:22 PM Fujii Masao  wrote:
>
> On Mon, Apr 8, 2019 at 5:30 PM Masahiko Sawada  wrote:
> >
> > On Mon, Apr 8, 2019 at 5:15 PM Fujii Masao  wrote:
> > >
> > > On Mon, Apr 8, 2019 at 3:58 PM Julien Rouhaud  wrote:
> > > >
> > > > On Mon, Apr 8, 2019 at 8:01 AM Fujii Masao  
> > > > wrote:
> > > > >
> > > > > 2019年4月8日(月) 14:22 Tsunakawa, Takayuki 
> > > > > :
> > > > >>
> > > > >> From: Alvaro Herrera [mailto:alvhe...@2ndquadrant.com]
> > > > >> > "vacuum_truncate" gets my vote too.
> > > > >>
> > > > >> +1
> > > > >
> > > > >
> > > > > +1
> > > > > ISTM that we have small consensus to
> > > > > use "vacuum_truncate".
> > > >
> > > > I'm also fine with this name, and I also saw reports that this option
> > > > is already needed in some production workload, as Andres explained.
> > >
> > > OK, so I pushed the "vacuum_truncate" version of the patch.
> >
> > Thank you!
> >
> > "TRUNCATE" option for vacuum command should be added to the open items?
>
> Yes, I think.
> Attached is the patch which adds TRUNCATE option into VACUUM.

Thanks.

I just reviewed the patch, it works as expected, no special comment on the code.

Minor nitpicking:

-  lock on the table.
+  lock on the table. The TRUNCATE parameter
+  to , if specified, overrides the value
+  of this option.

maybe "parameter of" instead of "parameter to"?

Re: reloption to prevent VACUUM from truncating empty pages at the end of relation

[Masahiko Sawada]

On Mon, Apr 8, 2019 at 7:22 PM Fujii Masao  wrote:
>
> On Mon, Apr 8, 2019 at 5:30 PM Masahiko Sawada  wrote:
> >
> > On Mon, Apr 8, 2019 at 5:15 PM Fujii Masao  wrote:
> > >
> > > On Mon, Apr 8, 2019 at 3:58 PM Julien Rouhaud  wrote:
> > > >
> > > > On Mon, Apr 8, 2019 at 8:01 AM Fujii Masao  
> > > > wrote:
> > > > >
> > > > > 2019年4月8日(月) 14:22 Tsunakawa, Takayuki 
> > > > > :
> > > > >>
> > > > >> From: Alvaro Herrera [mailto:alvhe...@2ndquadrant.com]
> > > > >> > "vacuum_truncate" gets my vote too.
> > > > >>
> > > > >> +1
> > > > >
> > > > >
> > > > > +1
> > > > > ISTM that we have small consensus to
> > > > > use "vacuum_truncate".
> > > >
> > > > I'm also fine with this name, and I also saw reports that this option
> > > > is already needed in some production workload, as Andres explained.
> > >
> > > OK, so I pushed the "vacuum_truncate" version of the patch.
> >
> > Thank you!
> >
> > "TRUNCATE" option for vacuum command should be added to the open items?
>
> Yes, I think.

Added.

> Attached is the patch which adds TRUNCATE option into VACUUM.

Thank you for the patch! I will review it.

Regards,

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

Re: [HACKERS] Block level parallel vacuum

[Kyotaro HORIGUCHI]

Hello.

# Is this still living? I changed the status to "needs review"

At Sat, 6 Apr 2019 06:47:32 +0900, Masahiko Sawada  
wrote in 
> > Indeed. How about the following description?
> >
> 
> Attached the updated version patches.

Thanks.

heapam.h is including access/parallel.h but the file doesn't use
parallel.h stuff and storage/shm_toc.h and storage/dsm.h are
enough.

+ * DSM keys for parallel lazy vacuum. Since we don't need to worry about DSM
+ * keys conflicting with plan_node_id we can use small integers.

Yeah, this is right, but "plan_node_id" seems abrupt
there. Please prepend "differently from parallel execution code"
or .. I think no excuse is needed to use that numbers. The
executor code is already making an excuse for the large numbers
as unusual instead.

+ * Macro to check if we in a parallel lazy vacuum. If true, we're in parallel
+ * mode and prepared the DSM segments.
+ */
+#define IsInParallelVacuum(lps) (((LVParallelState *) (lps)) != NULL)

we *are* in?

The name "IsInParallleVacuum()" looks (to me) like suggesting
"this process is a parallel vacuum worker".  How about
ParallelVacuumIsActive?


+typedef struct LVIndStats
+typedef struct LVDeadTuples
+typedef struct LVShared
+typedef struct LVParallelState

The names are confusing, and the name LVShared is too
generic. Shared-only structs are better to be marked in the name.
That is, maybe it would be better that LVIndStats were
LVSharedIndStats and LVShared were LVSharedRelStats.

It might be better that LVIndStats were moved out from LVShared,
but I'm not confident.

+static void
+lazy_parallel_vacuum_or_cleanup_indexes(LVRelStats *vacrelstats, Relation *Irel
...
+   lazy_begin_parallel_index_vacuum(lps, vacrelstats, for_cleanup);
...
+   do_parallel_vacuum_or_cleanup_indexes(Irel, nindexes, stats,
+  lps->lvshared, vacrelstats->dead_tuples);
...
+   lazy_end_parallel_index_vacuum(lps, !for_cleanup);

The function takes the parameter for_cleanup, but the flag is
used by the three subfunctions in utterly ununified way. It seems
to me useless to store for_cleanup in lvshared and lazy_end is
rather confusing. There's no explanation why "reinitialization"
== "!for_cleanup". In the first place,
lazy_begin_parallel_index_vacuum and
lazy_end_parallel_index_vacuum are called only from the function
and rather short so it doesn't seem reasonable that the are
independend functions.

regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center

Re: reloption to prevent VACUUM from truncating empty pages at the end of relation

[Fujii Masao]

On Mon, Apr 8, 2019 at 5:30 PM Masahiko Sawada  wrote:
>
> On Mon, Apr 8, 2019 at 5:15 PM Fujii Masao  wrote:
> >
> > On Mon, Apr 8, 2019 at 3:58 PM Julien Rouhaud  wrote:
> > >
> > > On Mon, Apr 8, 2019 at 8:01 AM Fujii Masao  wrote:
> > > >
> > > > 2019年4月8日(月) 14:22 Tsunakawa, Takayuki :
> > > >>
> > > >> From: Alvaro Herrera [mailto:alvhe...@2ndquadrant.com]
> > > >> > "vacuum_truncate" gets my vote too.
> > > >>
> > > >> +1
> > > >
> > > >
> > > > +1
> > > > ISTM that we have small consensus to
> > > > use "vacuum_truncate".
> > >
> > > I'm also fine with this name, and I also saw reports that this option
> > > is already needed in some production workload, as Andres explained.
> >
> > OK, so I pushed the "vacuum_truncate" version of the patch.
>
> Thank you!
>
> "TRUNCATE" option for vacuum command should be added to the open items?

Yes, I think.
Attached is the patch which adds TRUNCATE option into VACUUM.

Regards,

-- 
Fujii Masao


vacuum_truncate_v1.patch
Description: Binary data

Re: hyrax vs. RelationBuildPartitionDesc

[Amit Langote]

On 2019/03/14 10:40, Amit Langote wrote:
> On 2019/03/14 5:18, Tom Lane wrote:
>> Robert Haas  writes:
>>> On Wed, Mar 13, 2019 at 3:14 PM Tom Lane  wrote:
 Meanwhile, who's going to take point on cleaning up rd_partcheck?
 I don't really understand this code well enough to know whether that
 can share one of the existing partitioning-related sub-contexts.
>>
>>> To your question, I think it probably can't share a context.  Briefly,
>>> rd_partkey can't change ever, except that when a partitioned relation
>>> is in the process of being created it is briefly NULL; once it obtains
>>> a value, that value cannot be changed.  If you want to range-partition
>>> a list-partitioned table or something like that, you have to drop the
>>> table and create a new one.  I think that's a perfectly acceptable
>>> permanent limitation and I have no urge whatever to change it.
>>> rd_partdesc changes when you attach or detach a child partition.
>>> rd_partcheck is the reverse: it changes when you attach or detach this
>>> partition to or from a parent.
>>
>> Got it.  Yeah, it seems like the clearest and least bug-prone solution
>> is for those to be in three separate sub-contexts.  The only reason
>> I was trying to avoid that was the angle of how to back-patch into 11.
>> However, we can just add the additional context pointer field at the
>> end of the Relation struct in v11, and that should be good enough to
>> avoid ABI problems.
> 
> Agree that rd_partcheck needs its own context as you have complained in
> the past [1].
> 
> I think we'll need to back-patch this fix to PG 10 as well.  I've attached
> patches for all 3 branches.
> 
> Thanks,
> Amit
> 
> [1] https://www.postgresql.org/message-id/22236.1523374067%40sss.pgh.pa.us

Should I add this patch to Older Bugs [1]?

Thanks,
Amit

[1] https://wiki.postgresql.org/wiki/PostgreSQL_12_Open_Items

Re: reloption to prevent VACUUM from truncating empty pages at the end of relation

[Masahiko Sawada]

On Mon, Apr 8, 2019 at 5:15 PM Fujii Masao  wrote:
>
> On Mon, Apr 8, 2019 at 3:58 PM Julien Rouhaud  wrote:
> >
> > On Mon, Apr 8, 2019 at 8:01 AM Fujii Masao  wrote:
> > >
> > > 2019年4月8日(月) 14:22 Tsunakawa, Takayuki :
> > >>
> > >> From: Alvaro Herrera [mailto:alvhe...@2ndquadrant.com]
> > >> > "vacuum_truncate" gets my vote too.
> > >>
> > >> +1
> > >
> > >
> > > +1
> > > ISTM that we have small consensus to
> > > use "vacuum_truncate".
> >
> > I'm also fine with this name, and I also saw reports that this option
> > is already needed in some production workload, as Andres explained.
>
> OK, so I pushed the "vacuum_truncate" version of the patch.

Thank you!

"TRUNCATE" option for vacuum command should be added to the open items?


Regards,

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

Re: FETCH FIRST clause WITH TIES option

[Surafel Temesgen]

On Sun, Apr 7, 2019 at 1:39 AM Tomas Vondra 
wrote:

>
> 1) I've removed the costing changes. As Tom mentions elsewhere in this
> thread, that's probably not needed for v1 and it's true those estimates
> are probably somewhat sketchy anyway.
>
>
> 2) v8 did this (per my comment):
>
> -   ExecSetTupleBound(compute_tuples_needed(node),
> outerPlanState(node));
> +   if(node->limitOption == EXACT_NUMBER)
> +   ExecSetTupleBound(compute_tuples_needed(node),
> outerPlanState(node));
>
> but I noticed the comment immediately above that says
>
>   * Notify child node about limit.  Note: think not to "optimize" by
>   * skipping ExecSetTupleBound if compute_tuples_needed returns < 0.  We
>   * must update the child node anyway, in case this is a rescan and the
>   * previous time we got a different result.
>
> which applies to WITH_TIES too, no? So I've modified compute_tuples_needed
> to return -1, and reverted this change. Not sure, though.
>
>
>
I agree that passing -1 to  ExecSetTupleBound is more appropriate
implementation


3) I'm a bit confused by the initialization added to ExecInitLimit. It
> first gets the tuple descriptor from the limitstate (it should not do so
> directly but use ExecGetResultType). But when it creates the extra slot,
> it uses ops extracted from the outer plan. That's strange, I guess ...
>
> And then it extracts the descriptor from the outer plan and uses it when
> calling execTuplesMatchPrepare. But AFAIK it's going to be compared to
> the last_slot, which is using a descriptor from the limitstate.
>
> IMHO all of this should use descriptor/ops from the outer plan, no? It
> probably does not change anything because limit does not project, but it
> seems confusing.
>


agree

regards
Surafel

Re: reloption to prevent VACUUM from truncating empty pages at the end of relation

[Fujii Masao]

On Mon, Apr 8, 2019 at 5:20 PM Julien Rouhaud  wrote:
>
> On Mon, Apr 8, 2019 at 10:15 AM Fujii Masao  wrote:
> >
> > OK, so I pushed the "vacuum_truncate" version of the patch.
>
> Thanks!
>
> > But it has not been actually pushed into the community's git
> > repository yet.That's maybe because it's been a while since
> > my last commit and my commit bit is temporarily limited?
> > Anyway the patch has been pushed before April 8th 0:00 in AoE.
>
> Indeed, there's no mail yet, but I can see the commit at
> https://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=119dcfad988d5b5d9f52b256087869997670aa36

Thanks for the info, so I marked the patch as committed.

Regards,

-- 
Fujii Masao

Re: reloption to prevent VACUUM from truncating empty pages at the end of relation

[Julien Rouhaud]

On Mon, Apr 8, 2019 at 10:15 AM Fujii Masao  wrote:
>
> OK, so I pushed the "vacuum_truncate" version of the patch.

Thanks!

> But it has not been actually pushed into the community's git
> repository yet.That's maybe because it's been a while since
> my last commit and my commit bit is temporarily limited?
> Anyway the patch has been pushed before April 8th 0:00 in AoE.

Indeed, there's no mail yet, but I can see the commit at
https://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=119dcfad988d5b5d9f52b256087869997670aa36

Re: reloption to prevent VACUUM from truncating empty pages at the end of relation

[Fujii Masao]

On Mon, Apr 8, 2019 at 3:58 PM Julien Rouhaud  wrote:
>
> On Mon, Apr 8, 2019 at 8:01 AM Fujii Masao  wrote:
> >
> > 2019年4月8日(月) 14:22 Tsunakawa, Takayuki :
> >>
> >> From: Alvaro Herrera [mailto:alvhe...@2ndquadrant.com]
> >> > "vacuum_truncate" gets my vote too.
> >>
> >> +1
> >
> >
> > +1
> > ISTM that we have small consensus to
> > use "vacuum_truncate".
>
> I'm also fine with this name, and I also saw reports that this option
> is already needed in some production workload, as Andres explained.

OK, so I pushed the "vacuum_truncate" version of the patch.
But it has not been actually pushed into the community's git
repository yet.That's maybe because it's been a while since
my last commit and my commit bit is temporarily limited?
Anyway the patch has been pushed before April 8th 0:00 in AoE.

Regards,

-- 
Fujii Masao

Re: [HACKERS] Weaker shmem interlock w/o postmaster.pid

[Thomas Munro]

On Mon, Apr 8, 2019 at 6:42 PM Noah Misch  wrote:
> - lorikeet's FailedAssertion("!(vmq->mq_sender == ((void *)0))" looked
>   suspicious, but this happened six other times in the past year[2], always on
>   v10 lorikeet.

It happens on v11 too:

https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=lorikeet=2018-09-25%2010%3A06%3A31

The text changed slightly because we dropped an unnecessary extra
pointer-to-volatile:

FailedAssertion("!(mq->mq_sender == ((void *)0))"

So either two workers started with the same parallel worker number, or
something unexpectedly overwrote the shm_mq struct?

-- 
Thomas Munro
https://enterprisedb.com

Re: pg_rewind vs superuser

[Peter Eisentraut]

On 2019-04-04 12:43, Michael Paquier wrote:
> I would like to apply this down to 9.5 for the checkpoint part and
> down to 11 for the role part, so if anybody has any comments, please
> feel free.

How about some tests to show that this is actually true?

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