Forum OpenACS Improvement Proposals (TIPs): Re: TIP #37 (Proposed): Queries in XQL file conventions

First let me just say that I don't have a strong opinion on this matter yet. However, I concur with Alfred in that consistency is very important. Right now there are four places you can put your query, in a db independent xql file, in a db specific xql file, and inline. I think there should only be one place to put generic sql statements and I'm inclined to think that that place should should be an xql file. That's closest to what we've been doing so far and is easiest to explain to new developers. SQL *always* lives in xql or sql files.

Having said this I understand where Lars and Joel are coming from. Putting queries inline is certainly convenient and very speedy during development. So then again, for consisency, if inline SQL is preferable, let's just always keep it inline.

Related to this issue, the thing I feel strongest about is removing inline SQL that is not being used. Regardless of where you keep your SQL, please just keep it in one place... Preferably we would write a script to remove all obsolete inline SQL.

Collapse
Posted by Lars Pind on
Consistency is good, of course.

I agree that we should absolutely remove current inline queries where they're also in an .XQL file, given that the .XQL files always take precedence.

But if we do remove inline SQL where also in the .XQL file, I don't think consistency is a major concern under this scheme, since the process for finding the SQL code will not be interrupted.

With the proposed scheme, after removing extra inline queries, but before moving all common queries from .xql file into .tcl file:

1. Open the Tcl file of the page or library proc to find the db_* command. (100%)

2. If there's inline SQL, you've got the query (~20%)

3. If not, go look in the generic .XQL file (~50%)

4. If not, find it in the db-specific .XQL file (~30%)

With the proposed scheme, after removing extra inline queries, and after moving all common queries from .xql file into .tcl file:

1. Open the Tcl file of the page or library proc to find the db_* command. (100%)

2. If there's inline SQL, you've got the query (70%)

3. If not, find it in the db-specific .XQL file (30%)

If this proposal is rejected (after removing extra inline queries):

1. Open the Tcl file of the page or library proc to find the db_* command. The SQL part will be empty. (100%)

2. Open the generic .XQL file, to see if the query is there (70%)

3. If not there, go check the db-specific .XQL file (30%)

My point is that the proposal doesn't interfere with the development flow, but instead it lets you save the hassle of opening and searching through another file 20-70% of the time.

Oh, well, Don's and my position are clear. Can we get some OCT votes, so we can get this matter resolved and move on, please.

/Lars

Collapse
Posted by Jeff Davis on
I vote for generic queries in the tcl files.

I think Alfred's point is a good one, there is value to being able to write static code analyzers but I don't think sql in the tcl files is a significant roadblock to doing so (in particular, you need to have a pretty smart parser already to deduce the fully qualified qeury name and pulling the sql out at the same time is pretty trivial relative to that problem).

Given that to date we have not done anything fancy with reusable queries and that there is no support for paramaterizing queries (and I agree with lars it's better to put the query in a tcl proc and have that be the reuse mechanism), I just don't see a material downside to doing this.

So far we have been trying to keep database, application logic and display seperate and we try not to mix TCL code in .adp pages and HTML in .tcl files.

I think we should continue down that road. And make it an easy coding standard to follow (SQL goes in .xql files). Otherwise people might get confused and put an Oracle specific query in the .tcl file. I'm sure it won't happen to OCT members, but we are not the only ones developing and for newbies sticking to simple consistent rules is IMO better.

I'm not against it in a way that I'd veto it, but I'd adwise against it if asked.