Patch: literals in SQL replaced with parameters

[Vadim]

Hi,
I have refactored all code which access to sqlite. The most changes occured in dbwrapper.cpp.

Modified files:
M mmex\include\mmex.h
M mmex\include\util.h
M mmex\include\dbwrapper.h
M mmex\include\mmcoredb.h
M mmex\src\mmhomepagepanel.cpp
M mmex\src\assetdialog.cpp
M mmex\src\transdialog.cpp
M mmex\src\qifimport.cpp
M mmex\src\univcsvdialog.cpp
M mmex\src\mmtransaction.cpp
M mmex\src\mmcheckingpanel.cpp
M mmex\src\mmaccount.cpp
M mmex\src\dbwrapper.cpp
M mmex\src\stockspanel.cpp
M mmex\src\budgetingpanel.cpp
M mmex\src\budgetyearentrydialog.cpp
M mmex\src\billsdepositsdialog.cpp
M mmex\src\billsdepositspanel.cpp
M mmex\src\stockdialog.cpp
M mmex\src\newacctdialog.cpp
M mmex\src\mmex.cpp
M mmex\src\currencydialog.cpp
M mmex\src\budgetyeardialog.cpp
M mmex\src\filtertransdialog.cpp
M mmex\src\util.cpp
M mmex\src\assetspanel.cpp
M mmex\src\mmcurrency.cpp
M mmex\src\mmcoredb.cpp
M mmex\src\mmpayee.cpp


List of changes in headers:

1. util.h
These functions were deleted. Now you do not need these functions due to use of SQL parameters.

wxString mmCleanString(const wxString& orig);
wxString mmCleanQuotes(const wxString& orig);
wxString mmUnCleanString(const wxString& orig);

2. dbwrapper.h
Dummy class mmDBWrapper replaced by namespace mmDBWrapper.

3. mmex.h

mmCoreDB* core_ ==> boost::scoped_ptr<mmCoreDB> core_;
wxSQLite3Database* inidb_ ==> boost::scoped_ptr<wxSQLite3Database> inidb_;


Changes in sources

1. Literals in SQL replaced by SQL parameters. This is the main feature of this patch.

NEVER USE LITERALS INSTEAD OF PARAMETERS IN SQL!!!

This is the most common error and misunderstanding how someone should pass parameters to database.
This especially true for client-server databases (Oracle, etc). But sqlite has sql parameters support,
thus this feature is very important for sqlite too.

Functions mmCleanString\mmCleanQuotes\mmUnCleanString became unnesassary due to using parameters.

Why we must use parameters? When database engine executes SQL, one does following steps:
1.Parsing SQL statement.
2.Generating of execution plan for given statement.
3.SQL engine runs prepared execution plan.

SQL engine always have cache of already prepared SQL statements. When sql string matches with string in cache,
engine skips steps #1 & #2. Both these steps can take the same time as step #3.

When you pass to SQL engine strings with literals, almost every sql is unique. There are no cache hits.
Even worse, this flushes prepared statements from cache. But if you pass sql with parameters, the sql string does not change for each execution with different parameters values. Sql engine can find query in cache and execute one without performing first two steps.

2. General optimizations of code.

You should always select from query as much datas as possible.
Always join (if possible) tables in single query instead of executing innner SQL for every record of outer SQL.
This is terrible slow. Only really complex SQL could be better to organize by splitting into several queries.

// ------------

Due to lots of changes across all sources this patch requires goos testing.
I have carefully checked what I changed, but ...

Also I should mention it will be hard to merge these changes to trunk if head revision will go ahead significantly.

// ------------

Finally I recommend you forget about wxSQLite3StatementBuffer, Format, etc.
ALWAYS USE SQL WITH PARAMETERS. wxSQLite3StatementBuffer is completely useless class.

[Nikolay]

Hi Vadim,

Perfect job. I have compiled the program without problems. It's seems work, but in the first time the error appears.

After that works without problems.

[Vadim]

I didn't get any errors. Describe step by step how to reproduce this bug, please. Or give me the trace where (file\line) this error happened.

[Nikolay]

Hi Vadim,

Unfortunately, I do not know how to do tracing. But I have prepared DB. Could you, please, open uploaded DB? Is any errors?

DB deleted

[Vadim]

You should test debug build of mmex. The debug build provides much more checks for errors.
Cannot reproduce this bug. Try to apply patch and rebuild project from scratch in debug mode. I have updated archive.

[Vadim]

The bug has fixed. I've missed "," in SQL.

[elliswr]

I'll test tonight for mac.

[elliswr]

The patch failed for me on splittransactionsdialog.cpp

Code: Select all

patching file mmex/src/splittransactionsdialog.cpp
Hunk #1 FAILED at 84.
1 out of 1 hunk FAILED -- saving rejects to file mmex/src/splittransactionsdialog.cpp.rej

I am no svn expert, could you help me out on this one?

[elliswr]

I edited the code manually, but when I compile, I have 28 errors, most related to dbwrapper.cpp.

Example:

Code: Select all

static const std::string sql_base =

gives error: "Variable 'const std::string sql_base' has initializer but incomplete type".

and

Code: Select all

"from CHECKINGACCOUNT_V1 ca " + joinCURRENCYFORMATS("cf", "ca.ACCOUNTID");

gives error "Void value not ignored as it ought to be".

as well as numerous others of the same types of errors.

[Vadim]

Compiler doesn't see declaration of std::string. I've added #include <string> in dbwrapper.cpp & mmex.cpp. Archive updated.

[elliswr]

Lost 22 of the 28 errors. Now only six remain. All related to st.Bind(i++, numRepeats) or st.Bind(2, numRepeats);

Error is "Call of overloaded 'Bind(int, long int&)' is ambiguous in dbwrapper.cpp / bills depositsdialog.cpp"

[Vadim]

Fixed, archive updated.

[elliswr]

What was the cause of the bug?

I'll check updated archive tonight.

[Vadim]

That was not a bug. It's just a different compiler
Signatures of Bind are
Bind(int, int)
Bind(int, wxLongLong).

On Win32 sizeof(long) == sizeof(int), thus call to Bind(int, long) matches Bind(int, int).
On Mac call to Bind(int, long) may be matches Bind(int, int) and Bind(int, wxLongLong).
You can check yourself

[elliswr]

Vadim,
Works perfectly! And thanks for taking the time to explain the reason I was getting errors. You clearly have a better understanding of all this than I do, and I hope you don't mind if I come to you when I get stuck.

[madhan]

Hello Vadim, I think this is quite a significant patch to MMEX and I welcome the change as well as the reasons to do it.
MMEX started when I was working with databases the first time so naturally the code is crufty and definitely has room for improvement.

Having said that, I think the best way to get this patch is not me managing writing these changes in SVN and ensuring the fixes go in related to changes to this patch, which will get fairly inefficient and laborious.
It might be better if you or other people can be added to the MMEX project as contributors with SVN write access. I will send email to discuss this in greater detail.

[Vadim]

Patch applied, revision 410.