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

Not a big issue on a small site but every bit helps. By moving them into the .tcl you lose that ability. Also, if the potential of db_map is ever realized, how will this affect that use?

If we take this route we'll end up with a toolkit that has some generic queries embedded in Tcl, others in .xql files, no consistency whatsoever.

In order to get consistency, which is easier?  Move the embedded queries in your new code to .xql files, or to get rid of all the generic .xql files and embed these queries in .tcl files?

Because the latter is what we should do to avoid a mish-mash if your proposal is adopted.  We should shoot for consistency and and either have only RDBMS-specific queries in RDBMS-specific .xql files, with all generic queries embedded in .tcl, or continue according to today's development standard, all queries in .xql files.

However, I'll put my two cents in for the idea of having all the queries in the tcl scripts.  I don't know if it could be reasonably accomplished, having given no thought to how it would be implemented, but strictly from a programming usability POV having the queries in seperate files drives me nuts.  It's much harder to visualize what an unfamiliar piece of code does without being able to see all the code, no matter how well-named the query is.

Janine ... when we first discussed supporing both Oracle and PG and (potentially) other RDBMSs there was a lot of discussion of the merits of putting conditionalized queries inline in .tcl files and doing separate query files.  Both solutions have inherent uglinesses about them, the queryfile approach was adopted as the lesser of evils solution after a bunch of discussion among those who were going to do the porting.

I could adopt to either using query files exclusively or dumping them entirely, though.  It's this notion of having some of the toolkit programmed one way, the rest another way that irks me.

We already support too many different ways of doing different things IMO - look at the recent discussion regarding ad_proc parameter conventions!

This is just how I feel, though ...

Lars - one (slight) benefit is that inline queries are slightly more efficient, though in practice there's no evidence that queryfiles are a barrier to adequate scalability (the RDBMS parse, optimization and execution time far outweighs query lookup time).

As far as the general philosophy regarding changes to coding standards go ... I don't see much sense in deciding to make major changes to the way we do things unless we're also going to decide to make old code adhere to the new way of doing things.

And if we're going to drop query files for generic queries while maintaining them for RDMBS-specific queries ... it just feels like the worst of both approaches.  I'd much rather have all queries separated or all queries inline, myself.  One or the other.  And at the time the decision to use query files was made, the half-dozen or so of us discussing it seemed to agree on this point, at least.

I don't remember anyone suggesting that having some queries inline, others in query files was an agreeable solution.

To be honest if it weren't for the desire to support existing ACS packages with no change in our first OpenACS release, the db_* API procs would've been changed to take an explicit -sql parameter if SQL was being passed in (as we do for dynamic queries build from db_map calls) with no sql param passed in at all in the normal case.

Anyway I'm interested in feedback from other OCT members...

Part of our ability to do so (and produce military aviation quality code) rested on following a coding standard. It is MORE difficult for a novice programmer to figure out the system if things are done multiple ways.

I actually look forward to a time when we use the table-lite spec or the ancient (and secret) prototype-builder or some new-and-improved spec to generate XQL files from a table spec to allow rapid development (and forms and acs-objects, etc).. The more consistent we are with the way we build things, the easier the tool will be to write.

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

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.

Inline queries are a must for development, and for all the custom development that is for one database only and is never intended to be distributed to or used by anyone but the client, there generally isn't the slightest reason to use anything but inline queries.

Personally, in hindsight I don't think using XML (rather than a Tcl syntax) for the query files was such a great choice, but all the work was done way back when, and now that tdom is so much faster than ns_xml used to be it doesn't really matter much. I certainly don't see any reason to change that now.

In my experience there's only one practical problem with the current support for the mix of inline queries and queries in various different XQL files: It's very easy to accidentally define the same query in several different places, and then the query dispatcher will silently choose one of them. If feasible, it might be best to have a configuration switch to optionally make the query dispatcher throw an error whenever it encounters multiple definitions of the same query.

But some people did care so the decision was made to use queryfiles.  The drive in this direction originally was made by aD and Ben when it was decided to put querynames in the db_* API (if the conditionalized inline query approach is taken there's no reason for queries to have names).

But I will repeat that no one argued for a mixed approach, at the time of the discussion the value of choosing one and only one approach was accepted by all.  Adopting a mixed approach seems to me much worse than taking one direction or the other.

As far as inventing tools to make both camps happy ... it strikes me once again that as a community we have a very difficult time concentrating on the important things.

And this ain't one of them, frankly.  We shouldn't be thinking about writing a tool in order to avoid making a decision.

If we write any tool at all, it should probably be one to organize the contents of query files inline, using -generic, -postgresql, and -oracle switches.  Get rid of query files and the query name parameter and be done with it, if people don't like them.  This would maintain the consistency I feel is so important, at least, Lord knows I have no emotional investment in queryfiles per se, I'm just devoted to doing things one way and one way only unless there's extremely good justification for not doing so.

But maybe I'm flogging a dead horse.  The rest of the toolkit's a mishmash of conflicting coding styles, after all.

"I see XQL files as leaning way to heavily towards code organization (OpenACS maintainers) rather than ease of understanding for new folks or people trying to do work with OpenACS. ..it would be nice to come up with a solution that kept both camps happy."

a slow-learning OpenACS developer's perspective: I find keeping up with changes in development, including development style, has been the greatest problem. So many changes, so fast, is dizzying!

Any framework will have strengths and weaknesses. Even I see how the external named queries are great for reducing tcl code complexity, improving query presentation (xml) and providing reference names for discussion; And the inline queries are great for quick changes.

Having a system with two frameworks available can be a bonus if both can be supported without performance loss. Just document when is best to use each technique, please.

On the other hand, if that optional pre-processor is attractive and important purely because of performance problems (in even some cases) with handling the XQL files, then something else about the OpenACS XQL architecture or tools is probably wrong, and should be addressed separately.

Longer run, seems that Don has tentatively (hypothetically?) suggested eliminating the XQL querfiles altogether, and defining a simple syntax for putting all the db-specific SQL into the same Tcl files. Not merely a -sql switch for inline queries, but several at once, "-generic, -postgresql, and -oracle"

The main advantage I see there is it lets (indeed, forces) you to see the same query for all the databases together at once. Upside is this would push much greater cross-db knowledge, and help eliminate spurious drift and redundancy in the Oracle vs. Postgres queries. All "official" OpenACS packages are supposed to support both databases anyway, and this would make the work of doing so somewhat easier.

The downside of course, is that it does tie support for the different databases more closely together. In the past people seemed to think it advantageous that, at least in theory, some outside group could decide to add support for a 3rd database just by adding a whole bunch of new XQL files, without having to touch the rest of the toolkit at all. But given history of Oracle and PostgreSQL support, and the continual ongoing development of the OpenACS toolkit itself, I doubt that any such "just drop in support for a 3rd database" project would be that simple. My guess is it that in reality it would need extensive co-ordination and some changes from the core OpenACS developers no matter what, whether the SQL is all in separate XQL files or not. What do you think, is my guess here wrong?

But, really, Don's "-generic, -postgresql, -oracle" suggestion is a major change that would affect a lot of people, and nobody's going to do it anytime soon, right? (My guess is probably never, but not till OpenACS 5.1 at the very earliest.)

Six of one, half a dozen of the other...

Consistency is the most important point. Changing the way it works now would be a huge task. That effort could be better applied to other items,

Does this mean that any idea that might be good to implement, but doesn't make the top 3 priority list is in danger of being vetoed?!  Frankly, I don't think that's the proper way to prioritize.

Sweeping these kinds of things under the rug isn't the way to go, imo.  With Tcl TIPs, the one proposing the change is responsible for either implementing it, or finding someone who will implement it.  A TIP approval says nothing about when it needs to be done, or even that the OCT must take time to do it themselves. I see it as moving that proposal onto the official "todo" list.  Scheduling those todos comes later.

2) I think the original TIP was slightly unclear in that it did not describe the current situation fully.  The current situation is, I believe, that "best practice" is for all SQL to be in XQL files.  The veto means that this is still the case.

3) The status quo leaves two obvious questions which I don't believe were directly addressed:
  a) Will we reject packages that don't meet this best practice?  (No idea what the consensus would be - sounds like a good show-of-hands question in an OCT meeting)

  b) Will we change the kernel to enforce this best practice - ie, will we start ignoring any SQL in TCL files?  Argument for: enforces consistency.  Argument against: is a useful shortcut during development; requires lots of work with only a potential, future reward.  I think this one is a clear no, and status quo is "no," so unless someone starts a new TIP I don't think this issue is open.

What spurred this TIP is that we at Collaboraid have sped up our development by not wasting time creating and maintaining the .XQL files for common SQL.

In other words, we have contributed code with common SQL in the TCL files instead of the XQL files.

Vetoing or rejecting the current proposal incurs additional overhead on development. Specifically, it requires us to spend time that we could have on other things on the zero-gain task of moving SQL code out of Tcl files and into XQL files.

The time spent adhering to the coding standard required by this TIP could have been spent fixing bugs, pushing the release out, or adding new functionality.

As a result of the vote, we have chosen to prioritize our resources towards extra burdens on development over bug fixes.

Not a very useful prioritization IMO.

Oh, well, we can't all agree on everything. :)

/Lars

Not all coding standards are as quibbly as perhaps the "use .xql files" standard is.

But we had a similar experience with .LRN, where OpenForce delivered a product that in many places is full of Tcl code dropped helter-skelter into .adp files.  They did so without bothering to discuss the decision with the community, though they knew that the prohibition against doing so was a coding standard for not only OpenACS but aD's ACS 4.x project as well.

Why did they do it?  Because it saved them time when writing certain especially tricky pages (at least they claimed it did).

Now I don't think you'll find a single OCT member who will endorse that practice, but due to the informality of the community and the fact that .LRN was a standalone effort by OF/Sloan in those days we didn't realize they were doing this until the code was released.

And many of us went "YECH!" when we saw what they'd done.

So I'm hoping we all at least see the value of deciding such things at the TIP level, even if we disagree over the rejection of this particular one ...