Re: [HACKERS] Foreign Join pushdowns not working properly for outer joins

[David Rowley]

On 6 March 2017 at 18:51, Etsuro Fujita  wrote:
> On 2017/03/06 11:05, David Rowley wrote:
>> The attached patch, based on 9.6,  fixes the problem by properly
>> processing the foreign server options in
>> postgresGetForeignJoinPaths().
>
> I think the fix would work well, but another way I think is much simpler and
> more consistent with the existing code is to (1) move code for getting the
> server info from the outer's fpinfo before calling is_foreign_expr() in
> foreign_join_ok() and (2) add code for getting the shippable extensions info
> from the outer's fpinfo before calling that function, like the attached.

Thanks for looking over my patch.

I looked over yours and was surprised to see:

+ /* is_foreign_expr might need server and shippable-extensions info. */
+ fpinfo->server = fpinfo_o->server;
+ fpinfo->shippable_extensions = fpinfo_o->shippable_extensions;

That appears to do nothing for the other server options. For example
use_remote_estimate, which is used in estimate_path_cost_size(). As of
now, that's also broken. My patch fixes that problem too, yours
appears not to.

Even if you expanded the above code to process all server options, if
someone comes along later and adds a new option, my code will pick it
up, yours could very easily be forgotten about and be the cause of yet
more bugs.

It seems like a much better idea to keep the server option processing
in one location, which is what I did.

Do you think differently?

-- 
 David Rowley   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] Partitioned tables and relfilenode

[Amit Langote]

On 2017/03/06 16:49, Ashutosh Bapat wrote:
> On Mon, Mar 6, 2017 at 12:48 PM, Amit Langote wrote:
>> On 2017/03/06 15:41, Michael Paquier wrote:
>>> This comment is not completely correct. Children can be temp tables,
>>> they just cannot be temp tables of other backends. It seems to me that
>>> you could still keep this code simple and remove has_child..
>>
>> I updated the comment.  I recall having posted a patch for that once, but
>> perhaps went unnoticed.
> 
> The existing comment only specifies "temp tables" and not "temp table
> of other backends". The new comment keeps that part same and adds
> partitioned table case. So, I don't see any reason to change the "temp
> tables" to "temp table of other backends" in this patch.

Hmm.  A separate patch might be fine but why not fix the incorrect part
while we are updating the whole comment anyway.

Thanks,
Amit




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

Re: [HACKERS] Partitioned tables and relfilenode

[Ashutosh Bapat]

On Mon, Mar 6, 2017 at 12:48 PM, Amit Langote
 wrote:
> Thanks for the review.
>
> On 2017/03/06 15:41, Michael Paquier wrote:
>> On Fri, Mar 3, 2017 at 10:02 AM, Amit Langote
>>  wrote:
>>> Thanks.  I noticed that 'and' is duplicated in a line added by the commit
>>> to analyze.sgml.  Attached 0001 fixes that.  0002 and 0003 same as the
>>> last version.
>>
>> /*
>> -* If all the children were temp tables, pretend it's a non-inheritance
>> -* situation.  The duplicate RTE we added for the parent table is
>> -* harmless, so we don't bother to get rid of it; ditto for the useless
>> -* PlanRowMark node.
>> +* If all the children were temp tables or if the parent is a partitioned
>> +* table without any leaf partitions, pretend it's a non-inheritance
>> +* situation.  The duplicate RTE for the parent table we added in the
>> +* non-partitioned table case is harmless, so we don't bother to get rid
>> +* of it; ditto for the useless PlanRowMark node.
>>  */
>> -   if (list_length(appinfos) < 2)
>> +   if (!has_child)
>> This comment is not completely correct. Children can be temp tables,
>> they just cannot be temp tables of other backends. It seems to me that
>> you could still keep this code simple and remove has_child..
>
> I updated the comment.  I recall having posted a patch for that once, but
> perhaps went unnoticed.

The existing comment only specifies "temp tables" and not "temp table
of other backends". The new comment keeps that part same and adds
partitioned table case. So, I don't see any reason to change the "temp
tables" to "temp table of other backends" in this patch.

>
> About has_child, the other option is to make the minimum length of
> appinfos list relkind-based, but  the condition quickly becomes ugly. Do
> you have a suggestion?
>
+1.


-- 
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database 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] [GSoC] Self introduction and question to mentors

[Кирилл Бороздин]

Hello!

My name is Kirill Borozdin. I am a student of Ural Federal University from
Yekaterinburg, Russia.
I was trying to find some interesting algorithmic problem for GSoC and
finally ran into PostgreSQL and
"Sorting algorithms benchmark and implementation" task.
This problem looks both challenging and practical. I have the strong
background in algorithmic
problems (I like programming contests and passed to the ACM-ICPC World
Finals this year).
Also I do some CS research (my paper is waiting for review on Combinatorial
Pattern Matching 2017 conference).
I understand that this task will require a big number of experiments
because we want to achieve
a speed-up in real life cases.

Can you recommend me some articles or specific source files in PostgreSQL
codebase (it is so huge!)
which I can explore to be better prepared (in case I will be lucky enough
to be selected for GSoC)?

Best wishes,
Kirill Borozdin

-- 
С уважением, Кирилл Бороздин

Re: [HACKERS] Partitioned tables and relfilenode

[Amit Langote]

Thanks for the review.

On 2017/03/06 15:41, Michael Paquier wrote:
> On Fri, Mar 3, 2017 at 10:02 AM, Amit Langote
>  wrote:
>> Thanks.  I noticed that 'and' is duplicated in a line added by the commit
>> to analyze.sgml.  Attached 0001 fixes that.  0002 and 0003 same as the
>> last version.
> 
> /*
> -* If all the children were temp tables, pretend it's a non-inheritance
> -* situation.  The duplicate RTE we added for the parent table is
> -* harmless, so we don't bother to get rid of it; ditto for the useless
> -* PlanRowMark node.
> +* If all the children were temp tables or if the parent is a partitioned
> +* table without any leaf partitions, pretend it's a non-inheritance
> +* situation.  The duplicate RTE for the parent table we added in the
> +* non-partitioned table case is harmless, so we don't bother to get rid
> +* of it; ditto for the useless PlanRowMark node.
>  */
> -   if (list_length(appinfos) < 2)
> +   if (!has_child)
> This comment is not completely correct. Children can be temp tables,
> they just cannot be temp tables of other backends. It seems to me that
> you could still keep this code simple and remove has_child..

I updated the comment.  I recall having posted a patch for that once, but
perhaps went unnoticed.

About has_child, the other option is to make the minimum length of
appinfos list relkind-based, but  the condition quickly becomes ugly. Do
you have a suggestion?

> @@ -932,7 +932,6 @@ extractRelOptions(HeapTuple tuple, TupleDesc tupdesc,
> case RELKIND_RELATION:
> case RELKIND_TOASTVALUE:
> case RELKIND_MATVIEW:
> -   case RELKIND_PARTITIONED_TABLE:
> options = heap_reloptions(classForm->relkind, datum, false);
> break;
> Partitioned tables cannot have reloptions? What about all the
> autovacuum_* parameters then? Those are mainly not storage-related.

AFAIK, none of the heap reloptions will be applicable to partitioned table
relations once we eliminate storage.

About autovacuum_* parameters - we currently don't handle partitioned
tables in autovacuum.c, because no statistics are reported for them. That
is, relation_needs_vacanalyze() will never return true for dovacuum,
doanalyze and wraparound if it is passed a RELKIND_PARTITIONED_TABLE
relation.  That's something to be fixed separately though.  When we add
autovacuum support for partitioned tables, we may want to add a new set of
reloptions (new because partitioned tables still won't support all options
returned by heap_reloptions()).  Am I missing something?

Thanks,
Amit
>From 7cea5e205742c14893a8f52f213118b7de091062 Mon Sep 17 00:00:00 2001
From: amit 
Date: Fri, 3 Mar 2017 09:57:11 +0900
Subject: [PATCH 1/3] Fix duplicated 'and' in analyze.sgml

---
 doc/src/sgml/ref/analyze.sgml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/doc/src/sgml/ref/analyze.sgml b/doc/src/sgml/ref/analyze.sgml
index 49727e22df..45dee101df 100644
--- a/doc/src/sgml/ref/analyze.sgml
+++ b/doc/src/sgml/ref/analyze.sgml
@@ -64,7 +64,7 @@ ANALYZE [ VERBOSE ] [ table_name [
  
   The name (possibly schema-qualified) of a specific table to
   analyze.  If omitted, all regular tables, partitioned tables, and
-  and materialized views in the current database are analyzed (but not
+  materialized views in the current database are analyzed (but not
   foreign tables).  If the specified table is a partitioned table, both the
   inheritance statistics of the partitioned table as a whole and
   statistics of the individual partitions are updated.
-- 
2.11.0

>From 8ad7b95c56cd261b4581447aed1f3a6519b8ae06 Mon Sep 17 00:00:00 2001
From: amit 
Date: Mon, 6 Feb 2017 17:26:48 +0900
Subject: [PATCH 2/3] Avoid creating scan nodes for partitioned tables

* Currently, we create scan nodes for inheritance parents in their role
  as an inheritance set member.  Partitioned tables do not contain any
  data, so it's useless to create scan nodes for them.  So we need not
  create AppendRelInfo's for them in the planner prep phase.

* The planner prep phase turns off inheritance on the parent RTE if
  there isn't at least one child member (other than the parent itself
  which in normal cases is also a child member), which means the latter
  phases will not consider creating an Append plan and instead create a
  scan node. Avoid this if the RTE is a partitioned table, by noticing
  such cases in set_rel_size().  Per suggestion from Ashutosh Bapat.

* Since we do not add the RTE corresponding to the root partitioned
  table as the 1st child member of the inheritance set,
  inheritance_planner() must not assume the same when assigning
  nominalRelation to a ModifyTable node.

* In ExecInitModifyTable(), use nominalRelation to initialize tuple
  routing information, instead of the first resultRelInfo.  We set
  ModifyTable.nominalRelation to 

Re: [HACKERS] [BUG FIX] Removing NamedLWLockTrancheArray

[Kyotaro HORIGUCHI]

At Mon, 06 Mar 2017 15:44:52 +0900 (Tokyo Standard Time), Kyotaro HORIGUCHI 
 wrote in 
<20170306.154452.254472341.horiguchi.kyot...@lab.ntt.co.jp>
> Hello,
> 
> At Sat, 4 Mar 2017 10:07:50 +0530, Amit Kapila  
> wrote in 
> > On Fri, Mar 3, 2017 at 3:49 PM, Kyotaro HORIGUCHI
> >  wrote:
> > >> You can read about usage of LWLocks in extensions from below location:
> > >> https://www.postgresql.org/docs/devel/static/xfunc-c.html#idp86986416
> > >
> > > Thank you for the pointer. I understand that the document describes the 
> > > only
> > > correct way to use LWLock in extensions  and using LWLockRegisterTranche  
> > > is
> > > a non-standard or prohibit way to do that.
> > >
> > > By the way, in the case of orafce, it uses LWLockRegisterTranche  directly
> > > but only when !found. So if any backend other than the creator of the 
> > > shmem
> > > want to access tranche, the puch tranche is not found on the process and
> > > crashes. I think this is it.
> > >
> 
> (I can't recall what the 'the puch' was...)
> 
> > Yeah and I think that is expected if you use LWLockRegisterTranche.
> > You might want to read comments in lwlock.h to know the reason behind
> > the same.
> 
> Ah, yes. I understood that. And even if a module prudently
> registers its own tranche, it would be easily broken by some
> other modules' omission to call LWLockNewTransactionId() for all
> backends. As the result extension modules should not go that way.

It's wrong. The module knows the allocated tranche id so it can
register the tranche properly.

> > > If no other modules is installed, registeriing a tranche even if found 
> > > will
> > > supress the crash but it is not a solution at all.
> > >
> > > At least for 9.6 or 10, orafce should do that following the documentation.
> > >
> > 
> > Agreed.
> > 
> > > But it still can crash from the problem by the separate
> > > NamedLWLockTrancheArray. (ID range check in LWLockInitialize would be
> > > useless if it is not used by extensions)
> > >
> > 
> > What exact problem are you referring here?
> 
> At many places in lwlock.c, especially on TRACE_POSTGRESQ_*()
> macros, T_NAME(lock) is used and the macro just uses tranche id
> as index of the main tranche array (LWLockTrancheArray). On the
> other hand it is beyond the end of of the array for the "named
> tranche"s and can raise SEGV.
> 
> 
> I can guess two ways to fix this. One is change the definition of
> T_NAME.
> 
> | #define T_NAME(l) \
> |   ((l)->tranche < LWTRANCHE_FIRST_USER_DEFINED ? \
> |LWLockTrancheArray[(l)->tranche] : \
> |NamedLWLockTrancheArray[(l)->tranche - LWTRANCHE_FIRST_USER_DEFINED]
> 
> It makes the patch small but I don't thing the shape is
> desirable.
> 
> Then, the other way is registering named tranches into the main
> tranche array. The number and names of the requested named
> tranches are known to postmaster so they can be allocated and
> initialized at the time.
> 
> The mapping of the shared memory is inherited to backends so
> pointing to the addresses in shared memory will work in the
> !EXEC_BACKEND case. I confirmed that the behavior is ensured also
> in EXEC_BACKEND case.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center




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

Re: [HACKERS] ANALYZE command progress checker

[Andres Freund]

On 2017-03-03 15:33:15 -0500, David Steele wrote:
> On 3/1/17 1:25 PM, Andres Freund wrote:
> > On 2017-03-01 10:20:41 -0800, David Fetter wrote:
> >> On Wed, Mar 01, 2017 at 09:45:40AM -0500, Peter Eisentraut wrote:
> >>> On 2/28/17 04:24, vinayak wrote:
>  The view provides the information of analyze command progress details as 
>  follows
>  postgres=# \d pg_stat_progress_analyze
> View "pg_catalog.pg_stat_progress_analyze"
> >>>
> >>> Hmm, do we want a separate "progress" system view for every kind of
> >>> command?  What if someone comes up with a progress checker for CREATE
> >>> INDEX, REINDEX, CLUSTER, etc.?
> > 
> > I don't think that'd be that bad, otherwise the naming of the fields is
> > complicated.  I guess the alternative (or do both?) would be to to have
> > a pivoted table, but that'd painful to query.  Do you have a better idea?
> > 
> > 
> >> Some kind of design for progress seems like a good plan.  Some ideas:
> >>
> >> - System view(s)
> >>
> >> This has the advantage of being shown to work at least to a PoC by
> >> this patch, and is similar to extant system views like
> >> pg_stat_activity in the sense of capturing a moment in time.
> >>
> >> - NOTIFY
> >>
> >> Event-driven model as opposed to a polling one.  This is
> >> attractive on efficiency grounds, less so on reliability ones.
> >>
> >> - Something added to the wire protocol
> >>
> >> More specialized, limits the information to the session where the
> >> command was issued
> >>
> >> - Other things not named here
> > 
> > We now have a framework for this [1] (currently used by vacuum, but
> > extensible). The question is about presentation.  I'm fairly sure that
> > we shouldn't just add yet another framework, and I doubt that that's
> > what's proposed by Peter.
> 
> I think the idea of a general progress view is very valuable and there
> are a ton of operations it could be used for:  full table scans, index
> rebuilds, vacuum, copy, etc.
> However, I feel that this proposal is not flexible enough and comes too
> late in the release cycle to allow development into something that could
> be committed.

I'm not following. As I pointed out, we already have this framework?
This patch is just a short one using that framework?


> I propose we move this to the 2017-07 CF so the idea can be more fully
> developed.

I don't see that being warranted in this case, we're really not talking
about something complicated:
 doc/src/sgml/monitoring.sgml |  135 +++
 src/backend/catalog/system_views.sql |   16 
 src/backend/commands/analyze.c   |   34 
 src/backend/utils/adt/pgstatfuncs.c  |2 
 src/include/commands/progress.h  |   13 +++
 src/include/pgstat.h |3 
 src/test/regress/expected/rules.out  |   18 
 7 files changed, 219 insertions(+), 2 deletions(-)
excepting tests and docs, this is very little actual code.

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] Performance degradation in TPC-H Q18

[Kuntal Ghosh]

On Sat, Mar 4, 2017 at 11:09 AM, Robert Haas  wrote:
> On Sat, Mar 4, 2017 at 5:56 AM, Andres Freund  wrote:
>> attached is a patch to address this problem, and the one reported by
>> Dilip.  I ran a lot of TPC-H and other benchmarks, and so far this
>> addresses all the performance issues, often being noticeably faster than
>> with the dynahash code.
>>
>> Comments?
+errdetail("size %f/members %f: factor %.2f",
+  (double)tb->size, (double)tb->members,
+  (double) tb->members / tb->size),
In SH_STAT, we use 'filled' instead of 'factor'. For displaying size
and members, there is no need to convert those into double.


> I'm still not convinced that raising the fillfactor like this is going
> to hold up in testing, but I don't mind you committing it and we'll
> see what happens.  If it is possible to survive with a fillfactor that
> high, it certainly has some advantages, and it's obviously more likely
> to work out with the other logic you've added.  I think we need a lot
> more people playing with this to know whether any given approach is
> right, and I think getting something committed will help more than any
> amount of theoretical discussion.
+1

> I think DEBUG1 is far too high for something that could occur with
> some frequency on a busy system; I'm fairly strongly of the opinion
> that you ought to downgrade that by a couple of levels, say to DEBUG3
> or so.
+1

I've tested with TPC-H query 18 and it's working fine.

-- 
Thanks & Regards,
Kuntal Ghosh
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] ANALYZE command progress checker

[Michael Paquier]

On Sat, Mar 4, 2017 at 5:33 AM, David Steele  wrote:
> I think the idea of a general progress view is very valuable and there
> are a ton of operations it could be used for:  full table scans, index
> rebuilds, vacuum, copy, etc.
>
> However, I feel that this proposal is not flexible enough and comes too
> late in the release cycle to allow development into something that could
> be committed.

Well, each command really has its own requirements in terms of data to
store, so we either finish with a bunch of small tables that anyone
could query and join as they wish or a somewhat unique table that is
bloated with all the information, with a set of views on top of it to
query all the information. For extensibility's sake of each command
(for example imagine that REINDEX could be extended with a
CONCURRENTLY option and multiple phases), I would think that having a
table per command type would not be that bad.
-- 
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] [BUG FIX] Removing NamedLWLockTrancheArray

[Kyotaro HORIGUCHI]

Hello,

At Sat, 4 Mar 2017 10:07:50 +0530, Amit Kapila  wrote 
in 
> On Fri, Mar 3, 2017 at 3:49 PM, Kyotaro HORIGUCHI
>  wrote:
> >> You can read about usage of LWLocks in extensions from below location:
> >> https://www.postgresql.org/docs/devel/static/xfunc-c.html#idp86986416
> >
> > Thank you for the pointer. I understand that the document describes the only
> > correct way to use LWLock in extensions  and using LWLockRegisterTranche  is
> > a non-standard or prohibit way to do that.
> >
> > By the way, in the case of orafce, it uses LWLockRegisterTranche  directly
> > but only when !found. So if any backend other than the creator of the shmem
> > want to access tranche, the puch tranche is not found on the process and
> > crashes. I think this is it.
> >

(I can't recall what the 'the puch' was...)

> Yeah and I think that is expected if you use LWLockRegisterTranche.
> You might want to read comments in lwlock.h to know the reason behind
> the same.

Ah, yes. I understood that. And even if a module prudently
registers its own tranche, it would be easily broken by some
other modules' omission to call LWLockNewTransactionId() for all
backends. As the result extension modules should not go that way.

> > If no other modules is installed, registeriing a tranche even if found will
> > supress the crash but it is not a solution at all.
> >
> > At least for 9.6 or 10, orafce should do that following the documentation.
> >
> 
> Agreed.
> 
> > But it still can crash from the problem by the separate
> > NamedLWLockTrancheArray. (ID range check in LWLockInitialize would be
> > useless if it is not used by extensions)
> >
> 
> What exact problem are you referring here?

At many places in lwlock.c, especially on TRACE_POSTGRESQ_*()
macros, T_NAME(lock) is used and the macro just uses tranche id
as index of the main tranche array (LWLockTrancheArray). On the
other hand it is beyond the end of of the array for the "named
tranche"s and can raise SEGV.


I can guess two ways to fix this. One is change the definition of
T_NAME.

| #define T_NAME(l) \
|   ((l)->tranche < LWTRANCHE_FIRST_USER_DEFINED ? \
|LWLockTrancheArray[(l)->tranche] : \
|NamedLWLockTrancheArray[(l)->tranche - LWTRANCHE_FIRST_USER_DEFINED]

It makes the patch small but I don't thing the shape is
desirable.

Then, the other way is registering named tranches into the main
tranche array. The number and names of the requested named
tranches are known to postmaster so they can be allocated and
initialized at the time.

The mapping of the shared memory is inherited to backends so
pointing to the addresses in shared memory will work in the
!EXEC_BACKEND case. I confirmed that the behavior is ensured also
in EXEC_BACKEND case.

regards,

-- 
Kyotaro Horiguchi
NTT Open Source Software Center




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

Re: [HACKERS] Partitioned tables and relfilenode

[Michael Paquier]

On Fri, Mar 3, 2017 at 10:02 AM, Amit Langote
 wrote:
> Thanks.  I noticed that 'and' is duplicated in a line added by the commit
> to analyze.sgml.  Attached 0001 fixes that.  0002 and 0003 same as the
> last version.

/*
-* If all the children were temp tables, pretend it's a non-inheritance
-* situation.  The duplicate RTE we added for the parent table is
-* harmless, so we don't bother to get rid of it; ditto for the useless
-* PlanRowMark node.
+* If all the children were temp tables or if the parent is a partitioned
+* table without any leaf partitions, pretend it's a non-inheritance
+* situation.  The duplicate RTE for the parent table we added in the
+* non-partitioned table case is harmless, so we don't bother to get rid
+* of it; ditto for the useless PlanRowMark node.
 */
-   if (list_length(appinfos) < 2)
+   if (!has_child)
This comment is not completely correct. Children can be temp tables,
they just cannot be temp tables of other backends. It seems to me that
you could still keep this code simple and remove has_child..

@@ -932,7 +932,6 @@ extractRelOptions(HeapTuple tuple, TupleDesc tupdesc,
case RELKIND_RELATION:
case RELKIND_TOASTVALUE:
case RELKIND_MATVIEW:
-   case RELKIND_PARTITIONED_TABLE:
options = heap_reloptions(classForm->relkind, datum, false);
break;
Partitioned tables cannot have reloptions? What about all the
autovacuum_* parameters then? Those are mainly not storage-related.
-- 
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] TPC-H Q20 from 1 hour to 19 hours!

[Rafia Sabih]

Hello all,

This is to bring to notice a peculiar instance I found recently while
running TPC-H benchmark queries. Q20 of the benchmark took 19 hours to
complete when run on a machine with 512 GB RAM and 32 cores with
following parameter settings on the commit id -
0c2070cefa0e5d097b715c9a3b9b5499470019aa

work_mem = 1 GB
shared_buffers = 8 GB
effective_cache_size = 10 GB
scale factor = 300
max_parallel_workers_per_gather = 4

The output of explain_analyse is as follows,
QUERY PLAN
-
 Limit  (cost=60187550.59..60187550.59 rows=1 width=52) (actual
time=71064602.963..71064602.964 rows=1 loops=1)
   ->  Sort  (cost=60187550.59..60187550.59 rows=3 width=52) (actual
time=71064602.961..71064602.961 rows=1 loops=1)
 Sort Key: supplier.s_name
 Sort Method: top-N heapsort  Memory: 25kB
 ->  Nested Loop Semi Join  (cost=52960550.15..60187550.57
rows=3 width=52) (actual time=2163639.699..71063153.953 rows=52263
loops=1)
   Join Filter: (supplier.s_suppkey = lineitem.l_suppkey)
   Rows Removed by Join Filter: 155706242106
   ->  Nested Loop  (cost=963.43..10081.54 rows=12
width=56) (actual time=178.589..6282.852 rows=120358 loops=1)
 ->  Seq Scan on nation  (cost=0.00..0.41 rows=1
width=4) (actual time=0.018..0.042 rows=1 loops=1)
   Filter: (n_name = 'EGYPT'::bpchar)
   Rows Removed by Filter: 24
 ->  Bitmap Heap Scan on supplier
(cost=963.43..8881.13 rows=12 width=64) (actual
time=178.563..6143.786 rows=120358 loops=1)
   Recheck Cond: (s_nationkey = nation.n_nationkey)
   Heap Blocks: exact=57317
   ->  Bitmap Index Scan on
idx_supplier_nation_key  (cost=0.00..933.43 rows=12 width=0)
(actual time=133.218..133.218 rows=120358 loops=1)
 Index Cond: (s_nationkey = nation.n_nationkey)
   ->  Materialize  (cost=52959586.72..60024469.24 rows=85
width=16) (actual time=12.679..175.456 rows=1293693 loops=120358)
 ->  Merge Join  (cost=52959586.72..60024468.82
rows=85 width=16) (actual time=1525322.753..2419045.641 rows=1696742
loops=1)
   Merge Cond: ((lineitem.l_partkey =
partsupp.ps_partkey) AND (lineitem.l_suppkey = partsupp.ps_suppkey))
   Join Filter:
((partsupp.ps_availqty)::numeric > ((0.5 * sum(lineitem.l_quantity
   Rows Removed by Join Filter: 3771
   ->  GroupAggregate
(cost=48448912.90..53325163.98 rows=144696357 width=48) (actual
time=1342085.116..2178225.340 rows=156665300 loops=1)
 Group Key: lineitem.l_partkey,
lineitem.l_suppkey
 ->  Sort
(cost=48448912.90..49125364.33 rows=270580572 width=21) (actual
time=1342085.072..1495863.254 rows=273057944 loops=1)
   Sort Key: lineitem.l_partkey,
lineitem.l_suppkey
   Sort Method: external merge
Disk: 8282600kB
   ->  Bitmap Heap Scan on
lineitem  (cost=2847641.84..10552097.42 rows=270580572 width=21)
(actual time=241170.637..650122.225 rows=273058377 loops=1)
 Recheck Cond:
((l_shipdate >= '1994-01-01'::date) AND (l_shipdate < '1995-01-01
00:00:00'::timestamp without time zone))
 Heap Blocks: exact=3333
 ->  Bitmap Index Scan on
idx_l_shipdate  (cost=0.00..2779996.70 rows=270580572 width=0) (actual
time=190321.155..190321.155 rows=273058377 loops=1)
   Index Cond:
((l_shipdate >= '1994-01-01'::date) AND (l_shipdate < '1995-01-01
00:00:00'::timestamp without time zone))
   ->  Sort  (cost=4510673.81..4516734.42
rows=2424244 width=24) (actual time=183237.183..184039.914
rows=2602656 loops=1)
 Sort Key: partsupp.ps_partkey,
partsupp.ps_suppkey
 Sort Method: quicksort  Memory: 301637kB
 ->  Hash Join
(cost=379620.36..4253593.60 rows=2424244 width=24) (actual
time=8576.343..179703.563 rows=2602656 loops=1)
   Hash Cond: (partsupp.ps_partkey
= part.p_partkey)
   ->  Seq Scan on partsupp
(cost=0.00..2949730.80 rows=24000 width=20) (actual
time=0.149..71902.279 rows=24000 loops=1)
   ->  Hash
(cost=372044.59..372044.59 rows=606062 width=4) (actual
time=8574.476..8574.476 rows=650664 loops=1)
 

Re: [HACKERS] Foreign Join pushdowns not working properly for outer joins

[Etsuro Fujita]

On 2017/03/06 11:05, David Rowley wrote:

I've been asked to investigate a case of a foreign join not occurring
on the foreign server as would have been expected.



The attached patch, based on 9.6,  fixes the problem by properly
processing the foreign server options in
postgresGetForeignJoinPaths().

I ended up shifting the code which does this into functions to allow
it to be reused. I also ended up shifting out the code which processes
the table options so that it is consistent.

Reviews from people a bit closer to the foreign join pushdown code are welcome.


Thanks for working on this!

I think the fix would work well, but another way I think is much simpler 
and more consistent with the existing code is to (1) move code for 
getting the server info from the outer's fpinfo before calling 
is_foreign_expr() in foreign_join_ok() and (2) add code for getting the 
shippable extensions info from the outer's fpinfo before calling that 
function, like the attached.


Best regards,
Etsuro Fujita


foreign_outerjoin_pushdown_fix_efujita.patch
Description: binary/octet-stream

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

Re: [HACKERS] dropping partitioned tables without CASCADE

[Amit Langote]

On 2017/03/06 14:25, Simon Riggs wrote:
> On 6 March 2017 at 04:00, Ashutosh Bapat wrote:
>> Thinking about how to display partition which are further partitioned,
>> there are two options. Assume a partitioned table t1 with partitions
>> t1p1, which is further partitioned and t1p2. One could display \d+ t1
>> as
>>
>> \d+ t1
>> Table "public.t1"
>>  Column |  Type   | Collation | Nullable | Default | Storage | Stats
>> target | Description
>> +-+---+--+-+-+--+-
>>  a  | integer |   | not null | | plain   |  |
>> Partition key: RANGE (a)
>> Partitions: t1p1 FOR VALUES FROM (0) TO (100), HAS PARTITIONS
>> t1p2 FOR VALUES FROM (100) TO (200)
>>
>> OR
>>
>> \d+ t1
>> Table "public.t1"
>>  Column |  Type   | Collation | Nullable | Default | Storage | Stats
>> target | Description
>> +-+---+--+-+-+--+-
>>  a  | integer |   | not null | | plain   |  |
>> Partition key: RANGE (a)
>> Partitions: t1p1 FOR VALUES FROM (0) TO (100), PARTITION BY LIST(a)
>> t1p2 FOR VALUES FROM (100) TO (200)
>>
>> To me the first option looks fine.
> 
> +1
> 
> lowercase please

+1

Thanks,
Amit




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

Re: [HACKERS] dropping partitioned tables without CASCADE

[Ashutosh Bapat]

On Mon, Mar 6, 2017 at 11:05 AM, Simon Riggs  wrote:
> On 6 March 2017 at 05:29, Ashutosh Bapat
>  wrote:
>
>> Just to confirm, you want the output to look like this
 \d+ t1
 Table "public.t1"
  Column |  Type   | Collation | Nullable | Default | Storage | Stats
 target | Description
 +-+---+--+-+-+--+-
  a  | integer |   | not null | | plain   | 
  |
 Partition key: RANGE (a)
 Partitions: t1p1 FOR VALUES FROM (0) TO (100), HAS PARTITIONS
 t1p2 FOR VALUES FROM (100) TO (200)
>>
>>>
>>> lowercase please
>>
>> Except for HAS PARTITIONS, everything is part of today's output. Given
>> the current output, HAS PARTITIONS should be in upper case.
>
> "has partitions" is not part of the DDL, whereas "FOR VALUES FROM (0)
> TO (100)" is. So ISTM sensible to differentiate between DDL and
> non-ddl using upper and lower case.
>

Make sense. Will try to cook up a patch soon.

-- 
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database 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] Parallel bitmap heap scan

[Dilip Kumar]

On Thu, Mar 2, 2017 at 6:52 PM, Robert Haas  wrote:
> 0002 wasn't quite careful enough about the placement of #ifdef
> USE_PREFETCH, but otherwise looks OK.  Committed after changing that
> and getting rid of the local variable prefetch_iterator, which seemed
> to be adding rather than removing complexity after this refactoring.

0003 is rebased after this commit.

-- 
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com


0003-parallel-bitmap-heapscan-v7.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] dropping partitioned tables without CASCADE

[Simon Riggs]

On 6 March 2017 at 05:29, Ashutosh Bapat
 wrote:

> Just to confirm, you want the output to look like this
>>> \d+ t1
>>> Table "public.t1"
>>>  Column |  Type   | Collation | Nullable | Default | Storage | Stats
>>> target | Description
>>> +-+---+--+-+-+--+-
>>>  a  | integer |   | not null | | plain   |  
>>> |
>>> Partition key: RANGE (a)
>>> Partitions: t1p1 FOR VALUES FROM (0) TO (100), HAS PARTITIONS
>>> t1p2 FOR VALUES FROM (100) TO (200)
>
>>
>> lowercase please
>
> Except for HAS PARTITIONS, everything is part of today's output. Given
> the current output, HAS PARTITIONS should be in upper case.

"has partitions" is not part of the DDL, whereas "FOR VALUES FROM (0)
TO (100)" is. So ISTM sensible to differentiate between DDL and
non-ddl using upper and lower case.

-- 
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] dropping partitioned tables without CASCADE

[Ashutosh Bapat]

On Mon, Mar 6, 2017 at 10:55 AM, Simon Riggs  wrote:
> On 6 March 2017 at 04:00, Ashutosh Bapat
>  wrote:
>> On Mon, Mar 6, 2017 at 8:35 AM, Simon Riggs  wrote:
>>> On 6 March 2017 at 00:51, Amit Langote  
>>> wrote:
 On 2017/03/05 16:20, Simon Riggs wrote:
> I notice also that
>   \d+ 
> does not show which partitions have subpartitions.

 Do you mean showing just whether a partition is itself partitioned or
 showing its partitions and so on (because those partitions may themselves
 be partitioned)?  Maybe, we could do the former.
>>>
>>> I think \d+ should show the full information, in some form.
>>
>> For a multi-level inheritance hierarchy, we don't show children which
>> are further inherited. Same behaviour has been carried over to
>> partitioning. I don't say that that's good or bad.
>>
>> Given the recursive structure of partitioned tables, it looks readable
>> and manageable to print only the direct partitions in \d+. May be we
>> want to indicate the partitions that are further partitioned. If user
>> wants information about partitioned partitions, s/he can execute \d+
>> on the partition, repeating this process to any desired level. This
>> would work well in the interactive mode, keeping the output of \d+
>> manageable. Further someone writing a script to consume \d+ output of
>> a multi-level partitioned table, can code the above process in a
>> script.
>>
>> Thinking about how to display partition which are further partitioned,
>> there are two options. Assume a partitioned table t1 with partitions
>> t1p1, which is further partitioned and t1p2. One could display \d+ t1
>> as
>>
>> \d+ t1
>> Table "public.t1"
>>  Column |  Type   | Collation | Nullable | Default | Storage | Stats
>> target | Description
>> +-+---+--+-+-+--+-
>>  a  | integer |   | not null | | plain   |  |
>> Partition key: RANGE (a)
>> Partitions: t1p1 FOR VALUES FROM (0) TO (100), HAS PARTITIONS
>> t1p2 FOR VALUES FROM (100) TO (200)
>>
>> OR
>>
>> \d+ t1
>> Table "public.t1"
>>  Column |  Type   | Collation | Nullable | Default | Storage | Stats
>> target | Description
>> +-+---+--+-+-+--+-
>>  a  | integer |   | not null | | plain   |  |
>> Partition key: RANGE (a)
>> Partitions: t1p1 FOR VALUES FROM (0) TO (100), PARTITION BY LIST(a)
>> t1p2 FOR VALUES FROM (100) TO (200)
>>
>> To me the first option looks fine.
>
> +1

Just to confirm, you want the output to look like this
>> \d+ t1
>> Table "public.t1"
>>  Column |  Type   | Collation | Nullable | Default | Storage | Stats
>> target | Description
>> +-+---+--+-+-+--+-
>>  a  | integer |   | not null | | plain   |  |
>> Partition key: RANGE (a)
>> Partitions: t1p1 FOR VALUES FROM (0) TO (100), HAS PARTITIONS
>> t1p2 FOR VALUES FROM (100) TO (200)

>
> lowercase please

Except for HAS PARTITIONS, everything is part of today's output. Given
the current output, HAS PARTITIONS should be in upper case.

-- 
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database 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] dropping partitioned tables without CASCADE

[Simon Riggs]

On 6 March 2017 at 04:00, Ashutosh Bapat
 wrote:
> On Mon, Mar 6, 2017 at 8:35 AM, Simon Riggs  wrote:
>> On 6 March 2017 at 00:51, Amit Langote  wrote:
>>> On 2017/03/05 16:20, Simon Riggs wrote:
 I notice also that
   \d+ 
 does not show which partitions have subpartitions.
>>>
>>> Do you mean showing just whether a partition is itself partitioned or
>>> showing its partitions and so on (because those partitions may themselves
>>> be partitioned)?  Maybe, we could do the former.
>>
>> I think \d+ should show the full information, in some form.
>
> For a multi-level inheritance hierarchy, we don't show children which
> are further inherited. Same behaviour has been carried over to
> partitioning. I don't say that that's good or bad.
>
> Given the recursive structure of partitioned tables, it looks readable
> and manageable to print only the direct partitions in \d+. May be we
> want to indicate the partitions that are further partitioned. If user
> wants information about partitioned partitions, s/he can execute \d+
> on the partition, repeating this process to any desired level. This
> would work well in the interactive mode, keeping the output of \d+
> manageable. Further someone writing a script to consume \d+ output of
> a multi-level partitioned table, can code the above process in a
> script.
>
> Thinking about how to display partition which are further partitioned,
> there are two options. Assume a partitioned table t1 with partitions
> t1p1, which is further partitioned and t1p2. One could display \d+ t1
> as
>
> \d+ t1
> Table "public.t1"
>  Column |  Type   | Collation | Nullable | Default | Storage | Stats
> target | Description
> +-+---+--+-+-+--+-
>  a  | integer |   | not null | | plain   |  |
> Partition key: RANGE (a)
> Partitions: t1p1 FOR VALUES FROM (0) TO (100), HAS PARTITIONS
> t1p2 FOR VALUES FROM (100) TO (200)
>
> OR
>
> \d+ t1
> Table "public.t1"
>  Column |  Type   | Collation | Nullable | Default | Storage | Stats
> target | Description
> +-+---+--+-+-+--+-
>  a  | integer |   | not null | | plain   |  |
> Partition key: RANGE (a)
> Partitions: t1p1 FOR VALUES FROM (0) TO (100), PARTITION BY LIST(a)
> t1p2 FOR VALUES FROM (100) TO (200)
>
> To me the first option looks fine.

+1

lowercase please

-- 
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] Parallel seq. plan is not coming against inheritance or partition table

[Ashutosh Sharma]

>
> Right, but OTOH, if we assign parallel workers by default, then it is
> quite possible that it would result in much worse plans.  Consider a
> case where partition hierarchy has 1000 partitions and only one of
> them is big enough to allow parallel workers.  Now in this case, with
> your proposed fix it will try to scan all the partitions in parallel
> workers which I think can easily result in bad performance.

Right. But, there can also be a case where 999 partitions are large
and eligible for PSS. In such case as well, PSS won't be selected.

I think
> the right way to make such plans parallel is by using Parallel Append
> node (https://commitfest.postgresql.org/13/987/).  Alternatively, if
> you want to force parallelism in cases like the one you have shown in
> example, you can use Alter Table .. Set (parallel_workers = 1).

Okay, I was not aware of Parallel Append. Thanks.

With Regards,
Ashutosh Sharma
EnterpriseDB: http://www.enterprisedb.com


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

Re: [HACKERS] Print correct startup cost for the group aggregate.

[Ashutosh Bapat]

On Sat, Mar 4, 2017 at 2:50 PM, Robert Haas  wrote:
> On Thu, Mar 2, 2017 at 6:48 PM, Ashutosh Bapat
>  wrote:
>> On Thu, Mar 2, 2017 at 6:06 PM, Rushabh Lathia  
>> wrote:
>>> While reading through the cost_agg() I found that startup cost for the
>>> group aggregate is not correctly assigned. Due to this explain plan is
>>> not printing the correct startup cost.
>>>
>>> Without patch:
>>>
>>> postgres=# explain select aid, sum(abalance) from pgbench_accounts where
>>> filler like '%foo%' group by aid;
>>>  QUERY PLAN
>>> -
>>>  GroupAggregate  (cost=81634.33..85102.04 rows=198155 width=12)
>>>Group Key: aid
>>>->  Sort  (cost=81634.33..82129.72 rows=198155 width=8)
>>>  Sort Key: aid
>>>  ->  Seq Scan on pgbench_accounts  (cost=0.00..61487.89 rows=198155
>>> width=8)
>>>Filter: (filler ~~ '%foo%'::text)
>>> (6 rows)
>>>
>>> With patch:
>>>
>>> postgres=# explain select aid, sum(abalance) from pgbench_accounts where
>>> filler like '%foo%' group by aid;
>>>  QUERY PLAN
>>> -
>>>  GroupAggregate  (cost=82129.72..85102.04 rows=198155 width=12)
>>>Group Key: aid
>>>->  Sort  (cost=81634.33..82129.72 rows=198155 width=8)
>>>  Sort Key: aid
>>>  ->  Seq Scan on pgbench_accounts  (cost=0.00..61487.89 rows=198155
>>> width=8)
>>>Filter: (filler ~~ '%foo%'::text)
>>> (6 rows)
>>>
>>
>> The reason the reason why startup_cost = input_startup_cost and not
>> input_total_cost for aggregation by sorting is we don't need the whole
>> input before the Group/Agg plan can produce the first row. But I think
>> setting startup_cost = input_startup_cost is also not exactly correct.
>> Before the plan can produce one row, it has to transit through all the
>> rows belonging to the group to which the first row belongs. On an
>> average it has to scan (total number of rows)/(number of groups)
>> before producing the first aggregated row. startup_cost will be
>> input_startup_cost + cost to scan (total number of rows)/(number of
>> groups) rows + cost of transiting over those many rows. Total cost =
>> startup_cost + cost of scanning and transiting through the remaining
>> number of input rows.
>
> While that idea has some merit, I think it's inconsistent with current
> practice.  cost_seqscan(), for example, doesn't include the cost of
> reading the first page in the startup cost, even though that certainly
> must be done before returning the first row.

OTOH, while costing for merge join, initial_cost_mergejoin() seems to
consider the cost of scanning outer and inner relations upto the first
matching tuple on both sides in the startup_cost. Different policies
at different places.

> I think there have been
> previous discussions of switching over to the practice for which you
> are advocating here, but my impression (without researching) is that
> the current practice is more like what Rushabh did.
>

I am not sure Rushabh's approach is correct. Here's the excerpt from my mail.

>> The reason the reason why startup_cost = input_startup_cost and not
>> input_total_cost for aggregation by sorting is we don't need the whole
>> input before the Group/Agg plan can produce the first row.

With Rushabh's approach the startup cost of aggregation by sorting
would be way higher that it's right now. Secondly, it would match that
of hash aggregation and thus forcing hash aggregation to be chosen in
almost all the cases.
-- 
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database 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] dropping partitioned tables without CASCADE

[Ashutosh Bapat]

On Mon, Mar 6, 2017 at 8:35 AM, Simon Riggs  wrote:
> On 6 March 2017 at 00:51, Amit Langote  wrote:
>> On 2017/03/05 16:20, Simon Riggs wrote:
>>> I notice also that
>>>   \d+ 
>>> does not show which partitions have subpartitions.
>>
>> Do you mean showing just whether a partition is itself partitioned or
>> showing its partitions and so on (because those partitions may themselves
>> be partitioned)?  Maybe, we could do the former.
>
> I think \d+ should show the full information, in some form.

For a multi-level inheritance hierarchy, we don't show children which
are further inherited. Same behaviour has been carried over to
partitioning. I don't say that that's good or bad.

Given the recursive structure of partitioned tables, it looks readable
and manageable to print only the direct partitions in \d+. May be we
want to indicate the partitions that are further partitioned. If user
wants information about partitioned partitions, s/he can execute \d+
on the partition, repeating this process to any desired level. This
would work well in the interactive mode, keeping the output of \d+
manageable. Further someone writing a script to consume \d+ output of
a multi-level partitioned table, can code the above process in a
script.

Thinking about how to display partition which are further partitioned,
there are two options. Assume a partitioned table t1 with partitions
t1p1, which is further partitioned and t1p2. One could display \d+ t1
as

\d+ t1
Table "public.t1"
 Column |  Type   | Collation | Nullable | Default | Storage | Stats
target | Description
+-+---+--+-+-+--+-
 a  | integer |   | not null | | plain   |  |
Partition key: RANGE (a)
Partitions: t1p1 FOR VALUES FROM (0) TO (100), HAS PARTITIONS
t1p2 FOR VALUES FROM (100) TO (200)

OR

\d+ t1
Table "public.t1"
 Column |  Type   | Collation | Nullable | Default | Storage | Stats
target | Description
+-+---+--+-+-+--+-
 a  | integer |   | not null | | plain   |  |
Partition key: RANGE (a)
Partitions: t1p1 FOR VALUES FROM (0) TO (100), PARTITION BY LIST(a)
t1p2 FOR VALUES FROM (100) TO (200)

To me the first option looks fine. If the user is interested in
looking at the subpartitioned in any case s/he will have to execute
\d+. So beyond indicating that a partition has subpartitions, what
other information to include is debatable, given that \d+ on that
partition is going to print it anyway.
-- 
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database 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] Report the number of skipped frozen pages by manual VACUUM

[Yugo Nagata]

Hi,

I think this is good since the information is useful and it is
a little change.

One thing I'm bothered is that even when the number of frozen page is
one, it will say "1 frozen pages". In other messages, when the 
number of page is one, the word "page" rather than "pages" is used
by using ngettext().

In addition, the document (doc/src/sgml/ref/vacuum.sgml) need a modification
to use the new messages in its example.

BTW, this patch can't be applied after the following commit.

commit 9eb344faf54a849898d9be012ddfa8204cfeb57c
Author: Simon Riggs 
Date:   Fri Mar 3 19:18:25 2017 +0530

Allow vacuums to report oldestxmin


Regards,
Yugo Nagata

On Tue, 28 Feb 2017 16:40:28 +0900
Masahiko Sawada  wrote:

> On Fri, Feb 24, 2017 at 1:30 AM, Masahiko Sawada  
> wrote:
> > Hi all,
> >
> > The autovacuum reports the number of skipped frozen pages to the
> > VACUUM output. But these information is not appeared by manual VACUUM.
> > This information is useful for the user to check efficiency of VACUUM.
> >
> > Attached patch add this information to VACUUM output.
> >
> > * Example
> > =# VACUUM VERBOSE test
> > INFO:  vacuuming "public.test"
> > INFO:  "test": found 0 removable, 56 nonremovable row versions in 1
> > out of 45 pages
> > DETAIL:  0 dead row versions cannot be removed yet.
> > There were 0 unused item pointers.
> > Skipped 0 pages due to buffer pins, 44 frozen pages.
> > 0 pages are entirely empty.
> > CPU: user: 0.00 s, system: 0.00 s, elapsed: 0.00 s.
> > VACUUM
> >
> > I'll register it to next CF.
> >
> 
> Added this patch to CF 2017-03.
> 
> Regards,
> 
> --
> Masahiko Sawada
> NIPPON TELEGRAPH AND TELEPHONE CORPORATION
> NTT Open Source Software Center
> 
> 
> -- 
> Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers


-- 
Yugo Nagata 


-- 
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] dropping partitioned tables without CASCADE

[Simon Riggs]

On 6 March 2017 at 00:51, Amit Langote  wrote:
> On 2017/03/05 16:20, Simon Riggs wrote:
>> I notice also that
>>   \d+ 
>> does not show which partitions have subpartitions.
>
> Do you mean showing just whether a partition is itself partitioned or
> showing its partitions and so on (because those partitions may themselves
> be partitioned)?  Maybe, we could do the former.

I think \d+ should show the full information, in some form.

-- 
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: [WIP] RE: [HACKERS] DECLARE STATEMENT setting up a connection in ECPG

[Okano, Naoki]

Hi

I tried applying your patches. But it failed...
The error messages are as below.

$ git apply ../004_declareStmt_test_v4.patch
error: patch failed: src/interfaces/ecpg/test/expected/pgtypeslib-nan_test.c:82
error: src/interfaces/ecpg/test/expected/pgtypeslib-nan_test.c: patch does not 
apply

Regards,
Okano Naoki

Re: [HACKERS] SCRAM authentication, take three

[Michael Paquier]

On Mon, Mar 6, 2017 at 11:50 AM, Michael Paquier
 wrote:
> On Fri, Mar 3, 2017 at 2:43 PM, Michael Paquier
>  wrote:
>> I am attaching 0009 and 0010 that address those problems (pushed on
>> github as well) that can be applied on top of the latest set.
>
> While doing more tests with my module able to do SASLprep, I have
> noticed that calculations related to Hangul characters were incorrect:
>  /* Constants for calculations wih Hangul characters */
> -#define SBASE  0xAC00
> -#define LBASE  0x1100
> -#define VBASE  0x1161
> -#define TBASE  0x11A7
> +#define SBASE  0xEAB080/* U+AC00 */
> +#define LBASE  0xE18480/* U+1100 */
> +#define VBASE  0xE185A1/* U+1161 */
> +#define TBASE  0xE186A7/* U+11A7 */

Here is as well an extra patch with this set of fixes, to be applied
on top of the rest. Those are on my github as well, that's for the
archive's sake, and that's better than sending a full set of patches
again.
-- 
Michael


0011-Set-of-fixes-for-SASLprep.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] SCRAM authentication, take three

[Michael Paquier]

On Fri, Mar 3, 2017 at 2:43 PM, Michael Paquier
 wrote:
> I am attaching 0009 and 0010 that address those problems (pushed on
> github as well) that can be applied on top of the latest set.

While doing more tests with my module able to do SASLprep, I have
noticed that calculations related to Hangul characters were incorrect:
 /* Constants for calculations wih Hangul characters */
-#define SBASE  0xAC00
-#define LBASE  0x1100
-#define VBASE  0x1161
-#define TBASE  0x11A7
+#define SBASE  0xEAB080/* U+AC00 */
+#define LBASE  0xE18480/* U+1100 */
+#define VBASE  0xE185A1/* U+1161 */
+#define TBASE  0xE186A7/* U+11A7 */

Once the following is applied things get better:
-- Test for U+FB01, Latin Small Ligature Fi
=# select array_to_utf8(pg_sasl_prepare(utf8_to_array('fi')));
 array_to_utf8
---
 fi
(1 row)
-- Test for U+1E9B, Latin Small Letter Long S with Dot Above
-- This one was failing previously.
=# select array_to_utf8(pg_sasl_prepare(utf8_to_array('ẛ')));
 array_to_utf8
---
 ṡ
(1 row)
-- Test for U+2075, superscript 5
=# select array_to_utf8(pg_sasl_prepare(utf8_to_array('⁵')));
 array_to_utf8
---
 5
(1 row)
-- 
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] Parallel seq. plan is not coming against inheritance or partition table

[Amit Kapila]

On Sat, Mar 4, 2017 at 9:52 PM, Ashutosh Sharma  wrote:
> Hi All,
>
> From following git commit onwards, parallel seq scan is never getting
> selected for inheritance or partitioned tables.
>
> 
> commit 51ee6f3160d2e1515ed6197594bda67eb99dc2cc
> Author: Robert Haas 
> Date:   Wed Feb 15 13:37:24 2017 -0500
>
> Replace min_parallel_relation_size with two new GUCs.
> 
>
>
> Steps to reproduce:
> ==
> create table t1 (a integer);
>
> create table t1_1 (check (a >=1  and a < 100)) inherits (t1);
> create table t1_2 (check (a >= 100 and a < 200)) inherits (t1);
>
> insert into t1_1 select generate_series(1, 90);
> insert into t1_2 select generate_series(100, 190);
>
> analyze t1;
> analyze t1_1;
> analyze t1_2;
>
> explain analyze select * from t1 where a < 5 OR a > 195;
>
> EXPLAIN ANALYZE output:
> 
> 1) Prior to "Replace min_parallel_relation_size with two new GUCs" commit,
>
> postgres=# explain analyze select * from t1 where a < 5 OR a > 195;
>   QUERY PLAN
> ---
>  Gather  (cost=1000.00..25094.71 rows=48787 width=4) (actual
> time=0.431..184.264 rows=4 loops=1)
>Workers Planned: 2
>Workers Launched: 2
>->  Append  (cost=0.00..19216.01 rows=20328 width=4) (actual
> time=0.025..167.465 rows=1 loops=3)
>  ->  Parallel Seq Scan on t1  (cost=0.00..0.00 rows=1 width=4)
> (actual time=0.001..0.001 rows=0 loops=3)
>Filter: ((a < 5) OR (a > 195))
>  ->  Parallel Seq Scan on t1_1  (cost=0.00..9608.00 rows=20252
> width=4) (actual time=0.023..76.644 rows=1 loops=3)
>Filter: ((a < 5) OR (a > 195))
>Rows Removed by Filter: 283334
>  ->  Parallel Seq Scan on t1_2  (cost=0.00..9608.01 rows=75
> width=4) (actual time=89.505..89.505 rows=0 loops=3)
>Filter: ((a < 5) OR (a > 195))
>Rows Removed by Filter: 30
>  Planning time: 0.343 ms
>  Execution time: 188.624 ms
> (14 rows)
>
> 2) From "Replace min_parallel_relation_size with two new GUCs" commit onwards,
>
> postgres=# explain analyze select * from t1 where a < 5 OR a > 195;
> QUERY PLAN
> --
>  Append  (cost=0.00..34966.01 rows=50546 width=4) (actual
> time=0.021..375.747 rows=4 loops=1)
>->  Seq Scan on t1  (cost=0.00..0.00 rows=1 width=4) (actual
> time=0.004..0.004 rows=0 loops=1)
>  Filter: ((a < 5) OR (a > 195))
>->  Seq Scan on t1_1  (cost=0.00..17483.00 rows=50365 width=4)
> (actual time=0.016..198.393 rows=4 loops=1)
>  Filter: ((a < 5) OR (a > 195))
>  Rows Removed by Filter: 850001
>->  Seq Scan on t1_2  (cost=0.00..17483.01 rows=180 width=4)
> (actual time=173.310..173.310 rows=0 loops=1)
>  Filter: ((a < 5) OR (a > 195))
>  Rows Removed by Filter: 91
>  Planning time: 0.812 ms
>  Execution time: 377.831 ms
> (11 rows)
>
> RCA:
> 
> From "Replace min_parallel_relation_size with two new GUCs" commit
> onwards, we are not assigning parallel workers for the child rel with
> zero heap pages. This means we won't be able to create a partial
> append path as this requires all the child rels within an Append Node
> to have a partial path.
>

Right, but OTOH, if we assign parallel workers by default, then it is
quite possible that it would result in much worse plans.  Consider a
case where partition hierarchy has 1000 partitions and only one of
them is big enough to allow parallel workers.  Now in this case, with
your proposed fix it will try to scan all the partitions in parallel
workers which I think can easily result in bad performance.  I think
the right way to make such plans parallel is by using Parallel Append
node (https://commitfest.postgresql.org/13/987/).  Alternatively, if
you want to force parallelism in cases like the one you have shown in
example, you can use Alter Table .. Set (parallel_workers = 1).


-- 
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] Statement-level rollback

[Tsunakawa, Takayuki]

From: David Steele [mailto:da...@pgmasters.net]
> Whatever the merits of this patch, it's a pretty major behavioral change
> with a large potential impact.  Even if what is enumerated here is the full
> list (which I doubt), it's pretty big.
> 
> Given that this landed on March 28 with no discussion beforehand, I recommend
> that we immediately move this patch to the 2017-07 CF.

OK, I moved it to 2017-7.  I will participate in the review of existing 
patches.  In parallel with that, I'll keep developing this feature and 
sometimes submit revised patches and new findings.  I'd be happy if anyone 
could give feedback then.

Regards
Takayuki Tsunakawa



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

[HACKERS] Foreign Join pushdowns not working properly for outer joins

[David Rowley]

I've been asked to investigate a case of a foreign join not occurring
on the foreign server as would have been expected.

I've narrowed this down and the problem seems to only occur with outer
type joins.

The problem can be reproduced by the attached test_case.sql

Upon investigation I've discovered that the problem relates to the
citext extension not being in the shippable_extensions List for the
joinrel. Since the extension is not white listed, the qual on the
citext column is disallowed from being pushed down into the foreign
server by is_shippable().

This happens to work fine for INNER JOINs since the qual makes it into
baserestrictinfo an is properly classified by the following fragment
in postgresGetForeignRelSize()

/*
* Identify which baserestrictinfo clauses can be sent to the remote
* server and which can't.
*/
classifyConditions(root, baserel, baserel->baserestrictinfo,
  >remote_conds, >local_conds);

The attached patch, based on 9.6,  fixes the problem by properly
processing the foreign server options in
postgresGetForeignJoinPaths().

I ended up shifting the code which does this into functions to allow
it to be reused. I also ended up shifting out the code which processes
the table options so that it is consistent.

Reviews from people a bit closer to the foreign join pushdown code are welcome.

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


test_caee.sql
Description: Binary data


foreign_outerjoin_pushdown_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] Re: new high availability feature for the system with both asynchronous and synchronous replication

[Higuchi, Daisuke]

From: Robert Haas [mailto:robertmh...@gmail.com]
>> This patch enables walsender for async to wait until walsender for sync 
>> confirm
>> WAL is flashed to Disk. This feature is activated when GUC parameter
>> "async_walsender_delay" is set on.
> So this new option makes asynchronous replication synchronous?
No, this feature only delays the start of WAL transfer of asynchronous 
replication. 
Asynchronous replication on this feature does not wait for response from 
standby. 
(This behavior does not be changed, so it is the same as before. )

Regards, 
Daisuke Higuchi


-- 
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] dropping partitioned tables without CASCADE

[Amit Langote]

On 2017/03/05 16:20, Simon Riggs wrote:
> I notice also that
>   \d+ 
> does not show which partitions have subpartitions.

Do you mean showing just whether a partition is itself partitioned or
showing its partitions and so on (because those partitions may themselves
be partitioned)?  Maybe, we could do the former.

> I'm worried that these things illustrate something about the catalog
> representation that we may need to improve, but I don't have anything
> concrete to say on that at present.

Perhaps.  As Ashutosh said though, it does not seem like a big problem in
this particular case.

Thanks,
Amit




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

Re: [HACKERS] Cost model for parallel CREATE INDEX

[Peter Geoghegan]

On Sat, Mar 4, 2017 at 2:15 PM, Peter Geoghegan  wrote:
> So, I agree with Robert that we should actually use heap size for the
> main, initial determination of # of workers to use, but we still need
> to estimate the size of the final index [1], to let the cost model cap
> the initial determination when maintenance_work_mem is just too low.
> (This cap will rarely be applied in practice, as I said.)
>
> [1] 
> https://wiki.postgresql.org/wiki/Parallel_External_Sort#bt_estimated_nblocks.28.29_function_in_pageinspect

Having looked at it some more, this no longer seems worthwhile. In the
next revision, I will add a backstop that limits the use of
parallelism based on a lack of maintenance_work_mem in a simpler
manner. Namely, the worker will have to be left with a
maintenance_work_mem/nworkers share of no less than 32MB in order for
parallel CREATE INDEX to proceed. There doesn't seem to be any great
reason to bring the volume of data to be sorted into it.

I expect the cost model to be significantly simplified in the next
revision in other ways, too. There will be no new index storage
parameter, nor a disable_parallelddl GUC. compute_parallel_worker()
will be called in a fairly straightforward way within
plan_create_index_workers(), using heap blocks, as agreed to already.
pg_restore will avoid parallelism (that will happen by setting
"max_parallel_workers_maintenance  = 0" when it runs), not because it
cannot trust the cost model, but because it prefers to parallelize
things its own way (with multiple restore jobs), and because execution
speed may not be the top priority for pg_restore, unlike a live
production system.

-- 
Peter Geoghegan


-- 
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] REINDEX CONCURRENTLY 2.0

[Andreas Karlsson]

On 03/05/2017 07:56 PM, Robert Haas wrote:

On Sat, Mar 4, 2017 at 12:34 PM, Andres Freund  wrote:

I agree that'd it be nicer not to have this, but not having the feature at all 
is a lot worse than this wart.


I, again, give that a firm "maybe".  If the warts end up annoying 1%
of the users who try to use this feature, then you're right.  If they
end up making a substantial percentage of people who try to use this
feature give up on it, then we've added a bunch of complexity and
future code maintenance for little real gain.  I'm not ruling out the
possibility that you're 100% correct, but I'm not nearly as convinced
of that as you are.


I agree that these warts are annoying but I also think the limitations
can be explained pretty easily to users (e.g. by explaining it in the 
manual how REINDEX CONCURRENTLY creates a new index to replace the old 
one), and I think that is the important question about if a useful 
feature with warts should be merged or not. Does it make things 
substantially harder for the average user to grok?


And I would argue that his feature is useful for quite many, based on my 
experience running a semi-large database. Index bloat happens and 
without REINDEX CONCURRENTLY it can be really annoying to solve, 
especially for primary keys. Certainly more people have problems with 
index bloat than the number of people who store index oids in their 
database.


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] REINDEX CONCURRENTLY 2.0

[Peter Geoghegan]

On Sat, Mar 4, 2017 at 9:34 AM, Andres Freund  wrote:
> On March 4, 2017 1:16:56 AM PST, Robert Haas  wrote:
>>Maybe.  But it looks to me like this patch is going to have
>>considerably more than its share of user-visible warts, and I'm not
>>very excited about that.  I feel like what we ought to be doing is
>>keeping the index OID the same and changing the relfilenode to point
>>to a newly-created one, and I attribute our failure to make that
>>design work thus far to insufficiently aggressive hacking.
>
> We literally spent years and a lot of emails waiting for that to happen. 
> Users now hack up solutions like this in userspace, obviously a bad solution.
>
> I agree that'd it be nicer not to have this, but not having the feature at 
> all is a lot worse than this wart.

IMHO, REINDEX itself is implemented in a way that is conceptually
pure, and yet quite user hostile.

I tend to tell colleagues that ask about REINDEX something along the
lines of: Just assume that REINDEX is going to block out even SELECT
statements referencing the underlying table. It might not be that bad
for you in practice, but the details are arcane such that it might as
well be that simple most of the time. Even if you have time to listen
to me explain it all, which you clearly don't, you're still probably
not going to be able to apply what you've learned in a way that helps
you.

-- 
Peter Geoghegan


-- 
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] REINDEX CONCURRENTLY 2.0

[Robert Haas]

On Sat, Mar 4, 2017 at 12:34 PM, Andres Freund  wrote:
> I agree that'd it be nicer not to have this, but not having the feature at 
> all is a lot worse than this wart.

I, again, give that a firm "maybe".  If the warts end up annoying 1%
of the users who try to use this feature, then you're right.  If they
end up making a substantial percentage of people who try to use this
feature give up on it, then we've added a bunch of complexity and
future code maintenance for little real gain.  I'm not ruling out the
possibility that you're 100% correct, but I'm not nearly as convinced
of that as you are.

-- 
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] Measuring replay lag

[Craig Ringer]

On 5 March 2017 at 15:31, Simon Riggs  wrote:
> On 1 March 2017 at 10:47, Thomas Munro  wrote:

>> This seems to be problematic.  Logical peers report LSN changes for
>> all three operations (write, flush, commit) only on commit.  I suppose
>> that might work OK for synchronous replication, but it makes it a bit
>> difficult to get lag measurements that don't look really strange and
>> sawtoothy when you have long transactions, and overlapping
>> transactions might interfere with the measurements in odd ways.

They do, but those sawtoothy measurements are a true reflection of the
aspect of lag that's most important - what the downstream replica has
flushed to disk and made visible.

If we have xacts X1, X2 and X3, which commit in that order, then after
X1 commits the lag is the difference between X1's commit time and
"now". A peer only sends feedback updating flush position for commits.
Once X1 is confirmed replayed by the peer, the lag flush_location is
the difference between X2's later commit time and "now". And so on.

So I'd very much expect a sawtooth lag graph for flush_location,
because that's how logical replication really replays changes. We only
start replaying any xact once it commits on the upstream, and we
replay changes strictly in upstream commit order. It'll rise linearly
then fall vertically in abrupt drops.



sent_location is updated based on the last-decoded WAL position, per
src/backend/replication/walsender.c:2396 or so:

record = XLogReadRecord(logical_decoding_ctx->reader,
logical_startptr, );
logical_startptr = InvalidXLogRecPtr;

/* xlog record was invalid */
if (errm != NULL)
elog(ERROR, "%s", errm);

if (record != NULL)
{
LogicalDecodingProcessRecord(logical_decoding_ctx,
logical_decoding_ctx->reader);

sentPtr = logical_decoding_ctx->reader->EndRecPtr;
}

so I would expect to see a smoother graph for sent_location based on
the last record processed by the XLogReader.

Though it's also a misleading graph, we haven't really sent that at
all, just decoded it and buffered it in a reorder buffer to send once
we decode a commit. Really, pg_stat_replication isn't quite expressive
enough to cover logical replication due to its reordering behaviour.
We can't really report the actual last LSN sent to the client very
easily, since we call into ReorderBufferCommit() and don't return
until we finish streaming the whole buffered xact, we'd need some kind
of callback to update the walsender with the lsn of the last row we
sent. And if we did this, sent_location would actually go backwards
sometimes, since usually with concurrent xacts the newest row in xact
committed at time T is newer, with higher LSN, than the oldest row in
xact with comit time T+n.

Later I'd like to add support for xact interleaving, where we can
speculatively stream rows from big in-progress xacts to the downstream
before the xact commits, and the downstream has to roll the xact back
if it aborts on the upstream. There are some snapshot management
issues to deal with there (not to mention downstream deadlock
hazards), but maybe we can limit the optimisation to xacts that made
no catalog changes to start with. I'm not at all sure how to report
sent_location then, though, or what a reasonable measure of lag will
look like.

> What we want from this patch is something that works for both, as much
> as that is possible.

If it shows a sawtooth pattern for flush lag, that's good, because
it's truthful. We can only flush after we replay commit, therefore lag
is always going to be sawtooth, with tooth size approximating xact
size and the baseline lag trend representing any sustained increase or
decrease in lag over time.

This would be extremely valuable to have.


-- 
 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] Other formats in pset like markdown, rst, mediawiki

[Jan Michálek]

2017-03-05 13:39 GMT+01:00 Pavel Stehule :

>
>
> 2017-03-05 13:22 GMT+01:00 Pavel Stehule :
>
>>
>>
>> 2017-03-05 13:08 GMT+01:00 Jan Michálek :
>>
>>> It is question if it is really new format, because formating is the same
>>> as aligned/wrapped format, changed is only style of lines.
>>>
>>
>> Please, don't do top posting https://en.wikipedia.org/wiki/
>> Posting_style#Top-posting
>>
>>
>>> 2017-03-05 12:36 GMT+01:00 Pavel Stehule :
>>>


 2017-03-05 11:40 GMT+01:00 Jan Michálek :

> I know, but, both new linestyles are created literally by cloning
> ascii linestyle and few lines in print_aligned_text. Both works with
> "aligned" and "wrapped" format. In rst is wrapped format useful, in my
> opinion, in markdown i can`t find how I can get newline in record (maybe 
> it
> is not posiible in main markdown types). So it is why i add markdown and
> rst as new linestyles. But it is not problem to change it in command to 
> use
> "\pset format", but i mean, that this is cleaner.
>

 Using a special linestyle for new format is possible probably. But new
 format should be switched with \pset format command.

 Not sure if wrapped or aligned behave is correct for markdown - it is
 task for markdown processing, not for psql.

>>>
>>
>>
>> In this case I am inclined to prefer setting via format setting - you can
>> set a linestyle and border in one step, and then is easy to return back to
>> previous format. I don't see a big benefit in enhancing set of ascii
>> linestyles. The collecting new features in formatting is more intuitive
>> (for me).
>>
>
>  This can be discussed what we prefer, and what we would to implement?
>
>
>
> 1. Nice formatted markdown tables
>
>
> | Tables| Are   | Cool  |
> | - |:-:| -:|
> | col 3 is  | right-aligned | $1600 |
> | col 2 is  | centered  |   $12 |
> | zebra stripes | are neat  |$1 |
>
> or 2. enough formatting
>
>
> Markdown | Less | Pretty
> --- | --- | ---
> *Still* | `renders` | **nicely**
> 1 | 2 | 3
>
> I personally prefer nice formated table, because more comfortable reading
source of document and easier editing with blocks (deleting whole columns
etc.).
I will change \pset to format.
I find, when adding <\br> for newline works in retext. I will try to add it
to patch.

| Tables | Are | Cool |

| - |:-:| -:|

| col 3 is | right-aligned | $1600 |

| col 2 is | centered | $12 |

| zebra stripes | are neat | $1 |


Jan



> Pavel
>
>
>
>


-- 
Jelen
Starší čeledín datovýho chlíva

Re: [HACKERS] Other formats in pset like markdown, rst, mediawiki

[Pavel Stehule]

2017-03-05 13:22 GMT+01:00 Pavel Stehule :

>
>
> 2017-03-05 13:08 GMT+01:00 Jan Michálek :
>
>> It is question if it is really new format, because formating is the same
>> as aligned/wrapped format, changed is only style of lines.
>>
>
> Please, don't do top posting https://en.wikipedia.org/wiki/
> Posting_style#Top-posting
>
>
>> 2017-03-05 12:36 GMT+01:00 Pavel Stehule :
>>
>>>
>>>
>>> 2017-03-05 11:40 GMT+01:00 Jan Michálek :
>>>
 I know, but, both new linestyles are created literally by cloning ascii
 linestyle and few lines in print_aligned_text. Both works with "aligned"
 and "wrapped" format. In rst is wrapped format useful, in my opinion, in
 markdown i can`t find how I can get newline in record (maybe it is not
 posiible in main markdown types). So it is why i add markdown and rst as
 new linestyles. But it is not problem to change it in command to use "\pset
 format", but i mean, that this is cleaner.

>>>
>>> Using a special linestyle for new format is possible probably. But new
>>> format should be switched with \pset format command.
>>>
>>> Not sure if wrapped or aligned behave is correct for markdown - it is
>>> task for markdown processing, not for psql.
>>>
>>
>
>
> In this case I am inclined to prefer setting via format setting - you can
> set a linestyle and border in one step, and then is easy to return back to
> previous format. I don't see a big benefit in enhancing set of ascii
> linestyles. The collecting new features in formatting is more intuitive
> (for me).
>

 This can be discussed what we prefer, and what we would to implement?



1. Nice formatted markdown tables


| Tables| Are   | Cool  |
| - |:-:| -:|
| col 3 is  | right-aligned | $1600 |
| col 2 is  | centered  |   $12 |
| zebra stripes | are neat  |$1 |

or 2. enough formatting


Markdown | Less | Pretty
--- | --- | ---
*Still* | `renders` | **nicely**
1 | 2 | 3

Pavel

Re: [HACKERS] Other formats in pset like markdown, rst, mediawiki

[Pavel Stehule]

2017-03-05 13:08 GMT+01:00 Jan Michálek :

> It is question if it is really new format, because formating is the same
> as aligned/wrapped format, changed is only style of lines.
>

Please, don't do top posting
https://en.wikipedia.org/wiki/Posting_style#Top-posting


> 2017-03-05 12:36 GMT+01:00 Pavel Stehule :
>
>>
>>
>> 2017-03-05 11:40 GMT+01:00 Jan Michálek :
>>
>>> I know, but, both new linestyles are created literally by cloning ascii
>>> linestyle and few lines in print_aligned_text. Both works with "aligned"
>>> and "wrapped" format. In rst is wrapped format useful, in my opinion, in
>>> markdown i can`t find how I can get newline in record (maybe it is not
>>> posiible in main markdown types). So it is why i add markdown and rst as
>>> new linestyles. But it is not problem to change it in command to use "\pset
>>> format", but i mean, that this is cleaner.
>>>
>>
>> Using a special linestyle for new format is possible probably. But new
>> format should be switched with \pset format command.
>>
>> Not sure if wrapped or aligned behave is correct for markdown - it is
>> task for markdown processing, not for psql.
>>
>


In this case I am inclined to prefer setting via format setting - you can
set a linestyle and border in one step, and then is easy to return back to
previous format. I don't see a big benefit in enhancing set of ascii
linestyles. The collecting new features in formatting is more intuitive
(for me).


Regards

Pavel


>
>> Regards
>>
>> Pavel
>>
>>
>>
>>>
>>> Je;
>>>
>>>
>>> jelen=# \pset linestyle rst
>>> Line style is rst.
>>> jelen=# \pset format wrapped
>>> Output format is wrapped.
>>> jelen=# SELECT repeat('Goodnight Irene ', 30);
>>> +---
>>> --+
>>> |   repeat
>>> |
>>> +===
>>> ==+
>>> | Goodnight Irene Goodnight Irene Goodnight Irene Goodnight Irene
>>> Goodnight I.|
>>> |.rene Goodnight Irene Goodnight Irene Goodnight Irene Goodnight Irene
>>> Goodni.|
>>> |.ght Irene Goodnight Irene Goodnight Irene Goodnight Irene Goodnight
>>> Irene G.|
>>> |.oodnight Irene Goodnight Irene Goodnight Irene Goodnight Irene
>>> Goodnight Ir.|
>>> |.ene Goodnight Irene Goodnight Irene Goodnight Irene Goodnight Irene
>>> Goodnig.|
>>> |.ht Irene Goodnight Irene Goodnight Irene Goodnight Irene Goodnight
>>> Irene Go.|
>>> |.odnight Irene Goodnight Irene
>>> |
>>> +---
>>> --+
>>>
>>> (1 row)
>>>
>>> jelen=#
>>>
>>>
>>> 2017-03-01 15:00 GMT+01:00 Peter Eisentraut <
>>> peter.eisentr...@2ndquadrant.com>:
>>>
 If you want to implement a new table format, you should be looking at
 \pset format, not \pset linestyle.  \pset format sets different table
 formats, such as html, latex, and asciidoc.  \pset linestyle just
 chooses between different styles for the plain-text table format.

 On 3/1/17 06:31, Jan Michálek wrote:
 > Regression test corrected.
 >
 > 2017-03-01 11:43 GMT+01:00 Jan Michálek  >:
 >
 > Sorry, I have some errors in my diff, i had copy something from
 bad
 > folder. I will fix it.
 >
 > 2017-03-01 0:27 GMT+01:00 Jan Michálek  >:
 >
 > There it is, what i have.
 > I need i small help with psql.out, because \pset format
 wrapped.
 > I don`t know, how to have it in fixed width.
 >
 > 2017-02-28 14:23 GMT+01:00 Jan Michálek <
 godzilalal...@gmail.com
 > >:
 >
 > Current state is something like this (diff is attached).
 > I currently haven`t regression test, tab completion etc.,
 I
 > will add this thing following structure of asciidoc
 commit.
 >
 > Output is tested using retext, rst is OK, md have problem
 > with cells with newline (i must find out, how it is
 possible
 > create table with this in markdown).
 >
 > [jelen@laptak patch_postgre_rst]$
 > [jelen@laptak psql]$ ./psql
 > psql (9.6.2, server 9.6.1)
 > Type "help" for help.
 >
 > jelen=# \pset linestyle markdown
 > Line style is markdown.
 > jelen=# values(E'nasral Franta\nna trabanta','Žluťoučký
 > kobyly'), (,E'a\tb') \g | xclip
 > jelen=# values(E'nasral Franta\nna trabanta','Žluťoučký
 > kobyly'), (,E'a\tb') \g
 >
 > |column1| column2  |
 > 

Re: [HACKERS] Other formats in pset like markdown, rst, mediawiki

[Jan Michálek]

It is question if it is really new format, because formating is the same as
aligned/wrapped format, changed is only style of lines.

2017-03-05 12:36 GMT+01:00 Pavel Stehule :

>
>
> 2017-03-05 11:40 GMT+01:00 Jan Michálek :
>
>> I know, but, both new linestyles are created literally by cloning ascii
>> linestyle and few lines in print_aligned_text. Both works with "aligned"
>> and "wrapped" format. In rst is wrapped format useful, in my opinion, in
>> markdown i can`t find how I can get newline in record (maybe it is not
>> posiible in main markdown types). So it is why i add markdown and rst as
>> new linestyles. But it is not problem to change it in command to use "\pset
>> format", but i mean, that this is cleaner.
>>
>
> Using a special linestyle for new format is possible probably. But new
> format should be switched with \pset format command.
>
> Not sure if wrapped or aligned behave is correct for markdown - it is task
> for markdown processing, not for psql.
>
> Regards
>
> Pavel
>
>
>
>>
>> Je;
>>
>>
>> jelen=# \pset linestyle rst
>> Line style is rst.
>> jelen=# \pset format wrapped
>> Output format is wrapped.
>> jelen=# SELECT repeat('Goodnight Irene ', 30);
>> +---
>> --+
>> |   repeat
>> |
>> +===
>> ==+
>> | Goodnight Irene Goodnight Irene Goodnight Irene Goodnight Irene
>> Goodnight I.|
>> |.rene Goodnight Irene Goodnight Irene Goodnight Irene Goodnight Irene
>> Goodni.|
>> |.ght Irene Goodnight Irene Goodnight Irene Goodnight Irene Goodnight
>> Irene G.|
>> |.oodnight Irene Goodnight Irene Goodnight Irene Goodnight Irene
>> Goodnight Ir.|
>> |.ene Goodnight Irene Goodnight Irene Goodnight Irene Goodnight Irene
>> Goodnig.|
>> |.ht Irene Goodnight Irene Goodnight Irene Goodnight Irene Goodnight
>> Irene Go.|
>> |.odnight Irene Goodnight Irene
>> |
>> +---
>> --+
>>
>> (1 row)
>>
>> jelen=#
>>
>>
>> 2017-03-01 15:00 GMT+01:00 Peter Eisentraut <
>> peter.eisentr...@2ndquadrant.com>:
>>
>>> If you want to implement a new table format, you should be looking at
>>> \pset format, not \pset linestyle.  \pset format sets different table
>>> formats, such as html, latex, and asciidoc.  \pset linestyle just
>>> chooses between different styles for the plain-text table format.
>>>
>>> On 3/1/17 06:31, Jan Michálek wrote:
>>> > Regression test corrected.
>>> >
>>> > 2017-03-01 11:43 GMT+01:00 Jan Michálek >> > >:
>>> >
>>> > Sorry, I have some errors in my diff, i had copy something from bad
>>> > folder. I will fix it.
>>> >
>>> > 2017-03-01 0:27 GMT+01:00 Jan Michálek >> > >:
>>> >
>>> > There it is, what i have.
>>> > I need i small help with psql.out, because \pset format
>>> wrapped.
>>> > I don`t know, how to have it in fixed width.
>>> >
>>> > 2017-02-28 14:23 GMT+01:00 Jan Michálek <
>>> godzilalal...@gmail.com
>>> > >:
>>> >
>>> > Current state is something like this (diff is attached).
>>> > I currently haven`t regression test, tab completion etc., I
>>> > will add this thing following structure of asciidoc commit.
>>> >
>>> > Output is tested using retext, rst is OK, md have problem
>>> > with cells with newline (i must find out, how it is
>>> possible
>>> > create table with this in markdown).
>>> >
>>> > [jelen@laptak patch_postgre_rst]$
>>> > [jelen@laptak psql]$ ./psql
>>> > psql (9.6.2, server 9.6.1)
>>> > Type "help" for help.
>>> >
>>> > jelen=# \pset linestyle markdown
>>> > Line style is markdown.
>>> > jelen=# values(E'nasral Franta\nna trabanta','Žluťoučký
>>> > kobyly'), (,E'a\tb') \g | xclip
>>> > jelen=# values(E'nasral Franta\nna trabanta','Žluťoučký
>>> > kobyly'), (,E'a\tb') \g
>>> >
>>> > |column1| column2  |
>>> > |---|--|
>>> > | nasral Franta | Žluťoučký kobyly |
>>> > | na trabanta   |  |
>>> > | ' | a   b|
>>> >
>>> >
>>> > (2 rows)
>>> >
>>> > jelen=# \pset linestyle rst
>>> > Line style is rst.
>>> > jelen=# values(E'nasral Franta\nna trabanta','Žluťoučký
>>> > kobyly'), (,E'a\tb') \g
>>> > +---+--+
>>> > |column1| column2  |
>>> > +===+==+
>>> > | nasral 

Re: [HACKERS] Other formats in pset like markdown, rst, mediawiki

[Pavel Stehule]

2017-03-05 11:40 GMT+01:00 Jan Michálek :

> I know, but, both new linestyles are created literally by cloning ascii
> linestyle and few lines in print_aligned_text. Both works with "aligned"
> and "wrapped" format. In rst is wrapped format useful, in my opinion, in
> markdown i can`t find how I can get newline in record (maybe it is not
> posiible in main markdown types). So it is why i add markdown and rst as
> new linestyles. But it is not problem to change it in command to use "\pset
> format", but i mean, that this is cleaner.
>

Using a special linestyle for new format is possible probably. But new
format should be switched with \pset format command.

Not sure if wrapped or aligned behave is correct for markdown - it is task
for markdown processing, not for psql.

Regards

Pavel



>
> Je;
>
>
> jelen=# \pset linestyle rst
> Line style is rst.
> jelen=# \pset format wrapped
> Output format is wrapped.
> jelen=# SELECT repeat('Goodnight Irene ', 30);
> +---
> --+
> |   repeat
> |
> +===
> ==+
> | Goodnight Irene Goodnight Irene Goodnight Irene Goodnight Irene
> Goodnight I.|
> |.rene Goodnight Irene Goodnight Irene Goodnight Irene Goodnight Irene
> Goodni.|
> |.ght Irene Goodnight Irene Goodnight Irene Goodnight Irene Goodnight
> Irene G.|
> |.oodnight Irene Goodnight Irene Goodnight Irene Goodnight Irene Goodnight
> Ir.|
> |.ene Goodnight Irene Goodnight Irene Goodnight Irene Goodnight Irene
> Goodnig.|
> |.ht Irene Goodnight Irene Goodnight Irene Goodnight Irene Goodnight Irene
> Go.|
> |.odnight Irene Goodnight Irene
> |
> +---
> --+
>
> (1 row)
>
> jelen=#
>
>
> 2017-03-01 15:00 GMT+01:00 Peter Eisentraut  com>:
>
>> If you want to implement a new table format, you should be looking at
>> \pset format, not \pset linestyle.  \pset format sets different table
>> formats, such as html, latex, and asciidoc.  \pset linestyle just
>> chooses between different styles for the plain-text table format.
>>
>> On 3/1/17 06:31, Jan Michálek wrote:
>> > Regression test corrected.
>> >
>> > 2017-03-01 11:43 GMT+01:00 Jan Michálek > > >:
>> >
>> > Sorry, I have some errors in my diff, i had copy something from bad
>> > folder. I will fix it.
>> >
>> > 2017-03-01 0:27 GMT+01:00 Jan Michálek > > >:
>> >
>> > There it is, what i have.
>> > I need i small help with psql.out, because \pset format wrapped.
>> > I don`t know, how to have it in fixed width.
>> >
>> > 2017-02-28 14:23 GMT+01:00 Jan Michálek <
>> godzilalal...@gmail.com
>> > >:
>> >
>> > Current state is something like this (diff is attached).
>> > I currently haven`t regression test, tab completion etc., I
>> > will add this thing following structure of asciidoc commit.
>> >
>> > Output is tested using retext, rst is OK, md have problem
>> > with cells with newline (i must find out, how it is possible
>> > create table with this in markdown).
>> >
>> > [jelen@laptak patch_postgre_rst]$
>> > [jelen@laptak psql]$ ./psql
>> > psql (9.6.2, server 9.6.1)
>> > Type "help" for help.
>> >
>> > jelen=# \pset linestyle markdown
>> > Line style is markdown.
>> > jelen=# values(E'nasral Franta\nna trabanta','Žluťoučký
>> > kobyly'), (,E'a\tb') \g | xclip
>> > jelen=# values(E'nasral Franta\nna trabanta','Žluťoučký
>> > kobyly'), (,E'a\tb') \g
>> >
>> > |column1| column2  |
>> > |---|--|
>> > | nasral Franta | Žluťoučký kobyly |
>> > | na trabanta   |  |
>> > | ' | a   b|
>> >
>> >
>> > (2 rows)
>> >
>> > jelen=# \pset linestyle rst
>> > Line style is rst.
>> > jelen=# values(E'nasral Franta\nna trabanta','Žluťoučký
>> > kobyly'), (,E'a\tb') \g
>> > +---+--+
>> > |column1| column2  |
>> > +===+==+
>> > | nasral Franta+| Žluťoučký kobyly |
>> > | na trabanta   |  |
>> > +---+--+
>> > | ' | a   b|
>> > +---+--+
>> >
>> > (2 rows)
>> >
>> > jelen=#
>> >
>> > 2017-02-24 0:46 

Re: [HACKERS] Other formats in pset like markdown, rst, mediawiki

[Jan Michálek]

I know, but, both new linestyles are created literally by cloning ascii
linestyle and few lines in print_aligned_text. Both works with "aligned"
and "wrapped" format. In rst is wrapped format useful, in my opinion, in
markdown i can`t find how I can get newline in record (maybe it is not
posiible in main markdown types). So it is why i add markdown and rst as
new linestyles. But it is not problem to change it in command to use "\pset
format", but i mean, that this is cleaner.

Je;


jelen=# \pset linestyle rst
Line style is rst.
jelen=# \pset format wrapped
Output format is wrapped.
jelen=# SELECT repeat('Goodnight Irene ', 30);
+-+
|
repeat|
+=+
| Goodnight Irene Goodnight Irene Goodnight Irene Goodnight Irene Goodnight
I.|
|.rene Goodnight Irene Goodnight Irene Goodnight Irene Goodnight Irene
Goodni.|
|.ght Irene Goodnight Irene Goodnight Irene Goodnight Irene Goodnight Irene
G.|
|.oodnight Irene Goodnight Irene Goodnight Irene Goodnight Irene Goodnight
Ir.|
|.ene Goodnight Irene Goodnight Irene Goodnight Irene Goodnight Irene
Goodnig.|
|.ht Irene Goodnight Irene Goodnight Irene Goodnight Irene Goodnight Irene
Go.|
|.odnight Irene Goodnight
Irene   |
+-+

(1 row)

jelen=#


2017-03-01 15:00 GMT+01:00 Peter Eisentraut :

> If you want to implement a new table format, you should be looking at
> \pset format, not \pset linestyle.  \pset format sets different table
> formats, such as html, latex, and asciidoc.  \pset linestyle just
> chooses between different styles for the plain-text table format.
>
> On 3/1/17 06:31, Jan Michálek wrote:
> > Regression test corrected.
> >
> > 2017-03-01 11:43 GMT+01:00 Jan Michálek  > >:
> >
> > Sorry, I have some errors in my diff, i had copy something from bad
> > folder. I will fix it.
> >
> > 2017-03-01 0:27 GMT+01:00 Jan Michálek  > >:
> >
> > There it is, what i have.
> > I need i small help with psql.out, because \pset format wrapped.
> > I don`t know, how to have it in fixed width.
> >
> > 2017-02-28 14:23 GMT+01:00 Jan Michálek  > >:
> >
> > Current state is something like this (diff is attached).
> > I currently haven`t regression test, tab completion etc., I
> > will add this thing following structure of asciidoc commit.
> >
> > Output is tested using retext, rst is OK, md have problem
> > with cells with newline (i must find out, how it is possible
> > create table with this in markdown).
> >
> > [jelen@laptak patch_postgre_rst]$
> > [jelen@laptak psql]$ ./psql
> > psql (9.6.2, server 9.6.1)
> > Type "help" for help.
> >
> > jelen=# \pset linestyle markdown
> > Line style is markdown.
> > jelen=# values(E'nasral Franta\nna trabanta','Žluťoučký
> > kobyly'), (,E'a\tb') \g | xclip
> > jelen=# values(E'nasral Franta\nna trabanta','Žluťoučký
> > kobyly'), (,E'a\tb') \g
> >
> > |column1| column2  |
> > |---|--|
> > | nasral Franta | Žluťoučký kobyly |
> > | na trabanta   |  |
> > | ' | a   b|
> >
> >
> > (2 rows)
> >
> > jelen=# \pset linestyle rst
> > Line style is rst.
> > jelen=# values(E'nasral Franta\nna trabanta','Žluťoučký
> > kobyly'), (,E'a\tb') \g
> > +---+--+
> > |column1| column2  |
> > +===+==+
> > | nasral Franta+| Žluťoučký kobyly |
> > | na trabanta   |  |
> > +---+--+
> > | ' | a   b|
> > +---+--+
> >
> > (2 rows)
> >
> > jelen=#
> >
> > 2017-02-24 0:46 GMT+01:00 Michael Paquier
> >  >>:
> >
> > On Fri, Feb 24, 2017 at 3:09 AM, Jan Michálek
> >  > > wrote:
> > > I can try it, doesn`t look dificult, but I`m worry,
> that I`m not able to
> > > write clean, pretty code.
> >
> >

Re: [HACKERS] Cleanup: avoid direct use of ip_posid/ip_blkid

[Pavan Deolasee]

On Thu, Mar 2, 2017 at 9:55 PM, Peter Eisentraut <
peter.eisentr...@2ndquadrant.com> wrote:

> On 2/22/17 08:38, Pavan Deolasee wrote:
> > One reason why these macros are not always used is because they
> > typically do assert-validation to ensure ip_posid has a valid value.
> > There are a few places in code, especially in GIN and also when we are
> > dealing with user-supplied TIDs when we might get a TID with invalid
> > ip_posid. I've handled that by defining and using separate macros which
> > skip the validation. This doesn't seem any worse than what we are
> > already doing.
>
> I wonder why we allow that.  Shouldn't the tid type reject input that
> has ip_posid == 0?
>

Yes, I think it seems sensible to disallow InvalidOffsetNumber (or >
MaxOffsetNumber) in user-supplied value. But there are places in GIN and
with INSERT ON CONFLICT where we seem to use special values in ip_posid to
mean different things. So we might still need some way to accept invalid
values there.

Thanks,
Pavan

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

Re: [HACKERS] dropping partitioned tables without CASCADE

[Simon Riggs]

On 5 March 2017 at 07:59, Ashutosh Bapat
 wrote:
>>
>> I used a slight modification of the case mentioned on the docs. I
>> confirm this fails repeatably for me on current HEAD.
>>
>> CREATE TABLE cities (
>>  city_id  bigserial not null,
>>  name text not null,
>>  population   bigint
>> ) PARTITION BY LIST (left(lower(name), 1));
>>
>> CREATE TABLE cities_ab
>> PARTITION OF cities (
>>   CONSTRAINT city_id_nonzero CHECK (city_id != 0)
>> ) FOR VALUES IN ('a', 'b')
>> PARTITION BY RANGE (population);
>>
>> drop table cities;
>> ERROR:  cannot drop table cities because other objects depend on it
>> DETAIL:  table cities_ab depends on table cities
>> HINT:  Use DROP ... CASCADE to drop the dependent objects too.
>
> I think that's what this patch fixes. Do you see this behaviour after
> applying the patch?

It does seems as if I've made a mistake there. The patch passes.
Thanks for checking.

I will apply tomorrow if no further comments.

-- 
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] patch: function xmltable

[Pavel Stehule]

Hi

I used your idea about special columns when COLUMNS are not explicitly
defined.

All lines that you are dislike removed.

Now, almost all code, related to this behave, is in next few lines.

+   /*
+* Use implicit column when it is necessary. The COLUMNS clause is
optional
+* on Oracle and DB2. In this case a result is complete row of XML type.
+*/
+   if (rtf->columns == NIL)
+   {
+   RangeTableFuncCol *fc = makeNode(RangeTableFuncCol);
+   A_Const *n = makeNode(A_Const);
+
+   fc->colname = "xmltable";
+   fc->typeName = makeTypeNameFromOid(XMLOID, -1);
+   n->val.type = T_String;
+   n->val.val.str = ".";
+   n->location = -1;
+
+   fc->colexpr = (Node *) n;
+   rtf->columns = list_make1(fc);
+   }

all regress tests passing.

Regards

Pavel


xmltable-50.patch.gz
Description: GNU Zip compressed 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] dropping partitioned tables without CASCADE

[Ashutosh Bapat]

>
> I used a slight modification of the case mentioned on the docs. I
> confirm this fails repeatably for me on current HEAD.
>
> CREATE TABLE cities (
>  city_id  bigserial not null,
>  name text not null,
>  population   bigint
> ) PARTITION BY LIST (left(lower(name), 1));
>
> CREATE TABLE cities_ab
> PARTITION OF cities (
>   CONSTRAINT city_id_nonzero CHECK (city_id != 0)
> ) FOR VALUES IN ('a', 'b')
> PARTITION BY RANGE (population);
>
> drop table cities;
> ERROR:  cannot drop table cities because other objects depend on it
> DETAIL:  table cities_ab depends on table cities
> HINT:  Use DROP ... CASCADE to drop the dependent objects too.

I think that's what this patch fixes. Do you see this behaviour after
applying the patch?

>
> I notice also that
>   \d+ 
> does not show which partitions have subpartitions.

I will confirm this once I have access to the code.

>
> I'm worried that these things illustrate something about the catalog
> representation that we may need to improve, but I don't have anything
> concrete to say on that at present.

AFAIK, catalogs have everything needed to fix this; it's just the
matter to using that information wherever it's needed.
-- 
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company


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