Bug 130195

Created attachment 16873 [details]
Testcase for bug #130195

Data file to illustrate the problem. You can load all three "VAR" vectors in
the datawizard and look at the result. Not quite what one would expect :-(

This bug is per-datasource plugin and really nothing to do with Kst proper.  I'm tempted to close and let individual bugs be filed against each source that creates inaccessible fields.

It's true that this is per-datasource. The only thing I see that kst could do is check for duplicate entries the first time it gets the list, and warn the user that some fields will not be accessible, with a hint of which datasource is being used and how to create the report...

SVN commit 662309 by arwalker:

BUG:130195 warn the user if there are duplicate field names in their data source

 M  +18 -0     kstdatasource.cpp  


--- branches/work/kst/1.5/kst/src/libkst/kstdatasource.cpp #662308:662309
@@ -334,6 +334,24 @@
     QString typeSuggestion;
     rc = (*i).plugin->fieldList(kConfigObject, fn, QString::null, &typeSuggestion, complete);
     if (!rc.isEmpty()) {
+      //
+      // check for duplicate field names and warn the user if necessary...
+      //
+      QStringList::const_iterator it = rc.begin();
+      QString str;
+
+      for (; it != rc.end(); ) {
+        str = (*it);
+        ++it;
+        if (it != rc.end()) {
+          if (rc.find(it, str) != rc.end()) {
+            KstDebug::self()->log(i18n( "The datasource has at least one duplicate field name; '%1'. As a result one or more fields will not be accessible." ).arg(str), KstDebug::Error);
+
+            break;
+          }
+        }
+      }
+
       if (outType) {
         if (typeSuggestion.isEmpty()) {
           *outType = (*i).plugin->provides()[0];

Right, this never really got fixed... I think it's sane to do something about it, I'm just a bit worried about the performance penalty here. 
All these calls to rc.find() might turn out to become a problem when there are many variables in the file. I have colleagues who routinely use data files with 10 000 variables ! I haven't had a chance to test this yet, but isn't there a quicker way ? QStringList sorting being O(n*log n), maybe sorting a copy of the list, then looking only at consecutive entries in the sorted list would be faster overall ? Note that I don't know how fast/slow find() is, though...

SVN commit 662958 by arwalker:

CCBUG:130195 Make search for duplicate field names more efficient

 M  +19 -21    kstdatasource.cpp  


--- branches/work/kst/1.5/kst/src/libkst/kstdatasource.cpp #662957:662958
@@ -330,31 +330,13 @@
 
   QValueList<PluginSortContainer> bestPlugins = bestPluginsForSource(fn, type);
   QStringList rc;
-  for (QValueList<PluginSortContainer>::Iterator i = bestPlugins.begin(); i != bestPlugins.end(); ++i) {
+  for (QValueList<PluginSortContainer>::Iterator it = bestPlugins.begin(); it != bestPlugins.end(); ++it) {
     QString typeSuggestion;
-    rc = (*i).plugin->fieldList(kConfigObject, fn, QString::null, &typeSuggestion, complete);
+    rc = (*it).plugin->fieldList(kConfigObject, fn, QString::null, &typeSuggestion, complete);
     if (!rc.isEmpty()) {
-      //
-      // check for duplicate field names and warn the user if necessary...
-      //
-      QStringList::const_iterator it = rc.begin();
-      QString str;
-
-      for (; it != rc.end(); ) {
-        str = (*it);
-        ++it;
-        if (it != rc.end()) {
-          if (rc.find(it, str) != rc.end()) {
-            KstDebug::self()->log(i18n( "The datasource has at least one duplicate field name; '%1'. As a result one or more fields will not be accessible." ).arg(str), KstDebug::Error);
-
-            break;
-          }
-        }
-      }
-
       if (outType) {
         if (typeSuggestion.isEmpty()) {
-          *outType = (*i).plugin->provides()[0];
+          *outType = (*it).plugin->provides()[0];
         } else {
           *outType = typeSuggestion;
         }
@@ -363,6 +345,22 @@
     }
   }
 
+  if (!rc.isEmpty()) {
+    //
+    // check for duplicate field names and warn the user if necessary...
+    //
+    QMap<QString, QString> map;
+
+    for (QStringList::const_iterator it = rc.begin(); it != rc.end(); ++it) {
+      map.insert(*it, *it);
+    }
+
+    if (map.size() != rc.size()) {
+      KstDebug::self()->log( i18n("The datasource '%1' has at least one duplicate field name. As a result one or more fields will not be accessible.").arg(filename), KstDebug::Error);
+    }
+  }
+
+
   return rc;
 }