Re: [PATCHES] [EMAIL PROTECTED]: Re: [BUGS] Problem identifying constraints which should not be inherited]

2008-05-07 Thread Alex Hunsaker
On Wed, May 7, 2008 at 12:20 AM, Alex Hunsaker [EMAIL PROTECTED] wrote:
  Find attached a diff from v4-v5, and a full patch.

   src/backend/commands/tablecmds.c   |  242 
 +++-

  src/backend/utils/cache/syscache.c |   12 --

  src/include/catalog/indexing.h |2 -
   src/include/utils/syscache.h   |1 -
   4 files changed, 153 insertions(+), 104 deletions(-)

  Currently this loops through all the constraints for a relation (old
  behavior of MergeAttributesIntoExisting)... Do you think its worth
  adding a non-unique index to speed this up?  If so I can whip up a
  patch real quick if you think its worth it... else


*sigh* Here is a fiix for a possible bogus failed to find constraint
error when we are trying to drop a constraint that is not a check
constraint
(interesting no regression tests failed... caught it while reviewing
the patch I just posted)

*** a/src/backend/commands/tablecmds.c
--- /bsrc/backend/commands/tablecmds.c
*** ATExecDropConstraint(Relation rel, const
*** 5080,5094 

con = (Form_pg_constraint) GETSTRUCT(tuple);

-   if (con-contype != CONSTRAINT_CHECK)
-   continue;
-
if (strcmp(NameStr(con-conname),
   constrName) != 0)
continue;
else
found = true;

if (con-coninhcount = 0)
elog(ERROR, relation %u has
non-inherited constraint \%s\,
childrelid, constrName);
--- 5080,5095 

con = (Form_pg_constraint) GETSTRUCT(tuple);

if (strcmp(NameStr(con-conname),
   constrName) != 0)
continue;
else
found = true;

+   /* Right now only CHECK constraints
can be inherited */
+   if (con-contype != CONSTRAINT_CHECK)
+   continue;
+
if (con-coninhcount = 0)
elog(ERROR, relation %u has
non-inherited constraint \%s\,
childrelid, constrName);

-- 
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] plpgsql CASE statement - last version

2008-05-07 Thread Pavel Stehule
Hello

I am sending little bit smarter version - without redundant parsing.
When test expression is defined, then expressions in WHEN part are
modified like

$x in ( origin_expression )

$x is referenced to invisible *case* variable that carries result of
test expression.
Regards
Pavel Stehule



2008/5/3 Pavel Stehule [EMAIL PROTECTED]:
 Hello

 2008/5/3 Tom Lane [EMAIL PROTECTED]:
 Pavel Stehule [EMAIL PROTECTED] writes:
 2008/5/2 Heikki Linnakangas [EMAIL PROTECTED]:
 How about taking a completely different strategy, and implement the
 CASE-WHEN construct fully natively in plpgsql, instead of trying to convert
 it to a single SQL CASE-WHEN expression? It's not a very good match anyway;

 It was first variant. It's  simpler for parsing and slower for
 execution :(. It means more than once expression evaluation and for
 simple case value casting and comparation.

 I agree with Heikki: this patch is seriously ugly, and slower for
 execution isn't a good enough reason for saddling us with having
 to maintain such a kluge in the parser.

 I don't really see why you should need to have multiple expression
 evaluations, anyhow.  Can't you evaluate the test expression once
 and inject its value into the comparisons using CaseTestExpr,
 the same way the core CASE-expression code works?



 I have to look on this way.

 Regards
 Pavel Stehule

   regards, tom lane


*** ./doc/src/sgml/plpgsql.sgml.orig	2008-05-03 02:11:36.0 +0200
--- ./doc/src/sgml/plpgsql.sgml	2008-05-06 11:05:05.0 +0200
***
*** 1601,1606 
--- 1601,1622 
paraliteralIF ... THEN ... ELSEIF ... THEN ... ELSE//
   /listitem
  /itemizedlist
+ 
+ and four forms of literalCASE/:
+ itemizedlist
+  listitem
+   paraliteralCASE ... WHEN ... THEN ... END CASE//
+  /listitem
+  listitem
+   paraliteralCASE ... WHEN ... THEN ... ELSE ... END CASE//
+  /listitem
+  listitem
+   paraliteralCASE WHEN ... THEN ... END CASE//
+  /listitem
+  listitem
+   paraliteralCASE WHEN ... THEN ... ELSE ... END CASE//
+  /listitem
+ /itemizedlist 
  /para
  
  sect3
***
*** 1751,1756 
--- 1767,1838 
 literalELSEIF/ is an alias for literalELSIF/.
/para
   /sect3
+ 
+  sect3
+   titleSimple literalCASE/ statement/title
+ synopsis
+ CASE replaceableexpression/replaceable
+ WHEN replaceableexpression/replaceable optional, replaceableexpression/replaceable optional ... /optional/optional THEN
+   replaceablestatements/replaceable
+   optional WHEN replaceableexpression/replaceable optional, replaceableexpression/replaceable optional ... /optional/optional THEN
+   replaceablestatements/replaceable 
+   optional WHEN replaceableexpression/replaceable optional, replaceableexpression/replaceable optional ... /optional/optional THEN
+   replaceablestatements/replaceable 
+ ... /optional/optional
+   optional ELSE
+  replaceablestatements/replaceable /optional
+ END CASE;
+ /synopsis
+ para
+  Provide conditional execution based on equality of operands. If no case is matched,
+  then is ELSE clause executed. If statement doesn't contains ELSE clause,
+  then literalCASE_NOT_FOUND/literal exception is raised.
+ /para
+ paraHere is example:
+ programlisting
+ CASE a
+ WHEN 1, 2 THEN
+ msg := 'one or two';
+ ELSE
+ msg := 'other value than one or two';
+ END CASE; 
+ /programlisting
+ /para
+  /sect3
+ 
+  sect3
+   titleSearched literalCASE/ statement/title
+ synopsis
+ CASE
+ WHEN replaceableboolean-expression/replaceable THEN
+   replaceablestatements/replaceable
+   optional WHEN replaceableboolean-expression/replaceable THEN
+   replaceablestatements/replaceable 
+   optional WHEN replaceableboolean-expression/replaceable THEN
+   replaceablestatements/replaceable 
+ ... /optional/optional
+   optional ELSE
+  replaceablestatements/replaceable /optional
+ END CASE;
+ /synopsis
+ para
+  Provide conditional execution based on truth of 
+  replaceableboolean-expression/replaceable. If no case is matched,
+  then is ELSE clause executed. If statement doesn't contains ELSE clause,
+  then literalCASE_NOT_FOUND/literal exception is raised. 
+ /para
+ para Here is example:
+ programlisting
+ CASE 
+ WHEN a BETWEEN 0 AND 10 THEN
+ msg := 'value is between zero and ten';
+ WHEN a BETWEEN 11 AND 20 THEN
+ 	msg := 'value is between eleven and twenty';
+ END CASE;
+ /programlisting
+ /para
+ 
+  /sect3
 /sect2
  
 sect2 id=plpgsql-control-structures-loops
*** ./src/backend/catalog/sql_feature_packages.txt.orig	2008-05-06 11:01:18.0 +0200
--- ./src/backend/catalog/sql_feature_packages.txt	2008-05-06 11:05:05.0 +0200
***
*** 41,46 
--- 41,48 
  F671	Enhanced integrity management
  F701	Enhanced integrity 

Re: [PATCHES] column level privileges

2008-05-07 Thread Peter Eisentraut
Am Mittwoch, 7. Mai 2008 schrieb Tom Lane:
  1.1 Add a column named 'attrel' in pg_attribute catalog to store
  column privileges. Now all column privileges are stored, no matter
  whether they could be implied from table-level privilege.

 What this actually means, but doesn't say, is that there's no
 table-level representation of INSERT/UPDATE privilege any more at all.
 I think this is a pretty fundamental design error.  In the first place
 it bloats pg_attribute with data that's entirely redundant for the
 typical case where per-column privileges aren't used.  In the
 second place it slows privilege checking for the typical case, since
 instead of one check for the relation you have to do one for each
 attribute.  There are some other problems too, like having to extend
 pg_shdepend to include an objsubid column, and some other places where
 the patch has to do awkward things because it's now lacking table-level
 information about privilege checks.

I haven't read the patch, but there is also a semantic issue, namely what 
happens to columns added after the grant.  If the GRANT was to the table, new 
columns should get the same privileges.

-- 
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] column level privileges

2008-05-07 Thread Stephen Frost
* Andrew Dunstan ([EMAIL PROTECTED]) wrote:
 Tom Lane wrote:
 I'm not sure where we go from here.  Your GSOC student has disappeared,
 right?  Is anyone else willing to take up the patch and work on it?

 No, he has not disappeared at all. He is going to work on fixing issues  
 and getting the work up to SQL99 level.

Great!

 Your review should help enormously.

 Stephen, perhaps you would like to work with him.

I'd be happy to.

Thanks,

Stephen


signature.asc
Description: Digital signature


Re: [PATCHES] [EMAIL PROTECTED]: Re: [BUGS] Problem identifying constraints which should not be inherited]

2008-05-07 Thread Tom Lane
Alex Hunsaker [EMAIL PROTECTED] writes:
 Currently this loops through all the constraints for a relation (old
 behavior of MergeAttributesIntoExisting)... Do you think its worth
 adding a non-unique index to speed this up?

No.  If we were to refactor pg_constraint as I mentioned earlier,
then it could have a natural primary key (reloid, constrname)
(replacing the existing nonunique index on reloid) and then a number
of things could be sped up.  But just piling more indexes on a
fundamentally bad design doesn't appeal to me ...

Will review the revised patch today.

regards, tom lane

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


[PATCHES] [Fwd: Re: [HACKERS] pg_dump additional options for performance]

2008-05-07 Thread Simon Riggs
Re-sending post as discussed with Bruce...

On Sun, 2008-03-23 at 12:45 -0300, Alvaro Herrera wrote:
 Bruce Momjian wrote:
  
  Added to TODO:
  
  o Allow pre/data/post files when dumping a single object, for
performance reasons
  
http://archives.postgresql.org/pgsql-hackers/2008-02/msg00205.php
 
 When dumping a single object??  Do you mean database?

It would be for whatever set of objects are specified through the use of
databases, table include/exclude switches.

I've written a patch that implements these new switches on the commands
as shown

pg_dump --schema-pre-load
pg_dump --schema-post-load

pg_restore --schema-pre-load
pg_restore --schema-post-load

I have not implemented --schema-pre-file=xxx style because they don't
make any sense when using pg_restore in direct database connection mode.
On reflection I don't see any particular need to produce multiple files
as output, which just complicates an already horrendous user interface.

This is a minimal set of changes and includes nothing at all about
directories, parallelisation in the code etc..

This has the following use cases amongst others...

* dump everything to a file, then use pg_restore first --schema-pre-load
and then --data-only directly into the database, then pg_restore
--schema-post-load to a file so we can edit that file into multiple
pieces to allow index creation in parallel

* dump of database into multiple files by manually specifying which
tables go where, then reload in parallel using multiple psql sessions


The patch tests OK after some testing, though without a test suite that
probably isn't more than a few percentage points of all the possible
code paths. There are no docs for it, as yet.

---

Further thinking on this

Some further refinement might replace --data-only and --schema-only with
--want-schema-pre
--want-data
--want-schema-post
--want-schema (same as --want-schema-pre --want-schema-post)

These could be used together e.g. --want-schema-pre --want-data
whereas the existing --data-only type switches cannot.

Which would be a straightforward and useful change to the enclosed
patch.

That way of doing things is hierarchically extensible to include further
subdivisions of the set of SQL commands produced, e.g. divide
--want-post-schema into objects required to support various inter-table
dependencies and those that don't such as additional indexes. I don't
personally think we need that though.


Comments?

-- 
  Simon Riggs
  2ndQuadrant  http://www.2ndQuadrant.com
Index: src/bin/pg_dump/pg_backup.h
===
RCS file: /home/sriggs/pg/REPOSITORY/pgsql/src/bin/pg_dump/pg_backup.h,v
retrieving revision 1.45
diff -c -r1.45 pg_backup.h
*** src/bin/pg_dump/pg_backup.h	25 Jan 2007 03:30:43 -	1.45
--- src/bin/pg_dump/pg_backup.h	20 Mar 2008 11:48:34 -
***
*** 88,94 
  	int			use_setsessauth;/* Use SET SESSION AUTHORIZATION commands
   * instead of OWNER TO */
  	char	   *superuser;		/* Username to use as superuser */
! 	int			dataOnly;
  	int			dropSchema;
  	char	   *filename;
  	int			schemaOnly;
--- 88,94 
  	int			use_setsessauth;/* Use SET SESSION AUTHORIZATION commands
   * instead of OWNER TO */
  	char	   *superuser;		/* Username to use as superuser */
! 	int			dumpObjFlags;	/* which objects types to dump */
  	int			dropSchema;
  	char	   *filename;
  	int			schemaOnly;
Index: src/bin/pg_dump/pg_backup_archiver.c
===
RCS file: /home/sriggs/pg/REPOSITORY/pgsql/src/bin/pg_dump/pg_backup_archiver.c,v
retrieving revision 1.152
diff -c -r1.152 pg_backup_archiver.c
*** src/bin/pg_dump/pg_backup_archiver.c	14 Jan 2008 19:27:41 -	1.152
--- src/bin/pg_dump/pg_backup_archiver.c	20 Mar 2008 11:48:34 -
***
*** 56,62 
  static void _selectTablespace(ArchiveHandle *AH, const char *tablespace);
  static void processEncodingEntry(ArchiveHandle *AH, TocEntry *te);
  static void processStdStringsEntry(ArchiveHandle *AH, TocEntry *te);
! static teReqs _tocEntryRequired(TocEntry *te, RestoreOptions *ropt, bool include_acls);
  static void _disableTriggersIfNecessary(ArchiveHandle *AH, TocEntry *te, RestoreOptions *ropt);
  static void _enableTriggersIfNecessary(ArchiveHandle *AH, TocEntry *te, RestoreOptions *ropt);
  static TocEntry *getTocEntryByDumpId(ArchiveHandle *AH, DumpId id);
--- 56,62 
  static void _selectTablespace(ArchiveHandle *AH, const char *tablespace);
  static void processEncodingEntry(ArchiveHandle *AH, TocEntry *te);
  static void processStdStringsEntry(ArchiveHandle *AH, TocEntry *te);
! static int _tocEntryRequired(TocEntry *te, RestoreOptions *ropt, bool include_acls);
  static void _disableTriggersIfNecessary(ArchiveHandle *AH, TocEntry *te, RestoreOptions *ropt);
  static void _enableTriggersIfNecessary(ArchiveHandle *AH, TocEntry *te, RestoreOptions *ropt);
  static TocEntry 

Re: [PATCHES] [NOVICE] encoding problems

2008-05-07 Thread Bruce Momjian
Cliff Nieuwenhuis wrote:
 On Tuesday 11 March 2008 11:41:35 Tom Lane wrote:
  Cliff Nieuwenhuis [EMAIL PROTECTED] writes:
   I'm not sure how to ask this question.  I have written a function, and
   with PostgreSQL 8.0.13 I can do a \df+ and see something like this
   under Source Code:
 DECLARE
 result text;
   ...
  
   If I create the same function on my computer running PostgreSQL 8.3.0 and
   try the \df+ then the Source Code shows:
  
   \x09DECLARE
   \x09\x09result text;
   ...
 
  That's not an encoding problem, that's an intentional behavioral change
  in the way that psql formats strings for display.
 
  I guess it's a bit annoying if you were hoping that tabs would be useful
  for pretty-printing purposes.  Should we reconsider what's done with a
  tab in mbprint.c?
 
  regards, tom lane
 
 My vote would be to go back to the old way, or at least have that as an 
 option 
 of some sort.  I use command-line psql all the time -- to me, psql offers the 
 same advantages as using a command-line interface for other work. I find the 
 extra characters really get in the way.

Yes, I think our psql display of tabs needs improving too.  Our current
behavior is to output tab as 0x09:

test= SELECT E'\011';
 ?column?
--
 \x09
(1 row)

test= CREATE FUNCTION xx() RETURNS text AS E'
test' SELECT   ''a''::text
test' WHERE1 = 1'
test- LANGUAGE SQL;
CREATE FUNCTION

test= SELECT prosrc FROM pg_proc WHERE proname = 'xx';
   prosrc
-

 SELECT\x09'a'::text
 WHERE\x091 = 1
(1 row)

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\x09'a'::text
: WHERE\x091 = 1
Description |

I have implemented the following patch which outputs tab as a tab.  It
also assumes a tab has a width of 4, which is its average width:

test= SELECT E'\011';
 ?column?
--

(1 row)

test= SELECT prosrc FROM pg_proc WHERE proname = 'xx';
   prosrc
-

 SELECT 'a'::text
 WHERE  1 = 1
(1 row)

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 |

The only downside I see for this patch is that we are never sure of the
display width of tab because we don't know what tab stop we are at. 
With '\x09' we knew exactly how wide it was.

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

  + If your life is a hard drive, Christ can be your backup. +
Index: src/bin/psql/mbprint.c
===
RCS file: /cvsroot/pgsql/src/bin/psql/mbprint.c,v
retrieving revision 1.30
diff -c -c -r1.30 mbprint.c
*** src/bin/psql/mbprint.c	16 Apr 2008 18:18:00 -	1.30
--- src/bin/psql/mbprint.c	7 May 2008 15:18:25 -
***
*** 315,320 
--- 315,330 
  linewidth += 2;
  ptr += 2;
  			}
+ 			else if (*pwcs == '\t')		/* Tab */
+ 			{
+ strcpy((char *) ptr, \t);
+ /*
+  *	We don't know what tab stop we are on, so assuming 8-space
+  *	tabs, the average width of a tab is 4.
+  */
+ linewidth += 4;
+ ptr += 1;
+ 			}
  			else if (w  0)		/* Other control char */
  			{
  sprintf((char *) ptr, \\x%02X, *pwcs);

-- 
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-07 Thread Tom Lane
Bruce Momjian [EMAIL PROTECTED] writes:
 I have implemented the following patch which outputs tab as a tab.  It
 also assumes a tab has a width of 4, which is its average width:

That pretty much completely sucks; it will undo all the hard work we've
put into nice formatting of the output, because seven times out of eight
this assumption is wrong.

An actually acceptable solution would involve emitting the correct
number of spaces depending on how much we've put out so far.

regards, tom lane

-- 
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-07 Thread Bruce Momjian
Tom Lane wrote:
 Bruce Momjian [EMAIL PROTECTED] writes:
  I have implemented the following patch which outputs tab as a tab.  It
  also assumes a tab has a width of 4, which is its average width:
 
 That pretty much completely sucks; it will undo all the hard work we've
 put into nice formatting of the output, because seven times out of eight
 this assumption is wrong.
 
 An actually acceptable solution would involve emitting the correct
 number of spaces depending on how much we've put out so far.

Even if we knew the column position at output time, when we are doing
aligned column width computations, we don't know the width of the
previous columns so we would have no way to know how far the tab would
extend in the current column.

The only other idea I have is to output four spaces rather than '\x09'
for a 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


Re: [PATCHES] [NOVICE] encoding problems

2008-05-07 Thread Alvaro Herrera
Bruce Momjian wrote:

 Even if we knew the column position at output time, when we are doing
 aligned column width computations, we don't know the width of the
 previous columns so we would have no way to know how far the tab would
 extend in the current column.

If you start counting every line from the start of the current column,
it will align correctly regardless of the previous columns.

-- 
Alvaro Herrerahttp://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

-- 
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-07 Thread Bruce Momjian
Alvaro Herrera wrote:
 Bruce Momjian wrote:
 
  Even if we knew the column position at output time, when we are doing
  aligned column width computations, we don't know the width of the
  previous columns so we would have no way to know how far the tab would
  extend in the current column.
 
 If you start counting every line from the start of the current column,
 it will align correctly regardless of the previous columns.

At this stage you don't know the width of previous columns because you
don't know if a very wide value is coming in a later row, so there is no
way to output the width of the cell with a tab you are looking at now.

Unless I am misunderstanding you.

-- 
  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-07 Thread Alvaro Herrera
Bruce Momjian wrote:
 Alvaro Herrera wrote:

  If you start counting every line from the start of the current column,
  it will align correctly regardless of the previous columns.
 
 At this stage you don't know the width of previous columns because you
 don't know if a very wide value is coming in a later row, so there is no
 way to output the width of the cell with a tab you are looking at now.
 
 Unless I am misunderstanding you.

Surely psql computes the width of all cells before printing anything.

-- 
Alvaro Herrerahttp://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.

-- 
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-07 Thread Bruce Momjian
Alvaro Herrera wrote:
 Bruce Momjian wrote:
  Alvaro Herrera wrote:
 
   If you start counting every line from the start of the current column,
   it will align correctly regardless of the previous columns.
  
  At this stage you don't know the width of previous columns because you
  don't know if a very wide value is coming in a later row, so there is no
  way to output the width of the cell with a tab you are looking at now.
  
  Unless I am misunderstanding you.
 
 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.

-- 
  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-07 Thread Alvaro Herrera
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 Herrerahttp://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

-- 
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-07 Thread Bruce Momjian
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.

OK, so I am not really using tabs in the output, but outputting the
proper number of spaces to make it look like a tab?  That works.  Let me
try it.

-- 
  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] Snapshot management, final

2008-05-07 Thread Alvaro Herrera
Tom Lane wrote:

 I looked over this patch and I think it still needs work.

Thanks for the thorough review.  I'll be working on these problems soon.

-- 
Alvaro Herrerahttp://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

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


[PATCHES] libpq thread-locking

2008-05-07 Thread Magnus Hagander
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...

Comments?

//Magnus
Index: src/interfaces/libpq/fe-connect.c
===
RCS file: /cvsroot/pgsql/src/interfaces/libpq/fe-connect.c,v
retrieving revision 1.357
diff -c -r1.357 fe-connect.c
*** src/interfaces/libpq/fe-connect.c	31 Mar 2008 02:43:14 -	1.357
--- src/interfaces/libpq/fe-connect.c	7 May 2008 19:17:39 -
***
*** 3835,3848 
  		while (InterlockedExchange(mutex_initlock, 1) == 1)
  			 /* loop, another thread own the lock */ ;
  		if (singlethread_lock == NULL)
! 			pthread_mutex_init(singlethread_lock, NULL);
  		InterlockedExchange(mutex_initlock, 0);
  	}
  #endif
  	if (acquire)
! 		pthread_mutex_lock(singlethread_lock);
  	else
! 		pthread_mutex_unlock(singlethread_lock);
  #endif
  }
  
--- 3835,3867 
  		while (InterlockedExchange(mutex_initlock, 1) == 1)
  			 /* loop, another thread own the lock */ ;
  		if (singlethread_lock == NULL)
! 		{
! 			if (pthread_mutex_init(singlethread_lock, NULL))
! /*
!  * We have no way to pass back error values, but we don't want to
!  * proceed without having any locking. abort() seems to be the least
!  * evil thing to do here.
!  */
! abort();
! 		}
  		InterlockedExchange(mutex_initlock, 0);
  	}
  #endif
  	if (acquire)
! 	{
! 		if (pthread_mutex_lock(singlethread_lock))
! 			/* 
! 			 * We have no way to pass back error values, but we don't want to proceed
! 			 * without having any locking. abort() seems to be the least evil thing
! 			 * to do here.
! 			 */
! 			abort();
! 	}
  	else
! 	{
! 		if (pthread_mutex_unlock(singlethread_lock))
! 			abort();
! 	}
  #endif
  }
  
Index: src/interfaces/libpq/fe-secure.c
===
RCS file: /cvsroot/pgsql/src/interfaces/libpq/fe-secure.c,v
retrieving revision 1.104
diff -c -r1.104 fe-secure.c
*** src/interfaces/libpq/fe-secure.c	31 Mar 2008 02:43:14 -	1.104
--- src/interfaces/libpq/fe-secure.c	7 May 2008 19:17:39 -
***
*** 796,807 
  pq_lockingcallback(int mode, int n, const char *file, int line)
  {
  	if (mode  CRYPTO_LOCK)
! 		pthread_mutex_lock(pq_lockarray[n]);
  	else
! 		pthread_mutex_unlock(pq_lockarray[n]);
  }
  #endif   /* ENABLE_THREAD_SAFETY */
  
  static int
  init_ssl_system(PGconn *conn)
  {
--- 796,816 
  pq_lockingcallback(int mode, int n, const char *file, int line)
  {
  	if (mode  CRYPTO_LOCK)
! 	{
! 		if (pthread_mutex_lock(pq_lockarray[n]))
! 			abort();
! 	}
  	else
! 	{
! 		if (pthread_mutex_unlock(pq_lockarray[n]))
! 			abort();
! 	}
  }
  #endif   /* ENABLE_THREAD_SAFETY */
  
+ /*
+  * Also see similar code in fe-connect.c, default_threadlock()
+  */
  static int
  init_ssl_system(PGconn *conn)
  {
***
*** 817,827 
  		while (InterlockedExchange(mutex_initlock, 1) == 1)
  			 /* loop, another thread own the lock */ ;
  		if (init_mutex == NULL)
! 			pthread_mutex_init(init_mutex, NULL);
  		InterlockedExchange(mutex_initlock, 0);
  	}
  #endif
! 	pthread_mutex_lock(init_mutex);
  
  	if (pq_initssllib  pq_lockarray == NULL)
  	{
--- 826,840 
  		while (InterlockedExchange(mutex_initlock, 1) == 1)
  			 /* loop, another thread own the lock */ ;
  		if (init_mutex == NULL)
! 		{
! 			if (pthread_mutex_init(init_mutex, NULL))
! return -1;
! 		}
  		InterlockedExchange(mutex_initlock, 0);
  	}
  #endif
! 	if (pthread_mutex_lock(init_mutex))
! 		return -1;
  
  	if (pq_initssllib  pq_lockarray == NULL)
  	{
***
*** 836,842 
  			return -1;
  		}
  		for (i = 0; i  CRYPTO_num_locks(); i++)
! 			pthread_mutex_init(pq_lockarray[i], NULL);
  
  		CRYPTO_set_locking_callback(pq_lockingcallback);
  	}
--- 849,858 
  			return -1;
  		}
  		for (i = 0; i  CRYPTO_num_locks(); i++)
! 		{
! 			if (pthread_mutex_init(pq_lockarray[i], NULL))
! return -1;
! 		}
  
  		CRYPTO_set_locking_callback(pq_lockingcallback);
  	}
Index: src/interfaces/libpq/pthread-win32.c
===
RCS file: /cvsroot/pgsql/src/interfaces/libpq/pthread-win32.c,v
retrieving revision 1.15
diff -c -r1.15 pthread-win32.c
*** src/interfaces/libpq/pthread-win32.c	1 Jan 2008 19:46:00 -	1.15
--- src/interfaces/libpq/pthread-win32.c	7 May 2008 19:17:39 -
***
*** 32,51 
  	return NULL;
  }
  
! void
  pthread_mutex_init(pthread_mutex_t *mp, void *attr)
  {
  	*mp = CreateMutex(0, 0, 0);
  }
  
! void
  pthread_mutex_lock(pthread_mutex_t *mp)
  {
! 	WaitForSingleObject(*mp, INFINITE);
  }
  
! void
  pthread_mutex_unlock(pthread_mutex_t *mp)
  {
! 	ReleaseMutex(*mp);
  }
--- 32,58 
  	return NULL;
 

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

2008-05-07 Thread Bruce Momjian
Richard Wang wrote:
 I expected 0 ^ 123.3 to be 0, but it reported error as follows
 
 postgres=# select 0 ^ 123.3;
 ERROR:  cannot take logarithm of zero
 
 I find that there is a bug in numeric_power() function
 the function caculates a ^ b based on the algorithm e ^ (lna * b)
 as you see, ln0 is not valid 

I have developed the attached patch which fixes 0 ^ 123.3.  It also
fixes the case for 0 ^ 0.0 so it returns 1 instead of an error --- see
the C comment for why one is the proper return value.  float pow()
already returned one in this case:

test= select 0 ^ 0;
 ?column?
--
1
(1 row)

test= select 0 ^ 0.0;
 ?column?
--
1
(1 row)

test= select 0 ^ 3.4;
 ?column?
--
1
(1 row)

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

  + If your life is a hard drive, Christ can be your backup. +
Index: src/backend/utils/adt/numeric.c
===
RCS file: /cvsroot/pgsql/src/backend/utils/adt/numeric.c,v
retrieving revision 1.110
diff -c -c -r1.110 numeric.c
*** src/backend/utils/adt/numeric.c	21 Apr 2008 00:26:45 -	1.110
--- src/backend/utils/adt/numeric.c	7 May 2008 20:05:01 -
***
*** 5170,5175 
--- 5170,5187 
  	int			local_rscale;
  	double		val;
  
+ 	/*
+ 	 *	This avoids log(0) for cases of 0 raised to a non-integer.
+ 	 *	We also treat 0 ^ 0 == 1 because it is the best value for discrete
+ 	 *	mathematics, and most programming languages do this.
+ 	 *	http://en.wikipedia.org/wiki/Exponentiation#Zero_to_the_zero_power
+ 	 */
+ 	if (cmp_var(base, const_zero) == 0)
+ 	{
+ 		set_var_from_var(const_one, result);
+ 		return;
+ 	}
+ 	
  	/* If exp can be represented as an integer, use power_var_int */
  	if (exp-ndigits == 0 || exp-ndigits = exp-weight + 1)
  	{
***
*** 5266,5280 
  	NumericVar	base_prod;
  	int			local_rscale;
  
- 	/* Detect some special cases, particularly 0^0. */
- 
  	switch (exp)
  	{
  		case 0:
- 			if (base-ndigits == 0)
- ereport(ERROR,
- 		(errcode(ERRCODE_FLOATING_POINT_EXCEPTION),
- 		 errmsg(zero raised to zero is undefined)));
  			set_var_from_var(const_one, result);
  			result-dscale = rscale;	/* no need to round */
  			return;
--- 5278,5286 

-- 
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-07 Thread Bruce Momjian
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. +
Index: src/bin/psql/mbprint.c
===
RCS file: /cvsroot/pgsql/src/bin/psql/mbprint.c,v
retrieving revision 1.30
diff -c -c -r1.30 mbprint.c
*** src/bin/psql/mbprint.c	16 Apr 2008 18:18:00 -	1.30
--- src/bin/psql/mbprint.c	7 May 2008 20:27:39 -
***
*** 315,320 
--- 315,328 
  linewidth += 2;
  ptr += 2;
  			}
+ 			else if (*pwcs == '\t')		/* Tab */
+ 			{
+ do
+ {
+ 	*ptr++ = ' ';
+ 	linewidth++;
+ } while (linewidth % 8 != 0);
+ 			}
  			else if (w  0)		/* Other control char */
  			{
  sprintf((char *) ptr, \\x%02X, *pwcs);

-- 
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] [GENERAL] pgbench not setting scale size correctly?

2008-05-07 Thread Bruce Momjian
Tom Lane wrote:
 Greg Smith [EMAIL PROTECTED] writes:
  On Fri, 14 Mar 2008, Tom Lane wrote:
  Yeah, -s is only meaningful when given with -i.  Maybe someday we ought
  to fix pgbench to complain if you try to set it at other times.
 
  You have to pass -s in to the actual run if you're specifying your own 
  custom script(s) using -f and you want the :scale variable to be defined. 
 
 Right, I knew that at one time ;-)
 
  The way the option parsing code is done would make complaining in the case 
  where your parameter is ignored a bit of a contortion.  The part that 
  detects based on the database is after all the other parsing because the 
  connection has to be brought up first.
 
 Yeah.  But couldn't we have that part issue a warning if -s had been set
 on the command line?

Patch attached that issues a warning.

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

  + If your life is a hard drive, Christ can be your backup. +
Index: contrib/pgbench/pgbench.c
===
RCS file: /cvsroot/pgsql/contrib/pgbench/pgbench.c,v
retrieving revision 1.79
diff -c -c -r1.79 pgbench.c
*** contrib/pgbench/pgbench.c	19 Mar 2008 03:33:21 -	1.79
--- contrib/pgbench/pgbench.c	7 May 2008 21:36:42 -
***
*** 1627,1632 
--- 1627,1635 
  		}
  	}
  
+ 	if (!is_init_mode  scale)
+ 		fprintf(stderr, Scale specification ignored because init mode (-i) not specified\n);
+ 	
  	if (argc  optind)
  		dbName = argv[optind];
  	else

-- 
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-07 Thread Tom Lane
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?

regards, tom lane

-- 
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] printTable API (was: Show INHERIT in \du)

2008-05-07 Thread Alvaro Herrera
Brendan Jurd escribió:

 On Thu, Apr 17, 2008 at 7:27 AM, Alvaro Herrera  wrote:

   Thanks.  I looked the patch over and did some minor changes.  Modified
   version attached.
 
 Cool, I had a look through your changes and they all seemed fine to
 me.  In particular, moving the header comments to the ... header ...
 file seemed like a smart move. =)

FWIW I just noticed something else.  With this patch we add pg_strdup
calls into print.c.  pg_strdup lives in common.c.

This is fine as psql is concerned, but we have another problem which is
that in bin/scripts there are two programs that want to use
printQuery().  The problem is that there's no pg_strdup there :-(

The easy solution is to add pg_strdup to bin/scripts/common.c, but there
we don't have a global progname, so the error report in the out of
memory case cannot carry the name of the program crashing.

I don't like that, but I don't see any other solution.  Ideas welcome.

-- 
Alvaro Herrerahttp://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

-- 
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] [GENERAL] pgbench not setting scale size correctly?

2008-05-07 Thread Greg Smith

On Wed, 7 May 2008, Bruce Momjian wrote:


Patch attached that issues a warning.


This doesn't take into account the -F case and the warning isn't quite 
right because of that as well.  When I get a break later today I'll create 
a new patch that handles that correctly, I see where it should go now that 
I look at this again.


--
* Greg Smith [EMAIL PROTECTED] http://www.gregsmith.com Baltimore, MD

--
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-07 Thread Peter Eisentraut
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?

Considering that 0::float8 ^ 0::float8 yields 1, making the numeric operator 
do the same might not be completely unreasonable, but I find the rationale 
that it is better for discrete mathematics fairly ludicrous on multiple 
levels.

-- 
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-07 Thread Bruce Momjian
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?

Yes:

The evaluation of 0^0 presents a problem, because different mathematical
reasoning leads to different results. The best choice for its value
depends on the context. According to Benson (1999), The choice whether
to define 00 is based on convenience, not on correctness.[2] There are
two principal treatments in practice, one from discrete mathematics and
the other from analysis.

...

The computer programming languages that evaluate 00 to be 1[8] include
J, Java, Python, Ruby, Haskell, ML, Scheme, MATLAB, bc, R programming
language, and Microsoft Windows' Calculator.

-- 
  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-07 Thread Alvaro Herrera
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.

-- 
Alvaro Herrerahttp://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

-- 
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-07 Thread Bruce Momjian
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. +
Index: src/backend/utils/adt/numeric.c
===
RCS file: /cvsroot/pgsql/src/backend/utils/adt/numeric.c,v
retrieving revision 1.110
diff -c -c -r1.110 numeric.c
*** src/backend/utils/adt/numeric.c	21 Apr 2008 00:26:45 -	1.110
--- src/backend/utils/adt/numeric.c	7 May 2008 23:18:31 -
***
*** 5170,5175 
--- 5170,5190 
  	int			local_rscale;
  	double		val;
  
+ 	/*
+ 	 *	This avoids log(0) for cases of 0 raised to a non-integer.
+ 	 *	Also, while 0 ^ 0 can be either 1 or indeterminate (error), we
+ 	 *	treat it as one because most programming languages do this.
+ 	 *	http://en.wikipedia.org/wiki/Exponentiation#Zero_to_the_zero_power
+ 	 */
+ 	if (cmp_var(base, const_zero) == 0)
+ 	{
+ 		if (cmp_var(exp, const_zero) == 0)
+ 			set_var_from_var(const_one, result);
+ 		else
+ 			set_var_from_var(const_zero, result);
+ 		return;
+ 	}
+ 	
  	/* If exp can be represented as an integer, use power_var_int */
  	if (exp-ndigits == 0 || exp-ndigits = exp-weight + 1)
  	{
***
*** 5266,5280 
  	NumericVar	base_prod;
  	int			local_rscale;
  
- 	/* Detect some special cases, particularly 0^0. */
- 
  	switch (exp)
  	{
  		case 0:
- 			if (base-ndigits == 0)
- ereport(ERROR,
- 		(errcode(ERRCODE_FLOATING_POINT_EXCEPTION),
- 		 errmsg(zero raised to zero is undefined)));
  			set_var_from_var(const_one, result);
  			result-dscale = rscale;	/* no need to round */
  			return;
--- 5281,5289 

-- 
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-07 Thread Alvaro Herrera
Bruce Momjian wrote:

 Ah, got it, and I updated the patch to remove the commment about
 discrete.

The page also says that 0^x is undefined when x is negative, not sure
about that one but I don't see it in your patch.

-- 
Alvaro Herrerahttp://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

-- 
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-07 Thread Bruce Momjian
Alvaro Herrera wrote:
 Bruce Momjian wrote:
 
  Ah, got it, and I updated the patch to remove the commment about
  discrete.
 
 The page also says that 0^x is undefined when x is negative, not sure
 about that one but I don't see it in your patch.

That one was already handled:

test= select 0 ^ -1;
ERROR:  invalid argument for power function
test= select 0 ^ -1.0;
ERROR:  invalid argument for power function

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