Hey John, Sorry i misread it. I was convinced that it was an empty string instead of a cast... My bad... So right now, everything is working fine, i don't have errors anymore.
About the quoting: 1. Right now (r501) we have: abc -> "\"abc\"" <empty> -> "" null -> "" 2. With r500, we had abc -> "abc" <empty> -> "" null -> "" 3. Initialy (not sure when it changed), we had: abc -> "\"abc\"" <empty> -> "<NULL>" null -> "<NULL>" Not sure which one is best, though, depending how backward compat is important and what make most sense. My code support all of them. What is your opinion on that ? Next week, i'll try to restore my performance patch and hopefuly get a stable version that i'll be able to use for some time. Thanks, ________________________________________ From: K. John Wu [[email protected]] Sent: March-24-12 10:40 PM To: Dominique Prunier Cc: FastBit Users Subject: Re: [FastBit-users] Changes in src/colValues.cpp@495 breaks ibis::bundle::getUInt for CATEGORY Hi, Dominique, Looks like I have got ride of the check for *str != 0 already. Regarding function fastbit_result_set_get_string, a 'return 0' is nearly the same as 'return static_cast<const char*>(0)' - this is something silly about exactly how the 0 in 'return 0' is cast into 'const char*' specified as the return type. My suggestion is to distinguish invalid string (i.e., char* str=0) and an empty string (i.e., char* str=""). In the first case we have 'str == 0', in the second case we have 'str != 0' and '*str == 0'. In C/C++, this is not too hard to do. I think there is a way to assign a NULL object as well. Maybe you can try to use the NULL object to mirror the nil pointer in C. John On 3/24/12 3:58 PM, Dominique Prunier wrote: > Hey John, > > I perfectly agree that the best thing for colUInts::write is to accurately > return the actual values that came in. However, I see that you restored the > quoting for non null strings: > - if (str != 0 && *str != 0) > - out << str; > + if (str != 0) > + out << '"' << str << '"'; > Both (quoted or unquoted) works for me since my code support both, it is up > to you but if we want to return the exact value, we'd have to remove them. > > About the C API, my problem is that the function fastbit_result_set_getString > uses an empty string as the mark of some errors. So when you call > fastbit_result_set_getString, there is no way to find out if it failed or if > you got a NULL: > if (rset == 0) > return static_cast<const char*>(0); > if (pos >= rset->strbuf.size()) > return static_cast<const char*>(0); > try { > std::string tmp = rset->results->getString(pos); > rset->strbuf[pos].swap(tmp); > return rset->strbuf[pos].c_str(); > } > Maybe that should be replaced with: > if (rset == 0) > return 0; > if (pos >= rset->strbuf.size()) > return 0; > try { > std::string tmp = rset->results->getString(pos); > rset->strbuf[pos].swap(tmp); > return rset->strbuf[pos].c_str(); > } > > That being said, if i change my code to accept empty string, it seems to work > fine. > > Thanks, > > -----Original Message----- > From: K. John Wu [mailto:[email protected]] > Sent: Saturday, March 24, 2012 3:46 PM > To: Dominique Prunier > Cc: FastBit Users > Subject: Re: [FastBit-users] Changes in src/colValues.cpp@495 breaks > ibis::bundle::getUInt for CATEGORY > > Just checked in SVN R501. The details of the changes are given below. > > John > > > On 3/23/12 2:47 PM, Dominique Prunier wrote: >> Hey John, >> >> I'm testing r500 right now and i have a few issues: >> >> 1. In category::stringSearch: >> else if (ind == 1 && dic.size() == 1) { // special case with one value >> getNullMask(hits); // all valid entries >> } >> We can't do this shortcut, this is not nulls that make an error but also >> empty strings (which is what i have in my test case). When i remove these 3 >> lines, it works again > > Your are absolutely right. In fact both special cases need to be > removed. This has been done now. > >> >> 2. Most of my select distinct (using bundles) that returns empty strings >> (key 0) on categories were broken. I think this is related to the fact that >> the bundle used to write "<NULL>" for key 0 instead of "". Unfortunately, in >> the C API (fastbit_result_set_getString), the empty string is the mark of an >> error, so there is no way to distinguish them anymore. In this particular >> case, this is probably the CAPI which is wrong, maybe we should return a >> null pointer on error instead of an empty string. If i restore the else in >> colUInts::write (else out << "<NULL>";), my queries works again > > Printing out an empty string as "<NULL>" was a total random action on > my part. One rationale thing is to do would be give back exactly what > was on input, i.e., "". I am giving this a try now. Please let me > know how it goes for you. > >> >> 3. This is not really an issue for me, but it seems also that the quoting of >> string in colUInts::write got removed. I'm perfectly fine with that, i used >> to try to strip quotes. It actually happened at r468 but my code was >> checking if the string was quoted or not before trying to remove them, so it >> didn't break anything. >> > The problem is somewhat unclear to me. The C API is using a string > object to hold the strings. An empty string should return a valid > pointer const char*. Now that the empty strings are actually printed > as "", we should not have a problem with empty strings in the > std::string objects. Hopefully the problem will go away. If it > persists, I will need some details.. _______________________________________________ FastBit-users mailing list [email protected] https://hpcrdm.lbl.gov/cgi-bin/mailman/listinfo/fastbit-users
