sorry for the forward, apparently the gmail reply to all lab went away and the button i click without paying attention to what it says that used to be reply-all has switched back to plain reply.
-rm ---------- Forwarded message ---------- From: Ross McFarland <[email protected]> Date: Sun, Aug 30, 2009 at 5:33 PM Subject: Re: [Drizzle-discuss] (old)libdrizzle observations and (new)libdrizzle proposals To: Eric Day <[email protected]> On Sat, Aug 29, 2009 at 2:19 PM, Eric Day<[email protected]> wrote: > Hi Ross, > > On Sat, Aug 29, 2009 at 07:54:06AM -0700, Ross McFarland wrote: >> but it's not optional, at least with the current setup. if you try to connect >> without providing it you'll get an error. > > If you hit a case where you could not connect without a DB context, > this is a bug somewhere. I tend to follow the rule that all optional > parameters are not part of the "constructor", but this may be a case > where since it's so common we may want to add it and just allow NULL > for no DB. looks like i was mistaken, it's that you have to call set_db or else most any query you run will return a no database selected error. so while not a hard requirement it is a most of the time requirement. still prob shouldn't be required in the constructor since you may have cases where you want to connect as a user who doesn't have access to any databases on the system to do some work (maybe for monitoring/logging/statistics/whatever.) >> seems cleaner. at that point do queries happen on the drizzle object, which >> automatically selects a connection for you? > > The intent was to have a simple con object to run single queries > on. You would not be using the top level drizzle object for more > advanced uses like concurrent queries. This was just a shorthand for > a fairly common case, but I agree it breaks the OO nature. I'll be > removing this. will the library really be able to intelligently manage/use connections? i have a hard time imagining that it will be able to cleanly choose which connections to use for what on the behalf of a client application. and problems with this would be a nightmare to try and debug since they'd likely all be timing related. if you have access to do things directly on connections you could skip around it, but what cases would you really be able to use this? > If you really dig into how the other language bindings work, like PHP, > Python, ..., you'll notice some really common patterns. Sure, these > extra things did not need to be in libdrizzle, but they are there > because each of the language bindings would have needed to write > the same bit of code to manage this. There were enough use cases > (language bindings, advanced library usage, ...) where this made > sense to be an optional feature in the main library. i'm saying if you want to make it easy and as error-proof as possible to bind the ref-counting should happen in libdrizzle rather than being re-written in each and every binding (and non-trivial application.) if there are references floating around to each other inside of the library there's the potential for problems even in C clients. stack based objects wouldn't use the ref-counting since their lifetimes are controlled by stack frames. (if things reference each other in the library, which is likely the case, this could cause problems.) >> i don't really like the idea, but based on the current design of objects >> referring around to other object they need it seems like libdrizzle objects >> need >> reference counting. at least it would make more sense than forcing every >> binding >> (and non-trivial app) to keep track of them on their own, reinventing the >> wheel >> over and over. > > So, the reference counting is really already in there. This is the > set of object lists that exist in objects (ie, all connections for > a drizzle object, ...). The external reference tracking done by the > language APIs can do this using the (void *)/callbacks I mentioned > before, there is only so much libdrizzle can do to help out there, > and I believe this interface supplies all it can. If you have the > time and interest, I would really check out how things work in the > Drizzle PHP extension as far as references go and how they relate to > C objects. Some of this will make a lot more sense. it doesn't seem like it's ref-counting at the moment or else it shouldn't be double freeing the result object. > In any case, I completely agree this is not a common case and should > not be part of any default behaviors. I may even create a separate > section in the header files to put these more advanced functions in. i'm pretty sure there won't be any cases where the behavior is actually preferable. the language bindings are better off doing a create in the init and free in the destroy every time (ref-counting taken in to account.) i don't see how it's preferable to have libdrizzle keep track of some memory for you and do a recursive free when you'll have to keep track of when it's going to do it for you to avoid double freeing. seems much cleaner and less error prone to do what would be expected: you malloc you free... >> this is a pretty good example of why this process has been frustraiting to >> me. >> most of your answers tell me how to do whatever i'm talking about with the >> current design. i'm not claiming that these things can't be done, i'm saying >> that they're more non-obvious, error-prone, or complicated than they >> need to be. > > Agreed, and this is why I'm now convinced that the automatic memory > cleanup/free will be switched to optional, and off by default. Like > I said, the use case in my header was libdrizzle-managed heap memory, > not stack. this doesn't have anything to do with stack allocated objects. i went through and converted everything to drizzle allocated objects in my double-free test case and the exact same problem happens. you can't reuse a stack-based or libdrizzle allocated result b/c the connection will get two references to it in it's list and then try to free them both. moreover keeping list like this seems like it has the potential for huge problems in long running applications where you might use a connection for 100's or 1000's of queries and then build up a list of 100's of results to free. of course you could call free on them yourself, but this facility is designed so that you don't have to and if you're using language bindings that rely on it you have no control over this and will be stuck with the 'leaks' (think a web server with persistent connections...) >> if you force calling a free inbetween next calls (as your saying is probably >> really necessary) it's going to be more verbose, but then you can at least >> skip >> the init case. i don't really like it either, but in the case of actual oo >> languages init is just the constrctor. anyway, this is essentialy an iterator >> pattern so it probably should follow a iterator semantics to make use of >> people's familiarity with the pattern. > > I would argue the current iterator interface is simple for folks > to use in libdrizzle. For example, the init() does not return the > first element, it just inits it. I think you should be iterating in > the context of a higher level object (ie, result, not raw rows). For > example, your simple_port.c would change from: > > drizzle_row_initialize (&row, &query); > drizzle_row_dump (&row, ""); > drizzle_field_initialize (&field, &row); > drizzle_field_dump (&field, ""); > while (drizzle_field_next (&field) == DRIZZLE_RET_OK) > drizzle_field_dump (&field, ""); > drizzle_field_destroy (&field); > > while (drizzle_row_next2 (&row) == DRIZZLE_RET_OK) > { > drizzle_row_dump (&row, ""); > drizzle_field_initialize (&field, &row); > drizzle_field_dump (&field, ""); > while (drizzle_field_next (&field) == DRIZZLE_RET_OK) > drizzle_field_dump (&field, ""); > drizzle_field_destroy (&field); > } > > To: > > drizzle_result_initialize (&result, &query); > > /* drizzle_result_next_row calls drizzle_row_init(&row) */ > while (drizzle_result_next_row (&result, &row) == DRIZZLE_RET_OK) > { > drizzle_row_dump (&row, ""); > /* drizzle_row_field_next calls drizzle_field_init(&field) */ > while (drizzle_row_next_field (&row, &field) == DRIZZLE_RET_OK) > { > drizzle_field_dump (&field, ""); > drizzle_field_destroy (&field); > } > drizzle_row_destroy(&row); > } > > This seems a bit cleaner to me, but again, this may just be a style > difference. It's also much easier to iterate in the context of > non-blocking I/O, since you need to keep some state between calls. how does drizzle_result_next row know whether or not to call drizzle_row_init? it doesn't know if the row you pass in is a brand new object or whether it's been passed in before. and the above semantics would only work for stack allocated objects... >> the ideal case is that someone opens up the header(s) (and maybe an example >> or >> two.) then goes to town; never having to look at doc or ask a question on the >> list. i don't think anything about the problem libdrizzle is solving >> requires a >> libarary where this couldn't be the case, but a lot of the current design >> decisions will prevent it from being. (memory management, >> lifetime/association >> kknowledge, naming, what object functions are on, ...) > > Agreed. This is certainly what I'm striving for. cool. i realize this has shifted to me pointing out problems more than talking about solutions. i started out with ldrizzle-proto, which has all of my 'solutions' written out for this reason, but that quickly changed when i had to justify/discuss the differences. i'm not sure what the best path forward is, but i feel like there's to much going on in the current api to fix without reasonably drastic changes. -- -rm _______________________________________________ Mailing list: https://launchpad.net/~drizzle-discuss Post to : [email protected] Unsubscribe : https://launchpad.net/~drizzle-discuss More help : https://help.launchpad.net/ListHelp
