Re: [HACKERS] Parallel Seq Scan

[Amit Langote]

On 13-03-2015 PM 01:37, Amit Langote wrote:
> By the way, is it right that TupleQueueFunnel.queue has one shm_mq_handle per
> initialized parallel worker? If so, how does TupleQueueFunnel.maxqueues relate
> to ParallelContext.nworkers (of the corresponding parallel context)?
> 
> Why I asked this is because in CreateTupleQueueFunnel():
> 
> funnel->maxqueues = 8;
> funnel->queue = palloc(funnel->maxqueues * sizeof(shm_mq_handle *));
> 
> So, is the hardcoded "8" intentional or an oversight?
> 

Oh, I see that in RegisterTupleQueueOnFunnel(), the TupleQueueFunnel.queue is
expanded (repalloc'd) if needed as per corresponding pcxt->nworkers.

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] Parallel Seq Scan

[Amit Langote]

On 13-03-2015 AM 10:24, Amit Kapila wrote:
> On Thu, Mar 12, 2015 at 4:22 PM, Amit Langote 
>> From Robert's description[1], it looked like the NestLoop with Funnel
> would
>> have Funnel as either outer plan or topmost plan node or NOT a
> parameterised
>> plan. In that case, would this case arise or am I missing something?
>>
> 
> Probably not if the costing is right and user doesn't manually disable
> plans (like by set enable_* = off).  However we should have rescan code
> incase it chooses the plan such that Funnel is inner node and I think
> apart from that also in few cases Rescan is required.
> 

I see, thanks.

By the way, is it right that TupleQueueFunnel.queue has one shm_mq_handle per
initialized parallel worker? If so, how does TupleQueueFunnel.maxqueues relate
to ParallelContext.nworkers (of the corresponding parallel context)?

Why I asked this is because in CreateTupleQueueFunnel():

funnel->maxqueues = 8;
funnel->queue = palloc(funnel->maxqueues * sizeof(shm_mq_handle *));

So, is the hardcoded "8" intentional or an oversight?

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] EvalPlanQual behaves oddly for FDW queries involving system columns

[Etsuro Fujita]

On 2015/03/13 0:50, Tom Lane wrote:

The tableoid problem can be fixed much less invasively as per the attached
patch.  I think that we should continue to assume that ctid is not
meaningful (and hence should read as (4294967295,0)) in FDWs that use
ROW_MARK_COPY, and press forward on fixing the locking issues for
postgres_fdw by letting it use ROW_MARK_REFERENCE or something close to
that.  That would also cause ctid to read properly for rows from
postgres_fdw.


OK, thanks!

BTW, what do you think about opening/locking foreign tables selected for 
update at InitPlan, which the original patch does?  As I mentioned in 
[1], ISTM that ExecOpenScanRelation called from ExecInitForeignScan is 
assuming that.


Best regards,
Etsuro Fujita

[1] http://www.postgresql.org/message-id/54bcbbf8.3020...@lab.ntt.co.jp


--
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] CATUPDATE confusion?

[Stephen Frost]

* Robert Haas (robertmh...@gmail.com) wrote:
> On Sat, Mar 7, 2015 at 6:40 PM, Adam Brightwell
>  wrote:
> >> pg_shadow, pg_user and pg_group were added when role support was added,
> >> specifically for backwards compatibility.  I don't believe there was
> >> ever discussion about keeping them because filtering pg_roles based on
> >> rolcanlogin was too onerous.  That said, we already decided recently
> >> that we wanted to keep them updated to match the actual attributes
> >> available (note that the replication role attribute modified those
> >> views) and I think that makes sense on the removal side as well as the
> >> new-attribute side.
> >>
> >> I continue to feel that we really should officially deprecate those
> >> views as I don't think they're actually all that useful any more and
> >> maintaining them ends up bringing up all these questions, discussion,
> >> and ends up being largely busy-work if no one really uses them.
> >
> > Deprecation would certainly seem like an appropriate path for 'usecatupd'
> > (and the mentioned views).  Though, is there a 'formal' deprecation
> > policy/process that the community follows?  I certainly understand and
> > support giving client/dependent tools the time and opportunity to update
> > accordingly.  Therefore, I think having them read from 'rolsuper' for the
> > time being would be ok, provided that they are actually removed at the next
> > possible opportunity.  Otherwise, I'd probably lean towards just removing
> > them now and getting it over with.
> 
> Unless some popular tool like pgAdmin is using these views, I'd say we
> should just nuke them outright.  If it breaks somebody's installation,
> then they can always recreate the view on their particular system.
> That seems like an adequate workaround to me.

As near as I can tell, pgAdmin3 does still use pg_user (though I don't
think it uses pg_group or pg_shadow except when connected to an ancient
server) in some cases.  Where it is used, based on my quick review at
least, it looks like it'd be pretty easy to fix.

The rolcatupdate usage appears to all be associated with pg_authid
though, and the changes required to remove the places where it shows up
doesn't look particularly bad either.  There are no references to
usecatupdate.  Where there are references to 'use*', they'd have to also
be updated with the change to pg_user, naturally.

Looking at phppgadmin, there are quite a few more uses of pg_user there,
along with references to pg_group and even pg_shadow (for 8.0 and
earlier backends).  Amusingly, the only place 'catupdate' is referenced
there is in the Polish language file.  Updating phppgadmin to not
reference pg_user or the other views looks like it'd be a bit more work,
but not a huge effort either.

Basically, in my view at least, these programs are likely to continue to
use these backwards compatibility views until we either formally
deprecate them or (more likely) until we actually remove them and things
break.  As such, I'm not sure if this information really helps us make a
decision here.

Thanks!

Stephen


signature.asc
Description: Digital signature

Re: [HACKERS] Providing catalog view to pg_hba.conf file - Patch submission

[Peter Eisentraut]

On 3/4/15 1:34 AM, Haribabu Kommi wrote:
> On Wed, Mar 4, 2015 at 12:35 PM, Haribabu Kommi
>  wrote:
>> + foreach(line, parsed_hba_lines)
>>
>> In the above for loop it is better to add "check_for_interrupts" to
>> avoid it looping
>> if the parsed_hba_lines are more.
> 
> Updated patch is attached with the addition of check_for_interrupts in
> the for loop.

I tried out your latest patch.  I like that it updates even in running
sessions when the file is reloaded.

The permission checking is faulty, because unprivileged users can
execute pg_hba_settings() directly.

Check the error messages against the style guide (especially
capitalization).

I don't like that there is a hard-coded limit of 16 options 5 pages away
from where it is actually used.  That could be done better.

I'm not sure about the name "pg_hba_settings".  Why not just "pg_hba" or
"pg_hba_conf" if you're going for a representation that is close to the
file (as opposed to pg_settings, which is really a lot more abstract
than any particular file).

I would put the line_number column first.

I continue to think that it is a serious mistake to stick special values
like 'all' and 'replication' into the arrays without additional
decoration.  That makes this view useless or at least dangerous for
editing tools or tools that want to reassemble the original file.
Clearly at least one of those has to be a use case.  Otherwise we can
just print out the physical lines without interpretation.

The "mask" field can go, because address is of type inet, which can
represent masks itself.  (Or should it be cidr then?  Who knows.)  The
preferred visual representation of masks in pg_hba.conf has been
"address/mask" for a while now, so we should preserve that.
Additionally, you can then use the existing inet/cidr operations to do
things like checking whether some IP address is contained in an address
specification.

I can't tell from the documentation what the "compare_method" field is
supposed to do.  I see it on the code, but that is not a natural
representation of pg_hba.conf.  In fact, this just supports my earlier
statement.  Why are special values in the address field special, but not
in the user or database fields?

uaImplicitReject is not a user-facing authentication method, so it
shouldn't be shown (or showable).

I would have expected options to be split into keys and values.

All that code to reassemble the options from the parsed struct
representation seems crazy to me.  Surely, we could save the strings as
we parse them?

I can't make sense of the log message "pg_hba.conf not reloaded,
pg_hba_settings may show stale information".  If load_hba() failed, the
information isn't stale, is it?

In any case, printing this to the server log seems kind of pointless,
because someone using the view is presumably doing so because they can't
or don't want to read the server log.  The proper place might be a
warning when the view is actually called.



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

Re: Custom/Foreign-Join-APIs (Re: [HACKERS] [v9.5] Custom Plan API)

[Kouhei Kaigai]

> On Mon, Mar 9, 2015 at 11:18 PM, Kouhei Kaigai  wrote:
> > The attached patch integrates a suggestion from Ashutosh Bapat.
> > It allows to track set of relations involved in a join, but
> > replaced by foreign-/custom-scan. It enables to make correct
> > EXPLAIN output, if FDW/CSP driver makes human readable symbols
> > using deparse_expression() or others.
> >
> > Differences from v7 is identical with what I posted on the
> > join push-down support thread.
> 
> I took a look at this patch today and noticed that it incorporates not
> only documentation for the new functionality it adds, but also for the
> custom-scan functionality whose documentation I previously excluded
> from commit on the grounds that it needed more work, especially to
> improve the English.  That decision was not popular at the time, and I
> think we need to remedy it before going further with this.  I had
> hoped that someone else would care about this work enough to help with
> the documentation, but it seems not, so today I went through the
> documentation in this patch, excluded all of the stuff specific to
> custom joins, and heavily edited the rest.  The result is attached.
> 
> If there are no objections, I'll commit this; then, someone can rebase
> this patch over these changes and we can proceed from there.
> 
Thanks for your help. I tried to check the documentation from the
implementation standpoint, however, I have no objection here.

Best regards,
--
NEC OSS Promotion Center / PG-Strom Project
KaiGai Kohei 


-- 
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] Using 128-bit integers for sum, avg and statistics aggregates

[Andreas Karlsson]

On 03/10/2015 02:26 AM, Peter Geoghegan wrote:

On Mon, Mar 9, 2015 at 5:37 PM, Andreas Karlsson  wrote:

int128-agg-v7.patch


I see a spelling error:

"+ * On platforms which support 128-bit integers some aggergates instead use a"


Fixed.


I think you should talk about the new thing first (just after the
extant, first sentence "Integer data types use Numeric..."). Refer to
where 128-bit integers are used and how, and only then the other stuff
(exceptions). After that, put the  PolyNumAggState struct definition
and basic functions. Then int2_accum() and so on.


Good idea! Do you think the rewritten comment is clear enough, or do I 
need to go into more detail?


/*
 * Integer data types use Numeric accumulators to share code and avoid risk
 * of overflow.  To speed up aggregation 128-bit integer accumulators are
 * used instead where sum(X) or sum(X*X) fit into 128-bits, and there is
 * platform support.
 *
 * For int2 and int4 inputs sum(X) will fit into a 64-bit accumulator, 
hence

 * we use faster special-purpose accumulator routines for SUM and AVG of
 * these datatypes.
 */

#ifdef HAVE_INT128
typedef struct Int128AggState

--
Andreas Karlsson


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

[Amit Kapila]

On Thu, Mar 12, 2015 at 4:22 PM, Amit Langote 
wrote:
>
> On 10-03-2015 PM 01:09, Amit Kapila wrote:
> > On Tue, Mar 10, 2015 at 6:50 AM, Haribabu Kommi <
kommi.harib...@gmail.com>
> >> Is this patch handles the cases where the re-scan starts without
> >> finishing the earlier scan?
> >>
> >
> > Do you mean to say cases like ANTI, SEMI Join (in nodeNestLoop.c)
> > where we scan the next outer tuple and rescan inner table without
> > completing the previous scan of inner table?
> >
> > I have currently modelled it based on existing rescan for seqscan
> > (ExecReScanSeqScan()) which means it will begin the scan again.
> > Basically if the workers are already started/initialized by previous
> > scan, then re-initialize them (refer function ExecReScanFunnel() in
> > patch).
> >
>
> From Robert's description[1], it looked like the NestLoop with Funnel
would
> have Funnel as either outer plan or topmost plan node or NOT a
parameterised
> plan. In that case, would this case arise or am I missing something?
>

Probably not if the costing is right and user doesn't manually disable
plans (like by set enable_* = off).  However we should have rescan code
incase it chooses the plan such that Funnel is inner node and I think
apart from that also in few cases Rescan is required.


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

Re: [HACKERS] Bug in point releases 9.3.6 and 9.2.10?

[Peter Geoghegan]

On Thu, Mar 12, 2015 at 5:56 PM, Andres Freund  wrote:
> So, no hot standby enabled?

Right.

> There've been some issues with seek(END) sometimes returning the wrong
> length in the past. And I've seen a report that might indicate a similar
> bug has been reintroduced. That could certainly cause such anerror.

Perhaps.

>> I am unfamiliar with this provisioning code, so, as I
>> mentioned, offhand I cannot be entirely sure that there isn't some
>> other code run when the problem originally arises (that I should have
>> included in my report).
>
> It's probably worthwhile to dig out what's happening.

I'll get to that after the backtrace.

>> What I can tell you is that I saw the same error messages when I
>> manually ran the statements generated by the above code within a
>> transaction...until I ran "VACUUM FULL pg_auth_members;".
>
> You can reproduce that problem? How easily? If you can, please produce a
> backtrace. It'll certainly be interesting to see whether that access is
> through an index or whatnot.

I suspect I can reproduce it quite easily. The next step should be to
do a backtrace. I'll look at that tomorrow.

-- 
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] Bug in point releases 9.3.6 and 9.2.10?

[Andres Freund]

On 2015-03-12 17:42:33 -0700, Peter Geoghegan wrote:
> On Thu, Mar 12, 2015 at 5:21 PM, Andres Freund  wrote:
> > On 2015-03-12 16:42:24 -0700, Peter Geoghegan wrote:
> >> We want to create a new role when this happens, for various reasons.
> >> This occurs after recovery ends, but before the database has been
> >> "unfenced". The template code that generates various ALTER ROLE
> >> statements in our internal provisioning system - which has apparently
> >> worked just fine for a long time - is:
> >
> > Is this all the code that's exececuted after recovery? How are these
> > forks brought up? Promoted how? Is it a common 'source' database?
> 
> We do PITR up to a recovery target.

So, no hot standby enabled?

> I'm not sure what other code might have already been run at this
> point, but it won't have been much.

> > Have you looked at these files? Are they indeed zero bytes when this
> > error occurs?

> I think that they are indeed zero. I looked at that last week, when I
> didn't consider that this might be a more widespread issue. I'll check
> again later.

> > Any chance that the new nodes also use a different kernel version or
> > such?
> 
> They may differ, but that doesn't seem likely to be relevant, at least
> to me.

There've been some issues with seek(END) sometimes returning the wrong
length in the past. And I've seen a report that might indicate a similar
bug has been reintroduced. That could certainly cause such anerror.

> I am unfamiliar with this provisioning code, so, as I
> mentioned, offhand I cannot be entirely sure that there isn't some
> other code run when the problem originally arises (that I should have
> included in my report).

It's probably worthwhile to dig out what's happening.

> What I can tell you is that I saw the same error messages when I
> manually ran the statements generated by the above code within a
> transaction...until I ran "VACUUM FULL pg_auth_members;".

You can reproduce that problem? How easily? If you can, please produce a
backtrace. It'll certainly be interesting to see whether that access is
through an index or whatnot.

Greetings,

Andres Freund

-- 
 Andres Freund 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] Bug in point releases 9.3.6 and 9.2.10?

[Peter Geoghegan]

In a hurry right now, so unfortunately I'll need to be brief for now.

On Thu, Mar 12, 2015 at 5:21 PM, Andres Freund  wrote:
> On 2015-03-12 16:42:24 -0700, Peter Geoghegan wrote:
>> We want to create a new role when this happens, for various reasons.
>> This occurs after recovery ends, but before the database has been
>> "unfenced". The template code that generates various ALTER ROLE
>> statements in our internal provisioning system - which has apparently
>> worked just fine for a long time - is:
>
> Is this all the code that's exececuted after recovery? How are these
> forks brought up? Promoted how? Is it a common 'source' database?

We do PITR up to a recovery target. We're talking about the same issue
occurring on entirely distinct customer databases, with entirely
distinct major PG versions. I'm not sure what other code might have
already been run at this point, but it won't have been much. As I
said, the only common factor that I know of is all affected databases
being on the latest point release.

> Have you looked at these files? Are they indeed zero bytes when this
> error occurs?

I think that they are indeed zero. I looked at that last week, when I
didn't consider that this might be a more widespread issue. I'll check
again later.

> Do you still have a base backup from the relevant time, so you could
> repeat the whole thing?

Yes.

>> The only common factor is that this occurs on the latest point
>> releases (either 9.3.6 and 9.2.10, at least so far). In all cases I've
>> seen so far, the relation in question is the pg_auth_members heap
>> relation. For example:
>
> Any chance that the new nodes also use a different kernel version or
> such?

They may differ, but that doesn't seem likely to be relevant, at least
to me. This has happened something like 6 or 7 times already, starting
late last week. I am unfamiliar with this provisioning code, so, as I
mentioned, offhand I cannot be entirely sure that there isn't some
other code run when the problem originally arises (that I should have
included in my report). What I can tell you is that I saw the same
error messages when I manually ran the statements generated by the
above code within a transaction...until I ran "VACUUM FULL
pg_auth_members;".

> This filenode got to be pg_auth_member's original one, given it's below
> FirstNormalObjectId. I get a lower value, but that's probably caused by
> having fewer collations and other data generated during initdb. That
> implies that the table hasn't ever been rewritten.
>
> What's 12811?

It's the same catalog, pg_auth_member. As I said, the messages you saw
are on entirely different customer databases, servers and (sometimes)
PG version.

-- 
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] Bug in point releases 9.3.6 and 9.2.10?

[Andres Freund]

Hi,

On 2015-03-12 16:42:24 -0700, Peter Geoghegan wrote:
> We want to create a new role when this happens, for various reasons.
> This occurs after recovery ends, but before the database has been
> "unfenced". The template code that generates various ALTER ROLE
> statements in our internal provisioning system - which has apparently
> worked just fine for a long time - is:

Is this all the code that's exececuted after recovery? How are these
forks brought up? Promoted how? Is it a common 'source' database?

> db.execute("ALTER ROLE #{old_database_user} RENAME TO #{database_user}")
> db.execute("ALTER ROLE #{database_user} PASSWORD '#{database_password}' 
> LOGIN")
> db.execute("CREATE ROLE \"#{old_database_user}\" PASSWORD
> '#{old_database_password}' IN ROLE \"#{database_user}\" LOGIN")
> 
> I've seen multiple reports of apparent corruption, appearing as the
> resulting ALTER ROLE statements are executed:
> 
> PG::DataCorrupted: ERROR: could not read block 0 in file
> "global/12811": read only 0 of 8192 bytes
> or:
> PG::DataCorrupted: ERROR: could not read block 0 in file
> "global/12785": read only 0 of 8192 bytes
> or:
> PG::DataCorrupted: ERROR: could not read block 0 in file
> "global/12811": read only 0 of 8192 bytes

Have you looked at these files? Are they indeed zero bytes when this
error occurs?

Do you still have a base backup from the relevant time, so you could
repeat the whole thing?

> The only common factor is that this occurs on the latest point
> releases (either 9.3.6 and 9.2.10, at least so far). In all cases I've
> seen so far, the relation in question is the pg_auth_members heap
> relation. For example:

Any chance that the new nodes also use a different kernel version or
such?

> redacteddb=#  select pg_relation_filenode(oid), oid, relname, relkind
> from pg_class where pg_relation_filenode(oid) = 12785;
>  pg_relation_filenode | oid  | relname | relkind
> --+--+-+-
> 12785 | 1261 | pg_auth_members | r
> (1 row)

This filenode got to be pg_auth_member's original one, given it's below
FirstNormalObjectId. I get a lower value, but that's probably caused by
having fewer collations and other data generated during initdb. That
implies that the table hasn't ever been rewritten.

What's 12811?

Greetings,

Andres Freund

-- 
 Andres Freund 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: Custom/Foreign-Join-APIs (Re: [HACKERS] [v9.5] Custom Plan API)

[Thom Brown]

On 12 March 2015 at 21:28, Tom Lane  wrote:
> Robert Haas  writes:
>> I took a look at this patch today and noticed that it incorporates not
>> only documentation for the new functionality it adds, but also for the
>> custom-scan functionality whose documentation I previously excluded
>> from commit on the grounds that it needed more work, especially to
>> improve the English.  That decision was not popular at the time, and I
>> think we need to remedy it before going further with this.  I had
>> hoped that someone else would care about this work enough to help with
>> the documentation, but it seems not, so today I went through the
>> documentation in this patch, excluded all of the stuff specific to
>> custom joins, and heavily edited the rest.  The result is attached.
>
> Looks good; I noticed one trivial typo --- please s/returns/return/ under
> ExecCustomScan.  Also, perhaps instead of just "set ps_ResultTupleSlot"
> that should say "fill ps_ResultTupleSlot with the next tuple in the
> current scan direction".

Also:

s/initalization/initialization/

-- 
Thom


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

[HACKERS] Bug in point releases 9.3.6 and 9.2.10?

[Peter Geoghegan]

Heroku Postgres runs provisioning code that performs certain actions
on roles when creating a new "fork" of an existing database. This
often causes the new fork to be on the latest point release, where the
database being forked was not.

We want to create a new role when this happens, for various reasons.
This occurs after recovery ends, but before the database has been
"unfenced". The template code that generates various ALTER ROLE
statements in our internal provisioning system - which has apparently
worked just fine for a long time - is:

db.execute("ALTER ROLE #{old_database_user} RENAME TO #{database_user}")
db.execute("ALTER ROLE #{database_user} PASSWORD '#{database_password}' LOGIN")
db.execute("CREATE ROLE \"#{old_database_user}\" PASSWORD
'#{old_database_password}' IN ROLE \"#{database_user}\" LOGIN")

I've seen multiple reports of apparent corruption, appearing as the
resulting ALTER ROLE statements are executed:

PG::DataCorrupted: ERROR: could not read block 0 in file
"global/12811": read only 0 of 8192 bytes
or:
PG::DataCorrupted: ERROR: could not read block 0 in file
"global/12785": read only 0 of 8192 bytes
or:
PG::DataCorrupted: ERROR: could not read block 0 in file
"global/12811": read only 0 of 8192 bytes


The only common factor is that this occurs on the latest point
releases (either 9.3.6 and 9.2.10, at least so far). In all cases I've
seen so far, the relation in question is the pg_auth_members heap
relation. For example:

redacteddb=#  select pg_relation_filenode(oid), oid, relname, relkind
from pg_class where pg_relation_filenode(oid) = 12785;
 pg_relation_filenode | oid  | relname | relkind
--+--+-+-
12785 | 1261 | pg_auth_members | r
(1 row)


Running "VACUUM FULL pg_auth_members;" has made the problem go away
(to the extent that the above code doesn't trip up and everything is
at least superficially okay) on at least one occasion. I'm currently
investigating how consistently that works as a short term remediation.

Theories?
-- 
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] Moving Pivotal's Greenplum work upstream

[Josh Berkus]

On 03/12/2015 03:24 PM, Ewan Higgs wrote:
> Hi all,
> There has been some press regarding Pivotal's intent to release
> Greenplum source as part of an Open Development Platform (along with
> some of their Hadoop projects). Can anyone speak on whether any of
> Greenplum might find its way upstream? For example, if(!) the work is
> being released under an appropriate license, are people at Pivotal
> planning to push patches for the parallel architecture and associated
> query planner upstream?

At this point, we have no idea.  We'll know better when the code is
actually available.

-- 
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.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] Reduce pinning in btree indexes

[Peter Geoghegan]

On Sat, Feb 14, 2015 at 4:19 PM, Kevin Grittner  wrote:
> At some point we could consider building on this patch to recheck
> index conditions for heap access when a non-MVCC snapshot is used,
> check the visibility map for referenced heap pages when the TIDs
> are read for an index-only scan, and/or skip LP_DEAD hinting for
> non-WAL-logged indexes.  But all those are speculative future work;
> this is a conservative implementation that just didn't modify
> pinning where there were any confounding factors.

I don't have the bandwidth for a full review, but I took a quick look at this.

I think you should call out those "confounding factors" in the README.
It's not hard to piece them together from
_bt_drop_lock_and_maybe_pin(), but I think you should anyway.

Don't use BUFFER_LOCK_SHARE -- use BT_READ, as the existing nbtree
LockBuffer() callers do. You're inconsistent about that in V3.

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

[HACKERS] Moving Pivotal's Greenplum work upstream

[Ewan Higgs]

Hi all,There has been some press regarding Pivotal's intent to release 
Greenplum source as part of an Open Development Platform (along with some of 
their Hadoop projects). Can anyone speak on whether any of Greenplum might find 
its way upstream? For example, if(!) the work is being released under an 
appropriate license, are people at Pivotal planning to push patches for the 
parallel architecture and associated query planner upstream?

Thanks,Ewan 

Re: Custom/Foreign-Join-APIs (Re: [HACKERS] [v9.5] Custom Plan API)

[Tom Lane]

Robert Haas  writes:
> I took a look at this patch today and noticed that it incorporates not
> only documentation for the new functionality it adds, but also for the
> custom-scan functionality whose documentation I previously excluded
> from commit on the grounds that it needed more work, especially to
> improve the English.  That decision was not popular at the time, and I
> think we need to remedy it before going further with this.  I had
> hoped that someone else would care about this work enough to help with
> the documentation, but it seems not, so today I went through the
> documentation in this patch, excluded all of the stuff specific to
> custom joins, and heavily edited the rest.  The result is attached.

Looks good; I noticed one trivial typo --- please s/returns/return/ under
ExecCustomScan.  Also, perhaps instead of just "set ps_ResultTupleSlot"
that should say "fill ps_ResultTupleSlot with the next tuple in the
current scan direction".

regards, tom lane


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

Re: [HACKERS] forward vs backward slashes in msvc build code

[Alvaro Herrera]

Peter Eisentraut wrote:
> On 3/4/15 11:00 PM, Andrew Dunstan wrote:
> > 
> > On 03/04/2015 10:37 PM, Peter Eisentraut wrote:

> >> I can't tell from just looking at the code how chkpass would normally
> >> find crypt.  The msvc tools neither parse SHLIB_LINK nor have hardcoded
> >> knowledge.  Any idea?
> 
> > Which library is it in? There are sections at the top of Mkvcbuild.pm
> > for including various libraries in contrib modules that need them.
> 
> This is contrib/chkpass not finding the crypt symbol, which is
> presumably in some library.  But I can't see how it would normally find
> it, without my patch.

It seems crypt is provided by libpgport.  So chkpass should be mentioned
in @contrib_uselibpgport, but isn't.  Maybe the fix is just to add it
there?

-- 
Álvaro Herrerahttp://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: Custom/Foreign-Join-APIs (Re: [HACKERS] [v9.5] Custom Plan API)

[Robert Haas]

On Mon, Mar 9, 2015 at 11:18 PM, Kouhei Kaigai  wrote:
> The attached patch integrates a suggestion from Ashutosh Bapat.
> It allows to track set of relations involved in a join, but
> replaced by foreign-/custom-scan. It enables to make correct
> EXPLAIN output, if FDW/CSP driver makes human readable symbols
> using deparse_expression() or others.
>
> Differences from v7 is identical with what I posted on the
> join push-down support thread.

I took a look at this patch today and noticed that it incorporates not
only documentation for the new functionality it adds, but also for the
custom-scan functionality whose documentation I previously excluded
from commit on the grounds that it needed more work, especially to
improve the English.  That decision was not popular at the time, and I
think we need to remedy it before going further with this.  I had
hoped that someone else would care about this work enough to help with
the documentation, but it seems not, so today I went through the
documentation in this patch, excluded all of the stuff specific to
custom joins, and heavily edited the rest.  The result is attached.

If there are no objections, I'll commit this; then, someone can rebase
this patch over these changes and we can proceed from there.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
diff --git a/doc/src/sgml/custom-scan.sgml b/doc/src/sgml/custom-scan.sgml
new file mode 100644
index 000..fa6f457
--- /dev/null
+++ b/doc/src/sgml/custom-scan.sgml
@@ -0,0 +1,284 @@
+
+
+
+ Writing A Custom Scan Provider
+
+ 
+  custom scan provider
+  handler for
+ 
+
+ 
+  PostgreSQL supports a set of experimental facilities which
+  are intended to allow extension modules to add new scan types to the system.
+  Unlike a foreign data wrapper, which is only
+  responsible for knowing how to scan its own foreign tables, a custom scan
+  provider can provide an alternative method of scanning any relation in the
+  system.  Typically, the motivation for writing a custom scan provider will
+  be to allow the use of some optimization not supported by the core
+  system, such as caching or some form of hardware acceleration.  This chapter
+  outlines how to write a new custom scan provider.
+ 
+
+ 
+  Implementing a new type of custom scan is a three-step process.  First,
+  during planning, it is necessary to generate access paths representing a
+  scan using the proposed strategy.  Second, if one of those access paths
+  is selected by the planner as the optimal strategy for scanning a
+  particular relation, the access path must be converted to a plan.
+  Finally, it must be possible to execute the plan and generate the same
+  results that would have been generated for any other access path targeting
+  the same relation.
+ 
+
+ 
+  Implementing Custom Paths
+
+  
+A custom scan provider will typically add paths by setting the following
+hook, which is called after the core code has generated what it believes
+to be the complete and correct set of access paths for the relation.
+
+typedef void (*set_rel_pathlist_hook_type) (PlannerInfo *root,
+RelOptInfo *rel,
+Index rti,
+RangeTblEntry *rte);
+extern PGDLLIMPORT set_rel_pathlist_hook_type set_rel_pathlist_hook;
+
+  
+
+  
+Although this hook function can be used to examine, modify, or remove
+paths generated by the core system, a custom scan provider will typically
+confine itself to generating CustomPath objects and adding
+them to rel using add_path.  The custom scan
+provider is responsible for initializing the CustomPath
+object, which is declared like this:
+
+typedef struct CustomPath
+{
+Path  path;
+uint32flags;
+List *custom_private;
+const CustomPathMethods *methods;
+} CustomPath;
+
+  
+
+  
+path must be initialized as for any other path, including
+the row-count estimate, start and total cost, and sort ordering provided
+by this path.  flags is a bitmask, which should include
+CUSTOMPATH_SUPPORT_BACKWARD_SCAN if the custom path can support
+a backward scan and CUSTOMPATH_SUPPORT_MARK_RESTORE if it
+can support mark and restore.  Both capabilities are optional.
+custom_private can be used to store the custom path's
+private data.  Private data should be stored in a form that can be handled
+by nodeToString, so that debugging routines which attempt to
+print the custom path will work as designed.  methods must
+point to a (usually statically allocated) object implementing the required
+custom path methods, of which there are currently only two, as further
+detailed below.
+  
+
+  
+  Custom Path Callbacks
+
+  
+
+Plan *(*PlanCustomPath) (PlannerInfo *root,
+ RelOptInfo *rel,
+ CustomPath *best_path,
+ 

Re: [HACKERS] Reduce pinning in btree indexes

[Kevin Grittner]

Kevin Grittner  wrote:

> Because these changes are just to a comment and a README file,
> I'm posting a patch-on-patch v3a (to be applied on top of v3).

Here is some additional comment work that I hope will make things
easier to understand for whoever next visits this code.  There is
also a minor reorganization of some code that should not have any
impact except to skip the restore memcpy() calls where they are not
needed in a corner case that I'm not sure we can currently even
reach.  That reorganization is intended more for code clarity than
for the marginal performance it could provide.

I'm posting this as v3b to be applied on top of v3 and v3a, so that
these changes can be reviewed and commented on separately.

--
Kevin Grittner
EDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

bt-nopin-v3b.patch
Description: invalid/octet-stream

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

Re: [HACKERS] Turning off HOT/Cleanup sometimes

[Robert Haas]

On Thu, Mar 12, 2015 at 3:48 PM, Peter Eisentraut  wrote:
> On 3/12/15 5:41 AM, Andres Freund wrote:
>> On 2015-03-11 20:55:18 -0400, Peter Eisentraut wrote:
>>> I don't think so.  Andres basically wanted a nontrival algorithm to
>>> determine how much pruning to do during a read-only scan.  And Robert
>>> basically said, that's not really possible.
>>
>> I don't think either of us made really strong statements.
>
> I didn't mean to put words in your mouth.  I just wanted to summarize
> the thread as, Andres wanted more fine-tuning on the behavior, Robert
> expressed serious doubts that that will lead to an acceptable result.

Or to put that another way, I'm not sure there's one behavior here
that will please everybody.

-- 
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] OOM-killer issue when updating a inheritance table which has large number of child tables

[Stephen Frost]

* Tom Lane (t...@sss.pgh.pa.us) wrote:
> chenhj  writes:
> > In my test(PG9.3.4), i found when update a parent table which has a large 
> > number of child tables, the execute plan will consume lots of memory. And 
> > possibly cause OOM.
> 
> See
> file:///net/sss1/home/postgres/pgsql/doc/src/sgml/html/ddl-partitioning.html#DDL-PARTITIONING-CAVEATS
> 
> particularly the last paragraph.

Or perhaps, if you're on the Internet, this instead:

http://www.postgresql.org/docs/9.4/static/ddl-partitioning.html#DDL-PARTITIONING-CAVEATS

:)

Thanks,

Stephen


signature.asc
Description: Digital signature

Re: [HACKERS] OOM-killer issue when updating a inheritance table which has large number of child tables

[Tom Lane]

Stephen Frost  writes:
> * Tom Lane (t...@sss.pgh.pa.us) wrote:
>> See
>> file:///net/sss1/home/postgres/pgsql/doc/src/sgml/html/ddl-partitioning.html#DDL-PARTITIONING-CAVEATS

> Or perhaps, if you're on the Internet, this instead:
> http://www.postgresql.org/docs/9.4/static/ddl-partitioning.html#DDL-PARTITIONING-CAVEATS

Ooops, pasted a link to my local copy.  Sorry bout that ...

regards, tom lane


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

Re: [HACKERS] forward vs backward slashes in msvc build code

[Peter Eisentraut]

On 3/4/15 11:00 PM, Andrew Dunstan wrote:
> 
> On 03/04/2015 10:37 PM, Peter Eisentraut wrote:
>> On 2/15/15 6:55 AM, Michael Paquier wrote:
>>> I tested quickly the second patch with MS 2010 and I am getting a
>>> build failure: chkpass cannot complete because of crypt missing. On
>>> master build passes. More details here:
>>> "C:\Users\mpaquier\git\postgres\pgsql.sln" (default target) (1) ->
>>> "C:\Users\mpaquier\git\postgres\chkpass.vcxproj" (default target)
>>> (36) ->
>>> (Link target) ->
>>>chkpass.obj : error LNK2019: unresolved external symbol crypt
>>> referenced in function chkpass_in
>>> [C:\Users\ioltas\git\postgres\chkpass.vcxproj]
>>>.\Release\chkpass\chkpass.dll : fatal error LNK1120: 1 unresolved
>>> externals [C:\Users\mpaquier\git\postgres\chkpass.vcxproj]
>> I can't tell from just looking at the code how chkpass would normally
>> find crypt.  The msvc tools neither parse SHLIB_LINK nor have hardcoded
>> knowledge.  Any idea?

> Which library is it in? There are sections at the top of Mkvcbuild.pm
> for including various libraries in contrib modules that need them.

This is contrib/chkpass not finding the crypt symbol, which is
presumably in some library.  But I can't see how it would normally find
it, without my patch.




-- 
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] Turning off HOT/Cleanup sometimes

[Peter Eisentraut]

On 3/12/15 5:41 AM, Andres Freund wrote:
> On 2015-03-11 20:55:18 -0400, Peter Eisentraut wrote:
>> I don't think so.  Andres basically wanted a nontrival algorithm to
>> determine how much pruning to do during a read-only scan.  And Robert
>> basically said, that's not really possible.
> 
> I don't think either of us made really strong statements.

I didn't mean to put words in your mouth.  I just wanted to summarize
the thread as, Andres wanted more fine-tuning on the behavior, Robert
expressed serious doubts that that will lead to an acceptable result.



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

Re: [HACKERS] logical column ordering

[Peter Eisentraut]

On 3/12/15 10:07 AM, Andres Freund wrote:
> I actually wonder if it'd not make more sense to define it as the
> physical column number. That'd reduce the invasiveness and risk of the
> patch considerably.

Clearly, the number of places where attnum has to be changed to
something else is not zero, and so it doesn't matter if a lot or a few
have to be changed.  They all have to be looked at and considered.



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

Re: [HACKERS] logical column ordering

[Peter Eisentraut]

On 3/11/15 10:16 PM, Tom Lane wrote:

> I think using an OID would break more stuff than it fixes in dependency
> tracking; in particular you would now need an explicit dependency link
> from each column to its table, because the "sub-object" knowledge would
> no longer work.

That might not be a bad thing, but ...

> In any case this patch is going to be plenty big enough
> already without saddling it with a major rewrite of the dependency system.

... is true.



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

Re: [HACKERS] pg_rewind in contrib

[Heikki Linnakangas]

On 03/12/2015 08:49 AM, Amit Kapila wrote:

With attached modified script, I am able to reproduce the
error (I have used the latest pg_rewind patch (pg_rewind-bin-8))


Thanks! That reproduced the error for me too. Not sure why I couldn't 
reproduce it earlier.


The cause was a silly typo in truncate_target_file:


@@ -397,7 +397,7 @@ truncate_target_file(const char *path, off_t newsize)

snprintf(dstpath, sizeof(dstpath), "%s/%s", datadir_target, path);

-   fd = open(path, O_WRONLY, 0);
+   fd = open(dstpath, O_WRONLY, 0);
if (fd < 0)
pg_fatal("could not open file \"%s\" for truncation: %s\n",
 dstpath, strerror(errno));


Attached is a new version of the patch, including that fix, and rebased 
over current git master.


- Heikki



pg_rewind-bin-9.patch.gz
Description: application/gzip

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

Re: [HACKERS] get_object_address support for additional object types

[Alvaro Herrera]

Stephen Frost wrote:

> I thought the rest of it looked alright.  I agree it's a bit odd how the
> opfamily is handled but I agree with your assessment that there's not
> much better we can do with this object representation.

Actually, on second thought I revisited this and changed the
representation for opfamilies and opclasses: instead of putting the AM
name in objargs, we can put it as the first element of objname instead.
That way, objargs is unused for opfamilies and opclasses, and we're free
to use it for the type arguments in amops and amprocs.  This makes the
lists consistent for the four cases: in objname, amname first, then
qualified opclass/opfamily name.  For amop/amproc, the member number
follows.  Objargs is unused in opclass/opfamily, and it's a two-element
list of types in amop/amproc.

The attached patch changes the grammar to comply with the above, and
adds the necessary get_object_address and getObjectIdentityParts support
code for amop/amproc objects.

The only thing a bit unusual is that in does_not_exist_skipping() we
need to do an list_copy_tail() to strip out the amname before reporting
the "skipping" message, for DROP OPERATOR CLASS/FAMILY IF NOT EXISTS.
I don't think this is a problem.  In return, the code to deconstruct
amop and amproc addresses is more sensible.

-- 
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
>From 35336c997b8be18d890a3a8bdf2822f757f70faf Mon Sep 17 00:00:00 2001
From: Alvaro Herrera 
Date: Fri, 6 Mar 2015 12:39:50 -0300
Subject: [PATCH] Support opfamily members in get_object_address

In the spirit of 890192e99af and 4464303405f, have get_object_address
understand pg_amop and pg_amproc objects.  There is no way to refer to
such objects directly in the grammar, but in event triggers and
pg_get_object_address it becomes possible to become involved with them.

Reviewed by: Stephen Frost
---
 src/backend/catalog/objectaddress.c  | 234 +--
 src/backend/commands/dropcmds.c  |  24 ++-
 src/backend/commands/event_trigger.c |   2 +
 src/backend/parser/gram.y|  43 ++---
 src/include/nodes/parsenodes.h   |   2 +
 src/test/regress/expected/object_address.out |  60 ---
 src/test/regress/sql/object_address.sql  |  16 +-
 7 files changed, 264 insertions(+), 117 deletions(-)

diff --git a/src/backend/catalog/objectaddress.c b/src/backend/catalog/objectaddress.c
index 142bc68..46ea09a 100644
--- a/src/backend/catalog/objectaddress.c
+++ b/src/backend/catalog/objectaddress.c
@@ -492,9 +492,9 @@ ObjectTypeMap[] =
 	/* OCLASS_OPFAMILY */
 	{ "operator family", OBJECT_OPFAMILY },
 	/* OCLASS_AMOP */
-	{ "operator of access method", -1 },	/* unmapped */
+	{ "operator of access method", OBJECT_AMOP },
 	/* OCLASS_AMPROC */
-	{ "function of access method", -1 },	/* unmapped */
+	{ "function of access method", OBJECT_AMPROC },
 	/* OCLASS_REWRITE */
 	{ "rule", OBJECT_RULE },
 	/* OCLASS_TRIGGER */
@@ -552,9 +552,12 @@ static ObjectAddress get_object_address_attrdef(ObjectType objtype,
 		   List *objname, Relation *relp, LOCKMODE lockmode,
 		   bool missing_ok);
 static ObjectAddress get_object_address_type(ObjectType objtype,
-		List *objname, bool missing_ok);
+		ListCell *typecell, bool missing_ok);
 static ObjectAddress get_object_address_opcf(ObjectType objtype, List *objname,
-		List *objargs, bool missing_ok);
+		bool missing_ok);
+static ObjectAddress get_object_address_opf_member(ObjectType objtype,
+			  List *objname, List *objargs, bool missing_ok);
+
 static ObjectAddress get_object_address_usermapping(List *objname,
 			   List *objargs, bool missing_ok);
 static ObjectAddress get_object_address_defacl(List *objname, List *objargs,
@@ -567,8 +570,7 @@ static void getRelationTypeDescription(StringInfo buffer, Oid relid,
 		   int32 objectSubId);
 static void getProcedureTypeDescription(StringInfo buffer, Oid procid);
 static void getConstraintTypeDescription(StringInfo buffer, Oid constroid);
-static void getOpFamilyIdentity(StringInfo buffer, Oid opfid, List **objname,
-	List **objargs);
+static void getOpFamilyIdentity(StringInfo buffer, Oid opfid, List **objname);
 static void getRelationIdentity(StringInfo buffer, Oid relid, List **objname);
 
 /*
@@ -661,7 +663,8 @@ get_object_address(ObjectType objtype, List *objname, List *objargs,
 	ObjectAddress	domaddr;
 	char		   *constrname;
 
-	domaddr = get_object_address_type(OBJECT_DOMAIN, objname, missing_ok);
+	domaddr = get_object_address_type(OBJECT_DOMAIN,
+	  list_head(objname), missing_ok);
 	constrname = strVal(linitial(objargs));
 
 	address.classId = ConstraintRelationId;
@@ -685,7 +688,7 @@ get_object_address(ObjectType objtype, List *objname, List *objargs,
 break;
 			case OBJECT_TYPE:
 			case OBJECT_DOMAIN:
-address = get_object_address_type(objtype, 

Re: [HACKERS] Documentation of bt_page_items()'s ctid field

[Tom Lane]

Jeff Janes  writes:
> I think we do want this.  It was asked how much we want to include
> internals of the btree code into pageinspect documentation, but I think the
> nature of pageinspect makes that unavoidable.

Yeah.  Committed with a little bit of additional wordsmithing.

regards, tom lane


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

Re: [HACKERS] Cube extension kNN support

[Stas Kelvich]

Documentation along with style fix.



distances2r3.patch
Description: Binary data


> On 08 Feb 2015, at 00:32, Alexander Korotkov  wrote:
> 
> Hi!
> 
> On Sat, Feb 7, 2015 at 12:45 PM, Stas Kelvich  wrote:
> I had updated old patch with kNN operators for cube data structures. Copying 
> description from old message:
> 
> Following distance operators introduced:
> 
> <#> taxicab distance
> <->  euclidean distance
> <=> chebyshev distance
> 
> For example:
> SELECT * FROM objects ORDER BY objects.coord <-> '(137,42,314)'::cube LIMIT 
> 10;
> 
> Also there is operator "->" for selecting ordered rows directly from index.
> This request selects rows ordered ascending by 3rd coordinate:
> 
> SELECT * FROM objects ORDER BY objects.coord->3 LIMIT 10;
> 
> For descendent ordering suggested syntax with minus before coordinate.
> This request selects rows ordered descending by 4th coordinate:
> 
> SELECT * FROM objects ORDER BY objects.coord->-4 LIMIT 10;
> 
> I've checked the patch. The first notes are so:
> 1) Check coding style, in particular braces. Postgres coding style require 
> using it for multiline statements.
> 2) Update documentation according to new features.
> 
> --
> With best regards,
> Alexander Korotkov.


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

Re: [HACKERS] pg_dump: CREATE TABLE + CREATE RULE vs. relreplident

[Tom Lane]

Robert Haas  writes:
> On Thu, Mar 12, 2015 at 12:46 PM, Tom Lane  wrote:
>> Sorry, that's mere historical revisionism.

> Can we please keep this a little more civil?  Saying it that way
> implies bad faith.  You could instead write "I think you are
> mistaken".

Hmm, I take that phrase as being a bit of a joke.  I didn't mean it
to be uncivil, and if Andrew perceives it as such, I apologize.

regards, tom lane


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

Re: [HACKERS] Parallel Seq Scan

[Thom Brown]

On 12 March 2015 at 16:20, Thom Brown  wrote:
> On 12 March 2015 at 15:29, Amit Kapila  wrote:
>> On Thu, Mar 12, 2015 at 8:33 PM, Thom Brown  wrote:
>>>
>>> On 12 March 2015 at 14:46, Amit Kapila  wrote:
>>> > One additional change (we need to SetLatch() in
>>> > HandleParallelMessageInterrupt)
>>> > is done to handle the hang issue reported on parallel-mode thread.
>>> > Without this change it is difficult to verify the patch (will remove
>>> > this
>>> > change
>>> > once new version of parallel-mode patch containing this change will be
>>> > posted).
>>>
>>> Applied parallel-mode-v7.patch and parallel_seqscan_v10.patch, but
>>> getting this error when building:
>>>
>>> gcc -Wall -Wmissing-prototypes -Wpointer-arith
>>> -Wdeclaration-after-statement -Wendif-labels
>>> -Wmissing-format-attribute -Wformat-security -fno-strict-aliasing
>>> -fwrapv -fexcess-precision=standard -O2 -I../../../../src/include
>>> -D_GNU_SOURCE   -c -o brin.o brin.c -MMD -MP -MF .deps/brin.Po
>>> In file included from ../../../../src/include/nodes/execnodes.h:18:0,
>>>  from ../../../../src/include/access/brin.h:14,
>>>  from brin.c:18:
>>> ../../../../src/include/access/heapam.h:119:34: error: unknown type
>>> name ‘ParallelHeapScanDesc’
>>>  extern void heap_parallel_rescan(ParallelHeapScanDesc pscan,
>>> HeapScanDesc scan);
>>>   ^
>>>
>>> Am I missing another patch here?
>>
>> Yes, the below parallel-heap-scan patch.
>> http://www.postgresql.org/message-id/ca+tgmoyjetgeaxuszrona7bdtwzptqexpjntv1gkcavmgsd...@mail.gmail.com
>>
>> Please note that parallel_setup_cost and parallel_startup_cost are
>> still set to zero by default, so you need to set it to higher values
>> if you don't want the parallel plans once parallel_seqscan_degree
>> is set.  I have yet to comeup with default values for them, needs
>> some tests.
>
> Thanks.  Getting a problem:
>
> createdb pgbench
> pgbench -i -s 200 pgbench
>
> CREATE TABLE pgbench_accounts_1 (CHECK (bid = 1)) INHERITS (pgbench_accounts);
> ...
> CREATE TABLE pgbench_accounts_200 (CHECK (bid = 200)) INHERITS
> (pgbench_accounts);
>
> WITH del AS (DELETE FROM pgbench_accounts WHERE bid = 1 RETURNING *)
> INSERT INTO pgbench_accounts_1 SELECT * FROM del;
> ...
> WITH del AS (DELETE FROM pgbench_accounts WHERE bid = 200 RETURNING *)
> INSERT INTO pgbench_accounts_200 SELECT * FROM del;
>
> VACUUM ANALYSE;
>
> # SELECT name, setting FROM pg_settings WHERE name IN
> ('parallel_seqscan_degree','max_worker_processes','seq_page_cost');
>   name   | setting
> -+-
>  max_worker_processes| 20
>  parallel_seqscan_degree | 8
>  seq_page_cost   | 1000
> (3 rows)
>
> # EXPLAIN SELECT DISTINCT bid FROM pgbench_accounts;
> ERROR:  too many dynamic shared memory segments
>
>
> And separately, I've seen this in the logs:
>
> 2015-03-12 16:09:30 GMT [7880]: [4-1] user=,db=,client= LOG:
> registering background worker "parallel worker for PID 7889"
> 2015-03-12 16:09:30 GMT [7880]: [5-1] user=,db=,client= LOG:
> registering background worker "parallel worker for PID 7889"
> 2015-03-12 16:09:30 GMT [7880]: [6-1] user=,db=,client= LOG:
> registering background worker "parallel worker for PID 7889"
> 2015-03-12 16:09:30 GMT [7880]: [7-1] user=,db=,client= LOG:
> registering background worker "parallel worker for PID 7889"
> 2015-03-12 16:09:30 GMT [7880]: [8-1] user=,db=,client= LOG:
> registering background worker "parallel worker for PID 7889"
> 2015-03-12 16:09:30 GMT [7880]: [9-1] user=,db=,client= LOG:
> registering background worker "parallel worker for PID 7889"
> 2015-03-12 16:09:30 GMT [7880]: [10-1] user=,db=,client= LOG:
> registering background worker "parallel worker for PID 7889"
> 2015-03-12 16:09:30 GMT [7880]: [11-1] user=,db=,client= LOG:
> registering background worker "parallel worker for PID 7889"
> 2015-03-12 16:09:30 GMT [7880]: [12-1] user=,db=,client= LOG:
> starting background worker process "parallel worker for PID 7889"
> 2015-03-12 16:09:30 GMT [7880]: [13-1] user=,db=,client= LOG:
> starting background worker process "parallel worker for PID 7889"
> 2015-03-12 16:09:30 GMT [7880]: [14-1] user=,db=,client= LOG:
> starting background worker process "parallel worker for PID 7889"
> 2015-03-12 16:09:30 GMT [7880]: [15-1] user=,db=,client= LOG:
> starting background worker process "parallel worker for PID 7889"
> 2015-03-12 16:09:30 GMT [7880]: [16-1] user=,db=,client= LOG:
> starting background worker process "parallel worker for PID 7889"
> 2015-03-12 16:09:30 GMT [7880]: [17-1] user=,db=,client= LOG:
> starting background worker process "parallel worker for PID 7889"
> 2015-03-12 16:09:30 GMT [7880]: [18-1] user=,db=,client= LOG:
> starting background worker process "parallel worker for PID 7889"
> 2015-03-12 16:09:30 GMT [7880]: [19-1] user=,db=,client= LOG:
> starting background worker process "parallel worker for PID 7889"
> 2015-03-12 16:09:30 GMT [7880]: [20

Re: [HACKERS] pg_dump: CREATE TABLE + CREATE RULE vs. relreplident

[Robert Haas]

On Thu, Mar 12, 2015 at 12:46 PM, Tom Lane  wrote:
>> As it happens it does not; the issue came up originally because of a
>> hack I came up with, and I've never used any pg version so old it didn't
>> have CREATE OR REPLACE VIEW. Nor does it look like the change was ever
>> backpatched (or at least not that far).
>
> Sorry, that's mere historical revisionism.

Can we please keep this a little more civil?  Saying it that way
implies bad faith.  You could instead write "I think you are
mistaken".

-- 
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] pg_dump: CREATE TABLE + CREATE RULE vs. relreplident

[Tom Lane]

Andrew Gierth  writes:
> "Tom" == Tom Lane  writes:
>  Tom> Yeah, possibly.  The existing pg_dump coding dates from before we
>  Tom> had CREATE OR REPLACE VIEW.

> As it happens it does not; the issue came up originally because of a
> hack I came up with, and I've never used any pg version so old it didn't
> have CREATE OR REPLACE VIEW. Nor does it look like the change was ever
> backpatched (or at least not that far).

Sorry, that's mere historical revisionism.  The oldest PG version I still
have in captivity is 7.0, and in it pg_dump does this:

$ createdb db1
CREATE DATABASE
$ psql db1
Welcome to psql, the PostgreSQL interactive terminal.

Type:  \copyright for distribution terms
   \h for help with SQL commands
   \? for help on internal slash commands
   \g or terminate with semicolon to execute query
   \q to quit

db1=# select version();
 version  
--
 PostgreSQL 7.0.3 on hppa2.0-hp-hpux10.20, compiled by gcc 2.95.3
(1 row)

db1=# create table t1 (f1 int, f2 text);
CREATE
db1=# create view v1 as select * from t1;
CREATE 148340 1
db1=# \q
$ pg_dump db1
\connect - postgres
CREATE TABLE "t1" (
"f1" int4,
"f2" text
);
CREATE TABLE "v1" (
"f1" int4,
"f2" text
);
COPY "t1" FROM stdin;
\.
CREATE RULE "_RETv1" AS ON SELECT TO v1 DO INSTEAD SELECT t1.f1, t1.f2 FROM t1;
$ 


Later (in 7.1, looks like) we improved the pg_dump code to dump views as
views, but the underlying ability to dump the ON SELECT rule separately
was still there.  I think what you are remembering is commit
86a069bbed9264daaa85270ece0a2d5959017336, but that just re-enabled the
aboriginal behavior when we discover a circularity involving a view rule.
If I'd had to write actual new dumping code, I probably would not have
done it like that, and might have hit on the CREATE OR REPLACE VIEW
solution instead.

OTOH, some experimenting shows that 7.3 is the oldest version that accepts
the syntax CREATE OR REPLACE VIEW, so at the time we might not have wanted
to use that solution in pg_dump anyway.

regards, tom lane


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

Re: [HACKERS] pg_dump: CREATE TABLE + CREATE RULE vs. relreplident

[Alvaro Herrera]

Andrew Gierth wrote:
> > "Tom" == Tom Lane  writes:

>  Tom> Yeah, possibly.  The existing pg_dump coding dates from before we
>  Tom> had CREATE OR REPLACE VIEW.
> 
> As it happens it does not; the issue came up originally because of a
> hack I came up with, and I've never used any pg version so old it didn't
> have CREATE OR REPLACE VIEW. Nor does it look like the change was ever
> backpatched (or at least not that far).
> 
> http://www.postgresql.org/message-id/20986.1102296...@sss.pgh.pa.us

Wow --- We've had CREATE OR REPLACE VIEW since 2002:

commit 248c67d7ed505d98d3a94cd3954835255317ff16
Author: Tom Lane 
Date:   Mon Sep 2 02:13:02 2002 +

CREATE OR REPLACE VIEW, CREATE OR REPLACE RULE.
Gavin Sherry, Neil Conway, and Tom Lane all got their hands dirty
on this one ...


What is newer is the ability to add columns:

commit ff1ea2173a92dea975d399a4ca25723f83762e55
Author: Bruce Momjian 
Date:   Sat Dec 6 23:22:46 2008 +

Allow CREATE OR REPLACE VIEW to add columns to the _end_ of the view.

Robert Haas

(Andrew's original post is from 2004)

-- 
Álvaro Herrerahttp://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 Scan

[Thom Brown]

On 12 March 2015 at 15:29, Amit Kapila  wrote:
> On Thu, Mar 12, 2015 at 8:33 PM, Thom Brown  wrote:
>>
>> On 12 March 2015 at 14:46, Amit Kapila  wrote:
>> > One additional change (we need to SetLatch() in
>> > HandleParallelMessageInterrupt)
>> > is done to handle the hang issue reported on parallel-mode thread.
>> > Without this change it is difficult to verify the patch (will remove
>> > this
>> > change
>> > once new version of parallel-mode patch containing this change will be
>> > posted).
>>
>> Applied parallel-mode-v7.patch and parallel_seqscan_v10.patch, but
>> getting this error when building:
>>
>> gcc -Wall -Wmissing-prototypes -Wpointer-arith
>> -Wdeclaration-after-statement -Wendif-labels
>> -Wmissing-format-attribute -Wformat-security -fno-strict-aliasing
>> -fwrapv -fexcess-precision=standard -O2 -I../../../../src/include
>> -D_GNU_SOURCE   -c -o brin.o brin.c -MMD -MP -MF .deps/brin.Po
>> In file included from ../../../../src/include/nodes/execnodes.h:18:0,
>>  from ../../../../src/include/access/brin.h:14,
>>  from brin.c:18:
>> ../../../../src/include/access/heapam.h:119:34: error: unknown type
>> name ‘ParallelHeapScanDesc’
>>  extern void heap_parallel_rescan(ParallelHeapScanDesc pscan,
>> HeapScanDesc scan);
>>   ^
>>
>> Am I missing another patch here?
>
> Yes, the below parallel-heap-scan patch.
> http://www.postgresql.org/message-id/ca+tgmoyjetgeaxuszrona7bdtwzptqexpjntv1gkcavmgsd...@mail.gmail.com
>
> Please note that parallel_setup_cost and parallel_startup_cost are
> still set to zero by default, so you need to set it to higher values
> if you don't want the parallel plans once parallel_seqscan_degree
> is set.  I have yet to comeup with default values for them, needs
> some tests.

Thanks.  Getting a problem:

createdb pgbench
pgbench -i -s 200 pgbench

CREATE TABLE pgbench_accounts_1 (CHECK (bid = 1)) INHERITS (pgbench_accounts);
...
CREATE TABLE pgbench_accounts_200 (CHECK (bid = 200)) INHERITS
(pgbench_accounts);

WITH del AS (DELETE FROM pgbench_accounts WHERE bid = 1 RETURNING *)
INSERT INTO pgbench_accounts_1 SELECT * FROM del;
...
WITH del AS (DELETE FROM pgbench_accounts WHERE bid = 200 RETURNING *)
INSERT INTO pgbench_accounts_200 SELECT * FROM del;

VACUUM ANALYSE;

# SELECT name, setting FROM pg_settings WHERE name IN
('parallel_seqscan_degree','max_worker_processes','seq_page_cost');
  name   | setting
-+-
 max_worker_processes| 20
 parallel_seqscan_degree | 8
 seq_page_cost   | 1000
(3 rows)

# EXPLAIN SELECT DISTINCT bid FROM pgbench_accounts;
ERROR:  too many dynamic shared memory segments


And separately, I've seen this in the logs:

2015-03-12 16:09:30 GMT [7880]: [4-1] user=,db=,client= LOG:
registering background worker "parallel worker for PID 7889"
2015-03-12 16:09:30 GMT [7880]: [5-1] user=,db=,client= LOG:
registering background worker "parallel worker for PID 7889"
2015-03-12 16:09:30 GMT [7880]: [6-1] user=,db=,client= LOG:
registering background worker "parallel worker for PID 7889"
2015-03-12 16:09:30 GMT [7880]: [7-1] user=,db=,client= LOG:
registering background worker "parallel worker for PID 7889"
2015-03-12 16:09:30 GMT [7880]: [8-1] user=,db=,client= LOG:
registering background worker "parallel worker for PID 7889"
2015-03-12 16:09:30 GMT [7880]: [9-1] user=,db=,client= LOG:
registering background worker "parallel worker for PID 7889"
2015-03-12 16:09:30 GMT [7880]: [10-1] user=,db=,client= LOG:
registering background worker "parallel worker for PID 7889"
2015-03-12 16:09:30 GMT [7880]: [11-1] user=,db=,client= LOG:
registering background worker "parallel worker for PID 7889"
2015-03-12 16:09:30 GMT [7880]: [12-1] user=,db=,client= LOG:
starting background worker process "parallel worker for PID 7889"
2015-03-12 16:09:30 GMT [7880]: [13-1] user=,db=,client= LOG:
starting background worker process "parallel worker for PID 7889"
2015-03-12 16:09:30 GMT [7880]: [14-1] user=,db=,client= LOG:
starting background worker process "parallel worker for PID 7889"
2015-03-12 16:09:30 GMT [7880]: [15-1] user=,db=,client= LOG:
starting background worker process "parallel worker for PID 7889"
2015-03-12 16:09:30 GMT [7880]: [16-1] user=,db=,client= LOG:
starting background worker process "parallel worker for PID 7889"
2015-03-12 16:09:30 GMT [7880]: [17-1] user=,db=,client= LOG:
starting background worker process "parallel worker for PID 7889"
2015-03-12 16:09:30 GMT [7880]: [18-1] user=,db=,client= LOG:
starting background worker process "parallel worker for PID 7889"
2015-03-12 16:09:30 GMT [7880]: [19-1] user=,db=,client= LOG:
starting background worker process "parallel worker for PID 7889"
2015-03-12 16:09:30 GMT [7880]: [20-1] user=,db=,client= LOG:  worker
process: parallel worker for PID 7889 (PID 7913) exited with exit code
0
2015-03-12 16:09:30 GMT [7880]: [21-1] user=,db=,client= LOG:
unregistering background worker "parallel

Re: [HACKERS] EvalPlanQual behaves oddly for FDW queries involving system columns

[Tom Lane]

Etsuro Fujita  writes:
> On 2015/03/12 13:35, Tom Lane wrote:
>> I don't like the execMain.c changes much at all.  They look somewhat
>> like they're intended to allow foreign tables to adopt a different
>> locking strategy,

> I didn't intend such a thing.  My intention is, foreign tables have
> markType = ROW_MARK_COPY as ever, but I might not have correctly
> understood what you pointed out.  Could you elaborate on that?

I think the real fix as far as postgres_fdw is concerned is in fact
to let it adopt a different ROW_MARK strategy, since it has meaningful
ctid values.  However, that is not a one-size-fits-all answer.  The
fundamental problem I've got with this patch is that it's trying to
impose some one-size-fits-all assumptions on all FDWs about whether
ctids are meaningful; which is wrong, not to mention not backwards
compatible.

>> I don't see the need for the change in nodeForeignscan.c.  If the FDW has
>> returned a physical tuple, it can fix that for itself, while if it has
>> returned a virtual tuple, the ctid will be garbage in any case.

> If you leave nodeForeignscan.c unchanged, file_fdw would cause the
> problem that I pointed out upthread.  Here is an example.

But that's self-inflicted damage, because in fact ctid correctly shows
as invalid in this case in HEAD, without this patch.

The tableoid problem can be fixed much less invasively as per the attached
patch.  I think that we should continue to assume that ctid is not
meaningful (and hence should read as (4294967295,0)) in FDWs that use
ROW_MARK_COPY, and press forward on fixing the locking issues for
postgres_fdw by letting it use ROW_MARK_REFERENCE or something close to
that.  That would also cause ctid to read properly for rows from
postgres_fdw.

regards, tom lane

diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c
index 33b172b..5a1c3b3 100644
*** a/src/backend/executor/execMain.c
--- b/src/backend/executor/execMain.c
*** EvalPlanQualFetchRowMarks(EPQState *epqs
*** 2438,2444 
  			/* build a temporary HeapTuple control structure */
  			tuple.t_len = HeapTupleHeaderGetDatumLength(td);
  			ItemPointerSetInvalid(&(tuple.t_self));
! 			tuple.t_tableOid = InvalidOid;
  			tuple.t_data = td;
  
  			/* copy and store tuple */
--- 2438,2445 
  			/* build a temporary HeapTuple control structure */
  			tuple.t_len = HeapTupleHeaderGetDatumLength(td);
  			ItemPointerSetInvalid(&(tuple.t_self));
! 			tuple.t_tableOid = getrelid(erm->rti,
! 		epqstate->estate->es_range_table);
  			tuple.t_data = td;
  
  			/* copy and store tuple */

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

Re: [HACKERS] pg_dump: CREATE TABLE + CREATE RULE vs. relreplident

[Andrew Gierth]

> "Tom" == Tom Lane  writes:

 >> I've wondered for a while whether this wouldn't have been better
 >> handled as:

 >> create view qwr(colnames...) as select null::type1, null::type2, ...;
 >> /* ... */
 >> create or replace view qwr as ...;

 Tom> Yeah, possibly.  The existing pg_dump coding dates from before we
 Tom> had CREATE OR REPLACE VIEW.

As it happens it does not; the issue came up originally because of a
hack I came up with, and I've never used any pg version so old it didn't
have CREATE OR REPLACE VIEW. Nor does it look like the change was ever
backpatched (or at least not that far).

http://www.postgresql.org/message-id/20986.1102296...@sss.pgh.pa.us

Of course, at the time I myself didn't think of using a view rather than
a table for the initial creation; I was focused on rules because that
was the only way to get updateable views then. So arguably it is at
least partly my fault.

 Tom> But we'll have to live with pg_dump files that do this for the
 Tom> indefinite future, so I agree some fix is needed on the backend
 Tom> side.

Certainly.

-- 
Andrew (irc:RhodiumToad)


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

[Amit Kapila]

On Thu, Mar 12, 2015 at 8:33 PM, Thom Brown  wrote:
>
> On 12 March 2015 at 14:46, Amit Kapila  wrote:
> > One additional change (we need to SetLatch() in
> > HandleParallelMessageInterrupt)
> > is done to handle the hang issue reported on parallel-mode thread.
> > Without this change it is difficult to verify the patch (will remove
this
> > change
> > once new version of parallel-mode patch containing this change will be
> > posted).
>
> Applied parallel-mode-v7.patch and parallel_seqscan_v10.patch, but
> getting this error when building:
>
> gcc -Wall -Wmissing-prototypes -Wpointer-arith
> -Wdeclaration-after-statement -Wendif-labels
> -Wmissing-format-attribute -Wformat-security -fno-strict-aliasing
> -fwrapv -fexcess-precision=standard -O2 -I../../../../src/include
> -D_GNU_SOURCE   -c -o brin.o brin.c -MMD -MP -MF .deps/brin.Po
> In file included from ../../../../src/include/nodes/execnodes.h:18:0,
>  from ../../../../src/include/access/brin.h:14,
>  from brin.c:18:
> ../../../../src/include/access/heapam.h:119:34: error: unknown type
> name ‘ParallelHeapScanDesc’
>  extern void heap_parallel_rescan(ParallelHeapScanDesc pscan,
> HeapScanDesc scan);
>   ^
>
> Am I missing another patch here?

Yes, the below parallel-heap-scan patch.
http://www.postgresql.org/message-id/ca+tgmoyjetgeaxuszrona7bdtwzptqexpjntv1gkcavmgsd...@mail.gmail.com

Please note that parallel_setup_cost and parallel_startup_cost are
still set to zero by default, so you need to set it to higher values
if you don't want the parallel plans once parallel_seqscan_degree
is set.  I have yet to comeup with default values for them, needs
some tests.



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

Re: [HACKERS] assessing parallel-safety

[Robert Haas]

[ deferring responses to some points until a later time ]

On Thu, Feb 19, 2015 at 1:19 AM, Noah Misch  wrote:
>> This seems backwards to me.  If some hypothetical selectivity
>> estimator were PROPARALLEL_UNSAFE, then any operator that uses that
>> function would also need to be PROPARALLEL_UNSAFE.
>
> It's fair to have a PROPARALLEL_UNSAFE estimator for a PROPARALLEL_SAFE
> function, because the planning of a parallel query is often not itself done in
> parallel mode.  In that case, "SELECT * FROM tablename WHERE colname OP 0"
> might use a parallel seqscan but fail completely if called from inside a
> function running in parallel mode.  That is to say, an affected query can
> itself use parallelism, but placing the query in a function makes the function
> PROPARALLEL_UNSAFE.  Surprising, but not wrong.
>
> Rereading my previous message, I failed to make the bottom line clear: I
> recommend marking eqsel etc. PROPARALLEL_UNSAFE but *not* checking an
> estimator's proparallel before calling it in the planner.

But what do these functions do that is actually unsafe?

>> > - Assuming you don't want to propagate XactLastRecEnd from the slave back 
>> > to
>> >   the master, restrict XLogInsert() during parallel mode.  Restrict 
>> > functions
>> >   that call it, including pg_create_restore_point, pg_switch_xlog and
>> >   pg_stop_backup.
>>
>> Hmm.  Altogether prohibiting XLogInsert() in parallel mode seems
>> unwise, because it would foreclose heap_page_prune_opt() in workers.
>> I realize there's separate conversation about whether pruning during
>> SELECT queries is good policy, but in the interested of separating
>> mechanism from policy, and in the sure knowledge that allowing at
>> least some writes in parallel mode is certainly going to be something
>> people will want, it seems better to look into propagating
>> XactLastRecEnd.
>
> Good points; that works for me.

The key design decision here seems to be this: How eagerly do we need
to synchronize XactLastRecEnd?  What exactly goes wrong if it isn't
synchronized?  For example, if the value were absolutely critical in
all circumstances, one could imagine storing a shared XactLastRecEnd
in shared memory.  This doesn't appear to be the case: the main
requirement is that we definitely need an up-to-date value at commit
time.  Also, at abort time, we don't really the value for anything
critical, but it's worth kicking the WAL writer so that any
accumulated WAL gets flushed.

Here's an incremental patch - which I will incorporate into the
parallel mode patch if it seems about right to you - that tries to
tackle all this.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
commit 26bd8f366415906a67abd49824fe3a980d5d4555
Author: Robert Haas 
Date:   Thu Mar 12 11:09:19 2015 -0400

Synchronize XactLastRecEnd.

diff --git a/src/backend/access/transam/parallel.c b/src/backend/access/transam/parallel.c
index 225ec89..a97c899 100644
--- a/src/backend/access/transam/parallel.c
+++ b/src/backend/access/transam/parallel.c
@@ -15,6 +15,7 @@
 #include "postgres.h"
 
 #include "access/xact.h"
+#include "access/xlog.h"
 #include "access/parallel.h"
 #include "commands/async.h"
 #include "libpq/libpq.h"
@@ -73,10 +74,15 @@ typedef struct FixedParallelState
 	/* Entrypoint for parallel workers. */
 	parallel_worker_main_type	entrypoint;
 
-	/* Track whether workers have attached. */
+	/* Mutex protects remaining fiedlds. */
 	slock_t		mutex;
+
+	/* Track whether workers have attached. */
 	int			workers_expected;
 	int			workers_attached;
+
+	/* Maximum XactLastRecEnd of any worker. */
+	XLogRecPtr	last_xlog_end;
 } FixedParallelState;
 
 /*
@@ -90,6 +96,9 @@ int ParallelWorkerNumber = -1;
 /* Is there a parallel message pending which we need to receive? */
 bool ParallelMessagePending = false;
 
+/* Pointer to our fixed parallel state. */
+static FixedParallelState *MyFixedParallelState;
+
 /* List of active parallel contexts. */
 static dlist_head pcxt_list = DLIST_STATIC_INIT(pcxt_list);
 
@@ -257,6 +266,7 @@ InitializeParallelDSM(ParallelContext *pcxt)
 	SpinLockInit(&fps->mutex);
 	fps->workers_expected = pcxt->nworkers;
 	fps->workers_attached = 0;
+	fps->last_xlog_end = 0;
 	shm_toc_insert(pcxt->toc, PARALLEL_KEY_FIXED, fps);
 
 	/* Serialize GUC state to dynamic shared memory. */
@@ -398,6 +408,9 @@ LaunchParallelWorkers(ParallelContext *pcxt)
  * important to call this function afterwards.  We must not miss any errors
  * the workers may have thrown during the parallel operation, or any that they
  * may yet throw while shutting down.
+ *
+ * Also, we want to update our notion of XactLastRecEnd based on worker
+ * feedback.
  */
 void
 WaitForParallelWorkersToFinish(ParallelContext *pcxt)
@@ -431,6 +444,15 @@ WaitForParallelWorkersToFinish(ParallelContext *pcxt)
 		WaitLatch(&MyProc->procLatch, WL_LATCH_SET, -1);
 		ResetLatch(&MyProc->procLatch);
 	}
+
+	if (pcxt->toc != NULL)
+	{
+		FixedParallelState *fps;
+
+		fps

[HACKERS] pg_xlog_replay_resume() considered armed and dangerous

[Andres Freund]

Hi,

I think it's quite confusing that a function named
pg_xlog_replay_resume() can cause a node to be promoted.

That this is happened is kind of documented in the recovery.conf section
of the manual:
"The intended use of the pause setting is to allow queries to be executed
against the database to check if this recovery target is the most
desirable point for recovery. The paused state can be resumed by using
pg_xlog_replay_resume() (see Table 9-69), which then causes recovery to
end. If this recovery target is not the desired stopping point, then
shut down the server, change the recovery target settings to a later
target and restart to continue recovery."

But it's not mentioned at all in the section about the functions:
http://www.postgresql.org/docs/devel/static/functions-admin.html#FUNCTIONS-RECOVERY-CONTROL-TABLE

Promotion only happens when a node is paused due to a recovery target
setting, and not when it's stopped due to pg_xlog_replay_pause().

I think this, at the very least, needs a big caveat in the documentation
of the resume function. But a different API would probably be
better. I'd actually argue that for now pg_xlog_replay_resume() should
refuse to work if paused due to a recovery target. Promotion should be
done via the normal promotion methods.

Greetings,

Andres Freund

-- 
 Andres Freund 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] Parallel Seq Scan

[Thom Brown]

On 12 March 2015 at 14:46, Amit Kapila  wrote:
> One additional change (we need to SetLatch() in
> HandleParallelMessageInterrupt)
> is done to handle the hang issue reported on parallel-mode thread.
> Without this change it is difficult to verify the patch (will remove this
> change
> once new version of parallel-mode patch containing this change will be
> posted).

Applied parallel-mode-v7.patch and parallel_seqscan_v10.patch, but
getting this error when building:

gcc -Wall -Wmissing-prototypes -Wpointer-arith
-Wdeclaration-after-statement -Wendif-labels
-Wmissing-format-attribute -Wformat-security -fno-strict-aliasing
-fwrapv -fexcess-precision=standard -O2 -I../../../../src/include
-D_GNU_SOURCE   -c -o brin.o brin.c -MMD -MP -MF .deps/brin.Po
In file included from ../../../../src/include/nodes/execnodes.h:18:0,
 from ../../../../src/include/access/brin.h:14,
 from brin.c:18:
../../../../src/include/access/heapam.h:119:34: error: unknown type
name ‘ParallelHeapScanDesc’
 extern void heap_parallel_rescan(ParallelHeapScanDesc pscan,
HeapScanDesc scan);
  ^

Am I missing another patch here?

-- 
Thom


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

Re: [HACKERS] recovery_target_action doesn't work for anything but shutdown

[Andres Freund]

On 2015-03-12 15:52:02 +0100, Andres Freund wrote:
> I guess what you actually intended to test was StandbyModeRequested?

Err, EnableHotStandby.

Greetings,

Andres Freund

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


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

[HACKERS] recovery_target_action doesn't work for anything but shutdown

[Andres Freund]

Hi,

Unless I'm missing something recovery_target_action = promote/pause
don't work.

There's the following block of code in readRecoveryCommandFile():
/*
 * Override any inconsistent requests. Not that this is a change
 * of behaviour in 9.5; prior to this we simply ignored a request
 * to pause if hot_standby = off, which was surprising behaviour.
 */
if (recoveryTargetAction == RECOVERY_TARGET_ACTION_PAUSE &&
recoveryTargetActionSet &&
standbyState == STANDBY_DISABLED)
recoveryTargetAction = RECOVERY_TARGET_ACTION_SHUTDOWN;

the problem is that when the recovery command file is read standbyState
will always still be STANDBY_DISABLED. Which makes sense, because we
can't even know we're in recovery before readRecoveryCommandFile().

I guess what you actually intended to test was StandbyModeRequested?

Greetings,

Andres Freund

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


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

Re: [HACKERS] logical column ordering

[Tom Lane]

Alvaro Herrera  writes:
> However, there's a difference between making a query silently given
> different results, and breaking it completely forcing the user to
> re-study how to write it.  I think the latter is better.  In that light
> we should just drop attnum as a column name, and use something else:
> maybe (attidnum, attlognum, attphysnum).  So all queries in the wild
> would be forced to be updated, but we would not silently change
> semantics instead.

Hm.  I'm on board with renaming like that inside the backend, but
I'm very much less excited about forcibly breaking client queries.
I think there is little if any client-side code that will care about
either attidnum or attphysnum, so forcing people to think about it
will just create make-work.

regards, tom lane


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

Re: [HACKERS] pg_dump: CREATE TABLE + CREATE RULE vs. relreplident

[Andres Freund]

Hi,

On 2015-03-12 14:25:24 +0100, Marko Tiikkaja wrote:
> My colleague Per Lejontand brought to my attention that when dumping views
> with circular dependencies from a postgres version older than 9.4 using a
> recent pg_dump, the SQL looks something like the following:
> 
>   create table qwr();
>   create rule "_RETURN" as on select to qwr do instead select;
> 
> In this case the relreplident column in pg_class for the view ends up being
> 'd', instead of the 'n' normally used for views.  Patch to update
> relreplident when turning a table into a view is attached; this makes sure
> that the identity is NOTHING regardless of how the view was created.

I think that's a good idea.

> I consider this a bug fix, and suggest back patching to 9.4.

I agree on backpatching it. Arguably we could additionally avoid
emitting the ALTER TABLE ... REPLICA IDENTITY for views that have
already been created with identity set like this. But I doubt it's worth
it.

Greetings,

Andres Freund

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


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

Re: [HACKERS] pg_dump: CREATE TABLE + CREATE RULE vs. relreplident

[Tom Lane]

Andrew Gierth  writes:
> "Marko" == Marko Tiikkaja  writes:
>  Marko>   create table qwr();
>  Marko>   create rule "_RETURN" as on select to qwr do instead select;

> I've wondered for a while whether this wouldn't have been better handled
> as:

> create view qwr(colnames...) as select null::type1, null::type2, ...;
> /* ... */
> create or replace view qwr as ...;

Yeah, possibly.  The existing pg_dump coding dates from before we had
CREATE OR REPLACE VIEW.

But we'll have to live with pg_dump files that do this for the indefinite
future, so I agree some fix is needed on the backend side.

regards, tom lane


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

Re: [HACKERS] logical column ordering

[Andres Freund]

Hi,

On 2015-03-11 22:16:52 -0400, Tom Lane wrote:
> I agree though that it's worth considering defining pg_attribute.attnum as
> the logical column position so as to minimize the effects on client-side
> code.

I actually wonder if it'd not make more sense to define it as the
physical column number. That'd reduce the invasiveness and risk of the
patch considerably. It means that most existing code doesn't have to be
changed and can just continue to refer to attnum like today. There's
much less risk of it being wrongly used to refer to the physical offset
instead of creation order.  Queries against attnum would still give a
somewhat sane response.

It would make some ALTER TABLE commands a bit more complex if we want to
allow reordering the physical order. But that seems like a much more
localized complexity than previous patches in this thread (although I've
not looked at the last version).

Greetings,

Andres Freund

-- 
 Andres Freund 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] OOM-killer issue when updating a inheritance table which has large number of child tables

[Tom Lane]

chenhj  writes:
> In my test(PG9.3.4), i found when update a parent table which has a large 
> number of child tables, the execute plan will consume lots of memory. And 
> possibly cause OOM.

See
file:///net/sss1/home/postgres/pgsql/doc/src/sgml/html/ddl-partitioning.html#DDL-PARTITIONING-CAVEATS

particularly the last paragraph.

regards, tom lane


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

Re: [HACKERS] shebang for tcl postgresql modules

[Tom Lane]

Jozef Mlich  writes:
> I would like to ask you if is there any reason to pretend tcl scripts
> are shell scripts?

That is the time-honored standard formatting for tcl scripts, see for
example page 2 here:
http://www.tcl.tk/doc/styleGuide.pdf

While we might get away with ignoring that convention for this specific
use-case, it carries a risk of breaking things, eg for people who have
multiple Tcl installations.  I'm disinclined to change it, especially
not if the reason is "rpmdiff is too dumb to recognize tcl scripts".
Somebody's gonna need to fix that anyway.

regards, tom lane


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

Re: [HACKERS] logical column ordering

[Alvaro Herrera]

Tomas Vondra wrote:
> On 12.3.2015 14:17, Alvaro Herrera wrote:
> > Tomas Vondra wrote:
> >> On 12.3.2015 03:16, Tom Lane wrote:
> > 
> >>> I agree though that it's worth considering defining 
> >>> pg_attribute.attnum as the logical column position so as to minimize 
> >>> the effects on client-side code. I doubt there is much stuff 
> >>> client-side that cares about column creation order, but there is 
> >>> plenty that cares about logical column order. OTOH this would 
> >>> introduce confusion into the backend code, since Alvaro's definition 
> >>> of attnum is what most of the backend should care about.
> >>
> >> IMHO reusing attnum for logical column order would actually make it more
> >> complex, especially if we allow users to modify the logical order using
> >> ALTER TABLE. Because if you change it, you have to walk through all the
> >> places where it might be referenced and update those too (say, columns
> >> referenced in indexes and such). Keeping attnum immutable makes this
> >> much easier and simpler.
> > 
> > I think you're misunderstanding. The suggestion, as I understand it,
> > is to rename the attnum column to something else (maybe, say,
> > attidnum), and rename attlognum to attnum. That preserves the
> > existing property that "ORDER BY attnum" gives you the correct view
> > of the table from the point of view of the user. That's very useful
> > because it means clients looking at pg_attribute need less changes,
> > or maybe none at all.
> 
> Hmm ... I understood it as a suggestion to drop attlognum and just
> define (attnum, attphysnum).

Pretty sure it wasn't that.

> > I think this wouldn't be too difficult to implement, because there 
> > aren't that many places that refer to the column-identity attribute
> > by name; most of them just grab the TupleDesc->attrs array in
> > whatever order is appropriate and scan that in a loop. Only a few of
> > these use att->attnum inside the loop --- that's what would need to
> > be changed, and it should be pretty mechanical.
> 
> I think it's way more complicated. We may fix all the pieces of the
> code, but that's not all - attnum is referenced in various system views,
> catalogs and such. For example pg_stats view does this:
> 
>   FROM pg_statistic s JOIN pg_class c ON (c.oid = s.starelid)
> JOIN pg_attribute a ON (c.oid = attrelid AND attnum = s.staattnum)
>   LEFT JOIN pg_namespace n ON (n.oid = c.relnamespace)
>   WHERE NOT attisdropped
> AND has_column_privilege(c.oid, a.attnum, 'select');
> 
> information_schema also uses attnum on many places too.

Those can be fixed with relative ease to refer to attidnum instead.

> I see the catalogs as a kind of public API, and redefining the meaning
> of an existing column this way seems tricky, especially when we
> reference it from other catalogs - I'm pretty sure there's plenty of SQL
> queries in various tools that rely on this.

That's true, but then we've never promised that system catalogs remain
unchanged forever.  That would essentially stop development.

However, there's a difference between making a query silently given
different results, and breaking it completely forcing the user to
re-study how to write it.  I think the latter is better.  In that light
we should just drop attnum as a column name, and use something else:
maybe (attidnum, attlognum, attphysnum).  So all queries in the wild
would be forced to be updated, but we would not silently change
semantics instead.

> Which actually breaks the catalog definition as specified here:
> 
>   http://www.postgresql.org/docs/devel/static/catalog-pg-index.html
> 
> which explicitly says that indkey references pg_attribute.attnum.

That's a simple doc fix.

> But maybe we don't really care about breaking this API and it is a good
> approach - I need to think about it and try it.

Yeah, thanks.


-- 
Álvaro Herrerahttp://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] Install shared libs in lib/ and bin/ with MSVC (Was: install libpq.dll in bin directory on Windows / Cygwin)

[Michael Paquier]

On Wed, Mar 11, 2015 at 10:03 PM, Asif Naeem  wrote:
> Thank you Michael overall v3 patch looks good to me, There is one
> observation that it is not installing following lib files that are required
> for dev work i.e.

Thanks for your input.

>> inst\lib\libpq.lib
>> inst\lib\libpgtypes.lib
>> inst\lib\libecpg_compat.lib
>> inst\lib\libecpg.lib
>
> Please do let me know if I missed something but was not there a need to
> avoid installing related .dll files in lib (duplicate) along with bin
> directory e.g?

Yeas, right. Sorry for missing something like that. In HEAD, the
following things are installed regarding the shared libraries:
- In lib/, all .dll and .lib
- In bin/, only libpq.dll

So I have recoded the patch to use an hash of arrays (makes the code
more readable IMO) to be able to track more easily what to install
where, and process now does the following for shared libraries:
- In lib/, install all .dll and .lib
- In bin/, install all .dll

I hope that's fine to you.
Regards,
-- 
Michael
diff --git a/src/tools/msvc/Install.pm b/src/tools/msvc/Install.pm
index eba9aa0..c35a9dd 100644
--- a/src/tools/msvc/Install.pm
+++ b/src/tools/msvc/Install.pm
@@ -91,7 +91,6 @@ sub Install
 	}
 
 	CopySolutionOutput($conf, $target);
-	lcopy($target . '/lib/libpq.dll', $target . '/bin/libpq.dll');
 	my $sample_files = [];
 	my @top_dir  = ("src");
 	@top_dir = ("src\\bin", "src\\interfaces") if ($insttype eq "client");
@@ -108,12 +107,8 @@ sub Install
 		$target . '/lib/',
 		"$conf\\",
 		"postgres\\postgres.lib",
-		"libpq\\libpq.lib",
-		"libecpg\\libecpg.lib",
 		"libpgcommon\\libpgcommon.lib",
-		"libpgport\\libpgport.lib",
-		"libpgtypes\\libpgtypes.lib",
-		"libecpg_compat\\libecpg_compat.lib");
+		"libpgport\\libpgport.lib");
 	CopyContribFiles($config, $target);
 	CopyIncludeFiles($target);
 
@@ -236,8 +231,16 @@ sub CopySolutionOutput
 	while ($sln =~ $rem)
 	{
 		my $pf = $1;
-		my $dir;
-		my $ext;
+
+		# Hash of array to list things to install. This has the
+		# shape similar to that for each project to indicate
+		# what should be installed where. The hash key indicates
+		# the installation location, and the array elements define
+		# the file type to install:
+		# 'bin' => [ 'dll', 'lib' ]
+		# 'lib' => [ 'lib' ]
+		my %install_list;
+		my $is_sharedlib = 0;
 
 		$sln =~ s/$rem//;
 
@@ -247,22 +250,47 @@ sub CopySolutionOutput
 
 		my $proj = read_file("$pf.$vcproj")
 		  || croak "Could not open $pf.$vcproj\n";
+
+		# Check if this project uses a shared library by looking if
+		# SO_MAJOR_VERSION is defined in its Makefile, whose path
+		# can be found using the resource file of this project.
+		if (($vcproj eq 'vcxproj' &&
+			 $proj =~ qr{ResourceCompile\s*Include="([^"]+)"}) ||
+			($vcproj eq 'vcproj' &&
+			 $proj =~ qr{File\s*RelativePath="([^\"]+)\.rc"}))
+		{
+			my $projpath = dirname($1);
+			my $mfname = -e "$projpath/GNUmakefile" ?
+"$projpath/GNUmakefile" : "$projpath/Makefile";
+			my $mf = read_file($mfname) ||
+croak "Could not open $mfname\n";
+
+			if ($mf =~ /^SO_MAJOR_VERSION\s*=\s*(.*)$/mg)
+			{
+$is_sharedlib = 1;
+			}
+		}
+
 		if ($vcproj eq 'vcproj' && $proj =~ qr{ConfigurationType="([^"]+)"})
 		{
 			if ($1 == 1)
 			{
-$dir = "bin";
-$ext = "exe";
+push( @{ $install_list { 'bin' } }, "exe");
 			}
 			elsif ($1 == 2)
 			{
-$dir = "lib";
-$ext = "dll";
+push( @{ $install_list { 'lib' } }, "dll");
+if ($is_sharedlib)
+{
+	push( @{ $install_list { 'bin' } }, "dll");
+	push( @{ $install_list { 'lib' } }, "lib");
+}
 			}
 			else
 			{
 
-# Static lib, such as libpgport, only used internally during build, don't install
+# Static libraries, such as libpgport, only used internally
+# during build, don't install.
 next;
 			}
 		}
@@ -271,18 +299,21 @@ sub CopySolutionOutput
 		{
 			if ($1 eq 'Application')
 			{
-$dir = "bin";
-$ext = "exe";
+push( @{ $install_list { 'bin' } }, "exe");
 			}
 			elsif ($1 eq 'DynamicLibrary')
 			{
-$dir = "lib";
-$ext = "dll";
+push( @{ $install_list { 'lib' } }, "dll");
+if ($is_sharedlib)
+{
+	push( @{ $install_list { 'bin' } }, "dll");
+	push( @{ $install_list { 'lib' } }, "lib");
+}
 			}
 			else# 'StaticLibrary'
 			{
-
-# Static lib, such as libpgport, only used internally during build, don't install
+# Static lib, such as libpgport, only used internally
+# during build, don't install.
 next;
 			}
 		}
@@ -290,8 +321,16 @@ sub CopySolutionOutput
 		{
 			croak "Could not parse $pf.$vcproj\n";
 		}
-		lcopy("$conf\\$pf\\$pf.$ext", "$target\\$dir\\$pf.$ext")
-		  || croak "Could not copy $pf.$ext\n";
+
+		# Install each element
+		foreach my $dir ( keys %install_list )
+		{
+			foreach my $ext ( @{ $install_list{ $dir } } )
+			{
+lcopy("$conf\\$pf\\$pf.$ext", "$target\\$dir\\$pf.$ext")
+	|| croak "Could not copy $pf.$ext\n";
+			}
+		}
 		lcopy("$conf\\$pf\\$pf.pdb", "$target\\symbols\\$pf.pd

Re: [HACKERS] logical column ordering

[Tomas Vondra]

On 12.3.2015 14:17, Alvaro Herrera wrote:
> Tomas Vondra wrote:
>> On 12.3.2015 03:16, Tom Lane wrote:
> 
>>> I agree though that it's worth considering defining 
>>> pg_attribute.attnum as the logical column position so as to minimize 
>>> the effects on client-side code. I doubt there is much stuff 
>>> client-side that cares about column creation order, but there is 
>>> plenty that cares about logical column order. OTOH this would 
>>> introduce confusion into the backend code, since Alvaro's definition 
>>> of attnum is what most of the backend should care about.
>>
>> IMHO reusing attnum for logical column order would actually make it more
>> complex, especially if we allow users to modify the logical order using
>> ALTER TABLE. Because if you change it, you have to walk through all the
>> places where it might be referenced and update those too (say, columns
>> referenced in indexes and such). Keeping attnum immutable makes this
>> much easier and simpler.
> 
> I think you're misunderstanding. The suggestion, as I understand it,
> is to rename the attnum column to something else (maybe, say,
> attidnum), and rename attlognum to attnum. That preserves the
> existing property that "ORDER BY attnum" gives you the correct view
> of the table from the point of view of the user. That's very useful
> because it means clients looking at pg_attribute need less changes,
> or maybe none at all.

Hmm ... I understood it as a suggestion to drop attlognum and just
define (attnum, attphysnum).

> I think this wouldn't be too difficult to implement, because there 
> aren't that many places that refer to the column-identity attribute
> by name; most of them just grab the TupleDesc->attrs array in
> whatever order is appropriate and scan that in a loop. Only a few of
> these use att->attnum inside the loop --- that's what would need to
> be changed, and it should be pretty mechanical.

I think it's way more complicated. We may fix all the pieces of the
code, but that's not all - attnum is referenced in various system views,
catalogs and such. For example pg_stats view does this:

  FROM pg_statistic s JOIN pg_class c ON (c.oid = s.starelid)
JOIN pg_attribute a ON (c.oid = attrelid AND attnum = s.staattnum)
  LEFT JOIN pg_namespace n ON (n.oid = c.relnamespace)
  WHERE NOT attisdropped
AND has_column_privilege(c.oid, a.attnum, 'select');

information_schema also uses attnum on many places too.

I see the catalogs as a kind of public API, and redefining the meaning
of an existing column this way seems tricky, especially when we
reference it from other catalogs - I'm pretty sure there's plenty of SQL
queries in various tools that rely on this. Just google for "pg_indexes
indkeys unnest" and you'll find posts like this one from Craig:


http://stackoverflow.com/questions/18121103/how-to-get-the-index-column-orderasc-desc-nulls-first-from-postgresql

specifically tell people to do this:

SELECT
...
FROM (
  SELECT
pg_class.relname,
...
unnest(pg_index.indkey) AS k
  FROM pg_index
  INNER JOIN pg_class ON pg_index.indexrelid = pg_class.oid
) i
...
INNER JOIN pg_attribute ON (pg_attribute.attrelid = i.indrelid
AND pg_attribute.attnum = k);

which specifically tells people to match attnum vs. indkeys. If we
redefine the meaning of attnum, and instead match indkeys against a
different column (say, attidnum), all those queries will be broken.

Which actually breaks the catalog definition as specified here:

  http://www.postgresql.org/docs/devel/static/catalog-pg-index.html

which explicitly says that indkey references pg_attribute.attnum.

But maybe we don't really care about breaking this API and it is a good
approach - I need to think about it and try it.

-- 
Tomas Vondrahttp://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] OOM-killer issue when updating a inheritance table which has large number of child tables

[David Fetter]

On Thu, Mar 12, 2015 at 06:55:48PM +0800, chenhj wrote:
> Hi
> 
> In my test(PG9.3.4), i found when update a parent table which has a
> large number of child tables, the execute plan will consume lots of
> memory. And possibly cause OOM.

At the moment, partitioning into thousands of tables is not supported.

If you can reproduce the problem in PostgreSQL 9.3.6, or whichever
happens to be the most recent minor version by the time you do the
test, that will help.

Just generally, it helps to provide a complete test case which
reproduces the problem if at all possible.

Cheers,
David.
-- 
David Fetter  http://fetter.org/
Phone: +1 415 235 3778  AIM: dfetter666  Yahoo!: dfetter
Skype: davidfetter  XMPP: david.fet...@gmail.com

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


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

Re: [HACKERS] pg_dump: CREATE TABLE + CREATE RULE vs. relreplident

[Andrew Gierth]

> "Marko" == Marko Tiikkaja  writes:

 Marko> Hi,

 Marko> My colleague Per Lejontand brought to my attention that when
 Marko> dumping views with circular dependencies from a postgres version
 Marko> older than 9.4 using a recent pg_dump, the SQL looks something
 Marko> like the following:

 Marko>   create table qwr();
 Marko>   create rule "_RETURN" as on select to qwr do instead select;

I've wondered for a while whether this wouldn't have been better handled
as:

create view qwr(colnames...) as select null::type1, null::type2, ...;
/* ... */
create or replace view qwr as ...;

-- 
Andrew (irc:RhodiumToad)


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

[HACKERS] pg_dump: CREATE TABLE + CREATE RULE vs. relreplident

[Marko Tiikkaja]

Hi,

My colleague Per Lejontand brought to my attention that when dumping 
views with circular dependencies from a postgres version older than 9.4 
using a recent pg_dump, the SQL looks something like the following:


  create table qwr();
  create rule "_RETURN" as on select to qwr do instead select;

In this case the relreplident column in pg_class for the view ends up 
being 'd', instead of the 'n' normally used for views.  Patch to update 
relreplident when turning a table into a view is attached; this makes 
sure that the identity is NOTHING regardless of how the view was created.


I consider this a bug fix, and suggest back patching to 9.4.


.m
diff --git a/src/backend/rewrite/rewriteDefine.c 
b/src/backend/rewrite/rewriteDefine.c
index f540432..a88d73e 100644
--- a/src/backend/rewrite/rewriteDefine.c
+++ b/src/backend/rewrite/rewriteDefine.c
@@ -597,6 +597,7 @@ DefineQueryRewrite(char *rulename,
classForm->relhaspkey = false;
classForm->relfrozenxid = InvalidTransactionId;
classForm->relminmxid = InvalidMultiXactId;
+   classForm->relreplident = REPLICA_IDENTITY_NOTHING;
 
simple_heap_update(relationRelation, &classTup->t_self, 
classTup);
CatalogUpdateIndexes(relationRelation, classTup);

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

Re: [HACKERS] logical column ordering

[Alvaro Herrera]

Tomas Vondra wrote:
> On 12.3.2015 03:16, Tom Lane wrote:

> > I agree though that it's worth considering defining 
> > pg_attribute.attnum as the logical column position so as to minimize 
> > the effects on client-side code. I doubt there is much stuff 
> > client-side that cares about column creation order, but there is 
> > plenty that cares about logical column order. OTOH this would 
> > introduce confusion into the backend code, since Alvaro's definition 
> > of attnum is what most of the backend should care about.
> 
> IMHO reusing attnum for logical column order would actually make it more
> complex, especially if we allow users to modify the logical order using
> ALTER TABLE. Because if you change it, you have to walk through all the
> places where it might be referenced and update those too (say, columns
> referenced in indexes and such). Keeping attnum immutable makes this
> much easier and simpler.

I think you're misunderstanding.  The suggestion, as I understand it, is
to rename the attnum column to something else (maybe, say, attidnum),
and rename attlognum to attnum.  That preserves the existing property
that "ORDER BY attnum" gives you the correct view of the table from the
point of view of the user.  That's very useful because it means clients
looking at pg_attribute need less changes, or maybe none at all.

I think this wouldn't be too difficult to implement, because there
aren't that many places that refer to the column-identity attribute by
name; most of them just grab the TupleDesc->attrs array in whatever
order is appropriate and scan that in a loop.  Only a few of these use
att->attnum inside the loop --- that's what would need to be changed,
and it should be pretty mechanical.

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


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

Re: [HACKERS] logical column ordering

[Tomas Vondra]

On 12.3.2015 03:16, Tom Lane wrote:
> Peter Eisentraut  writes:
>> Side idea: Let attnum be the logical number, introduce attphysnum 
>> as the storage position, and add an oid to pg_attribute as the 
>> eternal identifier.
> 
>> That way you avoid breaking pretty much all user code that looks at
>> pg_attribute, which will probably do something like ORDER BY 
>> attnum.
> 
>> Also, one could get rid of all sorts of ugly code that works around
>> the lack of an oid in pg_attribute, such as in the dependency
>> tracking.
> 
> I think using an OID would break more stuff than it fixes in 
> dependency tracking; in particular you would now need an explicit 
> dependency link from each column to its table, because the 
> "sub-object" knowledge would no longer work. In any case this patch 
> is going to be plenty big enough already without saddling it with a 
> major rewrite of the dependency system.

Exactly. I believe Alvaro considered that option in the past.

> I agree though that it's worth considering defining 
> pg_attribute.attnum as the logical column position so as to minimize 
> the effects on client-side code. I doubt there is much stuff 
> client-side that cares about column creation order, but there is 
> plenty that cares about logical column order. OTOH this would 
> introduce confusion into the backend code, since Alvaro's definition 
> of attnum is what most of the backend should care about.

IMHO reusing attnum for logical column order would actually make it more
complex, especially if we allow users to modify the logical order using
ALTER TABLE. Because if you change it, you have to walk through all the
places where it might be referenced and update those too (say, columns
referenced in indexes and such). Keeping attnum immutable makes this
much easier and simpler.

regards

-- 
Tomas Vondrahttp://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] alter user/role CURRENT_USER

[Kyotaro HORIGUCHI]

Thank you for completing this and very sorry not to respond these
days.

I understood that it is committed after I noticed that rebasing
my code failed..

Although after committed, I found some issues as I looked on
it. Please forgive me to comment it now after all this time.

Several changes in docs according to the changed syntax and one
change in code itself to allow CURRENT_USER in GRANT  TO
 syntax.


At Mon, 9 Mar 2015 15:50:32 -0300, Alvaro Herrera  
wrote in <20150309185032.gq3...@alvh.no-ip.org>
> Alvaro Herrera wrote:
> 
> > With this patch applied, doing
> > \h ALTER ROLE
> > in psql looks quite odd: note how wide it has become.  Maybe we should
> > be doing this differently?  (Hmm, why don't we accept ALL in the first SET
> > line?  Maybe that's just a mistake and the four lines should be all
> > identical in the first half ...)
> 
> I have fixed the remaining issues, completed the doc changes, and
> pushed.  Given the lack of feedback I had to follow my gut on the best
> way to change the docs.  I added the regression test you submitted with
> some additional changes, mainly to make sure they don't fail immediately
> when other databases exist; maybe some more concurrency or platform
> issues will show up there, but let's see what the buildfarm says.
> 
> Thanks Horiguchi-san for the patch and everyone for the reviews.  (It's
> probably worthwhile giving things an extra look.)


| =# alter role current_user rename to "PubLic";
| ERROR:  CURRENT_USER cannot be used as a role name
| LINE 1: alter role current_user rename to "PubLic";
|^

The error message sounds somewhat different from the intention. I
think the following message would be clearer.

| ERROR:  CURRENT_USER cannot be used as a role name here


The document sql-altergroup.html says

| ALTER GROUP role_specification ADD USER user_name [, ... ]

But current_user is also usable in user_name list. So the doc
should be as following, but it would not be necessary to be fixed
because it is an obsolete commnand..

| ALTER GROUP role_specification ADD USER role_specification [, ... ]

"ALTER GROUP role_spec ADD/DROP USER role_spec" is naturally
denied so I think no additional description is needed.


sql-alterpolicy.html

"ALTER POLICY name ON table_name TO" also accepts current_user
and so as the role to which the policy applies.

# As a different topic, the syntax "ALTER POLICY  ON
#  TO " looks a bit wired, it might be better be to
# be "ON  APPLY TO " but I shouldn't try to fix it
# since it is a long standing syntax..


sql-createtablespace.html

"OWNER username" should be "OWNER user_name | (CURRENT|SESSION)_USER"


sql-drop-owned.html, sql-reassign-owned.html

"name" should be " {name | (CURRENT|SESSION)_USER}"

For REASSIGN OWNED, TO clause also needs the same fix.

==
sql-grant.html, sql-revoke.html,

"GRANT  TO " and "REVOKE  FROM " are
the modern equivalents of the deprecated syntaxes "ALTER 
ADD USER " and "ALTER  DROP USER "
respectively. But the current parser infrastructure doesn't allow
coexistence of the two following syntaxes but I couldn't find the
way to their coexistence.

# more precisely, I guess the GRANT followed by both
# privelege_list and role_list will steps out of the realm of
# LALR(1), which I don't know well of..

"GRANT  ON ..." 
"GRANT  TO ..."

After some struggle, I decided to add special rules
(CURRENT|SESSION)_USER to the non-terminal "privilege" and make a
room to store RoleSpec in AccessPriv. This is quite ugly but
there seems no way other than that to accomplish it.. (AccessPriv
already conveys other than the information different from what
its name represents:p)

After this fix, the commands like following are processed
properly. public and none are simply handled as nonexistent
names.

GRANT test1 TO CURRENT_USER;

GRANT  ON  TO  properly rejects CURRENT_USER
as .

But the change in gram.y cuases changes in preproc.y as following,

>  privilege:
>  SELECT opt_column_list
>  { 
...
> |  ColId opt_column_list
>  { 
>  $$ = cat_str(2,$1,$2);
> }
+ |  CURRENT_USER
+  { 
+  $$ = mm_strdup("current_user");
+ }
+ |  SESSION_USER
+  { 
+  $$ = mm_strdup("session_user");
+ }
> ;

I don't understand what this change causes...

=

I haven't looked on the docs for syntaxes related to
AlterOwnerStatement. But perhaps they don't be wrong.

regards,

-- 
Kyotaro Horiguchi
NTT Open Source Software Center
>From 26afc656576c8778921ff44519e3de86866ab138 Mon Sep 17 00:00:00 2001
From: Kyotaro Horiguchi 
Date: Thu, 12 Mar 2015 20:56:14 +0900
Subject: [PATCH] Some additional changes for ALTER ROLE/USER CURRENT_USER.

Some documents are not edit according to new specs. Addition to it,
"GRANT  TO " and "REVOKE  FROM " syntaxes,
which are the modern replacement of "ALTER GROUP ADD/DROP USER"
syntax, are not accepted by the previous patch.

This patch fixes some docs and changes the syntax of GRANT command to
accept CURRENT_ROLE and SESSION_ROLE as other similar command does.

Re: [HACKERS] Proposal : REINDEX xxx VERBOSE

[Sawada Masahiko]

On Thu, Mar 12, 2015 at 6:36 AM, Jim Nasby  wrote:
> On 3/11/15 6:33 AM, Sawada Masahiko wrote:
>>>
>>> As a refresher, current commands are:
>>> >
>>> >VACUUM (ANALYZE, VERBOSE) table1 (col1);
>>> >REINDEX INDEX index1 FORCE;
>>> >COPY table1 FROM 'file.txt' WITH (FORMAT csv);
>>> >CREATE MATERIALIZED VIEW mv1 WITH (storageparam, ...) AS qry
>>> > WITH
>>> >DATA;
>>> >CREATE EXTENSION ext1 WITH SCHEMA s1 VERSION v1 FROM over;
>>> >CREATE ROLE role WITH LOGIN;
>>> >GRANT  WITH GRANT OPTION;
>>> >CREATE VIEW v1 AS qry WITH CASCADED CHECK OPTION;
>>> >ALTER DATABASE db1 WITH CONNECTION LIMIT 50;
>>> >DECLARE c1 INSENSITIVE SCROLL CURSOR WITH HOLD;
>
> >>>
> >>>
> >>>
> >>>BTW, I'm fine with Tom's bare-word with WITH idea. That seems to be
> >>> the
> >>>most
> >>>consistent with everything else. Is there a problem with doing that?
> >>> I
> >>>know
> >>>getting syntax is one of the hard parts of new features, but it
> >>> seems
> >>>like
> >>>we reached consensus here...

 >>
 >>
 >>Attached is latest version patch based on Tom's idea as follows.
 >>REINDEX { INDEX | ... } name WITH ( options [, ...] )
>>>
>>> >
>>> >
>>> >Are the parenthesis necessary? No other WITH option requires them, other
>>> >than create table/matview (COPY doesn't actually require them).
>>> >
>>
>> I was imagining EXPLAIN syntax.
>> Is there some possibility of supporting multiple options for REINDEX
>> command in future?
>> If there is, syntax will be as follows, REINDEX { INDEX | ... } name
>> WITH VERBOSE, XXX, XXX;
>> I thought style with parenthesis is better than above style.
>
>
> The thing is, ()s are actually an odd-duck. Very little supports it, and
> while COPY allows it they're not required. EXPLAIN is a different story,
> because that's not WITH; we're actually using () *instead of* WITH.
>
> So because almost all commands that use WITH doen't even accept (), I don't
> think this should either. It certainly shouldn't require them, because
> unlike EXPLAIN, there's no need to require them.
>

I understood what your point is.
Attached patch is changed syntax, it does not have parenthesis.

Regards,

---
Sawada Masahiko
diff --git a/doc/src/sgml/ref/reindex.sgml b/doc/src/sgml/ref/reindex.sgml
index 0a4c7d4..3cea35f 100644
--- a/doc/src/sgml/ref/reindex.sgml
+++ b/doc/src/sgml/ref/reindex.sgml
@@ -22,6 +22,11 @@ PostgreSQL documentation
  
 
 REINDEX { INDEX | TABLE | SCHEMA | DATABASE | SYSTEM } name [ FORCE ]
+REINDEX { INDEX | TABLE | SCHEMA | DATABASE | SYSTEM } name WITH options [, ...]
+
+where option can be one of:
+
+VERBOSE
 
  
 
@@ -159,6 +164,15 @@ REINDEX { INDEX | TABLE | SCHEMA | DATABASE | SYSTEM } relpersistence);
+	reindex_index(indOid, false, indexRelation->relpersistence, verbose);
 
 	return indOid;
 }
@@ -1761,9 +1775,23 @@ RangeVarCallbackForReindexIndex(const RangeVar *relation,
  *		Recreate all indexes of a table (and of its toast table, if any)
  */
 Oid
-ReindexTable(RangeVar *relation)
+ReindexTable(RangeVar *relation, List *options)
 {
 	Oid			heapOid;
+	bool		verbose = false;
+	ListCell	*lc;
+
+	/* Parse list of options */
+	foreach(lc, options)
+	{
+		char *opt = (char *) lfirst(lc);
+		if (strcmp(opt, "verbose") == 0)
+			verbose = true;
+		else
+			ereport(ERROR,
+	(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+	 errmsg("unrecognized REINDEX option \"%s\"", opt)));
+	}
 
 	/* The lock level used here should match reindex_relation(). */
 	heapOid = RangeVarGetRelidExtended(relation, ShareLock, false, false,
@@ -1771,7 +1799,8 @@ ReindexTable(RangeVar *relation)
 
 	if (!reindex_relation(heapOid,
 		  REINDEX_REL_PROCESS_TOAST |
-		  REINDEX_REL_CHECK_CONSTRAINTS))
+		  REINDEX_REL_CHECK_CONSTRAINTS,
+		  verbose))
 		ereport(NOTICE,
 (errmsg("table \"%s\" has no indexes",
 		relation->relname)));
@@ -1788,7 +1817,7 @@ ReindexTable(RangeVar *relation)
  * That means this must not be called within a user transaction block!
  */
 void
-ReindexMultipleTables(const char *objectName, ReindexObjectType objectKind)
+ReindexMultipleTables(const char *objectName, ReindexObjectType objectKind, List *options)
 {
 	Oid			objectOid;
 	Relation	relationRelation;
@@ -1800,12 +1829,29 @@ ReindexMultipleTables(const char *objectName, ReindexObjectType objectKind)
 	List	   *relids = NIL;
 	ListCell   *l;
 	int			num_keys;
+	bool		verbose;
+	int			elevel = DEBUG2;
 
 	AssertArg(objectName);
 	Assert(objectKind == REINDEX_OBJECT_SCHEMA ||
 		   objectKind == REINDEX_OBJECT_SYSTEM ||
 		   objectKind == REINDEX_OBJECT_DATABASE);
 
+	/* Parse list of options */
+	foreach(l, options)
+	{
+		char *opt = (char *) lfirst(l);
+		if (strcmp(opt, "verbose") == 0)
+		{
+			verbose = true;
+			elevel = INFO;
+		}
+		else
+			ereport(ERROR,
+	(errcode(ER

Re: [HACKERS] [PATCH] Add transforms feature

[Pavel Stehule]

Hi

I am looking to code.

Some small issues:


1. fix missing semicolon pg_proc.h

Oid protrftypes[1]; /* types for which to apply
transforms */

2. strange load lib by in sql scripts:

DO '' LANGUAGE plperl;
SELECT NULL::hstore;

use load plperl; load hstore; instead

3. missing documentation for new contrib modules,

4. pg_dump - wrong comment

+<-><-->/*
+<-><--> * protrftypes was added at v9.4
+<-><--> */

4. Why guc-use-transforms? Is there some possible negative side effect of
transformations, so we have to disable it? If somebody don't would to use
some transformations, then he should not to install some specific
transformation.

5. I don't understand to motivation for introduction of protrftypes in
pg_proc and TRANSFORM clause for CREATE FUNCTION - it is not clean from
documentation, and examples in contribs works without it. Is it this
functionality really necessary? Missing tests, missing examples.

Regards

Pavel


2015-03-08 16:34 GMT+01:00 Peter Eisentraut :

> On 3/6/15 9:56 AM, Pavel Stehule wrote:
> > Hi
> >
> > I am checking this patch, but it is broken still
>
> Here is an updated patch.
>
> (Note that because of the large pg_proc.h changes, it is likely to get
> outdated again soon.  But for reviewing, you can always apply it against
> an older version of master.)
>
>

Re: [HACKERS] Precedence of standard comparison operators

[Greg Stark]

On Wed, Mar 11, 2015 at 8:36 PM, Kevin Grittner  wrote:

> Right; and they may have millions of lines of code which have been
> carefully tested and in production for years, and which may
> suddenly start to generate incorrect results on queries *or mangle
> existing data* with the fix unless they change their SQL code.
>

Well not suddenly. When doing a major upgrade. And we provide a tool to
help them identify the problems.

But having a warning enabled by default means the problem is effectively
not fixed at all. People will not be able to write code that's valid
according to the docs and the spec without extra parentheses or disabling
the warning.


-- 
greg

Re: [HACKERS] How about to have relnamespace and relrole?

[Jeevan Chalke]

The following review has been posted through the commitfest application:
make installcheck-world:  tested, passed
Implements feature:   tested, passed
Spec compliant:   tested, passed
Documentation:tested, passed

Looks good. Passing it to committer.

The new status of this patch is: Ready for Committer


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

[HACKERS] OOM-killer issue when updating a inheritance table which has large number of child tables

[chenhj]

Hi


In my test(PG9.3.4), i found when update a parent table which has a large 
number of child tables, the execute plan will consume lots of memory. And 
possibly cause OOM.


For example:
 create table maintb(id int,name char(10));
 create table childtb_1 (CHECK ( id BETWEEN 1 AND 200)) inherits(maintb);
 create table childtb_2 (CHECK ( id BETWEEN 201 AND 400)) inherits(maintb);
 ...
 create table childtb_n ...




When there are 100 child tables,the following update statement will consume 
about 8MB memory when invoking pg_plan_queries()
update maintb set name = 'a12345' where id=1;


And, when there are 1000 child tables,the same update statement will consume 
717MB memory when invoking pg_plan_queries().


Does this a known problem, and could that be improved in the future?


BTW:
The following comment is according my debuging when update the parent table 
with 1000 child tables
src/backend/optimizer/plan/planner.c
static Plan *
inheritance_planner(PlannerInfo *root)
{
...
foreach(lc, root->append_rel_list)//### loop 1001 time
{
...
subroot.parse = (Query *)
adjust_appendrel_attrs(root,
 (Node *) parse,
 appinfo);//### allocate about 300KB memory a 
time.


...
subroot.append_rel_list = (List *) 
copyObject(root->append_rel_list);//### allocate about 400KB memory a time.


...
}
...
}


Best Regards
Chen Huajun

Re: [HACKERS] Parallel Seq Scan

[Amit Langote]

On 10-03-2015 PM 01:09, Amit Kapila wrote:
> On Tue, Mar 10, 2015 at 6:50 AM, Haribabu Kommi 
>> Is this patch handles the cases where the re-scan starts without
>> finishing the earlier scan?
>>
> 
> Do you mean to say cases like ANTI, SEMI Join (in nodeNestLoop.c)
> where we scan the next outer tuple and rescan inner table without
> completing the previous scan of inner table?
> 
> I have currently modelled it based on existing rescan for seqscan
> (ExecReScanSeqScan()) which means it will begin the scan again.
> Basically if the workers are already started/initialized by previous
> scan, then re-initialize them (refer function ExecReScanFunnel() in
> patch).
> 

>From Robert's description[1], it looked like the NestLoop with Funnel would
have Funnel as either outer plan or topmost plan node or NOT a parameterised
plan. In that case, would this case arise or am I missing something?

Thanks,
Amit

[1]
http://www.postgresql.org/message-id/CA+TgmobM7X6jgre442638b+33h1EWa=vczqnsvzedx057zh...@mail.gmail.com



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

[HACKERS] shebang for tcl postgresql modules

[Jozef Mlich]

Dear hackers,

I would like to ask you if is there any reason to pretend tcl scripts
are shell scripts?

I am speaking namely about these files

http://git.postgresql.org/gitweb/?p=postgresql.git;a=blob_plain;f=src/pl/tcl/modules/pltcl_delmod.in;hb=HEAD
http://git.postgresql.org/gitweb/?p=postgresql.git;a=blob_plain;f=src/pl/tcl/modules/pltcl_listmod.in;hb=HEAD
http://git.postgresql.org/gitweb/?p=postgresql.git;a=blob_plain;f=src/pl/tcl/modules/pltcl_loadmod.in;hb=HEAD


Here is the part of code I am speaking about:

#! /bin/sh
# Start tclsh \
exec @TCLSH@ "$0" "$@"

instead of
#! /usr/bin/tclsh

or 
#! @TCLSH@

I am asking because our test suite is triggering errors on this [1]. In
this case, it seems easier to modify code rather then test suite. Please
apply attached patch if there is no particular reason for use
of /bin/sh.

[1] https://bugzilla.redhat.com/show_bug.cgi?id=1199464


-- 
Jozef Mlich 
Associate Software Engineer - EMEA ENG Developer Experience
Mobile: +420 604 217 719
http://cz.redhat.com/
Red Hat, Inc.
diff --git a/src/pl/tcl/modules/pltcl_delmod.in b/src/pl/tcl/modules/pltcl_delmod.in
index daa4fac..6b8cda4 100644
--- a/src/pl/tcl/modules/pltcl_delmod.in
+++ b/src/pl/tcl/modules/pltcl_delmod.in
@@ -1,8 +1,6 @@
-#! /bin/sh
+#! @TCLSH@
 # src/pl/tcl/modules/pltcl_delmod.in
 #
-# Start tclsh \
-exec @TCLSH@ "$0" "$@"
 
 #
 # Code still has to be documented
diff --git a/src/pl/tcl/modules/pltcl_listmod.in b/src/pl/tcl/modules/pltcl_listmod.in
index 7d930ff..c3ea138 100644
--- a/src/pl/tcl/modules/pltcl_listmod.in
+++ b/src/pl/tcl/modules/pltcl_listmod.in
@@ -1,8 +1,6 @@
-#! /bin/sh
+#! @TCLSH@
 # src/pl/tcl/modules/pltcl_listmod.in
 #
-# Start tclsh \
-exec @TCLSH@ "$0" "$@"
 
 #
 # Code still has to be documented
diff --git a/src/pl/tcl/modules/pltcl_loadmod.in b/src/pl/tcl/modules/pltcl_loadmod.in
index 645c6bb..06b459b 100644
--- a/src/pl/tcl/modules/pltcl_loadmod.in
+++ b/src/pl/tcl/modules/pltcl_loadmod.in
@@ -1,6 +1,5 @@
-#! /bin/sh
-# Start tclsh \
-exec @TCLSH@ "$0" "$@"
+#! @TCLSH@
+#
 
 #
 # Code still has to be documented

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

Re: [HACKERS] proposal: searching in array function - array_position

[Jim Nasby]

On 3/11/15 1:19 AM, Pavel Stehule wrote:

2015-03-11 2:57 GMT+01:00 Robert Haas mailto:robertmh...@gmail.com>>:

On Tue, Mar 10, 2015 at 5:53 PM, Jim Nasby mailto:jim.na...@bluetreble.com>> wrote:
> I don't think we need both array_offset and array_offset_start; can't both
> SQL functions just call one C function?

Not if you want the opr_sanity tests to pass.

(But I'm seriously starting to wonder if that's actually a smart rule
for us to be enforcing.  It seems to be something of a pain in the
neck, and I'm unclear as to whether it is preventing any real
problem.)


It is simple protection against some oversights.  I am not against this
check - this rule cleans a interface between C and SQL. More, the
additional C code is usually very short and trivial.

But it should be commented well.


Ahh, ok, makes more sense now. If the separate C functions are serving a 
purpose that's fine. I think the comment should mention it though, as 
it's not exactly the most obvious thing.

--
Jim Nasby, Data Architect, Blue Treble Consulting
Data in Trouble? Get it in Treble! http://BlueTreble.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] Precedence of standard comparison operators

[Peter Eisentraut]

On 3/10/15 4:34 PM, Tom Lane wrote:
> There's one more reason, too: the code I have is designed to give correct
> warnings within the context of a parser that parses according to the
> spec-compliant rules.  It's possible that a similar approach could be used
> to generate correct warnings within a parsetree built according to the old
> rules, but it would be entirely different in detail and would need a lot
> of additional work to develop.  I don't particularly want to do that
> additional work.

So you want to change the precedence behavior in the next release and
have a warning about "this code would have worked differently before".
My understanding was that we would keep the precedence behavior for a
while but warn about "this code would work differently in the future".



-- 
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] Turning off HOT/Cleanup sometimes

[Andres Freund]

On 2015-03-11 20:55:18 -0400, Peter Eisentraut wrote:
> I don't think so.  Andres basically wanted a nontrival algorithm to
> determine how much pruning to do during a read-only scan.  And Robert
> basically said, that's not really possible.

I don't think either of us made really strong statements.

> We have seen some benchmarks that show significant improvements.  We
> have seen some (constructed ones) that show problems.

FWIW, it's not that constructed. It's just a mixture of read with write
load.

Greetings,

Andres Freund

-- 
 Andres Freund 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] One question about security label command

[Robert Haas]

On Tue, Mar 10, 2015 at 6:58 PM, Kohei KaiGai  wrote:
> ERRCODE_FEATURE_NOT_SUPPORTED is suitable error code here.
> Please see the attached one.

Committed.  I did not bother back-patching this, but I can do that if
people think it's important.  The sepgsql regression tests don't seem
to pass for me any more; I wonder if some expected-output changes are
needed as a result of core changes.

I'm guessing these tests are not running in an automated fashion anywhere?

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


regression.diffs
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] Doing better at HINTing an appropriate column within errorMissingColumn()

[Robert Haas]

On Tue, Mar 10, 2015 at 4:03 PM, Peter Geoghegan  wrote:
> On Tue, Mar 10, 2015 at 11:28 AM, Robert Haas  wrote:
>> I'm prepared to commit this version if nobody finds a problem with it.
>> It passes the additional regression tests you wrote.
>
> Looks good to me. Thanks.

OK, committed.

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


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

Re: [HACKERS] EvalPlanQual behaves oddly for FDW queries involving system columns

[Etsuro Fujita]

On 2015/03/12 13:35, Tom Lane wrote:
> I don't like the execMain.c changes much at all.  They look somewhat
> like they're intended to allow foreign tables to adopt a different
> locking strategy,

I didn't intend such a thing.  My intention is, foreign tables have
markType = ROW_MARK_COPY as ever, but I might not have correctly
understood what you pointed out.  Could you elaborate on that?

> The changes are not all consistent
> either, eg this:
> 
> ! if (erm->relation &&
> ! erm->relation->rd_rel->relkind != RELKIND_FOREIGN_TABLE)
> 
> is not necessary if this Assert is accurate:
> 
> ! if (erm->relation)
> ! {
> ! Assert(erm->relation->rd_rel->relkind == 
> RELKIND_FOREIGN_TABLE);

I modified InitPlan so that relations get opened for foreign tables
as well as local tables.  So, I think the change would be necessary.

> I don't see the need for the change in nodeForeignscan.c.  If the FDW has
> returned a physical tuple, it can fix that for itself, while if it has
> returned a virtual tuple, the ctid will be garbage in any case.

If you leave nodeForeignscan.c unchanged, file_fdw would cause the
problem that I pointed out upthread.  Here is an example.

[Create a test environment]

postgres=# create foreign table foo (a int) server file_server options
(filename '/home/pgsql/foo.csv', format 'csv');
CREATE FOREIGN TABLE
postgres=# select tableoid, ctid, * from foo;
 tableoid |  ctid  | a
--++---
16459 | (4294967295,0) | 1
(1 row)

postgres=# create table tab (a int, b int);
CREATE TABLE
postgres=# insert into tab values (1, 1);
INSERT 0 1

[Run concurrent transactions]

In terminal1:
postgres=# begin;
BEGIN
postgres=# update tab set b = b * 2;
UPDATE 1

In terminal2:
postgres=# select foo.tableoid, foo.ctid, foo.* from foo, tab where
foo.a = tab.a for update;

In terminal1:
postgres=# commit;
COMMIT

In terminal2:(When committing the UPDATE query in terminal1, psql in
terminal2 will display the result for the SELECT-FOR-UPDATE query as
shown below.)
 tableoid | ctid  | a
--+---+---
16459 | (0,0) | 1
(1 row)

Note the value of the ctid has changed!

Rather than changing nodeForeignscan.c, it might be better to change
heap_form_tuple to set the t_ctid of a formed tuple to be invalid.

Thanks for the review!

Best regards,
Etsuro Fujita


-- 
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] Install shared libs in lib/ and bin/ with MSVC (Was: install libpq.dll in bin directory on Windows / Cygwin)

[Asif Naeem]

Thank you Michael overall v3 patch looks good to me, There is one
observation that it is not installing following lib files that are required
for dev work i.e.
>
> inst\lib\libpq.lib
> inst\lib\libpgtypes.lib
> inst\lib\libecpg_compat.lib
> inst\lib\libecpg.lib

Please do let me know if I missed something but was not there a need to
avoid installing related .dll files in lib (duplicate) along with bin
directory e.g.

src\tools\msvc\Install.pm

>
> if ($is_sharedlib)
> {
> @dirs = qw(bin);
> }
> else
> {
> @dirs = qw(lib);
> }


Thanks.


On Mon, Mar 9, 2015 at 1:16 PM, Michael Paquier 
wrote:

> On Wed, Mar 4, 2015 at 4:13 AM, Michael Paquier wrote:
> > Those entries are removed as well in the patch.
>
> Please find attached a new version of the patch, fixing a failure for
> plperl installation that contains GNUmakefile instead of Makefile.
> --
> Michael
>

Re: [HACKERS] Proposal : REINDEX xxx VERBOSE

[Sawada Masahiko]

On Tue, Mar 10, 2015 at 5:05 PM, Jim Nasby  wrote:
> On 3/9/15 9:43 PM, Sawada Masahiko wrote:
>>
>> On Fri, Mar 6, 2015 at 11:07 AM, Jim Nasby 
>> wrote:
>>>
>>> On 3/2/15 10:58 AM, Sawada Masahiko wrote:


 On Wed, Feb 25, 2015 at 4:58 PM, Jim Nasby 
 wrote:
>
>
> On 2/24/15 8:28 AM, Sawada Masahiko wrote:
>>>
>>>
>>>
>>> According to the above discussion, VACUUM and REINDEX should have
>>> trailing options. Tom seems (to me) suggesting that SQL-style
>>> (bare word preceded by WITH) options and Jim suggesting '()'
>>> style options? (Anyway VACUUM gets the*third additional*  option
>>> sytle, but it is the different discussion from this)
>
>
>
>
> Well, almost everything does a trailing WITH. We need to either stick
> with
> that for consistency, or add leading () as an option to those WITH
> commands.
>
> Does anyone know why those are WITH? Is it ANSI?
>
> As a refresher, current commands are:
>
>VACUUM (ANALYZE, VERBOSE) table1 (col1);
>REINDEX INDEX index1 FORCE;
>COPY table1 FROM 'file.txt' WITH (FORMAT csv);
>CREATE MATERIALIZED VIEW mv1 WITH (storageparam, ...) AS qry WITH
> DATA;
>CREATE EXTENSION ext1 WITH SCHEMA s1 VERSION v1 FROM over;
>CREATE ROLE role WITH LOGIN;
>GRANT  WITH GRANT OPTION;
>CREATE VIEW v1 AS qry WITH CASCADED CHECK OPTION;
>ALTER DATABASE db1 WITH CONNECTION LIMIT 50;
>DECLARE c1 INSENSITIVE SCROLL CURSOR WITH HOLD;
>>>
>>>
>>>
>>> BTW, I'm fine with Tom's bare-word with WITH idea. That seems to be the
>>> most
>>> consistent with everything else. Is there a problem with doing that? I
>>> know
>>> getting syntax is one of the hard parts of new features, but it seems
>>> like
>>> we reached consensus here...
>>
>>
>> Attached is latest version patch based on Tom's idea as follows.
>> REINDEX { INDEX | ... } name WITH ( options [, ...] )
>
>
> Are the parenthesis necessary? No other WITH option requires them, other
> than create table/matview (COPY doesn't actually require them).
>
 We have discussed about this option including FORCE option, but I
 think there are not user who want to use both FORCE and VERBOSE option
 at same time.
>>>
>>>
>>>
>>> I find that very hard to believe... I would expect a primary use case for
>>> VERBOSE to be "I ran REINDEX, but it doesn't seem to have done
>>> anything...
>>> what's going on?" and that's exactly when you might want to use FORCE.
>>>
>>
>> In currently code, nothing happens even if FORCE option is specified.
>> This option completely exist for backward compatibility.
>> But this patch add new syntax including FORCE option for now.
>
>
> I forgot that. There's no reason to support it with the new stuff then.
>
> --
> Jim Nasby, Data Architect, Blue Treble Consulting
> Data in Trouble? Get it in Treble! http://BlueTreble.com

Attached patch is latest version patch changed syntax a little.
This patch adds following syntax for now.
 REINDEX { INDEX | ... } name WITH (VERBOSE);

But we are under the discussion regarding parenthesis, so there is
possibility of change.

Regards,

---
Sawada Masahiko


000_reindex_verbose_v4.patch
Description: Binary data

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

[HACKERS] timestamp format escape rules

[Rikard Pavelic]

Currently Postgres uses DATE WHITESPACE TIME for timestamp/tz datatype.

Due whitespace it needs to be escaped when returned as record.
How likely is to change WHITESPACE to something more ISO like, like 'T'?

This would allow it to be serialized without the need for escaping.

I'm asking this since we often return nested records directly from
Postgres and when reading deeply nested records, we are mostly skipping
on quotes/escapes.

Regards,
Rikard

-- 
Rikard Pavelic
https://dsl-platform.com/
http://templater.info/


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

Re: [HACKERS] proposal: disallow operator "=>" and use it for named parameters

[Tom Lane]

Kevin Grittner  writes:
> Tom Lane  wrote:
>> Presumably we are going to change it at some point; maybe we
>> should just do it rather than waiting another 5 years.

> +1

> It has been deprecated long enough that I don't see the point of waiting.

Uh, just to clarify, this has nothing to do with how long the operator has
been deprecated.  The issue is whether pg_dump should dump a function-call
syntax that will not be recognized by any pre-9.5 release, when there is
an alternative that will be recognized back to 9.0.

BTW, I just noticed another place that probably should be changed:

regression=# select foo(x => 1);
ERROR:  42883: function foo(x := integer) does not exist
LINE 1: select foo(x => 1);
   ^
HINT:  No function matches the given name and argument types. You might need to 
add explicit type casts.
LOCATION:  ParseFuncOrColumn, parse_func.c:516

regards, tom lane


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

Re: [HACKERS] proposal: disallow operator "=>" and use it for named parameters

[Kevin Grittner]

Tom Lane  wrote:
> Robert Haas  writes:
>> On Tue, Mar 10, 2015 at 11:50 AM, Tom Lane  wrote:
>>> Was there any consideration given to whether ruleutils should start
>>> printing NamedArgExprs with "=>"?  Or do we think that needs to wait?
>>
>> I have to admit that I didn't consider that.  What do you think?  I
>> guess I'd be tentatively in favor of changing that to match, but I
>> could be convinced otherwise.

> Presumably we are going to change it at some point; maybe we
> should just do it rather than waiting another 5 years.

+1

It has been deprecated long enough that I don't see the point of waiting.

--
Kevin Grittner
EDB: 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] Precedence of standard comparison operators

[Tom Lane]

Robert Haas  writes:
> On Tue, Mar 10, 2015 at 12:45 PM, David G. Johnston
>  wrote:
>> I would vote for Auto meaning On in the .0 release.

> I don't think users will appreciate an auto value whose meaning might
> change at some point, and I doubt we've have much luck identifying the
> correct point, either.  Users will upgrade over the course of years,
> not months, and will not necessarily complete their application
> retesting within any particular period of time thereafter.

Yeah, I think that's too cute by far.  And people do not like things like
this changing in point releases.  If we do this, I envision it as being
on by default in 9.5 and then changing the default in 9.6 or 9.7 or so.

Another possibility is to leave it on through beta testing with the intent
to turn it off before 9.5 final; that would give us more data about
whether there are real issues than we're likely to get otherwise.

regards, tom lane


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

Re: [HACKERS] proposal: disallow operator "=>" and use it for named parameters

[Pavel Stehule]

2015-03-10 17:07 GMT+01:00 Petr Jelinek :

> On 10/03/15 17:01, Pavel Stehule wrote:
>
>>
>>
>> 2015-03-10 16:50 GMT+01:00 Tom Lane > >:
>>
>> Robert Haas mailto:robertmh...@gmail.com>>
>> writes:
>>
>> > Committed with a few documentation tweaks.
>>
>> Was there any consideration given to whether ruleutils should start
>> printing NamedArgExprs with "=>"?  Or do we think that needs to wait?
>>
>>
>> I didn't think about it? I don't see any reason why it have to use
>> deprecated syntax.
>>
>>
> There is one, loading the output into older version of Postgres. Don't
> know if that's important one though.


I don't think so it is a hard issue. We doesn't support downgrades - and if
somebody needs it, it can fix it with some regexp. We should to use
preferred syntax everywhere - and preferred syntax should be ANSI.

I forgot it :(

Pavel


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

Re: [HACKERS] proposal: searching in array function - array_position

[Pavel Stehule]

2015-03-10 16:53 GMT+01:00 Jim Nasby :

> On 3/10/15 9:30 AM, Robert Haas wrote:
>
>> On Sat, Mar 7, 2015 at 1:06 PM, Petr Jelinek 
>> wrote:
>>
>>> You still duplicate the type cache code, but many other array functions
>>> do
>>> that too so I will not hold that against you. (Maybe somebody should
>>> write
>>> separate patch that would put all that duplicate code into common
>>> function?)
>>>
>>> I think this patch is ready for committer so I am going to mark it as
>>> such.
>>>
>>
>> The documentation in this patch needs some improvements to the
>> English.  Can anyone help with that?
>>
>
> I'll take a look at it.
>
>  The documentation should discuss what happens if the array is
>> multi-dimensional.
>>
>> The code for array_offset and for array_offset_start appear to be
>> byte-for-byte identical.  There's no comment explaining why, but I bet
>> it's to make the opr_sanity test pass.  How about adding a comment?
>>
>> The comment for array_offset_common refers to array_offset_start as
>> array_offset_startpos.
>>
>> I am sure in agreement with the idea that it would be good to factor
>> out the common typecache code (for setting up my_extra).  Any chance
>> we get a preliminary patch that does that refactoring, and then rebase
>> the main patch on top of it?  I agree that it's not really this
>> patch's job to solve that problem, but it would be nice.
>>
>
> Since this patch is here and ready to go I would prefer that we commit it
> and refactor later. I can tackle that unless Pavel specifically wants to.


I'll look on this part this evening - but I don't have any idea how to find
some common pattern yet. So I am with Jim - we can do it later.

Regards

Pavel


>
> --
> Jim Nasby, Data Architect, Blue Treble Consulting
> Data in Trouble? Get it in Treble! http://BlueTreble.com
>

Re: [HACKERS] Stateful C-language function with state managed by third-party library

[Tom Lane]

Denys Rtveliashvili  writes:
> My function neeeds to call a third-party library which would create a state 
> and then that state should be kept for the duration of the current query. The 
> library can deallocate that state in a correct way.

> I understand that fn_extra is normally used for this and usually the state is 
> created in a memory context which is deallocated at the end of the query. So 
> normally it is not an issue. However, I cannot make that library use 
> PostgreSQL utilities for memory management.

> I am afraid that for long-running sessions it may cause serious memory leaks 
> if they do not deallocate state correctly and in a timely manner.

> Is there a mechanism for adding a finalizer hook which would be called and 
> passed that pointer after the query is complete? Or perhaps there is another 
> mechanism? I looked in the documentation and in the source but I do not see 
> it mentioned.

In HEAD, you could use a memory context reset callback for this purpose.

I don't believe there's any fully satisfactory solution in the released
branches; the closest you could get is an ExprContext callback, which
has the fatal-for-this-purpose defect that it's only called on successful
query completion, not if an error occurs.

regards, tom lane


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

Re: [HACKERS] proposal: searching in array function - array_position

[Jim Nasby]

On 3/10/15 9:30 AM, Robert Haas wrote:

On Sat, Mar 7, 2015 at 1:06 PM, Petr Jelinek  wrote:

You still duplicate the type cache code, but many other array functions do
that too so I will not hold that against you. (Maybe somebody should write
separate patch that would put all that duplicate code into common function?)

I think this patch is ready for committer so I am going to mark it as such.


The documentation in this patch needs some improvements to the
English.  Can anyone help with that?


I'll take a look at it.


The documentation should discuss what happens if the array is multi-dimensional.

The code for array_offset and for array_offset_start appear to be
byte-for-byte identical.  There's no comment explaining why, but I bet
it's to make the opr_sanity test pass.  How about adding a comment?

The comment for array_offset_common refers to array_offset_start as
array_offset_startpos.

I am sure in agreement with the idea that it would be good to factor
out the common typecache code (for setting up my_extra).  Any chance
we get a preliminary patch that does that refactoring, and then rebase
the main patch on top of it?  I agree that it's not really this
patch's job to solve that problem, but it would be nice.


Since this patch is here and ready to go I would prefer that we commit 
it and refactor later. I can tackle that unless Pavel specifically wants to.

--
Jim Nasby, Data Architect, Blue Treble Consulting
Data in Trouble? Get it in Treble! http://BlueTreble.com


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

Re: [HACKERS] patch : Allow toast tables to be moved to a different tablespace

[Julien Tachoires]

On 10/03/2015 13:27, Alvaro Herrera wrote:
> Robert Haas wrote:
>> On Mon, Mar 9, 2015 at 7:26 PM, Andreas Karlsson  wrote:
> 
>>> I think we should allow moving the indexes for consistency. With this patch
>>> we can move everything except for TOAST indexes.
>>
>> It might make sense to always put the TOAST index with the TOAST
>> table, but it seems strange to put the TOAST index with the heap and
>> the TOAST table someplace else.  Or at least, that's how it seems to
>> me.
> 
> Agreed.  It doesn't seem necessary to allow moving the toast index to a
> tablespace other than the one containing the toast table.  In other
> words, if you move the toast table, the index always follows it, and
> there's no option to move it independently.

This behaviour is already implemented, the TOAST index always follows
the TOAST table. I'll add a couple of regression tests about it.

--
Julien



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