On Sun, 2007-06-24 at 07:49 -0500, Tim Chase wrote:
> > Be aware, though, that there are lots of SQL constructs
> > (including a lot that crop up in stored procedures) that are
> > not handled correctly in the initial SQL parsing. For
> > backend-specific stuff, that's a small bug. For the more
> > general initial SQL, it's an unsolvable problem.
>
> Where would I go digging into understanding these limitations
> better? It seems to be in
>
> management.py: get_custom_sql_for_model()
That's the only place we touch those files. It's all in the one spot.
[...]
> I did notice while wandering through this code that is should
> handle most SQL, assuming that a terminal ";"
That's one assumption that isn't true, though -- somebody had an example
in a ticket once where a semicolon was inside a stored procedure and was
not a statement separator (in that you couldn't split the input at that
point).
Ticket #3214 has some quite complex changes to try and work around that
and other cases (it wasn't the counter-example I was thinking of, but it
is addressing a similar problem). You might want to have a look at that
and see if it seems reasonable. I haven't reviewed it in detail in a
while.
> (followed by
> optional whitespace before the EOL) separates statements. This
> should be sufficient for defining most stored procedures.
>
> However, I did notice that the regexp for pruning out comments is
> a bit lax, as it misses the event where a comment-marker is in a
> string:
>
> insert into app_tbl (column_name) values ('has--dashes');
>
> chokes. Granted, it's a tad pathological and fairly obvious to
> catch that something has gone wonky, but it would be best to
> avoid if possible.
True.
>
> The following two small changes tighten that up a bit to prevent
> that error case.
>
> + comment_re = re.compile("""^((?:'[^']*'|[^'])*)--.*""")
> for sql_file in sql_files:
> if os.path.exists(sql_file):
> fp = open(sql_file, 'U')
> for statement in statements.split(fp.read()):
> # Remove any comments from the file
> - statement = re.sub(r"--.*[\n\Z]", "", statement)
> + statement = comment_re.sub(r'\1', statement)
> if statement.strip():
> output.append(statement + ";")
> fp.close()
Patches not in Trac have very little chance of being remembered. :-)
One general note: we've been reluctant to tweak too much because it's a
constant arms race between pathological cases: you fix one corner-cases
and two others pop up. The current philosophy is "only simple SQL". Your
case might be sufficiently simple, though. I'll have to go back over the
old tickets when I get enthusiasm for it.
> It also saves you from re-compiling the regexp for every file.
> Not a huge savings, but just a general nicity. :)
Well, your optimisation is actually a degradation of performance in the
common case (the common case being no initial SQL files), since we're
nitpicking. :-)
Current code doesn't build the reg-exp unless it's needed. Of course,
the difference is so small as to be essentially zero between the two
cases.
> > So if you are using complicated SQL in your stored procedures,
> > you may need to load it manually.
>
> With the above patch, it seems the only caveat in
> back-end-specific SQL would be to ensure that your SP doesn't
> have any lines ending with a semi-colon or otherwise it will get
> split across execute() calls.
I can't remember all the problems that have come up, but I have a memory
it was a little worse than that. Might be bad memory on my part, though.
I'm willing to believe you for now.
File a ticket with your patch anyway, since it looks pretty safe and
I'll give it a look over. If you could have a look at the patch in #3214
and see what you think of that, too, I'd appreciate it. Some of those
tickets require somebody with actual motivation to look over them and
try to find holes -- when I'm reading the code, it will be with all due
care and a wish to find bugs, but since I don't use complex initial SQL,
I'm likely to miss some corner case.
Thanks for having a look at all this, anyway.
Regards,
Malcolm
--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Django users" group.
To post to this group, send email to [email protected]
To unsubscribe from this group, send email to [EMAIL PROTECTED]
For more options, visit this group at
http://groups.google.com/group/django-users?hl=en
-~----------~----~----~----~------~----~------~--~---
