[mapguide-internals] Code Reveiw request: Remove IsNull check when get property from feature source

[Trevor Wekel]

I agree with Traian.  Faster IsNull processing should be pushed down to FDO.  I assume there is a ReadNext() call made to Fdo before these methods are called.  Perhaps the entire record including "null" state for each property could be cached during the ReadNext() call?  Nulls  are fairly common, especially in databases, so this exception will get hit regularly.

The other issue is memory leaks in FDO and MapGuide.  Since nulls are common, the stack unwind should be checked for all providers and all "Get" calls to ensure that no memory leaks will occur.

Regards,
Trevor

-----Original Message-----
From: mapguide-internals-bounces at lists.osgeo.org [mailto:mapguide-internals-bounces at lists.osgeo.org] On Behalf Of Martin Morrison
Sent: August 10, 2011 5:57 AM
To: MapGuide Internals Mail List
Subject: RE: [mapguide-internals] Code Reveiw request: Remove IsNull check when get property from feature source

The only gotcha is NULL is a valid value.  MapGuide/FDO should handle it better.

Martin Morrison
Application Engineer
Engineering Design Systems, Inc.
3780 Peters Creek Rd Ext SW
Roanoke, VA  24018
540.345.1410
gis.edsi.com


-----Original Message-----
From: mapguide-internals-bounces at lists.osgeo.org [mailto:mapguide-internals-bounces at lists.osgeo.org] On Behalf Of Zac Spitzer
Sent: Wednesday, August 10, 2011 4:12 AM
To: MapGuide Internals Mail List
Subject: Re: [mapguide-internals] Code Reveiw request: Remove IsNull check when get property from feature source

as most, if not all have databases have a not null flag per column?

that metadata could help with handling nullable columns?

it does get tricky with the different handling of empty strings

On Wed, Aug 10, 2011 at 4:43 PM, Traian Stanev <traian.stanev at autodesk.com> wrote:
>
> The initial implementation for the rendering code indeed used optimistic null handling via try-catch instead of checking for IsNull, for performance reasons. For most data sets, not checking for null is a lot faster, because in FDO checking for IsNull gets the property value a second time. However, there were some sample data sets with lots of null values, which incurred a big overhead because of the number of catches that happened -- so someone added the IsNull. For such data, having the IsNull was a lot faster due to the inherent slowness of catching.
>
> The facts have not changed since then -- removing the null check will make things faster for data sets with no null values but slower for data sets with lots of null values.
>
> Really, MapGuide should not have to work around FDO being slow. There must be some way to optimize FDO providers where this is slow to cache the value after checking it for null and then quickly return it knowing that the next property value requested will likely be for the property that was previously checked for null.
>
> Traian
>
>
>
>
>
> -----Original Message-----
> From: mapguide-internals-bounces at lists.osgeo.org 
> [mailto:mapguide-internals-bounces at lists.osgeo.org] On Behalf Of Zac 
> Spitzer
> Sent: Wednesday, August 10, 2011 12:36 AM
> To: MapGuide Internals Mail List
> Subject: Re: [mapguide-internals] Code Reveiw request: Remove IsNull 
> check when get property from feature source
>
> that old thread was titled [mapguide-internals] Exception thrown on null values?
>
> here is some of it, dates back to 08, which was pre-nabble days?
>
> http://osgeo-org.1803224.n2.nabble.com/RE-mapguide-internals-Exception
> -thrown-on-null-values-td2051195.html
>
> from my *limited* understanding, this patch catches any exceptions and 
> then gracefully handles them which should be a big win
>
> z
>
>
> On Wed, Aug 10, 2011 at 2:28 PM, Jason Birch <jason at jasonbirch.com> wrote:
>> Wasn't the isnull check initially put in place to deal with stability 
>> issues (something to do with FDO exceptions not unwinding properly or
>> something?)
>>  It was a long time ago, but it sounds pretty familiar.
>>
>> On 1 August 2011 01:15, Buddy Hu wrote:
>>
>>> Hi Lists,
>>>
>>> Please add your comments for this patch:
>>> http://trac.osgeo.org/mapguide/ticket/1766
>>>
>>> Regards,
>>> Buddy
>>> _______________________________________________
>>> mapguide-internals mailing list
>>> mapguide-internals at lists.osgeo.org
>>> http://lists.osgeo.org/mailman/listinfo/mapguide-internals
>>>
>> _______________________________________________
>> mapguide-internals mailing list
>> mapguide-internals at lists.osgeo.org
>> http://lists.osgeo.org/mailman/listinfo/mapguide-internals
>>
>
>
>
> --
> Zac Spitzer
> Solution Architect / Director
> Ennoble Consultancy Australia
> http://www.ennoble.com.au
> http://zacster.blogspot.com
> +61 405 847 168
> _______________________________________________
> mapguide-internals mailing list
> mapguide-internals at lists.osgeo.org
> http://lists.osgeo.org/mailman/listinfo/mapguide-internals
>
> _______________________________________________
> mapguide-internals mailing list
> mapguide-internals at lists.osgeo.org
> http://lists.osgeo.org/mailman/listinfo/mapguide-internals
>
>



--
Zac Spitzer
Solution Architect / Director
Ennoble Consultancy Australia
http://www.ennoble.com.au
http://zacster.blogspot.com
+61 405 847 168
_______________________________________________
mapguide-internals mailing list
mapguide-internals at lists.osgeo.org
http://lists.osgeo.org/mailman/listinfo/mapguide-internals