Re: [HACKERS] Issue in Behavior of Interval Datatype

2012-08-04 Thread Amit Kapila
From: Tom Lane [mailto:t...@sss.pgh.pa.us] 
Sent: Saturday, August 04, 2012 1:48 AM
Amit Kapila amit.kap...@huawei.com writes:
 select (interval '56:48'  minute to second);
 result$B!'(B00:56:48
 select (interval '-56:48'  minute to second);
 result$B!'(B-56:48:00
 select (interval '+56:48'  minute to second);
 result$B!'(B56:48:00

 I have checked the code and found that in function DecodeInterval(), for
 timezone case (DTK_TZ) it uses INTERVAL_FULL_RANGE irrespective of range
 passed by user.

 However if use the range passed as argument in function DecodeInterval(),
 the result of using $B!F(B+$B!G(B or $B!F(B-$B!F(B is same as
without using it.

 Is there any particular reason for ignoring the range for DTK_TZ case in
 DecodeInterval() function?

 I think you are right; this if-block should be exactly like the DTK_TIME
 case except for handling the prepended sign.  That also raises the
 question why it is changing the tmask value returned by DecodeTime.
 It seems to be doing exactly the wrong thing there.  Test case:

 regression=# select (interval '56:48 56:48'  );
 ERROR:  invalid input syntax for type interval: 56:48 56:48
 LINE 1: select (interval '56:48 56:48'  );
 ^
 regression=# select (interval '56:48 +56:48'  );
  interval 
 --
 56:48:00
 (1 row)

 The second one fails to fail because an inappropriate tmask value got
included into fmask.

Yes, this is right that tmask need not be changed, also one more thing I
have noticed that
in file Interval.c also there is a function DecodeInterval() which is
currently little different
from DecodeInterval() in datetime.c for DTK_TZ case.
For example Assert check is commented.
Why the Assert check is commented out there?
May be due to some defect, but I am not able to think the reason for same.

Shouldn't we make this change (don't change tmask in case of DTK_TZ) in
Interval.c?

Will fix.

Thank you very much.

With Regards,
Amit Kapila.


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


Re: [HACKERS] [WIP] Performance Improvement by reducing WAL for Update Operation

2012-08-04 Thread Amit Kapila
From: Heikki Linnakangas [mailto:heikki.linnakan...@enterprisedb.com] 
Sent: Saturday, August 04, 2012 1:33 AM
On 03.08.2012 14:46, Amit kapila wrote:
 Currently the change is done only for fixed length columns for simple
tables and the tuple should not contain NULLS.

 This is a Proof of concept, the design and implementation needs to be
changed based on final design required for handling other scenario's

 Update operation:
 -
 1. Check for the simple table or not.(No toast, No before update
triggers)
 2. Works only for not null tuples.
 3. Identify the modified columns from the target entry.
 4. Based on the modified column list, check for any variable length
columns are modified, if so this optimization is not applied.
 5. Identify the offset and length for the modified columns and store it
as an optimized WAL tuple in the following format.
 Note: Wal update header is modified to denote whether wal update
optimization is done or not.
  WAL update header + Tuple header(no change from previous format)
+
  [offset(2bytes)] [length(2 bytes)] [changed data value]
  [offset(2bytes)] [length(2 bytes)] [changed data value]



 The performance will need to be re-verified after you fix these 
 limitations. Those limitations need to be fixed before this can be
applied.

Yes, I agree that solution should fix these limitations and performance
numbers needs to be re-verified. 
Currently in my mind the work to be done is as follows:

1. Solution which can handle Variable length columns and NULLs
2. Handling of Before Triggers
3. Can the solution for fixed length columns be same as Variable length
columns and NULLS.
4. Make the final code patch which addresses all the above.

Please suggest if there are more things that needs to be handled?

For the 3rd point, currently the solution for fixed length columns cannot
handle the case of variable length columns and NULLS. The reason is for
fixed length columns there is no need of diff technology between old and new
tuple, however for other cases it will be required.
For fixed length columns, if we just note the OFFSET, LENGTH, VALUE of
changed columns of new tuple in WAL, it will be sufficient to do the replay
of WAL. However to handle other cases we need to use diff mechanism.

Can we do something like if the changed columns are fixed length and doesn't
contain NULL's, then store [OFFSET, LENGTH, VALUE] format in WAL and for
other cases store diff format.

This has advantage that for Updates containing only fixed length columns
don't have to pay penality of doing diff between new and old tuple. Also we
can do the whole work in 2 parts, one for fixed length columns and second to
handle other cases. 


 It would be nice to use some well-known binary delta algorithm for this, 
 rather than invent our own. OTOH, we have more knowledge of the 
 attribute boundaries, so a custom algorithm might work better. 

I shall work on this and post after initial work.

 In any case, I'd like to see the code to do the delta encoding/decoding to
be 
 put into separate functions, outside of heapam.c. It would be good for 
 readability, and we might want to reuse this in other places too.

Agreed. I shall take care of doing it in suggested way.


With Regards,
Amit Kapila.



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


[HACKERS] [WIP] Performance Improvement by reducing WAL for Update Operation

2012-08-04 Thread Amit Kapila
Missed one point which needs to be handled is pg_upgrade

-Original Message-
From: pgsql-hackers-ow...@postgresql.org
[mailto:pgsql-hackers-ow...@postgresql.org] On Behalf Of Amit Kapila
Sent: Saturday, August 04, 2012 12:12 PM
To: 'Heikki Linnakangas'
Cc: pgsql-hackers@postgresql.org
Subject: Re: [HACKERS] [WIP] Performance Improvement by reducing WAL for
Update Operation

From: Heikki Linnakangas [mailto:heikki.linnakan...@enterprisedb.com] 
Sent: Saturday, August 04, 2012 1:33 AM
On 03.08.2012 14:46, Amit kapila wrote:
 Currently the change is done only for fixed length columns for simple
tables and the tuple should not contain NULLS.

 This is a Proof of concept, the design and implementation needs to be
changed based on final design required for handling other scenario's

 Update operation:
 -
 1. Check for the simple table or not.(No toast, No before update
triggers)
 2. Works only for not null tuples.
 3. Identify the modified columns from the target entry.
 4. Based on the modified column list, check for any variable length
columns are modified, if so this optimization is not applied.
 5. Identify the offset and length for the modified columns and store it
as an optimized WAL tuple in the following format.
 Note: Wal update header is modified to denote whether wal update
optimization is done or not.
  WAL update header + Tuple header(no change from previous format)
+
  [offset(2bytes)] [length(2 bytes)] [changed data value]
  [offset(2bytes)] [length(2 bytes)] [changed data value]



 The performance will need to be re-verified after you fix these 
 limitations. Those limitations need to be fixed before this can be
applied.

Yes, I agree that solution should fix these limitations and performance
numbers needs to be re-verified. 
Currently in my mind the work to be done is as follows:

1. Solution which can handle Variable length columns and NULLs
2. Handling of Before Triggers
3. Can the solution for fixed length columns be same as Variable length
columns and NULLS.
4. Make the final code patch which addresses all the above.

Please suggest if there are more things that needs to be handled?

For the 3rd point, currently the solution for fixed length columns cannot
handle the case of variable length columns and NULLS. The reason is for
fixed length columns there is no need of diff technology between old and new
tuple, however for other cases it will be required.
For fixed length columns, if we just note the OFFSET, LENGTH, VALUE of
changed columns of new tuple in WAL, it will be sufficient to do the replay
of WAL. However to handle other cases we need to use diff mechanism.

Can we do something like if the changed columns are fixed length and doesn't
contain NULL's, then store [OFFSET, LENGTH, VALUE] format in WAL and for
other cases store diff format.

This has advantage that for Updates containing only fixed length columns
don't have to pay penality of doing diff between new and old tuple. Also we
can do the whole work in 2 parts, one for fixed length columns and second to
handle other cases. 


 It would be nice to use some well-known binary delta algorithm for this, 
 rather than invent our own. OTOH, we have more knowledge of the 
 attribute boundaries, so a custom algorithm might work better. 

I shall work on this and post after initial work.

 In any case, I'd like to see the code to do the delta encoding/decoding to
be 
 put into separate functions, outside of heapam.c. It would be good for 
 readability, and we might want to reuse this in other places too.

Agreed. I shall take care of doing it in suggested way.


With Regards,
Amit Kapila.



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


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


Re: [HACKERS] [WIP] Performance Improvement by reducing WAL for Update Operation

2012-08-04 Thread Heikki Linnakangas

On 04.08.2012 11:01, Amit Kapila wrote:

Missed one point which needs to be handled is pg_upgrade


I don't think there's anything to do for pg_upgrade. This doesn't change 
the on-disk data format, just the WAL format, and pg_upgrade isn't 
sensitive to WAL format changes.


--
  Heikki Linnakangas
  EnterpriseDB   http://www.enterprisedb.com

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


Re: [HACKERS] Word-smithing doc changes

2012-08-04 Thread Bruce Momjian
On Fri, Aug  3, 2012 at 01:23:53PM -0400, Bruce Momjian wrote:
 On Fri, Aug  3, 2012 at 12:55:30PM -0400, Álvaro Herrera wrote:
  Excerpts from Bruce Momjian's message of vie ago 03 09:59:36 -0400 2012:
   On Fri, Aug  3, 2012 at 12:26:56AM -0400, Bruce Momjian wrote:
The concurrent index documentation under discussion above was never
updated, so I took a stab at it, attached.

Greg, I looked at adding a mention of the virtual transaction wait to
the explicit-locking section as you suggested, and found those were
all user-visible locking, while this is internal locking.  I did find a
clear description of transaction id locking in the pg_locks system view
docs, so I just referenced that.
   
   I found a way to clarify the wording further;  patch attached.
  
  Looks sane to me.
  
  Are we backpatching this to 9.1?  I no longer remember if the original
  wording is there or just in 9.2.
 
 I wasn't planning to, but will do as you suggest for 9.1.

Done.

-- 
  Bruce Momjian  br...@momjian.ushttp://momjian.us
  EnterpriseDB http://enterprisedb.com

  + It's impossible for everything to be true. +

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


Re: [HACKERS] [WIP] Performance Improvement by reducing WAL for Update Operation

2012-08-04 Thread Bruce Momjian
On Sat, Aug  4, 2012 at 05:21:06PM +0300, Heikki Linnakangas wrote:
 On 04.08.2012 11:01, Amit Kapila wrote:
 Missed one point which needs to be handled is pg_upgrade
 
 I don't think there's anything to do for pg_upgrade. This doesn't
 change the on-disk data format, just the WAL format, and pg_upgrade
 isn't sensitive to WAL format changes.

Correct.

-- 
  Bruce Momjian  br...@momjian.ushttp://momjian.us
  EnterpriseDB http://enterprisedb.com

  + It's impossible for everything to be true. +

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


Re: [HACKERS] Issue in Behavior of Interval Datatype

2012-08-04 Thread Tom Lane
Amit Kapila amit.kap...@huawei.com writes:
 Yes, this is right that tmask need not be changed, also one more thing I
 have noticed that
 in file Interval.c also there is a function DecodeInterval() which is
 currently little different
 from DecodeInterval() in datetime.c for DTK_TZ case.
 For example Assert check is commented.
 Why the Assert check is commented out there?  

Assert doesn't work in client-side code.

More generally, nobody is maintaining ecpg's copy of the datetime code,
and that's been true for a very long time.  I'm not personally
interested in trying to re-sync that copy.  It would be more useful to
figure out a way to get rid of it.  There have been past discussions of
how we could make a single copy of the code work in both backend and
frontend contexts, but the frontend environment is so impoverished by
comparison (no Assert, no elog, no palloc) that it hasn't looked like an
attractive idea.  It's also fairly unclear whether anyone is actually
using ecpg's client-side datetime support, which means there's little
motivation to put a lot of work into it.

regards, tom lane

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


Re: [HACKERS] WIP pgindent replacement

2012-08-04 Thread Bruce Momjian
On Fri, Aug  3, 2012 at 02:53:08PM -0400, Bruce Momjian wrote:
 On Fri, Aug  3, 2012 at 02:08:34PM -0400, Andrew Dunstan wrote:
  $ find . -type f -exec file {} \;|
 egrep -i 'perl.*(script|module)'|grep -v '\.p[lm]:'
  
  and got:
  
  ./src/pl/plperl/ppport.h: awk script text
  ./src/tools/pginclude/pgcheckdefines: a /usr/bin/perl -w script text 
  executable
  ./src/tools/git_changelog: a /usr/bin/perl script text executable
  
  The last two are Perl scripts without Perl file extensions, so let's
  just go with 'pgindent' and I will hard-code those into the perltidy
  instructions.
  
  
  
  
  Your pattern has produced a false positive, though. Wouldn't it be
  better not to hardcode anything?
 
 You mean use an actual 'grep' to find the Perl programs --- I can do
 that.

OK, pgindent replaced by Perl version.  I had to go with both a file
extension check and 'file' output check because some Perl scripts only
match one category;  checks are:

(
find . -name \*.pl -o -name \*.pm

find . -type f -exec file {} \; |
egrep -i ':.*perl[0-9]*\' |
cut -d: -f1
) |
sort -u |
xargs perltidy --profile=src/tools/pgindent/perltidyrc

-- 
  Bruce Momjian  br...@momjian.ushttp://momjian.us
  EnterpriseDB http://enterprisedb.com

  + It's impossible for everything to be true. +

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


Re: [HACKERS] cataloguing NOT NULL constraints

2012-08-04 Thread Bruce Momjian
On Fri, Jul 22, 2011 at 12:14:30PM -0400, Robert Haas wrote:
 On Thu, Jul 21, 2011 at 7:51 PM, Alvaro Herrera
 alvhe...@commandprompt.com wrote:
  I think that there probably ought to be a way to display the NOT NULL
  constraint names (perhaps through \d+). For example, if you're
  planning to support NOT VALID on top of this in the future, then there
  needs to be a way to get the constraint's name to validate it.
 
  Absolutely true.  Another thing that needs to be done here is to let the
  ALTER TABLE and ALTER DOMAIN commands use the constraint names; right
  now, they simply let you add the constraint but not specify the name.
  That should probably be revisited.
 
 That, at least, seems like something that should be fixed before commit.

I have added a URL of this thread to the TODO list.

-- 
  Bruce Momjian  br...@momjian.ushttp://momjian.us
  EnterpriseDB http://enterprisedb.com

  + It's impossible for everything to be true. +

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


[HACKERS] Pgadmin3 v1.14.2 foreign keys

2012-08-04 Thread Mary F. Masterson
I searched the archives and didn't find the answer as to how to add
foreign keys to a table using pgadminIII v1.14.2.  I am very familiar with
relational databases and foreign keys, indices, etc., but I am a newbie to
postgresql. For instance, say I have two tables, address and
addresstype, both within the same schema.  The column named type in
the address table is the foreign key to the addresstype table, the
primary key of which is named addresstype_id.  I created those columns
and added the primary key constraints without any difficulty.  I tried
adding the foreign key constraint, by right-clicking the constraints
object for the address table in the object browser window and selecting
add foreign key.  I cannot figure out the next steps.  Any help, please?
 Thank you.

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


[HACKERS] Pgadmin3 v1.14.2 foreign keys

2012-08-04 Thread Mary F. Masterson
Sorry for the mis-post.  Got the answer on the pgsql-general.

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