Django

Code

Ticket #4680 (new)

Opened 3 years ago

Last modified 4 months ago

Fixes problem in initial_sql where "--" is in a string

Reported by: Tim Chase Assigned to: anonymous
Milestone: Component: django-admin.py
Version: SVN Keywords: initial-sql manage.py sprintdec01
Cc: code.djangoproject@tim.thechases.com Triage Stage: Accepted
Has patch: 1 Needs documentation: 0
Needs tests: 0 Patch needs improvement: 1

Description

In the core/management.py get_custom_sql_for_model() function, if the initial SQL has a "--" in a string, it chokes.

The attached diff should fix the problem. Test cases:

select '--' as foo; -> select '--' as foo;
select '--' as foo; -- select stuff -> select '--' as foo;
select '--' as foo; -- select stuff -- because I want to -> select '--' as foo;
select * foo; -- select stuff -- because I want to -> select * foo;
select * foo; -- select stuff -> select * foo;

Attachments

initial_sql.diff (0.9 kB) - added by Tim Chase on 06/24/07 18:55:13.
"svn diff" against trunk
django-ignore-comments-in-sql.patch (5.2 kB) - added by shaleh on 11/30/07 21:01:59.
updated patch. Seems simpler than the previous one. Added test casesto simple.sql as well as a unittest for custom_sql_for_model itself

Change History

06/24/07 18:55:13 changed by Tim Chase

  • attachment initial_sql.diff added.

"svn diff" against trunk

06/25/07 16:04:06 changed by Gary Wilson <gary.wilson@gmail.com>

  • needs_better_patch changed.
  • stage changed from Unreviewed to Accepted.
  • needs_tests set to 1.
  • needs_docs changed.

09/15/07 13:31:15 changed by mboersma

  • owner changed from nobody to mboersma.
  • status changed from new to assigned.

09/16/07 00:53:23 changed by anonymous

  • owner changed from mboersma to anonymous.
  • status changed from assigned to new.

11/30/07 21:01:59 changed by shaleh

  • attachment django-ignore-comments-in-sql.patch added.

updated patch. Seems simpler than the previous one. Added test casesto simple.sql as well as a unittest for custom_sql_for_model itself

11/30/07 21:53:36 changed by shaleh

  • keywords changed from initial-sql manage.py to initial-sql manage.py sprintdec01.
  • needs_tests deleted.

added tests

12/01/07 22:16:01 changed by Simon G <dev@simon.net.nz>

  • stage changed from Accepted to Ready for checkin.

12/02/07 18:19:02 changed by mtredinnick

  • needs_better_patch set to 1.
  • stage changed from Ready for checkin to Accepted.

The first patch doesn't work (as demonstrated by the tests in the second patch). I don't like the code changes in the second patch, though. Having to a linear scan through every character, one at a time, in the initial SQL strings seems inefficient. There must be a better way.

12/03/07 12:51:17 changed by shaleh

Yeah, I was not entirely happy either BUT how often is this called? Is it really in a time efficient section? In other words, while it may not be 100% pretty do we really care? My understanding is this code only happens at application initialization time from manage.py or similar. We could make a sample sql file that had a thousand or two entries and see just how long it takes.

I am willing to help get this finalized, but I do not see a clear regex only solution out of this.

09/10/08 00:21:11 changed by adrian

  • component changed from Tools to django-admin.py.

05/14/09 16:56:05 changed by jammycakes

There are a couple of related problems here:

  1. It chokes on strings that contain a semicolon
  2. It also chokes when you're trying to define stored procedures, e.g. 
delimiter $$

create procedure DoSomething()
begin
   select count(*) from MyTable ;
end

$$

delimiter ;

Wouldn't a better solution be to spawn a separate process (mysql -e /path/to/sql/file) to handle the initial SQL as a batch than to try and reinvent the database engine's parser?

11/25/09 15:19:41 changed by anonymous

This seems to have been fixed in bug #6394.

Add/Change #4680 (Fixes problem in initial_sql where "--" is in a string)




Change Properties
Action