Re: [HACKERS] pg_background (and more parallelism infrastructure patches)

2014-09-20 Thread Amit Kapila
On Fri, Sep 12, 2014 at 12:07 AM, Robert Haas robertmh...@gmail.com wrote:
 On Thu, Sep 11, 2014 at 7:34 AM, Amit Kapila amit.kapil...@gmail.com
wrote:
  Don't we need some way to prohibit changing GUC by launching process,
  once it has shared the existing GUC?

 Nope.  I mean, eventually, for true parallelism ... we absolutely will
 need that.  But pg_background itself doesn't need that; it's perfectly
 fine for configuration settings to get changed in the background
 worker.  So it's a different piece of infrastructure from this patch
 set.

Okay, but as there is no predictability (it can be either same as what
launching process has at the when it has launched background worker
or it could be some different value if got changed later due to sighup)
which GUC value will be used by background worker, it might be good
to clarify the same in pg_bacground docs (which are not there currently,
but I assume it will eventually be part of this patch).


  3.
  Can't we handle all these or other changes inside exec_simple_query()
  based on some parameter or may be a use a hook similar to what we
  do in ProcessUtility?
 
  Basically it looks bit odd to have a duplicate (mostly) copy of
  exec_simple_query().

 It is.  But I didn't think hacking up exec_simple_query() was a better
 option.  We could do that if most people like that approach, but to me
 it seemed there were enough differences to make it unappealing.

Okay, but I think in that case we need to carefully evaluate the
differences else it might lead to behaviour differences in statement
execution.  Few observations related to differences are as follows:

1.
Keeping transaction control (Start/commit) outside the function
execute_sql_string() could lead to EndCommand() message being
sent before transaction end which could be a problem in case
transaction commit fails due to any reason. exec_simple_query() takes
care of the same by calling finish_xact_command() before reporting
command-complete for last parse tree.  It even has comment indicating
that we should follow this protocol.


2.
+static void
+execute_sql_string(const char *sql)
{
..

+ /* Be sure to advance the command counter after the last script command */

+ CommandCounterIncrement();
}

Won't CommandCounterIncrement() required after every command like
we do in exec_simple_query()?

3.
+static void
+execute_sql_string(const char *sql)
{
..
+ /*
+ * Send a CommandComplete message even if we suppressed the query
+
 * results.  The user backend will report these in the absence of
+ * any true query results.
+
 */
+ EndCommand(completionTag, DestRemote);
+
+ /* Clean up the portal. */
+
PortalDrop(portal, false);
..
}

Whats the need to reverse the order of execution for EndCommand()
and PortalDrop()?  Any error in PortalDrop() will lead to wrong
message being sent to client.

4.
What is the reason for not logging statements executed via
pg_background, even though it allows to report the sql statement
to various monitoring tools by setting debug_query_string?

5.
Isn't it better to add a comment why  execute_sql_string() uses
binary format for result?

  4.
  Won't it be better if pg_background_worker_main() can look more
  like PostgresMain() (in terms of handling different kind of messages),
  so that it can be extended in future to handle parallel worker.

 I don't think that a parallel worker will look like pg_background in
 much more than broad outline.  Some of the same constructs will get
 reused, but overall I think it's a different problem that I'd rather
 not conflate with this patch.

No issues.

  5.
  pg_background_result()
  {
  ..
  /* Read and processes messages from the shared memory queue. */
  }
 
  Shouldn't the processing of messages be a separate function as
  we do for pqParseInput3().

 I guess we could.  It's not all that much code, though.

Sure, please take a call based on what you feel is right here, I
mentioned it because I felt it might be little bit easier for other people
to understand that code.

Some other comments are as follows:

1.
+pg_background_result(PG_FUNCTION_ARGS)
{
..
..
+ /*
+ * Whether we succeed or fail, a future invocation of this function
+
 * may not try to read from the DSM once we've begun to do so.
+ * Accordingly, make arrangements to
clean things up at end of query.
+ */
+ dsm_unkeep_mapping(info-seg);

There can be certain scenarios where user might like to invoke this
again.  Assume, user calls function
pg_background_launch('select count(*) from t1') and this statement
execution via background worker is going to take very long time before
it could return anything. After sometime user tries to retrieve data via
pg_background_result(), but the call couldn't came out as it is waiting
for results, so user presses cancel and on again trying after sometime,
he won't get any data.  I think this behaviour is bit annoying.

I am able to reproduce this by halting the background worked via
debugger.

postgres=# select pg_background_launch('select 

Re: [HACKERS] RLS Design

2014-09-20 Thread Craig Ringer
On 09/20/2014 12:38 AM, Stephen Frost wrote:

 I would not (nor do I feel that I did..) have committed it over a
 specific request to not do so from another committer.  I had been hoping
 that there would be another review coming from somewhere, but there is
 always a trade-off between waiting longer to get a review ahead of a
 commit and having it committed and then available more easily for others
 to work with, review, and generally moving forward.

Y'know what helps with that? Publishing clean git branches for
non-trivial work, rather than just lobbing patches around.

I'm finding the reliance on a patch based workflow increasingly
frustrating for complex work, and wonder if it's time to revisit
introducing a git repo+ref to the commitfest app.

I find the need to find the latest patch on the list, apply it, and fix
it up really frustrating. git am --3way helps a lot, but only if the
patch is created with git format-patch.

Perhaps it's time to look at whether git can do more to help us with the
testing and review process.

-- 
 Craig Ringer   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training  Services


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


[HACKERS] WITH CHECK OPTION bug [was RLS Design]

2014-09-20 Thread Dean Rasheed
On 20 September 2014 06:13, Andrew Gierth and...@tao11.riddles.org.uk wrote:
 Adam == Brightwell, Adam adam.brightw...@crunchydatasolutions.com 
 writes:

  Adam At any rate, this appears to be a previously existing issue
  Adam with WITH CHECK OPTION.  Thoughts?

 It's definitely an existing issue; you can reproduce it more simply,
 no need to mess with different users.

 The issue as far as I can tell is that the withCheckOption exprs are
 not being processed anywhere in setrefs.c, so it only works at all by
 pure fluke: for most operators, the opfuncid is also filled in by
 eval_const_expressions, but for whatever reason SAOPs escape this
 treatment. Same goes for other similar cases:

 create table colors (name text);
 create view vc1 as select * from colors where name is distinct from 'grue' 
 with check option;
 create view vc2 as select * from colors where name in ('red','green','blue') 
 with check option;
 create view vc3 as select * from colors where nullif(name,'grue') is null 
 with check option;

 insert into vc1 values ('red'); -- fails
 insert into vc2 values ('red'); -- fails
 insert into vc3 values ('red'); -- fails


Oh dear. I remember thinking at the time I wrote the WITH CHECK OPTION
stuff that I needed to check all the places that did returningLists
processing, because they would probably need similar processing for
withCheckOptionLists, but somehow I missed that one place.

Fortunately it looks pretty trivial though. The patch attached fixes
the above test cases.

Obviously this needs to be fixed in 9.4 and HEAD.

Regards,
Dean
diff --git a/src/backend/optimizer/plan/setrefs.c b/src/backend/optimizer/plan/setrefs.c
new file mode 100644
index 5bf84c1..9ddc8ad
*** a/src/backend/optimizer/plan/setrefs.c
--- b/src/backend/optimizer/plan/setrefs.c
*** set_plan_refs(PlannerInfo *root, Plan *p
*** 696,701 
--- 696,704 
  Assert(splan-plan.targetlist == NIL);
  Assert(splan-plan.qual == NIL);
  
+ splan-withCheckOptionLists =
+ 	fix_scan_list(root, splan-withCheckOptionLists, rtoffset);
+ 
  if (splan-returningLists)
  {
  	List	   *newRL = NIL;

-- 
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_receivexlog and replication slots

2014-09-20 Thread Amit Kapila
On Wed, Sep 10, 2014 at 10:09 AM, Michael Paquier michael.paqu...@gmail.com
wrote:


 New patches taking into account all those comments are attached.

I have looked into refactoring related patch and would like
to share my observations with you:

1.
+ * Run IDENTIFY_SYSTEM through a given connection and give back to caller
+ * some result information if
requested:
+ * - Start LSN position
+ * - Current timeline ID
+ * - system identifier

This API also gets plugin if requested, so I think it is better
to mention the same in function header as well.

2.
+ /* Get LSN start position if necessary */
+ if (sscanf(PQgetvalue(res, 0, 2), %X/%X, hi, lo)
!= 2)
+ {
+ fprintf(stderr,
+ _(%s: could not parse transaction
log location \%s\\n),
+ progname, PQgetvalue(res, 0, 2));
+
return false;
+ }
+ if (startpos != NULL)
+ *startpos = ((uint64) hi)  32 | lo;

Isn't it better if we try to get the LSN position only incase
startpos!=NULL (as BaseBackup don't need this)

3.
/*
- * Run IDENTIFY_SYSTEM so we can get the timeline and current xlog
- * position.
+ * Identify
server, obtaining start LSN position and current timeline ID
+ * at the same time.
  */

I find existing comments okay, is there a need to change
in this case?  Part of the new comment mentions
obtaining start LSN position, actually the start position is
identified as part of next function call FindStreamingStart(),
only incase it return InvalidXLogRecPtr, the value returned
by IDENTIFY_SYSTEM will be used.

4.
  /*
  * Figure out where to start streaming.
@@ -492,6 +459,12 @@ main(int argc, char **argv)

pqsignal(SIGINT, sigint_handler);
 #endif

+ /* Obtain a connection before doing anything */
+ conn
= GetConnection();
+ if (!conn)
+ /* Error message already written in GetConnection() */
+
exit(1);
+

Is there a reason for moving this function out of StreamLog(),
there is no harm in moving it, but there doesn't seem to be any
problem even if it is called inside StreamLog().

5.
+bool
+RunIdentifySystem(PGconn *conn, char **sysid, TimeLineID *starttli,
+
XLogRecPtr *startpos, char **plugin)
+{
+ PGresult   *res;
+ uint32 hi, lo;
+
+ /* Leave if
no connection present */
+ if (conn == NULL)
+ return false;

Shouldn't there be Assert instead of if (conn == NULL), as
RunIdentifySystem() is not expected to be called without connection.

6.
main()
{
..

+ /* Identify system, obtaining start LSN position at the same time */
+ if (!RunIdentifySystem(conn,
NULL, NULL, startpos, plugin_name))
+ disconnect_and_exit(1);
+
+ /*
+ * Check that there
is a plugin name, a logical slot needs to have
+ * one absolutely.
+ */
+ if (!plugin_name)
+ {
+
fprintf(stderr,
+ _(%s: no plugin found for replication slot \%s
\\n),
+ progname, replication_slot);
+ disconnect_and_exit(1);
+
}
+
+ /* Stream loop */

a.
Generally IdentifySystem is called as first API, but I think you
have changed its location after CreateReplicationSlot as you want
to double check the value of plugin, not sure if that is necessary or
important enough that we start calling it later.
b.
Above call is trying to get startpos, won't it overwrite the value
passed by user (case 'I':) for startpos?

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


Re: [HACKERS] WITH CHECK OPTION bug [was RLS Design]

2014-09-20 Thread Michael Paquier
On Sat, Sep 20, 2014 at 7:03 AM, Dean Rasheed dean.a.rash...@gmail.com wrote:
 Fortunately it looks pretty trivial though. The patch attached fixes
 the above test cases.
 Obviously this needs to be fixed in 9.4 and HEAD.
Wouldn't it be better if bundled with some regression tests?
-- 
Michael


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


Re: [HACKERS] gist vacuum seaq access

2014-09-20 Thread Костя Кузнецов
Heikki.I have idea. when i begining vacuum i must create structure like hash table. And the first action i fill this hash table.in code this look like:for( blkno = ..; all pages; blkno++) {  if(! gistPageIsLeaf(blkno)) { for( all tuples in blkno ) { hash[ blkno that is referenced by the tuple] = blkno; }  }}getting is parent of page is fast and simple( hash[page] ).But this solution is need 2 full reading of index. but Alexander says me that: this solution may be have a problem, because there is maintenance_work_mem.



[HACKERS] Should we excise the remnants of borland cc support?

2014-09-20 Thread Andres Freund
Hi,

At the moment there's some rememnants of support for borland CC. I don't
believe it's likely that any of it still works. I can't remember ever
seing a buildfarm animal running it either - not surprising it's ~15
years since the last release.
Since there's both msvc and mingw support for windows builds - borlands
only platform - I see little point in continuing to support it.

The reason I'm wondering is that the atomics patch cargo cults forward
some stuff specific to borland and I'd rather not do that. And I'd
rather be explicit about stopping to do so than slyly doing 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


[HACKERS] Help to startup

2014-09-20 Thread Tapan Halani
Hello everyone..i am new to PostgreSQL project. I had prior experience with
sql+ , with oracle 11g database server. Kindly help me grasp more about the
project.

Thank you,
Tapan Halani


Re: [HACKERS] A mechanism securing web applications in DBMS

2014-09-20 Thread Zhaomo Yang
Stephen,

 Yes- but that's pretty trivially done, given that you've stipulated that
 a single connection DB connection must be used from authentication until
 de-authentication.  All that is needed is an additional column in the
 auth table which is populated with a pseudo-random value which is
 guaranteed to be unique and constant for the duration of the
 authenticated time- and the database backend PID is perfect for that.
 The auth function can call the pg_backend_pid() function directly and
 then the policies can include a 'pid = pg_backend_pid()' as part of the
 join to the auth table.  The auth function can also complain loudly if
 an entry in the pid table is found with the current PID during auth (and
 similar- the de-auth function can complain if an entry with the current
 PID is *not* found).  This would eliminate the need for the on-connect
 triggers, I believe (though those are interesting for other reasons..).


You are right. Using unlogged table is a good idea. I'll try it out.
Thanks for your advice!

  It'd be very interesting to see this done with the unlogged table,
 security definer functions, and the row-level policies patch which we're
 working on.  I'd further suggest that the application also use multiple
 roles which are set noinherit and 'set role' based on the operation
 which it's currently being used for- this would add another level of
 protection.  Using stored procedures (for more than just the auth and
 de-auth functions as suggested here) can also be a good idea.


Currently auth functions are security definer functions. I'm gonna try
to create a patch using unlogged table + RLS and put it online (e.g.
this mail list) so that people can try it.

Thanks,
Zhaomo

On Sat, Sep 13, 2014 at 4:00 PM, Stephen Frost sfr...@snowman.net wrote:
 Zhaomo,

 * Zhaomo Yang (zhy...@cs.ucsd.edu) wrote:
  Have you considered just using a regular, but unlogged, table?  That
  would also avoid any risk that the application manages to drop or shadow
  the temp table somehow with a fake table that changes who is currently
  authenticated, and avoids having to figure out how to deal with the temp
  table vanishing due to the connections going away.

 So then all the currently logged in users will be stored in the same
 table, which means we also need to make sure that the correct row in
 that table is used when the row-level security policy refers to the
 current application-level user.

 Yes- but that's pretty trivially done, given that you've stipulated that
 a single connection DB connection must be used from authentication until
 de-authentication.  All that is needed is an additional column in the
 auth table which is populated with a pseudo-random value which is
 guaranteed to be unique and constant for the duration of the
 authenticated time- and the database backend PID is perfect for that.
 The auth function can call the pg_backend_pid() function directly and
 then the policies can include a 'pid = pg_backend_pid()' as part of the
 join to the auth table.  The auth function can also complain loudly if
 an entry in the pid table is found with the current PID during auth (and
 similar- the de-auth function can complain if an entry with the current
 PID is *not* found).  This would eliminate the need for the on-connect
 triggers, I believe (though those are interesting for other reasons..).

 Let me send you a copy of our paper in a separate email which is a
 thorough description of the mechanism (including background, threat
 model, how it works, etc), which should give you an better idea on
 every aspect of the mechanism. Please do not distribute it because it
 has been accepted for publication. Note that the implementation we
 show in the paper is just a prototype (we made the changes so that we
 could implement it quickly). Our goal always is to integrate our
 mechanism into open source DBMS's like PG and MySQL cleanly.

 It'd be very interesting to see this done with the unlogged table,
 security definer functions, and the row-level policies patch which we're
 working on.  I'd further suggest that the application also use multiple
 roles which are set noinherit and 'set role' based on the operation
 which it's currently being used for- this would add another level of
 protection.  Using stored procedures (for more than just the auth and
 de-auth functions as suggested here) can also be a good idea.

 Thanks,

 Stephen


-- 
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] Help to startup

2014-09-20 Thread Andrew Dunstan


On 09/16/2014 01:51 PM, Tapan Halani wrote:


Hello everyone..i am new to PostgreSQL project. I had prior experience 
with sql+ , with oracle 11g database server. Kindly help me grasp more 
about the project.






The first thing you need to do is learn to ask your question in the 
right forum. pgsql-hackers is not that forum. pgsql-general is, but you 
should ask specific questions there, not just completely general and 
essentially unanswerable questions such as this. Before you ask 
questions there you should read the excellent PostgreSQL documentation 
at http://www.postgresql.org/docs/current/static/index.html


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] Should we excise the remnants of borland cc support?

2014-09-20 Thread Andrew Dunstan


On 09/20/2014 09:24 AM, Andres Freund wrote:

Hi,

At the moment there's some rememnants of support for borland CC. I don't
believe it's likely that any of it still works. I can't remember ever
seing a buildfarm animal running it either - not surprising it's ~15
years since the last release.
Since there's both msvc and mingw support for windows builds - borlands
only platform - I see little point in continuing to support it.

The reason I'm wondering is that the atomics patch cargo cults forward
some stuff specific to borland and I'd rather not do that. And I'd
rather be explicit about stopping to do so than slyly doing it.



I thought the Borland stuff was there only so we could build client 
libraries for use with things like Delphi.


It might be worth casting the net a little wider to find out if it still 
has any users.


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_receivexlog and replication slots

2014-09-20 Thread Michael Paquier
On Sat, Sep 20, 2014 at 7:09 AM, Amit Kapila amit.kapil...@gmail.com wrote:
 I have looked into refactoring related patch and would like
 to share my observations with you:
Thanks! Useful input is always welcome.

 1.
 + * Run IDENTIFY_SYSTEM through a given connection and give back to caller
 This API also gets plugin if requested, so I think it is better
 to mention the same in function header as well.
True.

 2.
 + /* Get LSN start position if necessary */
 Isn't it better if we try to get the LSN position only incase
 startpos!=NULL (as BaseBackup don't need this)
OK. Let's do that for consistency with the other fields.

 3.
 I find existing comments okay, is there a need to change
 in this case?  Part of the new comment mentions
 obtaining start LSN position, actually the start position is
 identified as part of next function call FindStreamingStart(),
 only incase it return InvalidXLogRecPtr, the value returned
 by IDENTIFY_SYSTEM will be used.
Hopefully I am following you correctly here: comment has been updated
to mention that the start LSN and TLI fetched from IDENTIFY_SYSTEM are
always fetched but used only if no valid position is found in output
folder of pg_receivexlog.

 4.
 + /* Obtain a connection before doing anything */
 + conn = GetConnection();
 + if (!conn)
 + /* Error message already written in GetConnection() */
 Is there a reason for moving this function out of StreamLog(),
 there is no harm in moving it, but there doesn't seem to be any
 problem even if it is called inside StreamLog().
For pg_recvlogical, this move is done to reduce code redundancy, and
actually it makes sense to just grab one connection in the main() code
path before performing any replication commands. For pg_receivexlog,
the move is done because it makes the code more consistent with
pg_recvlogical, and also it is a preparatory work for the second patch
that introduces the create/drop slot. Even if 2nd patch is not
accepted I figured out that it would not hurt to simply grab one
connection in the main code path before doing any action.

 5.
 Shouldn't there be Assert instead of if (conn == NULL), as
 RunIdentifySystem() is not expected to be called without connection.
Fine for me. That's indeed more consistent with the rest this way.

 6.
 + /* Identify system, obtaining start LSN position at the same time */
 + if (!RunIdentifySystem(conn,
 NULL, NULL, startpos, plugin_name))
 + disconnect_and_exit(1);
 a.
 Generally IdentifySystem is called as first API, but I think you
 have changed its location after CreateReplicationSlot as you want
 to double check the value of plugin, not sure if that is necessary or
 important enough that we start calling it later.
Funny part here is that even the current code on master and
REL9_4_STABLE of pg_recvlogical.c is fetching a start position when
creating a replication slot that is not used as two different actions
cannot be used at the same time. That's a matter of removing this line
in code though:
startpos = ((uint64) hi)  32 | lo;
As that's only cosmetic for 9.4, and this patch makes things more
correct I guess that's fine to do nothing and just try to get this
patch in.

 b.
 Above call is trying to get startpos, won't it overwrite the value
 passed by user (case 'I':) for startpos?
Yep, that's a bug. The value that user could give would have been
overwritten all the time. Current code does not even care about
checking startpos in IDENTIFY_SYSTEM so we're fine by just removing
this part.

Updated patches are attached.
-- 
Michael
From a7743619dc93870c815920872c3b5a6971c25714 Mon Sep 17 00:00:00 2001
From: Michael Paquier mich...@otacoo.com
Date: Mon, 1 Sep 2014 20:48:43 +0900
Subject: [PATCH 1/2] Refactoring of pg_basebackup utilities

Code duplication is reduced with the introduction of new APIs for each
individual replication command:
- IDENTIFY_SYSTEM
- CREATE_REPLICATION_SLOT
- DROP_REPLICATION_SLOT
A couple of variables used to identify a timeline ID are changed as well
to be more consistent with core code.
---
 src/bin/pg_basebackup/pg_basebackup.c  |  21 +
 src/bin/pg_basebackup/pg_receivexlog.c |  52 +++
 src/bin/pg_basebackup/pg_recvlogical.c | 132 +++-
 src/bin/pg_basebackup/streamutil.c | 155 +
 src/bin/pg_basebackup/streamutil.h |  10 +++
 5 files changed, 211 insertions(+), 159 deletions(-)

diff --git a/src/bin/pg_basebackup/pg_basebackup.c b/src/bin/pg_basebackup/pg_basebackup.c
index 8b9acea..0ebda9a 100644
--- a/src/bin/pg_basebackup/pg_basebackup.c
+++ b/src/bin/pg_basebackup/pg_basebackup.c
@@ -1569,8 +1569,8 @@ BaseBackup(void)
 {
 	PGresult   *res;
 	char	   *sysidentifier;
-	uint32		latesttli;
-	uint32		starttli;
+	TimeLineID	latesttli;
+	TimeLineID	starttli;
 	char	   *basebkp;
 	char		escaped_label[MAXPGPATH];
 	char	   *maxrate_clause = NULL;
@@ -1624,23 +1624,8 @@ BaseBackup(void)
 	/*
 	 * Run IDENTIFY_SYSTEM so we can get the timeline
 	 */
-	res = PQexec(conn, 

Re: [HACKERS] Should we excise the remnants of borland cc support?

2014-09-20 Thread Andres Freund
On September 20, 2014 4:03:43 PM CEST, Andrew Dunstan and...@dunslane.net 
wrote:

On 09/20/2014 09:24 AM, Andres Freund wrote:
 Hi,

 At the moment there's some rememnants of support for borland CC. I
don't
 believe it's likely that any of it still works. I can't remember ever
 seing a buildfarm animal running it either - not surprising it's ~15
 years since the last release.
 Since there's both msvc and mingw support for windows builds -
borlands
 only platform - I see little point in continuing to support it.

 The reason I'm wondering is that the atomics patch cargo cults
forward
 some stuff specific to borland and I'd rather not do that. And I'd
 rather be explicit about stopping to do so than slyly doing it.


I thought the Borland stuff was there only so we could build client 
libraries for use with things like Delphi.

That really still relies on a 15 year old compiler?

The stuff I was thinking of - barriers and spinlocks among others - is backend 
only anyway?

Andres

-- 
Please excuse brevity and formatting - I am writing this on my mobile phone.

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] RLS Design

2014-09-20 Thread Josh Berkus
On 09/20/2014 12:23 AM, Craig Ringer wrote:
 On 09/20/2014 12:38 AM, Stephen Frost wrote:
 
 I would not (nor do I feel that I did..) have committed it over a
 specific request to not do so from another committer.  I had been hoping
 that there would be another review coming from somewhere, but there is
 always a trade-off between waiting longer to get a review ahead of a
 commit and having it committed and then available more easily for others
 to work with, review, and generally moving forward.
 
 Y'know what helps with that? Publishing clean git branches for
 non-trivial work, rather than just lobbing patches around.
 
 I'm finding the reliance on a patch based workflow increasingly
 frustrating for complex work, and wonder if it's time to revisit
 introducing a git repo+ref to the commitfest app.
 
 I find the need to find the latest patch on the list, apply it, and fix
 it up really frustrating. git am --3way helps a lot, but only if the
 patch is created with git format-patch.
 
 Perhaps it's time to look at whether git can do more to help us with the
 testing and review process.

We discussed this at the last developer meeting, without coming up with
a written procedure.  Your ideas can help ...


-- 
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] Hide 'Execution time' in EXPLAIN (COSTS OFF)

2014-09-20 Thread Christoph Berg
Re: Andres Freund 2014-06-04 20140604194544.gb...@awork2.anarazel.de
  I'm unconvinced that this'd add much to our regression testing capability,
  though.  The standard thing is to do an EXPLAIN to check the plan shape
  and then run the query to see if it gets the right answer.  Checking row
  counts is pretty well subsumed by the latter, and is certainly not an
  adequate substitute for it.
  
  So on the whole, -1 ... this is an unintuitive and
  non-backwards-compatible change that doesn't look like it buys much.
 
 I've added the regression test I want this for.
 
 0001 is the bugfix making me look into it
 0002 is COSTS OFF removing the display of execution time
 0003 is the regression test
 
 Note that 0003 will require a kill -9 without 0001.
 
 I am not sure myself if the test is really worth it. On one hand it's an
 area that had seen several hard to find bugs over the years and is
 likely to see further changes (e.g. CSN stuff) in the near future, on
 the other hand the tests are tricky and require specific ordering.

Hi,

there's another problem in this area: 9.4 adds Planning time to the
EXPLAIN output. If you don't want to see that, you need to use (costs
off), but this, well, also disables the costs. If you are running
regression tests to actually test the costs, you've lost in 9.4.

This problem just emerged in the Multicorn FDW where the regression
tests were monitoring the costs, but in 9.4 (costs off) kills that.

https://github.com/Kozea/Multicorn/pull/7

Can we have EXPLAIN (timing off) in 9.4+ hide the Planning time
line? That would even be backwards compatible with 9.x where it would
be a no-op.

(I don't have an opinion how much this should affect the EXPLAIN
(analyze, timing off) output, but there's probably a sane solution.)

Christoph
-- 
c...@df7cb.de | http://www.df7cb.de/


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


[HACKERS] conditional query in where has name collision. bug?

2014-09-20 Thread bill wilson
This a toy example from a 'upsert' script that appends new data:

select a from (select  1 as a) as t1 where not exists (select true from
(select 2 as a) t2 where a=a) ;
 a
───
(0 rows)

select a from (select  1 as a) as t1 where not exists (select true from
(select 2 as a) t2 where t1.a=t2.a)  ;
 a
───
 1
(1 row)

Please tell me this a bug. We can see in the first query without naming the
tables that 'a=a' uses table t2 for both columns. Luckily,  the incorrect
first query fails from primary key conditions in real life. This goes
against the implied convention that when in doubt raise an error. It is
also a very idiomatic way to to real life inserts. So, I cannot believe
that I am the first to find this.  The behavior reminds me of old school
mysql: lets allow an int and a string to add and just cast the string as an
int because that must be what the user wants and will be convenient.


Re: [HACKERS] conditional query in where has name collision. bug?

2014-09-20 Thread Andrew Gierth
 bill == bill wilson bill.wilson.h...@gmail.com writes:

 bill This a toy example from a 'upsert' script that appends new data:
 bill select a from (select 1 as a) as t1 where not exists (select
 bill true from (select 2 as a) t2 where a=a) ;
 bill  a
 bill ───
 bill (0 rows)

 bill Please tell me this a bug. We can see in the first query
 bill without naming the tables that 'a=a' uses table t2 for both
 bill columns.

Of course it's not a bug. The column a in the inner scope hides the
column a in the outer scope, as explicitly required by the spec
(6.6 identifier chain in sql2008).

-- 
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] Minor improvement in lock.sgml

2014-09-20 Thread Alvaro Herrera
Robert Haas wrote:
 On Tue, Sep 16, 2014 at 7:20 AM, Etsuro Fujita
 fujita.ets...@lab.ntt.co.jp wrote:
  Here is a patch to a bit improve the reference page for the LOCK
  command.  I think it'd be better for the isolation level to be in
  capitals and wrapped in the literal tags.
 
 It's done that way elsewhere in the same page, so committed.
 
 Overall, there is some ambiguity about this.  Most places follow your
 proposed style if literalREAD COMMITTED/literal,
 literalREPEATABLE READ/literal, and
 literalSERIALIZABLE/literal, but mvcc.sgml, which discusses
 isolation levels extensively, just writes them as Read Committed,
 Repeatable Read, and Serializable.

This use of initial capitals everywhere in the mvcc chapter looks
strange to me.  I would say that that text needs to use firstterm,
like we do elsewhere for terms being explained; and subsequent usage of
the same terms would use them without any particular markup and in
lowercase.  For instance, one paragraph there would be:

   para
The acronymSQL/acronym standard defines four levels of
transaction isolation.  The most strict is firsttermserializable/,
which is defined by the standard in a paragraph which says that any
concurrent execution of a set of serializable transactions is guaranteed
to produce the same effect as running them one at a time in some order.
The other three levels are defined in terms of phenomena, resulting from
interaction between concurrent transactions, which must not occur at
each level.  The standard notes that due to the definition of
serializable, ...

and there's a later one that would look like

   para
In productnamePostgreSQL/productname, you can request any of the
four standard transaction isolation levels.  But internally, there are
only three distinct isolation levels, which correspond to the levels
firsttermread committed/, firsttermrepeatable read/, and the
aforementioned serializable.  When you select the level firsttermread
uncommitted/ you really get read committed, and phantom reads are not 
possible
in the productnamePostgreSQL/productname implementation of repeatable
read, so ...

Maybe there's additional markup of the isolation level names to be had
in appearances other than the first, but I don't know what -- initial
capitals don't seem to cut it.  Maybe we should say the level foo as
appropriate (so the last line above would be implementation of the
level repeatable read)

CLP$10

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