Re: [PATCHES] Updatable views

2008-05-08 Thread Bernd Helmle
--On Mittwoch, Mai 07, 2008 20:38:59 +0100 Simon Riggs 
[EMAIL PROTECTED] wrote:



Where are we on this feature?


Any update, Bernd?


I've merged the patch into current -HEAD and updated some parts. My current 
*working* state can be reviewed at


http://git.postgresql.org/?p=~psoo/postgresql.git;a=shortlog;h=updatable_views

I'm still not sure how to implement a reliable CHECK OPTION, but short on 
time i haven't done a very deep investigation yet. Next idea was to look at 
the updatable cursor stuff, maybe something there can be reused.


--
 Thanks

   Bernd

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


Re: [PATCHES] Updatable views

2008-05-08 Thread Simon Riggs
On Thu, 2008-05-08 at 13:48 +0200, Bernd Helmle wrote:
 --On Mittwoch, Mai 07, 2008 20:38:59 +0100 Simon Riggs 
 [EMAIL PROTECTED] wrote:
 
  Where are we on this feature?
 
  Any update, Bernd?
 
 I've merged the patch into current -HEAD and updated some parts. My current 
 *working* state can be reviewed at
 
 http://git.postgresql.org/?p=~psoo/postgresql.git;a=shortlog;h=updatable_views
 
 I'm still not sure how to implement a reliable CHECK OPTION, but short on 
 time i haven't done a very deep investigation yet. Next idea was to look at 
 the updatable cursor stuff, maybe something there can be reused.

Your earlier patch seemed to add two rules if the view had a with check
option? One with a pass through and another one with a do-nothing and a
where clause.

As I understand it

 CREATE VIEW x AS SELECT * FROM foo WHERE where-clause WITH CHECK OPTION

should generate an INSERT rule like this

 CREATE RULE somename AS ON INSERT TO x WHERE where-clause DO INSERT ...

which seems straightforward, no?

The SQLStandard default is CASCADED and it seems easier not to worry too
much about the LOCAL option until we have the basics working. I'm not
even sure that we *want* the LOCAL option anyway having read what it
means, plus it isn't supported by many other DBMS.

Do you store anything in the catalog to mark the view as updatable or
not? I couldn't see that but it seemed easier than trying to resolve all
of the updatability characteristics at run-time.

I may be able to help some with the patch, if you'd like?

-- 
  Simon Riggs
  2ndQuadrant  http://www.2ndQuadrant.com


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


Re: [PATCHES] Updatable views

2008-05-08 Thread Peter Eisentraut
Am Donnerstag, 8. Mai 2008 schrieb Simon Riggs:
  CREATE RULE somename AS ON INSERT TO x WHERE where-clause DO INSERT ...

 which seems straightforward, no?

Double evaluation is the key word.  The conclusion was more or less that you 
can't implement check constraints using the rules system.  You need to check 
them in the executor.

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


Re: [PATCHES] Updatable views

2008-05-08 Thread Bernd Helmle
--On Donnerstag, Mai 08, 2008 13:28:14 +0100 Simon Riggs 
[EMAIL PROTECTED] wrote:



On Thu, 2008-05-08 at 13:48 +0200, Bernd Helmle wrote:

--On Mittwoch, Mai 07, 2008 20:38:59 +0100 Simon Riggs
[EMAIL PROTECTED] wrote:

 Where are we on this feature?

 Any update, Bernd?

I've merged the patch into current -HEAD and updated some parts. My
current  *working* state can be reviewed at

http://git.postgresql.org/?p=~psoo/postgresql.git;a=shortlog;h=updatabl
e_views

I'm still not sure how to implement a reliable CHECK OPTION, but short
on  time i haven't done a very deep investigation yet. Next idea was to
look at  the updatable cursor stuff, maybe something there can be reused.


Your earlier patch seemed to add two rules if the view had a with check
option? One with a pass through and another one with a do-nothing and a
where clause.

As I understand it

 CREATE VIEW x AS SELECT * FROM foo WHERE where-clause WITH CHECK OPTION

should generate an INSERT rule like this

 CREATE RULE somename AS ON INSERT TO x WHERE where-clause DO INSERT ...



This was indeed the implementation i've proposed. We have rejected this 
idea then because it doesn't work with volatile functions reliable due to 
double evaluation:


http://archives.postgresql.org/pgsql-patches/2006-08/msg00483.php

Tom's example even demonstrates a serious constraint in rule based updates, 
since you get side effects in such conditions you won't expect, even 
without a CHECK OPTION.



which seems straightforward, no?

The SQLStandard default is CASCADED and it seems easier not to worry too
much about the LOCAL option until we have the basics working. I'm not
even sure that we *want* the LOCAL option anyway having read what it
means, plus it isn't supported by many other DBMS.

Do you store anything in the catalog to mark the view as updatable or
not? I couldn't see that but it seemed easier than trying to resolve all
of the updatability characteristics at run-time.


I'm not sure want you mean, but pg_rewrite.ev_kind stores the nature of the 
rule. Updatability is determined by the checkTree() function internally. 
It's easy to query pg_rewrite to examine wether a view is updatable or not.




I may be able to help some with the patch, if you'd like?



You're welcome ;)


--
  Simon Riggs
  2ndQuadrant  http://www.2ndQuadrant.com




--
 Thanks

   Bernd

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


Re: [PATCHES] Updatable views

2008-05-08 Thread Simon Riggs
On Thu, 2008-05-08 at 14:56 +0200, Peter Eisentraut wrote:
 Am Donnerstag, 8. Mai 2008 schrieb Simon Riggs:
   CREATE RULE somename AS ON INSERT TO x WHERE where-clause DO INSERT ...
 
  which seems straightforward, no?
 
 Double evaluation is the key word.  The conclusion was more or less that you 
 can't implement check constraints using the rules system.  You need to check 
 them in the executor.

That makes sense. I can't see how we would make LOCAL CHECK CONSTRAINTs
work with rules anyhow.

So that means WITH CHECK CONSTRAINT is going to end up executed in a
similar place to constraint evaluation on underlying tables.

That leaves me in a difficult position with MERGE though. MERGE does
something similar with conditional-WHEN clause evaluation, plus
transformation of the sub-statements is only sensible when we have
updatable views. :-(

-- 
  Simon Riggs
  2ndQuadrant  http://www.2ndQuadrant.com


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


Re: [PATCHES] Updatable views

2008-05-08 Thread Bernd Helmle
--On Donnerstag, Mai 08, 2008 14:42:50 +0100 Simon Riggs 
[EMAIL PROTECTED] wrote:



That makes sense. I can't see how we would make LOCAL CHECK CONSTRAINTs
work with rules anyhow.


One of the idea's that came up through the discussion was to make the 
rewriter responsible for collecting check constraints such as the local 
check condition. They would be pushed down to the executor then where the 
correct constraints would be applied. However, i'm currently not in the 
position to say if this is doable right now.


The original updatable views patch tracked the state of required and 
applied rule conditions during rewrite. This way it applied only the rule 
conditions of the specified view in cascading updates.


--
 Thanks

   Bernd

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


Re: [PATCHES] Updatable views

2008-05-08 Thread Simon Riggs
On Thu, 2008-05-08 at 17:20 +0200, Bernd Helmle wrote:
 --On Donnerstag, Mai 08, 2008 14:42:50 +0100 Simon Riggs 
 [EMAIL PROTECTED] wrote:
 
  That makes sense. I can't see how we would make LOCAL CHECK CONSTRAINTs
  work with rules anyhow.
 
 One of the idea's that came up through the discussion was to make the 
 rewriter responsible for collecting check constraints such as the local 
 check condition. They would be pushed down to the executor then where the 
 correct constraints would be applied. However, i'm currently not in the 
 position to say if this is doable right now.

That's what I was thinking too.

 The original updatable views patch tracked the state of required and 
 applied rule conditions during rewrite. This way it applied only the rule 
 conditions of the specified view in cascading updates.

Yes, seems like the only way we'll get LOCAL CHECK CONSTRAINTS to work.

Are you planning to work on this?

-- 
  Simon Riggs
  2ndQuadrant  http://www.2ndQuadrant.com


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


Re: [PATCHES] libpq thread-locking

2008-05-08 Thread Bruce Momjian
Magnus Hagander wrote:
 Attached patch adds some error checking to the thread locking stuff in
 libpq. Previously, if thread locking failed for some reason, we would
 just fall through and do things without locking. This patch makes us
 abort() instead. It's not the greatest thing probably, but our API
 doesn't let us pass back return values...

I have looked over the patch and it seems fine, though I am concerned
about the abort() case with no output.  I realize stderr might be going
nowhere, but in fe-print.c we do an fprintf(stderr) for memory failures
so for consistency I think we should do the same here.  If there is
concern about code bloat, I suggest a macro at the top of the file for
thread failure exits:

#define THEAD_FAILURE(str) \
do { \
fprintf(stderr, libpq_gettext(Thread failure:  %s\n)); \
exit(1); \
} while(0)

-- 
  Bruce Momjian  [EMAIL PROTECTED]http://momjian.us
  EnterpriseDB http://enterprisedb.com

  + If your life is a hard drive, Christ can be your backup. +

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


Re: [PATCHES] libpq thread-locking

2008-05-08 Thread Bruce Momjian
Bruce Momjian wrote:
 Magnus Hagander wrote:
  Attached patch adds some error checking to the thread locking stuff in
  libpq. Previously, if thread locking failed for some reason, we would
  just fall through and do things without locking. This patch makes us
  abort() instead. It's not the greatest thing probably, but our API
  doesn't let us pass back return values...
 
 I have looked over the patch and it seems fine, though I am concerned
 about the abort() case with no output.  I realize stderr might be going
 nowhere, but in fe-print.c we do an fprintf(stderr) for memory failures
 so for consistency I think we should do the same here.  If there is
 concern about code bloat, I suggest a macro at the top of the file for
 thread failure exits:
 
   #define THEAD_FAILURE(str) \
   do { \
   fprintf(stderr, libpq_gettext(Thread failure:  %s\n)); \
   exit(1); \
   } while(0)

Oh, this is Tom saying he doesn't like stderr and the added code lines
for failure:

http://archives.postgresql.org/pgsql-patches/2008-04/msg00254.php

I think the macro and consistency suggest doing as I outlined.

-- 
  Bruce Momjian  [EMAIL PROTECTED]http://momjian.us
  EnterpriseDB http://enterprisedb.com

  + If your life is a hard drive, Christ can be your backup. +

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


Re: [PATCHES] [NOVICE] encoding problems

2008-05-08 Thread Bruce Momjian

Applied.

---

Bruce Momjian wrote:
 Alvaro Herrera wrote:
  Bruce Momjian wrote:
   Alvaro Herrera wrote:
  
Surely psql computes the width of all cells before printing anything.
   
   It does, but if you have a value that has a tab, how do you know what
   tab stop you are on because you don't know the final width of the
   previous columns at that time, so there is no way to know the width of
   that cell.
  
  My point is that you don't need to align the tabstops with the start of
  the line, but with the start of the _column_.  So the width of the
  previous column doesn't matter.
 
 Alvaro, using spaces instead of the terminal hard tabs was a very good
 idea.  The output is now:
 
   test= \x
   Expanded display is on.
   
   test= \df+ xx
   List of functions
   -[ RECORD 1 ]---+
   Schema  | public
   Name| xx
   Result data type| text
   Argument data types |
   Volatility  | volatile
   Owner   | postgres
   Language| sql
   Source code | SELECT  'a'::text
   : WHERE   1 = 1
   Description |
 
 
 Patch attached.  It substitutes spaces for the tab.
 
 -- 
   Bruce Momjian  [EMAIL PROTECTED]http://momjian.us
   EnterpriseDB http://enterprisedb.com
 
   + If your life is a hard drive, Christ can be your backup. +


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

-- 
  Bruce Momjian  [EMAIL PROTECTED]http://momjian.us
  EnterpriseDB http://enterprisedb.com

  + If your life is a hard drive, Christ can be your backup. +

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


Re: [PATCHES] [HACKERS] bug in numeric_power() function

2008-05-08 Thread Bruce Momjian

Applied.

---

Bruce Momjian wrote:
 Alvaro Herrera wrote:
  Bruce Momjian wrote:
   Tom Lane wrote:
Bruce Momjian [EMAIL PROTECTED] writes:
 I have developed the attached patch which fixes 0 ^ 123.3.

Did you actually read the wikipedia entry you cited?
  
  But that's about 0^0, not about 0^123.3.  See this other subsection:
  
  http://en.wikipedia.org/wiki/Exponentiation#Powers_of_zero
  
  0^123.3 is 0, not 1.
 
 Ah, got it, and I updated the patch to remove the commment about
 discrete.
 
 -- 
   Bruce Momjian  [EMAIL PROTECTED]http://momjian.us
   EnterpriseDB http://enterprisedb.com
 
   + If your life is a hard drive, Christ can be your backup. +


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

-- 
  Bruce Momjian  [EMAIL PROTECTED]http://momjian.us
  EnterpriseDB http://enterprisedb.com

  + If your life is a hard drive, Christ can be your backup. +

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


Re: [PATCHES] Updatable views

2008-05-08 Thread Bernd Helmle
--On Donnerstag, Mai 08, 2008 16:34:39 +0100 Simon Riggs 
[EMAIL PROTECTED] wrote:



Are you planning to work on this?



Yes, i do. But i have to finish other things first until i can get back 
full attention  to it, hopefully very soon.


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


Re: [PATCHES] Updatable views

2008-05-08 Thread Simon Riggs
On Thu, 2008-05-08 at 21:37 +0200, Bernd Helmle wrote:
 --On Donnerstag, Mai 08, 2008 16:34:39 +0100 Simon Riggs 
 [EMAIL PROTECTED] wrote:
 
  Are you planning to work on this?
 
 
 Yes, i do. But i have to finish other things first until i can get back 
 full attention  to it, hopefully very soon.

OK, cool.

It looks to me that the way MERGE processes WHEN clauses is almost
identical to the way updatable views process WITH CHECK OPTION. I'll
assist you with at least that part of it, if I can.

In the meantime, I'll assume its there and get on with the rest of
MERGE.

-- 
  Simon Riggs
  2ndQuadrant  http://www.2ndQuadrant.com


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


Re: [PATCHES] [NOVICE] encoding problems

2008-05-08 Thread Guillaume Smet
On Thu, May 8, 2008 at 9:11 PM, Bruce Momjian [EMAIL PROTECTED] wrote:

 Applied.

As I mentioned it before, is there any chance for this fix to be
backported to 8.3 branch? IMHO it's a usability regression.

Thanks.

-- 
Guillaume

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


Re: [PATCHES] [NOVICE] encoding problems

2008-05-08 Thread Bruce Momjian
Guillaume Smet wrote:
 On Thu, May 8, 2008 at 9:11 PM, Bruce Momjian [EMAIL PROTECTED] wrote:
 
  Applied.
 
 As I mentioned it before, is there any chance for this fix to be
 backported to 8.3 branch? IMHO it's a usability regression.

No, we don't change behaviors in back branches unless we get lots of
complaints, and we haven't in this case.

-- 
  Bruce Momjian  [EMAIL PROTECTED]http://momjian.us
  EnterpriseDB http://enterprisedb.com

  + If your life is a hard drive, Christ can be your backup. +

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


Re: [PATCHES] [NOVICE] encoding problems

2008-05-08 Thread Guillaume Smet
On Fri, May 9, 2008 at 4:38 AM, Bruce Momjian [EMAIL PROTECTED] wrote:
 No, we don't change behaviors in back branches unless we get lots of
 complaints, and we haven't in this case.

I suspect it's annoying for a lot of people, just not annoying enough
to make them complain about it.

I understand your point of view but I really think it's more a
regression fix than a behavior change.

That said, if nobody is following me on this one, I'll live with it -
it's just annoying, not blocking.

Thanks for fixing it anyway.

-- 
Guillaume

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


Re: [PATCHES] [NOVICE] encoding problems

2008-05-08 Thread Bruce Momjian
Guillaume Smet wrote:
 On Fri, May 9, 2008 at 4:38 AM, Bruce Momjian [EMAIL PROTECTED] wrote:
  No, we don't change behaviors in back branches unless we get lots of
  complaints, and we haven't in this case.
 
 I suspect it's annoying for a lot of people, just not annoying enough
 to make them complain about it.
 
 I understand your point of view but I really think it's more a
 regression fix than a behavior change.
 
 That said, if nobody is following me on this one, I'll live with it -
 it's just annoying, not blocking.

If I can get other hackers to say we should backpatch we can consider
it.

Let me add though that as the patch is coded it is not the same as 8.2,
but better, so it is hard to say we should actually improve 8.3 over
8.2 in a minor release.  As you can see 8.3.X  behavior will not match
8.3 and that might be worse than just keeping it constant until 8.4.

-- 
  Bruce Momjian  [EMAIL PROTECTED]http://momjian.us
  EnterpriseDB http://enterprisedb.com

  + If your life is a hard drive, Christ can be your backup. +

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