[Zope-DB] Dynamically generate sql-query in ZSQL Method (and more...)

[Charlie Clark]

Hi Ian,

thank you for some very good, constructive criticisms. I think that, by 
induction, you're wrong - if ZSQL was really that bad it would have been 
replaced by something better. In your first e-mail you hit the nail on the 
head - the combination of ZSQL, DTML and in my opinion to a lesser extent 
the ZMI can be really nasty. But then again not as nasty as some of the 
alternatives out there.

First of all: three cheers for Zope and ZSQL! Referring back to last week's 
ZODBCDA thread: there are a lot of people coming to Zope and particularly 
Plone with little programming experience. Zope opens up a whole new world 
of possibilities for them. Not being a born programmer myself I am very 
happy with the way Zope and Python encourage modular development with 
frequent refactoring so much so that it has become natural for me. This 
fits in well with the demands placed on the incredible variety of 
applications that are springing up around the world all the time. But there 
are certainly pitfalls! I just wonder if they are as deep with Zope as they 
with competing technologies. PHP is my pet hate for a very similar reason: 
it allows you to make a mess easily and I've yet to find a real saving 
grace for it.

Three reasons to really like ZSQLs
1) you can test them on their own
2) they are modular so they encourage reuse kind of by definition
3) as objects in the ZMI they are flexible and can be moved and changed as 
required.

This doesn't mean that your criticisms aren't valid so let's go through 
them.

On 2004-04-07 at 01:12:12 [+0200], Ian Bicking wrote:
> On Apr 6, 2004, at 3:23 PM, Jim Penny wrote:
> > Ian has told you how to do it.  Now, I will tell you, don't do it.
> 
> I would disagree with you -- not for some principled reason, but for a
> very practical reason: Z SQL Methods suck.  A lot.  A huge, huge
> amount.  They are horrible.  Generating your own SQL inline sucks.  A
> lot.  It is horrible.  *Sometimes* the inline method is less horrible.
> These are the unfortunate choices foisted upon us by the environment.

This is rhetoric! What is your preferred alternative? Dymanic code is 
*always* going to present a problem in that is only debuggable at runtime.
 
> (Actually, it's Z SQL plus the ZMI plus DTML that is the real poison,
> IMHO -- they all work together to make things extra unpleasant)

Heavy nodding in this corner.
 
> Note also that well designed SQL generation in Python isn't horrible.
> A Python script like:
> 
> ## select_all
> ## parameters: table_name
> # re isn't available in the default Zope installation; I'd actually
> usually
> # do this with an extension method...
> import re
> assert re.search(r'^[a-zA-Z_][a-zA-Z0-9_]*$', table_name), "Invalid
> table name: %s" % table_name
> return container.raw_sql(sql='SELECT * FROM %s' % table_name)
 
Thanx for the code!

> That isn't so bad.  In fact, I would assert it is much better than the
> Z SQL analog.

No need to assert. This can be checked AFAIK as this is all open source.

> There's several ways to write a query, depending on how many of the Z
> SQL features you use (e.g., dtml-and, dtml-sqltest, dtml-sqlgroup,
> etc).  They all look quite different, and some will feel very foreign
> to someone who knows SQL but not Z SQL.

Yes, but this in my opinion is down to the Zope Book not being updated. The 
only DTML I have in my own ZSQLs is <dtml-if> </dtml-if>, <dtml-sqlvar> and 
<dtml-var>. The rest is completely unnecessary cruft and should be 
deprecated and buried quickly and quietly although some might want to 
preserve <dtml-sqldelimiter>. I would contend that <dtml-sqlvar> is a 
life-saver for many out there and I would personally like to thank whoever 
came up with it for making me aware of the need to check what is being 
inserted.

However, things could be improved. The ZSQL interface is clumsy and 
unfinished and if we're going to have type checking it should be when 
variables are declared. A suggestion: parametes should no longer be entered 
in a single text box but must be declared individually along with there 
type. Type and other checking could then occur in the background improving 
legibility and, hopefully, safety.

ie.
## name, string
## age, int
## dob, date

INSERT INTO person (?, ?, ?) VALUES (name, age, dob)

or something similar-
 
> It uses DTML, or DTML-like constructs.  Most of the things that are bad
> about DTML for templates are also bad for DTML generating SQL.  You
> could write entire books about why DTML is bad.

> It usually looks like normal SQL, but is far enough away from SQL to be
> difficult to work with in source form, since it uses <dtml-sqlvar>
> instead of like parameters like ? or even %s.  It's usually a long way
> from Z SQL source to something you can feed to EXPLAIN.

Indeed. But I wouldn't limit this to DTML. I'd throw in CFM, PHP, ASP, JSP, 
AppleScript and SQL itself for that matter. Going beyond placeholders and 
even the very simplest of control structures is going to cause problems in 
the end. SQL is my opinion only good for declarative work and even then is 
inconsistent enough to be the subject of many books, though I'd cite Fabian 
Pascal as one of the more salient critics.
  
> It is difficult to debug.  There's one largely hidden feature in being
> able to pass src__ in as a keyword argument to see what the SQL source
> actually is.  That's about all the debugging you get.  AFAIK, there's
> not good log of the SQL queries sent, how long they took, etc.

This is usually down to how the stuff has been coded. If there are 
<dtml-tests> and <dtml-groups> in there you're right, but otherwise I find 
ZSQL eminiently maintainable.

> Connection paths are pretty much hard-coded.  You can't tell a method
> to use a different connection.  This would be useful if you wanted to
> use database-level security, and so you need two separate connections
> with separate users.  You might be able to do this with acquisition,
> but whole books could be written on why acquisition is bad.  Actually,
> it would probably be a chapter in the DTML book.

I don't think you mean what you say here. I think you mean that you can't 
change the connection at runtime; you certainly can change the connection 
in a ZSQL. Acquisition isn't so much as bad by default but by use through 
opacity. The resulting problems are one of the reasons why it's not in Zope 
3 if I understand things correctly.
 
> It has crufty parameters.  Put "a, b, c" in , and you get parameters
> with names like "a,", "b," and "c".  Default parsing seems wonky too.
> 
> It doesn't signal errors properly -- if you forget to pass in a keyword
> argument, it's pretty much ignored.  Or if you pass in extra arguments.
>   Why have a parameter list at all?

This is not my experience or at least not with INSERTs. I think you may be 
referring to SELECT something FROM a RELATION WHERE a_critermium IS 'met'
The default behaviour here is similar to that in SQL anyway.
 
> It doesn't deal with failures very well.  Why not give me the SQL in
> the exception when there's an error?  It doesn't deal with any of the
> hard parts of failure in general, connection failures, concurrency
> conflicts, etc.

Surely this is the DA that does this? Most of the ones that I've worked 
with do? mxODBCZopeDA and Zpsycopg certainly do though some colour 
highlighting might be nice.
 
> You can't nest or factor your methods well.  This is largely DTML's
> fault, but Z SQL makes it worse since all methods are concrete.  You
> can't save values, go through control structures, etc., except using
> plain DTML constructs which have their own issues.  For instance,
> consider turning a list of items into SQL for use with IN, like
> (<dtml-in items prefix=loop><dtml-unless
> loop_start>,</dtml-unless><dtml-sqlvar loop_item type=int></dtml-in>).
> That's lame, *and* you can't abstract out the lameness.  Well, maybe
> with the use of src__ and other stuff, but that's it's own lameness.
> (Hmm... I see now there's an option to dtml-sqlvar to handle this
> particular case... so many options, but I can't use any of them from
> the outside, nest dtml expressions, etc)

sorry, can't see the real application of this. I think that ZSQL is 
probably next on the list to be stripped down now that we have 
PythonScripts and ZPT. But seeing as assuming you use it carefully you 
shouldn't have too many problems. Using it carefully means do not use DTML 
for control structures. Call a PythonScript to do the work.
 
> Oh, on the subject, every issue is dealt with using another dtml-sqlvar
> option or another tag, there's not a good set of orthogonal functions.
> 
> SQL Brains are cool, if a still little under powered.  But they don't
> see updates unless you go twiddle all sorts of forms.
> 
> You can't set any attributes on result rows.  Even if you know why it
> might not be a good idea.  Even if you try really hard.  (Though you
> can add a dummy column to your query, and then reassign the column)
> This is annoying particularly when using brains.

You have the results.names() call to any results set.
 
> Caching is stupid.  You can cache, but you can't invalidate the cache.
> There's a product out there that provides a cacheable SQL method, but
> it's a whole separate product and item type, and you can't easily go
> back and forth, or even upgrade.

You can always make a call to the global cache. But you're right being able 
to flush individuals caches would be useful and doable.
 
> All methods return result objects with multiple rows.  It's common to
> see:
> 
>    <dtml-in "get_some_object(id=whatever)"><dtml-var
> some_column>...</dtml-in>
> 
> When in fact you only expect their to be one row in the result.  This
> is where DTML's suckiness gets involved (dtml-in/dtml-with, all the
> flattened namespaces, etc).

DTML apart - please use ZPT or PythonScripts - all Python DB calls return a 
list of tuples. What really annoys me is when it's an empty list in which 
case
results[0].attribute generates an index error. This has to be encapsulated 
not in <tal:conditon here/myQuery tal:content="python 
here.myQuery[0].attribute"/> but by checking the length of the list!!!
 
> You start needing all sorts of methods for little things.  Like, say
> you want to fetch a value from a sequence, like "SELECT
> nextval('sequence_name') as seq".  Do you create a new method for this?

Of course: one method for one act. Call this through a (possibly generic) 
script which does the necessary work. Avoid DMTL.

>   What an annoying process!  (ZMI partially at fault)  And you get
> lameness like <dtml-in get_sequence_name><dtml-var seq></dtml-in>  Oh,
> it hurts me just to type this!  Or container.get_sequence_name()[0][0].
>   This leads to code being highly coupled.  You want to put a Python
> Script in the middle for some reason?  Now you have to return something
> like [[id]].  Or [{'seq': id'}]?  Only one way to find out... oh, the
> coupled pain!  (This is why I keep a firm Python Script barrier between
> my Z SQL methods and my "good code" -- I don't want Z SQL cruftiness to
> infect everything else with it's lameness)

Again, the problem doesn't really seem to be ZSQL but rather DTML.
 
> No positional parameters.  Seems like an okay idea, except for all the
> methods that are fetch-row-with-this-id, where positional is better.
> (Something else for the Python Script barrier)

Explicit is always better but maybe you have a particular example to show 
what you mean? Tuple unpacking perhaps?
 
> SQL methods are often tightly bound to specific scripts, templates,
> etc.  But it's hard to express this, and they are just more cruft in
> the ZMI.  If something is tightly coupled, I want to know it, I want to
> know that changes effect only one place, or that I shouldn't use it
> from the outside.  At least inline SQL gives you this.

I agree with you on this. One of the drawbacks with it being so easy to 
change things in Zope is that you often forget one method or another. 
Adding dependency management to the ZMI would be a real boon, I think.
 
> dtml-sqlvar doesn't have any good types.  type=date please?  Some
> attempt at making database more compatible would be nice.  Strings and
> floats are easy.  Solving difficult problems is actually useful, just
> solving easy problems is a programming cop-out.

Is type checking in the programming language actually necessary, I wonder? 
Surely the database will raise a TypeError if appropriate. In addition it 
is the work of individual DAs do to type mapping so maybe type-checking 
should be removed from ZSQL altogther and ZSQL / ZopeDAs should just go 
about preventing SQL injections?
 
> Doesn't do anything with database introspection.  Again, I know it's
> hard, but hard is useful.  And it's not that hard.

What, in particular, are you looking for?
 
> The test pane is really lame.  I won't go into all of why it's lame.
> It could be useful, but it's only distracting.  (I might *think* I can
> just test a quick query in it, but I'm always wrong)

I think you're in a minority here. It needs fixing.
 
> I think that's it.  I'd say it felt good to vent, except I'm going to
> have to work with Z SQL methods again tomorrow, and venting doesn't
> make up for that.

Oh, come on! Think positive: you might have to be fixing an even more 
tightly coupled PHP system including migration from one RDBMS to another 
with the prospect of literally hundreds of changes to be made and really 
pathetic error reporting.

And now the call to arms: if ZSQL is stuck then we can fix it. That is 
after all the beauty of Open Source, isn't it? It's possible that within 
Zope Corporation they have enhanced ZSQLs that fix all their woes but they 
haven't chosen to release them. I've already discussed some of these issues 
with Marc-André in respect to the mxODBCZopeDA which would allow for 
"enhanced" ZSQLs to allow quoting to be done by the driver. However, any 
such product would out of necessary be tied to our DA, with the sincere 
hope that others would follow suit.

As has been said several times the whole Zope/RDBMS part needs reworking 
and we can do this if we want to. It's unlikely to happen otherwise.

I would like to tentatively suggest a BOF (birds of a feather) on this at 
EuroPython assuming others interested are also intending to attend. I'm not 
sure whether further, detailed discussion should occur on this list but I 
suspect that there are not yet enough conspirators to warrant setting up 
another list. Does anyone know what the state of Zope 3 is in this respect?

Respectfully

Charlie Clark
-- 
Charlie Clark
eGenix.com

Professional Python Services directly from the Source
 >>> Python/Zope Consulting and Support ...        http://www.egenix.com/
 >>> mxODBC.Zope.Database.Adapter ...             http://zope.egenix.com/
 >>> mxODBC, mxDateTime, mxTextTools ...        http://python.egenix.com/
________________________________________________________________________

::: Try mxODBC.Zope.DA for Windows,Linux,Solaris,FreeBSD for free ! ::::