Forum OpenACS Development: New Tcl API proc "package_exec_plsql" ...

Collapse
1: New Tcl API proc "package_exec_plsql" ...
Posted by Don Baccus on 01/03/04 02:54 AM
Since we've had "package_instantiate_object" for ages, now, and since that proc needs to figure out how to call an object type's "new" function, I decided to add a vanilla PL/[pg]SQL-caller proc to package-procs.

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.

Just do: 


    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.
Collapse
2: Re: New Tcl API proc "package_exec_plsql" ... (response to 1)
Posted by Dave Bauer on 01/03/04 03:11 PM
Don, Thank you! I thought about this, but looking at package_instantiate_object as a model, I couldn't get through writing it myself.

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.

Collapse
3: Re: New Tcl API proc "package_exec_plsql" ... (response to 1)
Posted by Don Baccus on 01/03/04 05:45 PM
Actually package_instantiate_object accepts either the "var_list" list-of-lists or the "extra_args" ns_set (which Ben added).  I never talked to Ben as to why he thought the ns_set variant was easier to work with, I intend (as time permits) to  munge things to use the var-list approach as it really is easier to deal with and one can always lappend to it, can pass it around from proc to proc, etc.
Collapse
4: Re: New Tcl API proc "package_exec_plsql" ... (response to 3)
Posted by Tilmann Singer on 01/03/04 08:31 PM
Very cool - this will even make a lot of -postgresql.xql and -oracle.xql files obsolete.

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)?

Collapse
5: Re: New Tcl API proc "package_exec_plsql" ... (response to 1)
Posted by Don Baccus on 01/03/04 08:47 PM
Well I mentioned above that I didn't really like tying it into the acs-subsite package-proc code ... but, that's where all the underlying pieces it uses live, so for now at least it feels like the right place.

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.

Collapse
6: Re: New Tcl API proc "package_exec_plsql" ... (response to 1)
Posted by Jun Yamog on 01/04/04 01:16 PM
Hi,

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! :)

Collapse
7: Re: New Tcl API proc "package_exec_plsql" ... (response to 6)
Posted by Guan Yang on 01/04/04 01:35 PM
What is the advantage of ns_set over arrays and array get/set?
Collapse
8: Re: New Tcl API proc "package_exec_plsql" ... (response to 7)
Posted by Andrew Piskorski on 01/04/04 02:31 PM
Guan, AFAIK there is no advantage. There are only two good reasons to use an ns_set rather than a Tcl array:
  1. You're using an AOLserver API which uses ns_set.
  2. Maybe because you need to allow multiple key/value pairs all with the same key. ns_set supports this directly, Tcl arrays do not.
Many ns_set operations scale very poorly (O(n) with the number of items in the set), and the ns_set API is generally less powerful and more annoying to work with than the Tcl array API.

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.

Collapse
9: Re: New Tcl API proc "package_exec_plsql" ... (response to 1)
Posted by Don Baccus on 01/04/04 07:10 PM
Andrew, I think "legacy" nails it.  When AOLserver first arrived on the scene Tcl didn't have "array get" and "array set" operations and they were much, much less useful than they are today.

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.

Collapse
10: Re: New Tcl API proc "package_exec_plsql" ... (response to 1)
Posted by Tom Jackson on 01/04/04 08:41 PM

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!

Collapse
11: Re: New Tcl API proc "package_exec_plsql" ... (response to 1)
Posted by Jun Yamog on 01/07/04 10:56 AM
I used arrays for the get/set proc.  For the list stuff the normal stuff is multirow.  And the list of ns sets is the alternative result.

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.

Collapse
12: Re: New Tcl API proc "package_exec_plsql" ... (response to 1)
Posted by Andrew Piskorski on 01/07/04 12:01 PM
ns_set preserves key order, right, thanks Tom, I forgot that one.

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.

Collapse
13: Re: New Tcl API proc "package_exec_plsql" ... (response to 1)
Posted by Guan Yang on 01/07/04 08:01 PM
Will this be available in 5.0?
Collapse
14: Re: New Tcl API proc "package_exec_plsql" ... (response to 1)
Posted by Don Baccus on 01/07/04 11:19 PM
We'll we've been in a "feature freeze" state forever on 5.0 and I hate to do anything, no matter how trivial, that might add even one extra day to the release at this point.

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 ...

Collapse
15: Re: New Tcl API proc "package_exec_plsql" ... (response to 14)
Posted by C. R. Oldham on 01/07/04 11:31 PM
Don,

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.

--cro

Collapse
16: Re: New Tcl API proc "package_exec_plsql" ... (response to 14)
Posted by Andrew Piskorski on 01/08/04 12:17 AM
Other than the "rules are rules" point of view (which does have some point), I don't see why not to add it, either now or soon. The only practical drawback is that there might be some bug with this new feature that we haven't discovered yet and will, in theory, make the 5.0.0 release look bad if some new developer finds it. But, really, that's not very significant, is 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?