ThinkGeo.com    |     Documentation    |     Premium Support

Breaking change in shape file feature ColumnValues?

I've been working with Ben Bai on the OOM exception described elsewhere in this forum.  As part of that process, Ben has provided some custom build binaries that dump resources used internally by the TG Map component when the OOM occurs, and using this we believe there is a solution that has been integrated with your code base.  Ben provided a final diagnostic dump build to try on our test sites to try to confirm the fix, or get another dump should it repro again (sometimes takes weeks).  He also indicated that the prod build wasn't updating correctly, so he sent me an update of the prod binaries so that we can begin acceptance testing in parallel while confirming the fix.  Both of these are to my knowledge based on your current code base.  And both of these last 2 builds exhibit the problem I will describe below.  However, I can't give you a version number because your current Prod Build 9.0.0.142 still contains binaries built on 11/11/15, long before the changes Ben provided for our fix.  I reported this first to him, but he is working on another project now and suggested I post here, so I'm including all this for your background.  If you need more specifics about the changes and code base used for those builds, you will need to contact Ben directly.



Also, I really need an official Prod build containing the changes prompted by the OOM investigation.  



This is the content describing our current issue...



The last 2 ThinkGeo Map builds (built early December and 12/8) appear to have breaking changes regarding feature access in static layers. The following code works on older builds, but fails on the last update you gave me for the diagnostic dumps, and on the prod build you sent me earlier today. In the code below, the layer's shape file does have the indicated "field" (column name) in it's DB file along with associated values. The call to layer.FeatureSource.GetAllFeatures for that columnName returns 369 features. But then the subsequent call to get the column value using feature.ColumnValues[columnName] throws KeyNotFoundException. And in fact, when I look at column values, the count is (always) zero for all 369 features, even though I know for a fact that there should be values for that column. And of course there is also the fact that this code worked on earlier versions of DesktopEdition.  With the same shape files and the same code, the previous drop you (Ben) gave me (that generated the OOM dump files I sent you last) worked without issue.


layer.Open( );

Collection<feature> features = layer.FeatureSource.GetAllFeatures( new[] {columnName} );

foreach( Feature feature in features )

{

cachedValueStyle.ValuesCache.Add( feature.Id, feature.ColumnValues[columnName] );

}

layer.Close( );</feature>



Hi Russ, 
  
 I have check API change log between 9.0.0.142 and 9.0.0.168, there aren’t any breaking changes in both of versions, I make a simple sample to test your statements, the issue can’t be reproduced, I guess the issue is caused by your data file. Can you send a simple sample to me so that I can reproduce it. 
  
 Thanks, 


I tried to put together a simple repro this morning and failed.  I then went back to our product code and looked for anything relevant that might be going on to affect it.  I either duplicated in the repro or eliminated everything I could that preceded that attempt to access feature values without completely rewriting everything.  This happens VERY early, so there is really not that much.  Configuring some other shape files and their drawing styles, setting zoom levels, that’s about it.   
  
 So at this point I’m not sure what drives this exception, but as far as I can tell it is not our code or the file itself.  I can “fix” the exception by simply reverting to 9.0.0.66.  I don’t know what to say except please look again.  When I do nothing but change a reference from your build made a few months back to the latest build and that alone introduces an exception, the most likely culprit by far is the referenced binaries.  I wish I could figure out how to give you a reduced repro, but at this moment I cannot.

Hi Russ,



The issue has been fixed in version 9.0.0.168. Please click here to download.



Any questions please let us know.



Thanks,
Peter

In order to adopt the change and put it through our acceptance testing, which is the whole goal that started all this, I need it as an official Prod build from you, not a private dev build.  Looks like your published Dev build is 9.0.170.0 (which would seem to contain the fix?) at the moment, but the published Prod build is only 9.0.0.142.  When will it be available as an official (presumably well tested?) Prod build?

It looks like Ben (and others?) attempted to update the Product Center Full Production build.  However, that update appears to be missing this Shape file regression bug fix talked about on this thread.  And I was under the impression that the Full Production Build was a tested build that could reasonably be expected to work, where as the daily builds have incremental update and fixes for topics in this forum or whatever.  At some point, all that gets rolled up and tested (one would hope?) before making a new production build available.   
  
 In any case, Ben sent me an email indicating that I could pick up the latest 174 production build, which theoretically should have been fully tested, have the fix for the OOM he was working on, AND have the shape file regression fixed.  I have no way to confirm the OOM fix present or not, but I can confirm that the shape regression fix is not in that build.  Which also brings into question my seemingly invalid assumption about the tested state of production builds. 
  
 So here is my main point.  We HAVE to have a solid tested functional official build of your Desktop Edition VERY soon.  We’ve got a long delayed release (mostly due to issues with your component) that finally went out the door only to discover an intermittent OOM exception in your component.  Now we’ve had to start an unplanned short term “dot release” update to address that problem for our customers.  But now, because of the shape file regression, I can’t even produce a build with Ben’s OOM fix (and retained dump implementation should the fix not work) for testing out our problematic sites.  Because there is no direct repro, and it takes weeks to manifest, we need to run that code at the most problematic sites for weeks without repro to have any confidence that the OOM is fixed.  AND because of that delay, we want to start acceptance testing of the dot release update so that IF the OOM fix is solid, we will be ready for a general release to all customers ASAP.  But I can’t get EITHER build done because it seems impossible to get a drop from TG that works and contains both fixes.   
  
 PLEASE address this right away.  To recap: 
  
 1) I need a solid working build complete with OOM fix, shape file regression fix, AND including Ben’s dump code “just in case”.  This is to test at problem sites to confirm that the OOM no longer reproduces.   
  
 2) I need a solid test *production* build that works and contains the OOM fix, and the shape file regression fix.  This is absolutely essential for us to start our side of the process (basically ~4 weeks) toward getting a fix for our customers already experiencing crashes due to your OOM exceptions.   


Hi Russ,



I did test by referencing version 9.0.0.174 it works fine. Here is the screenshot on my side.



Any questions please let me know.



Thanks,

Peter

From your post, I thought you understood the problem earlier?  And I’m not surprised to see the code you wrote working, I wrote about the same and saw it work, as I explained earlier when I stated that my attempt at a reduced repro worked.  But for some unknown reason, in the real code, it did not.  I thought it must have something to do with some other unknown and not obvious side effect, and though I looked and experimented for what it might be, I could not find why my real code worked on 66, and fails with the same problem on ALL subsequent builds we’ve tried.   
  
 But then I was playing around with it after seeing your response, and thought about testing it with ReturningColumnsType.AllColumns.  All the expected columns were there!  And I could see the one requested in the debugger, but when requested by column name, returned it the features but NO column values.   
  
 Then it dawned on me, I could see the problem right there in the debugger.  The column name casing was different!  The older version of your code matched case-insensitive, the newer versions are case sensitive.  So I tested by changing the case of the column name right there in the debugger and then re-running the same code, and it worked!   
  
 So just as I originally posted, this is a *breaking change*.  A very old piece of code in our product, one dating all the way back to our pre-rewrite implementation (using your old 2.0 map), up-cased ALL these field names before serializing.  I’m guessing to make subsequent comparisons simpler?  So all our field names are uppercase, and have been for years, and this change is going to break every customer we’ve got that uses the feature in question. 
  
 So, first of all, as I’ve pleaded for before, we as your customers REALLY need a comprehensive list of your breaking changes when you make them.  Every single update we’ve tried to take from you has had some sort of undocumented breaking change.  And that means I spend hours trying to figure it out, and often wind up resorting to posts here, days of waiting and review, eventually to find a solution.  Events that no longer fire, changes in behavior, all of these things need to be disclosed for a customer to evaluate the impact of applying an update.  As it is, I’m always very reluctant to pick up updated builds from ThinkGeo. 
  
 Second, I would really like for you to roll the change back.  I suspect the original implementation was based on a SQL type semantics where case sensitivity is not even supported.  Something that changed in your code introduced the case sensitive requirement.  If there is some reason you can’t roll back this change, perhaps some unknown benefit that drove it, then perhaps a case-insensitive overload could be added.  Otherwise I will have to write some sort of fix-up code that goes through all the possible column values and do a case-insensitive map back to update our serialized column names with case-sensitive options.

Hi Russ, 
  
 Sorry for misunderstanding the problem before and thanks for you more information about the breaking change. The change has been rolled back and it will be available in 9.0.0.175 or later which you can get from Product Center. 
  
 Thanks, 
 Peter

Thanks, but all I see is build 174 on the product center.

Ok, someone updated to 175 now.  To whoever, thank you.

Hi Russ, 
  
 Any questions please let us know. 
  
 Thanks, 
 Peter

It was just brought to my attention that I never posted where this fixed it or not.  I had meant to include that along with saying that 175 had appeared and that I had downloaded it.  In any case, it appears to have fixed the regression.

Hi Russ, 
  
 Sorry for the inconvenience before. Any questions please let us know. 
  
 Thanks, 
 Peter