Forum OpenACS Development: New Tcl API proc "package_exec_plsql" ...
The immediate benefit is that PL/[pg]SQL calls, which are one of the major sources of the need to write separate code for both RDBMSs, can be replaced by a simple Tcl call.
set var_list [list \ [list foo $foo_value] \ [list bar $bar_value]] set result [package_exec_plsql -var_list $varlist package func]
I like the idea of doing this, am not entirely thrilled about tying it into the package-proc code but
- That code exists already
- Our PL/SQL funcs and procs in Oracle are always buried in packages
- It requires the same function param definition hack as package_instantiate_object does in PG *but* this gives us the same default param value semantics as we have in Oracle Currently this is on HEAD and I'm using it on my portal+openacs integration effort, and am interested in comments. I'm thinking maybe we should TIP that package objects should be instantiated using package_instantiate_object (many people do this already) and rather than call db_exec_plsql directly we should use package_exec_plsql or at least something very much like it.
This makes it much easier to work with both databases. Most of the database-specific queries are of the pl/sql pgplsql variety.
One difference I see is that package_exec_plsql accepts a list of lists, which package_instantiate_object accepts an ns_set of parameters.
I think the list format is easier to work with.
And thanks to your posting I now know that 'package' stands for pl/sql packages in this case - I always thought about openacs packages and was confused. Wouldn't it be less confusing if it was just part of db-procs.tcl and called something like db_plsql (since db_exec_plsql is already taken)?
db_* should remain a low-level interface to the database driver, I think. This is not a low-level interface and in the PG case depends on explicit metadata (including the defining of default param values) that aren't part of PG.
This package procs are nice. Lars pointed me to it.
Regarding the ns_sets and lists. Maybe Ben came to the same situation as me. Initially started using list of lists for bcms when passing stuff from proc to proc. It was nice and easy at first. Then as time went by, I ended up making code to parse the list of lists. Me and Deds talked a bit, he used ns sets. I then started to experiment with list of ns sets. It turned out to be better, but it was cludgy at start. Maybe because I wasn't used to ns sets.
Anyway I am not sure if my experience does apply... But hey Happy New Year! :)
- You're using an AOLserver API which uses ns_set.
- Maybe because you need to allow multiple key/value pairs all with the same key. ns_set supports this directly, Tcl arrays do not.
On the other hand, the ns_set C code is very simple and thus might be easier than Tcl arrays to embed in other C code.
For use from C code, using ns_set could also be easier than Tcl arrays in some cases, as ns_set has an explicit and complete API in both Tcl and C. Tcl arrays, on the other hand, don't really have any complete C API. Some Tcl array operations are really easy from C, but for others you'll need to code up your own (relatively simple) helper functions. E.g., I needed to write one for "array names", basically it's just a wrapper around Tcl_ArrayObjCmd. (Yes you can just call Tcl_Eval from C to execute the "array names" Tcl command directly, but this is a Bad Idea if you're doing this from within an inner loop; the performance hit can be very large.)
I'd say always use a Tcl array (or nsv when appropriate) by default, never use an ns_set unless you have a very specific and concrete reason why you should do so.
Actually, ns_set looks like a legacy data structure to me, I'd never choose to use it for anything new. Even if I really needed a set-like data structure that supports duplicate keys, I probably wouldn't use ns_set, I'd code my own using Tcl Arrays (and maybe Tcl lists too) underneath.
Jun, it would be very easy to write a generalized put/get proc to mimic the ns_set operations on a list of lists. After all an ns_set is just a list of lists! :)
I've been working on the partially refactored portals package (the one now in contrib). One bug I had to fix to get it close to working was to remove caching of ns_sets because an ns_set is a pointer to a local data structure ... can't cache that handle and see the local data structure in another thread that attempts to use the cached value rather than reconstruct it from the database. Oops ... one more nail in ns_set's coffin IMO.
ns_set exists because it allows ordered access, named access and multiple values for the same key, all of these are needed when working with headers, config file structures, and sql rows.
Nice API addition, thanks Don!
I didn't know that ns_sets was going to be nailed. Hehehehe. It was there the db_ proc was there... also it made things simpler for me. No more do it your own parsing code, etc. I made one initially, then I was pointed that ns sets may likely be the answer.
Anyway I will just wait to a new proc.
I think Jeff Davis said he once looked into the overheard due to using ns_sets for all the database stuff, and it was noticeable but not all that large. Maybe he said 10% or 20% or something like that, I really don't remember.
But in an unrelated project of my own I once made the mistake of using ns_sets with thousands of keys (or more) - this got quite slow. That's what originally made me pay attention to the fact that ns_set is often O(n) with set size, think about the fact that it's a legacy data structure, etc. (In that project, I didn't actually need any of the order preservation or multi-key properties of ns_set, so I just changed it to use Tcl arrays.)
My guess though is that any time you have a list of ns_sets, you probably don't have that many keys in each ns_set. And you probably don't have ns_set operations in an inner loop either like I did. So performance probably won't be a problem, you just get to deal with the possibly annoying programming aspects Don was talking about above.
ns_set seems fine to me for what it's intended for, just keep in mind that its O(n) performance means that ns_set is not suitable as a general purpose data structure.
On the other hand, being a brand new API routine no code usees it, depends on it, etc so adding it won't break any of our code, not at all.
So it would be safe from that POV.
I'm open one way or the other ... it is a convenience for anyone planning to do custom development under 5.0 ...
I don't think you should do it. "Feature freeze" should mean "feature freeze". It's easy for someone doing custom development on 5.0 to add it.
And surely since it can't possibly break anything, this handy feature should at least go in for 5.0.1? And if 5.0.1 is ok why not for 5.0.0 right now, unless perhaps you want to wait for 5.0.1 to see that it gets extra testing? It doesn't need to wait for 5.1, however many unknown months that is away, does it?
And no I don't think it's particularly reasonable to expect users to be back-porting stuff from Head to 5.0 all the time. If you're going to recommend that as being easy (as opposed to "if you really need it you can try, but I think it'll be tough, that's why we haven't done it yet"), then why not just "back port" it sooner for everybody to use, like right now? If it's not good enough to go into the stable branch than presumably it's not good enough to recommend for people to back-port either, no?