Hi Ross, On Wed, Aug 26, 2009 at 02:31:53PM -0700, Ross McFarland wrote: > i'd heard about a new protocol, which i guess i assumed was part of a > (new)libdrizzle.
It will be, but I wrote the new libdrizzle to replace the old one using the same protocol for now. The new protocol will sit behind the same interface (much like the MySQL version does today). > > We decided providing > > both would be the most robust (and this is fairly consistent for > > all objects). > > this one can definitely be chocked up as a difference of opinion (that i don't > feel _that_ strongly about.) mainly i don't like the inconsistency of half of > the time returning ret-codes and the other half pointers and taking the > ret-codes as pass by pointer params. Yeah, I can understand. It's not perfect, but it was either this or more function calls to provide both APIs. I didn't really want to get into the business of passing double pointers around, this tends to scare more people. > having the library do the malloc for you doesn't really seem to buy much, at > least what it does buy you (removes a couple lines, malloc and error check) i > don't think is worth the loss of consistency in the api. It's not uncommon for libraries to do the memory management for you, and like I said, I wanted to make it easy for both types of users. Perhaps we should propose a shorter list of design questions to the mailing list to see what folks think, this would be one of them. > it's not just the things that aren't named for the object the really operate > on. > > there's a lot of calls/paths that just 'feel' a little funky. the results > objects in general fall in to this category. to me it really seems like > they're > almost a query object, but not quite. i'm not sure in what situation you need > a > result object that using a query object wouldn't be cleaner. This gets down to naming of objects and the lifetime of those objects. To me a result object was something that could last much longer than a query object. Otherwise you have unusable fields in a result object until the query is sent, or unusable fields in the query object until a result is starting to be read. It seems best to separate these objects out. It may also be the case that a single query can produce more than one result set (it can in MySQL, we've not added this back into Drizzle yet). SO having a 1-1 binding of query-result would prevent this. > other situations require calling a set of functions before an 'object' is > usable or at least allow you to create situations are that are invalid/errors. > creating connections is one of those (db should be a param to con_create since > you'll always have to set_db before you connect) I wanted to keep all _create and _destroy functions consistent taking only the "this". I didn't want to introduce extra parameters here. Also, it is possible to connect without any DB context, you can just set it later in the connection or possibly reference tables via database names as well (not sure if this is 100% supported in the server still, but thats the plan). > all of the multiple query stuff, which i've only glanced at, seems to have a > lot > of potential for mistake and just generally (to me again) feel funky. > > DRIZZLE_API > drizzle_query_st *drizzle_query_add(drizzle_st *drizzle, > drizzle_query_st *query, > drizzle_con_st *con, > drizzle_result_st *result, > const char *query_string, size_t size, > drizzle_query_options_t options, > const void *data); > > looking at that i have no clue what this function does. it takes a lot of > things > and returns a query object. is that object a single query? is it a set of > queries/what's the query added to? there's both a drizzle object and a con > object going in, what if a drizzle object that isn't associated with the con > object is passed? ... why is the drizzle object the first parameter, seems > like > if this is a query function and i'm passing in a query object it should be the > first param, essentially self/this. > > these aren't un-grok-able problems or anything, i'm sure if you explained it > to > me i would be able to make sense of and use it all. Agreed, this is not the most inutitive, and like I said before, the biggest thing missing here is documentation. The concurrent query interface was something I added as a "prototype" and still needs some refining (broader use of query objects for example). I can get into more specific answer here, but the idea is that you can add this in the context of a drizzle object with con being optional (in which case it would choose a con from the pool of available cons). One of my todos has been to refine this and clean it up. Also, right now the concurrent query interface only supports fully buffered results, which I still need to fix. There is a lot to do in query.c, but the basics are there. > i don't think libdrizzle is wrong/broken, but do feel like it's > non-obvious/sub-optimal/sometimes just doesn't feel right. i can't sit down > and > list all of the things i think can be improved and frankly wouldn't have the > energy to spell-out/fight for all of them. i'd love to work towards addressing > them, and it is with that in mind that i started working on ldrizzle-proto. > that > said others may not see problems and/or it may be too late to make those kinds > of changes to the libdrizzle api and if that's the case i'll let it go. This was a bit of a surprise because you've been the first person to mention these things (which is great feedback). Others who have used it or wrapped it have not mentioned many issues or difficulties getting up to speed with it. > there aren't any that are missing, that i know of, but calling them causes > problems. if you run test in ldrizzle-proto as-is with valgrind > --leak-check=full --show-reachable=yes --leak-resolution=high you'll see 1 > access error and a couple things being leaked. > > for example if you uncomment the drizzle_free call at the end of drizzle.c > (in my drizzle_destroy(Drizzle * drizzle) call) you'll get segfaults that > seem to come from drizzle_free calling drizzle_con_free, which calls > drizzle_result_free which blows up b/c all of my objects are stack > allocated. i thought the DRIZZLE_ALLOCATED/DRIZZLE_AUTO_ALLOCATED options > might > have something to do with this, but setting/unsetting them didn't seem to > change > anything. Generally, you should not be touching DRIZZLE_ALLOCATED/DRIZZLE_AUTO_ALLOCATED. They are available for advanced use cases, but most folks don't need them. If you are getting access violations, it is probably because of proper allocation order like you mention, or possibly double freeing of objects. For example, if you free a result object that had buffered rows in it, all those rows are automatically freed as well, you don't need to do so manually. Same with connection objects added to a drizzle object, you can free a con object before you free the drizzle object, but never the other way around (drizzle_destroy will free all connections added to it). I always check for valgrind issues before releases and are not seeing many. For example, run your queries through examples/client with valgrind, it should be free (if not please let me know). > so this seems to be the biggest problem/misunderstanding i've had. if > drizzle_con_query isn't blocking (even without NON_BLOCKING) then that gets > the behavior i'd imagine most cases want/need. drizzle_con_query will block, as will all potentially blocking functions would, without NON_BLOCKING. Not sure what you mean here. > the memset isn't necessary, just was easier than listing out/initializing all > of > the fields individually. if the desired initial state is NULLs for all > pointers > and 0's for ints etc. then memset effectively is the same as listing out all > of > the structs fields and setting them to NULL/0/etc. which shouldn't confuse > valgrind. i never heard anything about memset being less effecient than > setting > each of the fields individually. i would of guessed that memset would be a > highly optimzied operation. regardless wasn't an important point... Yes, memset usually has the same effect, but if your objects contain fields or buffers that do not need to be zeroed out, it is less efficient. For example, take a connection object that has a 8k buffer in it. It is more efficient to set the individual fields than to memset the entire thing due to the extra memory writes of the buffer. Also, when you memset the entire buffer, you loose the possibility of valgrind finding invalid read issues for you. > thanks for taking the time to look my stuff over and respond. i look > forward to continuing the discussion. Same here! I'm very interested in working out these details to have a simple yet powerful interface (as best we can, the two do not always go hand-in-hand). I did a majority of this work on my own and only a couple people have really read through and critiqued it, so I'm very grateful for this type of feedback. I'm going to be digging back into this type of design code in libdrizzle soon, and we're going to have one more big backwards incompatible release soon with the new protocol, so now is the time to refine the API. Thanks! -Eric _______________________________________________ Mailing list: https://launchpad.net/~drizzle-discuss Post to : [email protected] Unsubscribe : https://launchpad.net/~drizzle-discuss More help : https://help.launchpad.net/ListHelp
