Re: [PATCHES] still alive?

[Bernd Helmle]

--On Mittwoch, September 17, 2008 22:37:29 +0300 Heikki Linnakangas 
<[EMAIL PROTECTED]> wrote:



Just send them to pgsql-hackers.


Oh, that's fine, since discussions are already moved from -patches and are 
continued there. The only disadvantage i see is that -hackers is a little 
more frequented than -patches, but that isn't something which can't be 
managed with some intelligent filtering.


So +1 from my side, if that counts.

--
 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] still alive?

[Bernd Helmle]

--On Donnerstag, September 11, 2008 15:39:01 +0300 Peter Eisentraut 
<[EMAIL PROTECTED]> wrote:




Hmm, let's try this:

Anyone who thinks the patches list should remain as separate from
hackers, shout now (with rationale)!


Seems i've missed something, what's then supposed to hold patches now?

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

[PATCHES] Add missing descriptions for aggregates, functions and conversions

[Bernd Helmle]

Please find attached a patch that adds some missing descriptions for 
aggregates, functions and conversions. This will add COMMENTs to the 
conversion sql script as well. Most of the descriptions are taken from the 
documentation (especially for the statistic functions). I didn't bother 
with some internal functions like text_pattern_lt, if we agree they should 
be described as well i can add them, too.


--
 Thanks

   Bernd? function_descr.patch
Index: src/backend/utils/mb/conversion_procs/Makefile
===
RCS file: /projects/cvsroot/pgsql/src/backend/utils/mb/conversion_procs/Makefile,v
retrieving revision 1.19
diff -c -b -r1.19 Makefile
*** src/backend/utils/mb/conversion_procs/Makefile	18 Mar 2008 16:24:50 -	1.19
--- src/backend/utils/mb/conversion_procs/Makefile	8 Jul 2008 15:06:47 -
***
*** 175,182 
--- 175,184 
  		obj=$$1; shift; \
  		echo "-- $$se --> $$de"; \
  		echo "CREATE OR REPLACE FUNCTION $$func (INTEGER, INTEGER, CSTRING, INTERNAL, INTEGER) RETURNS VOID AS '$$"libdir"/$$obj', '$$func' LANGUAGE C STRICT;"; \
+ 	echo "COMMENT ON FUNCTION $$func(INTEGER, INTEGER, CSTRING, INTERNAL, INTEGER) IS 'internal conversion function for $$se to $$de';"; \
  		echo "DROP CONVERSION pg_catalog.$$name;"; \
  		echo "CREATE DEFAULT CONVERSION pg_catalog.$$name FOR '$$se' TO '$$de' FROM $$func;"; \
+ 	echo "COMMENT ON CONVERSION pg_catalog.$$name IS 'conversion for $$se to $$de';"; \
  	done > $@
  else
  	echo "-- No conversion support, for lack of shared library support" > $@
Index: src/include/catalog/pg_proc.h
===
RCS file: /projects/cvsroot/pgsql/src/include/catalog/pg_proc.h,v
retrieving revision 1.504
diff -c -b -r1.504 pg_proc.h
*** src/include/catalog/pg_proc.h	3 Jul 2008 20:58:46 -	1.504
--- src/include/catalog/pg_proc.h	8 Jul 2008 15:06:47 -
***
*** 235,241 
  DATA(insert OID =  110 (  unknownout	   PGNSP PGUID 12 1 0 f f t f i 1 2275	"705" _null_ _null_ _null_	unknownout - _null_ _null_ ));
  DESCR("I/O");
  DATA(insert OID = 111 (  numeric_fac	   PGNSP PGUID 12 1 0 f f t f i 1 1700 "20" _null_ _null_ _null_	numeric_fac - _null_ _null_ ));
! 
  DATA(insert OID = 115 (  box_above_eq	   PGNSP PGUID 12 1 0 f f t f i 2  16 "603 603" _null_ _null_ _null_	box_above_eq - _null_ _null_ ));
  DESCR("is above (allows touching)");
  DATA(insert OID = 116 (  box_below_eq	   PGNSP PGUID 12 1 0 f f t f i 2  16 "603 603" _null_ _null_ _null_	box_below_eq - _null_ _null_ ));
--- 235,241 
  DATA(insert OID =  110 (  unknownout	   PGNSP PGUID 12 1 0 f f t f i 1 2275	"705" _null_ _null_ _null_	unknownout - _null_ _null_ ));
  DESCR("I/O");
  DATA(insert OID = 111 (  numeric_fac	   PGNSP PGUID 12 1 0 f f t f i 1 1700 "20" _null_ _null_ _null_	numeric_fac - _null_ _null_ ));
! DESCR("equivalent to factorial");
  DATA(insert OID = 115 (  box_above_eq	   PGNSP PGUID 12 1 0 f f t f i 2  16 "603 603" _null_ _null_ _null_	box_above_eq - _null_ _null_ ));
  DESCR("is above (allows touching)");
  DATA(insert OID = 116 (  box_below_eq	   PGNSP PGUID 12 1 0 f f t f i 2  16 "603 603" _null_ _null_ _null_	box_below_eq - _null_ _null_ ));
***
*** 3220,3340 
--- 3220,3444 
  /* Aggregates (moved here from pg_aggregate for 7.3) */
  
  DATA(insert OID = 2100 (  avgPGNSP PGUID 12 1 0 t f f f i 1 1700 "20" _null_ _null_ _null_  aggregate_dummy - _null_ _null_ ));
+ DESCR("the average (arithmetic mean) as numeric of all bigint values");
  DATA(insert OID = 2101 (  avgPGNSP PGUID 12 1 0 t f f f i 1 1700 "23" _null_ _null_ _null_  aggregate_dummy - _null_ _null_ ));
+ DESCR("the average (arithmetic mean) as numeric of all integer values");
  DATA(insert OID = 2102 (  avgPGNSP PGUID 12 1 0 t f f f i 1 1700 "21" _null_ _null_ _null_  aggregate_dummy - _null_ _null_ ));
+ DESCR("the average (arithmetic mean) as numeric of all smallint values");
  DATA(insert OID = 2103 (  avgPGNSP PGUID 12 1 0 t f f f i 1 1700 "1700" _null_ _null_ _null_ aggregate_dummy - _null_ _null_ ));
+ DESCR("the average (arithmetic mean) as numeric of all numeric values");
  DATA(insert OID = 2104 (  avgPGNSP PGUID 12 1 0 t f f f i 1 701 "700" _null_ _null_ _null_  aggregate_dummy - _null_ _null_ ));
+ DESCR("the average (arithmetic mean) as float8 of all float4 values");
  DATA(insert OID = 2105 (  avgPGNSP PGUID 12 1 0 t f f f i 1 701 "701" _null_ _null_ _null_  aggregate_dummy - _null_ _null_ ));
+ DESCR("the average (arithmetic mean) as float8 of all float8 values");
  DATA(insert OID = 2106 (  avgPGNSP PGUID 12 1 0 t f f f i 1 1186 "1186" _null_ _null_ _null_ aggregate_dummy - _null_ _null_ ));
+ DESCR("the average (arithmetic mean) as interval of all interval values");
  
  DATA(insert OID = 2107 (  sumPGNSP PGUID 12 1 0 t f f f i 1 1700 "20" _null_ _null_ _null_  aggregate_dummy - _null_ _null

Re: [PATCHES] Updatable views

[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

[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

[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

[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




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] psql command aliases support

[Bernd Helmle]

--On Donnerstag, April 03, 2008 14:36:59 +0100 Gregory Stark 
<[EMAIL PROTECTED]> wrote:



\o foo instead of \i foo -- check your keyboard layout


The point is here you've typed a different command entirely.
Unsurprisingly it's going to do something different.


Uh well, you surely don't know that if you accidently hit o instead of 
ione reason more for an alias \output and \input ;)


To be serious, i take your point, but i'm under the impression that you 
simply can't safe the user against all mistakes they are going to make 
ever. Your concerns apply to every existing alias implementation as well 
and you aren't safe against decisions of your distributor to add 
conflicting aliases to their default bashrc as well.


--
 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] psql command aliases support

[Bernd Helmle]

--On Freitag, April 04, 2008 01:11:51 +1100 Brendan Jurd 
<[EMAIL PROTECTED]> wrote:



What is the virtue of allowing such a syntax in the first place?  I
can't think of any other context where it's okay to issue a command
together with arguments without some kind of delimiter, for the
obvious reason that it introduces ambiguities such as those Greg
described.


Yeah, i always felt very uncomfortable with this behavior. Anyways, it's 
too late to complain about that and it shouldn't block new features which 
can increase usability on the other side.


--
 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] psql command aliases support

[Bernd Helmle]

--On Donnerstag, April 03, 2008 03:13:47 +0100 Gregory Stark 
<[EMAIL PROTECTED]> wrote:



I think you're crazy to think shells are more likely to have conflicts.
Shells require a whole token match, not just the first letter.

In other words, any alias *starting with* the letters c, d, e, f, g, h, i,
o, s, w, z would be a conflict. Just for maximum confusion the list of
letters which cause conflicts when capitalized would be entirely
different.

Picture a newbie typoing on their \old alias and trying to figure out
where all their data is going... Hopefully they weren't too attached to
whatever was in their "ldd" file yesterday.


Of course, the patch doesn't work this way. Only complete tokens delivered 
by the parser are substituted, illustrated here with your example:



#= \alias old SELECT version();
#= \old
 version
---
PostgreSQL 8.4devel on x86_64-unknown-linux-gnu, compiled by GCC gcc (GCC) 
4.2.3 (Debian 4.2.3-2)

(1 row)

#= \o foo
#= \old
#=
zsh: suspended  psql
% cat foo
 version
---
PostgreSQL 8.4devel on x86_64-unknown-linux-gnu, compiled by GCC gcc (GCC) 
4.2.3 (Debian 4.2.3-2)

(1 row)

--
 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] psql command aliases support

[Bernd Helmle]

--On Dienstag, April 01, 2008 11:39:59 -0400 Tom Lane <[EMAIL PROTECTED]> 
wrote:



Do we really want such a thing?


Well, i use aliases everytime and everywhere they got implemented and i 
found it quite useful to _extend_ existing behavior (integrating additional 
functionality in an easy way)



The space of backslash command names
is so densely populated already that it's hard to imagine creating
aliases without conflicting with existing (or future) command names


I often found existing backslash command sometimes overloaded or simply not 
providing information i really need (for example, an easy way to get 
information about current locales, encoding and user settings). You simply 
can't catch all requirements DBA's and users want within a all-catching 
implementation. Using this way, users are able to implement their own 
command shortcuts


Overriding existing backslash commands (as my first example shows) is only 
an implementation-specific detail which could easily forbidden. However, 
defining your own shortcuts for your psql-sessions looks quite useful to 
me, like my 2nd example tries to illustrate.



It seems like mostly a recipe for confusion.


So what? This could happen in every shell that supports aliases as well. I 
don't get your point...?


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

[PATCHES] psql command aliases support

[Bernd Helmle]

Folks,

please find attached a patch which implements psql command aliases. They 
work the same way as on bash, zsh and others, for example:


#= \alias d \dt+
#= \d
 List of relations
Schema | Name | Type  | Owner | Description
+--+---+---+-
public | foo  | table | bernd |
(1 row)

=# \alias current_query SELECT current_query, NOW() - query_start FROM 
pg_stat_activity WHERE current_query NOT LIKE 'IDLE%';


#= \current_query
current_query 
| ?column?

---+--
SELECT current_query, NOW() - query_start FROM pg_stat_activity WHERE 
current_query NOT LIKE 'IDLE%'; | 00:00:00

(1 row)


#= \unalias d
#= \d
  List of relations
Schema | Name | Type  | Owner
+--+---+---
public | foo  | table | bernd
(1 row)


I hope i broke nothing and maybe we find this useful for 8.4, documentation 
included.


--
 Thanks

   BerndIndex: doc/src/sgml/ref/psql-ref.sgml
===
RCS file: /projects/cvsroot/pgsql/doc/src/sgml/ref/psql-ref.sgml,v
retrieving revision 1.199
diff -c -B -r1.199 psql-ref.sgml
*** doc/src/sgml/ref/psql-ref.sgml	30 Mar 2008 18:10:20 -	1.199
--- doc/src/sgml/ref/psql-ref.sgml	1 Apr 2008 14:05:05 -
***
*** 685,690 
--- 685,708 

  

+ \alias aliasname aliascommand
+ 
+ 
+ The \alias backslash command allows defining your own backslash commands or an alias for an existing built-in command. The alias is allowed to call a SQL command or any other backslash command or aliases already defined. For example:
+ 
+ => \alias watch_idle_tx SELECT xact_start, datid, datname, procpid, usesysid FROM pg_stat_activity WHERE current_query LIKE '%IDLE%';
+ => \watch_idle_tx
+   xact_start   | datid | datname | procpid | usesysid
+ ---+---+-+-+--
+  2008-03-07 14:59:25.618827+01 | 24576 | db  |   28864 |   10
+ (1 row)
+ 
+ Calling \alias without an aliasname just shows a list of all defined aliases, with an existing aliasname its assigned aliascommand is shown. You can use the \unalias to remove an alias.
+ 
+ 
+   
+ 
+   
 \cd [ directory ]
 
  
***
*** 1849,1854 
--- 1867,1880 
 

  
+   
+ \unalias aliascmd
+ 
+ 
+ Removes the specified alias from the list.
+ 
+ 
+   
  

  \w {filename | |command}
Index: src/bin/psql/command.c
===
RCS file: /projects/cvsroot/pgsql/src/bin/psql/command.c,v
retrieving revision 1.186
diff -c -B -r1.186 command.c
*** src/bin/psql/command.c	1 Jan 2008 19:45:55 -	1.186
--- src/bin/psql/command.c	1 Apr 2008 14:05:06 -
***
*** 199,204 
--- 199,251 
  	backslashResult status = PSQL_CMD_SKIP_LINE;
  
  	/*
+ 	 * We want to look for aliases first, since we want them to "override" a
+ 	 * built-in command, so our users are able to define their "own" behavior
+ 	 * on their favourite backslash command.
+ 	 */
+ 	const char *aliascmd = GetVariable(pset.aliases, cmd);
+
+ 	if (aliascmd)
+ 	{
+ 		/* Save the current alias substitution string.
+ 		 * We do this to prevent infinite recursion into the aliases
+ 		 * list.
+ 		 */
+ 		const char *substalias = GetVariable(pset.substaliases, cmd);
+ 
+ 		if (substalias != NULL)
+ 		{
+ 			/* already substituted, nothing more to do */
+ 			DeleteVariable(pset.substaliases, cmd);
+ 		}
+ 		else
+ 		{
+ 			char *opt;
+ 			SetVariable(pset.substaliases, cmd, "1");
+ 
+ 			/* save the substituted command to the query buffer but save
+ 			 * additional parameter lists also
+ 			 */
+ 			resetPQExpBuffer(query_buf);
+ 			appendPQExpBuffer(query_buf, aliascmd);
+ 
+ 			if ((opt = psql_scan_slash_option(scan_state,
+ 			  OT_WHOLE_LINE, NULL, false)))
+ 			{
+ appendPQExpBuffer(query_buf, " ");
+ appendPQExpBuffer(query_buf, opt);
+ 			}
+ 
+ 			success = true;
+ 			return (status = PSQL_CMD_ALIAS);
+ 		}
+ 
+ 	}
+ 
+ 	/* clear recursive alias list */
+ 	ClearVariableSpace(pset.substaliases);
+ 
+ 	/*
  	 * \a -- toggle field alignment This makes little sense but we keep it
  	 * around.
  	 */
***
*** 1037,1042 
--- 1084,1143 
  	else if (strcmp(cmd, "?") == 0)
  		slashUsage(pset.popt.topt.pager);
  
+ 	else if (strcmp(cmd, "alias") == 0)
+ 	{
+ 		char *aliasname = psql_scan_slash_option(scan_state,
+  OT_NORMAL, NULL, false);
+ 		char *aliascmd  = psql_scan_slash_option(scan_state,
+  OT_WHOLE_LINE, NULL, false);
+ 
+ 		/* if no aliasname was specified, then print the whole alias list */
+ 		if (!aliasname)
+ 

[PATCHES] Show login privilege in psql \du command

[Bernd Helmle]

Please find attached a tiny patch which adds a "Login" field to the \du 
command, displaying wether a role has the LOGIN privilege granted or not. I 
found this useful, since you have to query the system catalog directly to 
find out which role is permitted to login.


--
 Thanks

   BerndIndex: src/bin/psql/describe.c
===
RCS file: /projects/cvsroot/pgsql/src/bin/psql/describe.c,v
retrieving revision 1.159
diff -c -B -r1.159 describe.c
*** src/bin/psql/describe.c	22 Aug 2007 02:25:34 -	1.159
--- src/bin/psql/describe.c	13 Sep 2007 13:54:25 -
***
*** 1575,1580 
--- 1575,1581 
  "  CASE WHEN r.rolsuper THEN '%s' ELSE '%s' END AS \"%s\",\n"
  		   "  CASE WHEN r.rolcreaterole THEN '%s' ELSE '%s' END AS \"%s\",\n"
  			 "  CASE WHEN r.rolcreatedb THEN '%s' ELSE '%s' END AS \"%s\",\n"
+ 			 "  CASE WHEN r.rolcanlogin THEN '%s' ELSE '%s' END AS \"%s\",\n"
  		"  CASE WHEN r.rolconnlimit < 0 THEN CAST('%s' AS pg_catalog.text)\n"
  	  "   ELSE CAST(r.rolconnlimit AS pg_catalog.text)\n"
  	  "  END AS \"%s\", \n"
***
*** 1583,1588 
--- 1584,1590 
  	  _("yes"), _("no"), _("Superuser"),
  	  _("yes"), _("no"), _("Create role"),
  	  _("yes"), _("no"), _("Create DB"),
+ 	  _("yes"), _("no"), _("Login"),
  	  _("no limit"), _("Connections"),
  	  _("Member of"));
  

---(end of broadcast)---
TIP 4: Have you searched our list archives?

   http://archives.postgresql.org

[PATCHES] Bug in WAL backup documentation

[Bernd Helmle]

Our WAL backup documentation says in some parts of it:

..."%p is replaced by the absolute path of the file to archive..." [1]

I think this is (at least for 8.1 and upcoming 8.2 releases) wrong, since 
the archiver

replaces this with pg_xlog/ only, so that the archive command
is invoked with the relative path to the database data directory and its 
xlog files. This
applies to the restore_command as well. Attached is a small patch against 
HEAD, which

will adjust things in the documentation.

[1] 
http://www.postgresql.org/docs/8.1/interactive/backup-online.html#BACKUP-ARCHIVING-WAL


--
 Thanks

   Bernd

wal_backup_doc.patch
Description: Binary data

---(end of broadcast)---
TIP 9: In versions below 8.0, the planner will ignore your desire to
   choose an index scan if your joining column's datatypes do not
   match

Re: [PATCHES] Updatable views

[Bernd Helmle]

--On Mittwoch, August 30, 2006 12:01:25 -0400 Tom Lane <[EMAIL PROTECTED]> 
wrote:



Bernd Helmle <[EMAIL PROTECTED]> writes:

[ latest views patch ]


This is the first time I've actually looked at this patch, and I am
dismayed.  viewUpdate.c looks like nothing so much as a large program
with a small program struggling to get out.  What is all the stuff about
handling multiple base rels?  SQL92, at least, does not say that a join
is updatable, and AFAICT this patch is rejecting that too ... though
it's hard to tell with the conditions for allowing the join to be
updatable scattered through a lot of different functions.  And some of
the code seems to be expecting multiple implicit rules and other parts
not.  I get the impression that a lot of this code is left over from a
more ambitious first draft and ought to be removed in the name of
readability/maintainability.



I not sure what parts of the code you are refering to exactly, but I admit 
that
there are code parts that could deal with multiple base relations and 
rules.

get_base_base_relation() is an example, it is used to create lookup tables
for reversed columns so we could break them down to the correct position in
their base tables. Restricting that to only one base relation wouldn't make 
any

difference. Furthermore, SQL99 allows at least updatable views with joined
relations which preserve their keys in the view definition. So i don't 
think it's that

bad to leave parts of the code that way for future improvements.


I'm unclear as to why you've got DO INSTEAD NOTHING rules in there ---
the spec says that a WITH CHECK OPTION violation results in an error,
not in nothing happening, so it doesn't seem to me that we should need
any NOTHING rules to implement the spec.  It would probably help if


Well, instead of something like

"ERROR:  cannot insert into a view
HINT:  You need an unconditional ON INSERT DO INSTEAD rule."

you will get

"ERROR:  view update commands violates rule condition"

with the correct error code set, because the view update check function is 
fired before.
The first one isn't very useful for someone who simply wants to insert data 
into the
view which isn't allowed to get in. You never get the view update check 
function fired

without the DO INSTEAD rule applied to a view created with a check option.


there were some header documentation that explained exactly how the
module intends to transform a SELECT to create the various action rules.



I agree with you, maybe it's a good to add a README to src/backend/rewrite?


The pg_dump changes seem pretty odd too.  Why wouldn't you just
ignore implicit rules during a dump, expecting the system to
regenerate them when the view is reloaded?


Uhm, you're right. It's easier to exclude them in the SELECT query directly 
instead
of selecting them, iterating over and filter them out. I'll fix that. 
(Looks like this is a

"cannot see the wood for the trees"-mistake)


--
 Thanks

   Bernd

---(end of broadcast)---
TIP 1: if posting/reading through Usenet, please send an appropriate
  subscribe-nomail command to [EMAIL PROTECTED] so that your
  message can get through to the mailing list cleanly

Re: [HACKERS] [PATCHES] Updatable views

[Bernd Helmle]

--On Donnerstag, August 24, 2006 11:02:43 -0500 Jaime Casanova 
<[EMAIL PROTECTED]> wrote:



Actually the code delete implicit rules based on a field added to
pg_rewrite but that catalog has a unique index on ev_class, rulename:
"pg_rewrite_rel_rulename_index" UNIQUE, btree (ev_class, rulename)

i guess bernd's comment is about this index giving an error if we try
to insert the new rule with the same name on the same event...


No, this wasn't the problem, since we are going to drop any implicit
rule that collides with an user defined one (however, this approach is
discussable, but nobody has put his comments on this yet and i think this
is important for backwards compatibility). I don't think we need ev_kind in 
the
index at all, in my opinion implicit and user defined rules of the same 
event

shouldn't live together (_RETURN rules are marked as implicit ones now, too)

--
 Thanks

   Bernd

---(end of broadcast)---
TIP 2: Don't 'kill -9' the postmaster

Re: [HACKERS] [PATCHES] Updatable views

[Bernd Helmle]

--On Donnerstag, August 24, 2006 11:00:45 -0400 Tom Lane 
<[EMAIL PROTECTED]> wrote:



If the code is dependent on recognizing names to know what it's doing,
then I'd say you have a fundamentally broken approach.  Consider adding
a flag column to pg_rewrite to distinguish these rules, instead.


This is the approach the code already follows (it uses an additional
ev_kind column which distinguishes rules between implicit rules with
no, local or cascaded check option and explicit ones).

Turns out that i was thinking too difficult when looking at the code which
drops implicit rulessorry for the noise.

--
 Thanks

   Bernd

---(end of broadcast)---
TIP 9: In versions below 8.0, the planner will ignore your desire to
  choose an index scan if your joining column's datatypes do not
  match

Re: [PATCHES] Updatable views

[Bernd Helmle]

--On Montag, August 21, 2006 02:07:41 -0400 Alvaro Herrera 
<[EMAIL PROTECTED]> wrote:



So, I'll appreciate if somebody else takes the responsability to fix the
remaining issues.  I've put a lot of XXX's and some FIXME's.  Some
functions are in need of some comments as well.


While working on Alvaro's suggestions to fix the code i got the opinion
that we need to reject any attempts to name a user defined rule
as

"_INSERT"
"_NOTHING_INSERT"
"_DELETE"
"_NOTHING_DELETE"
"_UPDATE"
"_NOTHING_UPDATE"

because this confuses the code when replacing an existing implicit
rule with its own user defined one:

[EMAIL PROTECTED]:bernd #= create or replace view v_second as select id, name, 
usr from second where usr =

current_user with check option;
NOTICE:  CREATE VIEW will create implicit INSERT/UPDATE/DELETE rules
CREATE VIEW
[EMAIL PROTECTED]:bernd #= CREATE OR REPLACE RULE "_INSERT" AS ON INSERT TO 
v_second DO INSTEAD NOTHING;

ERROR:  tuple already updated by self

This is because the code tries to drop the existing implicit insert rule 
from pg_rewrite
and then to replace it with the new one (note the "_INSERT" caption of the 
rule, any

other labeled rule works as expected).

We could avoid this by using a CommandCounterIncrement() (brute force 
method),

but it seems to me that we should do the same here as with "_RETURN" rules
at the moment.

Any comments?

--
 Thanks

   Bernd

---(end of broadcast)---
TIP 6: explain analyze is your friend

Re: [HACKERS] [PATCHES] COPY view

[Bernd Helmle]

--On Mittwoch, August 23, 2006 08:24:55 -0400 Tom Lane <[EMAIL PROTECTED]> 
wrote:



What are these open issues for the updatable views patch you are seeing
exactly?


Didn't Alvaro list a bunch of issues when he put the patch back up for
comment?  I have not looked at it myself yet.


Indeed he did and this helps a lot to clean up some parts of the code (oh, 
thanks
to him for reviewing this, i think i forgot that :( ). I thought you were 
refering to

some specific showstoppers i've missed.

--
 Thanks

   Bernd

---(end of broadcast)---
TIP 2: Don't 'kill -9' the postmaster

Re: [HACKERS] [PATCHES] COPY view

[Bernd Helmle]

--On Dienstag, August 22, 2006 23:12:21 -0400 Tom Lane <[EMAIL PROTECTED]> 
wrote:



At the moment, with the online-index and updatable-views patches both
pretty seriously broken, and no sign that the bitmap-index people are
awake at all, I might take it on myself to fix this one instead of those
others.  But is that what I should be spending my time on in the waning
days of the 8.2 freeze cycle?  Speak now or hold your peace.


What are these open issues for the updatable views patch you are seeing 
exactly?
I'm currently trying to update this patch based on alvaros comments in the 
code and

i see the INSERT...RETURNING stuff as the only "big hurd" at the moment
(however, i haven't looked at this closer, but saw your and Jaime's 
comments on this...).

It would be nice if we could summarize all open things so everybody who is
able to work on this gets a complete overview.

--
 Thanks

   Bernd

---(end of broadcast)---
TIP 1: if posting/reading through Usenet, please send an appropriate
  subscribe-nomail command to [EMAIL PROTECTED] so that your
  message can get through to the mailing list cleanly

Re: [PATCHES] Updatable views

[Bernd Helmle]

--On Montag, August 21, 2006 02:07:41 -0400 Alvaro Herrera 
<[EMAIL PROTECTED]> wrote:



Hi,

This is the patch for updatable views I've been able to come up with.  A
nasty bug was just discovered in the upcoming Mammoth Replicator release
so I'm not sure if I'm going to have time to work more on it soon.

So, I'll appreciate if somebody else takes the responsability to fix the
remaining issues.  I've put a lot of XXX's and some FIXME's.
Some
functions are in need of some comments as well.



I'll try to complete the missing comments and to make some statements
cleaner.


The new files are src/backend/rewrite/viewUpdate.c and
src/include/rewrite/viewUpdate.h.  The third file, upd-views.sql, is
intended to be a new regression test.  Extra points if the table therein
is completed correctly.

I haven't tested the array stuff at all.

Comments from Bernd and Jaime are especially welcome if I've broken
something that used to work on their patch :-)


I see that the current patch doesn't support subqueries in the WHERE-clause 
anymore.
You can find one example in the attached SQL-script. Is there a reason why 
you

dropped this?

--
 Thanks

   Bernd

regress_view_update_check_option2.sql
Description: Binary data

---(end of broadcast)---
TIP 9: In versions below 8.0, the planner will ignore your desire to
   choose an index scan if your joining column's datatypes do not
   match

Re: [PATCHES] Patch for updatable views

[Bernd Helmle]

--On Dienstag, Juli 25, 2006 18:29:39 -0500 Jaime Casanova 
<[EMAIL PROTECTED]> wrote:



On 7/25/06, Bernd Helmle <[EMAIL PROTECTED]> wrote:

Hi folks,

please find attached an implementation for updatable views. Included are
support for pg_dump and information_schema, regression test and
documentation are missing. Also, there's currently no upgrade path for
older PostgreSQL versions and user defined rules on views.


i'm testing the functionality... seems good to me... i will work on
docs and regress if no one objects and bernd is not doing it...


AFAICS, the view will not be updateable if there are casts in the
select list (seems fair to let that to future revisions), but i think
we must say it.



I thought about it a while and came to the conclusion that this
makes no sense. Casting in a view's target list involves
computation, function calls or constant values usually, so i
think there's no reason to add more complexity to the code.



One thing to think of:

create table testing_serial (id serial primary key, name text);
CREATE TABLE

create view vtest_serial as select * from testing_serial;
CREATE VIEW

insert into vtest_serial values (default, 'name1');
psql:../view_test.sql:81: ERROR:  null value in column "id" violates
not-null constraint

insert into vtest_serial(name) values ('name2');
psql:../view_test.sql:82: ERROR:  null value in column "id" violates
not-null constraint

i still think that in updateable views we need to inherit the defaut
value of the base table, i still see this code commented in
rewriteHandler.c



Err, forgot to change the comment into a #if..#end block...

It's easy to add column defaults to a view if you need them, but
i'm not sure creating them automatically is the right way.



psql:../view_test.sql:73: ERROR:  cannot insert into a view
HINT:  You need an unconditional ON INSERT DO INSTEAD rule.

BTW, we must change this message for something more like 'cannot
insert into a  non updateable view'

-
+   /*
+* I will do this only in case of relkind == RELKIND_VIEW.
+* This is the last attempt to get a value for expr before we
+* consider that expr must be NULL.
+*/
+ /*if (expr == NULL && rel->rd_rel->relkind == RELKIND_VIEW) */
+ /*{ */
+ /*expr = (Node *)makeNode(SetToDefault); */
+ /*return expr; */
+ /*}*/
+

if this functionality will be accepted this is the time to discuss it
otherwise drop this comment.



Yepp.


With this code we still can create a different default for the view
with ALTER TABLE ADD DEFAULT



Hmm, this is something i haven't considered yet




I have some code which drops the implicit created rules silently if
someone wants to have its own rule, but this needs some discussion, i
think.



+ #if 0
+   /*
+* Implicit rules should be dropped automatically when someone
+* wants to have its *own* rules on the view. is_implicit is set
+* to NO_OPTION_EXCPLICIT in this case so we drop all implicit
+* rules on the specified event type immediately.
+*
+* ???FIXME: do we want this behavior???
+   */
+
+   if ( ev_kind == NO_OPTION_EXPLICIT )
+deleteImplicitRulesOnEvent(event_relation, event_type);
+ #endif

This is a must for compatibility with older versions. Otherwise we
will have views with user defined rules and implicit rules that will
have an unexpected behaviour.



Like the rule regression tests which fail exactly because of this
reason. However, i'm not sure if the backend is the right place
to do such implicit things. One idea is to provide a stored procedure
that drops implicit or user defined rules on views (this could be
implemented easily) and to instruct users to follow this upgrade
path (or to teach pg_dump to do so).




The patch covers the whole SQL92 functionality and doesn't create any
rules, if a given view is considered not to be compatible with SQL92
definitions.


I think is necessary to send some NOTICE when we can't create rules at
all or when we can't create one of them (insert rules are not always
created because they need all not-null without defaults columns to be
in the select list)



I think we could do it the other way, like the NOTICEs you get when you
create tables with primary keys. This would be more consistent then.

[...]



--
 Thanks

   Bernd

---(end of broadcast)---
TIP 2: Don't 'kill -9' the postmaster

[PATCHES] Patch for updatable views

[Bernd Helmle]

Hi folks,

please find attached an implementation for updatable views. Included are 
support

for pg_dump and information_schema, regression test and documentation are
missing. Also, there's currently no upgrade path for older PostgreSQL 
versions and
user defined rules on views. I have some code which drops the implicit 
created
rules silently if someone wants to have its own rule, but this needs some 
discussion,

i think.

The patch covers the whole SQL92 functionality and doesn't create any 
rules, if
a given view is considered not to be compatible with SQL92 definitions. The 
supported

syntax is

CREATE VIEW foo AS  [ WITH [ CASCADED | LOCAL ] CHECK OPTION ]

The check option is implemented as a conditional rule with a simple system 
function, which
checks the given expression tree to be true or false and raises an error in 
the latter case.
There's also a little change in the rewriter semantics, as i treat implicit 
(view update rules
created automatically) and explicit rules (rules created by any user) 
differently.

This involves some changes to the system catalog (especially
pg_rewrite and pg_proc), so be prepared to do an initdb. There are new files
in src/backend/rewrite/view_update.c and src/include/rewrite/view_update.h, 
too.


Please note that the patch currently breaks some regression tests, but 
these are
mostly due to duplicated rules on views and additional notice messages. 
Also, i
have dropped support for updatable views which contains indexed array 
fields
of tables (like SELECT foo[3], foo[2] FROM bar). These are treated 
non-updatable and

someone needs his own rules here.

I hope there aren't too many open points here, so this patch could be 
considered

for inclusion in 8.2.

Looking forward your opinions...

--
 Thanks

   Bernd


pgsql-view_update_8.2dev.tar.bz2
Description: Binary data

---(end of broadcast)---
TIP 4: Have you searched our list archives?

   http://archives.postgresql.org

Re: [PATCHES] Schema move for opclasses, operator and conversions

[Bernd Helmle]

--On Donnerstag, Juni 01, 2006 23:03:39 -0400 Tom Lane <[EMAIL PROTECTED]> 
wrote:



Not to be unkind,


Don't worry ;)

< but is this feature actually worth the code space?

I can barely see a use case for relocating operators, and as for the
other two, who's ever going to use them?



Well, i thought it would be useful for people which are going to reorganize
an entire database schema. Since we have schema move capabilities for
tables, functions, sequences et al., someone could then easily setup a 
script to move

an entire schema to a new one.


I understand about orthogonality and all that, but we have limited
manpower, and every thousand lines of code costs us ongoing maintenance
effort whether it's ever used in the field or not.

What I'd love to see is a patch that *removes* a few thousand lines
of code by finding a way to do this just once for all object types...


so something like ALTER SCHEMA schema MOVE OBJECTS newschema?
Sounds like a reasonable idea

--
 Thanks

   Bernd

---(end of broadcast)---
TIP 9: In versions below 8.0, the planner will ignore your desire to
  choose an index scan if your joining column's datatypes do not
  match

[PATCHES] Schema move for opclasses, operator and conversions

[Bernd Helmle]

Please find attached a patch that allows operator classes, operator and 
conversions

to be moved between schemas. Regression and documentation patch included.

--
 Bernd

alter_schema.patch
Description: Binary data

---(end of broadcast)---
TIP 3: Have you checked our extensive FAQ?

   http://www.postgresql.org/docs/faq

[PATCHES] Schema move for opclasses, operator and conversions

[Bernd Helmle]

Please find attached a patch that allows operator classes, operator and 
conversions

to be moved between schemas. Regression and documentation patch included.

[ sorry if that occurs twice, but my first attempt used the wrong identity 
:( ]


--
 Thanks

   Bernd


alter_schema.patch
Description: Binary data

---(end of broadcast)---
TIP 6: explain analyze is your friend

[PATCHES] Add namespace dependency for conversions

[Bernd Helmle]

Currently we don't record any dependencies between namespaces and 
conversions.
This looks inconsistent to me, since we have dependencies on all other 
objects that live

within namespaces. Please find attached a small patch that fixes this issue.


--
 Thanks

   BerndIndex: src/backend/catalog/pg_conversion.c
===
RCS file: /projects/cvsroot/pgsql/src/backend/catalog/pg_conversion.c,v
retrieving revision 1.29
diff -c -r1.29 pg_conversion.c
*** src/backend/catalog/pg_conversion.c 5 Mar 2006 15:58:23 -   1.29
--- src/backend/catalog/pg_conversion.c 19 May 2006 13:40:40 -
***
*** 18,23 
--- 18,24 
  #include "catalog/dependency.h"
  #include "catalog/indexing.h"
  #include "catalog/pg_conversion.h"
+ #include "catalog/pg_namespace.h"
  #include "catalog/pg_proc.h"
  #include "catalog/namespace.h"
  #include "utils/builtins.h"
***
*** 124,129 
--- 125,136 
recordDependencyOnOwner(ConversionRelationId, HeapTupleGetOid(tup),
conowner);
  
+   /* create dependency on namespace */
+   myself.classId = ConversionRelationId;
+   referenced.classId = NamespaceRelationId;
+   referenced.objectId = connamespace;
+   recordDependencyOn(&myself, &referenced, DEPENDENCY_NORMAL);
+ 
heap_freetuple(tup);
heap_close(rel, RowExclusiveLock);
  

---(end of broadcast)---
TIP 2: Don't 'kill -9' the postmaster

Re: [PATCHES] ALTER OBJECT SET SCHEMA

[Bernd Helmle]

--On Donnerstag, Juli 28, 2005 23:12:37 -0400 Bruce Momjian 
 wrote:



Here is an updated version of your patch.  Would you supply SGML
documentation updates to match the code changes?  Thanks.


Here's my first shot on this. Let me know if there's somenthing missing or 
broken (note: English is not my native language, i hope there aren't too 
much faults).


--
 Bernd

alter_set_schema_doc.patch
Description: Binary data

---(end of broadcast)---
TIP 2: Don't 'kill -9' the postmaster

Re: [PATCHES] ALTER OBJECT SET SCHEMA

[Bernd Helmle]

--On Donnerstag, Juli 28, 2005 23:12:37 -0400 Bruce Momjian 
 wrote:



Here is an updated version of your patch.  Would you supply SGML
documentation updates to match the code changes?  Thanks.


Bruce, is there any requirement to add some regression tests, too?

--
 Bernd

---(end of broadcast)---
TIP 1: if posting/reading through Usenet, please send an appropriate
  subscribe-nomail command to [EMAIL PROTECTED] so that your
  message can get through to the mailing list cleanly

[PATCHES] ALTER OBJECT SET SCHEMA

[Bernd Helmle]

Here's my current patch for ALTER OBJECT SET SCHEMA: the attached patch 
file implements
schema "move" for FUNCTION, SEQUENCE, TYPE, DOMAIN and TABLE with all 
improvements discussed on -hackers recently. Altering OPERATOR, OPERATOR 
CLASS, AGGREGATE and CONVERSION are currently not implemented (since i ran 
out of time) :(


Supported syntax is

ALTER TABLE name SET SCHEMA name;
ALTER SEQUENCE name SET SCHEMA name;
ALTER FUNCTION name SET SCHEMA name;
ALTER TYPE name SET SCHEMA name;
ALTER DOMAIN name SET SCHEMA name;

TIA

--
 Bernd

pgsql_alter.patch.bz2
Description: Binary data

---(end of broadcast)---
TIP 5: Have you checked our extensive FAQ?

   http://www.postgresql.org/docs/faq