Re: [HACKERS] pgbench more operators & functions

[Pavel Stehule]

2017-05-30 7:19 GMT+02:00 Fabien COELHO :

>
> [doc about CASE...]
>>>
>>
>> I've improved it in attached v11:
>> - add a link to the CASE full documentation
>> - add an example expression with CASE ...
>>
>
> Do you think I should improve the doc further?


I am sorry, I didn't look there yet

The patch looks ok, but the main issue is missing regress tests. I know so
it is another patch. But it should be placed differently

1. first patch - initial infrastructure for pgbench regress tests
2. this patch + related regress tests

Regards

Pavel



>
>
> --
> Fabien.
>

Re: [HACKERS] Adding support for Default partition in partitioning

[Beena Emerson]

On Mon, May 29, 2017 at 9:33 PM, Jeevan Ladhe
 wrote:
> Hi,
>
> I have rebased the patch on latest commit with few cosmetic changes.
>
> The patch fix_listdatums_get_qual_for_list_v3.patch [1]  needs to be applied
> before applying this patch.
>
> [1] http://www.mail-archive.com/pgsql-hackers@postgresql.org/msg315490.html
>


This needs a rebase again.

-- 

Beena Emerson

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


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

Re: [HACKERS] pgbench more operators & functions

[Fabien COELHO]

[doc about CASE...]


I've improved it in attached v11:
- add a link to the CASE full documentation
- add an example expression with CASE ...


Do you think I should improve the doc further?

--
Fabien.


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

Re: [HACKERS] POC: Sharing record typmods between backends

[Thomas Munro]

On Fri, Apr 7, 2017 at 5:21 PM, Thomas Munro
 wrote:
> * It would be nice for the SharedRecordTypeRegistry to be able to
> survive longer than a single parallel query, perhaps in a per-session
> DSM segment.  Perhaps eventually we will want to consider a
> query-scoped area, a transaction-scoped area and a session-scoped
> area?  I didn't investigate that for this POC.

This seems like the right way to go.  I think there should be one
extra patch in this patch stack, to create a per-session DSA area (and
perhaps a "SharedSessionState" struct?) that worker backends can
attach to.  It could be created when you first run a parallel query,
and then reused for all parallel queries for the rest of your session.
So, after you've run one parallel query, all future record typmod
registrations would get pushed (write-through style) into shmem, for
use by other backends that you might start in future parallel queries.
That will avoid having to copy the leader's registered record typmods
into shmem for every query going forward (the behaviour of the current
POC patch).

> * Perhaps simplehash + an LWLock would be better than dht, but I
> haven't looked into that.  Can it be convinced to work in DSA memory
> and to grow on demand?

Any views on this?

> 1.  Apply dht-v3.patch[3].
> 2.  Apply shared-record-typmod-registry-v1.patch.
> 3.  Apply rip-out-tqueue-remapping-v1.patch.

Here's a rebased version of the second patch (the other two still
apply).  It's still POC code only and still uses a
per-parallel-context DSA area for space, not the per-session one I am
now proposing we develop, if people are in favour of the approach.

In case it wasn't clear from my earlier description, a nice side
effect of using a shared typmod registry is that you can delete 85% of
tqueue.c (see patch #3), so if you don't count the hash table
implementation we come out about even in terms of lines of code.

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


shared-record-typmod-registry-v2.patch
Description: Binary data

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

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

[Mithun Cy]

Thanks Robert,

On Wed, May 24, 2017 at 5:41 PM, Robert Haas  wrote:
> + *
> + * Once the "autoprewarm" bgworker has completed its prewarm task, it will
> + * start a new task to periodically dump the BlockInfoRecords related to 
> blocks
> + * which are currently in shared buffer pool. Upon next server restart, the
> + * bgworker will prewarm the buffer pool by loading those blocks. The GUC
> + * pg_prewarm.dump_interval will control the dumping activity of the 
> bgworker.
> + */
>
> Make this part of the file header comment.

-- Thanks, I have moved same as part of header description.

> Also, add an enabling GUC.
> The default can be true, but it should be possible to preload the
> library so that the SQL functions are available without a dynamic
> library load without requiring you to get the auto-prewarm behavior.
> I suggest pg_prewarm.autoprewarm = true / false.

-- Thanks, I have added a boolean GUC pg_prewarm.autoprewarm with
default true. I have made it as PGC_POSTMASTER level variable
considering the intention is here to avoid starting the autoprewarm
worker. Need help, have I missed anything? Currently, sql callable
functions autoprewarm_dump_now(), launch_autoprewarm_dump() are only
available after create extension pg_prewarm command will this change
now?
There is another GUC setting pg_prewarm.dump_interval if = -1 we stop
the running autoprewarm worker. I have a doubt should we combine these
2 entities into one such that it control the state of autoprewarm
worker?

> Your SigtermHandler and SighupHandler routines are still capitalized
> in a way that's not very consistent with what we do elsewhere.  I
> think all of our other signal handlers have names_like_this() not
> NamesLikeThis().
>

-- handler functions are renamed for example apw_sigterm_handler, as
similar in autovacuum.c

> + * ==types and variables used by autoprewam  
> =
>
> Spelling.

-- Fixed, Sorry.


> + * Meta-data of each persistent block which is dumped and used to load.
>
> Metadata

-- Fixed.


> +typedef struct BlockInfoRecord
> +{
> +Oiddatabase;/* database */
> +OidspcNode;/* tablespace */
> +Oidfilenode;/* relation's filenode. */
> +ForkNumberforknum;/* fork number */
> +BlockNumber blocknum;/* block number */
> +} BlockInfoRecord;
>
> spcNode is capitalized differently from all of the other members.

-- renamed from spcNode to spcnode.
>
> +LWLock   *lock;/* protects SharedState */
>
> Just declare this as LWLock lock, and initialize it using
> LWLockInitialize.  The way you're doing it is more complicated.

-- Fixed as suggested
LWLockInitialize(>lock, LWLockNewTrancheId());

> +static dsa_area *AutoPrewarmDSA = NULL;
>
> DSA seems like more than you need here.  There's only one allocation
> being performed.  I think it would be simpler and less error-prone to
> use DSM directly.  I don't even think you need a shm_toc.  You could
> just do:
>
> seg = dsm_create(segsize, 0);
> blkinfo = dsm_segment_address(seg);
> Then pass dsm_segment_handle(seg) to the worker using bgw_main_arg or
> bgw_extra, and have it call dsm_attach.  An advantage of this approach
> is that you only allocate the memory you actually need, whereas DSA
> will allocate more, expecting that you might do further allocations.

- Thanks Fixed. And we pass following arguments to subwrokers through bgw_extra
/*
 * The block_infos allocated to each sub-worker to do prewarming.
 */
typedef struct prewarm_elem
{
dsm_handle  block_info_handle;  /* handle to dsm seg of block_infos */
Oid database;   /* database to connect and load */
uint32  start_pos;  /* start position within block_infos from
 * which sub-worker start prewaring blocks. */
uint32  end_of_blockinfos;  /* End of block_infos in dsm */
} prewarm_elem;

to distribute the prewarm work among subworkers.

>
> +pg_qsort(block_info_array, num_blocks, sizeof(BlockInfoRecord),
> + blockinfo_cmp);
>
> I think it would make more sense to sort the array on the load side,
> in the autoprewarm worker, rather than when dumping.

Fixed Now sorting block_infos just before loading the blocks

> + errmsg("autoprewarm: could not open \"%s\": %m",
> +dump_file_path)));
>
> Use standard error message wordings!  Don't create new translatable
> messages by adding "autoprewarm" to error messages.  There are
> multiple instances of this throughout the patch.

-- Removed autoprewarm as part of sufix in error message in all of the places.


> +snprintf(dump_file_path, sizeof(dump_file_path),
> + "%s", AUTOPREWARM_FILE);
>
> This is completely pointless.  You can get rid of the dump_file_path
-- dump_file_path is removed.


>
> +SPI_connect();
> +

Re: [HACKERS] "create publication..all tables" ignore 'partition not supported' error

[Amit Langote]

On 2017/05/22 20:02, Kuntal Ghosh wrote:
> Yeah, it's a bug. While showing the table definition, we use the
> following query for showing the related publications:
> "SELECT pub.pubname\n"
>   " FROM pg_catalog.pg_publication pub\n"
>   " LEFT JOIN pg_catalog.pg_publication_rel pr\n"
>   "  ON (pr.prpubid = pub.oid)\n"
>   "WHERE pr.prrelid = '%s' OR pub.puballtables\n"
>   "ORDER BY 1;"
> 
> When pub.puballtables is TRUE, we should also check whether the
> relation is publishable or not.(Something like is_publishable_class
> method in pg_publication.c).

BTW, you can see the following too, because of the quoted query:

create publication pub2 for all tables;

-- system tables are not publishable, but...
\d pg_class

Publications:
"pub2"

-- unlogged tables are not publishable, but...
create unlogged table foo (a int);
\d foo

Publications:
"pub2"

-- temp tables are not publishable, but...
create temp table bar (a int);
\d bar

Publications:
"pub2"

The above contradicts what check_publication_add_tables() thinks are
publishable relations.

At first, I thought this would be fixed by simply adding a check on
relkind and relpersistence in the above query, both of which are available
to describeOneTableDetails() already.  But, it won't be possible to check
the system table status using the available information, so we might need
to add a SQL-callable version of is_publishable_class() after all.

> 
> However, I'm not sure whether this is the correct way to solve the problem.

AFAICS, the problem is with psql's describe query itself and no other
parts of the system are affected by this.  So, fixing the query after
adding appropriate backend support will do, I think.

Thanks,
Amit



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

Re: [HACKERS] [COMMITTERS] Re: pgsql: Code review focused on new node types added by partitioning supp

[Amit Langote]

On 2017/05/30 11:41, Stephen Frost wrote:
> * Tom Lane (t...@sss.pgh.pa.us) wrote:
>> Noah Misch  writes:
>>> On Mon, May 29, 2017 at 03:20:41AM +, Tom Lane wrote:
 Annotate the fact that somebody added location fields to PartitionBoundSpec
 and PartitionRangeDatum but forgot to handle them in
 outfuncs.c/readfuncs.c.  This is fairly harmless for production purposes
 (since readfuncs.c would just substitute -1 anyway) but it's still bogus.
 It's not worth forcing a post-beta1 initdb just to fix this, but if we
 have another reason to force initdb before 10.0, we should go back and
 clean this up.
>>
>>> +1 for immediately forcing initdb for this, getting it out of the way.  
>>> We're
>>> already unlikely to reach 10.0 without bumping catversion, but if we 
>>> otherwise
>>> did, releasing 10.0 with a 10beta1 catversion would have negative value.
>>
>> I'm not really for doing it that way, but I'm willing to apply the fix
>> if there's consensus for your position.  Anybody else have an opinion?
> 
> I tend to agree with Noah on this one.

+1

Thanks,
Amit



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

Re: [HACKERS] pg_resetwal is broken if run from v10 against older version of PG data directory

[Craig Ringer]

On 30 May 2017 at 00:00, Tom Lane  wrote:

> I think it's just horribly dangerous to run any version of
> pg_resetxlog/pg_resetwal

You can pretty much stop there ;)

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


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

Re: [HACKERS] pg_resetwal is broken if run from v10 against older version of PG data directory

[Amit Kapila]

On Mon, May 29, 2017 at 9:30 PM, Tom Lane  wrote:
> Amit Kapila  writes:
>> I think this happens due to commit
>> f82ec32ac30ae7e3ec7c84067192535b2ff8ec0e which renames pg_xlog to
>> pg_wal.  It does take care of making some of the modules like
>> pg_basebackup to understand both old and new directory structures, but
>> not done for all the modules.
>
> check
>
>> I think we should make similar changes
>> in pg_resetwal or at the very least update the docs to indicate
>> pg_resetwal can give an error if used against pre-10 data directory.
>
> I think it's just horribly dangerous to run any version of
> pg_resetxlog/pg_resetwal against any major version other than its
> own.  If people have been doing what the OP tried to do, it's only
> been sheerest luck if it didn't destroy their database beyond recall.
> The result will certainly fail to start up, because both pg_control
> and WAL page header versions will not match the server version.  The
> *best* case scenario is that redoing the reset with the correct version
> of pg_resetxlog/pg_resetwal gets you out of that with no new damage
> done.  The worst case is pretty awful --- for instance, a truly
> bullheaded user might try to get out of it by starting the newer server
> version in the old DB, likely causing irrecoverable catalog corruption.
>
> So we need to prevent this, not try to make it work.
>

Yeah, I was wrong in suggesting to try to make it work.  An explicit
error is a better way to deal with this.


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


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

Re: [HACKERS] Server ignores contents of SASLInitialResponse

[Noah Misch]

On Thu, May 25, 2017 at 10:52:23AM -0400, Michael Paquier wrote:
> On Thu, May 25, 2017 at 9:32 AM, Michael Paquier
>  wrote:
> > On Thu, May 25, 2017 at 8:51 AM, Heikki Linnakangas  wrote:
> >> On 05/24/2017 11:33 PM, Michael Paquier wrote:
> >>> I have noticed today that the server ignores completely the contents
> >>> of SASLInitialResponse. ... Attached is a patch to fix the problem.
> >>
> >> Fixed, thanks!
> >
> > Thanks for the commit.
> 
> Actually, I don't think that we are completely done here. Using the
> patch of upthread to enforce a failure on SASLInitialResponse, I see
> that connecting without SSL causes the following error:
> psql: FATAL:  password authentication failed for user "mpaquier"
> But connecting with SSL returns that:
> psql: duplicate SASL authentication request
> 
> I have not looked at that in details yet, but it seems to me that we
> should not take pg_SASL_init() twice in the scram authentication code
> path in libpq for a single attempt.

[Action required within three days.  This is a generic notification.]

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

[1] 
https://www.postgresql.org/message-id/20170404140717.GA2675809%40tornado.leadboat.com


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

Re: [HACKERS] Replication status in logical replication

[Noah Misch]

On Fri, May 19, 2017 at 11:33:48AM +0900, Masahiko Sawada wrote:
> On Wed, Apr 12, 2017 at 5:31 AM, Simon Riggs  wrote:
> > On 22 March 2017 at 02:50, Masahiko Sawada  wrote:
> >
> >> When using logical replication, I ran into a situation where the
> >> pg_stat_replication.state is not updated until any wal record is sent
> >> after started up. For example, I set up logical replication with 2
> >> subscriber and restart the publisher server, but I see the following
> >> status for a while (maybe until autovacuum run).
> > ...
> >
> >> Attached patch fixes this behavior by updating WalSndCaughtUp before
> >> trying to read next WAL if already caught up.
> >
> > Looks like a bug that we should fix in PG10, with backpatch to 9.4 (or
> > as far as it goes).
> >
> > Objections to commit?
> >
> 
> Seems we still have this issue. Any update or comment on this? Barring
> any objections, I'll add this to the open item so it doesn't get
> missed.

[Action required within three days.  This is a generic notification.]

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

[1] 
https://www.postgresql.org/message-id/20170404140717.GA2675809%40tornado.leadboat.com


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

Re: [HACKERS] [COMMITTERS] Re: pgsql: Code review focused on new node types added by partitioning supp

[Stephen Frost]

* Tom Lane (t...@sss.pgh.pa.us) wrote:
> Noah Misch  writes:
> > On Mon, May 29, 2017 at 03:20:41AM +, Tom Lane wrote:
> >> Annotate the fact that somebody added location fields to PartitionBoundSpec
> >> and PartitionRangeDatum but forgot to handle them in
> >> outfuncs.c/readfuncs.c.  This is fairly harmless for production purposes
> >> (since readfuncs.c would just substitute -1 anyway) but it's still bogus.
> >> It's not worth forcing a post-beta1 initdb just to fix this, but if we
> >> have another reason to force initdb before 10.0, we should go back and
> >> clean this up.
> 
> > +1 for immediately forcing initdb for this, getting it out of the way.  
> > We're
> > already unlikely to reach 10.0 without bumping catversion, but if we 
> > otherwise
> > did, releasing 10.0 with a 10beta1 catversion would have negative value.
> 
> I'm not really for doing it that way, but I'm willing to apply the fix
> if there's consensus for your position.  Anybody else have an opinion?

I tend to agree with Noah on this one.

Thanks!

Stephen


signature.asc
Description: Digital signature

Re: [HACKERS] [COMMITTERS] Re: pgsql: Code review focused on new node types added by partitioning supp

[Tom Lane]

Noah Misch  writes:
> On Mon, May 29, 2017 at 03:20:41AM +, Tom Lane wrote:
>> Annotate the fact that somebody added location fields to PartitionBoundSpec
>> and PartitionRangeDatum but forgot to handle them in
>> outfuncs.c/readfuncs.c.  This is fairly harmless for production purposes
>> (since readfuncs.c would just substitute -1 anyway) but it's still bogus.
>> It's not worth forcing a post-beta1 initdb just to fix this, but if we
>> have another reason to force initdb before 10.0, we should go back and
>> clean this up.

> +1 for immediately forcing initdb for this, getting it out of the way.  We're
> already unlikely to reach 10.0 without bumping catversion, but if we otherwise
> did, releasing 10.0 with a 10beta1 catversion would have negative value.

I'm not really for doing it that way, but I'm willing to apply the fix
if there's consensus for your position.  Anybody else have an opinion?

regards, tom lane


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

Re: [HACKERS] PATCH: recursive json_populate_record()

[Tom Lane]

Noah Misch  writes:
> On Mon, May 22, 2017 at 10:19:37PM +0300, Nikita Glukhov wrote:
>> Attached two small fixes for the previous committed patch:

> [Action required within three days.  This is a generic notification.]

> The above-described topic is currently a PostgreSQL 10 open item.  Andrew,
> since you committed the patch believed to have created it, you own this open
> item.

These fixes are committed in e45c5be99d08d7bb6708d7bb1dd0f5d99798c6aa and
68cff231e3a3d0ca9988cf1fde5a3be53235b3bb.

regards, tom lane


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

[HACKERS] Re: Create subscription with `create_slot=false` and incorrect slot name

[Noah Misch]

On Fri, May 26, 2017 at 05:05:37PM -0400, Peter Eisentraut wrote:
> On 5/25/17 19:16, Petr Jelinek wrote:
> >> The reported error is just one of many errors that can happen when DROP
> >> SUBSCRIPTION tries to drop the slot (doens't exist, still active, no
> >> permission, etc.).  We don't want to give the hint that is effectively
> >> "just forget about the slot then" for all of them.  So we would need
> >> some way to distinguish "you can't do this right now" from "this would
> >> never work" (400 vs 500 errors).
> >>
> > This specific error returns ERRCODE_UNDEFINED_OBJECT error code so we
> > could check for it and offer hint only for this case.
> 
> We would have to extend the walreceiver interface a little to pass that
> through, but it seems doable.

[Action required within three days.  This is a generic notification.]

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

[1] 
https://www.postgresql.org/message-id/20170404140717.GA2675809%40tornado.leadboat.com


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

Re: [HACKERS] PATCH: recursive json_populate_record()

[Noah Misch]

On Mon, May 22, 2017 at 10:19:37PM +0300, Nikita Glukhov wrote:
> Attached two small fixes for the previous committed patch:
> 
> 1. I've noticed a difference in behavior between json_populate_record()
> and  jsonb_populate_record() if we are trying to populate record from a
> non-JSON-object: json function throws an error but jsonb function returns
> a record with NULL fields. So I think it would be better to throw an error
> in jsonb case too, but I'm not sure.
> 
> 2. Also I've found a some kind of thinko in JsGetObjectSize() macro, but it
> seems that this obvious mistake can not lead to incorrect behavior.

[Action required within three days.  This is a generic notification.]

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

[1] 
https://www.postgresql.org/message-id/20170404140717.GA2675809%40tornado.leadboat.com


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

Re: [HACKERS] ALTER PUBLICATION documentation

[Noah Misch]

On Wed, May 24, 2017 at 07:24:08PM -0400, Peter Eisentraut wrote:
> On 5/22/17 17:50, Jeff Janes wrote:
> > "The first variant of this command listed in the synopsis can change all
> > of the publication properties specified in CREATE PUBLICATION
> > ."
> > 
> > That referenced first variant no longer exists.  I don't if that should
> > just be removed, or reworked to instead describe "ALTER PUBLICATION name
> > SET".
> 
> Yeah that got whacked around a bit too much.  I'll see about fixing it up.

[Action required within three days.  This is a generic notification.]

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

[1] 
https://www.postgresql.org/message-id/20170404140717.GA2675809%40tornado.leadboat.com


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

[HACKERS] Re: pg_dump ignoring information_schema tables which used in Create Publication.

[Noah Misch]

On Fri, May 26, 2017 at 10:46:12PM -0300, Euler Taveira wrote:
> 2017-05-26 17:52 GMT-03:00 Peter Eisentraut 
> :
> > You cannot publish a system catalog.  But a user-created table in
> > information_schema is not a system catalog.
> 
> Replication of information_schema tables works. However, pg_dump doesn't
> include information_schema tables into CREATE PUBLICATION command
> (user-defined information_schema tables aren't included in pg_dump even
> *before* logical replication). IMO allow publish/subscribe of tables into
> information_schema is harmless (they aren't special tables like catalogs).
> Also, how many people would create real tables into information_schema?
> Almost zero. Let's leave it alone. Since pg_dump doesn't document that
> information_schema isn't dumped, I think we shouldn't document this for
> logical replication.

[Action required within three days.  This is a generic notification.]

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

[1] 
https://www.postgresql.org/message-id/20170404140717.GA2675809%40tornado.leadboat.com


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

Re: [HACKERS] Why does logical replication launcher set application_name?

[Noah Misch]

On Tue, May 23, 2017 at 07:50:34PM +0200, Petr Jelinek wrote:
> On 20/04/17 21:33, Peter Eisentraut wrote:
> > On 4/18/17 13:18, Tom Lane wrote:
> >> I think you're thinking about it wrong.  To my mind the issue is that
> >> there should be some generic way to determine that a bgworker process
> >> is or is not laboring on behalf of an identifiable user.  It's great
> >> that we can tell which user it is when there is one, but clearly some
> >> bgworkers will be providing general services that aren't associated with
> >> a single user.  So it should be possible to set the userID to zero or
> >> some such when the bgworker is one that isn't associated with a
> >> particular user.  Maybe the owning user needs to become an additional
> >> parameter passed in struct BackgroundWorker.
> > 
> > I think this is probably a problem particular to the logical replication
> > launcher.  Other background workers either do work as a particular user,
> > as you say, or don't touch the database at all.  So a localized hack or
> > a simple hide-the-user flag might suffice for now.
> > 
> 
> But that still leaves the application_name issue. My proposal in general
> would be to add boolean that indicates that the worker is not using
> specific user (this can be easily set in
> InitializeSessionUserIdStandalone()) and will work for multiple things.
> 
> About application_name, perhaps we should just add bgw_type or bgw_group
> and show it as worker_type in activity and that's it?
> 
> I think this should be open item btw so I'll add it.

[Action required within three days.  This is a generic notification.]

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

[1] 
https://www.postgresql.org/message-id/20170404140717.GA2675809%40tornado.leadboat.com


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

Re: [HACKERS] walsender & parallelism

[Noah Misch]

On Tue, May 23, 2017 at 01:45:59PM -0400, Andres Freund wrote:
> On May 23, 2017 1:42:41 PM EDT, Petr Jelinek  
> wrote:
> >Hi,
> >
> >so this didn't really move anywhere AFAICS, do we think the approach
> >I've chosen is good or do we want to do something else here?
> 
> Can you add it to the open items list?

[Action required within three days.  This is a generic notification.]

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

[1] 
https://www.postgresql.org/message-id/20170404140717.GA2675809%40tornado.leadboat.com


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

Re: [HACKERS] Race conditions with WAL sender PID lookups

[Noah Misch]

On Sat, May 20, 2017 at 09:40:57PM +0900, Michael Paquier wrote:
> On Fri, May 19, 2017 at 3:01 PM, Masahiko Sawada  
> wrote:
> > Also, as Horiguchi-san pointed out earlier, walreceiver seems need the
> > similar fix.
> 
> Actually, now that I look at it, ready_to_display should as well be
> protected by the lock of the WAL receiver, so it is incorrectly placed
> in walreceiver.h. As you are pointing out, pg_stat_get_wal_receiver()
> is lazy as well, and that's new in 10, so we have an open item here
> for both of them. And I am the author for both things. No issues
> spotted in walreceiverfuncs.c after review.
> 
> I am adding an open item so as both issues are fixed in PG10. With the
> WAL sender part, I think that this should be a group shot.
> 
> So what do you think about the attached?

[Action required within three days.  This is a generic notification.]

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

[1] 
https://www.postgresql.org/message-id/20170404140717.GA2675809%40tornado.leadboat.com


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

[HACKERS] Re: transition table behavior with inheritance appears broken (was: Declarative partitioning - another take)

[Noah Misch]

This PostgreSQL 10 open item is past due for your status update.  Kindly send
a status update within 24 hours, and include a date for your subsequent status
update.  Refer to the policy on open item ownership:
https://www.postgresql.org/message-id/20170404140717.GA2675809%40tornado.leadboat.com


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

Re: [HACKERS] Get stuck when dropping a subscription during synchronizing table

[Noah Misch]

On Thu, May 18, 2017 at 10:27:51PM -0400, Peter Eisentraut wrote:
> On 5/18/17 11:11, Noah Misch wrote:
> > IMMEDIATE ATTENTION REQUIRED.  This PostgreSQL 10 open item is past due for
> > your status update.  Please reacquaint yourself with the policy on open item
> > ownership[1] and then reply immediately.  If I do not hear from you by
> > 2017-05-19 16:00 UTC, I will transfer this item to release management team
> > ownership without further notice.
> 
> There is no progress on this issue at the moment.  I will report again
> next Wednesday.

IMMEDIATE ATTENTION REQUIRED.  This PostgreSQL 10 open item is long past due
for your status update.  If I do not hear one from you by 2017-05-31 02:00
UTC, I will transfer this item to release management team ownership without
further notice.


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

Re: [HACKERS] Alter subscription..SET - NOTICE message is coming for table which is already removed

[Masahiko Sawada]

On Thu, May 25, 2017 at 8:15 PM, tushar  wrote:
> On 05/25/2017 04:40 PM, Masahiko Sawada wrote:
>>
>> I think you did ALTER SUBSCRIPTION while table sync for 100 tables is
>> running, right?
>
> Yes, i didn't wait too much while executing the commands.
>

I think it's better to add this to open items so that it doesn't get
missed. Barring any objections I'll add it.

Regards,

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


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

[HACKERS] Fix GetOldestXmin comment

[Masahiko Sawada]

Hi,

While reading source code, I realized that comment of GetOldestXmin mentions;

  * if rel = NULL and there are no transactions running in the current
  * database, GetOldestXmin() returns latestCompletedXid.

However, in that case if I understand correctly GetOldestXmin()
actually returns latestCompletedXid + 1 as follows;

  /*
   * We initialize the MIN() calculation with latestCompletedXid + 1. This
   * is a lower bound for the XIDs that might appear in the ProcArray later,
   * and so protects us against overestimating the result due to future
   * additions.
   */
  result = ShmemVariableCache->latestCompletedXid;
  Assert(TransactionIdIsNormal(result));
  TransactionIdAdvance(result);

Attached patch fixes the top comment of GetOldestXmin.

Regards,

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


getoldestxmin_comment.patch
Description: Binary data

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

Re: [HACKERS] Receive buffer size for the statistics socket

[Tom Lane]

I wrote:
>> I propose that it'd be a good idea to try to set the stats socket's
>> receive buffer size to be a minimum of, say, 100K on all platforms.
>> Code for this would be analogous to what we already have in pqcomm.c
>> (circa line 760) for forcing up the send buffer size, but SO_RCVBUF
>> not SO_SNDBUF.

> I propose committing this (less the debug logging part) to HEAD
> once the beta is out, and then back-patching if it doesn't break
> anything and seems to improve matters on frogmouth.

That went in in 8b0b6303e.  frogmouth had failed in 5 of the 23 HEAD runs
it made between 4e37b3e15 and 8b0b6303e.  Since then, it has shown zero
failures in 50 runs.  I don't know what confidence a statistician would
assign to the proposition that 8b0b6303e improved things, but this is good
enough for me.  I'm going to go back-patch it.

regards, tom lane


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

Re: [HACKERS] PATCH: recursive json_populate_record()

[Tom Lane]

Nikita Glukhov  writes:
> Attached two small fixes for the previous committed patch:

> 1. I've noticed a difference in behavior between json_populate_record()
> and  jsonb_populate_record() if we are trying to populate record from a
> non-JSON-object: json function throws an error but jsonb function returns
> a record with NULL fields. So I think it would be better to throw an error
> in jsonb case too, but I'm not sure.

Agreed on the error.  I reformatted the code a bit.

> 2. Also I've found a some kind of thinko in JsGetObjectSize() macro, but it
> seems that this obvious mistake can not lead to incorrect behavior.

Hm, I think it actually was wrong for the case of jsonb_cont == NULL,
wasn't it?  But maybe that case didn't arise for the sole call site.
Anyway, agreed.  Pushed both patches.

regards, tom lane


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

Re: [HACKERS] GSoC 2017: Foreign Key Arrays

[Mark Rofail]

>
> rhaas=# select oid, * from pg_opfamily where opfmethod = 2742;
>  oid  | opfmethod |opfname | opfnamespace | opfowner
> --+---++--+--
>  2745 |  2742 | array_ops  |   11 |   10
>  3659 |  2742 | tsvector_ops   |   11 |   10
>  4036 |  2742 | jsonb_ops  |   11 |   10
>  4037 |  2742 | jsonb_path_ops |   11 |   10
> (4 rows)

I am particulary intrested in array_ops but I have failed in locating the
code behind it. Where is it reflected in the source code

Best Regards,
Mark Rofail

Re: [HACKERS] logical replication busy-waiting on a lock

[Petr Jelinek]

On 29/05/17 21:44, Andres Freund wrote:
> 
> 
> On May 29, 2017 12:41:26 PM PDT, Petr Jelinek  
> wrote:
>> On 29/05/17 21:28, Andres Freund wrote:
>>>
>>> On May 29, 2017 12:25:35 PM PDT, Petr Jelinek
>>  wrote:

 Looking at the code more, the xid is only used as parameter for
 SnapBuildBuildSnapshot() which never does anything with that
>> parameter,
 I wonder if it's really needed then.
>>>
>>> Not at a computer, but by memory that'll trigger the snapshot export
>> routine to include it.  Import in turn requires the xid to check if the
>> source is still alive.  But there's better ways, e.g. using the virtual
>> xactid.
>>>
>>
>> It does, and while that's unfortunate the logical replication does not
>> actually export the snapshots. It uses the USE_SNAPSHOT option where
>> the
>> snapshot is just installed into current transaction but not exported.
>> So
>> not calling the GetTopTransactionId() would solve it at least for that
>> in-core use-case. I don't see any bad side effects from doing so yet,
>> so
>> it might be good enough solution for PG10.
> 
> In the general case you can't do so, because of vacuum and such.  Even for LR 
> we need to make sure the exporting session didn't die badly, deleting the 
> slot.  Hence suggestion to use the virtual xid.
> 

I am not quite sure I understand (both the vxid suggestion and for the
session dying badly). Maybe we can discuss bit more when you get to
computer so it's easier for you to expand on what you mean.

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


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

Re: [HACKERS] pg_resetwal is broken if run from v10 against older version of PG data directory

[Tom Lane]

Michael Paquier  writes:
> Agreed. Shouldn't this be back-patched? PG_CONTROL_VERSION has not
> been bumped between 9.4 and 9.5. Attached is a patch for HEAD.

Pushed with minor adjustments.  Notably, I didn't take your addition
of canonicalize_path() and referencing the file by absolute path.
That seems like an independent and rather debatable behavioral change.
All the other files that pg_resetwal touches are referenced with
relative paths; if we did want to change the reporting, shouldn't we
change it for all of them?

regards, tom lane


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

Re: [HACKERS] logical replication - still unstable after all these months

[Mark Kirkwood]

On 29/05/17 23:14, Petr Jelinek wrote:


On 29/05/17 03:33, Jeff Janes wrote:


What would you want to look at?  Would saving the WAL from the master be
helpful?


Useful info is, logs from provider (mainly the logical decoding logs
that mention LSNs), logs from subscriber (the lines about when sync
workers finished), contents of the pg_subscription_rel (with srrelid
casted to regclass so we know which table is which), and pg_waldump
output around the LSNs found in the logs and in the pg_subscription_rel
(+ few lines before and some after to get context). It's enough to only
care about LSNs for the table(s) that are out of sync.



I have a run that aborted with failure (accounts table md5 mismatch). 
Petr - would you like to have access to the machine ? If so send me you 
public key and I'll set it up.


Cheers

Mark


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

Re: [HACKERS] Bug when dumping "empty" operator classes

[Michael Paquier]

On Fri, May 26, 2017 at 8:14 AM, Daniel Gustafsson  wrote:
>> On 26 May 2017, at 17:08, Tom Lane  wrote:
>> I'll commit and back-patch this without a test case.  Possibly Frost will
>> be excited enough about it to add something to the pg_dump TAP tests,
>> but those tests are too opaque for me to want to do so.
>
> Thanks!

For the TAP tests I think that you are looking for something like the
attached. Stephen, perhaps you could consider including this test in
the suite?
-- 
Michael


pgdump-tap-empty-opclass.patch
Description: Binary data

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

Re: [HACKERS] Use of non-restart-safe storage by temp_tablespaces

[Tom Lane]

Andres Freund  writes:
> On May 29, 2017 12:15:37 PM PDT, Alvaro Herrera  
> wrote:
>> I think it'd be smart to support the use case directly, because there's
>> interest in it being actually supported (unlike the statu quo).
>> Something like restoring the tablespace to the empty state on boot, if
>> it's known to need it.

> Has the danger of making recovery harder after a restart where somebody 
> forgot to mount some subdirectory ...

Or even worse, the mount happens after PG starts (and creates directories
on the root volume, not knowing they should go onto the mount instead).

I'm too lazy to search the archives right now, but there was some case
years ago where somebody destroyed their database via an ill-thought-out
combination of automatic-initdb-if-$PGDATA-isn't-there and slow mounting.
We'd have to be very sure that any auto-directory-creation behavior didn't
have a potential for that.  Perhaps doing it only for temp tablespaces
alleviates some of the risk, but it still seems pretty scary.

regards, tom lane


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

Re: [HACKERS] [PATCH] relocation truncated to fit: citus build failure on s390x

[Tom Lane]

Christoph Berg  writes:
> Re: To Andres Freund 2016-04-28 <20160428080824.ga22...@msg.df7cb.de>
>>> I'm not clear why citus triggers this, when other extensions don't?

>> Maybe it's simply because citus.so is bigger than all the other
>> extension .so files:

I wonder what the overhead is of using -fPIC when -fpic would be
sufficient.  Whatever it is, the proposed patch imposes it on every
shlib or extension, to accommodate one single extension that isn't
even one we ship.

Maybe this is small enough to not be something we need to worry about,
but I'm wondering if we should ask citus and other large .so's to set
some additional make flag that would cue usage of -fPIC over -fpic.

regards, tom lane


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

Re: [HACKERS] logical replication busy-waiting on a lock

[Andres Freund]

On May 29, 2017 12:41:26 PM PDT, Petr Jelinek  
wrote:
>On 29/05/17 21:28, Andres Freund wrote:
>> 
>> On May 29, 2017 12:25:35 PM PDT, Petr Jelinek
> wrote:
>>>
>>> Looking at the code more, the xid is only used as parameter for
>>> SnapBuildBuildSnapshot() which never does anything with that
>parameter,
>>> I wonder if it's really needed then.
>> 
>> Not at a computer, but by memory that'll trigger the snapshot export
>routine to include it.  Import in turn requires the xid to check if the
>source is still alive.  But there's better ways, e.g. using the virtual
>xactid.
>> 
>
>It does, and while that's unfortunate the logical replication does not
>actually export the snapshots. It uses the USE_SNAPSHOT option where
>the
>snapshot is just installed into current transaction but not exported.
>So
>not calling the GetTopTransactionId() would solve it at least for that
>in-core use-case. I don't see any bad side effects from doing so yet,
>so
>it might be good enough solution for PG10.

In the general case you can't do so, because of vacuum and such.  Even for LR 
we need to make sure the exporting session didn't die badly, deleting the slot. 
 Hence suggestion to use the virtual xid.

Andres
-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.


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

Re: [HACKERS] logical replication busy-waiting on a lock

[Petr Jelinek]

On 29/05/17 21:28, Andres Freund wrote:
> 
> On May 29, 2017 12:25:35 PM PDT, Petr Jelinek  
> wrote:
>>
>> Looking at the code more, the xid is only used as parameter for
>> SnapBuildBuildSnapshot() which never does anything with that parameter,
>> I wonder if it's really needed then.
> 
> Not at a computer, but by memory that'll trigger the snapshot export routine 
> to include it.  Import in turn requires the xid to check if the source is 
> still alive.  But there's better ways, e.g. using the virtual xactid.
> 

It does, and while that's unfortunate the logical replication does not
actually export the snapshots. It uses the USE_SNAPSHOT option where the
snapshot is just installed into current transaction but not exported. So
not calling the GetTopTransactionId() would solve it at least for that
in-core use-case. I don't see any bad side effects from doing so yet, so
it might be good enough solution for PG10.

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


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

Re: [HACKERS] psql: Activate pager only for height, not width

[Christoph Berg]

Re: Jeff Janes 2017-05-29 

Re: [HACKERS] logical replication busy-waiting on a lock

[Petr Jelinek]

On 29/05/17 21:23, Andres Freund wrote:
> 
> 
> On May 29, 2017 12:21:50 PM PDT, Petr Jelinek  
> wrote:
>> On 29/05/17 20:59, Andres Freund wrote:
>>>
>>>
>>> On May 29, 2017 11:58:05 AM PDT, Petr Jelinek
>>  wrote:
 On 27/05/17 17:17, Andres Freund wrote:
>
>
> On May 27, 2017 9:48:22 AM EDT, Petr Jelinek
  wrote:
>> Actually, I guess it's the pid 47457 (COPY process) who is
>> actually
>> running the xid 73322726. In that case that's the same thing
 Masahiko
>> Sawada reported [1]. Which basically is result of snapshot builder
>> waiting for transaction to finish, that's normal if there is a
>> long
>> transaction running when the snapshot is being created (and the
>> COPY
 is
>> a long transaction).
>
> Hm.  I suspect the issue is that the exported snapshot needs an xid
 for some crosscheck, and that's what we're waiting for.  Could you
 check what happens if you don't assign one and just content the
>> error
 checks out?   Not at my computer, just theorizing.
>

 I don't think that's it, in my opinion it's the parallelization of
 table
 data copy where we create snapshot for one process but then the next
 one
 has to wait for the first one to finish. Before we fixed the
 snapshotting, the second one would just use the ondisk snapshot so
>> it
 would work fine (except the snapshot was corrupted of course). I
>> wonder
 if we could somehow give it a hint to ignore the read-only txes, but
 then we have no way to enforce the txes to stay read-only so it does
 not
 seem safe.
>>>
>>> Read-only txs have no xid ...
>>>
>>
>> That's what I mean by hinting, normally they don't but building initial
>> snapshot in snapshot builder calls GetTopTransactionId() (see
>> SnapBuildInitialSnapshot()) which will assign it xid.
> 
> That's precisely what I pointed out a few emails above, and what I suggest 
> changing.
> 

Ah didn't realize that's what you meant. I can try playing with it.

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


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

Re: [HACKERS] logical replication busy-waiting on a lock

[Andres Freund]

On May 29, 2017 12:25:35 PM PDT, Petr Jelinek  
wrote:
>On 29/05/17 21:21, Petr Jelinek wrote:
>> On 29/05/17 20:59, Andres Freund wrote:
>>>
>>>
>>> On May 29, 2017 11:58:05 AM PDT, Petr Jelinek
> wrote:
 On 27/05/17 17:17, Andres Freund wrote:
>
>
> On May 27, 2017 9:48:22 AM EDT, Petr Jelinek
  wrote:
>> Actually, I guess it's the pid 47457 (COPY process) who is
>actually
>> running the xid 73322726. In that case that's the same thing
 Masahiko
>> Sawada reported [1]. Which basically is result of snapshot
>builder
>> waiting for transaction to finish, that's normal if there is a
>long
>> transaction running when the snapshot is being created (and the
>COPY
 is
>> a long transaction).
>
> Hm.  I suspect the issue is that the exported snapshot needs an
>xid
 for some crosscheck, and that's what we're waiting for.  Could you
 check what happens if you don't assign one and just content the
>error
 checks out?   Not at my computer, just theorizing.
>

 I don't think that's it, in my opinion it's the parallelization of
 table
 data copy where we create snapshot for one process but then the
>next
 one
 has to wait for the first one to finish. Before we fixed the
 snapshotting, the second one would just use the ondisk snapshot so
>it
 would work fine (except the snapshot was corrupted of course). I
>wonder
 if we could somehow give it a hint to ignore the read-only txes,
>but
 then we have no way to enforce the txes to stay read-only so it
>does
 not
 seem safe.
>>>
>>> Read-only txs have no xid ...
>>>
>> 
>> That's what I mean by hinting, normally they don't but building
>initial
>> snapshot in snapshot builder calls GetTopTransactionId() (see
>> SnapBuildInitialSnapshot()) which will assign it xid.
>> 
>
>Looking at the code more, the xid is only used as parameter for
>SnapBuildBuildSnapshot() which never does anything with that parameter,
>I wonder if it's really needed then.

Not at a computer, but by memory that'll trigger the snapshot export routine to 
include it.  Import in turn requires the xid to check if the source is still 
alive.  But there's better ways, e.g. using the virtual xactid.

Andres
-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.


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

Re: [HACKERS] Surjective functional indexes

[Sven R. Kunze]

On 29.05.2017 21:25, Sven R. Kunze wrote:

[...] non-surjective functions.

non-injective of course


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

Re: [HACKERS] logical replication busy-waiting on a lock

[Petr Jelinek]

On 29/05/17 21:21, Petr Jelinek wrote:
> On 29/05/17 20:59, Andres Freund wrote:
>>
>>
>> On May 29, 2017 11:58:05 AM PDT, Petr Jelinek  
>> wrote:
>>> On 27/05/17 17:17, Andres Freund wrote:


 On May 27, 2017 9:48:22 AM EDT, Petr Jelinek
>>>  wrote:
> Actually, I guess it's the pid 47457 (COPY process) who is actually
> running the xid 73322726. In that case that's the same thing
>>> Masahiko
> Sawada reported [1]. Which basically is result of snapshot builder
> waiting for transaction to finish, that's normal if there is a long
> transaction running when the snapshot is being created (and the COPY
>>> is
> a long transaction).

 Hm.  I suspect the issue is that the exported snapshot needs an xid
>>> for some crosscheck, and that's what we're waiting for.  Could you
>>> check what happens if you don't assign one and just content the error
>>> checks out?   Not at my computer, just theorizing.

>>>
>>> I don't think that's it, in my opinion it's the parallelization of
>>> table
>>> data copy where we create snapshot for one process but then the next
>>> one
>>> has to wait for the first one to finish. Before we fixed the
>>> snapshotting, the second one would just use the ondisk snapshot so it
>>> would work fine (except the snapshot was corrupted of course). I wonder
>>> if we could somehow give it a hint to ignore the read-only txes, but
>>> then we have no way to enforce the txes to stay read-only so it does
>>> not
>>> seem safe.
>>
>> Read-only txs have no xid ...
>>
> 
> That's what I mean by hinting, normally they don't but building initial
> snapshot in snapshot builder calls GetTopTransactionId() (see
> SnapBuildInitialSnapshot()) which will assign it xid.
> 

Looking at the code more, the xid is only used as parameter for
SnapBuildBuildSnapshot() which never does anything with that parameter,
I wonder if it's really needed then.

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


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

Re: [HACKERS] Use of non-restart-safe storage by temp_tablespaces

[Andres Freund]

On May 29, 2017 12:15:37 PM PDT, Alvaro Herrera  
wrote:
>Claudio Freire wrote:
>> On Mon, May 29, 2017 at 3:53 PM, Bruce Momjian 
>wrote:
>> > Right now we don't document that temp_tablespaces can use
>> > non-restart-safe storage, e.g. /tmp, ramdisks.  Would this be safe?
>> > Should we document this?
>> 
>> I have set up things like that, but it's nontrivial.
>
>I think it'd be smart to support the use case directly, because there's
>interest in it being actually supported (unlike the statu quo).
>Something like restoring the tablespace to the empty state on boot, if
>it's known to need it.

Has the danger of making recovery harder after a restart where somebody forgot 
to mount some subdirectory ...

Andres
-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.


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

Re: [HACKERS] Surjective functional indexes

[Sven R. Kunze]

On 29.05.2017 19:21, Christoph Berg wrote:

I think the term you were looking for is "projection".
https://en.wikipedia.org/wiki/Projection_(set_theory)


Maybe, I am seeing too much of a connection here but recently Raymond 
Hettinger held a very interesting talk [1] at PyCon 2017.


For those without the time or bandwidth to watch: it describes the 
history of the modern dict in Python in several steps.


1) avoiding having a database scheme with columns and rows and indexes
2) introducing hashing with bucket lists
3...6) several improvements
7) in the end looks like a database table with indexes again ;)

If you have the time, just go ahead and watch the 30 min video. He can 
explain things definitely better than me.



In order to draw the line back on-topic, if I am not completely 
mistaken, his talks basically shows that over time even datastructures 
with different APIs such as dicts (hashes, maps, sets, etc.) internally 
converge towards a relational-database-y design because of performance 
and resources reasons.



Thus let me think that also in the on-topic case, we might best be 
supporting the much narrow use-case of "Projection" (a term also used in 
relation database theory btw. ;-) ) instead of non-surjective functions.



Cheers,
Sven


[1] https://www.youtube.com/watch?v=npw4s1QTmPg


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

Re: [HACKERS] logical replication busy-waiting on a lock

[Andres Freund]

On May 29, 2017 12:21:50 PM PDT, Petr Jelinek  
wrote:
>On 29/05/17 20:59, Andres Freund wrote:
>> 
>> 
>> On May 29, 2017 11:58:05 AM PDT, Petr Jelinek
> wrote:
>>> On 27/05/17 17:17, Andres Freund wrote:


 On May 27, 2017 9:48:22 AM EDT, Petr Jelinek
>>>  wrote:
> Actually, I guess it's the pid 47457 (COPY process) who is
>actually
> running the xid 73322726. In that case that's the same thing
>>> Masahiko
> Sawada reported [1]. Which basically is result of snapshot builder
> waiting for transaction to finish, that's normal if there is a
>long
> transaction running when the snapshot is being created (and the
>COPY
>>> is
> a long transaction).

 Hm.  I suspect the issue is that the exported snapshot needs an xid
>>> for some crosscheck, and that's what we're waiting for.  Could you
>>> check what happens if you don't assign one and just content the
>error
>>> checks out?   Not at my computer, just theorizing.

>>>
>>> I don't think that's it, in my opinion it's the parallelization of
>>> table
>>> data copy where we create snapshot for one process but then the next
>>> one
>>> has to wait for the first one to finish. Before we fixed the
>>> snapshotting, the second one would just use the ondisk snapshot so
>it
>>> would work fine (except the snapshot was corrupted of course). I
>wonder
>>> if we could somehow give it a hint to ignore the read-only txes, but
>>> then we have no way to enforce the txes to stay read-only so it does
>>> not
>>> seem safe.
>> 
>> Read-only txs have no xid ...
>> 
>
>That's what I mean by hinting, normally they don't but building initial
>snapshot in snapshot builder calls GetTopTransactionId() (see
>SnapBuildInitialSnapshot()) which will assign it xid.

That's precisely what I pointed out a few emails above, and what I suggest 
changing.

Andres
-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.


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

Re: [HACKERS] logical replication busy-waiting on a lock

[Petr Jelinek]

On 29/05/17 20:59, Andres Freund wrote:
> 
> 
> On May 29, 2017 11:58:05 AM PDT, Petr Jelinek  
> wrote:
>> On 27/05/17 17:17, Andres Freund wrote:
>>>
>>>
>>> On May 27, 2017 9:48:22 AM EDT, Petr Jelinek
>>  wrote:
 Actually, I guess it's the pid 47457 (COPY process) who is actually
 running the xid 73322726. In that case that's the same thing
>> Masahiko
 Sawada reported [1]. Which basically is result of snapshot builder
 waiting for transaction to finish, that's normal if there is a long
 transaction running when the snapshot is being created (and the COPY
>> is
 a long transaction).
>>>
>>> Hm.  I suspect the issue is that the exported snapshot needs an xid
>> for some crosscheck, and that's what we're waiting for.  Could you
>> check what happens if you don't assign one and just content the error
>> checks out?   Not at my computer, just theorizing.
>>>
>>
>> I don't think that's it, in my opinion it's the parallelization of
>> table
>> data copy where we create snapshot for one process but then the next
>> one
>> has to wait for the first one to finish. Before we fixed the
>> snapshotting, the second one would just use the ondisk snapshot so it
>> would work fine (except the snapshot was corrupted of course). I wonder
>> if we could somehow give it a hint to ignore the read-only txes, but
>> then we have no way to enforce the txes to stay read-only so it does
>> not
>> seem safe.
> 
> Read-only txs have no xid ...
> 

That's what I mean by hinting, normally they don't but building initial
snapshot in snapshot builder calls GetTopTransactionId() (see
SnapBuildInitialSnapshot()) which will assign it xid.

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


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

Re: [HACKERS] Use of non-restart-safe storage by temp_tablespaces

[Alvaro Herrera]

Claudio Freire wrote:
> On Mon, May 29, 2017 at 3:53 PM, Bruce Momjian  wrote:
> > Right now we don't document that temp_tablespaces can use
> > non-restart-safe storage, e.g. /tmp, ramdisks.  Would this be safe?
> > Should we document this?
> 
> I have set up things like that, but it's nontrivial.

I think it'd be smart to support the use case directly, because there's
interest in it being actually supported (unlike the statu quo).
Something like restoring the tablespace to the empty state on boot, if
it's known to need it.

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


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

Re: [HACKERS] psql: Activate pager only for height, not width

[Jeff Janes]

On Sun, May 28, 2017 at 10:09 PM, Brendan Jurd  wrote:

> Hello hackers,
>
> I am often frustrated by the default behaviour of the psql pager, which
> will activate a pager if the output is deemed to be "too wide" for the
> terminal, regardless of the number of lines output, and of the
> pager_min_lines setting.
>
> This behaviour is sometimes desirable, but in my use patterns it is more
> often the case that I want the pager to activate for output longer than
> terminal height, whereas for output a little wider than the terminal, I am
> happy for there to be some wrapping.  This is especially the case with "\d"
> output for tables, where, at 80 columns, very often the only wrapping is in
> the table borders and constraint/trigger definitions.
>
> Usually I turn the pager off completely, and only switch it on when I am
> about to execute something that will return many rows, but what I'd really
> like is some way to tell psql to activate the pager as normal for height,
> but to ignore width.  My first thought was an alternate mode to \pset pager
> -- to {'on' | 'off' | 'always'} we could add 'height'.
>
> Another option is to add the ability to specify the number of columns
> which psql considers "too wide", analogous to pager_min_lines.  I could
> then set pager_min_cols to something around 150 which would work nicely for
> my situation.
>
> I don't have strong opinions about how the options are constructed, as
> long as it is possible to obtain the behaviour.
>
> I would be happy to produce a patch, if this seems like an acceptable
> feature add.
>

I'd like a feature like this.  I often run into the problem where one or
two lines of an EXPLAIN plan are wide enough to wrap, which causes the
pager to kick in.  Then when I exit the pager, it clears the contents so I
can't see that plan vertically adjacent to the one I'm trying to compare it
to.  I'd rather have the pager kick in if greater than some settable number
of the lines are too wide, or if the wrapped lines would push the height
above the height limit.  If just one line is too wide, I'd rather just deal
with them being wrapped.

(You can configure the pager not to redraw the screen when exited, but I
want it to redraw the screen after looking at 10,000 rows, just not after
looking ten rows, one of which was 170 characters wide)

Cheers,

Jeff

Re: [HACKERS] Use of non-restart-safe storage by temp_tablespaces

[Claudio Freire]

On Mon, May 29, 2017 at 3:53 PM, Bruce Momjian  wrote:
> Right now we don't document that temp_tablespaces can use
> non-restart-safe storage, e.g. /tmp, ramdisks.  Would this be safe?
> Should we document this?

I have set up things like that, but it's nontrivial.

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.

Or just use the tablespaces, I forget which. But in essence, it won't
do to do just that. What I ended up doing was backing up the empty
tablespace and adding a restore to system bootup scripts, so that
after a restart postgres finds an empty tablespace there.

So, there's a lot of internal hackery to document if you'd like to
document that kind of usage.


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

Re: [HACKERS] logical replication busy-waiting on a lock

[Andres Freund]

On May 29, 2017 11:58:05 AM PDT, Petr Jelinek  
wrote:
>On 27/05/17 17:17, Andres Freund wrote:
>> 
>> 
>> On May 27, 2017 9:48:22 AM EDT, Petr Jelinek
> wrote:
>>> Actually, I guess it's the pid 47457 (COPY process) who is actually
>>> running the xid 73322726. In that case that's the same thing
>Masahiko
>>> Sawada reported [1]. Which basically is result of snapshot builder
>>> waiting for transaction to finish, that's normal if there is a long
>>> transaction running when the snapshot is being created (and the COPY
>is
>>> a long transaction).
>> 
>> Hm.  I suspect the issue is that the exported snapshot needs an xid
>for some crosscheck, and that's what we're waiting for.  Could you
>check what happens if you don't assign one and just content the error
>checks out?   Not at my computer, just theorizing.
>> 
>
>I don't think that's it, in my opinion it's the parallelization of
>table
>data copy where we create snapshot for one process but then the next
>one
>has to wait for the first one to finish. Before we fixed the
>snapshotting, the second one would just use the ondisk snapshot so it
>would work fine (except the snapshot was corrupted of course). I wonder
>if we could somehow give it a hint to ignore the read-only txes, but
>then we have no way to enforce the txes to stay read-only so it does
>not
>seem safe.

Read-only txs have no xid ...
-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.


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

Re: [HACKERS] logical replication busy-waiting on a lock

[Petr Jelinek]

On 27/05/17 17:17, Andres Freund wrote:
> 
> 
> On May 27, 2017 9:48:22 AM EDT, Petr Jelinek  
> wrote:
>> Actually, I guess it's the pid 47457 (COPY process) who is actually
>> running the xid 73322726. In that case that's the same thing Masahiko
>> Sawada reported [1]. Which basically is result of snapshot builder
>> waiting for transaction to finish, that's normal if there is a long
>> transaction running when the snapshot is being created (and the COPY is
>> a long transaction).
> 
> Hm.  I suspect the issue is that the exported snapshot needs an xid for some 
> crosscheck, and that's what we're waiting for.  Could you check what happens 
> if you don't assign one and just content the error checks out?   Not at my 
> computer, just theorizing.
> 

I don't think that's it, in my opinion it's the parallelization of table
data copy where we create snapshot for one process but then the next one
has to wait for the first one to finish. Before we fixed the
snapshotting, the second one would just use the ondisk snapshot so it
would work fine (except the snapshot was corrupted of course). I wonder
if we could somehow give it a hint to ignore the read-only txes, but
then we have no way to enforce the txes to stay read-only so it does not
seem safe.

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


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

[HACKERS] Use of non-restart-safe storage by temp_tablespaces

[Bruce Momjian]

Right now we don't document that temp_tablespaces can use
non-restart-safe storage, e.g. /tmp, ramdisks.  Would this be safe? 
Should we document this?

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

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


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

Re: [HACKERS] fix side-effect in get_qual_for_list()

[Jeevan Ladhe]

On Mon, May 29, 2017 at 11:55 PM, Tom Lane  wrote:

> Jeevan Ladhe  writes:
> > I have rebased the patch on recent commit.
>
> Pushed with some further tweaking.
>

Thanks Tom for taking care of this.

Regards,
Jeevan Ladhe

Re: [HACKERS] logical replication busy-waiting on a lock

[Andres Freund]

On 2017-05-29 11:38:20 -0700, Jeff Janes wrote:
> Related, but not the same.  It would be nice if they didn't block, but if
> they do have to block, shouldn't it wait on a semaphore, rather than doing
> a tight loop?  It looks like maybe a latch didn't get reset when it should
> have or something.

The code certainly is trying to just block using a lock (on the xid of
the running xact), there shouldn't be any busy looping going on...
There's no latch involved, so something is certainly weird here.

- Andres


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

Re: [HACKERS] logical replication busy-waiting on a lock

[Jeff Janes]

On Sat, May 27, 2017 at 6:48 AM, Petr Jelinek 
wrote:

>
>
> Actually, I guess it's the pid 47457 (COPY process) who is actually
> running the xid 73322726. In that case that's the same thing Masahiko
> Sawada reported [1].


Related, but not the same.  It would be nice if they didn't block, but if
they do have to block, shouldn't it wait on a semaphore, rather than doing
a tight loop?  It looks like maybe a latch didn't get reset when it should
have or something.


[1]
> https://www.postgresql.org/message-id/flat/CAD21AoC2KJdavS7MFffmSsRc1dn3V
> g_0xmuc%3DUpBrZ-_MUxh-Q%40mail.gmail.com


Cheers,

Jeff

Re: [HACKERS] fix side-effect in get_qual_for_list()

[Tom Lane]

Jeevan Ladhe  writes:
> I have rebased the patch on recent commit.

Pushed with some further tweaking.

regards, tom lane


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

Re: [HACKERS] Surjective functional indexes

[Christoph Berg]

Re: Konstantin Knizhnik 2017-05-29 <592bbd90.3060...@postgrespro.ru>
> On 05/27/2017 09:50 PM, Peter Eisentraut wrote:
> > On 5/25/17 12:30, Konstantin Knizhnik wrote:
> > > Functions like (info->>'name') are named "surjective" ni mathematics.
> > A surjective function is one where each value in the output type can be
> > obtained by some input value.  That's not what you are after here.  The
> > behavior you are describing is a not-injective function.
> > 
> > I think you are right that in practice most functions are not injective.
> >   But I think there is still quite some difference between a function
> > like the one you showed that selects a component from a composite data
> > structure and, for example, round(), where in practice any update is
> > likely to change the result of the function.
> > 
> Thank you, I will rename "surjective" parameter to "injective" with "false" 
> as default value.

I think the term you were looking for is "projection".

https://en.wikipedia.org/wiki/Projection_(set_theory)

Christoph


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

Re: [HACKERS] pg_resetwal is broken if run from v10 against older version of PG data directory

[Michael Paquier]

On Mon, May 29, 2017 at 10:02 AM, Tom Lane  wrote:
> Michael Paquier  writes:
>> On Mon, May 29, 2017 at 9:00 AM, Tom Lane  wrote:
>>> So we need to prevent this, not try to make it work.  I don't think
>>> we can insist on a version match in pg_control, because part of the
>>> point of pg_resetxlog/pg_resetwal is to recover if pg_control is
>>> unreadable.  But I think we could look at PG_VERSION, which is only a
>>> text file.
>
>> Agreed. Shouldn't this be back-patched? PG_CONTROL_VERSION has not
>> been bumped between 9.4 and 9.5. Attached is a patch for HEAD.
>
> Yeah, I'm thinking it would be a good idea to enforce this in all
> branches.  Your patch looks sane in a quick once-over, but I didn't
> test it.

Thanks. I can provide patches for back-branches as needed.
-- 
Michael


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

Re: [HACKERS] pg_resetwal is broken if run from v10 against older version of PG data directory

[Tom Lane]

Michael Paquier  writes:
> On Mon, May 29, 2017 at 9:00 AM, Tom Lane  wrote:
>> So we need to prevent this, not try to make it work.  I don't think
>> we can insist on a version match in pg_control, because part of the
>> point of pg_resetxlog/pg_resetwal is to recover if pg_control is
>> unreadable.  But I think we could look at PG_VERSION, which is only a
>> text file.

> Agreed. Shouldn't this be back-patched? PG_CONTROL_VERSION has not
> been bumped between 9.4 and 9.5. Attached is a patch for HEAD.

Yeah, I'm thinking it would be a good idea to enforce this in all
branches.  Your patch looks sane in a quick once-over, but I didn't
test it.

regards, tom lane


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

Re: [HACKERS] pg_resetwal is broken if run from v10 against older version of PG data directory

[Michael Paquier]

On Mon, May 29, 2017 at 9:00 AM, Tom Lane  wrote:
> So we need to prevent this, not try to make it work.  I don't think
> we can insist on a version match in pg_control, because part of the
> point of pg_resetxlog/pg_resetwal is to recover if pg_control is
> unreadable.  But I think we could look at PG_VERSION, which is only a
> text file.  In bad corruption scenarios, if that somehow got corrupted
> (which seems unlikely since it's never written to post-initdb),
> you could fill in the correct contents by hand and then
> pg_resetxlog/pg_resetwal would run.

Agreed. Shouldn't this be back-patched? PG_CONTROL_VERSION has not
been bumped between 9.4 and 9.5. Attached is a patch for HEAD.
-- 
Michael


resetwal-restrict.patch
Description: Binary data

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

Re: [HACKERS] Adding support for Default partition in partitioning

[Jeevan Ladhe]

Hi,

I have rebased the patch on latest commit with few cosmetic changes.

The patch fix_listdatums_get_qual_for_list_v3.patch [1]

 needs to be applied
before applying this patch.

[1] http://www.mail-archive.com/pgsql-hackers@postgresql.org/msg315490.html

Regards,
Jeevan Ladhe


On Mon, May 29, 2017 at 2:28 PM, Jeevan Ladhe  wrote:

>
>
>> The existing comment is not valid
>> /*
>>  * A null partition key is only acceptable if null-accepting
>> list
>>  * partition exists.
>>  */
>> as we allow NULL to be stored in default. It should be updated.
>>
>
> Sure Beena, as stated earlier will update this on my next version of patch.
>
>
> Regards,
> Jeevan Ladhe
>


default_partition_v15.patch
Description: Binary data

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

Re: [HACKERS] pg_resetwal is broken if run from v10 against older version of PG data directory

[Tom Lane]

Amit Kapila  writes:
> I think this happens due to commit
> f82ec32ac30ae7e3ec7c84067192535b2ff8ec0e which renames pg_xlog to
> pg_wal.  It does take care of making some of the modules like
> pg_basebackup to understand both old and new directory structures, but
> not done for all the modules.

check

> I think we should make similar changes
> in pg_resetwal or at the very least update the docs to indicate
> pg_resetwal can give an error if used against pre-10 data directory.

I think it's just horribly dangerous to run any version of
pg_resetxlog/pg_resetwal against any major version other than its
own.  If people have been doing what the OP tried to do, it's only
been sheerest luck if it didn't destroy their database beyond recall.
The result will certainly fail to start up, because both pg_control
and WAL page header versions will not match the server version.  The
*best* case scenario is that redoing the reset with the correct version
of pg_resetxlog/pg_resetwal gets you out of that with no new damage
done.  The worst case is pretty awful --- for instance, a truly
bullheaded user might try to get out of it by starting the newer server
version in the old DB, likely causing irrecoverable catalog corruption.

So we need to prevent this, not try to make it work.  I don't think
we can insist on a version match in pg_control, because part of the
point of pg_resetxlog/pg_resetwal is to recover if pg_control is
unreadable.  But I think we could look at PG_VERSION, which is only a
text file.  In bad corruption scenarios, if that somehow got corrupted
(which seems unlikely since it's never written to post-initdb),
you could fill in the correct contents by hand and then
pg_resetxlog/pg_resetwal would run.

regards, tom lane


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

Re: [HACKERS] [PATCH] relocation truncated to fit: citus build failure on s390x

[Christoph Berg]

Re: To Andres Freund 2016-04-28 <20160428080824.ga22...@msg.df7cb.de>
> > I'm not clear why citus triggers this, when other extensions don't?
> 
> Maybe it's simply because citus.so is bigger than all the other
> extension .so files:
> 
>-fpic
>  Generate position-independent code (PIC) suitable for use
>  in a shared library, if supported for the target machine.
>  Such code accesses all constant addresses through a global
>  offset table (GOT).  The dynamic loader resolves the GOT
>  entries when the program starts (the dynamic loader is not
>  part of GCC; it is part of the operating system).  If the
>  GOT size for the linked executable exceeds a machine-
>  specific maximum size, you get an error message from the
>  linker indicating that -fpic does not work; in that case,
>  recompile with -fPIC instead.  (These maximums are 8k on
>  the SPARC and 32k on the m68k and RS/6000.  The 386 has no
>  such limit.)
> 
>  Position-independent code requires special support, and
>  therefore works only on certain machines.  For the 386, GCC
>  supports PIC for System V but not for the Sun 386i.  Code
>  generated for the IBM RS/6000 is always
>  position-independent.
> 
>  When this flag is set, the macros "__pic__" and "__PIC__"
>  are defined to 1.
> 
>-fPIC
>  If supported for the target machine, emit
>  position-independent code, suitable for dynamic linking and
>  avoiding any limit on the size of the global offset table.
>  This option makes a difference on the m68k, PowerPC and
>  SPARC.
> 
>  Position-independent code requires special support, and
>  therefore works only on certain machines.
> 
>  When this flag is set, the macros "__pic__" and "__PIC__"
>  are defined to 2.
> 
> This doesn't mention s390(x), but citus.so 382952 bytes (on amd64) is
> well beyond the 8k/32k limits mentioned above.
> 
> PostgreSQL itself links correctly on s390x:
> ... -I/usr/include/mit-krb5 -fPIC -pie -I../../../../src/include
> 
> I'm not an expert in compiler flags, but it seems to me CFLAGS_SL is
> wrong on s390(x) in Makefile.port, it should use -fPIC like sparc.

After talking to a s390x Debian porter, -fPIC is the correct flag to
use. The quoted patch made the previously failing builds for citus and
pglogical (which have larger-than-average .so files) on s390x succeed
(and the sparc64 case still works):

--- a/src/makefiles/Makefile.linux
+++ b/src/makefiles/Makefile.linux
@@ -5,7 +5,8 @@ export_dynamic = -Wl,-E
 rpath = -Wl,-rpath,'$(rpathdir)',--enable-new-dtags
 DLSUFFIX = .so
 
-ifeq "$(findstring sparc,$(host_cpu))" "sparc"
+# Enable -fPIC to avoid "relocation truncated to fit" linker errors
+ifneq "$(filter sparc% s390%,$(host_cpu))" ""
 CFLAGS_SL = -fPIC
 else
 CFLAGS_SL = -fpic


The patch was made against 9.6; I'd opt to include it in master and
the back branches.

https://buildd.debian.org/status/logs.php?pkg=citus=s390x
https://buildd.debian.org/status/logs.php?pkg=pglogical=s390x

Christoph


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

Re: [HACKERS] pg_resetwal is broken if run from v10 against older version of PG data directory

[Michael Paquier]

On Mon, May 29, 2017 at 3:19 AM, Amit Kapila  wrote:
> I think this happens due to commit
> f82ec32ac30ae7e3ec7c84067192535b2ff8ec0e which renames pg_xlog to
> pg_wal.  It does take care of making some of the modules like
> pg_basebackup to understand both old and new directory structures, but
> not done for all the modules.  I think we should make similar changes
> in pg_resetwal or at the very least update the docs to indicate
> pg_resetwal can give an error if used against pre-10 data directory.

Contrary to pg_basebackup which makes clear in its documentation that
it is compatible with past server versions down to 9.1, pg_resetwal
does not mention such guarantees. And actually, it seems to me that it
is rather unsafe to use it across major versions as the size of
ControlFileData varies across major versions so you can write junk
bytes in the control file by using pg_resetwal from v10 on a 9.6
server.
-- 
Michael


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

Re: [HACKERS] pg_resetwal is broken if run from v10 against older version of PG data directory

[Tom Lane]

tushar  writes:
> I have installed PG v9.6 / v9.5 , if we run pg_resetwal from v10 
> binaries against data directory of v9.6/9.5 ,getting this error -

> centos@centos-cpula bin]$ ./pg_resetwal -D /tmp/pg9.6/bin/data/
> pg_resetwal: pg_control exists but is broken or unknown version; ignoring it
> pg_resetwal: could not open directory "pg_wal": No such file or directory

This is pretty much exactly what I'd expect.  Why would you think
that pg_resetwal should work with other major versions?

(Maybe we should have it look at PG_VERSION and refuse to do anything
if that doesn't have the expected contents.)

regards, tom lane


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

Re: [HACKERS] Extra Vietnamese unaccent rules

[Dang Minh Huong]

> On May 29, 29 Heisei, at 10:47, Thomas Munro  
> wrote:
> 
> On Sun, May 28, 2017 at 7:55 PM, Dang Minh Huong  wrote:
>> Thanks for reporting and lecture about unicode.
>> I attached a patch as the instruction from Thomas. Could you confirm it.
> 
> -   is_plain_letter(table[codepoint.combining_ids[0]]) and \
> +   (is_plain_letter(table[codepoint.combining_ids[0]]) or\
> +len(table[codepoint.combining_ids[0]].combining_ids) > 1) and \
> 
> Shouldn't you use "or is_letter_with_marks()", instead of "or len(...)
>> 1"?  Your test might catch something that isn't based on a 'letter'
> (according to is_plain_letter).  Otherwise this looks pretty good to
> me.  Please add it to the next commitfest.

Thanks for confirm, sir.
I will add it to the next CF soon.

> I expect that some users in Vietnam will consider this to be a bugfix,
> which raises the question of whether to backpatch it.  Perhaps we
> could consider fixing it for 10.  Then users of older versions could
> grab the rules file from 10 to use with 9.whatever if they want to do
> that and reindex their data as appropriate.

I am also inclined to the fixing it for 10, because it will not affect to 
current users.
But do you want to back-patch to all supported versions Kha Nguyen?
# I would also want to note that, not only Vietnamese characters were missed to 
add from the rule list.


---
Thanks and best regards,
Dang Minh Huong

Re: [HACKERS] Fix a typo in execExpr.c

[Magnus Hagander]

On Mon, May 29, 2017 at 4:04 PM, Masahiko Sawada 
wrote:

> Hi,
>
> Attached for $subject.
>
> s/Expession/Expression/
>

Thanks, applied!

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

[HACKERS] Fix a typo in execExpr.c

[Masahiko Sawada]

Hi,

Attached for $subject.

s/Expession/Expression/

Regards,

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


fix_typo_in_execExpr_c.patch
Description: Binary data

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

Re: [HACKERS] Inconsistent syntax for NumericOnly grammar production

[Vik Fearing]

On 05/28/2017 11:16 PM, Tom Lane wrote:
> The inconsistency here means that you can do, for example,
> 
> regression=# set random_page_cost = +4;
> SET
> regression=# set random_page_cost = 4.2;
> SET
> 
> but not
> 
> regression=# set random_page_cost = +4.2;
> ERROR:  syntax error at or near "4.2"
> LINE 1: set random_page_cost = +4.2;
> ^
> Any objections to allowing "+ FCONST" here?  I'm inclined to
> fix this and back-patch it as well.

Seems like the right thing to do; no objections here.
-- 
Vik Fearing  +33 6 46 75 15 36
http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support


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

[HACKERS] question about replication docs

[Dave Cramer]

The following makes an explicit reference to the simple query protocol
being the only protocol allowed in walsender mode. It is my understanding
this is true for logical replication as well ??


51.3. Streaming Replication Protocol
To initiate streaming replication, the frontend sends the replication
parameter in the startup message. A Boolean value of true tells the backend
to go into walsender mode, wherein a small set of replication commands can
be issued instead of SQL statements. Only the simple query protocol can be
used in walsender mode. Replication commands are logged in the server log
when log_replication_commands is enabled. Passing database as the value
instructs walsender to connect to the database specified in the dbname
parameter, which will allow the connection to be used for logical
replication from that database.


Dave Cramer

Re: [HACKERS] logical replication - still unstable after all these months

[Petr Jelinek]

On 29/05/17 03:33, Jeff Janes wrote:
> On Sun, May 28, 2017 at 3:17 PM, Mark Kirkwood
> >
> wrote:
> 
> The framework ran 600 tests last night, and I see 3 'NOK' results,
> i.e 3 failed test runs (all scale 25 and 8 pgbench clients). Given
> the way the test decides on failure (gets tired of waiting for the
> table md5's to match) - it begs the question 'What if it had waited
> a bit longer'? However from what I can see in all cases:
> 
> - the rowcounts were the same in master and replica
> - the md5 of pgbench_accounts was different
> 
> 
> All four tables should be wrong if there is still a transaction it is
> waiting for, as all the changes happen in a single transaction.  

Not necessarily, if the bug is in the sync worker or in the sync to
apply handover code (which is one of the more complicated parts of all
of the logical replication, so it's prime candidate) then it can easily
be just one table.

> I also got a failure, after 87 iterations of a similar test case.  It
> waited for hours, as mine requires manual intervention to stop waiting. 
> On the subscriber, one account still had a zero balance, while the
> history table on the subscriber agreed with both history and accounts on
> the publisher and the account should not have been zero, so definitely a
> transaction atomicity got busted.

I am glad others are able to reproduce this, my machine is still at 0
failures after 800 cycles.

> 
> What would you want to look at?  Would saving the WAL from the master be
> helpful?
> 

Useful info is, logs from provider (mainly the logical decoding logs
that mention LSNs), logs from subscriber (the lines about when sync
workers finished), contents of the pg_subscription_rel (with srrelid
casted to regclass so we know which table is which), and pg_waldump
output around the LSNs found in the logs and in the pg_subscription_rel
(+ few lines before and some after to get context). It's enough to only
care about LSNs for the table(s) that are out of sync.

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


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

Re: [HACKERS] fix side-effect in get_qual_for_list()

[Jeevan Ladhe]

Hi,

I have rebased the patch on recent commit.

With recent commits, some of the hunks in the v2 patch related to
castNode, are not needed.

PFA.

Regards,
Jeevan Ladhe

On Sat, May 27, 2017 at 1:16 AM, Jeevan Ladhe  wrote:

> Hi Ashutosh,
>
> Thanks for catching this. For now this isn't a problem since
>> generate_partition_qual() is crafting PartitionBoundInfo which it
>> doesn't use anywhere else. But if the function gets used where the
>> PartitionBoundSpec is being used somewhere else as well.
>
>
> Yes, this behavior currently does not affect adversely, but I think this
> function is quite useful for future enhancements and should be fixed.
>
> While you are
>> at it, can we use castNode() in place of
>> PartitionBoundSpec *spec = (PartitionBoundSpec *) bound; Or do you
>> think it should be done separately?
>>
>
> I have made this change at couple of places applicable.
>
> PFA.
>
> Regards,
> Jeevan Ladhe
>


fix_listdatums_get_qual_for_list_v3.patch
Description: Binary data

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

Re: [HACKERS] pg_resetwal is broken if run from v10 against older version of PG data directory

[Amit Kapila]

On Mon, May 29, 2017 at 3:20 PM, tushar  wrote:
> On 05/29/2017 03:10 PM, Amit Kapila wrote:
>>
>>   What makes you think above is a valid usage and should
>> pass?
>
> with earlier versions ,for instance - v.96 v/s v9.5 ,pg_resetwal was giving
> pg_control values .
>
> Installed v9.6 and v9.5  and run pg_resetwal of v9.6 against data directory
> of v9.5.
>
> [centos@centos-cpula ~]$ /tmp/pg9.6/bin/pg_resetxlog -D /tmp/pg9.5/bin/data/
> pg_resetxlog: pg_control exists but is broken or unknown version; ignoring
> it
> Guessed pg_control values:
>

I think this happens due to commit
f82ec32ac30ae7e3ec7c84067192535b2ff8ec0e which renames pg_xlog to
pg_wal.  It does take care of making some of the modules like
pg_basebackup to understand both old and new directory structures, but
not done for all the modules.  I think we should make similar changes
in pg_resetwal or at the very least update the docs to indicate
pg_resetwal can give an error if used against pre-10 data directory.

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


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

Re: retry shm attach for windows (WAS: Re: [HACKERS] OK, so culicidae is *still* broken)

[Amit Kapila]

On Fri, May 26, 2017 at 8:21 PM, Magnus Hagander  wrote:
>
> On Fri, May 26, 2017 at 8:24 AM, Michael Paquier 
> wrote:
>>
>> On Fri, May 26, 2017 at 8:20 AM, Amit Kapila 
>> wrote:
>> > I think the real question here is, shall we backpatch this fix or we
>> > want to do this just in Head or we want to consider it as a new
>> > feature for PostgreSQL-11.  I think it should be fixed in Head and the
>> > change seems harmless to me, so we should even backpatch it.
>>
>> The thing is not invasive, so backpatching is a low-risk move. We can
>> as well get that into HEAD first, wait a bit for dust to settle on it,
>> and then backpatch.
>
>
>
> I would definitely suggest putting it in HEAD (and thus, v10) for a while to
> get some real world exposure before backpatching.
>

make sense to me, so I have added an entry in "Older Bugs" section in
PostgreSQL 10 Open Items.


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


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

Re: [HACKERS] Allow GiST opcalsses without compress\decompres functions

[Andrew Borodin]

2017-05-29 0:00 GMT+05:00 Tom Lane :
> But the opclass's consistent function might expect to be always given an
> untoasted input, in which case you'd need the decompress function to fix
> that up.

In cube data is detoasted at least 4 times before going to
g_cube_internal_consistent(), including once in g_cube_consistent()
But I'll address that in another patch.

Here's new version of the patch, with:
1. Corrected docs
2. Any combination of dompress\decompress\fetch is allowed
3. (Existence of fetch) or (absence of compress) leads to IOS

The last point seems counterintuitive, but I do not see anything wrong.

Best regards, Andrey Borodin, Octonica.


0001-Allow-uncompressed-GiST-2.patch
Description: Binary data

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

Re: [HACKERS] Default Partition for Range

[Jeevan Ladhe]

Hi Beena,

I went through your patch, and here are some of my comments:

- For generating a qual for default range partition, instead of scanning
for all
the children and collecting all the boundspecs, how about creating and
negating
an expression from the lists of lowerdatums and upperdatums in boundinfo.

- Wrong comment:
+ int default_index; /* Index of the default list partition. */

- Suggested by Robert earlier on default list partitioning thread, instead
of
abbreviating is_def/found_def use is(found)_default etc.

- unrelated change:
- List   *all_parts;
+ List   *all_parts;

- typo: partiton should be partition, similar typo at other places too.
+  * A non-default range partiton table does not currently allow partition
keys

- Useless hunk for this patch:
- Oiddefid;
+ Oid defid;

- better to use IsA here, instead of doing explicit check:
+ bound->content[i] = (datum->type == T_DefElem)
+ ? RANGE_DATUM_DEFAULT

- It is better to use head of either lowerdatums or upperdatums list to
verify
if its a default partition and get rid of the constant PARTITION_DEFAULT
altogether.

+ {
+ /*
+ * If the partition is the default partition switch back to
+ * PARTITION_STRATEGY_RANGE
+ */
+ if (spec->strategy == PARTITION_DEFAULT)
+ {
+ is_def = true;
+ result_spec->strategy = PARTITION_STRATEGY_RANGE;
+ }

- I am sorry, but I could not understand following hunk. Does this change
really
belongs to this patch? If not, it will be better to handle it separately.

@@ -2242,33 +2387,16 @@ get_partition_for_tuple(PartitionDispatch *pd,
  ecxt->ecxt_scantuple = slot;
  FormPartitionKeyDatum(parent, slot, estate, values, isnull);

- if (key->strategy == PARTITION_STRATEGY_RANGE)
+ if (key->strategy == PARTITION_STRATEGY_LIST && isnull[0])
  {
  /*
- * Since we cannot route tuples with NULL partition keys through
- * a range-partitioned table, simply return that no partition
- * exists
+ * A null partition key is only acceptable if null-accepting list
+ * partition exists.
  */
- for (i = 0; i < key->partnatts; i++)
- {
- if (isnull[i])
- {
- *failed_at = parent;
- *failed_slot = slot;
- result = -1;
- goto error_exit;
- }
- }
+ if (partition_bound_accepts_nulls(partdesc->boundinfo))
+ cur_index = partdesc->boundinfo->null_index;

- Change not related to this patch:
- List   *all_parts;
+ List   *all_parts;

- In the function get_qual_from_partbound() is_def is always going to be
false
for range partition, the function get_qual_for_range can be directly passed
false instead.

- Following comment for function get_qual_for_range_default() implies that
this
function returns bool, but the actually it returns a List.
+ *
+ * If DEFAULT is the only partiton for the table then this returns TRUE.
+ *


Regards,
Jeevan Ladhe

On Wed, May 24, 2017 at 12:52 AM, Beena Emerson 
wrote:

> Hello,
>
> On Mon, May 22, 2017 at 11:29 AM, Ashutosh Bapat <
> ashutosh.ba...@enterprisedb.com> wrote:
>
>> On Mon, May 22, 2017 at 7:27 AM, Beena Emerson 
>> wrote:
>> Would it be more readable if this reads as NOT
>> (constraint_for_partition_1 || constraint_for_partition_2 ||
>> constraint_for_partition_3)? That way, the code to create
>> constraint_for_partition_n can be reused and there's high chance that
>> we will end up with consistent constraints?
>>
>
> PFA the patch which gives the default partition constraint as you have
> suggested.
>
>
>>
>> >
>> > It still needs more work:
>> > 1. Handling addition of new partition after default, insertion of data,
>> few
>> > more bugs
>> > 2. Documentation
>> > 3. Regression tests
>> >
>>
>> I think, the default partition support for all the strategies
>> supporting it should be a single patch since most of the code will be
>> shared?
>>
>>
> Dependency on list default patch:
> gram.y : adding the syntax
> partition.c:
> - default_index member in PartitionBoundInfoData;
> - check_new_partition_bound : the code for adding a partition after
> default has been completely reused.
> - isDefaultPartitionBound function is used.
>
> The structures are same  but the handling of list and range is very
> different and the code mostly has  the switch case construct to handle the
> 2 separately. I feel it could be taken separately.
>
> As suggested in the default list thread, I have created
> a partition_bound_has_default macro and avoided usage of has_default in
> range code. This has to be used for list as well.
> Another suggestion for list which has to be implemented in this patch in
> removal of PARTITION_DEFAULT. Ii have not done this in this version.
>
> --
>
> Beena Emerson
>
> EnterpriseDB: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>
>
> --
> Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers
>
>

Re: [HACKERS] pg_resetwal is broken if run from v10 against older version of PG data directory

[tushar]

On 05/29/2017 03:10 PM, Amit Kapila wrote:

  What makes you think above is a valid usage and should
pass?
with earlier versions ,for instance - v.96 v/s v9.5 ,pg_resetwal was 
giving pg_control values .


Installed v9.6 and v9.5  and run pg_resetwal of v9.6 against data 
directory of v9.5.


[centos@centos-cpula ~]$ /tmp/pg9.6/bin/pg_resetxlog -D /tmp/pg9.5/bin/data/
pg_resetxlog: pg_control exists but is broken or unknown version; 
ignoring it

Guessed pg_control values:

pg_control version number:960
Catalog version number:   201608131
Database system identifier:   6425491233437069295
Latest checkpoint's TimeLineID:   1
Latest checkpoint's full_page_writes: off
Latest checkpoint's NextXID:  0:3
Latest checkpoint's NextOID:  1
Latest checkpoint's NextMultiXactId:  1
Latest checkpoint's NextMultiOffset:  0
Latest checkpoint's oldestXID:3
Latest checkpoint's oldestXID's DB:   0
Latest checkpoint's oldestActiveXID:  0
Latest checkpoint's oldestMultiXid:   1
Latest checkpoint's oldestMulti's DB: 0
Latest checkpoint's oldestCommitTsXid:0
Latest checkpoint's newestCommitTsXid:0
Maximum data alignment:   8
Database block size:  8192
Blocks per segment of large relation: 131072
WAL block size:   8192
Bytes per WAL segment:16777216
Maximum length of identifiers:64
Maximum columns in an index:  32
Maximum size of a TOAST chunk:1996
Size of a large-object chunk: 2048
Date/time type storage:   64-bit integers
Float4 argument passing:  by value
Float8 argument passing:  by value
Data page checksum version:   0


Values to be changed:

First log segment after reset:00010002

If these values seem acceptable, use -f to force reset.
[centos@centos-cpula ~]$

--
regards,tushar
EnterpriseDB  https://www.enterprisedb.com/
The Enterprise PostgreSQL Company



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

Re: [HACKERS] "cannot specify finite value after UNBOUNDED" ... uh, why?

[Amit Langote]

On 2017/05/29 16:04, Tom Lane wrote:
> Would someone please defend the restrictions imposed by the
> "seen_unbounded" checks in transformPartitionBound
> (parse_utilcmd.c:3365..3396 in current HEAD)?  They sure look to me like
> nothing but sloppy thinking, and/or protection of downstream sloppy
> thinking.  Why should the boundedness of one partitioning column's
> range matter to any other partitioning column's range?

If an earlier column's value is unbounded per the bound specification of a
range partition, later columns of an input row would never be compared
against their respective partition bounds, because the row's partition key
would trivially have been determined to be greater or less than the
partition's lower or upper bound, respectively, based on the comparison
against aforementioned unbounded column's infinite value.  Note that it's
the row comparison logic at work here.

Having said that, having finite values after an unbounded value doesn't
really affect tuple-routing code per se (which is what the above paragraph
is meant to describe the actions of), but it does affect the code that
creates constraint expression from the relpartbound value of a range
partition, at least because of the way get_qual_for_range() implements it.

If we had allowed them, an incorrect constraint expression would result as
illustrated below; consider a range partition:

create table foo (a int, b int) partition by range (a, b);
create table foo1 partition of foo for values in (unbounded, -1) to
(unbounded, 1);

foo1, as defined above, basically accepts any row with non-null a, if
inserted through foo via tuple-routing, because any non-null value of a is
trivially >= -infinity and < +infinity.

But, get_qual_for_range() would end up returning an expression that looks
like:

(b >= -1) and (b < 1)

which means that (2, 1) would fail to be inserted directly into foo1 due
to the above constraint, even if inserting it through foo would work,
because column b would not be considered in the latter.

Maybe there is a way to rewrite the code that generates the constraint
expression to somehow not emit the above expression, but any new code to
accomplish that might be more complicated than it is now.

See the following message:

https://www.postgresql.org/message-id/CA%2BTgmoYWnV2GMnYLG-Czsix-E1WGAbo4D%2B0tx7t9NdfYBDMFsA%40mail.gmail.com

I tend to agree with what Robert said there in the first part; there is no
point in allowing finite values to be specified after the last UNBOUNDED
in FROM and TO lists, which might mislead the user to believe that they
are actually significant.

Thanks,
Amit



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

Re: [HACKERS] pg_resetwal is broken if run from v10 against older version of PG data directory

[Amit Kapila]

On Mon, May 29, 2017 at 1:48 PM, tushar  wrote:
> Hi,
>
> I have installed PG v9.6 / v9.5 , if we run pg_resetwal from v10 binaries
> against data directory of v9.6/9.5 ,getting this error -
>
> centos@centos-cpula bin]$ ./pg_resetwal -D /tmp/pg9.6/bin/data/
> pg_resetwal: pg_control exists but is broken or unknown version; ignoring it
>

This appears to be an expected error as you are trying to use a
version of binaries that doesn't match the version stored in
pg_control.  What makes you think above is a valid usage and should
pass?

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


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

Re: [HACKERS] UPDATE of partition key

[Amit Kapila]

On Mon, May 29, 2017 at 11:20 AM, Amit Khandekar  wrote:
> On 24 May 2017 at 20:16, Amit Kapila  wrote:
>> On Wed, May 24, 2017 at 8:14 PM, Amit Kapila  wrote:
>>> Apart from above, there is one open issue [1]
>>>
>>
>> Forget to mention the link, doing it now.
>>
>> [1] - 
>> https://www.postgresql.org/message-id/CAA4eK1KEZQ%2BCyXbBzfn1jFHoEfa_OemDLhLyy7xfD1QUZLo1DQ%40mail.gmail.com
>
> I am not sure right now whether making the t_ctid of such tuples to
> Invalid would be a right option, especially because I think there can
> be already some other meaning if t_ctid is not valid.
>

AFAIK, this is used to point to current tuple itself or newer version
of a tuple or is used in speculative inserts (refer comments above
HeapTupleHeaderData in htup_details.h).  Can you mention what other
meaning are you referring here for InvalidBlockId in t_ctid?

> But may be we
> can check this more.
>
> If we decide to error out using some way, I would be inclined towards
> considering re-using some combinations of infomask bits (like
> HEAP_MOVED_OFF as suggested upthread) rather than using invalid t_ctid
> value.
>
> But I think, we can also take step-by-step approach even for v11. If
> we agree that it is ok to silently do the updates as long as we
> document the behaviour, we can go ahead and do this, and then as a
> second step, implement error handling as a separate patch. If that
> patch does not materialize, we at least have the current behaviour
> documented.
>

I think that is sensible approach if we find the second step involves
big or complicated changes.


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


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

Re: [HACKERS] Adding support for Default partition in partitioning

[Jeevan Ladhe]

>
> The existing comment is not valid
> /*
>  * A null partition key is only acceptable if null-accepting
> list
>  * partition exists.
>  */
> as we allow NULL to be stored in default. It should be updated.
>

Sure Beena, as stated earlier will update this on my next version of patch.


Regards,
Jeevan Ladhe

Re: [HACKERS] Adding support for Default partition in partitioning

[Jeevan Ladhe]

This patch needs a rebase on recent commits, and also a fix[1] that is
posted for get_qual_for_list().

I am working on both of these tasks. Will update the patch once I am done
with this.


Regards,

Jeevan Ladhe

On Mon, May 29, 2017 at 12:25 PM, Beena Emerson 
wrote:

> On Thu, May 25, 2017 at 3:03 PM, Jeevan Ladhe
>  wrote:
> >
> > Forgot to attach the patch.
> > PFA.
> >
> > On Thu, May 25, 2017 at 3:02 PM, Jeevan Ladhe <
> jeevan.la...@enterprisedb.com> wrote:
> >>
> >> Hi Rajkumar,
> >>
> >>> postgres=# CREATE TEMP TABLE temp_list_part (a int) PARTITION BY LIST
> (a);
> >>> CREATE TABLE
> >>> postgres=# CREATE TEMP TABLE temp_def_part (a int);
> >>> CREATE TABLE
> >>> postgres=# ALTER TABLE temp_list_part ATTACH PARTITION temp_def_part
> DEFAULT;
> >>> 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: Failed.
> >>> !>
> >>
> >>
> >> Thanks for reporting.
> >> PFA patch that fixes above issue.
> >>
>
>
> The existing comment is not valid
> /*
>  * A null partition key is only acceptable if null-accepting
> list
>  * partition exists.
>  */
> as we allow NULL to be stored in default. It should be updated.
>
> DROP TABLE list1;
> CREATE TABLE list1 (a int) PARTITION BY LIST (a);
> CREATE TABLE list1_1 (LIKE list1);
> ALTER TABLE  list1 ATTACH PARTITION list1_1 FOR VALUES IN (2);
> CREATE TABLE list1_def PARTITION OF list1 DEFAULT;
> INSERT INTO list1 VALUES (NULL);
> SELECT * FROM list1_def;
>  a
> ---
>
> (1 row)
>
>
> --
>
> Beena Emerson
>
> EnterpriseDB: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>

[HACKERS] pg_resetwal is broken if run from v10 against older version of PG data directory

[tushar]

Hi,

I have installed PG v9.6 / v9.5 , if we run pg_resetwal from v10 
binaries against data directory of v9.6/9.5 ,getting this error -


centos@centos-cpula bin]$ ./pg_resetwal -D /tmp/pg9.6/bin/data/
pg_resetwal: pg_control exists but is broken or unknown version; ignoring it
pg_resetwal: could not open directory "pg_wal": No such file or directory
[centos@centos-cpula bin]$

Steps to reproduce-
installed PG v9.6
installed PG v10
go to bin directory of v10 and run pg_resetwal , provide -D = data 
directory of v9.6.


--
regards,tushar
EnterpriseDB  https://www.enterprisedb.com/
The Enterprise PostgreSQL Company



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

[HACKERS] Macros bundling RELKIND_* conditions

[Ashutosh Bapat]

Hi,
We saw a handful bugs reported because RELKIND_PARTITIONED_TABLE was
not added to appropriate conditions on relkind. One such report is
[1]. On that thread I suggested that we encapsulate these conditions
as macros. Here's excerpt of my mail.

--
I noticed, that
after we introduced RELKIND_PARTITIONED_TABLE, we required to change a
number of conditions to include this relkind. We missed some places in
initial commits and fixed those later. I am wondering whether we
should creates macros clubbing relevant relkinds together based on the
purpose of the tests e.g. IS_RELKIND_HAS_STORAGE(). When a new relkind
is added, one can examine these macros to check whether the new
relkind fits in the given macro. If all those macros are placed
together, there is a high chance that we will not miss any place in
the initial commit itself.

For example, if we had a macro IS_RELKIND_WITH_STATS defined as
#define IS_RELKIND_WITH_STATS(relkind) \
((relkind) == RELKIND_RELATION || \
(relkind) == RELKIND_MATVIEW)

and CreateStatistics() and getExtendedStatistics() had following conditions
if (!IS_RELKIND_WITH_STATS(rel->rd_rel->relkind)) and if
(!IS_RELKIND_WITH_STATS(tabinfo->relkind)) resp. The patch would be
just adding
(relkind) == RELKIND_FOREIGN_TABLE || \
(relkind) == RELKIND_PARTITIONED_TABLE)

to that macro without requiring to find out where all we need to add
those two relkinds for statistics purposes.
-- excerpt ends

Peter Eisentraut thought that idea is worth a try. I gave it a try on
my way back from PGCon. Attached is a series of patches, one per
macro. This isn't a complete series but will give an idea of what's
involved. It might be possible to change switch cases at some places
to use if else with these macros. But I haven't done any changes
towards that.


[1] 
https://www.postgresql.org/message-id/CAKJS1f-BmGo410bh5RSPZUvOO0LhmHL2NYmdrC_Jm8pk_FfyCA%40mail.gmail.com
-- 
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company


pg_relkind_macros_patches_v1.tar.gzip
Description: Binary data

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

[HACKERS] "cannot specify finite value after UNBOUNDED" ... uh, why?

[Tom Lane]

Would someone please defend the restrictions imposed by the
"seen_unbounded" checks in transformPartitionBound
(parse_utilcmd.c:3365..3396 in current HEAD)?  They sure look to me like
nothing but sloppy thinking, and/or protection of downstream sloppy
thinking.  Why should the boundedness of one partitioning column's
range matter to any other partitioning column's range?

regards, tom lane


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

Re: [HACKERS] Adding support for Default partition in partitioning

[Beena Emerson]

On Thu, May 25, 2017 at 3:03 PM, Jeevan Ladhe
 wrote:
>
> Forgot to attach the patch.
> PFA.
>
> On Thu, May 25, 2017 at 3:02 PM, Jeevan Ladhe  
> wrote:
>>
>> Hi Rajkumar,
>>
>>> postgres=# CREATE TEMP TABLE temp_list_part (a int) PARTITION BY LIST (a);
>>> CREATE TABLE
>>> postgres=# CREATE TEMP TABLE temp_def_part (a int);
>>> CREATE TABLE
>>> postgres=# ALTER TABLE temp_list_part ATTACH PARTITION temp_def_part 
>>> DEFAULT;
>>> 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: Failed.
>>> !>
>>
>>
>> Thanks for reporting.
>> PFA patch that fixes above issue.
>>


The existing comment is not valid
/*
 * A null partition key is only acceptable if null-accepting list
 * partition exists.
 */
as we allow NULL to be stored in default. It should be updated.

DROP TABLE list1;
CREATE TABLE list1 (a int) PARTITION BY LIST (a);
CREATE TABLE list1_1 (LIKE list1);
ALTER TABLE  list1 ATTACH PARTITION list1_1 FOR VALUES IN (2);
CREATE TABLE list1_def PARTITION OF list1 DEFAULT;
INSERT INTO list1 VALUES (NULL);
SELECT * FROM list1_def;
 a
---

(1 row)


-- 

Beena Emerson

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


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

Re: [HACKERS] Surjective functional indexes

[Konstantin Knizhnik]

On 05/27/2017 09:50 PM, Peter Eisentraut wrote:

On 5/25/17 12:30, Konstantin Knizhnik wrote:

Functions like (info->>'name') are named "surjective" ni mathematics.

A surjective function is one where each value in the output type can be
obtained by some input value.  That's not what you are after here.  The
behavior you are describing is a not-injective function.

I think you are right that in practice most functions are not injective.
  But I think there is still quite some difference between a function
like the one you showed that selects a component from a composite data
structure and, for example, round(), where in practice any update is
likely to change the result of the function.


Thank you, I will rename "surjective" parameter to "injective" with "false" as 
default value.
Concerning "round" and other similar functions - obviously there are use cases 
when such functions are used for
functional indexes. This is why I want to allow user to make a choice and this 
is the reason of introducing this parameter.
The question is the default value of this parameter: should we by default 
preserve original Postgres behavior:
check only affected set of keys or should we pay extra cost for calculating 
value of the function (even if we managed to store
calculated value of the indexes expression for new tuple, we still have to 
calculate it for old tuple, so function will be calculated
at least twice more times).

--
Konstantin Knizhnik
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company



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