Changes to comparable method in TypeCompiler

[Mamta Satoor <ms...@gmail.com>]

Hi,

One of the methods in TypeCompiler(TC) interface is
  boolean    comparable(
       TypeId otherType,
       boolean forEquals,
       ClassFactory cf);

The abstract class implementation of TC is BaseTypeCompiler(BTC) and BTC has
a private field called correspondingTypeId which is used as one of the
operands in compareable method implementation. This TypeId is compared with
the first argument to the comparable method (which is the otherType). So,
the thing to note is that the comparison is between two TypeIds and not
TypeDescriptors.

TC also has another method called
 DataTypeDescriptor resolveArithmeticOperation(
       DataTypeDescriptor leftType,
       DataTypeDescriptor rightType,
       String operator
        )
       throws StandardException;
This particular method just throws an exception at the abstract class level
in BTC because it makes sense for only numeric types and hence is
implemented by NumericTypeCompiler. The NumericTypeCompiler uses the
parameters passed to the method to do whatever is required and ignores the
TypeId associated with the NumericTypeCompiler.

This is the background information about how there are 2 different kinds
of methods in TC. One type that uses the TypeId associated with BTC to
implement the methods and the other type which ignore TypeId and just uses
the passed parameters to provide the method implementation.

What I am proposing is to change the comparable method from type 1 to type
2. The reason for this is comparable method for character types should also
check the collation type and derivation while deciding whether the method
should return true or false. But since the collation information is not
associated with TypeId, we need to pass the TypeDescriptors of both the
types that need to be compared. In order to do this, I propose the
comparable method to change as follows

boolean    comparable(
       DataTypeDescriptor leftType,
       DataTypeDescriptor rightType,
       boolean forEquals,
       ClassFactory cf);

This change would require that all the implementation of TC change and all
the callers of this method should change too. The implementation changes in
all the TCs other than CharTypeCompiler would be minimal, ie they will
simply get the TypeId from the first operand rather than using the TypeId
associated with itself and then do the comparison based on that.
CharTypeCompiler will have little more involved changes in the sense that
now it should also compare the collation types and derivations of the 2
operands. We want to disallow comparison between character types with
different collations. as stated on the wiki page
http://wiki.apache.org/db-derby/BuiltInLanguageBasedOrderingDERBY-1478 under
Section Collation Determination Rule 12.

Despite these change, I think it is the right thing to do to make sure
collation gets considered in character type comparisons. It also fits well
with existing TypeCompiler interface.

I would very much appreciate community's response on this approach. In the
mean time, I will go ahead and pursue this to see how the implementation
goes.

thanks,
Mamta

Re: Changes to comparable method in TypeCompiler

[Army <qo...@gmail.com>]

Mamta Satoor wrote:

> Army, I just finished updating the wiki page to reflect the fact that the
> result character string from XMLSERIALIZE will have the same collation as
> current schema's character set. It is part of number 6 on the wiki page
> under section Collation Determination.

Thanks Mamta.

Army

Re: Changes to comparable method in TypeCompiler

[Mamta Satoor <ms...@gmail.com>]

Army, I just finished updating the wiki page to reflect the fact that the
result character string from XMLSERIALIZE will have the same collation as
current schema's character set. It is part of number 6 on the wiki page
under section Collation Determination.

Mamta


On 4/11/07, Mamta Satoor <ms...@gmail.com> wrote:
>
> Hi Army,
>
> Thanks for the sanity check on what I am proposing for comparable method.
>
> As for the changes required to instantiate the proper runtime class, ie
> SQLChar vs CollatorSQLChar, I plan to address that as part of Outstanding
> items section -> Language changes -> 3). I have not started looking at that
> yet. What I am working on is loading the correct collation type for various
> character types (Section Collation Determination) and disallowing comparison
> between character types with different collations at compile time.
>
> Thanks for bringing up XML.XMLSerialize(). I was not aware of it. I agree
> to the approach of "XMLSERIALIZE can be characterized in the same way as the
> CHAR and VARCHAR functions." I will update the wiki page for XMLSERIALIZE
> to reflect that.
>
> thanks a lot,
> Mamta
>
> On 4/11/07, Army <qo...@gmail.com> wrote:
> >
> > Mamta Satoor wrote:
> > >
> > > What I am proposing is to change the comparable method from type 1 to
> > type
> > > 2. The reason for this is comparable method for character types should
> > also
> > > check the collation type and derivation while deciding whether the
> > method
> > > should return true or false. But since the collation information is
> > not
> > > associated with TypeId, we need to pass the TypeDescriptors of both
> > the
> > > types that need to be compared. In order to do this, I propose the
> > > comparable method to change as follows
> > >
> > > boolean    comparable(
> > >       DataTypeDescriptor leftType,
> > >       DataTypeDescriptor rightType,
> > >       boolean forEquals,
> > >       ClassFactory cf);
> > >
> > > This change would require that all the implementation of TC change and
> > all
> > > the callers of this method should change too.
> >
> > I spent some time looking at this approach and it seems pretty well
> > contained: a
> > search for "comparable" in the codeline shows that aside from the
> > TypeCompiler
> > classes themselves, only a few other files would be affected.  I tried
> > to think
> > some of alternative approaches but none seemed as contained as the one
> > you
> > suggest here.
> >
> > ----
> >
> > On a completely different note, while looking at the relevant classes I
> > noticed
> > that we instantiate the non-collator character types (ex. SQLChar) in
> > the
> > "getNull()" method of TypeId.  Do you have plans to update that piece of
> > code to
> > account for collator types?  Are any changes needed there?  I searched
> > the wiki
> > page for "TypeId" but nothing came up.
> >
> > And similarly, we instantiate the non-collator character types in the
> > XMLSerialize() method of iapi/types/XML.java.  While XML values
> > themselves are
> > not orderable/comparable with any other values (and thus collation
> > should not be
> > a problem), the result of an XMLSERIALIZE operator is a normal SQL
> > character
> > value, which can be compared.  So will the code in XML.XMLSerialize()
> > have to be
> > updated to account for collation?  My guess is that XMLSERIALIZE can be
> > characterized in the same way as the CHAR and VARCHAR functions (number
> > 6 on the
> > wiki page):
> >
> >   [XMLSERIALIZE] behavior can be defined as similar to CAST ie, the
> > result
> >   character string of [XMLSERIALIZE] will have the same collation as
> > current
> >   schema's character set. The collation derivation will be implicit.
> >
> > Does that sound right?
> >
> > Sorry if that's a tad off-topic...
> >
> > Army
> >
> >
>

Re: Changes to comparable method in TypeCompiler

[Mamta Satoor <ms...@gmail.com>]

Hi Army,

Thanks for the sanity check on what I am proposing for comparable method.

As for the changes required to instantiate the proper runtime class, ie
SQLChar vs CollatorSQLChar, I plan to address that as part of Outstanding
items section -> Language changes -> 3). I have not started looking at that
yet. What I am working on is loading the correct collation type for various
character types (Section Collation Determination) and disallowing comparison
between character types with different collations at compile time.

Thanks for bringing up XML.XMLSerialize(). I was not aware of it. I agree to
the approach of "XMLSERIALIZE can be characterized in the same way as the
CHAR and VARCHAR functions." I will update the wiki page for XMLSERIALIZE
to reflect that.

thanks a lot,
Mamta

On 4/11/07, Army <qo...@gmail.com> wrote:
>
> Mamta Satoor wrote:
> >
> > What I am proposing is to change the comparable method from type 1 to
> type
> > 2. The reason for this is comparable method for character types should
> also
> > check the collation type and derivation while deciding whether the
> method
> > should return true or false. But since the collation information is not
> > associated with TypeId, we need to pass the TypeDescriptors of both the
> > types that need to be compared. In order to do this, I propose the
> > comparable method to change as follows
> >
> > boolean    comparable(
> >       DataTypeDescriptor leftType,
> >       DataTypeDescriptor rightType,
> >       boolean forEquals,
> >       ClassFactory cf);
> >
> > This change would require that all the implementation of TC change and
> all
> > the callers of this method should change too.
>
> I spent some time looking at this approach and it seems pretty well
> contained: a
> search for "comparable" in the codeline shows that aside from the
> TypeCompiler
> classes themselves, only a few other files would be affected.  I tried to
> think
> some of alternative approaches but none seemed as contained as the one you
> suggest here.
>
> ----
>
> On a completely different note, while looking at the relevant classes I
> noticed
> that we instantiate the non-collator character types (ex. SQLChar) in the
> "getNull()" method of TypeId.  Do you have plans to update that piece of
> code to
> account for collator types?  Are any changes needed there?  I searched the
> wiki
> page for "TypeId" but nothing came up.
>
> And similarly, we instantiate the non-collator character types in the
> XMLSerialize() method of iapi/types/XML.java.  While XML values themselves
> are
> not orderable/comparable with any other values (and thus collation should
> not be
> a problem), the result of an XMLSERIALIZE operator is a normal SQL
> character
> value, which can be compared.  So will the code in XML.XMLSerialize() have
> to be
> updated to account for collation?  My guess is that XMLSERIALIZE can be
> characterized in the same way as the CHAR and VARCHAR functions (number 6
> on the
> wiki page):
>
>   [XMLSERIALIZE] behavior can be defined as similar to CAST ie, the result
>   character string of [XMLSERIALIZE] will have the same collation as
> current
>   schema's character set. The collation derivation will be implicit.
>
> Does that sound right?
>
> Sorry if that's a tad off-topic...
>
> Army
>
>

Re: Changes to comparable method in TypeCompiler

[Army <qo...@gmail.com>]

Mamta Satoor wrote:
> 
> What I am proposing is to change the comparable method from type 1 to type
> 2. The reason for this is comparable method for character types should also
> check the collation type and derivation while deciding whether the method
> should return true or false. But since the collation information is not
> associated with TypeId, we need to pass the TypeDescriptors of both the
> types that need to be compared. In order to do this, I propose the
> comparable method to change as follows
> 
> boolean    comparable(
>       DataTypeDescriptor leftType,
>       DataTypeDescriptor rightType,
>       boolean forEquals,
>       ClassFactory cf);
> 
> This change would require that all the implementation of TC change and all
> the callers of this method should change too. 

I spent some time looking at this approach and it seems pretty well contained: a 
search for "comparable" in the codeline shows that aside from the TypeCompiler 
classes themselves, only a few other files would be affected.  I tried to think 
some of alternative approaches but none seemed as contained as the one you 
suggest here.

----

On a completely different note, while looking at the relevant classes I noticed 
that we instantiate the non-collator character types (ex. SQLChar) in the 
"getNull()" method of TypeId.  Do you have plans to update that piece of code to 
account for collator types?  Are any changes needed there?  I searched the wiki 
page for "TypeId" but nothing came up.

And similarly, we instantiate the non-collator character types in the 
XMLSerialize() method of iapi/types/XML.java.  While XML values themselves are 
not orderable/comparable with any other values (and thus collation should not be 
a problem), the result of an XMLSERIALIZE operator is a normal SQL character 
value, which can be compared.  So will the code in XML.XMLSerialize() have to be 
updated to account for collation?  My guess is that XMLSERIALIZE can be 
characterized in the same way as the CHAR and VARCHAR functions (number 6 on the 
wiki page):

   [XMLSERIALIZE] behavior can be defined as similar to CAST ie, the result
   character string of [XMLSERIALIZE] will have the same collation as current
   schema's character set. The collation derivation will be implicit.

Does that sound right?

Sorry if that's a tad off-topic...

Army