SQL Function initial SQL data does not work.

[Julien GODIN]

I created a model and trying to add some functions to it :
Full trace :

Failed to install custom SQL for frontend.Alarm model: unterminated dollar-quoted string at or near "$$ BEGIN RETURN + 1;"
LINE 1: ... FUNCTION increment(i integer) RETURNS integer AS $$ BEGIN R...

Here is the function I am trying to create :
( stolen from here : ​http://www.postgresql.org/docs/9.1/static/sql-createfunction.html )

CREATE OR REPLACE FUNCTION increment(i integer) RETURNS integer AS $$
       BEGIN
                RETURN i + 1;
       END;
$$ LANGUAGE plpgsql;

The problem is in the _split_statements(content) function where it checks if the lines ends with ";" and close the statement if so.
The problem is that PLSQL functions needs ";" to work.

[Julien GODIN]

Potential patch

[Aymeric Augustin]

Hardcoding CREATE and LANGUAGE doesn't sound like a good solution. What if there's simply a ; at the end of a line in a multi-line string?

Parsing SQL with regex is worse than parsing HTML with regex... If we want to fix these edge cases, we must fix them with proper parsing, not hacks.

[Julien GODIN]

So, to provide a summary of the situation :

• No regex.
• parse the SQL properly.

Should we be using this ? :
​http://code.google.com/p/python-sqlparse/
as i do not see any other solution so far.

[Aymeric Augustin]

Actually sqlparse is here now: ​https://github.com/andialbrecht/sqlparse

It supports Python 2.5+ and 3.2+, which is good, and provides the exact feature we need:
​https://sqlparse.readthedocs.org/en/latest/api/?highlight=split#sqlparse.split

I'm +0 on introducing this as on optional dependency and +1 on deprecating the current regex-based logic and eventually turning it into a mandatory dependency when you want to load initial SQL (which most projects don't)!

[Anssi Kääriäinen]

+1 on deprecating regex SQL parsing. I don't think we should be maintaining any other half-baked solution to SQL parsing either. So, I guess that makes me +1 on requiring sqlparse for initial SQL.

[Julien GODIN]

Proposed a pull request here :

​https://github.com/django/django/pull/2529

It remove the regex technique and uses sqlparse.

[Tim Graham <timograham@…>]

In 071c9337750b296d198cced56f3ffad0e176afb6:

Fixed #22401 -- Deprecated regular expression parsing of initial SQL in favor of installing sqlparse.

[Tim Graham]

There's a regression here because sqlparse does not strip trailing comments the way the regex does (see #4680 for when this behavior was added). tests/initial_sql_regress/sql/simple.sql contains a trailing comment on line 2 and this causes MySQL to error out with "Commands out of sync; you can't run this command now". Jenkins doesn't have sqlparse installed, so it's still using the regex logic.

You can reproduce this behavior with the following code:

>>> from django.db import connection
>>> cursor = connection.cursor()
>>> cursor.execute("SELECT 1; --")
>>> cursor.execute("SELECT 2;")
Traceback ...
django.db.utils.ProgrammingError: (2014, "Commands out of sync; you can't run this command now")

I've realized that initial SQL data is deprecated (pending confirmation from Andrew Godwin in #22444) so I'm inclined just to revert this and mark it "won't fix".

[Tim Graham <timograham@…>]

In 47927eb786f432cb069f0b00fd810c465a78fd71:

Revert "Fixed #22401 -- Deprecated regular expression parsing of initial SQL in favor of installing sqlparse."

This reverts commit 071c9337750b296d198cced56f3ffad0e176afb6.

This introduced a regression on MySQL and custom SQL is deprecated.

[Aymeric Augustin]

Possible solution:

>>> import sqlparse
>>> sqlparse.split("""
...     SELECT 1; -- comment
...     SELECT 2;
... """)
['SELECT 1; -- comment', 'SELECT 2;', '']
>>> [sqlparse.format(stmt, strip_comments=True) for stmt in _ if stmt]
['SELECT 1;', 'SELECT 2;']

[Tim Graham]

Aymeric, you think it's worth addressing this even though the feature is deprecated?

[Claude Paroz]

This is really a duplicate of #3214.

[Aymeric Augustin <aymeric.augustin@…>]

In 8b5b199e20ad2d8d3e91873ce0cd5d3035e05ece:

Fixed #3214 -- Stopped parsing SQL with regex.

Avoided introducing a new regex-based SQL splitter in the migrations
framework, before we're bound by backwards compatibility.

Adapted this change to the legacy "initial SQL data" feature, even
though it's already deprecated, in order to facilitate the transition
to migrations.

sqlparse becomes mandatory for RunSQL on some databases (all but
PostgreSQL). There's no API to provide a single statement and tell
Django not to attempt splitting. Since we have a more robust splitting
implementation, that seems like a good tradeoff. It's easier to add a
new keyword argument later if necessary than to remove one.

Many people contributed to both tickets, thank you all, and especially
Claude for the review.

Refs #22401.

[Aymeric Augustin <aymeric.augustin@…>]

In 3bb0f118ca375f25cd0c03a5733ee2ef9d79dfa5:

[1.7.x] Fixed #3214 -- Stopped parsing SQL with regex.

Avoided introducing a new regex-based SQL splitter in the migrations
framework, before we're bound by backwards compatibility.

Adapted this change to the legacy "initial SQL data" feature, even
though it's already deprecated, in order to facilitate the transition
to migrations.

sqlparse becomes mandatory for RunSQL on some databases (all but
PostgreSQL). There's no API to provide a single statement and tell
Django not to attempt splitting. Since we have a more robust splitting
implementation, that seems like a good tradeoff. It's easier to add a
new keyword argument later if necessary than to remove one.

Many people contributed to both tickets, thank you all, and especially
Claude for the review.

Refs #22401.

Backport of 8b5b199 from master