Re: [HACKERS] [PATCH] Support for foreign keys with arrays

2011-12-10 Thread Gabriele Bartolini

Hi Noah,

thanks for your feedback.

Il 20/11/11 14:05, Noah Misch ha scritto:

What about making ON UPDATE CASCADE an error?  That way, we can say that ARRAY
action  always applies to array elements, and plainaction  always applies to
entire rows.

SET DEFAULT should now be fine to allow.  It's ARRAY SET DEFAULT, in your new
terminology, that wouldn't make sense.


I have tried to gather your ideas with Gianni's and come to a 
compromise, which I hope you can both agree on.


The reason why I would be inclined to leave CASCADE act on rows (rather 
than array elements as Gianni suggests) is for backward compatibility 
(people that are already using referential integrity based on array 
values). For the same reason, I am not sure whether we should raise an 
error on update, but will leave this for later.


So, here is a summary:

--- - -
   |   ON|   ON|
Action | DELETE  | UPDATE  |
--- - -
CASCADE|   Row   |  Error  |
SET NULL   |   Row   |   Row   |
SET DEFAULT|   Row   |   Row   |
ARRAY CASCADE  | Element | Element |
ARRAY SET NULL | Element | Element |
NO ACTION  |-|-|
RESTRICT   |-|-|
--- - -

If that's fine with you guys, Marco and I will refactor the development 
based on these assumptions.


Thanks,
Gabriele

--
 Gabriele Bartolini - 2ndQuadrant Italia
 PostgreSQL Training, Services and Support
 gabriele.bartol...@2ndquadrant.it | www.2ndQuadrant.it


--
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] [REVIEW] pg_last_xact_insert_timestamp

2011-12-10 Thread Greg Smith

On 10/02/2011 07:10 AM, Robert Haas wrote:

Your proposals involve sending additional information from the master to
the slave, but the slave already knows both its WAL position and the
timestamp of the transaction it has most recently replayed, because
the startup process on the slave tracks that information and publishes
it in shared memory.  On the master, however, only the WAL position is
centrally tracked; the transaction timestamps are not.
   


This seems to be the question that was never really answered well enough 
to satisfy anyone, so let's rewind to here for a bit.  I wasn't 
following this closely until now, so I just did my own review from 
scratch against the latest patch.  I found a few issues, and I don't 
think all of them have been vented here yet:


-It adds overhead at every commit, even for people who aren't using it.  
Probably not enough to matter, but it's yet another thing going through 
the often maligned as too heavy pgstat system, often.


-In order to measure lag this way, you need access to both the master 
and the standby.  Yuck, dblink or application code doing timestamp math, 
either idea makes me shudder.  It would be nice to answer how many 
seconds of lag do have? directly from the standby.  Ideally I would 
like both the master and standby to know those numbers.


-After the server is restarted, you get a hole in the monitoring data 
until the first transaction is committed or aborted.  The way the 
existing pg_last_xact_replay_timestamp side of this computation goes 
NULL for some unpredictable period after restart is going to drive 
monitoring systems batty.  Building this new facility similarly will 
force everyone who writes a lag monitor to special case for that 
condition on both sides.  Sure, that's less user hostile than the status 
quo, but it isn't going to help PostgreSQL's battered reputation in the 
area of monitoring either.


-The transaction ID advancing is not a very good proxy for real-world 
lag.  You can have a very large amount of writing in between commits.  
The existing lag monitoring possibilities focus on WAL position instead, 
which is better correlated with the sort of activity that causes lag.  
Making one measurement of lag WAL position based (the existing ones) and 
another based on transaction advance (this proposal) is bound to raise 
some which of these is the correct lag? questions, when the two 
diverge.  Large data loading operations come to mind as a not unusual at 
all situation where this would happen.


I'm normally a fan of building the simplest thing that will do something 
useful, and this patch succeeds there.  But the best data to collect 
needs to have a timestamp that keeps moving forward in a way that 
correlates reasonably well to the system WAL load.  Using the 
transaction ID doesn't do that.  Simon did some hand waving around 
sending a timestamp every checkpoint.  That would allow the standby to 
compute its own lag, limit overhead to something much lighter than 
per-transaction, and better track write volume.  There could still be a 
bigger than normal discontinuity after server restart, if the server was 
down a while, but at least there wouldn't ever be a point where the 
value was returned by the master but was NULL.


But as Simon mentioned in passing, it will bloat the WAL streams for 
everyone.  Here's the as yet uncoded mini-proposal that seems to have 
slid by uncommented upon:


We can send regular special messages from WALSender to WALReceiver that 
do not form part of the WAL stream, so we don't bulk

up WAL archives. (i.e. don't use w messages).

Here's my understanding of how this would work.  Each time a 
commit/abort record appears in the WAL, that updates XLogCtl with the 
associated timestamp.  If WALReceiver received regular updates 
containing the master's clock timestamp and stored them 
similarly--either via regular streaming or the heartbeat--it could 
compute lag with the same resolution as this patch aims to do, for the 
price of two spinlocks:  just subtract the two timestamps.  No overhead 
on the master, and lag can be directly computed and queried from each 
standby.  If you want to feed that data back to the master so it can 
appear in pg_stat_replication in both places, send it periodically via 
the same channel sync rep and standby feedback use.  I believe that will 
be cheap in many cases, since it can piggyback on messages that will 
still be quite small relative to minimum packet size on most networks.  
(Exception for compressed/encrypted networks where packets aren't 
discrete in this way doesn't seem that relevant, presuming that if 
you're doing one of those I would think this overhead is the least of 
your worries)


Now, there's still one problem here.  This doesn't address the lots of 
write volume but no commit problem any better than the proposed patch 
does.  Maybe there's some fancy way to inject it along with the received 
WAL on the standby, I'm out of brain power 

Re: GiST for range types (was Re: [HACKERS] Range Types - typo + NULL string constructor)

2011-12-10 Thread Greg Smith

On 12/02/2011 06:48 AM, Alexander Korotkov wrote:

Rebased with head.


Could you comment a little more on what changed?  There were a couple of 
areas Tom commented on:


-General code fixes
-pull out and apply the changes related to the RANGE_CONTAIN_EMPTY 
flag, and also remove the  opclass entry

-Subdiff issues

The third one sounded hard to deal with, so presumably nothing there.  
I'm not sure if your updated rebase addresses either of those first two 
yet or not, or if it was just fixing bitrot from upstream code changes.


--
Greg Smith   2ndQuadrant USg...@2ndquadrant.com   Baltimore, MD
PostgreSQL Training, Services, and 24x7 Support  www.2ndQuadrant.us


--
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] Caching for stable expressions with constant arguments v3

2011-12-10 Thread Greg Smith

On 12/07/2011 04:58 PM, Marti Raudsepp wrote:

PS: I forgot to mention that 2 test cases covering the two above query
types are deliberately left failing in the v4-wip patch.
   


It's not completely clear what happens next with this.  Are you hoping 
code churn here has calmed down enough for Jaime or someone else to try 
and look at this more already?  Or should we wait for a new update based 
on the feedback that Heikki and Tom have already provided first, perhaps 
one that proposes fixes for these two test cases?


One general suggestion about the fight with upstream changes you've run 
into here.  Now that you're settling into the git workflow, you might 
consider publishing updates to a site like Github in the future too.  
That lets testing of the code at the point you wrote it always 
possible.  Given just the patch, reviewers normally must reconcile any 
bit rot before they can even compile your code to try it.  That gets 
increasingly sketchy the longer your patch waits before the next 
CommitFest considers it.  With a published git working tree, reviewers 
can pull that for some hands-on testing whenever, even if a merge 
wouldn't actually work out at that point.  You just need to be careful 
not to push an update that isn't self-consistent to the world.  I 
normally attach the best formatted patch I can and publish to Github.  
Then reviewers can use whichever they find easier, and always have the 
option of postponing a look at merge issues if they just want to play 
with the program.


--
Greg Smith   2ndQuadrant USg...@2ndquadrant.com   Baltimore, MD
PostgreSQL Training, Services, and 24x7 Support  www.2ndQuadrant.us


--
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 for type privileges

2011-12-10 Thread Yeb Havinga

On 2011-12-07 19:59, Peter Eisentraut wrote:
Two excellent finds. Here is an updated patch with fixes. 


Thanks.. I'm sorry I cannot yet provide a complete review, but since the 
end of the commitfest is near, I decided to mail them anyway instead of 
everything on dec 15.


* ExecGrant_type() prevents 'grant usage on domain' on a type, but the 
converse is possible.


postgres=# create domain myint as int2;
CREATE DOMAIN
postgres=# grant usage on type myint to public;
GRANT

* Cannot restrict access to array types. After revoking usage from the 
element type, the error is perhaps a bit misleading. (smallint[] vs 
smallint)


postgres= create table a (a int2[]);
ERROR:  permission denied for type smallint[]

* The patch adds the following text explaining the USAGE privilege on types.

  For types and domains, this privilege allow the use of the type or
  domain in the definition of tables, functions, and other schema objects.

Since other paragraphs in USAGE use the word 'creation' instead of 
'definition', I believe here the word 'creation' should be used too.  
IMHO it would also be good to describe what the USAGE privilege is not, 
but might be expected since it is such a generic term. USAGE on type: 
use of the type while creating new dependencies to the type, not usage 
in the sense of instantiating values of the type. If there are existing 
dependencies, revoking usage privileges will not return any warning and 
the dependencies still exist. Also other kinds of exceptions could be 
noted, such as the exception for array types and casts. The example you 
gave in the top mail about why restricting access to types can be 
useful, such as preventing that owners are prevented changing their 
types because others have 'blocked' them by their usage, is something 
that could also help readers of the documentation understand why 
privileges on types are useful for them (or not).


* The information schema view 'attributes' has this additional condition:
  AND (pg_has_role(t.typowner, 'USAGE')
   OR has_type_privilege(t.oid, 'USAGE'));

What happens is that attributes in a composite type are shown, or not, 
if the current user has USAGE rights. The strange thing here, is that 
the attribute in the type being show or not, doesn't match being able to 
use it (in the creation of e.g. a table). Maybe that is not intended, 
but I would expect it matching:


postgres=# create user c;
CREATE ROLE
postgres=# create type t as (a int2);
CREATE TYPE
postgres=# \c - c
You are now connected to database postgres as user c.
postgres= select udt_name,attribute_name from 
information_schema.attributes;

 udt_name | attribute_name
--+
 t| a
(1 row)

postgres= \c -
You are now connected to database postgres as user c.
postgres= \c - postgres
You are now connected to database postgres as user postgres.
postgres=# revoke usage on type int2 from public;
REVOKE
postgres=# \c - c
You are now connected to database postgres as user c.
postgres= select udt_name,attribute_name from 
information_schema.attributes;

 udt_name | attribute_name
--+
(0 rows)

postgres= create table m (a t);
CREATE TABLE
postgres= insert into m values (ROW(10));
INSERT 0 1
postgres=

Conversely:

postgres=# grant usage on type int2 to public;
GRANT
postgres=# revoke usage on type t from public;
REVOKE
postgres=# \c - c
You are now connected to database postgres as user c.
postgres= select udt_name,attribute_name from 
information_schema.attributes;

 udt_name | attribute_name
--+
 t| a
(1 row)

postgres= create table m2 (a t);
ERROR:  permission denied for type t
postgres=


regards,
Yeb Havinga


--
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] %TYPE and array declaration patch review

2011-12-10 Thread Greg Smith

On 11/30/2011 10:42 AM, Pavel Stehule wrote:

Regress tests are really large - it is question if about 900 lines is
necessary - should be more compact
   


Can't recall the last time I heard a complaint about having too many 
regression tests for new code.  We've got some bit rot, code convention 
suggestions, test scope concerns, and some fundamental design questions 
hanging over this one.  I don't think anyone expected this TODO item was 
going to turn into a 2626 line patch, but discovering a bunch of 
unexpected complexity underneath one of those is a regular event.  Stuff 
that's straighforward to implement tends to just get done instead of 
added there.


Tom's concerns about the grammar rewrite and way parsing is handled here 
seem the worst blockers for committing this, and I can't imagine how 
those could be resolved before this CommitFest is over.  I'm going to 
mark this one as returned with that and Pavel's feedback.  Perhaps 
Wojciech or someone else will come up with a clever way to address this, 
one that has less impact on existing code.  Adding more parser 
complexity than absolutely necessary is a much bigger problem than the 
regression tests being too long.


--
Greg Smith   2ndQuadrant USg...@2ndquadrant.com   Baltimore, MD
PostgreSQL Training, Services, and 24x7 Support  www.2ndQuadrant.us


--
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] WIP: SP-GiST, Space-Partitioned GiST

2011-12-10 Thread Tom Lane
I wrote:
 ... the leaf tuple datatype is hard-wired to be
 the same as the indexed column's type.  Why is that?  It seems to me
 to be both confusing and restrictive.  For instance, if you'd designed
 the suffix tree opclass just a little differently, it would be wanting
 to store char not text in leaf nodes.  Shouldn't we change this to
 allow the leaf data type to be specified explicitly?

After another day's worth of hacking, I now understand the reason for
the above: when an index is less than a page and an incoming value would
still fit on the root page, the incoming value is simply dumped into a
leaf tuple without ever calling any opclass-specific function at all.
To allow the leaf datatype to be different from the indexed column,
we'd need at least one more opclass support function, and it's not clear
that the potential gain is worth any extra complexity.

However, I now have another question: what is the point of the
allTheSame mechanism?  It seems to add quite a great deal of complexity,
without saving much of any space.  At least for the node key types used
so far, the null bitmap that's added to the node tuples eats just as
much space as storing a normal key would.  We could probably avoid that
by using custom tuple construction code instead of index_form_tuple,
but on the whole I think it'd be better to remove the concept.  For
one thing, it's giving me fits while attempting to fix the limitation
on storing long indexed values.  There's no reason why a suffix tree
representation shouldn't work for long strings, but you have to be
willing to cap the length of any given inner tuple's prefix to something
that will fit on a page --- and that breaks the badly-underdocumented
allTheSame logic in spgtextproc.c.  You can't choose to just drop the
node key (i.e., the next byte in the original string) unless there isn't
any because you reached the end of the string.  In general it's not
clear to me why it's sensible to drop a node key ever.

I'm also still wondering what your thoughts are on storing null values
versus full-index-scan capability.  I'm leaning towards getting rid of
the dead code, but if you have an idea how to remove the limitation,
maybe we should do that instead.

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] %TYPE and array declaration patch review

2011-12-10 Thread Pavel Stehule
2011/12/10 Greg Smith g...@2ndquadrant.com:
 On 11/30/2011 10:42 AM, Pavel Stehule wrote:

 Regress tests are really large - it is question if about 900 lines is
 necessary - should be more compact



 Can't recall the last time I heard a complaint about having too many
 regression tests for new code.  We've got some bit rot, code convention
 suggestions, test scope concerns, and some fundamental design questions
 hanging over this one.  I don't think anyone expected this TODO item was
 going to turn into a 2626 line patch, but discovering a bunch of unexpected
 complexity underneath one of those is a regular event.  Stuff that's
 straighforward to implement tends to just get done instead of added there.

this is just my opinion, it's not blocker (about regress tests length)

but maybe this patch should be divided to some smaller parts


Regards

Pavel

 Tom's concerns about the grammar rewrite and way parsing is handled here
 seem the worst blockers for committing this, and I can't imagine how those
 could be resolved before this CommitFest is over.  I'm going to mark this
 one as returned with that and Pavel's feedback.  Perhaps Wojciech or someone
 else will come up with a clever way to address this, one that has less
 impact on existing code.  Adding more parser complexity than absolutely
 necessary is a much bigger problem than the regression tests being too long.

 --
 Greg Smith   2ndQuadrant US    g...@2ndquadrant.com   Baltimore, MD
 PostgreSQL Training, Services, and 24x7 Support  www.2ndQuadrant.us


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

-- 
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] plpython SPI cursors

2011-12-10 Thread Peter Eisentraut
On mån, 2011-12-05 at 13:12 -0500, Bruce Momjian wrote:
 Jan Urbański wrote:
  On 05/12/11 18:58, Peter Eisentraut wrote:
   On ons, 2011-11-23 at 19:58 +0100, Jan Urba?ski wrote:
   On 20/11/11 19:14, Steve Singer wrote:
   Responding now to all questions and attaching a revised patch based on 
   your comments.
   
   Committed
   
   Please refresh the other patch.
  
  Great, thanks!
  
  I'll try to send an updated version of the other patch this evening.
 
 I assume this is _not_ related to this TODO item:
 
   Add a DB-API compliant interface on top of the SPI interface 

No, but this is:
http://petereisentraut.blogspot.com/2011/11/plpydbapi-db-api-for-plpython.html



-- 
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] static or dynamic libpgport

2011-12-10 Thread Andrew Dunstan



On 12/09/2011 06:27 PM, Bruce Momjian wrote:

In the Fedora world, a static lib would go in postgresql-devel, but a
dynamic lib would go in postgresql-libs, which is also where libpq is
shipped.

I am not against shipping a dynamic libpgport, but I will just point out
that this was never intended or anticipated.  Are there any symbols in
there that might conflict with other software?





Possibly. Below is a list of symbols from a recent build. The other 
thing is we'd need to turn on flags that make the object suitable for a 
dynamic library (e.g. -fpic). I'm not sure if that has any significant 
impact - probably not or else people would avoid using them far more.


I think we should deal with this. Just Peter's and Steve's responses 
upthread indicate a demand, and I came into this because a customer 
encountered enormous difficulty in doing an out of tree build of a well 
known piece of support software, so this is clearly a pain point. If 
we're not going to do it, we should probably think about adjusting 
people's expectations.


   [andrew@diego port]$ nm libpgport.a | grep ' T '
    T strlcat
    T strlcpy
    T getpeereid
    T pg_get_encoding_from_locale
    T pgfnames
   01a0 T pgfnames_cleanup
   01e0 T rmtree
   0240 T find_my_exec
   05f0 T find_other_exec
   0500 T pclose_check
   0760 T set_pglocale_pgservice
    T inet_net_ntop
   0030 T pg_set_block
    T pg_set_noblock
   0280 T canonicalize_path
   0110 T first_dir_separator
   0140 T first_path_var_separator
   0800 T get_doc_path
   0720 T get_etc_path
   0860 T get_home_path
   0820 T get_html_path
   0740 T get_include_path
   0780 T get_includeserver_path
   07a0 T get_lib_path
   07e0 T get_locale_path
   0840 T get_man_path
   08e0 T get_parent_directory
   0760 T get_pkginclude_path
   07c0 T get_pkglib_path
   0690 T get_progname
   0700 T get_share_path
   01b0 T join_path_components
   0170 T last_dir_separator
   01a0 T make_native_path
   0560 T path_contains_parent_reference
   0630 T path_is_prefix_of_path
   0610 T path_is_relative_and_below_cwd
    T pg_check_dir
    T pg_mkdir_p
    T pg_usleep
   0330 T pg_ascii_tolower
   0310 T pg_ascii_toupper
    T pg_strcasecmp
   0100 T pg_strncasecmp
   0290 T pg_tolower
   0210 T pg_toupper
   00d0 T pg_qsort
   00f0 T qsort_arg
    T simple_prompt
   0010 T pqGetpwuid
    T pqStrerror


cheers

andrew

--
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_cancel_backend by non-superuser

2011-12-10 Thread Greg Smith

On 10/02/2011 05:32 PM, Tom Lane wrote:

I'm with Noah on this. If allowing same-user cancels is enough to solve
95% or 99% of the real-world use cases, let's just do that.


And we're back full circle.  This is basically where Josh Kuperschmidt 
started in early 2010:  
http://archives.postgresql.org/message-id/4ec1cf761002051455i6e702999y7cf4699b4eb48...@mail.gmail.com


Then Torello's patch initially more ambitious patch got pruned down to 
the same sort of feature set.


Next, the day after the November CommitFest started (so it got kind of 
lost), Edward Muller took a shot at coding exactly this too, which he 
tells me happened without even knowing the other two were already 
floating around:  
http://archives.postgresql.org/message-id/cabm0hdx+xuc3dsncnb2z2mertw3crcc5kjmvh6kwhb7jnix...@mail.gmail.com


The picture of what people really want here is pretty clear now, after 
different people wanted same-user cancels (or more) badly enough to 
submit a patch adding it, in three cases now.


--
Greg Smith   2ndQuadrant USg...@2ndquadrant.com   Baltimore, MD
PostgreSQL Training, Services, and 24x7 Support  www.2ndQuadrant.us


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


Re: [HACKERS] psql line number reporting from stdin

2011-12-10 Thread Peter Eisentraut
On fre, 2011-12-09 at 13:44 -0500, Tom Lane wrote:
 Peter Eisentraut pete...@gmx.net writes:
  The problem is, this breaks the regression tests, because first the
  actual output changes, and second the line numbers get included, which
  will create a mess every time you edit a test.  Not sure whether we can
  work around that.  Ideas?
 
 Ugh, that's pretty nearly a deal-breaker.  Would it be sane to have a
 command line switch the regression test driver could specify to prevent
 inclusion of this info?

Perhaps.  I was thinking we could use an environment variable when
running under pg_regress.  This could also help, e.g., ecpg.


-- 
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] [v9.2] Fix Leaky View Problem

2011-12-10 Thread Kohei KaiGai
2011/12/9 Robert Haas robertmh...@gmail.com:
 On Thu, Dec 8, 2011 at 5:17 PM, Kohei KaiGai kai...@kaigai.gr.jp wrote:
 My first impression remind me an idea that I proposed before, even
 though it got negative response due to user visible changes.
 It requires superuser privilege to create new operators, since we
 assume superuser does not set up harmful configuration.

 I don't think that's acceptable from a usability point of view; this
 feature is important, but not important enough to go start ripping out
 other features that people are already using, like non-superuser
 operators.  I'm also pretty skeptical that it would fix the problem,
 because the superuser might fail to realize that creating an operator
 was going to create this type of security exposure.  After all, you
 and I also failed to realize that, so it's obviously a fairly subtle
 problem.

OK, I agree with your opinion. It may stand on a fiction story that
superuser understand all effects and risk of his operations.
If this assumption get broken, system's security is also broken.

 I feel like there must be some logic in the planner somewhere that is
 looking through the subquery RTE and figuring out that safe_foo.a is
 really the same variable as foo.a, and which therefore feels entitled
 to apply !!'s selectivity estimator to foo.a's statistics.  If that's
 the case, it might be possible to handicap that logic so that when the
 security_barrier flag is set, it doesn't do that, and instead treats
 safe_foo.a as a black box.  That would, obviously, degrade the quality
 of complex plans involving security views, but I think we should focus
 on getting something that meets our security goals first and then try
 to improve performance later.

I tried to investigate the code around size-estimation, and it seems to
me here is two candidates to put this type of checks.

The one is examine_simple_variable() that is used to pull a datum
from statistic information, but it locates on the code path restriction
estimator of operators; so user controlable, although it requires least
code changes just after if (rte-rtekind == RTE_SUBQUERY).
The other is clause_selectivity(). Its code path is not user controlable,
so we can apply necessary checks to prevent qualifier that reference
variable come from sub-query with security-barrier.

In my sense, clause_selectivity() is better place to apply this type of
checks. But, on the other hand, it provides get_relation_stats_hook
to allow extensions to control references to statistic data.
So, I wonder whether the PG core assumes this routine covers
all the code path here?


In addition, I also consider the case when we add a functionality that
forcibly adds restriction on WHERE clause of regular tables in the
future version, like:
  SELECT * FROM t WHERE a  0;
  ==  SELECT * FROM t WHERE sepgsql_policy(selinux_label) AND a  0;
Probably, same solution will be available to avoid unintentional references
to pg_statistic; as long as security_barrier is set on rte of regular tables.

Thanks,
-- 
KaiGai Kohei kai...@kaigai.gr.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] Review of VS 2010 support patches

2011-12-10 Thread Brar Piening

Andrew Dunstan wrote:


In the absence of reaction to this I've marked the patch as waiting 
on author, but if/when I have time I'll work on rearranging things as 
above.


Sorry for my non-reaction.

I'm currently trying to find some time window in my before chrismas 
schedule but it seems like I can't guarantee anything.


Anyhow I'll try to make it happen within this year.

Regards,

Brar

--
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] %TYPE and array declaration patch review

2011-12-10 Thread Wojciech Muła
On Sat, 10 Dec 2011 10:22:54 -0500 Greg Smith g...@2ndquadrant.com
wrote:

 Tom's concerns about the grammar rewrite and way parsing is handled
 here seem the worst blockers for committing this, and I can't imagine
 how those could be resolved before this CommitFest is over.  I'm
 going to mark this one as returned with that and Pavel's feedback.
 Perhaps Wojciech or someone else will come up with a clever way to
 address this, one that has less impact on existing code.  Adding more
 parser complexity than absolutely necessary is a much bigger problem
 than the regression tests being too long.

My first attempt to fix the issue was very short  simple, I slightly
modified existing C code to handle additional suffixes, see
http://archives.postgresql.org/pgsql-hackers/2011-08/msg00998.php

Pavel reviewed this and pointed that would be great if we were fix 
related issues. Thus, the second patch try to resolve:
1. handling array indices;
2. copying types;
3. handling %TYPE/%ROWTYPE in procedure definition.

Ad 1 - IMHO first attempt is good enough :) (except coding style)

Ad 2 - this is broken now, just few cases are handled correctly.
My code handled more cases manually, but now I see this have to
be solved with kind of generic pattern-matching procedure (maybe
with backtracking) - I hope shorter, cleaner and easier to understand
and maintain. Tom Lane wrote it would be good if resolving types were
handled in core, but IMO only PL/pgsql code will use this procedure.

Ad 3 - minor issue, could be fixed anytime


So, my proposition is to drop current approach and back to the
first proposition. Copying types have to be considered as separate
issue.

regards
Wojciech Muła

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


[HACKERS] CommitFest 2011-11 Update

2011-12-10 Thread Greg Smith
All of the information on the CommitFest app is as accurate as I could 
make it now, I made a pass over every open thread there to look for 
major changes that hadn't gotten message ID archive links.  Since the 
official start 13 patches have been committed and 6 bounced out.  
Reminder notes have gone out to most of the people who there's something 
waiting for, mostly off-list nagging, and several people have already 
gotten back to say they're planning to hash out open issues over this 
weekend.


Of the 34 patches still waiting for review or their author, there's a 
couple of the usual suspects responsible for a good chunk of them.


Greg Smith:  10 patches that troublemaker is meddling with in some 
form.  I'm clearly not going anywhere this upcoming week, as I've had 
panic over closing of CommitFest in the middle of December on my 
calendar for a while.  General plan of attack is:


-Try and break the deadlock over what to do with 
pg_last_xact_insert_timestamp (done with that for now)

-Update Configuration include directory to fix all of Noah's suggestions
-Make a similar pass over includeifexists in configuration file, which 
is a little cut and paste from the first one and may have some of the 
same errors.
-Revisit the unite recovery.conf and postgresql.conf from a new 
perspective that presumes those two features are coming.  I suspect we 
can implement some of the what I'd like is this requests people wanted 
here with these features, to chop away enough edge cases to make this 
more tractable.
-Mixed with the bigger above bits, during smaller chunks of time help 
rework pg_terminate_backend and pg_cancel_backend by not administrator 
user, Measuring relation free space, and Separate pg_stat_activity 
into current_query into state and query columns  features to commit 
quality.
-Resume slogging through the controversy around Core extensions 
relocation and see if that goes anywhere.
-Join in on any benchmarking help I can provide for some of Robert's 
patches, starting with Make pgBufferUsage track dirty buffers
-Continued work with Peter G on pg_stat_statements normalization.  I 
consider a major feature in the Performance Release theme 9.2 has 
taken on.


There's a normal sized plate lined up for Robert since he's been keeping 
up better; in no particular order:


-More review on the always a new surprise Fix Leaky Views Problem, again
-Extra fun with Tuplesort comparison overhead reduction
-His own avoid taking two snapshots per query and FlexLocks improvements

And then Dimitri is on:

-His Command Triggers
-Review on Prep object creation hooks
-Another likely pass over Robert's avoid taking two snapshots per query

Other people in similarly loaded and overlapping lists with the above 
include Fujii Masao and KaiGai Kohei.  So about half of the open items 
are from contributors who have a track record of just coming back for 
more every time.  It's not like we're going to wonder off based on 
whether our submission goes in now or we have to keep chugging along.  I 
think most of the above is achievable in 9.2.


What I'm happy about is that I'm not seeing any giant controversial 
things here, the sort that tend to hit the last CF and then push its 
boundary back too.  Last year at this time, patches on the table at this 
point were things like Extensions, Sync Rep, Per-column collation, and 
KNN-gist.  Those had the bad mix of we want this feature for the 
relase and this is hard.  That pushed some of them into early March 
before they got committed.  I think only Dimitri's Command Triggers has 
that sort of potential this time.  And I've been talking with him enough 
about the plan there to feel it chunks into useful pieces pretty well if 
the target feature set has to scale back.  We'll have to see if anyone 
tries to sneak one of the more complicated things into the final CF.


--
Greg Smith   2ndQuadrant USg...@2ndquadrant.com   Baltimore, MD
PostgreSQL Training, Services, and 24x7 Support  www.2ndQuadrant.us


--
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] Review of VS 2010 support patches

2011-12-10 Thread Greg Smith

On 12/10/2011 12:58 PM, Brar Piening wrote:
I'm currently trying to find some time window in my before chrismas 
schedule but it seems like I can't guarantee anything.


Anyhow I'll try to make it happen within this year.


That's fair, and Andrew or something else may get an itch to just plow 
forward and do it themselves.  I'm going to mark this one returned with 
feedback for now.  So long as we get an update from you before the 
January 15th CommitFest, this should still be feasible to slip into 9.2.


--
Greg Smith   2ndQuadrant USg...@2ndquadrant.com   Baltimore, MD
PostgreSQL Training, Services, and 24x7 Support  www.2ndQuadrant.us


--
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] Caching for stable expressions with constant arguments v3

2011-12-10 Thread Marti Raudsepp
On Sat, Dec 10, 2011 at 16:50, Greg Smith g...@2ndquadrant.com wrote:
 Or should we wait for a new update based on the
 feedback that Heikki and Tom have already provided first, perhaps one that
 proposes fixes for these two test cases?

Yes, I will post and updated and rebased version tomorrow.

 Now that you're settling into the git workflow, you might consider
 publishing updates to a site like Github in the future too.

It's in the 'cache' branch on my Github:
https://github.com/intgr/postgres/commits/cache
(I linked to it in the v3 posting, but forgot to mention it later)

Regards,
Marti

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

2011-12-10 Thread Jaime Casanova
On Mon, Nov 28, 2011 at 1:32 PM, Julien Tachoires jul...@gmail.com wrote:
 Hi Jaime,

 Please find a new version.


cool

 2) after CLUSTER the index of the toast table gets moved to the same
 tablespace as the main table


there is still a variant of this one, i created 3 tablespaces (datos_tblspc):


create table t1 (
 i serial primary key,
 t text
) tablespace datos_tblspc;

ALTER TABLE t1 SET TOAST TABLESPACE pg_default;
CLUSTER t1 USING t1_pkey;



 now, if we are now supporting this variants
 ALTER TABLE SET TABLE TABLESPACE
 ALTER TABLE SET TOAST TABLESPACE

 why not also support ALTER TABLE SET INDEX TABLESPACE which should
 have the same behaviour as ALTER INDEX SET TABLESPACE... just an idea,
 and of course not necessary for this patch


any opinion about this? maybe i can make a patch for that if there is
consensus that it could be good for symettry

-- 
Jaime Casanova         www.2ndQuadrant.com
Professional PostgreSQL: Soporte 24x7 y capacitación

-- 
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] static or dynamic libpgport

2011-12-10 Thread Tom Lane
Andrew Dunstan and...@dunslane.net writes:
 On 12/09/2011 06:27 PM, Bruce Momjian wrote:
 I am not against shipping a dynamic libpgport, but I will just point out
 that this was never intended or anticipated.  Are there any symbols in
 there that might conflict with other software?

 Possibly. Below is a list of symbols from a recent build.

This doesn't seem like much of an issue to me, since anything wanting to
link against libpgport would be designed to work with whatever it
provides, no?

 The other 
 thing is we'd need to turn on flags that make the object suitable for a 
 dynamic library (e.g. -fpic).

Right now, libpq laboriously rebuilds all the .o files it needs from
src/port/ so as to get them with -fpic.  It would be nice if we could
clean that up while we're doing this.  It might be all right to always
build the client-side version of libpgport with -fpic, though I'd be sad
if that leaked into the server-side build.

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_stat_statements with query tree based normalization

2011-12-10 Thread Peter Geoghegan
On 10 December 2011 13:56, Greg Smith g...@2ndquadrant.com wrote:
 I heard about some bitrot creeping in here too, but it seems gone now; I had
 no problem merging Peter's development branch against master.  I've attached
 a newer patch of the main code, which fixes most of the earlier issues there
 were disclaimers about.

I'm aware of some further bugs in the patch that Greg posted regarding
the synchronisation of executor and planner plugins, so please bear
with me while I squash them.

-- 
Peter Geoghegan       http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training and 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] Command Triggers

2011-12-10 Thread Andres Freund
Hi Peter,


On Sunday, December 04, 2011 08:01:34 PM Andres Freund wrote:
 On Sunday, December 04, 2011 05:34:44 PM Tom Lane wrote:
  Andres Freund and...@anarazel.de writes:
   I have two questions now:
   
   First, does anybody think it would be worth getting rid of the
   duplication from OpenIntoRel (formerly from execMain.c) in regard to
   DefineRelation()?
  
  That's probably reasonable to do, since as you say it would remove the
  opportunity for bugs-of-omission in the CTAS table creation step.
  OTOH, if you find yourself having to make any significant changes to
  DefineRelation, then maybe not.
 Building a CreateStmt seems to work well enough so far.
 The only problem with that approach so far that I found is that:

 CREATE TABLE collate_test2
 ( 
a int,
 b text COLLATE POSIX
 );
 
 CREATE TABLE collate_test1
 ( 
a int,
 b text COLLATE C NOT NULL
 );
 
 CREATE TABLE test_u AS SELECT a, b FROM collate_test1 UNION ALL SELECT a,
 b  FROM collate_test2; -- fail

 failed with:
 ERROR:  no collation was derived for column b with collatable type text
 HINT:  Use the COLLATE clause to set the collation explicitly.
 works now.
Could you explain why the above should fail? After all the UNION is valid 
outside the CREATE TABLE and you can even sort on b.

That tidbit is he only thing I couldn't quickly solve since the last 
submission...


Andres

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