Patch: literals in SQL replaced with parameters

Money Manager Ex Development related posts
(http://github.moneymanagerex.org/)
Vadim
MMEX Developer
Posts: 142
Joined: Mon Aug 03, 2009 7:35 am
Are you a spam bot?: No

Patch: literals in SQL replaced with parameters

Post by 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.
Attachments
diffs.zip
(35.66 KiB) Downloaded 313 times
Last edited by Vadim on Thu Sep 10, 2009 11:57 am, edited 4 times in total.

Nikolay
MMEX Developer
Posts: 1287
Joined: Sat Dec 06, 2008 2:27 pm
Are you a spam bot?: No
Location: Sankt-Petersburg, Russia

Re: Patch: literals in SQL replaced with parameters

Post by 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.
Attachments
error.png
(11.61 KiB) Downloaded 2341 times

Vadim
MMEX Developer
Posts: 142
Joined: Mon Aug 03, 2009 7:35 am
Are you a spam bot?: No

Re: Patch: literals in SQL replaced with parameters

Post by 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
MMEX Developer
Posts: 1287
Joined: Sat Dec 06, 2008 2:27 pm
Are you a spam bot?: No
Location: Sankt-Petersburg, Russia

Re: Patch: literals in SQL replaced with parameters

Post by 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
MMEX Developer
Posts: 142
Joined: Mon Aug 03, 2009 7:35 am
Are you a spam bot?: No

Re: Patch: literals in SQL replaced with parameters

Post by 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
MMEX Developer
Posts: 142
Joined: Mon Aug 03, 2009 7:35 am
Are you a spam bot?: No

Re: Patch: literals in SQL replaced with parameters

Post by Vadim »

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

elliswr
MMEX Developer
Posts: 102
Joined: Tue May 05, 2009 8:21 am
Are you a spam bot?: No
Location: Granbury, TX
Contact:

Re: Patch: literals in SQL replaced with parameters

Post by elliswr »

I'll test tonight for mac.

elliswr
MMEX Developer
Posts: 102
Joined: Tue May 05, 2009 8:21 am
Are you a spam bot?: No
Location: Granbury, TX
Contact:

Re: Patch: literals in SQL replaced with parameters

Post by 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
MMEX Developer
Posts: 102
Joined: Tue May 05, 2009 8:21 am
Are you a spam bot?: No
Location: Granbury, TX
Contact:

Re: Patch: literals in SQL replaced with parameters

Post by 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
MMEX Developer
Posts: 142
Joined: Mon Aug 03, 2009 7:35 am
Are you a spam bot?: No

Re: Patch: literals in SQL replaced with parameters

Post by Vadim »

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

Post Reply