Re: [HACKERS] OK, so culicidae is *still* broken

2017-04-30 Thread Amit Kapila
On Mon, May 1, 2017 at 10:23 AM, Tom Lane  wrote:
> Amit Kapila  writes:
>> Yeah, that's right.  Today, I have spent some time to analyze how and
>> where retry logic is required.  I think there are two places where we
>> need this retry logic, one is if we fail to reserve the memory
>> (pgwin32_ReserveSharedMemoryRegion) and second is if we fail to
>> reattach (PGSharedMemoryReAttach).
>
> I'm not following.  The point of the reserve operation is to ensure
> that the reattach will work.  What makes you think we need to add more
> code at that end?
>

We use VirtualAllocEx to allocate memory at a pre-defined address
(reserve operation) which can fail due to ASLR.  One recent example of
such a symptom is seen on pgsql-bugs [1], that failure is during
reserve operation and seems like something related to ASLR.  Another
point is the commit 7f3e17b4827b61ad84e0774e3e43da4c57c4487f (It
doesn't fail every time (which is explained by the Random part in
ASLR), but can fail with errors abut failing to reserve shared memory
region) also indicates that we can fail to reserve memory due to ASLR.


[1] - https://www.postgresql.org/message-id/14121.1485360296%40sss.pgh.pa.us

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


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


Re: [HACKERS] snapbuild woes

2017-04-30 Thread Petr Jelinek
On 01/05/17 04:29, Craig Ringer wrote:
> On 1 May 2017 at 09:54, Petr Jelinek  wrote:
> 
>> But, I still think we need to restart the tracking after new
>> xl_running_xacts. Reason for that is afaics any of the catalog snapshots
>> that we assigned to transactions at the end of SnapBuildCommitTxn might
>> be corrupted otherwise as they were built before we knew one of the
>> supposedly running txes was actually already committed and that
>> transaction might have done catalog changes.
> 
> Due to the race where LogStandbySnapshot() collects running-xacts info
> while a concurrent xact commits, such that the xl_xact_commit appears
> before the xl_running_xacts, but the xl_running_xacts still has the
> commited xact listed as running, right? Because we update PGXACT only
> after we write the commit to WAL, so there's a window where an xact is
> committed in WAL but not shown as committed in shmem.
> 

Yes that's what the patch at hand tries to fix, but Andres approached it
from too simplistic standpoint as we don't only care about the exported
snapshot but whatever catalog snapshots we made for the transactions we
already track, unless I am missing something.

-- 
  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] OK, so culicidae is *still* broken

2017-04-30 Thread Tom Lane
Amit Kapila  writes:
> Yeah, that's right.  Today, I have spent some time to analyze how and
> where retry logic is required.  I think there are two places where we
> need this retry logic, one is if we fail to reserve the memory
> (pgwin32_ReserveSharedMemoryRegion) and second is if we fail to
> reattach (PGSharedMemoryReAttach).

I'm not following.  The point of the reserve operation is to ensure
that the reattach will work.  What makes you think we need to add more
code at that end?

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] Declarative partitioning - another take

2017-04-30 Thread Amit Langote
On 2017/04/29 6:56, David Fetter wrote:
> On Fri, Apr 28, 2017 at 06:29:48PM +0900, Amit Langote wrote:
>> On 2017/04/28 7:36, David Fetter wrote:
>>> Did I notice correctly that there's no way to handle transition tables
>>> for partitions in AFTER STATEMENT triggers?
>>
>> Did you mean to ask about AFTER STATEMENT triggers defined on
>> "partitioned" tables?  Specifying transition table for them is disallowed
>> at all.
>>
>> ERROR:  "p" is a partitioned table
>> DETAIL:  Triggers on partitioned tables cannot have transition tables.
> 
> OK, I suppose.  It wasn't clear from the documentation.
> 
>> Triggers created on (leaf) partitions *do* allow specifying transition table.
> 
> That includes the upcoming "default" tables, I presume.

If a "default" table is also a "leaf" table, then yes.  A default
table/partition can also be itself a partitioned table, in which case, it
won't allow triggers that require transition tables.  AFAICS, it's the
table's being partitioned that stops it from supporting transition tables,
not whether it is a "default" partition or not.

>> Or are you asking something else altogether?
> 
> I was just fuzzy on the interactions among these features.
> 
>>> If not, I'm not suggesting that this be added at this late date, but
>>> we might want to document that.
>>
>> I don't see mentioned in the documentation that such triggers cannot be
>> defined on partitioned tables.  Is that what you are saying should be
>> documented?
> 
> Yes, but I bias toward documenting a lot, and this restriction could
> go away in some future version, which would make things more confusing
> in the long run.

Yeah, it would be a good idea to document this.

> I'm picturing a conversation in 2020 that goes
> something like this:
> 
> "On 10, you could have AFTER STATEMENT triggers on tables, foreigh
> tables, and leaf partition tables which referenced transition tables,
> but not on DEFAULT partitions.  On 11, you could on DEFAULT partition
> tables.  From 12 onward, you can have transition tables on any
> relation."

What we could document now is that partitioned tables don't allow
specifying triggers that reference transition tables.  Although, I am
wondering where this note really belongs - the partitioning chapter, the
triggers chapter or the CREATE TRIGGER reference page?  Maybe, Kevin and
Thomas have something to say about that.  If it turns out that the
partitioning chapter is a good place, here is a documentation patch.

Thanks,
Amit
>From 289b4d906abb50451392d0efe13926f710952ca0 Mon Sep 17 00:00:00 2001
From: amit 
Date: Mon, 1 May 2017 13:46:58 +0900
Subject: [PATCH] Document transition table trigger limitation of partitioned
 tables

---
 doc/src/sgml/ddl.sgml | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/doc/src/sgml/ddl.sgml b/doc/src/sgml/ddl.sgml
index 84c4f20990..5f5a956d41 100644
--- a/doc/src/sgml/ddl.sgml
+++ b/doc/src/sgml/ddl.sgml
@@ -3293,7 +3293,9 @@ ALTER TABLE measurement ATTACH PARTITION measurement_y2008m02
  
   
Row triggers, if necessary, must be defined on individual partitions,
-   not the partitioned table.
+   not the partitioned tables.  Also, triggers that require accessing
+   transition tables are not allowed to be defined on the partitioned
+   tables.
   
  
 
-- 
2.11.0


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


Re: [HACKERS] OK, so culicidae is *still* broken

2017-04-30 Thread Amit Kapila
On Tue, Apr 25, 2017 at 7:37 PM, Tom Lane  wrote:
> Craig Ringer  writes:
>> On 25 Apr. 2017 13:37, "Heikki Linnakangas"  wrote:
>>> For some data shared memory structures, that store no pointers, we wouldn't
>>> need to insist that they are mapped to the same address in every backend,
>>> though. In particular, shared_buffers. It wouldn't eliminate the problem,
>>> though, only make it less likely, so we'd still need to retry when it does
>>> happen.
>
>> Good point. Simply splitting out shared_buffers into a moveable segment
>> would make a massive difference. Much less chance of losing the dice roll
>> for mapping the fixed segment.
>
>> Should look at what else could be made cheaply relocatable too.
>
> I don't think it's worth spending any effort on.  We need the retry
> code anyway, and making it near impossible to hit that would only mean
> that it's very poorly tested.  The results upthread say that it isn't
> going to be hit often enough to be a performance problem, so why worry?
>

Yeah, that's right.  Today, I have spent some time to analyze how and
where retry logic is required.  I think there are two places where we
need this retry logic, one is if we fail to reserve the memory
(pgwin32_ReserveSharedMemoryRegion) and second is if we fail to
reattach (PGSharedMemoryReAttach). If we fail during reserve memory
operation, then we can simply terminate the process and recreate it
again, basically, add some kind of loop in internal_forkexec(). OTOH,
if we fail during reattach, I think we need an exit from the process
which means it can lead to a restart of all the processes unless we
want to add some special logic for handling process exit or
alternatively we might want to just try reattach operation in a loop
before giving up. Do we want to keep this retry logic for a certain
number of times say (10 times) or we want to try indefinitely?


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


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


Re: [HACKERS] CTE inlining

2017-04-30 Thread Pavel Stehule
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.

Regards

Pavel


>
> - Andres
>


Re: [HACKERS] Declarative partitioning - another take

2017-04-30 Thread Amit Langote
Thanks for the review.

On 2017/04/29 1:25, Robert Haas wrote:
>  On Fri, Apr 28, 2017 at 2:13 AM, Amit Langote
>  wrote:
 By the way, code changes I made in the attached are such that a subsequent
 patch could implement firing statement-level triggers of all the tables in
 a partition hierarchy, which it seems we don't want to do.  Should then
 the code be changed to not create ResultRelInfos of all the tables but
 only the root table (the one mentioned in the command)?  You will see that
 the patch adds fields named es_nonleaf_result_relations and
 es_num_nonleaf_result_relations, whereas just es_root_result_relation
 would perhaps do, for example.
>>>
>>> It seems better not to create any ResultRelInfos that we don't
>>> actually need, so +1 for such a revision to the patch.
>>
>> OK, done.  It took a bit more work than I thought.
> 
> So, this seems weird, because rootResultRelIndex is initialized even
> when splan->partitioned_rels == NIL, but isn't actually valid in that
> case.  ExecInitModifyTable seems to think it's always valid, though.

OK, rootResultRelIndex is now set to a value >= 0 only when the node
modifies a partitioned table.  And then ExecInitModifyTable() checks if
the index is valid before initializing the root table info.

> I think the way that you've refactored fireBSTriggers and
> fireASTriggers is a bit confusing.  Instead of splitting out a
> separate function, how about just having the existing function begin
> with if (node->rootResultRelInfo) resultRelInfo =
> node->rootResultRelInfo; else resultRelInfo = node->resultRelInfo; ?
> I think the way you've coded it is a holdover from the earlier design
> where you were going to call it multiple times, but now that's not
> needed.

OK, done that way.

> Looks OK, otherwise.

Attached updated patch.

Thanks,
Amit
>From 53c964f9f1fd3e27824080b0e9e5b672fa53cdf0 Mon Sep 17 00:00:00 2001
From: amit 
Date: Mon, 24 Apr 2017 14:55:08 +0900
Subject: [PATCH] Fire per-statement triggers of partitioned tables

Current implementation completely misses them, because it assumes
that no ResultRelInfos need to be created for partitioned tables,
whereas they *are* needed to fire the per-statment triggers, which
we *do* allow to be defined on the partitioned tables.

Reported By: Rajkumar Raghuwanshi
Patch by: Amit Langote
Report: https://www.postgresql.org/message-id/CAKcux6%3DwYospCRY2J4XEFuVy0L41S%3Dfic7rmkbsU-GXhhSbmBg%40mail.gmail.com
---
 doc/src/sgml/trigger.sgml   | 26 +++
 src/backend/executor/execMain.c | 55 --
 src/backend/executor/nodeModifyTable.c  | 42 +
 src/backend/nodes/copyfuncs.c   |  2 +
 src/backend/nodes/outfuncs.c|  3 ++
 src/backend/nodes/readfuncs.c   |  2 +
 src/backend/optimizer/plan/createplan.c |  1 +
 src/backend/optimizer/plan/planner.c|  3 ++
 src/backend/optimizer/plan/setrefs.c| 19 ++--
 src/include/nodes/execnodes.h   | 12 +
 src/include/nodes/plannodes.h   | 13 +-
 src/include/nodes/relation.h|  1 +
 src/test/regress/expected/triggers.out  | 81 +
 src/test/regress/sql/triggers.sql   | 70 
 14 files changed, 303 insertions(+), 27 deletions(-)

diff --git a/doc/src/sgml/trigger.sgml b/doc/src/sgml/trigger.sgml
index 2a718d7f47..5771bd5fdc 100644
--- a/doc/src/sgml/trigger.sgml
+++ b/doc/src/sgml/trigger.sgml
@@ -33,7 +33,8 @@

 A trigger is a specification that the database should automatically
 execute a particular function whenever a certain type of operation is
-performed.  Triggers can be attached to tables, views, and foreign tables.
+performed.  Triggers can be attached to tables (partitioned or not),
+views, and foreign tables.
   
 
   
@@ -111,14 +112,21 @@
 Statement-level BEFORE triggers naturally fire before the
 statement starts to do anything, while statement-level AFTER
 triggers fire at the very end of the statement.  These types of
-triggers may be defined on tables or views.  Row-level BEFORE
-triggers fire immediately before a particular row is operated on,
-while row-level AFTER triggers fire at the end of the
-statement (but before any statement-level AFTER triggers).
-These types of triggers may only be defined on tables and foreign tables.
-Row-level INSTEAD OF triggers may only be defined on views,
-and fire immediately as each row in the view is identified as needing to
-be operated on.
+triggers may be defined on tables, views, or foreign tables.  Row-level
+BEFORE triggers fire immediately before a particular row is
+operated on, while row-level AFTER triggers fire at the end of
+the statement (but before any statement-level AFTER triggers).
+These types of triggers may only be defined on non-partitioned tables and

Re: [HACKERS] snapbuild woes

2017-04-30 Thread Craig Ringer
On 1 May 2017 at 09:54, Petr Jelinek  wrote:

> But, I still think we need to restart the tracking after new
> xl_running_xacts. Reason for that is afaics any of the catalog snapshots
> that we assigned to transactions at the end of SnapBuildCommitTxn might
> be corrupted otherwise as they were built before we knew one of the
> supposedly running txes was actually already committed and that
> transaction might have done catalog changes.

Due to the race where LogStandbySnapshot() collects running-xacts info
while a concurrent xact commits, such that the xl_xact_commit appears
before the xl_running_xacts, but the xl_running_xacts still has the
commited xact listed as running, right? Because we update PGXACT only
after we write the commit to WAL, so there's a window where an xact is
committed in WAL but not shown as committed in shmem.

-- 
 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] snapbuild woes

2017-04-30 Thread Petr Jelinek
On 01/05/17 03:35, Andres Freund wrote:
> On 2017-04-30 17:59:21 -0700, Andres Freund wrote:
>> ISTM, we need a xip_status array in SnapBuild->running.  Then, whenever
>> a xl_running_xacts is encountered while builder->running.xcnt > 0, loop
>> for i in 0 .. xcnt_space, and end SnapBuildEndTxn() if xip_status[i] but
>> the xid is not xl_running_xacts?  Does that make sense?
> 
> A hasty implementation, untested besides check-world, of that approach
> is attached.  I'm going out for dinner now, will subject it to mean
> things afterwards.
> 
> Needs more testing, comment and commit message policing.  But I think
> the idea is sound?
> 

I agree with adding running, I think that's good thing even for the per
transaction tracking and snapshot exports - we could use the newly added
field to get rid of the issue we have with 'snapshot too large' when
there were many aborted transactions while we waited for running ones to
finish. Because so far only tracked committed transactions, any aborted
one would show as running inside the exported snapshot so it can grow
over maximum number of backends and can't be exported anymore. So +1 for
that part.

But, I still think we need to restart the tracking after new
xl_running_xacts. Reason for that is afaics any of the catalog snapshots
that we assigned to transactions at the end of SnapBuildCommitTxn might
be corrupted otherwise as they were built before we knew one of the
supposedly running txes was actually already committed and that
transaction might have done catalog changes.

-- 
  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] Newbie project: Detect attempts to run pg_dump etc on psql, \copy on server

2017-04-30 Thread Craig Ringer
This is a proposal for a new developer patch idea, in case someone's interested.

Keeping an eye on Stack Overflow has shown me that quite a few people
seem to get confused by pg_dump and pg_restore being command line
tools, not psql commands.

It'd be interesting if psql detected this and gave a useful error.
Ideally before they enter ; to terminate the line, since they're
probably following instructions that won't show one.

A related patch would be to have the parser detect \copy and other
psql backslash metacommands run as SQL queries on the server and
report a useful error. For \copy, it would HINT to look at their
client's manual for COPY support or user server-side COPY. For others
it'd say these are psql commands not postgres commands and only work
in the psql client.

One might argue that we don't need to pad things _this_ much for new
users. But I see users getting confused by psql metacommands vs
postgres server commands a lot. Similarly by pg_dump and pg_restore
being shell programs.

If it can be done at minimal cost to everyone else, it might be worth
it. Just an idea.

-- 
 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] snapbuild woes

2017-04-30 Thread Andres Freund
On 2017-04-30 17:59:21 -0700, Andres Freund wrote:
> ISTM, we need a xip_status array in SnapBuild->running.  Then, whenever
> a xl_running_xacts is encountered while builder->running.xcnt > 0, loop
> for i in 0 .. xcnt_space, and end SnapBuildEndTxn() if xip_status[i] but
> the xid is not xl_running_xacts?  Does that make sense?

A hasty implementation, untested besides check-world, of that approach
is attached.  I'm going out for dinner now, will subject it to mean
things afterwards.

Needs more testing, comment and commit message policing.  But I think
the idea is sound?

- Andres
>From 6160eda5177e7f538ff13eacc46acb2f2959c257 Mon Sep 17 00:00:00 2001
From: Andres Freund 
Date: Sun, 30 Apr 2017 18:24:06 -0700
Subject: [PATCH] Fix initial logical decoding snapshat race condition.

LogStandbySnapshot() has no interlock preventing
RecordTransactionCommit() from logging its commit record (abort
situation is similar), before the xl_running_xacts record is
generated.  That can lead to the situation that logical decoding
forever waits for a commit/abort record, that was logged in the past.

To fix, check for such "missed" transactions when xl_running_xacts are
observed, while in SNAPBUILD_FULL_SNAPSHOT.

This also reverts changes made to GetRunningTransactionData() and
LogStandbySnapshot() by b89e151 as the additional locking does not
solve the problem, and is not required anymore.

Author: Petr Jelined and Andres Freund
Reviewed-By: Andres Freund
Discussion: https://postgr.es/m/f37e975c-908f-858e-707f-058d3b1eb...@2ndquadrant.com
Backpatch: 9.4, where logical decoding was introduced
---
 src/backend/replication/logical/snapbuild.c | 98 +++--
 src/backend/storage/ipc/procarray.c |  5 +-
 src/backend/storage/ipc/standby.c   | 19 --
 3 files changed, 81 insertions(+), 41 deletions(-)

diff --git a/src/backend/replication/logical/snapbuild.c b/src/backend/replication/logical/snapbuild.c
index 068d214fa1..860562 100644
--- a/src/backend/replication/logical/snapbuild.c
+++ b/src/backend/replication/logical/snapbuild.c
@@ -203,6 +203,7 @@ struct SnapBuild
 		size_t		xcnt;		/* number of used xip entries */
 		size_t		xcnt_space; /* allocated size of xip */
 		TransactionId *xip;		/* running xacts array, xidComparator-sorted */
+		bool	   *xip_running; /* xid in ->xip still running? */
 	}			running;
 
 	/*
@@ -253,7 +254,7 @@ static bool ExportInProgress = false;
 static void SnapBuildEndTxn(SnapBuild *builder, XLogRecPtr lsn, TransactionId xid);
 
 /* ->running manipulation */
-static bool SnapBuildTxnIsRunning(SnapBuild *builder, TransactionId xid);
+static bool SnapBuildTxnIsRunning(SnapBuild *builder, TransactionId xid, int *off);
 
 /* ->committed manipulation */
 static void SnapBuildPurgeCommittedTxn(SnapBuild *builder);
@@ -700,7 +701,7 @@ SnapBuildProcessChange(SnapBuild *builder, TransactionId xid, XLogRecPtr lsn)
 	 * we got into the SNAPBUILD_FULL_SNAPSHOT state.
 	 */
 	if (builder->state < SNAPBUILD_CONSISTENT &&
-		SnapBuildTxnIsRunning(builder, xid))
+		SnapBuildTxnIsRunning(builder, xid, NULL))
 		return false;
 
 	/*
@@ -776,7 +777,7 @@ SnapBuildProcessNewCid(SnapBuild *builder, TransactionId xid,
  * only exist after we freshly started from an < CONSISTENT snapshot.
  */
 static bool
-SnapBuildTxnIsRunning(SnapBuild *builder, TransactionId xid)
+SnapBuildTxnIsRunning(SnapBuild *builder, TransactionId xid, int *off)
 {
 	Assert(builder->state < SNAPBUILD_CONSISTENT);
 	Assert(TransactionIdIsNormal(builder->running.xmin));
@@ -793,8 +794,17 @@ SnapBuildTxnIsRunning(SnapBuild *builder, TransactionId xid)
 		if (search != NULL)
 		{
 			Assert(*search == xid);
+
+			if (off)
+			{
+*off = search - builder->running.xip;
+Assert(builder->running.xip[*off] == xid);
+			}
+
 			return true;
 		}
+
+
 	}
 
 	return false;
@@ -928,6 +938,8 @@ SnapBuildPurgeCommittedTxn(SnapBuild *builder)
 static void
 SnapBuildEndTxn(SnapBuild *builder, XLogRecPtr lsn, TransactionId xid)
 {
+	int off;
+
 	if (builder->state == SNAPBUILD_CONSISTENT)
 		return;
 
@@ -938,10 +950,13 @@ SnapBuildEndTxn(SnapBuild *builder, XLogRecPtr lsn, TransactionId xid)
 	 * subxids and since they end at the same time it's sufficient to deal
 	 * with them here.
 	 */
-	if (SnapBuildTxnIsRunning(builder, xid))
+	if (SnapBuildTxnIsRunning(builder, xid, ))
 	{
 		Assert(builder->running.xcnt > 0);
 
+		Assert(builder->running.xip_running[off]);
+		builder->running.xip_running[off] = false;
+
 		if (!--builder->running.xcnt)
 		{
 			/*
@@ -1250,9 +1265,15 @@ SnapBuildFindSnapshot(SnapBuild *builder, XLogRecPtr lsn, xl_running_xacts *runn
 	 *
 	 * a) There were no running transactions when the xl_running_xacts record
 	 *	  was inserted, jump to CONSISTENT immediately. We might find such a
-	 *	  state we were waiting for b) or c).
+	 *	  state while waiting for b) or c).
 	 *
-	 * b) Wait for all toplevel transactions that were running to end. We
+	 * b) This (in a 

Re: [HACKERS] pg_dump emits ALTER TABLE ONLY partitioned_table

2017-04-30 Thread Amit Langote
Hi Stephen,

Sorry about the delay.

On 2017/04/27 23:17, Stephen Frost wrote:
> Amit,
> 
> * Amit Langote (langote_amit...@lab.ntt.co.jp) wrote:
 So to summarize what the patches do (some of these were posted earlier)

 0002: pg_dump: Do not emit WITH OPTIONS keywords with partition's columns
>>>
>>> I'm trying to understand why this is also different.  At least on an
>>> initial look, there seems to be a bunch more copy-and-paste from the
>>> existing TypedTable to Partition in gram.y, which seems to all boil down
>>> to removing the need for WITH OPTIONS to be specified.  If WITH OPTIONS
>>> would be too much to have included for partitioning, and it isn't
>>> actually necessary, why not just make it optional, and use the same
>>> construct for both, removing all the duplicaiton and the need for
>>> pg_dump to output it?
>>
>> OK, I think optionally allowing WITH OPTIONS keywords would be nice.
>>
>> So in lieu of this patch, I propose a patch that modifies gram.y to allow
>> WITH OPTIONS to specified.
> 
> The point I was trying to get at was that if you make WITH OPTIONS
> optional for the TypedTableElement case, you can remove all of the
> PartitionElement-related productions and then both the OF type_name and
> the PARTITION OF case will accept WITH OPTIONS as noise words and also
> work without WITH OPTIONS being specified.
> 
> This also makes the code actually match the documentation since, at
> least in the CREATE FOREIGN TABLE documentation, we claim that WITH
> OPTIONS is required.
> 
> Please see a proposed patch attached to accomplish this.

The committed patch looks good to me.  Thanks.

Now that WITH OPTIONS is optional even for CREATE TABLE OF, perhaps it
needs to be mentioned in the release notes?

>>> Given that we're now setting numParents for partitions, I
>>> would think we'd just move the if (tbinfo->partitionOf) branches up
>>> under the if (numParents > 0) branches, which I think would also help us
>>> catch additional issues, like the fact that a binary-upgrade with
>>> partitions in a different namespace from the partitioned table would
>>> fail because the ATTACH PARTITION code doesn't account for it:
>>>
>>> appendPQExpBuffer(q, "ALTER TABLE ONLY %s ",
>>>   fmtId(tbinfo->partitionOf->dobj.name));
>>> appendPQExpBuffer(q, "ATTACH PARTITION %s %s;\n",
>>>   fmtId(tbinfo->dobj.name),
>>>   tbinfo->partitiondef);
>>>
>>> unlike the existing inheritance code a few lines above, which does:
>>>
>>> appendPQExpBuffer(q, "ALTER TABLE ONLY %s INHERIT ",
>>>   fmtId(tbinfo->dobj.name));
>>> if (parentRel->dobj.namespace != tbinfo->dobj.namespace)
>>> appendPQExpBuffer(q, "%s.",
>>> 
>>> fmtId(parentRel->dobj.namespace->dobj.name));
>>> appendPQExpBuffer(q, "%s;\n",
>>>   fmtId(parentRel->dobj.name));
>>
>> That's a good point.  I put both cases under the if (numParents > 0) block
>> and appropriate text is emitted depending on whether the table is a
>> partition or plain inheritance child.
> 
> Right, ok.
> 
 0004: Change the way pg_dump retrieves partitioning info (removes a lot
   of unnecessary code and instead makes partitioning case use the old
   inheritance code, etc.)
>>>
>>> This looks pretty good, at least on first blush, probably in part
>>> because it's mostly removing code.  The CASE statement in the
>>> getTables() query isn't needed, once pg_get_partkeydef() has been
>>> changed to not throw an ERROR when passed a non-partitioned table OID.
>>
>> Oh yes, fixed.
> 
> Good.
> 
>> 0003: Change the way pg_dump retrieves partitioning info (removes a lot
>>   of unnecessary code and instead makes partitioning case use the old
>>   inheritance code, etc.)
> 
> This has conflicts due to my proposed patch as my patch changes pg_dump
> to not output the now-noise-words WITH OPTIONS at all.

Rebased.

>> 0004: Fix a bug in pg_dump's --binary-upgrade code that caused ALTER TABLE
>>   INHERIT to be emitted to attach a partition in addition to of ALTER
>>   TABLE ATTACH PARTITION and that no schema-name was emitted where it
>>   should have
> 
> Given that it's touching the same places, this would really make sense
> to merge into 0003 now.

OK, done.

> I'm going to continue to mull over the attached for a bit and add some
> tests to it, but if it looks good then I'll push it this afternoon,
> after which you'll hopefully have time to rebase 0003 and merge in 0004
> to that, which I can review and probably push tomorrow.

Attached updated patches.

Thanks,
Amit
>From 41d8aa34c6a780fb3f46dc19f41b901e89be8726 Mon Sep 17 00:00:00 2001
From: amit 
Date: Tue, 18 Apr 2017 10:59:35 +0900
Subject: [PATCH 

Re: [HACKERS] Logical replication in the same cluster

2017-04-30 Thread Andres Freund
On 2017-04-26 23:41:51 +0200, Petr Jelinek wrote:
> Yes that's result of how logical replication slots work, the transaction
> that needs to finish is your transaction. It can be worked around by
> creating the slot manually via the SQL interface for example and create
> the subscription using WITH (NOCREATE SLOT, SLOT NAME = 'your slot') .

Is there any chance the creation of the slot could be moved ahead, to
before an xid has been assigned?

- 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] Crash when partition column specified twice

2017-04-30 Thread Amit Langote
On 2017/04/29 2:53, Robert Haas wrote:
> On Fri, Apr 28, 2017 at 7:23 AM, Beena Emerson  
> wrote:
>> Hello Amit,
>>
>> The extra n->is_from_type = false; seems to be added by mistake?
>>
>> @@ -11888,6 +11891,8 @@ TableFuncElement:   ColId Typename
>> opt_collate_clause
>> n->is_local = true;
>> n->is_not_null = false;
>> n->is_from_type = false;
>> +   n->is_from_type = false;
>> +   n->is_from_parent = false;
>> n->storage = 0;
>> n->raw_default = NULL;
>> n->cooked_default = NULL;
> 
> Good catch.  Committed after fixing that issue.

Thanks both.

Regards,
Amit



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


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

2017-04-30 Thread Noah Misch
On Sat, Apr 29, 2017 at 11:18:45AM -0700, Noah Misch wrote:
> On Tue, Apr 25, 2017 at 03:26:06PM -0400, Peter Eisentraut wrote:
> > On 4/20/17 11:30, Peter Eisentraut wrote:
> > > On 4/19/17 23:04, Noah Misch wrote:
> > >> This PostgreSQL 10 open item is past due for your status update.  Kindly 
> > >> send
> > >> a status update within 24 hours, and include a date for your subsequent 
> > >> status
> > >> update.  Refer to the policy on open item ownership:
> > >> https://www.postgresql.org/message-id/20170404140717.GA2675809%40tornado.leadboat.com
> > > 
> > > We have a possible solution but need to work out a patch.  Let's say
> > > next check-in on Monday.
> > 
> > Update: We have a patch that looks promising, but we haven't made much
> > progress in reviewing the details.  I'll work on it this week, and
> > perhaps Michael also has time to work on it this week.  We could use
> > more eyes.  Next check-in Friday.
> 
> This PostgreSQL 10 open item is past due for your status update.  Kindly send
> a status update within 24 hours, and include a date for your subsequent status
> update.  Refer to the policy on open item ownership:
> https://www.postgresql.org/message-id/20170404140717.GA2675809%40tornado.leadboat.com

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.

[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] snapbuild woes

2017-04-30 Thread Andres Freund
Hi,

On 2017-04-15 05:18:49 +0200, Petr Jelinek wrote:
> + /*
> +  * c) we already seen the xl_running_xacts and tried to do the above.
> +  * However because of race condition in LogStandbySnapshot() there might
> +  * have been transaction reported as running but in reality has already
> +  * written commit record before the xl_running_xacts so decoding has
> +  * missed it. We now see xl_running_xacts that suggests all transactions
> +  * from the original one were closed but the consistent state wasn't
> +  * reached which means the race condition has indeed happened.
> +  *
> +  * Start tracking again as if this was the first xl_running_xacts we've
> +  * seen, with the advantage that because decoding was already running,
> +  * any transactions committed before the xl_running_xacts record will be
> +  * known to us so we won't hit with the same issue again.
> +  */

Unfortunately I don't think that's true, as coded.  You're using
information about committed transactions:

> + else if (TransactionIdFollows(running->oldestRunningXid,
> +   
> builder->running.xmax))
> + {
> + int off;
> +
> + SnapBuildStartXactTracking(builder, running);
> +
>   /*
> +  * Nark any transactions that are known to have committed 
> before the
> +  * xl_running_xacts as finished to avoid the race condition in
> +  * LogStandbySnapshot().
>*
> +  * We can use SnapBuildEndTxn directly as it only does the
> +  * transaction running check and handling without any additional
> +  * side effects.
>*/
> + for (off = 0; off < builder->committed.xcnt; off++)
> + SnapBuildEndTxn(builder, lsn, 
> builder->committed.xip[off]);

but a transaction might just have *aborted* before the new snapshot, no?
Since we don't keep track of those, I don't think this guarantees anything?

ISTM, we need a xip_status array in SnapBuild->running.  Then, whenever
a xl_running_xacts is encountered while builder->running.xcnt > 0, loop
for i in 0 .. xcnt_space, and end SnapBuildEndTxn() if xip_status[i] but
the xid is not xl_running_xacts?  Does that make sense?

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

2017-04-30 Thread 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?

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

2017-04-30 Thread Andres Freund
On 2017-04-30 11:34:48 +0300, Ilya Shkuratov wrote:
> Also, I would like to remind that the disabling optimization fence is 
> suggested
> to be OPTIONAL.
> So we don't break peoples' expectations, nor documented semantics.

I think however is that that's not good enough, because it'll surprise
people forever, whereas using something as an intentional barrier
usually is, well, intentional.

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

2017-04-30 Thread David Fetter
On Sun, Apr 30, 2017 at 11:54:48PM +0200, Tomas Vondra wrote:
> On 04/30/2017 06:28 AM, Tom Lane wrote:
> > Craig Ringer  writes:
> > > - as you noted, it is hard to decide when it's worth inlining vs
> > > materializing for CTE terms referenced more than once.
> > 
> > [ raised eyebrow... ]  Please explain why the answer isn't trivially
> > "never".
> > 
> > There's already a pretty large hill to climb here in the way of
> > breaking peoples' expectations about CTEs being optimization
> > fences.  Breaking the documented semantics about CTEs being
> > single-evaluation seems to me to be an absolute non-starter.
> > 
> 
> I'm not sure that's a universal expectation, though. I know there
> are people who actually do rely on that intentionally, no doubt
> about that. And we'd nee to make it work for them.
> 
> But I keep running into people who face serious performance issues
> exactly because not realizing this, and using CTEs as named
> subqueries. And when I tell them "optimization fence" they react
> "Whaaat?"
> 
> If I had to make up some numbers, I'd say the "What?" group is
> about 10x the group of people who intentionally rely on CTEs being
> optimization fences.

I suspect you're off by at least a couple of orders of magnitude here,
which make this even more important to deal with.

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

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


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


Re: [HACKERS] CTE inlining

2017-04-30 Thread Tomas Vondra

On 04/30/2017 09:46 AM, Andres Freund wrote:

Hi,

On 2017-04-30 13:58:14 +0800, Craig Ringer wrote:

We have OFFSET 0 for anyone really depending on it, and at least when you
read that you know to go "wtf" and look at the manual, wheras the CTE fence
behaviour is invisible and silent.


I don't think that's a good idea.  What if you need to prevent inlining
of something that actually needs an offset? What if the behaviour of
offset is ever supposed to change?  Relying more on that seems to just
be repeating the mistake around CTEs.



I agree with this. But OFFSET 0 would force people to modify the queries 
anyway, so why not just introduce a new keyword instead? Something like:


WITH FENCED a (SELECT ...)

But I think something like that was proposed not too long ago, and did 
not make it for some reason.


There's a lot of other CTE improvements that would be great. Say, being 
able to define indexes on them, but that's really a separate topic.


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

2017-04-30 Thread Tomas Vondra

On 04/30/2017 06:28 AM, Tom Lane wrote:

Craig Ringer  writes:

- as you noted, it is hard to decide when it's worth inlining vs
materializing for CTE terms referenced more than once.


[ raised eyebrow... ]  Please explain why the answer isn't trivially
"never".

There's already a pretty large hill to climb here in the way of
breaking peoples' expectations about CTEs being optimization
fences.  Breaking the documented semantics about CTEs being
single-evaluation seems to me to be an absolute non-starter.



I'm not sure that's a universal expectation, though. I know there are 
people who actually do rely on that intentionally, no doubt about that. 
And we'd nee to make it work for them.


But I keep running into people who face serious performance issues 
exactly because not realizing this, and using CTEs as named subqueries. 
And when I tell them "optimization fence" they react "Whaaat?"


If I had to make up some numbers, I'd say the "What?" group is about 
10x the group of people who intentionally rely on CTEs being 
optimization fences.


FWIW I don't know how to do this. There were multiple attempts at this 
in the past, none of them succeeded. But perhaps we could at least 
propagate some of the CTE features, so that the outside query can 
benefit from that (e.g. when the CTE is sorted, we could set the 
sortkeys). That wouldn't break the fence thing, but it would allow other 
stuff.


regard

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

2017-04-30 Thread Sven R. Kunze

On 27.04.2017 22:21, Robert Haas wrote:

On Thu, Apr 27, 2017 at 3:15 PM, Sven R. Kunze  wrote:

Just to make sound a little rounder:

CREATE TABLE ... PARTITION OF ... AS DEFAULT
CREATE TABLE ... PARTITION OF ... AS FALLBACK

or

CREATE TABLE ... PARTITION OF ... AS DEFAULT PARTITION
CREATE TABLE ... PARTITION OF ... AS FALLBACK PARTITION

Could any of these be feasible?

FALLBACK wouldn't be a good choice because it's not an existing parser
keyword.  We could probably insert AS before DEFAULT and/or PARTITION
afterwards, but they sort of seem like noise words.


You are right. I just thought it would make this variant more acceptable 
as people expressed concerns about understandability of the command.



SQL seems to have
been invented by people who didn't have any trouble remembering really
long command strings, but brevity is not without some merit.


For me, it's exactly the thing I like about SQL. It makes for an easy 
learning curve.



Sven


Re: [HACKERS] .pgpass's behavior has changed

2017-04-30 Thread Noah Misch
On Fri, Apr 28, 2017 at 04:54:32PM +0900, Kyotaro HORIGUCHI wrote:
> I noticed that the precedence between host and hostaddr in a
> connection string is reversed in regard to .pgpass lookup in
> devel.
> 
> For example the following connection string uses a .pgpass entry
> with "127.0.0.1", not "hoge".
> 
> "host=hoge hostaddr=127.0.0.1 port=5432 dbname=postgres"
> 
> 
> This change was introdueced by the commit
> 274bb2b3857cc987cfa21d14775cae9b0dababa5 and the current behavior
> contradicts the documentation.
> 
> https://www.postgresql.org/docs/devel/static/libpq-connect.html
> 
> > hostaddr
> > ...
> >   Note that authentication is likely to fail if host is not the
> >   name of the server at network address hostaddr. Also, note that
> >   host rather than hostaddr is used to identify the connection in
> >   a password file (see Section 33.15, “The Password File”).
> 
> I think this should be fixed for the same reason with the
> following commit.
> 
> > commit 11003eb55658df0caf183eef69c7a97d56a4f2d7
> > Author: Robert Haas 
> > Date:   Thu Dec 1 14:36:39 2016 -0500
> > 
> > libpq: Fix inadvertent change in PQhost() behavior.
> 
> But the above also leaves a bug so I sent another patch to fix
> it. The attched patch restores the 9.6's beavior of looking up
> .pgpass file in the same manner to the aother patch.

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

The above-described topic is currently a PostgreSQL 10 open item.  Robert,
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] Concurrent ALTER SEQUENCE RESTART Regression

2017-04-30 Thread Noah Misch
On Fri, Apr 28, 2017 at 04:55:45PM +0900, Michael Paquier wrote:
> On Thu, Apr 27, 2017 at 4:10 PM, Andres Freund  wrote:
> > On April 27, 2017 12:06:55 AM PDT, Michael Paquier 
> >  wrote:
> >>On Thu, Apr 27, 2017 at 3:23 PM, Andres Freund 
> >>wrote:
> >>> More fun:
> >>>
> >>> A: CREATE SEQUENCE someseq;
> >>> A: BEGIN;
> >>> A: ALTER SEQUENCE someseq MAXVALUE 10;
> >>> B: SELECT nextval('someseq') FROM generate_series(1, 1000);
> >>>
> >>> => ignores maxvalue
> >>
> >>Well, for this one that's because the catalog change is
> >>transactional...
> >
> > Or because the locking model is borked.
> 
> The operation actually relies heavily on the fact that the exclusive
> lock on the buffer of pg_sequence is hold until the end of the catalog
> update. And using heap_inplace_update() seems mandatory to me as the
> metadata update should be non-transactional, giving the attached. I
> have added some isolation tests. Thoughts? The attached makes HEAD map
> with the pre-9.6 behavior.

[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] Declarative partitioning - another take

2017-04-30 Thread Noah Misch
On Mon, Apr 24, 2017 at 07:43:29PM +0900, Amit Langote wrote:
> On 2017/04/21 17:00, Rajkumar Raghuwanshi wrote:
> > I am able to create statement triggers at root partition, but these
> > triggers, not getting fired on updating partition.

> It would be great if you could check if the patches fix the issues.
> 
> Also, adding this to the PostgreSQL 10 open items list.

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

The above-described topic is currently a PostgreSQL 10 open item.  Robert,
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] On How To Shorten the Steep Learning Curve Towards PG Hacking...

2017-04-30 Thread Andrew Borodin
Hi, Kang and everyone in this thread.

I'm planning to present the online course "Hacking PostgreSQL: data
access methods in action and under the hood" on edX on June 1st. It's
not announced yet, links will be available later.
This course I'm describing information that was crucial for me to
start hacking. Currently, my knowledge of technologies behind Postgres
is quite limited, though I know the border of my knowledge quite well.

Chances are that description from my point of view will not be helpful
in some cases: before starting contributing to Postgres I had already
held PhD in CS for database technology and I had already implemented 3
different commercial DBMS (all in different technologies, PLs,
paradigms, focuses, different prbolems being solved). And still,
production of minimally viable contribution took 3 months (I was
hacking for an hour a day, mostly at evenings).
That's why I decided that it worth talking about how to get there
before I'm already there. It's quite easy to forget that some concepts
are really hard before you get them.

The course will cover:
1. Major differences of Postgres from others
2. Dev tools as I use them
3. Concept of paged memory, latches and paged data structures
4. WAL, recovery, replication
5. Concurrency and locking in B-trees
6. GiST internals
7. Extensions
8. Text search and some of GIN
9. Postgres community mechanics
Every topic will consist of two parts: 1 - video lectures on YouTube
(in English and Russian, BTW my English is far from perfect) with
references to docs and other resources, 2 - practical tasks where you
change code slightly and observe differences (this part is mostly to
help the student to observe easy entry points).

Best regards, Andrey Borodin, Octonica.


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

2017-04-30 Thread Andres Freund
Hi,

On 2017-04-30 13:58:14 +0800, Craig Ringer wrote:
> We have OFFSET 0 for anyone really depending on it, and at least when you
> read that you know to go "wtf" and look at the manual, wheras the CTE fence
> behaviour is invisible and silent.

I don't think that's a good idea.  What if you need to prevent inlining
of something that actually needs an offset? What if the behaviour of
offset is ever supposed to change?  Relying more on that seems to just
be repeating the mistake around CTEs.


> Like the work Andes has been doing on our bizarre handing of SRFs in the
> SELECT target list I really think it's just something that needs to be
> done.

With help from Tom, luckily...

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