Thread

1. Re: Review: Patch: Allow substring/replace() to get/set bit values

   Kevin Grittner <kevin.grittner@wicourts.gov> — 2010-01-19T15:00:44Z

   Leonardo F  wrote:
    
   >> In the documentation, the get_bit and set_bit methods are added
   >> to a list where we state "The following SQL-standard functions
   >> work on bit strings as well as character strings"; however they
   >> are not SQL-standard functions and are implemented on binary
   >> strings, not character strings. 
   > 
   > Ok, I didn't change that, but I guess I can fix it.
    
   The items already on that list, and substring function that you
   added, *do* fit the description; get_bit and set_bit don't fit that
   description, so they don't belong on that list.  They must be
   described in some other way.
    
   >> There is a peculiar omission from the patch -- it adds support
   >> for the overlay function to bit strings but not binary strings.
   >> Since the standard drops the bit string as a standard type in the
   >> 2003 standard, in favor of boolean type and binary string types,
   >> it seems odd to omit binary string support (which is defined in
   >> the standard) but include bit string support (which isn't).  What
   >> the patch adds seems useful, and I don't think the omission
   >> should be considered fatal, but it would be nice to see us also
   >> cover binary strings if we can.
   >
   > Mmh, don't know how complicated that would be, but I can give it a
   > try: I guess it could be a simple SQL replacement (as for char
   > strings and bit strings?)
    
   The standard explicitly defines it as being equivalent to the
   substring and concatenation technique, so that should work.
    
   >> Well, there was one thing which is beyond my ken in this regard:
   >> I'm not sure that the cases from bits8 to (unsigned char *) are
   >> needed or appropriate, although on my machine they don't seem to
   >> hurt.  Perhaps someone with a better grasp of such issues can
   >> comment.
   > 
   > That's some kind of copy&paste from varlena.c byteaGetBit: I don't
   > know if they can be avoided.
    
   Ah, but there it's casting the result of VARDATA_ANY.  Here you
   already have the value in a bits8 variable.  I looked it over some
   more and I'm pretty confident we can omit those casts in bitsetbit.
    
   > [pointer is zero-based]
   >
   >> Ok; I found them bizarre too, but I kept it consistent.
    
   >> [int32 used for bit values]
   > 
   > Same as above: I tried to be consistent.
    
   >> [ERRCODE_ARRAY_SUBSCRIPT_ERROR used when query has no array]
   > 
   > Again, same as above: consistency; but I can change it if you want
    
   I'm not suggesting the above require changes; I mentioned those
   primarily for the benefit of whoever eventually commits this, to try
   to save time chasing down the issues.
    
   >> [questions use of oldByte and newByte local variables]
   > 
   > I agree, you version is cleaner.
    
   Cool.  Should look even tidier without those casts.  :-)
    
   -Kevin
   
   

2. Re: Review: Patch: Allow substring/replace() to get/set bit values

   Leonardo Francalanci <m_lists@yahoo.it> — 2010-01-20T10:27:49Z

   New version of the patch, let me know if I can fix/change something else.
   
   
   
   Leonardo