Re: [HACKERS] UPDATE of partition key

[Amit Khandekar]

On 2 May 2017 at 18:17, Robert Haas  wrote:
> On Tue, Apr 4, 2017 at 7:11 AM, Amit Khandekar  wrote:
>> Attached updated patch v7 has the above changes.
>
> This no longer applies.  Please rebase.

Thanks Robert for informing about this.

My patch has a separate function for emitting error message when a
partition constraint fails. And, the recent commit c0a8ae7be3 has
changes to correct the way the tuple is formed for displaying in the
error message. Hence there were some code-level conflicts.

Attached is the rebased patch, which resolves the above conflicts.


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

[Craig Ringer]

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

We should do the same for pg_dump in -Fc mode.

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

[Craig Ringer]

On 3 May 2017 at 07:00, Tomas Vondra  wrote:

> I'm not sure what you mean by "jerking this out from users". Isn't most of
> this thread about how to allow CTE inlining without hurting users
> unnecessarily?

He's referring to

Andreas Karlsson  wrote:
> 1. Just remove the optimization fence and let people add OFFSET 0 to their 
> queries if they want an optimization fence. This lets us keep pretending that 
> we do not have query hints (and therefore do not have to formalize any syntax 
> for them) while still allowing people to add optimization fences.

... and the various agreement expressed with it.

I wish that's what had been done in the first place, but it's a bit
harsh on users to do it now.

We can keep living in fantasty-land where we deny hints while telling
people to use a washy-worded semi-documented hint to work around
issues.

Or we can take responsibility and admit we've had to grandfather a
limitation in as a blessed hint. Then unblock our way toward this
performance enhancement / user foot-gun without yanking the rug out
from under users who've been relying on our odd implementation detail
to date:

* Add "WITH MATERIALIZED" or "WITH NO_INLINE" or whatever
* Change "WITH" to inline where safe, initially only for single reference terms
* Mention the change in the release notes

Note that I'm in no way advocating real hints. I won't say I love all
aspects of project policy there, but it's clear there are benefits,
and that hints bring plenty of problems. If this weren't already
documented and widely advocated as a way to "fix" poor
pushdown/pullup/inlining decisions I'd be firmly in the "just fix it"
camp.

> * Just removing the optimization fence and telling users to use OFFSET 0
> instead is a no-go, just like removing the fencing and not providing any
> sensible replacement.

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

At least then we're back down to one hint-not-a-hint instead of two,
and it doesn't really block any useful optimisations.

> * GUC is not the solution.

Strong agreement.

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

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

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

Hardly.

> Perhaps combining this with a GUC would be a solution. I mean, a GUC
> specifying the default behavior, and then INLINE / MATERIALIZE for
> individual CTEs in a query?

It'd be nice if we could do that for a couple of releases as an
interim measure, but people will then get locked into relying on it,
and we'll never be able to remove it.

It's not like we don't make users do other things for upgrades, and
don't change performance in other ways. We don't even HAVE a
performance-test farm to look at regressions, planner behaviour
changes, etc. Yes, I know it's hard and nobody's volunteering, the
point is, we're hardly free of undocumented and unintentional
query-specific regressions let alone documented and relnoted ones.

So a sad -1 to me for a GUC.

Anyone big enough to be significantly upset by this planner change
will have a QA/staging deployment system anyway. Or should, because we
make enough other changes in a major release to make their life way
more interesting than this!

-- 
 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] logical replication syntax (was DROP SUBSCRIPTION, query cancellations and slot handling)

[Petr Jelinek]

On 02/05/17 18:25, Alvaro Herrera wrote:
> Petr Jelinek wrote:
>> On 02/05/17 18:00, Robert Haas wrote:
>>> On Tue, May 2, 2017 at 11:49 AM, Alvaro Herrera
>>>  wrote:
 Petr Jelinek wrote:
> So the only way to fulfill the requirement you stated is to just not try
> to drop the slot, ever, on DROP SUBSCRIPTION. That makes the default
> behavior leave resources on upstream that will eventually cause that
> server to stop unless user notices before. I think we better invent
> something that limits how much inactive slots can hold back WAL and
> catalog_xmin in this release as well then.

 I don't understand why isn't the default behavior to unconditionally
 drop the slot.  Why do we ever want the slot to be kept?
>>>
>>> What if the remote server doesn't exist any more?
>>
>> Or what if the slot is used by other subscription (because you restored
>> dump containing subscriptions to another server for example), or you
>> have server that does not have outside network access anymore, or many
>> other reasons.
> 
> So there are two different scenarios: one is that you expect the slot
> drop to fail for whatever reason; the other is that you want the slot to
> be kept because it's needed for something else.  Maybe those two should
> be considered separately.
> 
> 1) keep the slot because it's needed for something else.
>I see two options:
>a) change the something else so that it uses another slot with the
>   same location.  Do we have some sort of "clone slot" operation?

Nope.

>b) have an option to dissociate the slot from the subscription prior
>   to the drop.
> 

We do have ALTER SUBSCRIPTION bla WITH (SLOT NAME = 'something') so this
is definitely doable but ALTER SUBSCRIPTION bla WITH (SLOT NAME = '') is
not very nice syntax, maybe something like RESET SLOT NAME?

> 2) don't drop because we know it won't work.  I see two options:
>c) ignore a drop slot failure, i.e. don't cause a transaction abort.
>   An easy way to implement this is just add a PG_TRY block, but we
>   dislike adding those and not re-throwing the error.
>d) rethink drop slot completely; maybe instead of doing it
>   immediately, it should be a separate task, so we first close the
>   current transaction (which dropped the subscription) and then we open
>   a second one to drop the slot, so that if the drop slot fails, the
>   subscription does not come back to life.
> 

Yeah I was thinking about ignoring failures with WARNING as well, but
never seemed right because it would not solve the case 1), but I didn't
think of b) which might solve it.

I was also thinking on how to map the behavior to RESTRICT vs CASCADE,
wonder if RESTRICT should check for slot existence and refuse to drop if
the slot exists (question is then should it bail on connection failure
or ignore it?) and CASCADE should try to drop the slot and only warn on
failure 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] logical replication syntax (was DROP SUBSCRIPTION, query cancellations and slot handling)

[Petr Jelinek]

On 02/05/17 22:40, Robert Haas wrote:
> On Tue, May 2, 2017 at 3:02 PM, Petr Jelinek
>  wrote:
>> That sounds okay. I know PeterE didn't like the lower case and
>> underscore separated words for options in the original patch, so I'd
>> like to hear his opinion on this. I am not sure how much advantage is
>> there in removing the '=' in between the key and value. That's the main
>> difference between generic options and definitions (well and definitions
>> can have 2 words for key, but that's something I have added anyway), I
>> don't really understand why we have both and use one for some commends
>> and the other for others btw.
> 
> I don't care all *that* much about the equals sign, although I think
> it's marginally more elegant without it, and VACUUM, EXPLAIN, and COPY
> all do it that way.  But I think allowing two-word names is just a
> pile of parser nastiness that we'd be far better off without.  If you
> use the same syntax as VACUUM, EXPLAIN, and COPY, all options are a
> single identifier.

Ok, Let me be clear, I actually happen to agree with your proposal. The
reason I am moaning is that I always seem to find myself doing tons of
mechanical work to rewrite some cosmetic aspect of some patch based on
which committer is paying attention in a specific week. So while I am
for doing exactly what you proposed, I'd like to see couple of +1s first
(Peter?) since I don't want to rewrite it to something different again
and it's also long past freeze.

To repeat the proposal:
- change the WITH (...) clauses in subscription/publication commands to:
(create_slot true/false, connect true/false, slot_name 'something',
copy_data true/false, etc)

- change the NOREFRESH to NO REFRESH in ALTER SUBSCRIPTION name SET
PUBLICATION (btw I originally had SKIP REFRESH there but changed it to
NOREFRESH for consistency with the other NO* stuff, wonder if SKIP would
sound more english).

- change the (publish insert/nopublish insert/publish update/nopublish
update), etc options to (publish 'update,insert').

And one question, if we are not using the definitions (key = value)
should we keep the WITH keyword in the syntax, would it be confusing?

>  If it's got to be multiple words, they can be
> separated by underscores.  But with what you've got right now, it's
> sometimes one identifier even when it's two words, and sometimes two
> identifiers. 
> 

The main inconsistency is the synchronous_commit which is that way to
make it clear it's same as the GUC it changes, but looks like that was a
mistake.

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

[Haribabu Kommi]

[Adding -hackers mailing list]

On Fri, Apr 28, 2017 at 6:28 PM,  wrote:

> The following bug has been logged on the website:
>
> Bug reference:  14634
> Logged by:  Henry Boehlert
> Email address:  henry_boehl...@agilent.com
> PostgreSQL version: 9.6.2
> Operating system:   Windows Server 2012 R2 6.3.9600
> Description:
>
> Executing command pg_basebackup -D -F t writes its output to stdout, which
> is open in text mode, causing LF to be converted to CR LF thus corrupting
> the resulting archive.
>
> To write the tar to stdout, on Windows stdout's mode should be temporarily
> switched to binary.
>
> https://msdn.microsoft.com/en-us/library/tw4k6df8.aspx
>

Thanks for reporting the issue.
With the attached patch, I was able to extract the tar file that gets
generated when the tar file is written into stdout. I tested the
the compressed tar also.

This bug needs to be fixed in back branches also.

Regards,
Hari Babu
Fujitsu Australia


pg_basebackup_tar_to_stdout_windows_fix.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_dump emits ALTER TABLE ONLY partitioned_table

[Stephen Frost]

Amit,

* Amit Langote (langote_amit...@lab.ntt.co.jp) wrote:
> Attached updated patches.

Please find an updated version which corrects the issue with
binary-upgrade of partitioned tables having partitions in other schemas,
along with a few other minor improvements.

If you could take a look at it, I'd appreciate it.  We already had a
test case in the pg_dump TAP tests for partitions existing in a schema
different from the partitioned table, but we weren't checking the
binary-upgrade case, so I've added a check to do that now.  I'm sure
additional tests would be good to add and will take a look at doing that
tomorrow, but this hopefully closes at least the latest issue.

Assuming this looks good to you, I'll push it tomorrow, possibly with
other minor adjustments and perhaps a few more tests.

Thanks!

Stephen
diff --git a/src/bin/pg_dump/common.c b/src/bin/pg_dump/common.c
new file mode 100644
index e2bc357..47191be
*** a/src/bin/pg_dump/common.c
--- b/src/bin/pg_dump/common.c
*** static int	numextmembers;
*** 68,75 
  
  static void flagInhTables(TableInfo *tbinfo, int numTables,
  			  InhInfo *inhinfo, int numInherits);
- static void flagPartitions(TableInfo *tblinfo, int numTables,
- 			  PartInfo *partinfo, int numPartitions);
  static void flagInhAttrs(DumpOptions *dopt, TableInfo *tblinfo, int numTables);
  static DumpableObject **buildIndexArray(void *objArray, int numObjs,
  Size objSize);
--- 68,73 
*** static int	DOCatalogIdCompare(const void
*** 77,84 
  static int	ExtensionMemberIdCompare(const void *p1, const void *p2);
  static void findParentsByOid(TableInfo *self,
   InhInfo *inhinfo, int numInherits);
- static void findPartitionParentByOid(TableInfo *self, PartInfo *partinfo,
-  int numPartitions);
  static int	strInArray(const char *pattern, char **arr, int arr_size);
  
  
--- 75,80 
*** getSchemaData(Archive *fout, int *numTab
*** 97,106 
  	NamespaceInfo *nspinfo;
  	ExtensionInfo *extinfo;
  	InhInfo*inhinfo;
- 	PartInfo*partinfo;
  	int			numAggregates;
  	int			numInherits;
- 	int			numPartitions;
  	int			numRules;
  	int			numProcLangs;
  	int			numCasts;
--- 93,100 
*** getSchemaData(Archive *fout, int *numTab
*** 238,247 
  	inhinfo = getInherits(fout, );
  
  	if (g_verbose)
- 		write_msg(NULL, "reading partition information\n");
- 	partinfo = getPartitions(fout, );
- 
- 	if (g_verbose)
  		write_msg(NULL, "reading event triggers\n");
  	getEventTriggers(fout, );
  
--- 232,237 
*** getSchemaData(Archive *fout, int *numTab
*** 255,265 
  		write_msg(NULL, "finding inheritance relationships\n");
  	flagInhTables(tblinfo, numTables, inhinfo, numInherits);
  
- 	/* Link tables to partition parents, mark parents as interesting */
- 	if (g_verbose)
- 		write_msg(NULL, "finding partition relationships\n");
- 	flagPartitions(tblinfo, numTables, partinfo, numPartitions);
- 
  	if (g_verbose)
  		write_msg(NULL, "reading column info for interesting tables\n");
  	getTableAttrs(fout, tblinfo, numTables);
--- 245,250 
*** getSchemaData(Archive *fout, int *numTab
*** 293,302 
  	getPolicies(fout, tblinfo, numTables);
  
  	if (g_verbose)
- 		write_msg(NULL, "reading partition key information for interesting tables\n");
- 	getTablePartitionKeyInfo(fout, tblinfo, numTables);
- 
- 	if (g_verbose)
  		write_msg(NULL, "reading publications\n");
  	getPublications(fout);
  
--- 278,283 
*** flagInhTables(TableInfo *tblinfo, int nu
*** 354,396 
  	}
  }
  
- /* flagPartitions -
-  *	 Fill in parent link fields of every target table that is partition,
-  *	 and mark parents of partitions as interesting
-  *
-  * modifies tblinfo
-  */
- static void
- flagPartitions(TableInfo *tblinfo, int numTables,
- 			  PartInfo *partinfo, int numPartitions)
- {
- 	int		i;
- 
- 	for (i = 0; i < numTables; i++)
- 	{
- 		/* Some kinds are never partitions */
- 		if (tblinfo[i].relkind == RELKIND_SEQUENCE ||
- 			tblinfo[i].relkind == RELKIND_VIEW ||
- 			tblinfo[i].relkind == RELKIND_MATVIEW)
- 			continue;
- 
- 		/* Don't bother computing anything for non-target tables, either */
- 		if (!tblinfo[i].dobj.dump)
- 			continue;
- 
- 		/* Find the parent TableInfo and save */
- 		findPartitionParentByOid([i], partinfo, numPartitions);
- 
- 		/* Mark the parent as interesting for getTableAttrs */
- 		if (tblinfo[i].partitionOf)
- 		{
- 			tblinfo[i].partitionOf->interesting = true;
- 			addObjectDependency([i].dobj,
- tblinfo[i].partitionOf->dobj.dumpId);
- 		}
- 	}
- }
- 
  /* flagInhAttrs -
   *	 for each dumpable table in tblinfo, flag its inherited attributes
   *
--- 335,340 
*** findParentsByOid(TableInfo *self,
*** 992,1031 
  }
  
  /*
-  * findPartitionParentByOid
-  *	  find a partition's parent in tblinfo[]
-  */
- static void
- findPartitionParentByOid(TableInfo *self, PartInfo *partinfo,
- 		 int numPartitions)
- {
- 

Re: [HACKERS] check with serial

[Vaishnavi Prabakaran]

On Tue, May 2, 2017 at 11:30 PM, Andrew Dunstan <
andrew.duns...@2ndquadrant.com> wrote:

>
> Here's a simple patch that does what I had in mind. It allows providing
> for an arbitrary schedule file in both the check and installcheck
> recipes. The historic behaviour is preserved.
>
>
Hmm, installcheck command with SCHEDULE set as "Parallel" does not honor
"MAXCONNOPT" settings in the attached patch.

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

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

Best Regards,
Vaishnavi,
Fujitsu Australia.

Re: [HACKERS] renaming "transaction log"

[Stephen Frost]

Peter,

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

Haven't looked at the patch, but +1 for this change.

Thanks!

Stephen


signature.asc
Description: Digital signature

Re: [HACKERS] [POC] hash partitioning

[Robert Haas]

On Tue, May 2, 2017 at 9:01 PM, Jeff Davis  wrote:
> On Tue, Feb 28, 2017 at 6:33 AM, Yugo Nagata  wrote:
>> In this patch, user can specify a hash function USING. However,
>> we migth need default hash functions which are useful and
>> proper for hash partitioning.
>
> I suggest that we consider the hash functions more carefully. This is
> (effectively) an on-disk format so it can't be changed easily later.
>
> 1. Consider a partition-wise join of two hash-partitioned tables. If
> that's a hash join, and we just use the hash opclass, we immediately
> lose some useful bits of the hash function. Same for hash aggregation
> where the grouping key is the partition key.

Hmm, that could be a problem in some cases.  I think there's probably
much less of a problem if the modulus isn't a power of two?

> To fix this, I think we need to include a salt in the hash API. Each
> level of hashing can choose a random salt.

Do you mean that we'd salt partitioning hashing differently from
grouping hashing which would be salted different from aggregation
hashing which, I suppose, would be salted differently from hash index
hashing?  Or do you mean that you'd have to specify a salt when
creating a hash-partitioned table, and make sure it's the same across
all compatibly partitioned tables you might want to hash-join?  That
latter sounds unappealing.

> 2. Consider a partition-wise join where the join keys are varchar(10)
> and char(10). We can't do that join if we just use the existing hash
> strategy, because 'foo' = 'foo   ' should match, but those values
> have different hashes when using the standard hash opclass.
>
> To fix this, we need to be smarter about normalizing values at a
> logical level before hashing. We can take this to varying degrees,
> perhaps even normalizing an integer to a numeric before hashing so
> that you can do a cross-type join on int=numeric.
>
> Furthermore, we need catalog metadata to indicate which hash functions
> are suitable for which cross-type comparisons. Or, to put it the other
> way, which typecasts preserve the partitioning.

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

> 3. We might want to use a hash function that is a little slower that
> is more resistant to collisions. We may even want to use a 64-bit
> hash.
>
> My opinion is that we should work on this hashing infrastructure
> first, and then support the DDL. If we get the hash functions right,
> that frees us up to create better plans, with better push-downs, which
> will be good for parallel query.

I am opposed to linking the fate of this patch to multiple
independent, possibly large, possibly difficult, possibly
controversial enhancements to the hashing mechanism.  If there are
simple things that can reasonably be done in this patch to make hash
partitioning better, great.  If you want to work on improving the
hashing mechanism as an independent project, also great.  But I think
that most people would rather have hash partitioning in v11 than wait
for v12 or v13 so that other hashing improvements can be completed; I
know I would.  If we say "we shouldn't implement hash partitioning
because some day we might make incompatible changes to the hashing
mechanism" then we'll never implement it, because that will always be
true.  Even the day after we change it, there still may come a future
day when we change it again.

The stakes have already been raised by making hash indexes durable;
that too is arguably making future changes to the hashing
infrastructure harder.  But I still think it was the right thing to
proceed with that work.  If we get 64-bit hash codes in the next
release, and we want hash indexes to use them, then we will have to
invalidate existing hash indexes (again).  That's sad, but not as sad
as it would have been to not commit the work to make hash indexes
durable. There's a chicken-and-egg problem here: without durable hash
indexes and hash partitioning, there's not much incentive to make
hashing better, but once we have them, changes create a backward
compatibility issue.  Such is life; nothing we do is infinitely
future-proof.

The last significant overhaul of the hashing mechanism that I know
about was in 2009, cf. 2604359251d34177a14ef58250d8b4a51d83103b and
8205258fa675115439017b626c4932d5fefe2ea8.  Until this email, I haven't
seen any complaints about the quality of that hash function either in
terms of speed or collision properties - what makes you think those
things are serious problems?  I *have* 

Re: [HACKERS] SUBSCRIPTIONS and pg_upgrade

[Noah Misch]

On Thu, Apr 13, 2017 at 12:11:30PM -0400, Robert Haas wrote:
> On Thu, Apr 13, 2017 at 12:05 PM, Peter Eisentraut
>  wrote:
> > On 4/12/17 18:31, Peter Eisentraut wrote:
> >> On 4/11/17 23:41, Noah Misch wrote:
> >>> On Tue, Apr 11, 2017 at 11:21:24PM -0400, Peter Eisentraut wrote:
>  On 4/9/17 22:16, Noah Misch wrote:
> > [Action required within three days.  This is a generic notification.]
> 
>  Patches have been posted.  Discussion is still going on a bit.
> >>>
> >>> By what day should the community look for your next update?
> >>
> >> tomorrow
> >
> > Everything has been committed, and this thread can be closed.
> 
> I wonder if we should have an --no-subscriptions option, now that they
> are dumped by default, just like we have --no-blobs, --no-owner,
> --no-password, --no-privileges, --no-acl, --no-tablespaces, and
> --no-security-labels.  It seems like there is probably a fairly large
> use case for excluding subscriptions even if you have sufficient
> permissions to dump them.

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

[Thomas Munro]

On Tue, May 2, 2017 at 3:01 AM, Robert Haas  wrote:
> [...]
> Only the rows in the parent show up in the transition table.  But now
> look what happens if I add an unrelated trigger that also uses
> transition tables to the children:
>
> rhaas=# CREATE FUNCTION u() RETURNS trigger LANGUAGE plpgsql AS
> $$begin null; end$$;
> CREATE FUNCTION
> rhaas=# CREATE TRIGGER x1 AFTER UPDATE ON p1 REFERENCING OLD TABLE AS
> old NEW TABLE AS new FOR EACH STATEMENT EXECUTE PROCEDURE u();
> CREATE TRIGGER
> rhaas=# UPDATE p SET b = 'whatever';
> NOTICE:  table p
> NOTICE:  table p got value 0
> NOTICE:  table p got value 1
> NOTICE:  table p got value 2
> UPDATE 3
>
> It seems pretty clear to me that this is busted.  The existence of
> trigger x1 on p1 shouldn't affect whether trigger x on p sees changes
> to p1's rows in its transition tables.

Yikes -- agreed.  See analysis and draft patch for discussion below.

> Either all changes to any
> descendants of p should be captured by the transition tables, or only
> changes to the root table should be captured.  If we do the former,
> the restriction against using transition tables in triggers on
> partitioned tables should be removed, I would think.  If we do the
> latter, then what we should document is not that partitioned tables
> have a restriction that doesn't apply to inheritance but rather that
> the restriction on the partitioned case flows from the fact that only
> the parent's changes are captured, and the parent is always empty in
> the partitioning case.  In deciding between these two cases, we should
> consider the case where the inheritance children have extra columns
> and/or different column orderings.

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

The transition table infrastructure only supports exactly one relation
being modified at each query level, and it's a bug that this example
captures tuples from p1 into the tuplestore intended for p's tuples
even though it is not even going to fire the after statement trigger
on p1.  It's only a coincidence that the tuples have compatible
TupleDescs.

The pre-existing behaviour for triggers with inheritance is that
STATEMENT triggers fire only for the directly named relation, but ROW
triggers fire for all affected relations.  The transition table patch
didn't change that, but it added code to AfterTriggerSaveEvent, which
is called by ExecAR(Insert|Update|Delete)Triggers, to capture
transitions.  That gets called for every updated relation (ie
including partitions and inheritance sub-tables) to implement the ROW
policy.  It needs to be taught not to capture transition tuples from
relations except the one directly named.

One solution to this problem is for nodeModifyTable.c to tell the
ExecAR* functions explicitly whether to capture transition tuples.  It
knows when it has modified the explicitly named relation in a
hierarchy (mt_whichplan == 0) without rerouting via a partitioned
table (mt_partition_dispatch_info == NULL).  See attached patch for
discussion (it lacks tests and needs better comments).  Does this make
sense?  Do you see a better way?

[1] 
https://www.postgresql.org/message-id/CACjxUsNhdm4ZCgaVreLK5kAwHTZUkqJAVXiywwi-HNVsuTLMnA%40mail.gmail.com

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


transition-tables-from-one-relation-only.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

[HACKERS] renaming "transaction log"

[Peter Eisentraut]

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

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


0001-Replace-transaction-log-with-write-ahead-log.patch
Description: invalid/octet-stream

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

Re: [HACKERS] Bug in prepared statement cache invalidation?

[Robert Haas]

On Tue, May 2, 2017 at 5:50 PM, Tom Lane  wrote:
> Robert Haas  writes:
>> Yeah.  I think there should be a way to tell a PL to flush any
>> internal caches it is maintaining, some variant of DISCARD.  But that
>> would require a bunch of code that nobody's written yet.
>
> That mechanism already exists, so far as the core code is concerned:
> register a syscache inval callback.  But you're right, getting plpgsql
> to actually do anything about changes in composite types would require
> a bunch of code that nobody's written yet.

Well, that'd be a way of doing automatic invalidations, not manual
ones.  Making DISCARD PROCEDURAL LANGUAGE CRAP work would a different
pile of code.

> If you'll pardon my bashing on a long-deceased horse, this would be
> noticeably easier if we stopped using the PLPGSQL_DTYPE_ROW code
> paths for composite-type variables.  That mechanism was really
> designed for cases like "SELECT ... INTO a,b,c" where the row
> contents are fully determined by the function text.  It's practically
> impossible to make it cope with dynamic changes; at the very least
> you have to recompile the whole function.

I guess that's also a project in need of some round tuits.

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


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

Re: [HACKERS] [POC] hash partitioning

[Jeff Davis]

On Tue, Feb 28, 2017 at 6:33 AM, Yugo Nagata  wrote:
> In this patch, user can specify a hash function USING. However,
> we migth need default hash functions which are useful and
> proper for hash partitioning.

I suggest that we consider the hash functions more carefully. This is
(effectively) an on-disk format so it can't be changed easily later.

1. Consider a partition-wise join of two hash-partitioned tables. If
that's a hash join, and we just use the hash opclass, we immediately
lose some useful bits of the hash function. Same for hash aggregation
where the grouping key is the partition key.

To fix this, I think we need to include a salt in the hash API. Each
level of hashing can choose a random salt.

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

To fix this, we need to be smarter about normalizing values at a
logical level before hashing. We can take this to varying degrees,
perhaps even normalizing an integer to a numeric before hashing so
that you can do a cross-type join on int=numeric.

Furthermore, we need catalog metadata to indicate which hash functions
are suitable for which cross-type comparisons. Or, to put it the other
way, which typecasts preserve the partitioning.

3. We might want to use a hash function that is a little slower that
is more resistant to collisions. We may even want to use a 64-bit
hash.


My opinion is that we should work on this hashing infrastructure
first, and then support the DDL. If we get the hash functions right,
that frees us up to create better plans, with better push-downs, which
will be good for parallel query.

Regards,
 Jeff Davis


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

Re: [HACKERS] Transition tables for triggers on foreign tables and views

[Kevin Grittner]

On Fri, Apr 28, 2017 at 10:56 PM, Tom Lane  wrote:

> They will fire if you have an INSTEAD OF row-level trigger; the existence
> of that trigger is what determines whether we implement DML on a view
> through the view's own triggers or through translation to an action on
> the underlying table.
>
> I do not think it'd be reasonable to throw an error for creation of
> a statement-level view trigger when there's no row-level trigger,
> because that just imposes a hard-to-deal-with DDL ordering dependency.
>
> You could make a case for having the updatable-view translation code
> print a WARNING if it notices that there are statement-level triggers
> that cannot be fired due to the translation.

Oh, I see -- you can add all the AFTER ... FOR EACH STATEMENT
triggers you want for an updatable view and they will quietly sit
there without firing no matter how many statements perform the
supposedly triggering action, but as soon as you add a INSTEAD OF
... FOR EACH ROW trigger they spring to life.  On the face of it that
seems to me to violate the POLA, but I kinda see how it evolved.

I need to look at this and the rather similar odd behavior under
inheritance.  I hope to post something Friday.

--
Kevin Grittner
VMware vCenter Server
https://www.vmware.com/


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

Re: [HACKERS] CTE inlining

[Tomas Vondra]

On 5/2/17 11:23 PM, Merlin Moncure wrote:

\On Tue, May 2, 2017 at 12:05 PM, Tomas Vondra
 wrote:

On 5/2/17 6:34 PM, David Fetter wrote:


On Tue, May 02, 2017 at 02:40:55PM +0200, Andreas Karlsson wrote:


On 05/02/2017 04:38 AM, Craig Ringer wrote:


On 1 May 2017 at 22:26, Andreas Karlsson  wrote:





...

I see some alternatives, none of them perfect.

1. Just remove the optimization fence and let people add OFFSET 0 to
their
queries if they want an optimization fence. This lets us keep pretending
that we do not have query hints (and therefore do not have to formalize
any
syntax for them) while still allowing people to add optimization fences.



+1

I get that people with gigantic PostgreSQL installations with
stringent performance requirements sometimes need to do odd things to
squeeze out the last few percentage points of performance.  As the
people (well, at least the people close to the ground) at these
organizations are fully aware, performance optimizations are extremely
volatile with respect to new versions of software, whether it's
PostgreSQL, Oracle, the Linux kernel, or what have you.  They expect
this, and they have processes in place to handle it.  If they don't,
it's pilot error.

We should not be penalizing all our other users to maintain the
fiction that people can treat performance optimizations as a "fire and
forget" matter.



Agreed.


2. Add a decorator for WITH (e.g. "WITH MATERIALIZED x (...") to add an
explicit optimization fence. This will for the first time add official
support for a query hint in the syntax which is a quite big precedent.



Yep.  It's one we should think very carefully before we introduce.



I think it's a mistake to see this as an introduction of query hits.

Firstly, it's a question whether it qualifies as a hint. I wouldn't call it
a hint, but let's assume there is a definition of query hints that includes
WITH MATERIALIZED.

More importantly, however, this is not introducing anything new. It's just a
different name for the current "WITH" semantics, and you can achieve the
same behavior by "OFFSET 0". And people are already using these as hints, so
I fail to see how this introduces anything new.

In fact, if you see the optimization fence as an implicit query hint, this
actually *removes* a hint (although most users are unaware of that behavior
and use it unintentionally).


+1 down the line.  More to the point, for several years now we've (or
at least I, but I'm not the only one) have been advocating for the
usage of CTE to avoid the undocumented and bizarre OFFSET 0 trick.
Jerking this out from users without giving a simple mechanic to get
the same behavior minus a major query rewrite is blatantly user
hostile.  I can't believe we're even contemplating it.   Also a GUC is
not a solution for pretty obvious reasons I think.



I'm not sure what you mean by "jerking this out from users". Isn't most 
of this thread about how to allow CTE inlining without hurting users 
unnecessarily?


I think we agree that:

* Just removing the optimization fence and telling users to use OFFSET 0 
instead is a no-go, just like removing the fencing and not providing any 
sensible replacement.


* GUC is not the solution.

Which leaves us with either WITH INLINE or WITH MATERIALIZE, or 
something along those lines.


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


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


Perhaps combining this with a GUC would be a solution. I mean, a GUC 
specifying the default behavior, and then INLINE / MATERIALIZE for 
individual CTEs in a query?


If you have an application intentionally using CTEs as a fence, just do

ALTER DATABASE x SET enable_guc_fencing = on

and you don't have to rewrite the queries.

regards

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


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

Re: [HACKERS] Shared Memory hash tables only at startup

[Thomas Munro]

On Tue, May 2, 2017 at 11:54 PM, hariprasath nallasamy
 wrote:
> Hi
>   It is clear that all of the shared memory hash tables are created during
> startup using ShmemInitHash()
>
> (In practice, all creations are done in the postmaster
> process; child processes should always be attaching to existing tables.)
>
> Is there any specific reason to do so or my understanding was wrong(we can
> create shared memory hash table at run time too?)

Because we use processes and not threads, the space has to be
allocated up front and then inherited by other process in order to
inherit the same memory map so that raw pointers can be exchanged
between backends.  DynaHash is a data structure made out of raw
pointers.

I have a proposal[1] that would provide dynamic concurrent hash tables
that can be created after startup using DSM/DSA memory, but then you
have to deal with kooky pointers, which falls out of Postgres's
policies of avoiding multithreading and embracing parallelism.

[1] 
https://www.postgresql.org/message-id/flat/CAEepm%3D3d8o8XdVwYT6O%3DbHKsKAM2pu2D6sV1S_%3D4d%2BjStVCE7w%40mail.gmail.com

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


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

Re: [HACKERS] changing mvstats output to be valid JSON

[Alvaro Herrera]

Alvaro Herrera wrote:

> While writing the recent ext.stats docs, I became annoyed by the output
> functions of the new types used by multivariate statistics: they are
> almost JSON, but not quite.  Since they can become largish, I propose
> that we make a point of ensuring the output of those types is valid
> JSON, so that they can be processed by JSON tools, particularly by
> jsonb_pretty().

Pushed.

-- 
Á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] Bug in prepared statement cache invalidation?

[Tom Lane]

Robert Haas  writes:
> Yeah.  I think there should be a way to tell a PL to flush any
> internal caches it is maintaining, some variant of DISCARD.  But that
> would require a bunch of code that nobody's written yet.

That mechanism already exists, so far as the core code is concerned:
register a syscache inval callback.  But you're right, getting plpgsql
to actually do anything about changes in composite types would require
a bunch of code that nobody's written yet.

If you'll pardon my bashing on a long-deceased horse, this would be
noticeably easier if we stopped using the PLPGSQL_DTYPE_ROW code
paths for composite-type variables.  That mechanism was really
designed for cases like "SELECT ... INTO a,b,c" where the row
contents are fully determined by the function text.  It's practically
impossible to make it cope with dynamic changes; at the very least
you have to recompile the whole function.

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] Bug in prepared statement cache invalidation?

[Robert Haas]

On Tue, May 2, 2017 at 3:10 PM, Konstantin Knizhnik
 wrote:
> On 05/02/2017 09:30 PM, Robert Haas wrote:
>>> I am not sure how critical is this problem. Definitely it rarely happens,
>>> but lack of normal workarounds (restart backend, recreate function?)
>>> seems
>>> to be  disappointing.
>>
>> The problem goes away if you reconnect.  The problematic cache is only
>> backend-lifetime.
>>
> Most of clients are not connected to the Postgres directly, them are using
> some kind of connection pooling.
> It means that backends are never restarted. And it will be necessary to
> restart the whole service just because we do not have
> dependency tracking mechanism for PL code. Even invalidation of all
> functions in case of DDL seems to be more acceptable solution.

Yeah.  I think there should be a way to tell a PL to flush any
internal caches it is maintaining, some variant of DISCARD.  But that
would require a bunch of code that nobody's written yet.

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


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

Re: [HACKERS] multi-column range partition constraint

[Robert Haas]

On Tue, May 2, 2017 at 2:51 AM, Amit Langote
 wrote:
> Per an off-list report from Olaf Gawenda (thanks Olaf), it seems that the
> range partition's constraint is sometimes incorrect, at least in the case
> of multi-column range partitioning.  See below:
>
> create table p (a int, b int) partition by range (a, b);
> create table p1 partition of p for values from (1, 1) to (10 ,10);
> create table p2 partition of p for values from (11, 1) to (20, 10);
>
> Perhaps unusual, but it's still a valid definition.  Tuple-routing puts
> rows where they belong correctly.
>
> -- ok
> insert into p values (10, 9);
> select tableoid::regclass, * from p;
>  tableoid | a  | b
> --++---
>  p1   | 10 | 9
> (1 row)
>
> -- but see this
> select tableoid::regclass, * from p where a = 10;
>  tableoid | a | b
> --+---+---
> (0 rows)
>
> explain select tableoid::regclass, * from p where a = 10;
> QUERY PLAN
> ---
>  Result  (cost=0.00..0.00 rows=0 width=12)
>One-Time Filter: false
> (2 rows)
>
> -- or this
> insert into p1 values (10, 9);
> ERROR:  new row for relation "p1" violates partition constraint
> DETAIL:  Failing row contains (10, 9).
>
> This is because of the constraint being generated is not correct in this
> case.  p1's constraint is currently:
>
>   a >= 1 and a < 10
>
> where it should really be the following:
>
>   (a > 1  OR (a = 1  AND b >= 1))
> AND
>   (a < 10 OR (a = 10 AND b < 10))
>
> Attached patch rewrites get_qual_for_range() for the same, along with some
> code rearrangement for reuse.  I also added some new tests to insert.sql
> and inherit.sql, but wondered (maybe, too late now) whether there should
> really be a declarative_partition.sql for these, moving in some of the old
> tests too.
>
> Adding to the open items list.

I think there are more problems here.  With the patch:

rhaas=# create table p (a int, b int) partition by range (a, b);
CREATE TABLE
rhaas=# create table p1 partition of p for values from (unbounded,0)
to (unbounded,1);
CREATE TABLE
rhaas=# insert into p1 values (-2,-2);
ERROR:  new row for relation "p1" violates partition constraint
DETAIL:  Failing row contains (-2, -2).
rhaas=# insert into p1 values (2,2);
ERROR:  new row for relation "p1" violates partition constraint
DETAIL:  Failing row contains (2, 2).

Really, the whole CREATE TABLE .. PARTITION statement is meaningless
and should be disallowed, because it's not meaningful to have a
partition bound specification with a non-unbounded value following an
unbounded value.

BTW, I think we should also add a function that prints the partition
constraint, and have psql display that in the \d+ output, because
people might need that - e.g. if you want to attach a partition
without having to validate it, you need to be able to apply an
appropriate constraint to it in advance, so you'll want to see what
the existing partition constraints look like.

While I'm glad we have partitioning has a feature, I'm starting to get
a bit depressed by the number of bugs that are turning up here.  This
was committed in early December, and ideally ought to have been stable
long before now.

Since Amit is back from vacation May 8th, I'll update no later than May 9th.

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


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

Re: [HACKERS] Row Level Security UPDATE Confusion

[Stephen Frost]

Robert,

* Robert Haas (robertmh...@gmail.com) wrote:
> On Fri, Apr 14, 2017 at 9:16 AM, Stephen Frost  wrote:
> > I agreed already up-thread that there's an issue there and will be
> > looking to fix it.  That comment was simply replying to Rod's point that
> > the documentation could also be improved.
> 
> OK, thanks.  The wrap for the next set of minor releases is, according
> to my understanding, scheduled for Monday, so you'd better jump on
> this soon if you're hoping to get a fix out this time around.

Thanks for the reminder.  I do know the release is looming and I am
hoping to have a patch tomorrow or Thursday.

Thanks again!

Stephen


signature.asc
Description: Digital signature

Re: [HACKERS] pg_dump emits ALTER TABLE ONLY partitioned_table

[Stephen Frost]

Amit,

* Amit Langote (langote_amit...@lab.ntt.co.jp) wrote:
> Now that WITH OPTIONS is optional even for CREATE TABLE OF, perhaps it
> needs to be mentioned in the release notes?

Doesn't strike me as rising to the level of needing to go into the
release notes, but I won't object if people feel that it needs
mentioning.

> Attached updated patches.

Thanks, but aren't the changes for handling pg_dump --binary-upgrade
when dealing with partitions whose parents are in another schema
backwards?

The source schema selected is for the partition, so we don't need to
schema-qualify the partition, but we do need to schema-qualify the
parent because it could be in another schema.  I think the approach to
use is to decide, first, if we need to schema-qualify the parent or not,
then set up a string which has the qualified (if necessary) name of the
parent, and then just use that when appropriate while building the ALTER
TABLE command.  Remember, we select the source schema of the table in
tbinfo at the top of dumpTableSchema() (see 'selectSourceSchema()'), so
it shouldn't ever be necessary to schema-qualify the table in tbinfo.

I've somehow managed to run out of time again today (it's gotten quite
busy lately), but I'll try to find time late tonight to continue working
on this, or I'll be working on it again tomorrow.  This *really* needs
some tests that actually cover this case, as it's clearly not hard to
get confused about what needs to be qualified and what doesn't.

Thanks!

Stephen


signature.asc
Description: Digital signature

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

[Robert Haas]

On Tue, May 2, 2017 at 5:15 PM, Alvaro Herrera  wrote:
> Robert Haas wrote:
>> On Tue, May 2, 2017 at 12:25 PM, Alvaro Herrera
>>  wrote:
>> > 2) don't drop because we know it won't work.  I see two options:
>> >c) ignore a drop slot failure, i.e. don't cause a transaction abort.
>> >   An easy way to implement this is just add a PG_TRY block, but we
>> >   dislike adding those and not re-throwing the error.
>>
>> Dislike doesn't seem like the right word.  Unless you rollback a
>> (sub)transaction, none of the cleanup that would normally do is done,
>
> True.  So one possible implementation is that we open a subtransaction
> before dropping the slot, and we abort it if things go south.  This is a
> bit slower, but not critically so.

I think that could work.  Subtransaction abort isn't as fast as I
would sometimes like, but for a DDL command the overhead is pretty
insignificant.

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


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

Re: [HACKERS] [GENERAL] Column rename in an extension update script

[Tom Lane]

Julien Rouhaud  writes:
> moving this thread to -hackers, since this looks like a bug.
> On 01/05/2017 08:54, Philippe BEAUDOIN wrote:
>> I am coding an update script for an extension. And I am in trouble when
>> trying to rename a column of an existing table.
>> 
>> Just after the ALTER TABLE statement, I want to access this table. But
>> at this time, the altered column is not visible with its new name.

> I can reproduce this issue.

> It looks like this is due to missing cache invalidation between the
> ALTER TABLE execution and next query planning (failure happens during
> pg_analyze_and_rewrite()).

Yes.  Kind of surprising nobody noticed this before --- you'd certainly
think that extension scripts would have lots of cases of statements
depending on DDL done just before them.  I think it must have been masked
by the fact that many DDL commands *end* with CommandCounterIncrement,
or at least have one pretty close to the end.  But not renameatt().

> AFAICT, CommandCounterIncrement() is responsible for handling
> invalidation messages, but in execute_sql_string() this function is
> called before executing a Stmt instead of after.  Moving the
> CommandCounterIncrement() just before the PopActiveSnapshot() call (and
> removing the final one) fixes this issue for me, but I have no idea if
> this is the right fix.

I'm inclined to add one before the parse step, rather than removing any.
The sequence of bump-the-command-counter-then-capture-a-snapshot is
pretty well established in places like spi.c, so I don't want to change
that usage.  Also, I think part of the point here was to ensure that
any DDL done *before* reaching execute_sql_string() is visible to the
first command.  Also note the assumption in ApplyExtensionUpdates that
execute_sql_string will do at least one CommandCounterIncrement, even
if the script is empty.  A CCI that has nothing to do is quite cheap,
and we're not that worried about performance here anyway IMO, so I'd
just as soon err on the side of having more than enough CCIs.

regards, tom lane


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

Re: [HACKERS] CTE inlining

[Merlin Moncure]

\On Tue, May 2, 2017 at 12:05 PM, Tomas Vondra
 wrote:
> On 5/2/17 6:34 PM, David Fetter wrote:
>>
>> On Tue, May 02, 2017 at 02:40:55PM +0200, Andreas Karlsson wrote:
>>>
>>> On 05/02/2017 04:38 AM, Craig Ringer wrote:

 On 1 May 2017 at 22:26, Andreas Karlsson  wrote:
>
>>>
>>>
>>> ...
>>>
>>> I see some alternatives, none of them perfect.
>>>
>>> 1. Just remove the optimization fence and let people add OFFSET 0 to
>>> their
>>> queries if they want an optimization fence. This lets us keep pretending
>>> that we do not have query hints (and therefore do not have to formalize
>>> any
>>> syntax for them) while still allowing people to add optimization fences.
>>
>>
>> +1
>>
>> I get that people with gigantic PostgreSQL installations with
>> stringent performance requirements sometimes need to do odd things to
>> squeeze out the last few percentage points of performance.  As the
>> people (well, at least the people close to the ground) at these
>> organizations are fully aware, performance optimizations are extremely
>> volatile with respect to new versions of software, whether it's
>> PostgreSQL, Oracle, the Linux kernel, or what have you.  They expect
>> this, and they have processes in place to handle it.  If they don't,
>> it's pilot error.
>>
>> We should not be penalizing all our other users to maintain the
>> fiction that people can treat performance optimizations as a "fire and
>> forget" matter.
>>
>
> Agreed.
>
>>> 2. Add a decorator for WITH (e.g. "WITH MATERIALIZED x (...") to add an
>>> explicit optimization fence. This will for the first time add official
>>> support for a query hint in the syntax which is a quite big precedent.
>>
>>
>> Yep.  It's one we should think very carefully before we introduce.
>>
>
> I think it's a mistake to see this as an introduction of query hits.
>
> Firstly, it's a question whether it qualifies as a hint. I wouldn't call it
> a hint, but let's assume there is a definition of query hints that includes
> WITH MATERIALIZED.
>
> More importantly, however, this is not introducing anything new. It's just a
> different name for the current "WITH" semantics, and you can achieve the
> same behavior by "OFFSET 0". And people are already using these as hints, so
> I fail to see how this introduces anything new.
>
> In fact, if you see the optimization fence as an implicit query hint, this
> actually *removes* a hint (although most users are unaware of that behavior
> and use it unintentionally).

+1 down the line.  More to the point, for several years now we've (or
at least I, but I'm not the only one) have been advocating for the
usage of CTE to avoid the undocumented and bizarre OFFSET 0 trick.
Jerking this out from users without giving a simple mechanic to get
the same behavior minus a major query rewrite is blatantly user
hostile.  I can't believe we're even contemplating it.   Also a GUC is
not a solution for pretty obvious reasons I think.

merlin


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

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

[Alvaro Herrera]

Robert Haas wrote:
> On Tue, May 2, 2017 at 12:25 PM, Alvaro Herrera
>  wrote:
> > 2) don't drop because we know it won't work.  I see two options:
> >c) ignore a drop slot failure, i.e. don't cause a transaction abort.
> >   An easy way to implement this is just add a PG_TRY block, but we
> >   dislike adding those and not re-throwing the error.
> 
> Dislike doesn't seem like the right word.  Unless you rollback a
> (sub)transaction, none of the cleanup that would normally do is done,

True.  So one possible implementation is that we open a subtransaction
before dropping the slot, and we abort it if things go south.  This is a
bit slower, but not critically so.

-- 
Á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] logical replication syntax (was DROP SUBSCRIPTION, query cancellations and slot handling)

[Robert Haas]

On Tue, May 2, 2017 at 3:02 PM, Petr Jelinek
 wrote:
> That sounds okay. I know PeterE didn't like the lower case and
> underscore separated words for options in the original patch, so I'd
> like to hear his opinion on this. I am not sure how much advantage is
> there in removing the '=' in between the key and value. That's the main
> difference between generic options and definitions (well and definitions
> can have 2 words for key, but that's something I have added anyway), I
> don't really understand why we have both and use one for some commends
> and the other for others btw.

I don't care all *that* much about the equals sign, although I think
it's marginally more elegant without it, and VACUUM, EXPLAIN, and COPY
all do it that way.  But I think allowing two-word names is just a
pile of parser nastiness that we'd be far better off without.  If you
use the same syntax as VACUUM, EXPLAIN, and COPY, all options are a
single identifier.  If it's got to be multiple words, they can be
separated by underscores.  But with what you've got right now, it's
sometimes one identifier even when it's two words, and sometimes two
identifiers.  When it's three words, it's two identifiers, with two of
them concatenated.

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


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

Re: [HACKERS] [GENERAL] Column rename in an extension update script

[Julien Rouhaud]

moving this thread to -hackers, since this looks like a bug.

On 01/05/2017 08:54, Philippe BEAUDOIN wrote:
> Hi all,
> 
> I am coding an update script for an extension. And I am in trouble when
> trying to rename a column of an existing table.
> 
> Just after the ALTER TABLE statement, I want to access this table. But
> at this time, the altered column is not visible with its new name.
> 

I can reproduce this issue.

It looks like this is due to missing cache invalidation between the
ALTER TABLE execution and next query planning (failure happens during
pg_analyze_and_rewrite()).

AFAICT, CommandCounterIncrement() is responsible for handling
invalidation messages, but in execute_sql_string() this function is
called before executing a Stmt instead of after.  Moving the
CommandCounterIncrement() just before the PopActiveSnapshot() call (and
removing the final one) fixes this issue for me, but I have no idea if
this is the right fix.

-- 
Julien Rouhaud
http://dalibo.com - http://dalibo.org


-- 
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] Row Level Security UPDATE Confusion

[Robert Haas]

On Fri, Apr 14, 2017 at 9:16 AM, Stephen Frost  wrote:
> I agreed already up-thread that there's an issue there and will be
> looking to fix it.  That comment was simply replying to Rod's point that
> the documentation could also be improved.

OK, thanks.  The wrap for the next set of minor releases is, according
to my understanding, scheduled for Monday, so you'd better jump on
this soon if you're hoping to get a fix out this time around.

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


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

Re: [HACKERS] modeling parallel contention (was: Parallel Append implementation)

[Robert Haas]

On Tue, Apr 18, 2017 at 2:48 AM, Amit Khandekar  wrote:
> After searching through earlier mails about parallel scan, I am not
> sure whether the shared state was considered to be a potential factor
> that might reduce parallel query gains, when deciding the calculation
> for number of workers for a parallel seq scan. I mean even today if we
> allocate 10 workers as against a calculated 4 workers count for a
> parallel seq scan, they might help. I think it's just that we don't
> know if they would *always* help or it would regress sometimes.

No, that's not considered, currently.  This is actually an issue even
for nodes that are not parallel-aware at all.  For example, consider
this:

Hash Join
-> Parallel Seq Scan
-> Hash
  -> Seq Scan

It is of course possible that the Parallel Seq Scan could run into
contention problems if the number of workers is large, but in my
experience there are bigger problems here.  The non-parallel Seq Scan
can also contend -- not of course over the shared mutex because there
isn't one, but over access to the blocks themselves.  Every one of
those blocks has a content lock and a buffer header and so on, and
having multiple processes accessing those things at the same time
scales well, but not perfectly.  The Hash node can also contend: if
the hash join spills to disk, you've got multiple processes reading
and writing to the temp directory at the same time and, of course,
that can be worse than just one process doing it -- sometimes much
worse.  It can also be better, depending on how much I/O gets
generated and how much I/O bandwidth you have.

The main things that keeps this from being a crippling issue right now
is the fact that we tend not to use that many parallel workers in the
first place.  We're trying to scale a query that would otherwise use 1
process out to 3 or 5 processes, and so the contention effects, in
many cases, are not too bad.  Multiple people (including David Rowley
as well as folks here at EnterpriseDB) have demonstrated that for
certain queries, we can actually use a lot more workers and everything
works great.  The problem is that for other queries, using a lot of
workers works terribly.  The planner doesn't know how to figure out
which it'll be - and honestly, I don't either.

/me crosses fingers, hopes someone smarter will work on this problem.

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


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

Re: [HACKERS] Bug in prepared statement cache invalidation?

[Konstantin Knizhnik]

On 05/02/2017 09:30 PM, Robert Haas wrote:

I am not sure how critical is this problem. Definitely it rarely happens,
but lack of normal workarounds (restart backend, recreate function?) seems
to be  disappointing.

The problem goes away if you reconnect.  The problematic cache is only
backend-lifetime.


Most of clients are not connected to the Postgres directly, them are using some 
kind of connection pooling.
It means that backends are never restarted. And it will be necessary to restart 
the whole service just because we do not have
dependency tracking mechanism for PL code. Even invalidation of all functions 
in case of DDL seems to be more acceptable solution.

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

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

[Petr Jelinek]

On 02/05/17 19:42, Robert Haas wrote:
> On Tue, May 2, 2017 at 8:45 AM, Petr Jelinek
>  wrote:
>> I am happy to implement something different, it's quite trivial to
>> change. But I am not going to propose anything different as I can't
>> think of better syntax (if I could I would have done it). I don't like
>> the OFF or FALSE (ie DROP SLOT OFF) any more than what is there now and
>> it also seems to not map very well to action (as opposed to output
>> option as it is in EXPLAIN). It might not be very close to SQL way but
>> that's because SQL way would be do not do any of those default actions
>> unless they are actually asked for (ie NODROP SLOT would be default and
>> DROP SLOT would be the option) but that's IMHO less user friendly.
> 
> So the cases where this "NO" prefixing comes up are:
> 
> 1. CREATE SUBSCRIPTION
> 
> where option can
> be:
> 
> | ENABLED | DISABLED
> | CREATE SLOT | NOCREATE SLOT
> | SLOT NAME = slot_name
> | COPY DATA | NOCOPY DATA
> | SYNCHRONOUS_COMMIT =  class="PARAMETER">synchronous_commit
> | NOCONNECT
> 
> I think it would have been a lot better to use the extensible options
> syntax for this instead of inventing something new that's not even
> consistent with itself.

I am not sure what you mean by this, we always have to invent option
names if they are new options, even if we use generic options (which I
guess is what you mean by "extensible options syntax"). I used the
definitions instead of generic options, this means that the supported
syntax also includes COPY DATA = true/false, CREATE SLOT = true/false
etc, the NO* are just shorthands, it's quite simple to remove those.

> You've got SYNCHRONOUS_COMMIT with a hyphen
> and CREATE SLOT with no hyphen and NOCOPY DATA with no hyphen and a
> space left out. With the extensible options syntax, this would be
> (enabled true/false, create_slot true/false, slot_name whatever,
> synchronous_commit true/false, connect true/false). If we're going to
> keep the present monstrosity, we can I think still change NOCONNECT to
> NO CONNECT, but there's no fixing NOCOPY DATA in this syntax model.

See above.

> 
> 2. ALTER SUBSCRIPTION
> 
> ALTER SUBSCRIPTION name SET PUBLICATION publication_name [, ...] {
> REFRESH WITH ( puboption [, ... ] ) | NOREFRESH }
> 
> There is no obvious reason why this could not have been spelled NO
> REFRESH instead of adding a new keyword.
> 
> 3. DROP SUBSCRIPTION
> 
> DROP SUBSCRIPTION [ IF EXISTS ] name [ DROP SLOT | NODROP SLOT ]
> 
> This is where we started, and I have nothing to add to what I (and
> Tom) have already said.
> 
> 4. CREATE PUBLICATION
> 
> CREATE PUBLICATION name
> [ FOR TABLE [ ONLY ]  class="parameter">table_name [ * ] [, ...]
>   | FOR ALL TABLES ]
> [ WITH ( option [, ... ] ) ]
> 
> where option can
> be:
> 
>   PUBLISH INSERT | NOPUBLISH INSERT
> | PUBLISH UPDATE | NOPUBLISH UPDATE
> | PUBLISH DELETE | NOPUBLISH DELETE
> 
> Again, the extensible options syntax like we use for EXPLAIN would
> have been better here.  You could have said (publish_insert
> true/false, publish_update true/false, publish_delete true/false), for
> instance, or combined them into a single option like (publish
> 'insert,update') to omit deletes.
> 
> So it doesn't actually look hard to get rid of all of the NO prefixes.
> 

That sounds okay. I know PeterE didn't like the lower case and
underscore separated words for options in the original patch, so I'd
like to hear his opinion on this. I am not sure how much advantage is
there in removing the '=' in between the key and value. That's the main
difference between generic options and definitions (well and definitions
can have 2 words for key, but that's something I have added anyway), I
don't really understand why we have both and use one for some commends
and the other for others btw.


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

[Heikki Linnakangas]

On 05/02/2017 09:57 PM, Robert Haas wrote:

Does db_user_namespace work with SCRAM?


Yes. Haven't tested it, come to think of it, but it should work.

- Heikki



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

Re: [HACKERS] Ongoing issues with representation of empty arrays

[Robert Haas]

On Tue, Apr 11, 2017 at 12:17 AM, Andrew Gierth
 wrote:
>> "Tom" == Tom Lane  writes:
>
>  >> First is contrib/intarray, _AGAIN_ (see past bugs such as #7730):
>  >> ...
>  >> I plan to fix this one properly, unless anyone has any objections.
>
>  Tom> Just to clarify, what do you think is "properly"?
>
> I would say, that any time an intarray function returns an empty result
> it should be the standard 0-dimensional representation that every other
> array operation uses.  The intarray functions all seem already able to
> take such values as inputs.  Also there should be regression tests for
> this (none of intarray's existing tests have any empty arrays at all).

This all sounds reasonable to me.

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


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

Re: [HACKERS] SCRAM authentication, take three

[Robert Haas]

On Tue, Apr 18, 2017 at 7:58 AM, Magnus Hagander  wrote:
>> Yeah, that would be reasonable. It can't be called just "password",
>> though, because there's no way to implement "password-or-md5-or-scram" in a
>> sensible way (see my reply to Simon at [1]). Unless we remove the support
>> for what "password" does today altogether, and redefine "password" to mean
>> just "md5-or-beyond". Which might not be a bad idea, but that's a separate
>> discussion.
>
> It is an interesting one though. "password" today is really only useful in
> the case of db_user_namespace=on, right? Given the very few people I think
> are using that feature, it wouldn't be unreasonable to rename it to
> something more closely related to that.

I think it would be nice to have something with the same functionality
as db_user_namespace that smells less like a giant hack.

Does db_user_namespace work with SCRAM?

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


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

Re: [HACKERS] Typos in comments in partition.c

[Robert Haas]

On Tue, May 2, 2017 at 5:58 AM, Etsuro Fujita
 wrote:
> I ran into typos in comments in RelationGetPartitionDispatchInfo() in
> partition.c.  Here is a small patch for: s/all the leaf partition/all the
> leaf partitions/ and s/we we/we/.

Committed.

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


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

Re: [HACKERS] Logical replication - TRAP: FailedAssertion in pgstat.c

[Robert Haas]

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

Is there anything left to be committed here?

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


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

Re: [HACKERS] Bug in prepared statement cache invalidation?

[Robert Haas]

On Tue, May 2, 2017 at 6:07 AM, Konstantin Knizhnik
 wrote:
> Thank you for explanation.
> May be I am missing something, but what is the problem with keeping
> dependencies for  PL functions?
> As you wrote, PL can inform core that functions depends on some set of
> relations/types/functions and so has to be recompiled if some of them are
> changed.
> It is not necessary to build closure of dependency graph - instead of it
> cascade invalidation can be used.
> So it is not clear to me where you see here the source of complexity and why
> this task may be "Turing-complete in some cases"?

Well, if the user's using PL/python or similar, they can do an
arbitrary computation and use the result as a table name.  There's no
way for the core system to know what table name that will end up
referencing.

> The problem can be with overloaded functions and PL languages without static
> type checking.
> In this case  resolving has to be performed at runtime during function
> evaluation. But there should be no such problem with PLpgSQL.

Yeah, maybe.  But what if the PL/pgsql function calls some other
function that creates a table, and then tries to access that table
from the original function, or something like that?  There are weird
cases, I think, where even in PL/pgSQL it's not easy to figure out for
sure what the dependencies are.

> I am not sure how critical is this problem. Definitely it rarely happens,
> but lack of normal workarounds (restart backend, recreate function?) seems
> to be  disappointing.

The problem goes away if you reconnect.  The problematic cache is only
backend-lifetime.

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


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

Re: [HACKERS] Cached plans and statement generalization

[Robert Haas]

On Tue, May 2, 2017 at 5:50 AM, Konstantin Knizhnik
 wrote:
>> I don't see something with a bunch of hard-coded rules for particular type
>> OIDs having any chance of being acceptable.
>
> Well, what I need is ...

Regarding this...

> Definitely copying of code is bad flaw. It will be much better and easier
> just to call three original functions instead of mixing gathering their code
> into the new function.
> But I failed to do it because ...

...and also this:

I am sympathetic to the fact that this is a hard problem to solve.
I'm just telling you that the way you've got it is not acceptable and
nobody's going to commit it like that (or if they do, they will end up
having to revert it).  If you want to have a technical discussion
about what might be a way to change the patch to be more acceptable,
cool, but I don't want to get into a long debate about whether what
you have is acceptable or not; I've already said what I think about
that and I believe that opinion will be widely shared.  I am not
trying to beat you up here, just trying to be clear.

> Thanks for your feedback.

Sure thing.

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


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

Re: [HACKERS] Declarative partitioning - another take

[Robert Haas]

On Tue, May 2, 2017 at 3:30 AM, Amit Langote
 wrote:
> You're right.  I agree that whatever text we add here should be pointing
> out that statement-level triggers of affected child tables are not fired,
> when root parent is specified in the command.
>
> Since there was least some talk of changing that behavior for regular
> inheritance so that statement triggers of any affected children are fired
> [1], I thought we shouldn't say something general that applies to both
> inheritance and partitioning.  But since nothing has happened in that
> regard, we might as well.
>
> How about the attached?

Looks better, but I think we should say "statement" instead of
"operation" for consistency with the previous paragraph, and it
certainly shouldn't be capitalized.

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


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

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

[Robert Haas]

On Tue, May 2, 2017 at 8:45 AM, Petr Jelinek
 wrote:
> I am happy to implement something different, it's quite trivial to
> change. But I am not going to propose anything different as I can't
> think of better syntax (if I could I would have done it). I don't like
> the OFF or FALSE (ie DROP SLOT OFF) any more than what is there now and
> it also seems to not map very well to action (as opposed to output
> option as it is in EXPLAIN). It might not be very close to SQL way but
> that's because SQL way would be do not do any of those default actions
> unless they are actually asked for (ie NODROP SLOT would be default and
> DROP SLOT would be the option) but that's IMHO less user friendly.

So the cases where this "NO" prefixing comes up are:

1. CREATE SUBSCRIPTION

where option can
be:

| ENABLED | DISABLED
| CREATE SLOT | NOCREATE SLOT
| SLOT NAME = slot_name
| COPY DATA | NOCOPY DATA
| SYNCHRONOUS_COMMIT = synchronous_commit
| NOCONNECT

I think it would have been a lot better to use the extensible options
syntax for this instead of inventing something new that's not even
consistent with itself. You've got SYNCHRONOUS_COMMIT with a hyphen
and CREATE SLOT with no hyphen and NOCOPY DATA with no hyphen and a
space left out. With the extensible options syntax, this would be
(enabled true/false, create_slot true/false, slot_name whatever,
synchronous_commit true/false, connect true/false). If we're going to
keep the present monstrosity, we can I think still change NOCONNECT to
NO CONNECT, but there's no fixing NOCOPY DATA in this syntax model.

2. ALTER SUBSCRIPTION

ALTER SUBSCRIPTION name SET PUBLICATION publication_name [, ...] {
REFRESH WITH ( puboption [, ... ] ) | NOREFRESH }

There is no obvious reason why this could not have been spelled NO
REFRESH instead of adding a new keyword.

3. DROP SUBSCRIPTION

DROP SUBSCRIPTION [ IF EXISTS ] name [ DROP SLOT | NODROP SLOT ]

This is where we started, and I have nothing to add to what I (and
Tom) have already said.

4. CREATE PUBLICATION

CREATE PUBLICATION name
[ FOR TABLE [ ONLY ] table_name [ * ] [, ...]
  | FOR ALL TABLES ]
[ WITH ( option [, ... ] ) ]

where option can
be:

  PUBLISH INSERT | NOPUBLISH INSERT
| PUBLISH UPDATE | NOPUBLISH UPDATE
| PUBLISH DELETE | NOPUBLISH DELETE

Again, the extensible options syntax like we use for EXPLAIN would
have been better here.  You could have said (publish_insert
true/false, publish_update true/false, publish_delete true/false), for
instance, or combined them into a single option like (publish
'insert,update') to omit deletes.

So it doesn't actually look hard to get rid of all of the NO prefixes.

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


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

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

[Robert Haas]

On Tue, May 2, 2017 at 12:25 PM, Alvaro Herrera
 wrote:
> 2) don't drop because we know it won't work.  I see two options:
>c) ignore a drop slot failure, i.e. don't cause a transaction abort.
>   An easy way to implement this is just add a PG_TRY block, but we
>   dislike adding those and not re-throwing the error.

Dislike doesn't seem like the right word.  Unless you rollback a
(sub)transaction, none of the cleanup that would normally do is done,
so you might leak buffer pins, memory, or other resources.  Unless the
code that can be run in the TRY/CATCH block is sufficiently restricted
as to make that a non-issue, which is rarely the case, it's not going
to work reliably at all.  I wonder why this API was even designed in a
way that made not re-throwing the error an option.

(I've wondered whether we should have some kind of mini-transaction
that is cheaper to abort but does only a critical subset of the
cleanup, but I haven't been able to figure out how you'd know whether
you only need to blow up the mini-transaction or whether you need to
kill the enclosing real (sub)transaction.)

>d) rethink drop slot completely; maybe instead of doing it
>   immediately, it should be a separate task, so we first close the
>   current transaction (which dropped the subscription) and then we open
>   a second one to drop the slot, so that if the drop slot fails, the
>   subscription does not come back to life.

Something like this might work, although it's not clear how it
interacts with DROP .. CASCADE.  See
http://postgr.es/m/ca+tgmob_hy0uqs9vq_9rdbgjpww3d3jbz6twakzowazigo4...@mail.gmail.com
for a very related point about adding subscriptions.

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


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

Re: [HACKERS] Potential hot-standby bug around xacts committed but in xl_running_xacts

[Simon Riggs]

On 2 May 2017 at 18:06, Andres Freund  wrote:

>> What I suggest is that with logical decoding in mind we do this
>> 1. Inject a new record XLOG_SNAPSHOT_START at the start of
>> LogStandbySnapshot(). We start logical decoding from there.
>> 2. Record any transactions that end
>> 3. Now the full XLOG_RUNNING_XACTS record arrives. We apply all xacts
>> that are seen as running, minus any ended between 1 and 3
>
>> This avoids the problems for the race but without holding locks while
>> we log XLOG_RUNNING_XACTS, something that was considered painful for
>> Hot Standby.
>
> I don't think that really solves it, because other transactions could
> just be stuck just after the XLogInsert() forever.  And it'd have the
> issue of having to backpatch a new record.  I'm working on an
> alternative approach, let's hope that that works out.

Backpatchable approach.

1. Record CurrentInsertPosition()
2. LogStandbySnapshot()
3. Insert custom logical wal message containing currentinsertposition
and LSN of (2)

When we decode
1. Read standby snapshot
2. Scan forwards until we see message written by step (3) above, which
is identifiable because it contains LSN of snapshot.
3. Read initial LSN from message then re-scan from LSN until
xl_running_xacts message collecting any commits/aborts and removing
them from snapshot.

No new WAL messages, no locking problem, race condition handled.

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


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

Re: [HACKERS] CTE inlining

[Tomas Vondra]

On 5/2/17 6:34 PM, David Fetter wrote:

On Tue, May 02, 2017 at 02:40:55PM +0200, Andreas Karlsson wrote:

On 05/02/2017 04:38 AM, Craig Ringer wrote:

On 1 May 2017 at 22:26, Andreas Karlsson  wrote:

>>

...

I see some alternatives, none of them perfect.

1. Just remove the optimization fence and let people add OFFSET 0 to their
queries if they want an optimization fence. This lets us keep pretending
that we do not have query hints (and therefore do not have to formalize any
syntax for them) while still allowing people to add optimization fences.


+1

I get that people with gigantic PostgreSQL installations with
stringent performance requirements sometimes need to do odd things to
squeeze out the last few percentage points of performance.  As the
people (well, at least the people close to the ground) at these
organizations are fully aware, performance optimizations are extremely
volatile with respect to new versions of software, whether it's
PostgreSQL, Oracle, the Linux kernel, or what have you.  They expect
this, and they have processes in place to handle it.  If they don't,
it's pilot error.

We should not be penalizing all our other users to maintain the
fiction that people can treat performance optimizations as a "fire and
forget" matter.



Agreed.


2. Add a decorator for WITH (e.g. "WITH MATERIALIZED x (...") to add an
explicit optimization fence. This will for the first time add official
support for a query hint in the syntax which is a quite big precedent.


Yep.  It's one we should think very carefully before we introduce.



I think it's a mistake to see this as an introduction of query hits.

Firstly, it's a question whether it qualifies as a hint. I wouldn't call 
it a hint, but let's assume there is a definition of query hints that 
includes WITH MATERIALIZED.


More importantly, however, this is not introducing anything new. It's 
just a different name for the current "WITH" semantics, and you can 
achieve the same behavior by "OFFSET 0". And people are already using 
these as hints, so I fail to see how this introduces anything new.


In fact, if you see the optimization fence as an implicit query hint, 
this actually *removes* a hint (although most users are unaware of that 
behavior and use it unintentionally).




3. Add a new GUC which can enable and disable the optimization fence. This
is a very clumsy tool, but maybe good enough for some users and some people
here in this thread have complained about our similar GUCs.


Any GUC would be unable to distinguish one WITH clause from another.
The hammer would then be guaranteed to be too big for precisely the
cases where it's most needed.



If I could, I'd give -1 million to a GUC-based approach, as that would 
make it entirely unusable in practice, I think.


Actually, I can give -1 million, so I'm giving it.

>>

4. Add some new more generic query hinting facility. This is a lot
of work and something which would be very hard to get consensus for.


Just the design of the thing would be the work of months at a minimum,
assuming we got to some consensus at all.  Maybe it's worth doing.



While I came to conclusion that query hints may be quite useful in some 
situations, I'm pretty sure this is not a battle you'd like to fight.



regards

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


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

Re: [HACKERS] CTE inlining

[Corey Huinker]

>
> I get that people with gigantic PostgreSQL installations with
> stringent performance requirements sometimes need to do odd things to
> squeeze out the last few percentage points of performance.  As the
> people (well, at least the people close to the ground) at these
> organizations are fully aware, performance optimizations are extremely
> volatile with respect to new versions of software, whether it's
> PostgreSQL, Oracle, the Linux kernel, or what have you.  They expect
> this, and they have processes in place to handle it.  If they don't,
> it's pilot error.
>

Well put. People on the ground in those situations go to great lengths to
freeze the query plan as-is. For them, an upgrade is something that is done
after months of planning. They might be surprised by the dropping of this
optimization fence, but the surprise won't be in production, and they've
got just as good of chance of being pleasantly surprised.


> > 2. Add a decorator for WITH (e.g. "WITH MATERIALIZED x (...") to add an
> > explicit optimization fence. This will for the first time add official
> > support for a query hint in the syntax which is a quite big precedent.
>
> Yep.  It's one we should think very carefully before we introduce.
>

There's a tiny, oblique precedence for this with Oracle's WITH [FUNCTION |
PROCEDURE] syntax. In both cases, the user is able to create an ephemeral
object that can be referenced later in the query. Good idea or bad, it's a
sign that others have been fine with that conceptual path.

Personally, I'm fine with WITH MATERIALIZED, but I'm also ok with just not
promising the fence. I think there is value in letting users break up a
complex query into understandable WITH-chunks, and that value shouldn't
prevent good performance. The fence will probably still be there anyway in
the case of INSERT/UPDATE RETURNING and cases where a CTE is referenced
more than once in the query that follows.

Re: [HACKERS] scram and \password

[Robert Haas]

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

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

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


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

Re: [HACKERS] [PROPOSAL] Use SnapshotAny in get_actual_variable_range

[Vladimir Borodin]

Hi.

> 25 апр. 2017 г., в 18:13, Dmitriy Sarafannikov  
> написал(а):
> 
> I'd like to propose to search min and max value in index with SnapshotAny in 
> get_actual_variable_range function.
> Current implementation scans index with SnapshotDirty which accepts 
> uncommitted rows and rejects dead rows.

The patch is already being discussed here [1].

[1] 
https://www.postgresql.org/message-id/05C72CF7-B5F6-4DB9-8A09-5AC897653113%40yandex.ru



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

Re: [HACKERS] CTE inlining

[David Fetter]

On Tue, May 02, 2017 at 02:40:55PM +0200, Andreas Karlsson wrote:
> On 05/02/2017 04:38 AM, Craig Ringer wrote:
> > On 1 May 2017 at 22:26, Andreas Karlsson  wrote:
> > > I am not sure I like decorators since this means adding an ad hoc query 
> > > hint
> > > directly into the SQL syntax which is something which I requires serious
> > > consideration.
> > 
> > And mangling the semantics of existing syntax doesn't?
> > 
> > That's what we do right now so we can pretend we don't have query
> > hints while still having query hints.
> 
> I am in favor of removing the optimization fence from CTEs, and strongly
> prefer no fence being the default behavior since SQL is a declarative
> language and I think it is reasonable to assume that CTEs can be inlined.
> But the question is how to best remove the fence while taking into account
> that quite many use them as optimization fences today.
> 
> I see some alternatives, none of them perfect.
> 
> 1. Just remove the optimization fence and let people add OFFSET 0 to their
> queries if they want an optimization fence. This lets us keep pretending
> that we do not have query hints (and therefore do not have to formalize any
> syntax for them) while still allowing people to add optimization fences.

+1

I get that people with gigantic PostgreSQL installations with
stringent performance requirements sometimes need to do odd things to
squeeze out the last few percentage points of performance.  As the
people (well, at least the people close to the ground) at these
organizations are fully aware, performance optimizations are extremely
volatile with respect to new versions of software, whether it's
PostgreSQL, Oracle, the Linux kernel, or what have you.  They expect
this, and they have processes in place to handle it.  If they don't,
it's pilot error.

We should not be penalizing all our other users to maintain the
fiction that people can treat performance optimizations as a "fire and
forget" matter.

> 2. Add a decorator for WITH (e.g. "WITH MATERIALIZED x (...") to add an
> explicit optimization fence. This will for the first time add official
> support for a query hint in the syntax which is a quite big precedent.

Yep.  It's one we should think very carefully before we introduce.

> 3. Add a new GUC which can enable and disable the optimization fence. This
> is a very clumsy tool, but maybe good enough for some users and some people
> here in this thread have complained about our similar GUCs.

Any GUC would be unable to distinguish one WITH clause from another.
The hammer would then be guaranteed to be too big for precisely the
cases where it's most needed.

> 4. Add some new more generic query hinting facility. This is a lot
> of work and something which would be very hard to get consensus for.

Just the design of the thing would be the work of months at a minimum,
assuming we got to some consensus at all.  Maybe it's worth doing.

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

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


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

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

[Alvaro Herrera]

Petr Jelinek wrote:
> On 02/05/17 18:00, Robert Haas wrote:
> > On Tue, May 2, 2017 at 11:49 AM, Alvaro Herrera
> >  wrote:
> >> Petr Jelinek wrote:
> >>> So the only way to fulfill the requirement you stated is to just not try
> >>> to drop the slot, ever, on DROP SUBSCRIPTION. That makes the default
> >>> behavior leave resources on upstream that will eventually cause that
> >>> server to stop unless user notices before. I think we better invent
> >>> something that limits how much inactive slots can hold back WAL and
> >>> catalog_xmin in this release as well then.
> >>
> >> I don't understand why isn't the default behavior to unconditionally
> >> drop the slot.  Why do we ever want the slot to be kept?
> > 
> > What if the remote server doesn't exist any more?
> 
> Or what if the slot is used by other subscription (because you restored
> dump containing subscriptions to another server for example), or you
> have server that does not have outside network access anymore, or many
> other reasons.

So there are two different scenarios: one is that you expect the slot
drop to fail for whatever reason; the other is that you want the slot to
be kept because it's needed for something else.  Maybe those two should
be considered separately.

1) keep the slot because it's needed for something else.
   I see two options:
   a) change the something else so that it uses another slot with the
  same location.  Do we have some sort of "clone slot" operation?
   b) have an option to dissociate the slot from the subscription prior
  to the drop.

2) don't drop because we know it won't work.  I see two options:
   c) ignore a drop slot failure, i.e. don't cause a transaction abort.
  An easy way to implement this is just add a PG_TRY block, but we
  dislike adding those and not re-throwing the error.
   d) rethink drop slot completely; maybe instead of doing it
  immediately, it should be a separate task, so we first close the
  current transaction (which dropped the subscription) and then we open
  a second one to drop the slot, so that if the drop slot fails, the
  subscription does not come back to life.

-- 
Á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] logical replication syntax (was DROP SUBSCRIPTION, query cancellations and slot handling)

[Petr Jelinek]

On 02/05/17 18:00, Robert Haas wrote:
> On Tue, May 2, 2017 at 11:49 AM, Alvaro Herrera
>  wrote:
>> Petr Jelinek wrote:
>>> So the only way to fulfill the requirement you stated is to just not try
>>> to drop the slot, ever, on DROP SUBSCRIPTION. That makes the default
>>> behavior leave resources on upstream that will eventually cause that
>>> server to stop unless user notices before. I think we better invent
>>> something that limits how much inactive slots can hold back WAL and
>>> catalog_xmin in this release as well then.
>>
>> I don't understand why isn't the default behavior to unconditionally
>> drop the slot.  Why do we ever want the slot to be kept?
> 
> What if the remote server doesn't exist any more?
> 

Or what if the slot is used by other subscription (because you restored
dump containing subscriptions to another server for example), or you
have server that does not have outside network access anymore, or many
other reasons.

-- 
  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] Re: Potential hot-standby bug around xacts committed but in xl_running_xacts

[Andres Freund]

On 2017-05-02 07:12:41 +0200, Simon Riggs wrote:
> /*
>  * The running-xacts snapshot can contain xids that were still visible
>  * in the procarray when the snapshot was taken, but were already
>  * WAL-logged as completed. They're not running anymore, so ignore
>  * them.
>  */
>  if (TransactionIdDidCommit(xid) || TransactionIdDidAbort(xid))
>  continue;

Ah, right.   Phew ;)


> What I suggest is that with logical decoding in mind we do this
> 1. Inject a new record XLOG_SNAPSHOT_START at the start of
> LogStandbySnapshot(). We start logical decoding from there.
> 2. Record any transactions that end
> 3. Now the full XLOG_RUNNING_XACTS record arrives. We apply all xacts
> that are seen as running, minus any ended between 1 and 3

> This avoids the problems for the race but without holding locks while
> we log XLOG_RUNNING_XACTS, something that was considered painful for
> Hot Standby.

I don't think that really solves it, because other transactions could
just be stuck just after the XLogInsert() forever.  And it'd have the
issue of having to backpatch a new record.  I'm working on an
alternative approach, let's hope that that works out.

- 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] Adding support for Default partition in partitioning

[Rahila Syed]

Please find attached updated patch with review comments by Robert and
Jeevan implemented.

The newly proposed syntax
CREATE TABLE .. PARTITION OF .. DEFAULT has got most votes on this thread.

If there is no more objection, I will go ahead and include that in the
patch.

Thank you,
Rahila Syed

On Mon, Apr 24, 2017 at 2:40 PM, Rahila Syed  wrote:

> Hello,
>
> Thank you for reviewing.
>
> >But that's not a good idea for several reasons.  For one thing, you
> >can also write FOR VALUES IN (DEFAULT, 5) or which isn't sensible.
> >For another thing, this kind of syntax won't generalize to range
> >partitioning, which we've talked about making this feature support.
> >Maybe something like:
>
> >CREATE TABLE .. PARTITION OF .. DEFAULT;
>
> I agree that the syntax should be changed to also support range
> partitioning.
>
> Following can also be considered as it specifies more clearly that the
> partition holds default values.
>
> CREATE TABLE ...PARTITION OF...FOR VALUES DEFAULT;
>
> >Maybe we should introduce a dedicated node type to
> >represent a default-specification in the parser grammar.  If not, then
> >let's at least encapsulate the test a little better, e.g. by adding
> >isDefaultPartitionBound() which tests not only IsA(..., DefElem) but
> >also whether the name is DEFAULT as expected.  BTW, we typically use
> >lower-case internally, so if we stick with this representation it
> >should really be "default" not "DEFAULT".
>
> isDefaultPartitionBound() function is created in the attached patch which
> checks for both node type and name.
>
> >Why abbreviate "default" to def here?  Seems pointless.
> Corrected in the attached.
>
> >Consider &&
> Fixed.
>
> >+ * default partiton for rows satisfying the new partition
> >Spelling.
> Fixed.
>
> >Missing apostrophe
> Fixed.
>
> >Definitely not safe against concurrency, since AccessShareLock won't
> >exclude somebody else's update.  In fact, it won't even cover somebody
> >else's already-in-flight transaction
> Changed it to AccessExclusiveLock
>
> >Normally in such cases we try to give more detail using
> >ExecBuildSlotValueDescription.
> This function is used in execMain.c and the error is being
> reported in partition.c.
> Do you mean the error reporting should be moved into execMain.c
> to use ExecBuildSlotValueDescription?
>
> >This variable starts out true and is never set to any value other than
> >true.  Just get rid of it and, in the one place where it is currently
> >used, write "true".  That's shorter and clearer.
> Fixed.
>
> >There's not really a reason to cast the result of stringToNode() to
> >Node * and then turn around and cast it to PartitionBoundSpec *.  Just
> >cast it directly to whatever it needs to be.  And use the new castNode
> >macro
> Fixed. castNode macro takes as input Node * whereas stringToNode() takes
> string.
> IIUC, castNode cant be used here.
>
> >The if (def_elem) test continues
> >early, but if the point is that the loop using cell3 shouldn't execute
> >in that case, why not just put if (!def_elem) { foreach(cell3, ...) {
> >... } } instead of reiterating the ReleaseSysCache in two places?
> Fixed in the attached.
>
> I will respond to further comments in following email.
>
>
> On Thu, Apr 13, 2017 at 12:48 AM, Robert Haas 
> wrote:
>
>> On Thu, Apr 6, 2017 at 7:30 AM, Rahila Syed 
>> wrote:
>> > Thanks a lot for testing and reporting this. Please find attached an
>> updated
>> > patch with the fix. The patch also contains a fix
>> > regarding operator used at the time of creating expression as default
>> > partition constraint. This was notified offlist by Amit Langote.
>>
>> I think that the syntax for this patch should probably be revised.
>> Right now the proposal is for:
>>
>> CREATE TABLE .. PARTITION OF ... FOR VALUES IN (DEFAULT);
>>
>> But that's not a good idea for several reasons.  For one thing, you
>> can also write FOR VALUES IN (DEFAULT, 5) or which isn't sensible.
>> For another thing, this kind of syntax won't generalize to range
>> partitioning, which we've talked about making this feature support.
>> Maybe something like:
>>
>> CREATE TABLE .. PARTITION OF .. DEFAULT;
>>
>> This patch makes the assumption throughout that any DefElem represents
>> the word DEFAULT, which is true in the patch as written but doesn't
>> seem very future-proof.  I think the "def" in "DefElem" stands for
>> "definition" or "define" or something like that, so this is actually
>> pretty confusing.  Maybe we should introduce a dedicated node type to
>> represent a default-specification in the parser grammar.  If not, then
>> let's at least encapsulate the test a little better, e.g. by adding
>> isDefaultPartitionBound() which tests not only IsA(..., DefElem) but
>> also whether the name is DEFAULT as expected.  BTW, we typically use
>> lower-case internally, so if we stick with this representation it
>> should really be "default" not "DEFAULT".
>>
>> Useless 

Re: [HACKERS] CTE inlining

[Thomas Kellerer]

> Relevant posts where users get confused by our behaviour:
> 

And Markus Winand's blog: http://modern-sql.com/feature/with/performance

Databases generally obey this principle, although PostgreSQL represents
a big exception 

and 

Besides PostgreSQL, all tested databases optimize with clauses in the
same way 
that they optimize views or derived tables"






--
View this message in context: 
http://www.postgresql-archive.org/CTE-inlining-tp5958992p5959313.html
Sent from the PostgreSQL - hackers mailing list archive at Nabble.com.


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

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

[Robert Haas]

On Tue, May 2, 2017 at 11:49 AM, Alvaro Herrera
 wrote:
> Petr Jelinek wrote:
>> So the only way to fulfill the requirement you stated is to just not try
>> to drop the slot, ever, on DROP SUBSCRIPTION. That makes the default
>> behavior leave resources on upstream that will eventually cause that
>> server to stop unless user notices before. I think we better invent
>> something that limits how much inactive slots can hold back WAL and
>> catalog_xmin in this release as well then.
>
> I don't understand why isn't the default behavior to unconditionally
> drop the slot.  Why do we ever want the slot to be kept?

What if the remote server doesn't exist any more?

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


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

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

[Alvaro Herrera]

Petr Jelinek wrote:

> So the only way to fulfill the requirement you stated is to just not try
> to drop the slot, ever, on DROP SUBSCRIPTION. That makes the default
> behavior leave resources on upstream that will eventually cause that
> server to stop unless user notices before. I think we better invent
> something that limits how much inactive slots can hold back WAL and
> catalog_xmin in this release as well then.

I don't understand why isn't the default behavior to unconditionally
drop the slot.  Why do we ever want the slot to be kept?

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


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

Re: [HACKERS] CTE inlining

[Tomas Vondra]

On 5/2/17 4:44 PM, Andrew Dunstan wrote:



On 05/02/2017 10:13 AM, Merlin Moncure wrote:

On Sun, Apr 30, 2017 at 6:21 PM, Andres Freund  wrote:

On 2017-04-30 07:19:21 +0200, Pavel Stehule wrote:

why we cannot to introduce GUC option - enable_cteoptfence ?

Doesn't really solve the issue, and we've generally shied away from GUCs
that influence behaviour after a few bad experiences.  What if you want
one CTE inlined, but another one not?

Yeah.  Are we absolutely opposed to SQL syntax against WITH that
allows or disallows fencing?   for example,

WITH [MATERIALIZED]

Pushing people to OFFSET 0 is a giant step backwards IMO, and as in
implementation detail is also subject to change.




Agreed, it's an ugly as sin and completely non-obvious hack.



Isn't OFFSET 0 an implementation detail anyway? Who says the planner 
couldn't get smarter in the future, realize OFFSET 0 is no-op?


In that case replacing CTE optimization fence with "OFFSET 0" would be 
akin to painting yourself into a corner, waiting for the pain to dry, 
walking over to another corner and painting yourself into that one.


cheers

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


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

Re: [HACKERS] Re: logical replication and PANIC during shutdown checkpoint in publisher

[Peter Eisentraut]

On 5/1/17 13:43, Peter Eisentraut wrote:
> On 4/30/17 20:52, Noah Misch wrote:
>> IMMEDIATE ATTENTION REQUIRED.  This PostgreSQL 10 open item is long past due
>> for your status update.  Please thoroughly reacquaint yourself with the 
>> policy
>> on open item ownership[1] and then reply immediately.  If I do not hear from
>> you by 2017-05-02 01:00 UTC, I will transfer this item to release management
>> team ownership without further notice.
> 
> I'm reviewing this now and will report tomorrow.

We have consensus around a patch, but we are still discussing and
discovering some new details.

There is also the question of whether to backpatch and how far.  (Could
be all the way to 9.2.)

I propose, if there are no new insights by Friday, I will commit the
current patch to master, which will fix the reported problem for PG10,
and punt the remaining side issues to "Older Bugs".

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


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

Re: [HACKERS] logical replication and PANIC during shutdown checkpoint in publisher

[Peter Eisentraut]

On 5/2/17 10:08, Michael Paquier wrote:
> On Tue, May 2, 2017 at 9:30 PM, Peter Eisentraut
>  wrote:
>> On 5/2/17 03:11, Petr Jelinek wrote:
>>> logical decoding can theoretically
>>> do HOT pruning (even if the chance is really small) so it's not safe to
>>> start logical replication either.
>>
>> This seems a bit impossible to resolve.  On the one hand, we want to
>> allow streaming until after the shutdown checkpoint.  On the other hand,
>> streaming itself might produce new WAL.
> 
> It would be nice to split things into two:
> - patch 1 adding the signal handling that wins a backpatch.
> - patch 2 fixing the side cases with logical decoding.

The side cases with logical decoding are also not new and would need
backpatching, AIUI.

>> Can we prevent HOT pruning during logical decoding?
> 
> It does not sound much difficult to do, couldn't you just make it a
> no-op with am_walsender?

That's my hope.

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


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

Re: [HACKERS] Logical replication in the same cluster

[Petr Jelinek]

On 02/05/17 16:37, Andres Freund wrote:
> On 2017-05-02 09:17:27 +0200, Petr Jelinek wrote:
>> Yes because otherwise we risk leaving slot on the upstream if the
>> command fails downstream.
> 
> Shouldn't temporary slots be able to solve that concern?  Create it as
> temporary at the beginning, mark it as permanent at the end?
> 

So we need ALTER_REPLICATION_SLOT? :)

But that aside, based on the conversation nearby [1], we'll see if we
even want to create slots in CREATE SUBSCRIPTION.

[1]
https://www.postgresql.org/message-id/flat/CA%2BTgmoZmkbpAWRzVKDVcHnTBkYjJEFS8%3D09RL-G3zgdozCLFHQ%40mail.gmail.com#CA+TgmoZmkbpAWRzVKDVcHnTBkYjJEFS8=09rl-g3zgdozcl...@mail.gmail.com

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


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

Re: [HACKERS] CTE inlining

[Andrew Dunstan]

On 05/02/2017 10:13 AM, Merlin Moncure wrote:
> On Sun, Apr 30, 2017 at 6:21 PM, Andres Freund  wrote:
>> On 2017-04-30 07:19:21 +0200, Pavel Stehule wrote:
>>> why we cannot to introduce GUC option - enable_cteoptfence ?
>> Doesn't really solve the issue, and we've generally shied away from GUCs
>> that influence behaviour after a few bad experiences.  What if you want
>> one CTE inlined, but another one not?
> Yeah.  Are we absolutely opposed to SQL syntax against WITH that
> allows or disallows fencing?   for example,
>
> WITH [MATERIALIZED]
>
> Pushing people to OFFSET 0 is a giant step backwards IMO, and as in
> implementation detail is also subject to change.
>
>



Agreed, it's an ugly as sin and completely non-obvious hack.

cheers

andrew

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



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

Re: [HACKERS] Logical replication in the same cluster

[Andres Freund]

On 2017-05-02 09:17:27 +0200, Petr Jelinek wrote:
> Yes because otherwise we risk leaving slot on the upstream if the
> command fails downstream.

Shouldn't temporary slots be able to solve that concern?  Create it as
temporary at the beginning, mark it as permanent at the end?

Greetings,

Andres Freund


-- 
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 syntax (was DROP SUBSCRIPTION, query cancellations and slot handling)

[Petr Jelinek]

On 02/05/17 15:31, Tom Lane wrote:
> Petr Jelinek  writes:
>> Let me expand, if we don't drop the slot by default when dropping
>> subscription, we'll have a lot of users with dead slots laying around
>> holding back WAL/catalog_xmin, that's really bad. If we do drop by
>> default like now, we need option to not do that, neither RESTRICT, nor
>> CASCADE fit that.
> 
> I'm not sure which part of "you can't have an option in DROP" isn't
> clear to you.  Whatever the default behavior is always has to work,
> because that is what's going to happen during DROP OWNED BY, or
> DROP DATABASE. 

You are saying it like there was some guarantee that those commands
can't fail because of other objects. AFAIK for example drop database can
trivially fail just because there is replication slot in it before PG10
(IIRC Craig managed to fix that in 10 for unrelated reasons).


> If you want more than one behavior during DROP,
> you need to drive that off something represented as a persistent
> property of the object, not off an option in the DROP command.
> 

I don't see how changing behavior as object property will change
anything in terms of not failing to cascade. If you use CREATE or ALTER
to say that subscription must drop slot and that fails then the cascade
will still fail so then you need to run ALTER again to change that
property. I fail to see how that's easier than running the DROP directly.

So the only way to fulfill the requirement you stated is to just not try
to drop the slot, ever, on DROP SUBSCRIPTION. That makes the default
behavior leave resources on upstream that will eventually cause that
server to stop unless user notices before. I think we better invent
something that limits how much inactive slots can hold back WAL and
catalog_xmin in this release as well then.

We should also not create the slot (at least by default) on CREATE
SUBSCRIPTION to have some symmetry.

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


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

Re: [HACKERS] CTE inlining

[Merlin Moncure]

On Sun, Apr 30, 2017 at 6:21 PM, Andres Freund  wrote:
> On 2017-04-30 07:19:21 +0200, Pavel Stehule wrote:
>> why we cannot to introduce GUC option - enable_cteoptfence ?
>
> Doesn't really solve the issue, and we've generally shied away from GUCs
> that influence behaviour after a few bad experiences.  What if you want
> one CTE inlined, but another one not?

Yeah.  Are we absolutely opposed to SQL syntax against WITH that
allows or disallows fencing?   for example,

WITH [MATERIALIZED]

Pushing people to OFFSET 0 is a giant step backwards IMO, and as in
implementation detail is also subject to change.

merlin


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

Re: [HACKERS] logical replication and PANIC during shutdown checkpoint in publisher

[Michael Paquier]

On Tue, May 2, 2017 at 9:30 PM, Peter Eisentraut
 wrote:
> On 5/2/17 03:11, Petr Jelinek wrote:
>> logical decoding can theoretically
>> do HOT pruning (even if the chance is really small) so it's not safe to
>> start logical replication either.
>
> This seems a bit impossible to resolve.  On the one hand, we want to
> allow streaming until after the shutdown checkpoint.  On the other hand,
> streaming itself might produce new WAL.

It would be nice to split things into two:
- patch 1 adding the signal handling that wins a backpatch.
- patch 2 fixing the side cases with logical decoding.

> Can we prevent HOT pruning during logical decoding?

It does not sound much difficult to do, couldn't you just make it a
no-op with am_walsender?
-- 
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] logical replication syntax (was DROP SUBSCRIPTION, query cancellations and slot handling)

[Joshua D. Drake]

On 05/02/2017 05:13 AM, Tom Lane wrote:

Robert Haas  writes:

On Thu, Apr 20, 2017 at 7:46 AM, Petr Jelinek
 wrote:

DROP SUBSCRIPTION mysub NODROP SLOT;



Having said that, I doubt that anyone would argue that CREATE USER is
anything but legacy syntax, or that our more recent syntax designs aren't
better models to follow.

It's not quite too late to revisit the syntax of the log rep commands
... shall we add this as an open item?


I would think so. Just in reading this, even if we keep a similar syntax 
it should be DROP SLOT NO or DROP SLOT FALSE.


JD




regards, tom lane





--
Command Prompt, Inc.  http://the.postgres.company/
+1-503-667-4564
PostgreSQL Centered full stack support, consulting and development.
Everyone appreciates your honesty, until you are honest with them.


--
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 syntax (was DROP SUBSCRIPTION, query cancellations and slot handling)

[Tom Lane]

Petr Jelinek  writes:
> Let me expand, if we don't drop the slot by default when dropping
> subscription, we'll have a lot of users with dead slots laying around
> holding back WAL/catalog_xmin, that's really bad. If we do drop by
> default like now, we need option to not do that, neither RESTRICT, nor
> CASCADE fit that.

I'm not sure which part of "you can't have an option in DROP" isn't
clear to you.  Whatever the default behavior is always has to work,
because that is what's going to happen during DROP OWNED BY, or
DROP DATABASE.  If you want more than one behavior during DROP,
you need to drive that off something represented as a persistent
property of the object, not off an option in the DROP command.

I agree that RESTRICT/CASCADE don't fit this, unless the model
is that the slot is always owned by the subscription, which might
be too restrictive.

regards, tom lane


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

Re: [HACKERS] check with serial

[Andrew Dunstan]

On 05/01/2017 09:39 AM, Andrew Dunstan wrote:
> The other day I wanted to run "make check" but with the serial schedule.
> This wasn't as easy as it should have been. Although we now have
> installcheck-parallel we don't have check-serial. Should we have that?
> Alternatively, should we allow a SCHEDULE=foo argument for the "check"
> target which defaults to parallel?
>
>


Here's a simple patch that does what I had in mind. It allows providing
for an arbitrary schedule file in both the check and installcheck
recipes. The historic behaviour is preserved.

cheers

andrew


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

diff --git a/src/test/regress/GNUmakefile b/src/test/regress/GNUmakefile
index b923ea1..7e100cb 100644
--- a/src/test/regress/GNUmakefile
+++ b/src/test/regress/GNUmakefile
@@ -127,13 +127,13 @@ tablespace-setup:
 REGRESS_OPTS = --dlpath=. $(EXTRA_REGRESS_OPTS)
 
 check: all tablespace-setup
-	$(pg_regress_check) $(REGRESS_OPTS) --schedule=$(srcdir)/parallel_schedule $(MAXCONNOPT) $(EXTRA_TESTS)
+	$(pg_regress_check) $(REGRESS_OPTS) --schedule=$(srcdir)/$(if $(SCHEDULE),$(SCHEDULE),parallel)_schedule $(MAXCONNOPT) $(EXTRA_TESTS)
 
 check-tests: all tablespace-setup
 	$(pg_regress_check) $(REGRESS_OPTS) $(MAXCONNOPT) $(TESTS) $(EXTRA_TESTS)
 
 installcheck: all tablespace-setup
-	$(pg_regress_installcheck) $(REGRESS_OPTS) --schedule=$(srcdir)/serial_schedule $(EXTRA_TESTS)
+	$(pg_regress_installcheck) $(REGRESS_OPTS) --schedule=$(srcdir)/$(if $(SCHEDULE),$(SCHEDULE),serial)_schedule $(EXTRA_TESTS)
 
 installcheck-parallel: all tablespace-setup
 	$(pg_regress_installcheck) $(REGRESS_OPTS) --schedule=$(srcdir)/parallel_schedule $(MAXCONNOPT) $(EXTRA_TESTS)

-- 
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 syntax (was DROP SUBSCRIPTION, query cancellations and slot handling)

[Petr Jelinek]

On 02/05/17 15:14, Petr Jelinek wrote:
> On 02/05/17 15:10, Tom Lane wrote:
>> Robert Haas  writes:
 On Thu, Apr 20, 2017 at 7:46 AM, Petr Jelinek
  wrote:
> DROP SUBSCRIPTION mysub NODROP SLOT;
>>
 I'm pretty uninspired by this choice of syntax.
>>
>> Actually, this command has got much worse problems than whether you like
>> the spelling of its option: it shouldn't have an option in the first
>> place.  I put it to you as an inviolable rule that no object DROP command
>> should ever have any options other than RESTRICT/CASCADE and IF EXISTS.
>> If it does, then you don't know what to do when the object is recursed
>> to by a cascaded drop.
>>
>> It's possible that we could allow exceptions to this rule for object types
>> that can never have any dependencies.  But a subscription doesn't qualify
>> --- it's got an owner, so DROP OWNED BY already poses this problem.  Looks
>> to me like it's got a dependency on a database, too.  (BTW, if
>> subscriptions are per-database, why is pg_subscription a shared catalog?
>> There were some pretty schizophrenic decisions here.)
> 
> Because otherwise we would need launcher process per database, not pretty.
> 
>>
>> So ISTM we need to get rid of the above-depicted syntax.  We could instead
>> have what-to-do-with-the-slot as a property of the subscription,
>> established at CREATE SUBSCRIPTION and possibly changed later by ALTER
>> SUBSCRIPTION.  Not quite sure what to call the option, maybe something
>> based on the concept of the subscription "owning" the slot.
>>
> 
> So what do you do if the upstream does not exist anymore when you are
> dropping subscription?
> 

Let me expand, if we don't drop the slot by default when dropping
subscription, we'll have a lot of users with dead slots laying around
holding back WAL/catalog_xmin, that's really bad. If we do drop by
default like now, we need option to not do that, neither RESTRICT, nor
CASCADE fit that.

-- 
  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 syntax (was DROP SUBSCRIPTION, query cancellations and slot handling)

[Petr Jelinek]

On 02/05/17 15:10, Tom Lane wrote:
> Robert Haas  writes:
>>> On Thu, Apr 20, 2017 at 7:46 AM, Petr Jelinek
>>>  wrote:
 DROP SUBSCRIPTION mysub NODROP SLOT;
> 
>>> I'm pretty uninspired by this choice of syntax.
> 
> Actually, this command has got much worse problems than whether you like
> the spelling of its option: it shouldn't have an option in the first
> place.  I put it to you as an inviolable rule that no object DROP command
> should ever have any options other than RESTRICT/CASCADE and IF EXISTS.
> If it does, then you don't know what to do when the object is recursed
> to by a cascaded drop.
> 
> It's possible that we could allow exceptions to this rule for object types
> that can never have any dependencies.  But a subscription doesn't qualify
> --- it's got an owner, so DROP OWNED BY already poses this problem.  Looks
> to me like it's got a dependency on a database, too.  (BTW, if
> subscriptions are per-database, why is pg_subscription a shared catalog?
> There were some pretty schizophrenic decisions here.)

Because otherwise we would need launcher process per database, not pretty.

> 
> So ISTM we need to get rid of the above-depicted syntax.  We could instead
> have what-to-do-with-the-slot as a property of the subscription,
> established at CREATE SUBSCRIPTION and possibly changed later by ALTER
> SUBSCRIPTION.  Not quite sure what to call the option, maybe something
> based on the concept of the subscription "owning" the slot.
> 

So what do you do if the upstream does not exist anymore when you are
dropping subscription?

-- 
  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 syntax (was DROP SUBSCRIPTION, query cancellations and slot handling)

[Tom Lane]

Robert Haas  writes:
>> On Thu, Apr 20, 2017 at 7:46 AM, Petr Jelinek
>>  wrote:
>>> DROP SUBSCRIPTION mysub NODROP SLOT;

>> I'm pretty uninspired by this choice of syntax.

Actually, this command has got much worse problems than whether you like
the spelling of its option: it shouldn't have an option in the first
place.  I put it to you as an inviolable rule that no object DROP command
should ever have any options other than RESTRICT/CASCADE and IF EXISTS.
If it does, then you don't know what to do when the object is recursed
to by a cascaded drop.

It's possible that we could allow exceptions to this rule for object types
that can never have any dependencies.  But a subscription doesn't qualify
--- it's got an owner, so DROP OWNED BY already poses this problem.  Looks
to me like it's got a dependency on a database, too.  (BTW, if
subscriptions are per-database, why is pg_subscription a shared catalog?
There were some pretty schizophrenic decisions here.)

So ISTM we need to get rid of the above-depicted syntax.  We could instead
have what-to-do-with-the-slot as a property of the subscription,
established at CREATE SUBSCRIPTION and possibly changed later by ALTER
SUBSCRIPTION.  Not quite sure what to call the option, maybe something
based on the concept of the subscription "owning" the slot.

I think this is a must-fix issue, and will put it on the open items
list.

regards, tom lane


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

Re: [HACKERS] CTE inlining

[Oleg Bartunov]

On Mon, May 1, 2017 at 7:22 AM, Pavel Stehule  wrote:
>
>
> 2017-05-01 1:21 GMT+02:00 Andres Freund :
>>
>> On 2017-04-30 07:19:21 +0200, Pavel Stehule wrote:
>> > why we cannot to introduce GUC option - enable_cteoptfence ?
>>
>> Doesn't really solve the issue, and we've generally shied away from GUCs
>> that influence behaviour after a few bad experiences.  What if you want
>> one CTE inlined, but another one not?
>
>
> It change behave in same sense like enable_nestloop, enable_hashjoin, ...
> with same limits.

And then we recall  plan hints :)

>
> Regards
>
> Pavel
>
>>
>>
>> - 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] UPDATE of partition key

[Robert Haas]

On Tue, Apr 4, 2017 at 7:11 AM, Amit Khandekar  wrote:
> Attached updated patch v7 has the above changes.

This no longer applies.  Please rebase.

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


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

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

[Petr Jelinek]

On 02/05/17 14:13, Tom Lane wrote:
> Robert Haas  writes:
>> On Thu, Apr 20, 2017 at 7:46 AM, Petr Jelinek
>>  wrote:
>>> DROP SUBSCRIPTION mysub NODROP SLOT;
> 
>> I'm pretty uninspired by this choice of syntax.  Logical replication
>> seems to have added a whole bunch of syntax that involves prefixing
>> words with "no".  In various places, there's NODROP, NOREFRESH, NOCOPY
>> DATA, NOCONNECT, and NOPUBLISH.  But "NO" is not an English prefix,
>> and there's no precedent of which I'm aware for such SQL syntax.  In
>> most places, we've chosen to name the option and then the user set it
>> to "on" or "off".  So for example you type EXPLAIN (ANALYZE, TIMING
>> OFF) or EXPLAIN (ANALYZE, TIMING FALSE), not EXPLAIN (ANALYZE,
>> NOTIMING).  I think most of the logical replication stuff could have
>> been done this way pretty easily, but for some reason it picked a
>> completely different approach.
> 
> I tend to agree with this criticism, but it's not quite true that we
> don't do this anywhere else --- see CREATE USER for one example, where
> we have monstrosities like NOBYPASSRLS.  Still, at least those are single
> words without arguments.  "NODROP SLOT" is pretty ugly, not least
> because if you want to claim CREATE USER as syntax precedent, you ought
> to spell it NODROPSLOT.
> 
> Having said that, I doubt that anyone would argue that CREATE USER is
> anything but legacy syntax, or that our more recent syntax designs aren't
> better models to follow.
> 
> It's not quite too late to revisit the syntax of the log rep commands
> ... shall we add this as an open item?

I am happy to implement something different, it's quite trivial to
change. But I am not going to propose anything different as I can't
think of better syntax (if I could I would have done it). I don't like
the OFF or FALSE (ie DROP SLOT OFF) any more than what is there now and
it also seems to not map very well to action (as opposed to output
option as it is in EXPLAIN). It might not be very close to SQL way but
that's because SQL way would be do not do any of those default actions
unless they are actually asked for (ie NODROP SLOT would be default and
DROP SLOT would be the option) but that's IMHO less user friendly.

-- 
  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 and PANIC during shutdown checkpoint in publisher

[Michael Paquier]

On Tue, May 2, 2017 at 9:27 PM, Peter Eisentraut
 wrote:
> On 5/2/17 03:43, Michael Paquier wrote:
>>> I don't think the code covers all because a) the SQL queries are not
>>> covered at all that I can see and b) logical decoding can theoretically
>>> do HOT pruning (even if the chance is really small) so it's not safe to
>>> start logical replication either.
>>
>> Ahhh. So START_REPLICATION can also now generate WAL. Good to know.
>
> And just looking at pg_current_wal_location(), running BASE_BACKUP also
> advances the WAL.

Indeed. I forgot the backup end record and the segment switch. We are
good for a backpatch down to 9.2 here.
-- 
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] CTE inlining

[Andreas Karlsson]

On 05/02/2017 04:38 AM, Craig Ringer wrote:

On 1 May 2017 at 22:26, Andreas Karlsson  wrote:

I am not sure I like decorators since this means adding an ad hoc query hint
directly into the SQL syntax which is something which I requires serious
consideration.


And mangling the semantics of existing syntax doesn't?

That's what we do right now so we can pretend we don't have query
hints while still having query hints.


I am in favor of removing the optimization fence from CTEs, and strongly 
prefer no fence being the default behavior since SQL is a declarative 
language and I think it is reasonable to assume that CTEs can be 
inlined. But the question is how to best remove the fence while taking 
into account that quite many use them as optimization fences today.


I see some alternatives, none of them perfect.

1. Just remove the optimization fence and let people add OFFSET 0 to 
their queries if they want an optimization fence. This lets us keep 
pretending that we do not have query hints (and therefore do not have to 
formalize any syntax for them) while still allowing people to add 
optimization fences.


2. Add a decorator for WITH (e.g. "WITH MATERIALIZED x (...") to add an 
explicit optimization fence. This will for the first time add official 
support for a query hint in the syntax which is a quite big precedent.


3. Add a new GUC which can enable and disable the optimization fence. 
This is a very clumsy tool, but maybe good enough for some users and 
some people here in this thread have complained about our similar GUCs.


4. Add some new more generic query hinting facility. This is a lot of 
work and something which would be very hard to get consensus for.


Andreas


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

Re: [HACKERS] Shared Memory hash tables only at startup

[Tom Lane]

hariprasath nallasamy  writes:
> *(In practice, all creations are done in the postmasterprocess; child
> processes should always be attaching to existing tables.)*

Yeah ...

> Is there any specific reason to do so or my understanding was wrong(we can
> create shared memory hash table at run time too?)

Well, if you don't mind resolving the race conditions that you're going to
have, you could possibly do that.  In practice though, unless it's a very
small hash table, it's going to need to be accounted for in the sizing
calculations in CreateSharedMemoryAndSemaphores, which means you might
as well create it during that function too.

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 and PANIC during shutdown checkpoint in publisher

[Peter Eisentraut]

On 5/2/17 03:11, Petr Jelinek wrote:
> logical decoding can theoretically
> do HOT pruning (even if the chance is really small) so it's not safe to
> start logical replication either.

This seems a bit impossible to resolve.  On the one hand, we want to
allow streaming until after the shutdown checkpoint.  On the other hand,
streaming itself might produce new WAL.

Can we prevent HOT pruning during logical decoding?

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


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

Re: [HACKERS] Regarding B-Tree Lookup

[Tom Lane]

Michael Paquier  writes:
> On Tue, May 2, 2017 at 6:12 PM, Mahi Gurram  wrote:
>> Please suggest me the easiest way to lookup into PK's B-Tree index for
>> getting TIDs.

> Why don't you just use SPI within your extension? No need to copy the
> logic for btree lookups this way.

There's not actually that much code needed, though -- basically
index_beginscan, index_rescan, index_getnext, index_endscan.  One possible
model to follow is systable_beginscan and friends, in genam.c.

I think that you could possibly get away with just applying
systable_beginscan to random user tables, even.  But it's at least a
conceptual mismatch, and there are some things in there, like the
test on IgnoreSystemIndexes, that you probably don't want.

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 and PANIC during shutdown checkpoint in publisher

[Peter Eisentraut]

On 5/2/17 03:43, Michael Paquier wrote:
>> I don't think the code covers all because a) the SQL queries are not
>> covered at all that I can see and b) logical decoding can theoretically
>> do HOT pruning (even if the chance is really small) so it's not safe to
>> start logical replication either.
> 
> Ahhh. So START_REPLICATION can also now generate WAL. Good to know.

And just looking at pg_current_wal_location(), running BASE_BACKUP also
advances the WAL.

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


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

Re: [HACKERS] Regarding B-Tree Lookup

[Craig Ringer]

On 2 May 2017 7:34 pm, "Michael Paquier"  wrote:

On Tue, May 2, 2017 at 6:12 PM, Mahi Gurram  wrote:
> I'm building some custom extension on top of postgres 9.6.1. As part of
> that, I would like to read Heap Tuple directly from my extension using
> Primary Key.
>
> By default, postgres table index(B-Tree) its PrimaryKey(PK). So, i would
> like to get TID by doing a lookup into PK's B-Tree index. Using which i
> could read HeapTuple directly.


Use the heapam and indexam.

There's a handy wrapper for simpler queries in genam. See
systable_beginscsn etc. AFAIK these aren't really restricted to system
tables.

>
> Please suggest me the easiest way to lookup into PK's B-Tree index for
> getting TIDs.

Why don't you just use SPI within your extension? No need to copy the
logic for btree lookups this way.
https://www.postgresql.org/docs/9.6/static/spi.html


SPI is certainly the simplest way.


> Suggesting a postgres extensions which does B-Tree lookup will also helps
> me.


Pglogical has lots of direct heap and index access via genam.

Re: [HACKERS] [Proposal]: Extends VisualStudio to automatically precompile EmbeddedSQL

[zosrothko]

Le 02/05/2017 à 13:22, Michael Paquier a écrit :
> The PostgreSQL uses a set of scripts in src/tools/msvc/ to generate
> things compiled with visual studio, so instinctively you would be
> looking at working on that. Honestly, just by looking at the files you
> are proposing, it is hard to make an idea of why this would be useful
> for ECPG, and please note as well that there are guidelines for
> submitting patches:
> https://wiki.postgresql.org/wiki/Submitting_a_Patch
Please find below a log of building a VS2017 project named PGembeddedSQL
using the joined ECPG extension.

Building the PGEmbeddedSQL.vcxproj with msbuild calls automatically the
ecpg precompiler to precompile the PGEmbeddedSQL.pgc source and produces
PGembeddedSQL.cxx, which in turn is compiled and linked to produce de
PGembeddedSQL.exe

C:\MXW\Clients\PagesJaunes\SQLDynamic\SQLDynamic>dir
 Le volume dans le lecteur C n'a pas de nom.
 Le numéro de série du volume est 6FAB-AFA0

 Répertoire de C:\MXW\Clients\PagesJaunes\SQLDynamic\SQLDynamic

02/05/2017  14:08  .
02/05/2017  14:08  ..
27/04/2017  17:11  Debug
27/04/2017  15:28 2 648 PGEmbeddedSQL.pgc
27/04/2017  15:13 9 557 PGEmbeddedSQL.vcxproj
27/04/2017  15:10 1 302 PGEmbeddedSQL.vcxproj.filters
27/04/2017  15:13   682 PGEmbeddedSQL.vcxproj.user

C:\MXW\Clients\PagesJaunes\SQLDynamic\SQLDynamic>msbuild
PGEmbeddedSQL.vcxproj
Microsoft (R) Build Engine, version 15.1.548.43366
Copyright (C) Microsoft Corporation. Tous droits réservés.

La génération a démarré 02/05/2017 14:09:56.
Projet
"C:\MXW\Clients\PagesJaunes\SQLDynamic\SQLDynamic\PGEmbeddedSQL.vcxproj"
sur le noud 1 (cibl
es par défaut).
InitializeBuildStatus:
  Création de
"C:\temp\postgres\Debug\PGEmbeddedSQL.tlog\unsuccessfulbuild", car
"AlwaysCreate" a é
  té spécifié.
EcpgTarget:
  Process "PGEmbeddedSQL.pgc" ecpg file
  cmd.exe /C
"C:\Users\FrancisANDRE\AppData\Local\Temp\tmpfdb390c096f443e5b56d9ee962f8dafd.cmd"

  ecpg.exe -o PGEmbeddedSQL.cxx  PGEmbeddedSQL.pgc

ClCompile:
  C:\Program Files (x86)\Microsoft Visual Studio 14.0\VC\bin\CL.exe /c
/ZI /nologo /W3 /WX- /Od /Oy
  - /D WIN32 /D _DEBUG /D _CONSOLE /D _UNICODE /D UNICODE /Gm /EHsc
/RTC1 /MDd /GS /fp:precise /Zc:
  wchar_t /Zc:forScope /Zc:inline /GR /Fo"C:\temp\postgres\Debug\\"
/Fd"C:\temp\postgres\Debug\vc14
  0.pdb" /Gd /TP /analyze- /errorReport:queue PGEmbeddedSQL.cxx
  PGEmbeddedSQL.cxx
Link:
  C:\Program Files (x86)\Microsoft Visual Studio 14.0\VC\bin\link.exe
/ERRORREPORT:QUEUE /OUT:"C:\t
  emp\postgres\PGEmbeddedSQLd.exe" /INCREMENTAL /NOLOGO libecpg.lib
libpq.lib kernel32.lib user32.l
  ib gdi32.lib winspool.lib comdlg32.lib advapi32.lib shell32.lib
ole32.lib oleaut32.lib uuid.lib o
  dbc32.lib odbccp32.lib kernel32.lib user32.lib gdi32.lib winspool.lib
comdlg32.lib advapi32.lib s
  hell32.lib ole32.lib oleaut32.lib uuid.lib odbc32.lib odbccp32.lib
/MANIFEST /MANIFESTUAC:"level=
  'asInvoker' uiAccess='false'" /manifest:embed /DEBUG
/PDB:"C:\temp\postgres\PGEmbeddedSQLd.pdb" /
  SUBSYSTEM:CONSOLE /TLBID:1 /DYNAMICBASE /NXCOMPAT
/IMPLIB:"C:\temp\postgres\PGEmbeddedSQLd.lib" /
  MACHINE:X86 C:\temp\postgres\Debug\PGEmbeddedSQL.obj
  PGEmbeddedSQL.vcxproj -> C:\temp\postgres\PGEmbeddedSQLd.exe
  PGEmbeddedSQL.vcxproj -> C:\temp\postgres\PGEmbeddedSQLd.pdb (Full PDB)
FinalizeBuildStatus:
  Suppression du fichier
"C:\temp\postgres\Debug\PGEmbeddedSQL.tlog\unsuccessfulbuild".
  Mise à jour de l'horodatage
"C:\temp\postgres\Debug\PGEmbeddedSQL.tlog\PGEmbeddedSQL.lastbuildsta
  te".
Génération du projet
"C:\MXW\Clients\PagesJaunes\SQLDynamic\SQLDynamic\PGEmbeddedSQL.vcxproj"
termi
née (cibles par défaut).


La génération a réussi.
0 Avertissement(s)
0 Erreur(s)

Temps écoulé 00:00:02.63

Without the extension, you would have to precompile the
PGembeddedSQL.pcg by hand

HTH

Francis
>
> The source code of ecpg is located in src/interfaces/ecpg by the way.
> If you are interested to work on it, that's the place where to look at
> first.



-- 
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 syntax (was DROP SUBSCRIPTION, query cancellations and slot handling)

[Thom Brown]

On 2 May 2017 at 12:55, Robert Haas  wrote:
> On Thu, Apr 20, 2017 at 7:46 AM, Petr Jelinek
>  wrote:
>> DROP SUBSCRIPTION mysub NODROP SLOT;
>
> I'm pretty uninspired by this choice of syntax.  Logical replication
> seems to have added a whole bunch of syntax that involves prefixing
> words with "no".  In various places, there's NODROP, NOREFRESH, NOCOPY
> DATA, NOCONNECT, and NOPUBLISH.  But "NO" is not an English prefix,
> and there's no precedent of which I'm aware for such SQL syntax.  In
> most places, we've chosen to name the option and then the user set it
> to "on" or "off".  So for example you type EXPLAIN (ANALYZE, TIMING
> OFF) or EXPLAIN (ANALYZE, TIMING FALSE), not EXPLAIN (ANALYZE,
> NOTIMING).  I think most of the logical replication stuff could have
> been done this way pretty easily, but for some reason it picked a
> completely different approach.

+1 for not upsetting my OCD.

Thom


-- 
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 syntax (was DROP SUBSCRIPTION, query cancellations and slot handling)

[Tom Lane]

Robert Haas  writes:
> On Thu, Apr 20, 2017 at 7:46 AM, Petr Jelinek
>  wrote:
>> DROP SUBSCRIPTION mysub NODROP SLOT;

> I'm pretty uninspired by this choice of syntax.  Logical replication
> seems to have added a whole bunch of syntax that involves prefixing
> words with "no".  In various places, there's NODROP, NOREFRESH, NOCOPY
> DATA, NOCONNECT, and NOPUBLISH.  But "NO" is not an English prefix,
> and there's no precedent of which I'm aware for such SQL syntax.  In
> most places, we've chosen to name the option and then the user set it
> to "on" or "off".  So for example you type EXPLAIN (ANALYZE, TIMING
> OFF) or EXPLAIN (ANALYZE, TIMING FALSE), not EXPLAIN (ANALYZE,
> NOTIMING).  I think most of the logical replication stuff could have
> been done this way pretty easily, but for some reason it picked a
> completely different approach.

I tend to agree with this criticism, but it's not quite true that we
don't do this anywhere else --- see CREATE USER for one example, where
we have monstrosities like NOBYPASSRLS.  Still, at least those are single
words without arguments.  "NODROP SLOT" is pretty ugly, not least
because if you want to claim CREATE USER as syntax precedent, you ought
to spell it NODROPSLOT.

Having said that, I doubt that anyone would argue that CREATE USER is
anything but legacy syntax, or that our more recent syntax designs aren't
better models to follow.

It's not quite too late to revisit the syntax of the log rep commands
... shall we add this as an open item?

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] vcregress support for single TAP tests

[Andrew Dunstan]

On 05/02/2017 12:19 AM, Vaishnavi Prabakaran wrote:
>
>
>
> On Mon, May 1, 2017 at 11:01 PM, Andrew Dunstan
>  > wrote:
>
>
>
> In the absence of further comments I'm going to apply this and
> back-patch it so we can get a significant improvement in how the
> buildfarm reports results from TAP tests, as well as increased
> coverage,
> on Windows/MSVC machines.
>
>
> I see vcregress is not processing the second argument passed to it.
> For example, "vcregress.bat check serial" runs the regress test in
> parallel mode.
> Though this issue is present even before this patch is applied, am
> writing here because this patch touches the argument passing to
> sub-routine logic. 
> Attached is the one fix I find it working for me.
>
>


Oh, you're right, my bad. Thanks. Will apply.

cheers

andrew

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



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

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

[Robert Haas]

On Thu, Apr 20, 2017 at 7:46 AM, Petr Jelinek
 wrote:
> DROP SUBSCRIPTION mysub NODROP SLOT;

I'm pretty uninspired by this choice of syntax.  Logical replication
seems to have added a whole bunch of syntax that involves prefixing
words with "no".  In various places, there's NODROP, NOREFRESH, NOCOPY
DATA, NOCONNECT, and NOPUBLISH.  But "NO" is not an English prefix,
and there's no precedent of which I'm aware for such SQL syntax.  In
most places, we've chosen to name the option and then the user set it
to "on" or "off".  So for example you type EXPLAIN (ANALYZE, TIMING
OFF) or EXPLAIN (ANALYZE, TIMING FALSE), not EXPLAIN (ANALYZE,
NOTIMING).  I think most of the logical replication stuff could have
been done this way pretty easily, but for some reason it picked a
completely different approach.

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


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

[HACKERS] Shared Memory hash tables only at startup

[hariprasath nallasamy]

Hi
  It is clear that all of the shared memory hash tables are created during
startup using
*ShmemInitHash() *

*(In practice, all creations are done in the postmasterprocess; child
processes should always be attaching to existing tables.)*

Is there any specific reason to do so or my understanding was wrong(we can
create shared memory hash table at run time too?)



TIA
 harry

Re: [HACKERS] Regarding B-Tree Lookup

[Michael Paquier]

On Tue, May 2, 2017 at 6:12 PM, Mahi Gurram  wrote:
> I'm building some custom extension on top of postgres 9.6.1. As part of
> that, I would like to read Heap Tuple directly from my extension using
> Primary Key.
>
> By default, postgres table index(B-Tree) its PrimaryKey(PK). So, i would
> like to get TID by doing a lookup into PK's B-Tree index. Using which i
> could read HeapTuple directly.
>
> Please suggest me the easiest way to lookup into PK's B-Tree index for
> getting TIDs.

Why don't you just use SPI within your extension? No need to copy the
logic for btree lookups this way.
https://www.postgresql.org/docs/9.6/static/spi.html

> Suggesting a postgres extensions which does B-Tree lookup will also helps
> me.

contrib/amcheck looks at raw btree data, though I am not sure that you
actually need to go down to that. But that's hard to reach a
conclusion without more details.
-- 
Michael


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

[HACKERS] [Proposal]: Extends VisualStudio to automatically precompile EmbeddedSQL

[Francis ANDRE]

Hi

I made an extension of VisualStudio that precompiles automaticaly C or
C++ source with PostgreSQL Embedded SQL. The extension is made of the 3
files joined and I have no idea where they should be placed in the
PostgreSQL source tree.

Anybody interested in pushing this extension?

FA



ecpg.props
Description: XML document

http://schemas.microsoft.com/developer/msbuild/2003;>
  


  EcpgTarget

  
  
$(MSBuildThisFileDirectory)$(MSBuildThisFileName).xml
  
  

  


  
@(Ecpg, '|')
  




  
  

$(ComputeLinkInputsTargets);
ComputeEcpgOutput;
  

$(ComputeLibInputsTargets);
ComputeEcpgOutput;
  
  
  

  
  
  
  


  

http://schemas.microsoft.com/winfx/2006/xaml; xmlns:sys="clr-namespace:System;assembly=mscorlib" xmlns:transformCallback="Microsoft.Cpp.Dev10.ConvertPropertyCallback">
  

  


  

  General

  
  

  Ecpg Options

  
  

  Command Line

  


https://www.postgresql.org/docs/9.5/static/app-ecpg.html;
  DisplayName="Output File Name"
  Description="Specifies that ecpg should write all its output to the given filename"
  Switch="o [value]"
  />

	  https://www.postgresql.org/docs/9.5/static/app-ecpg.html;
		Description="Define a C preprocessor symbol."
		Switch="D [value]"
  />

	  https://www.postgresql.org/docs/9.5/static/app-ecpg.html;
		Description="Specify an additional include path, used to find files included via EXEC SQL INCLUDE. Defaults are . (current directory), /usr/local/include, the PostgreSQL include directory which is defined at compile time (default: /usr/local/pgsql/include), and /usr/include, in that order."
		Switch="D [value]"
  />


	  https://www.gnu.org/software/ecpg/manual/html_node/Understanding.html#Understanding;
  Switch="v" />

https://www.postgresql.org/docs/9.5/static/app-ecpg.html;
  Switch="t" />

	https://www.postgresql.org/docs/9.5/static/app-ecpg.html;
	Switch="c" />

	 https://www.postgresql.org/docs/9.5/static/app-ecpg.html;
	  Switch="i" />

	  https://www.postgresql.org/docs/9.5/static/app-ecpg.html;
  DisplayName="RunTime"
  Description="Selects run-time behavior. Option can be one of the following:
		no_indicator:	Do not use indicators but instead use special values to represent null values. Historically there have been databases using this approach.
		prepare:		Prepare all statements before using them. Libecpg will keep a cache of prepared statements and reuse a statement if it gets executed again. If the cache runs full, libecpg will free the least used statement.
		questionmarks:	Allow question mark as placeholder for compatibility reasons. This used to be the default long ago.">

  
  
  
 

	  https://www.postgresql.org/docs/9.5/static/app-ecpg.html;
  DisplayName="CompatibilityMode"
  Description="Set a compatibility mode. mode can be INFORMIX or INFORMIX_SE.">

		  
		  
	  

	  
  

  





  
Execute Before
  
  
Specifies the targets for the build customization to run before.
  
  

  
  

  


  
Execute After
  
  
Specifies the targets for the build customization to run after.
  
  

  
  

  





  
Additional Options
  
  
Additional Options
  

  
  
  
  
   
-- 
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]: Extends VisualStudio to automatically precompile EmbeddedSQL

[Michael Paquier]

On Tue, May 2, 2017 at 6:35 PM, zosrothko  wrote:
> Hi
>
> I made an extension of VisualStudio that precompiles automaticaly C or
> C++ source with PostgreSQL Embedded SQL. The extension is made of the 3
> files joined and I have no idea where they should be placed in the
> PostgreSQL source tree.
>
> Anybody interested in pushing this extension?

The PostgreSQL uses a set of scripts in src/tools/msvc/ to generate
things compiled with visual studio, so instinctively you would be
looking at working on that. Honestly, just by looking at the files you
are proposing, it is hard to make an idea of why this would be useful
for ECPG, and please note as well that there are guidelines for
submitting patches:
https://wiki.postgresql.org/wiki/Submitting_a_Patch

The source code of ecpg is located in src/interfaces/ecpg by the way.
If you are interested to work on it, that's the place where to look at
first.
-- 
Michael


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

[HACKERS] Typos in comments in partition.c

[Etsuro Fujita]

Hi,

I ran into typos in comments in RelationGetPartitionDispatchInfo() in  
partition.c.  Here is a small patch for: s/all the leaf partition/all  
the leaf partitions/ and s/we we/we/.


Best regards,
Etsuro Fujita
diff --git a/src/backend/catalog/partition.c b/src/backend/catalog/partition.c
index e0d2665..8641ae1 100644
--- a/src/backend/catalog/partition.c
+++ b/src/backend/catalog/partition.c
@@ -988,7 +988,7 @@ RelationGetPartitionQual(Relation rel)
  *		Returns information necessary to route tuples down a partition tree
  *
  * All the partitions will be locked with lockmode, unless it is NoLock.
- * A list of the OIDs of all the leaf partition of rel is returned in
+ * A list of the OIDs of all the leaf partitions of rel is returned in
  * *leaf_part_oids.
  */
 PartitionDispatch *
@@ -1012,9 +1012,9 @@ RelationGetPartitionDispatchInfo(Relation rel, int lockmode,
 	 *
 	 * Cannot use find_all_inheritors() here, because then the order of OIDs
 	 * in parted_rels list would be unknown, which does not help, because we
-	 * we assign indexes within individual PartitionDispatch in an order that
-	 * is predetermined (determined by the order of OIDs in individual
-	 * partition descriptors).
+	 * assign indexes within individual PartitionDispatch in an order that is
+	 * predetermined (determined by the order of OIDs in individual partition
+	 * descriptors).
 	 */
 	*num_parted = 1;
 	parted_rels = list_make1(rel);

-- 
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 in prepared statement cache invalidation?

[Konstantin Knizhnik]

On 01.05.2017 16:09, Robert Haas wrote:


This problem has been discussed before but nobody's done anything
about it.  The problem is a bit tricky because the core system doesn't
know anything about the function caches maintained by individual PLs.
I suppose ideally there'd be a way for a PL to say "if the definition
of X changes, please tell me to recompile function Y".  That probably
wouldn't be perfect because the PL might not be able to figure out
everything on which they actually depend; that might be
Turing-complete in some cases.  But even a partial solution would
probably be welcomed by users.



Thank you for explanation.
May be I am missing something, but what is the problem with keeping 
dependencies for  PL functions?
As you wrote, PL can inform core that functions depends on some set of 
relations/types/functions and so has to be recompiled if some of them 
are changed.
It is not necessary to build closure of dependency graph - instead of it 
cascade invalidation can be used.
So it is not clear to me where you see here the source of complexity and 
why this task may be "Turing-complete in some cases"?


The problem can be with overloaded functions and PL languages without 
static type checking.
In this case  resolving has to be performed at runtime during function 
evaluation. But there should be no such problem with PLpgSQL.


But definitely introducing such dependency tracking mechanism for PL 
will require a lot of changes, in all PL implementations. Looks like no 
easy fix is possible here.
I am not sure how critical is this problem. Definitely it rarely 
happens, but lack of normal workarounds (restart backend, recreate 
function?) seems to be  disappointing.



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

Re: [HACKERS] Cached plans and statement generalization

[Konstantin Knizhnik]

On 01.05.2017 18:52, Robert Haas wrote:
On Fri, Apr 28, 2017 at 6:01 AM, Konstantin Knizhnik 
> wrote:



Any comments and suggestions for future improvement of this patch
are welcome.


+PG_TRY();
+{
+query = parse_analyze_varparams(parse_tree,
+query_string,
+ _types,
+ _params);
+}
+PG_CATCH();
+{
+/*
+ * In case of analyze errors revert back to original 
query processing
+ * and disable autoprepare for this query to avoid such 
problems in future.

+ */
+FlushErrorState();
+if (snapshot_set) {
+PopActiveSnapshot();
+}
+entry->disable_autoprepare = true;
+undo_query_plan_changes(parse_tree, const_param_list);
+MemoryContextSwitchTo(old_context);
+return false;
+}
+PG_END_TRY();

This is definitely not a safe way of using TRY/CATCH.

+
+/* Convert literal value to parameter value */
+switch (const_param->literal->val.type)
+{
+  /*
+   * Convert from integer literal
+   */
+  case T_Integer:
+switch (ptype) {
+  case INT8OID:
+params->params[paramno].value = 
Int64GetDatum((int64)const_param->literal->val.val.ival);

+break;
+  case INT4OID:
+params->params[paramno].value = 
Int32GetDatum((int32)const_param->literal->val.val.ival);

+break;
+  case INT2OID:
+if (const_param->literal->val.val.ival < SHRT_MIN
+|| const_param->literal->val.val.ival > SHRT_MAX)
+{
+ereport(ERROR,
+ (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
+ errmsg("smallint out of range")));
+}
+params->params[paramno].value = 
Int16GetDatum((int16)const_param->literal->val.val.ival);

+break;
+  case FLOAT4OID:
+params->params[paramno].value = 
Float4GetDatum((float)const_param->literal->val.val.ival);

+break;
+  case FLOAT8OID:
+params->params[paramno].value = 
Float8GetDatum((double)const_param->literal->val.val.ival);

+break;
+  case INT4RANGEOID:
+sprintf(buf, "[%ld,%ld]", 
const_param->literal->val.val.ival, const_param->literal->val.val.ival);

+getTypeInputInfo(ptype, , );
+params->params[paramno].value = 
OidInputFunctionCall(typinput, buf, typioparam, -1);

+break;
+  default:
+ pg_lltoa(const_param->literal->val.val.ival, buf);
+getTypeInputInfo(ptype, , );
+params->params[paramno].value = 
OidInputFunctionCall(typinput, buf, typioparam, -1);

+}
+break;
+  case T_Null:
+params->params[paramno].isnull = true;
+break;
+  default:
+/*
+ * Convert from string literal
+ */
+getTypeInputInfo(ptype, , );
+params->params[paramno].value = 
OidInputFunctionCall(typinput, const_param->literal->val.val.str, 
typioparam, -1);

+}

I don't see something with a bunch of hard-coded rules for particular 
type OIDs having any chance of being acceptable.




Well, what I need is to convert literal value represented in Value 
struct to parameter datum value.

Struct "value" contains union with integer literal and text.
So this peace of code is just provides efficient handling of most common 
cases (integer parameters) and uses type's input function in other cases.



This patch seems to duplicate a large amount of existing code.  That 
would be a good thing to avoid.


Yes,  I have to copy a lot of code from exec_parse_message + 
exec_bind_message + exec_execute_message functions.
Definitely copying of code is bad flaw. It will be much better and 
easier just to call three original functions instead of mixing gathering 
their code into the new function.

But I failed to do it because
1.  Autoprepare should be integrated into exec_simple_query. Before 
executing query in normal way, I need to perform cache lookup for 
previously prepared plan for this generalized query.
And generalization of query requires building of query tree (query 
parsing). In other words, parsing should be done before I can call 
exec_parse_message.
2. exec_bind_message works with parameters passed by client though 
libpwq protocol, while autoprepare deals with values of parameters 
extracted from literals.

Re: [HACKERS] multi-column range partition constraint

[Beena Emerson]

On Tue, May 2, 2017 at 2:47 PM, Amit Langote 
wrote:

> Hi Beena,
>
> On 2017/05/02 17:47, Beena Emerson wrote:
> > Hello Amit,
> >
> > On Tue, May 2, 2017 at 12:21 PM, Amit Langote <
> langote_amit...@lab.ntt.co.jp
> >> wrote:
> >
> >> Per an off-list report from Olaf Gawenda (thanks Olaf), it seems that
> the
> >> range partition's constraint is sometimes incorrect, at least in the
> case
> >> of multi-column range partitioning.  See below:
> >>
> >> create table p (a int, b int) partition by range (a, b);
> >> create table p1 partition of p for values from (1, 1) to (10 ,10);
> >> create table p2 partition of p for values from (11, 1) to (20, 10);
> >>
> >> Perhaps unusual, but it's still a valid definition.  Tuple-routing puts
> >> rows where they belong correctly.
> >>
> >> -- ok
> >> insert into p values (10, 9);
> >> select tableoid::regclass, * from p;
> >>  tableoid | a  | b
> >> --++---
> >>  p1   | 10 | 9
> >> (1 row)
> >>
> >> -- but see this
> >> select tableoid::regclass, * from p where a = 10;
> >>  tableoid | a | b
> >> --+---+---
> >> (0 rows)
> >>
> >> explain select tableoid::regclass, * from p where a = 10;
> >> QUERY PLAN
> >> ---
> >>  Result  (cost=0.00..0.00 rows=0 width=12)
> >>One-Time Filter: false
> >> (2 rows)
> >>
> >> -- or this
> >> insert into p1 values (10, 9);
> >> ERROR:  new row for relation "p1" violates partition constraint
> >> DETAIL:  Failing row contains (10, 9).
> >>
> >> This is because of the constraint being generated is not correct in this
> >> case.  p1's constraint is currently:
> >>
> >>   a >= 1 and a < 10
> >>
> >> where it should really be the following:
> >>
> >>   (a > 1  OR (a = 1  AND b >= 1))
> >> AND
> >>   (a < 10 OR (a = 10 AND b < 10))
> >>
> >
> >
> > IIUC, when we say range 1 to 10 we allow values from 1 to 9. Here we are
> > allowing a=10 be stored in p1 Is it okay?
>
> Yes it is.  Consider that the multi-column range partitioning uses
> tuple-comparison logic to determine if a row's partition key is >=
> lower_bound and < upper_bound tuple.
>
> In this case, p1's lower bound is a "tuple" (1, 1) and (10, 10) its upper
> bound.  Consider an input row with (2, -1) as its partition key.
> Tuple-comparison logic puts this row into p1, because:
>
> select (1, 1) <= (2, -1) and (2, -1) < (10, 10);
>  ?column?
> --
>  t
> (1 row)
>
> When performing tuple-comparison with the lower bound, since 2 > 1, the
> tuple comparison is done and the whole tuple is concluded to be > (1, 1);
> no attention is paid to the second column (or to the fact that -1 not >=
> 1).


> Now consider an input row with (10, 9) as its partition key, which too
> fits in p1, because:
>
> select (1, 1) <= (10, 9) and (10, 9) < (10, 10);
>  ?column?
> --
>  t
> (1 row)
>
> In this case, since 10 = 10, tuple-comparison considers the next column to
> determine the result.  In this case, since the second column 9 < 10, the
> whole tuple is concluded to be < (10, 10).
>
> However, neither of (1, 0), (10, 10), or (10, 11) is admissible into p1 by
> the above comparison logic:
>
> select (1, 1) <= (1, 0) and (1, 0) < (10, 10);
>  ?column?
> --
>  f
> (1 row)
>
> select (1, 1) <= (10, 10) and (10, 10) < (10, 10);
>  ?column?
> --
>  f
> (1 row)
>
> select (1, 1) <= (10, 11) and (10, 11) < (10, 10);
>  ?column?
> --
>  f
> (1 row)
>
> I havent been following these partition mails much. Sorry if I am missing
> > something obvious.
>
> No problem.
>

I have recently started looking at partition. I was wondering why 2nd
column is ignored and the exceptions.

For following partitions:
create table p1 partition of p for values from (1, 1) to (10 ,10);
create table p2 partition of p for values from (11, 1) to (20, 10);

IIUC, we can store values a = 1 to 9 , 11 to 19 and any value in b. But can
store 10 and 20 only when b <=9.

This still seems a bit confusing to me but thank you for your explanation.
These are just rules u have to abide by I guess!


>
> >> Attached patch rewrites get_qual_for_range() for the same, along with
> some
> >> code rearrangement for reuse.  I also added some new tests to insert.sql
> >> and inherit.sql, but wondered (maybe, too late now) whether there should
> >> really be a declarative_partition.sql for these, moving in some of the
> old
> >> tests too.
> >>
> > I got the following warning on compiling:
> > partition.c: In function ‘make_partition_op_expr’:
> > partition.c:1267:2: warning: ‘result’ may be used uninitialized in this
> > function [-Wmaybe-uninitialized]
> >   return result;
>
> Oops, fixed.  Updated patch attached.
>
>
Thank you,


-- 

Beena Emerson

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

[HACKERS] [Proposal]: Extends VisualStudio to automatically precompile EmbeddedSQL

[zosrothko]

Hi

I made an extension of VisualStudio that precompiles automaticaly C or
C++ source with PostgreSQL Embedded SQL. The extension is made of the 3
files joined and I have no idea where they should be placed in the
PostgreSQL source tree.

Anybody interested in pushing this extension?

zos




ecpg.props
Description: XML document

http://schemas.microsoft.com/developer/msbuild/2003;>
  


  EcpgTarget

  
  
$(MSBuildThisFileDirectory)$(MSBuildThisFileName).xml
  
  

  


  
@(Ecpg, '|')
  




  
  

$(ComputeLinkInputsTargets);
ComputeEcpgOutput;
  

$(ComputeLibInputsTargets);
ComputeEcpgOutput;
  
  
  

  
  
  
  


  

http://schemas.microsoft.com/winfx/2006/xaml; xmlns:sys="clr-namespace:System;assembly=mscorlib" xmlns:transformCallback="Microsoft.Cpp.Dev10.ConvertPropertyCallback">
  

  


  

  General

  
  

  Ecpg Options

  
  

  Command Line

  


https://www.postgresql.org/docs/9.5/static/app-ecpg.html;
  DisplayName="Output File Name"
  Description="Specifies that ecpg should write all its output to the given filename"
  Switch="o [value]"
  />

	  https://www.postgresql.org/docs/9.5/static/app-ecpg.html;
		Description="Define a C preprocessor symbol."
		Switch="D [value]"
  />

	  https://www.postgresql.org/docs/9.5/static/app-ecpg.html;
		Description="Specify an additional include path, used to find files included via EXEC SQL INCLUDE. Defaults are . (current directory), /usr/local/include, the PostgreSQL include directory which is defined at compile time (default: /usr/local/pgsql/include), and /usr/include, in that order."
		Switch="D [value]"
  />


	  https://www.gnu.org/software/ecpg/manual/html_node/Understanding.html#Understanding;
  Switch="v" />

https://www.postgresql.org/docs/9.5/static/app-ecpg.html;
  Switch="t" />

	https://www.postgresql.org/docs/9.5/static/app-ecpg.html;
	Switch="c" />

	 https://www.postgresql.org/docs/9.5/static/app-ecpg.html;
	  Switch="i" />

	  https://www.postgresql.org/docs/9.5/static/app-ecpg.html;
  DisplayName="RunTime"
  Description="Selects run-time behavior. Option can be one of the following:
		no_indicator:	Do not use indicators but instead use special values to represent null values. Historically there have been databases using this approach.
		prepare:		Prepare all statements before using them. Libecpg will keep a cache of prepared statements and reuse a statement if it gets executed again. If the cache runs full, libecpg will free the least used statement.
		questionmarks:	Allow question mark as placeholder for compatibility reasons. This used to be the default long ago.">

  
  
  
 

	  https://www.postgresql.org/docs/9.5/static/app-ecpg.html;
  DisplayName="CompatibilityMode"
  Description="Set a compatibility mode. mode can be INFORMIX or INFORMIX_SE.">

		  
		  
	  

	  
  

  





  
Execute Before
  
  
Specifies the targets for the build customization to run before.
  
  

  
  

  


  
Execute After
  
  
Specifies the targets for the build customization to run after.
  
  

  
  

  





  
Additional Options
  
  
Additional Options
  

  
  
  
  
   

-- 
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] multi-column range partition constraint

[Amit Langote]

Hi Beena,

On 2017/05/02 17:47, Beena Emerson wrote:
> Hello Amit,
> 
> On Tue, May 2, 2017 at 12:21 PM, Amit Langote > wrote:
> 
>> Per an off-list report from Olaf Gawenda (thanks Olaf), it seems that the
>> range partition's constraint is sometimes incorrect, at least in the case
>> of multi-column range partitioning.  See below:
>>
>> create table p (a int, b int) partition by range (a, b);
>> create table p1 partition of p for values from (1, 1) to (10 ,10);
>> create table p2 partition of p for values from (11, 1) to (20, 10);
>>
>> Perhaps unusual, but it's still a valid definition.  Tuple-routing puts
>> rows where they belong correctly.
>>
>> -- ok
>> insert into p values (10, 9);
>> select tableoid::regclass, * from p;
>>  tableoid | a  | b
>> --++---
>>  p1   | 10 | 9
>> (1 row)
>>
>> -- but see this
>> select tableoid::regclass, * from p where a = 10;
>>  tableoid | a | b
>> --+---+---
>> (0 rows)
>>
>> explain select tableoid::regclass, * from p where a = 10;
>> QUERY PLAN
>> ---
>>  Result  (cost=0.00..0.00 rows=0 width=12)
>>One-Time Filter: false
>> (2 rows)
>>
>> -- or this
>> insert into p1 values (10, 9);
>> ERROR:  new row for relation "p1" violates partition constraint
>> DETAIL:  Failing row contains (10, 9).
>>
>> This is because of the constraint being generated is not correct in this
>> case.  p1's constraint is currently:
>>
>>   a >= 1 and a < 10
>>
>> where it should really be the following:
>>
>>   (a > 1  OR (a = 1  AND b >= 1))
>> AND
>>   (a < 10 OR (a = 10 AND b < 10))
>>
> 
> 
> IIUC, when we say range 1 to 10 we allow values from 1 to 9. Here we are
> allowing a=10 be stored in p1 Is it okay?

Yes it is.  Consider that the multi-column range partitioning uses
tuple-comparison logic to determine if a row's partition key is >=
lower_bound and < upper_bound tuple.

In this case, p1's lower bound is a "tuple" (1, 1) and (10, 10) its upper
bound.  Consider an input row with (2, -1) as its partition key.
Tuple-comparison logic puts this row into p1, because:

select (1, 1) <= (2, -1) and (2, -1) < (10, 10);
 ?column?
--
 t
(1 row)

When performing tuple-comparison with the lower bound, since 2 > 1, the
tuple comparison is done and the whole tuple is concluded to be > (1, 1);
no attention is paid to the second column (or to the fact that -1 not >= 1).

Now consider an input row with (10, 9) as its partition key, which too
fits in p1, because:

select (1, 1) <= (10, 9) and (10, 9) < (10, 10);
 ?column?
--
 t
(1 row)

In this case, since 10 = 10, tuple-comparison considers the next column to
determine the result.  In this case, since the second column 9 < 10, the
whole tuple is concluded to be < (10, 10).

However, neither of (1, 0), (10, 10), or (10, 11) is admissible into p1 by
the above comparison logic:

select (1, 1) <= (1, 0) and (1, 0) < (10, 10);
 ?column?
--
 f
(1 row)

select (1, 1) <= (10, 10) and (10, 10) < (10, 10);
 ?column?
--
 f
(1 row)

select (1, 1) <= (10, 11) and (10, 11) < (10, 10);
 ?column?
--
 f
(1 row)


> I havent been following these partition mails much. Sorry if I am missing
> something obvious.

No problem.

>> Attached patch rewrites get_qual_for_range() for the same, along with some
>> code rearrangement for reuse.  I also added some new tests to insert.sql
>> and inherit.sql, but wondered (maybe, too late now) whether there should
>> really be a declarative_partition.sql for these, moving in some of the old
>> tests too.
>>
> I got the following warning on compiling:
> partition.c: In function ‘make_partition_op_expr’:
> partition.c:1267:2: warning: ‘result’ may be used uninitialized in this
> function [-Wmaybe-uninitialized]
>   return result;

Oops, fixed.  Updated patch attached.

Thanks,
Amit
>From d31d6d885a600cafc74b3068388905ae9e635ab0 Mon Sep 17 00:00:00 2001
From: amit 
Date: Tue, 2 May 2017 11:03:54 +0900
Subject: [PATCH] Emit "correct" range partition constraint expression

Currently emitted expression does not always describe the constraint
correctly, especially in the case of multi-column range key.

Code surrounding get_qual_for_*() has been rearranged a little to
move common code to a couple of new functions.

Original issue reported by Olaf Gawenda (olaf...@googlemail.com)
---
 src/backend/catalog/partition.c   | 620 ++
 src/include/nodes/pg_list.h   |  10 +
 src/test/regress/expected/inherit.out |  90 +
 src/test/regress/expected/insert.out  |  59 
 src/test/regress/sql/inherit.sql  |  18 +
 src/test/regress/sql/insert.sql   |  43 +++
 6 files changed, 619 insertions(+), 221 deletions(-)

diff --git a/src/backend/catalog/partition.c b/src/backend/catalog/partition.c
index e0d2665a91..25f51e3278 100644
--- a/src/backend/catalog/partition.c
+++ b/src/backend/catalog/partition.c
@@ -118,10 

Re: [HACKERS] [PostgreSQL 10] default of hot_standby should be "on"?

[Magnus Hagander]

On Fri, Apr 28, 2017 at 3:43 AM, Masahiko Sawada 
wrote:

> On Thu, Apr 27, 2017 at 11:05 PM, Huong Dangminh
>  wrote:
> >> >>> I would refrain from doing that, having some parameters listed in
> the
> >> >>> tests makes the intention behind those perl routines clear.
> >> >
> >> > Hmm, you've got a point. But when we changed the default values
> >> > related to replication we dropped some explicitly settings from the
> >> > regression test code.
> >>
> >> Looking at the patch. This is fine:
> >> -  # Change a setting and restart
> >> -  $node->append_conf('postgresql.conf', 'hot_standby = on');
> >> -  $node->restart();
> >>
> >> But not that:
> >>  print $conf "wal_log_hints = on\n";
> >> -print $conf "hot_standby = on\n";
> >>  print $conf "max_connections = 10\n";
> >>
> >> This is a minor point though.
>
> After some thoughts I agree to remain it in the perl code.
>
> >
> > Thanks, I attached the update patch.
>
> So it looks good to me.
>


Looks good to me as well. Applied, with only a minor further docs addition
saying that this is the default also on the high availability page. And per
the comments from Michael, I did not include the change to PostgresNode.pm.

Thanks!

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

[HACKERS] Regarding B-Tree Lookup

[Mahi Gurram]

Hi,

I'm building some custom extension on top of postgres 9.6.1. As part of
that, I would like to read Heap Tuple directly from my extension using
Primary Key.

By default, postgres table index(B-Tree) its PrimaryKey(PK). So, i would
like to get TID by doing a lookup into PK's B-Tree index. Using which i
could read HeapTuple directly.

Please suggest me the easiest way to lookup into PK's B-Tree index for
getting TIDs.

Suggesting a postgres extensions which does B-Tree lookup will also helps
me.

Awaiting your response.

Thanks & Best Regards
- Mahi

Re: [HACKERS] multi-column range partition constraint

[Beena Emerson]

Hello Amit,

On Tue, May 2, 2017 at 12:21 PM, Amit Langote  wrote:

> Per an off-list report from Olaf Gawenda (thanks Olaf), it seems that the
> range partition's constraint is sometimes incorrect, at least in the case
> of multi-column range partitioning.  See below:
>
> create table p (a int, b int) partition by range (a, b);
> create table p1 partition of p for values from (1, 1) to (10 ,10);
> create table p2 partition of p for values from (11, 1) to (20, 10);
>
> Perhaps unusual, but it's still a valid definition.  Tuple-routing puts
> rows where they belong correctly.
>
> -- ok
> insert into p values (10, 9);
> select tableoid::regclass, * from p;
>  tableoid | a  | b
> --++---
>  p1   | 10 | 9
> (1 row)
>
> -- but see this
> select tableoid::regclass, * from p where a = 10;
>  tableoid | a | b
> --+---+---
> (0 rows)
>
> explain select tableoid::regclass, * from p where a = 10;
> QUERY PLAN
> ---
>  Result  (cost=0.00..0.00 rows=0 width=12)
>One-Time Filter: false
> (2 rows)
>
> -- or this
> insert into p1 values (10, 9);
> ERROR:  new row for relation "p1" violates partition constraint
> DETAIL:  Failing row contains (10, 9).
>
> This is because of the constraint being generated is not correct in this
> case.  p1's constraint is currently:
>
>   a >= 1 and a < 10
>
> where it should really be the following:
>
>   (a > 1  OR (a = 1  AND b >= 1))
> AND
>   (a < 10 OR (a = 10 AND b < 10))
>


IIUC, when we say range 1 to 10 we allow values from 1 to 9. Here we are
allowing a=10 be stored in p1 Is it okay?

I havent been following these partition mails much. Sorry if I am missing
something obvious.


>
> Attached patch rewrites get_qual_for_range() for the same, along with some
> code rearrangement for reuse.  I also added some new tests to insert.sql
> and inherit.sql, but wondered (maybe, too late now) whether there should
> really be a declarative_partition.sql for these, moving in some of the old
> tests too.
>
> Adding to the open items list.
>
> Thanks,
> Amit
>
> PS: due to vacation, I won't be able to reply promptly until Monday 05/08.
>
>
I got the following warning on compiling:
partition.c: In function ‘make_partition_op_expr’:
partition.c:1267:2: warning: ‘result’ may be used uninitialized in this
function [-Wmaybe-uninitialized]
  return result;


-- 

Beena Emerson

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