Forum OpenACS Q&A: There's a problem with education/class/one.tcl
In displaying the assignments, there is a complex SQL statement. It has not been ported correctly, yet I am at odds at interpreting exactly how it works. Here's the origianal text:
I know that it's saying give me those assignments that the user is allowed to see and that is the latest version, but I'm going to need a clearer understanding of what's going on to get it fixed.# set sql "select edu_assignments.assignment_name, # edu_assignments.assignment_id, # edu_assignments.due_date, # pset.version_id, # pset.file_extension, # pset.url, # sol.url as sol_url, # sol.file_extension as sol_file_extension, # sol.version_id as sol_version_id, # answers.url as ans_url, # answers.file_extension as ans_file_extension, # answers.file_title as ans_filename, # answers.version_id as ans_version_id, # edu_assignments.electronic_submission_p # from edu_assignments, # edu_assignments edu_assignments1, # (select * from fs_versions_latest # where ad_general_permissions.user_has_row_permission_p($user_id, 'read', version_id, 'FS_VERSIONS') = 't') pset, # (select file_extension, url, version_id, task_id # from fs_versions_latest ver, # edu_task_solutions solutions # where ver.file_id = solutions.file_id # and ad_general_permissions.user_has_row_permission_p($user_id, 'read', version_id, 'FS_VERSIONS') = 't') sol, # (select file_extension, file_title, url, version_id, task_id # from fs_versions_latest ver, # edu_student_answers ans, # fs_files # where ver.file_id = ans.file_id # and fs_files.file_id = ver.file_id # and ad_general_permissions.user_has_row_permission_p($user_id, 'read', version_id, 'FS_VERSIONS') = 't' # and student_id = $user_id) answers # where edu_assignments.class_id = $class_id # and edu_assignments.file_id = pset.file_id(+) # and edu_assignments1.assignment_id = edu_assignments.assignment_id # and edu_assignments1.assignment_id = answers.task_id(+) # and edu_assignments.assignment_id = sol.task_id(+) # order by due_date"
Right now, it doesn't come up with any assignments, although it ought to.
I guess I ought to be more specific.
- What does ad_general_permissions.user_has_row_permissions_p($user_id,'read',version_id,'FS_VERSIONS') = 't' do? And why can't we use the same thing in Postgres, or write something that works the same in the SQL statment?
- It looks like answers, pset, and sol are the effects of these select statements. It also looks like:
- pset is joining ad_general_permissions to fs_versions_latest.
- sol is joinging edu_tasks_solutions with fs_versions_latest.
- answers is joining fs_versions_latest, edu_student_answers, and fs_files.
- Why is there edu_assignments1? What is the benefit of referencing a second instance of the table? Can't we just join answers to edu_assignments? If so, that would leave us with edu_answers, pset, answers, and sol to join. Is that right? Possible?
1.What does ad_general_permissions.user_has_row_permissions_p($user_id,'read',version_id,'FS_VERSIONS') = 't' do? And why can't we use
the same thing in Postgres, or write something that works the same in the SQL statment? I just spent the day digging through intranet, portals, etc weeding out bugs so am not up to tackling this one at the moment, but wanted to mention that. Also, a big clue - get your head around what the query really wants to do, then look into re-writing it within the limitations of PG rather than trying a mechanical translation. I just simplified and fixed some complex unfoldings of outer joins in the intranet module based on doing this (which is not a criticism of our team member who did the original unfolding, it was an early big effort on his part and he had no real help from me or others who'd been down that path before).
Along with this, I've found that some of the more complex queries I've dealt with can easily be restructured to avoid some of the porting hassle. No knock on aD here (though I'm not surrendering my right to knock 'me right and left as I see fit!) but taking a straightforward approach in Oracle that doesn't necessarily port easily is no sin on their part.
# set sql "select edu_assignments.assignment_name, # edu_assignments.assignment_id, # edu_assignments.due_date, # pset.version_id, # pset.file_extension, # pset.url, # sol.url as sol_url, # sol.file_extension as sol_file_extension, # sol.version_id as sol_version_id, # answers.url as ans_url, # answers.file_extension as ans_file_extension, # answers.file_title as ans_filename, # answers.version_id as ans_version_id, # edu_assignments.electronic_submission_p # from edu_assignments, # edu_assignments edu_assignments1, # fs_versions_latest pset, # solution_files sol, # student_answer_files answers # where edu_assignments.class_id = $class_id # and answers.student_id = $user_id # and user_has_row_permission_p($user_id, 'read', pset.version_id, 'FS_VERSIONS') = 't' # and edu_assignments1.assignment_id = edu_assignments.assignment_id # and edu_assignments.file_id = pset.file_id # and edu_assignments1.assignment_id = answers.task_id # and edu_assignments.assignment_id = sol.task_id # union # select edu_assignments.assignment_name, # edu_assignments.assignment_id, # edu_assignments.due_date, # pset.version_id, # pset.file_extension, # pset.url, # '' as sol_url, # '' as sol_file_extension, # '' as sol_version_id, # answers.url as ans_url, # answers.file_extension as ans_file_extension, # answers.file_title as ans_filename, # answers.version_id as ans_version_id, # edu_assignments.electronic_submission_p # from edu_assignments, # edu_assignments edu_assignments1, # fs_versions_latest pset, # student_answer_files answers # where edu_assignments.class_id = $class_id # and answers.student_id = $user_id # and user_has_row_permission_p($user_id, 'read', pset.version_id, 'FS_VERSIONS') = 't' # and edu_assignments1.assignment_id = edu_assignments.assignment_id # and edu_assignments.file_id = pset.file_id # and edu_assignments1.assignment_id = answers.task_id # and not exists (select 1 from solution_files # where task_id = edu_assignments.assignment_id) # union # select edu_assignments.assignment_name, # edu_assignments.assignment_id, # edu_assignments.due_date, # pset.version_id, # pset.file_extension, # pset.url, # sol.url as sol_url, # sol.file_extension as sol_file_extension, # sol.version_id as sol_version_id, # '' as ans_url, # '' as ans_file_extension, # '' as ans_filename, # '' as ans_version_id, # edu_assignments.electronic_submission_p # from edu_assignments, # edu_assignments edu_assignments1, # fs_versions_latest pset, # solution_files sol # where edu_assignments.class_id = $class_id # and user_has_row_permission_p($user_id, 'read', pset.version_id, 'FS_VERSIONS') = 't' # and edu_assignments1.assignment_id = edu_assignments.assignment_id # and edu_assignments.file_id = pset.file_id # and not exists (select 1 from student_answers # where task_id = edu_assignments1.assignment_id # and student_id = $user_id) # and edu_assignments.assignment_id = sol.task_id # union # select edu_assignments.assignment_name, # edu_assignments.assignment_id, # edu_assignments.due_date, # pset.version_id, # pset.file_extension, # pset.url, # '' as sol_url, # '' as sol_file_extension, # '' as sol_version_id, # '' as ans_url, # '' as ans_file_extension, # '' as ans_filename, # '' as ans_version_id, # edu_assignments.electronic_submission_p # from edu_assignments, # edu_assignments edu_assignments1, # fs_versions_latest pset # where edu_assignments.class_id = $class_id # and user_has_row_permission_p($user_id, 'read', pset.version_id, 'FS_VERSIONS') = 't' # and edu_assignments1.assignment_id = edu_assignments.assignment_id # and edu_assignments.file_id = pset.file_id # and not exists (select 1 from student_answers # where task_id = edu_assignments1.assignment_id # and student_id = $user_id) # and not exists (select 1 from solution_files # where task_id = edu_assignments.assignment_id) # union
# select edu_assignments.assignment_name, # edu_assignments.assignment_id, # edu_assignments.due_date, # pset.version_id, # pset.file_extension, # pset.url, # sol.url as sol_url, # sol.file_extension as sol_file_extension, # sol.version_id as sol_version_id, # answers.url as ans_url, # answers.file_extension as ans_file_extension, # answers.file_title as ans_filename, # answers.version_id as ans_version_id, # edu_assignments.electronic_submission_p # from edu_assignments, # edu_assignments edu_assignments1, # fs_versions_latest pset, # solution_files sol, # student_answer_files answers # where edu_assignments.class_id = $class_id # and answers.student_id = $user_id # and user_has_row_permission_p($user_id, 'read', pset.version_id, 'FS_VERSIONS') = 't' # and edu_assignments1.assignment_id = edu_assignments.assignment_id # and not exists (select 1 from fs_versions_latest # where file_id = edu_assignments.file_id) # and edu_assignments1.assignment_id = answers.task_id # and edu_assignments.assignment_id = sol.task_id # union # select edu_assignments.assignment_name, # edu_assignments.assignment_id, # edu_assignments.due_date, # pset.version_id, # pset.file_extension, # pset.url, # '' as sol_url, # '' as sol_file_extension, # '' as sol_version_id, # answers.url as ans_url, # answers.file_extension as ans_file_extension, # answers.file_title as ans_filename, # answers.version_id as ans_version_id, # edu_assignments.electronic_submission_p # from edu_assignments, # edu_assignments edu_assignments1, # fs_versions_latest pset, # student_answer_files answers # where edu_assignments.class_id = $class_id # and answers.student_id = $user_id # and user_has_row_permission_p($user_id, 'read', pset.version_id, 'FS_VERSIONS') = 't' # and edu_assignments1.assignment_id = edu_assignments.assignment_id # and not exists (select 1 from fs_versions_latest # where file_id = edu_assignments.file_id) # and edu_assignments1.assignment_id = answers.task_id # and not exists (select 1 from solution_files # where task_id = edu_assignments.assignment_id) # union # select edu_assignments.assignment_name, # edu_assignments.assignment_id, # edu_assignments.due_date, # pset.version_id, # pset.file_extension, # pset.url, # sol.url as sol_url, # sol.file_extension as sol_file_extension, # sol.version_id as sol_version_id, # '' as ans_url, # '' as ans_file_extension, # '' as ans_filename, # '' as ans_version_id, # edu_assignments.electronic_submission_p # from edu_assignments, # edu_assignments edu_assignments1, # fs_versions_latest pset, # solution_files sol # where edu_assignments.class_id = $class_id # and user_has_row_permission_p($user_id, 'read', pset.version_id, 'FS_VERSIONS') = 't' # and edu_assignments1.assignment_id = edu_assignments.assignment_id # and not exists (select 1 from fs_versions_latest # where file_id = edu_assignments.file_id) # and not exists (select 1 from student_answers # where task_id = edu_assignments1.assignment_id # and student_id = $user_id) # and edu_assignments.assignment_id = sol.task_id # union # select edu_assignments.assignment_name, # edu_assignments.assignment_id, # edu_assignments.due_date, # pset.version_id, # pset.file_extension, # pset.url, # '' as sol_url, # '' as sol_file_extension, # '' as sol_version_id, # '' as ans_url, # '' as ans_file_extension, # '' as ans_filename, # '' as ans_version_id, # edu_assignments.electronic_submission_p # from edu_assignments, # edu_assignments edu_assignments1, # fs_versions_latest pset # where edu_assignments.class_id = $class_id # and edu_assignments1.assignment_id = edu_assignments.assignment_id # and not exists (select 1 from fs_versions_latest # where file_id = edu_assignments.file_id) # and not exists (select 1 from student_answers # where task_id = edu_assignments1.assignment_id # and student_id = $user_id) # and not exists (select 1 from solution_files # where task_id = edu_assignments.assignment_id) # order by due_date"
Dan, did you commit this query to CVS?
Also, you might check out the way I rewrote /intranet/employees/admin/index.tcl and friends. I was able to get away from the horribly complex set of unions that resulted from mechanical translation. The result might be a bit slower, using subselects in the select clause of the main query, but I at least find it a heck of a lot easier to read. And it might not be slower, I suspect huge unfolded unions might slow down the optimizer as much as the subselects in my rewrite slow down the executor.
My rewrite trick won't apply to all complex outer joins, mechanically unfolded horrors like your example here are still going to be necessary at times. I haven't systematically gone through intranet yet to see if I can do similar simplifications (I was originally motivated by the fact that a couple of your big unions didn't quite work right, but I suspect most of them do).
In the case of the intranet module, it's all bogus anyway. If the turkey who wrote it always created an im_employee_info entry when you add an employee it wouldn't be necessary to outer-join to the table in the first place. Given that org-chart.tcl breaks in ACS Classic until you manually add "none" as a supervisor for your CEO (as opposed to the "none" created by magic via outer joins), aD doing it right in the first place sounds like it would've been a winner to me...
I'll commit it tonight. I'm at work now, and I'm stuck behind a firewall, so I don't have access to cvs.
Also, you might check out the way I rewrote /intranet/employees/admin/index.tcl and friends. I was able to get away from the horribly complex set of unions that resulted from mechanical translation.
I'll take a look at it. Ben fixed one of the intranet queries for me that had 16 unioned selects. It turned out that all of the outer-joined components were non-null, so in actuality, only two of the 16 outer join cases would actually occur. The fixed query had only two select clauses.
My rewrite trick won't apply to all complex outer joins, mechanically unfolded horrors like your example here are still going to be necessary at times.
When I was porting intranet, I suspected that many of the queries could be simplified, but given the size of the task I opted for the mechanical porting approach. Usually it went copy, paste, paste, paste, add not exits clauses and remove dangling tables in the from clause. Then test and bang it was done! I did alot of these in less that five minutes.
I haven't systematically gone through intranet yet to see if I can do similar simplifications (I was originally motivated by the fact that a couple of your big unions didn't quite work right, but I suspect most of them do).
That might be a could thing to do for all of the modules once they stabilize. I didn't want to spend too much time on intranet as my impression was that it was still a work in progress from aD. Major changes occured from 3.1 to 3.2, and there still seem to be some design issues that could be fixed. Have you looked at intranet in the 3.2.2 release? There might be some new fixes there.
I used the new sql statement that was submitted, and then had to spend the night last night trying to tweak it to work right. I actually have a final version that we should probably submit. I also had to do a major rework of the sql statement in /education/class/assignments-info.tcl. This too is available if you want it.
Which brings me to my next question. I'm spending 6-8 hours a day learning what I can about this system, and fixing errors as I go. I know that you have an organized development lifecycle. I also understand I'm not an official part of that group. How would you like me to work with you on this? Since I'm new, and prone to errors (even though I've been programming for a few years, all this web stuff is only about 8 months old to me now) maybe I ought to submit files to someone for a peer review. Maybe later I can actually make additions to the CVS tree. (I'll need someone to point me to a URL or two so that I can train myself with this tool when we get to that point).
A lot of what I've changed in the system has been in regard to karl goldstein's DPS. If you decide to port 3.3, let me know. Also, I've gone through the whole system and cleaned out all the nextval stuff. There were 300 some odd illegal references left in beta 3. I know that this kind of change blows your CVS tree apart (don't you have to check out and add one file at a time?). We'll need to work out some plan on using the hours I'm putting into the system for the general good, and not just for my own port of the system.
Finally, it looks like I'm going to venture out from under the corporate umbrella and beat my own path to success in about two or three weeks now. I start graduate school in about a month, and I've decided that I'd rather put in 60 hours a week working for myself than 80 hours a week working for someone else and myself. That means I'll have all kinds of time with this system.
Can you tell my mind is going in a million directions? Sorry.
It's good to see this much interest, but I'm worried that you're working on stuff that doesn't necessarily need fixing! For example, the nextval stuff is fixed by some automatic filtering of SQL queries in tcl/postgres.tcl. I'm sure a couple of errors were still there, but certainly not 300....
So, we're open to having more people on the team, definitely, we're not a closed development team. I recommend you not hesitate to post more of your thoughts, so that you can get the feedback from the whole team, and we can solve these issues together! I hope we can get your help on this in the long run, this is the kind of enthusiasm we need!
Stay posted, I'm in the process of upgrading the way we do CVS and development in preparation for 4.0 development. We won't be porting 3.3, since it's more experimental than anything else.
My experience in the team has been that it is very open to contributions, and a very fun team to work with. Lots of good learning involved. Just sign up at Sourceforge (we should move to SDM-only shortly) and let Ben know.
What you are doing here, of sharing your thoughts is very welcome. Because there's a lot of code, make sure you let the rest of the team know what you want to work with and before doing major changes or changing something that is going to drift us away too much from the aD code.
I've often times thought that something was broken, when it actually had a reason to be that way (e.g. decripting passwords from the DB). Thanks for the help Dave !
I'm glad that such a gaping hole wasn't actually left open. However, I know that the whole system hasn't felt nearly as flakey since I worked through this issue. (I used to run into a lot of buggy pages which I would ignore if I were just touring the toolkit, but would hammer through if it was important to me.)
Anyway, thanks for your encouragement and added insight into the issue.
I think the concern's that you spend time hacking through 300 of them when we could've gotten more work out of you if we'd known what you were doing :)
OpenACS needs lots and lots of testing. Not only do we have our own porting errors to worry about, but as distributed by aD the toolkit's got plenty of bugs that we'd like to weed out, too.
/education/class/assignments-info.tcl
where is it available??
do it needs some extra work or just copy the files....
thanx for your help!
P.S. I noticed you actually visited my site! Two people that aren't blood relatives or long-time friends have actually logged into my site! I'm exstatic. It needs a lot of work, doesn't it?
