1. Aah .
  2. sqlamp
Issue #2 resolved

[sqlamp] numchild, postgres, sqlachemy 0.6.x patch

created an issue

Hello Anton,

first thanks for sqlamp we are using it to implement a forum.

To support numchild (ho much child has a node) i've added numchild to youre code. While adding that pice of code i fixed some errors we had with Postgres and test.


In the attached file bench.txt is a results of a benchmark (test). I used SQLAlchemy 0.5.8 and 0.6.6 with Python2.4 and Python2.6 on Sqlite (file, memory), Postgres 8.4.6, MySQL 5.1.49 MYISAM. Ahh i used a MacBook Pro Ubuntu 10.10 64Bit Core2Duo 2,4GHz 4GB Ram to run this tests.


P.S. Sorry i just found now your public repo so i didn't used the fork function

Comments (4)

  1. Aah . repo owner
    • changed status to open

    Hi Josip,

    Thank you for your patches! The support of numchild is kind of controversial, so let's start from the obvious fixes.

    1. I committed the fix for MPOptions.order_by_clause(). It worked with SA 0.6.5 but has become broken with 0.6.6. Thanks for the patch. 2. Now test tables are dropped if it's needed. 3. I don't think it's good to change the signature of rebuild_all_trees() and imply session.commit() there. I emphasized possible locks in docs and changed FunctionalTestCase._corrupt_tree() to commit at the end. Now tests pass well with postgresql.

    Please review last four commits.

    The thing that I haven't yet checked is workaround for broken versions of sqlite and two integer parameters in query. I'll try to remember the version of sqlite3 which was broken for me...

    The numchild column is possibly the thing which I would deny because of performance concerns. I haven't yet run benchmarks but I suspect that adding this column may diminish advantages of cheap inserts in MP.

  2. lugensa reporter

    Hi Anton,

    thanks for the review of my patch.

    to 1.) Thanks. to 2.) Thanks. to 3.) Yes it was just a quick fix. It's better to do a commit outside.

    sqlite: we get this error with sqlite

    The numchild is one of our gretest needs in this patch. Changing the code in benchmark-tests.py form "node.mp.query_descendants().count()" to "node.mp_childnum" improves the read operation from 650 to 10000 queries per second ;) Look in sqlamp "bench_angri.txt" and sqlamp with numchild "bench_opt.txt". (I optimized your test to count() instead of len(...all()))

    It is right that insert operation are now slower but normally there should be more read than write operations in normal usage.

    I can also change the code to add a flag in MPManger to deactivate "mp_numchild".

  3. Aah . repo owner

    I applied a fix for sqlite 3.6.x. Still I wouldn't recommend using buggy versions of sqlite - such problem may (and will) bite you again somewhere else. And it could be not to easy to debug and realize that it is the bug in sqlite causes problems and not your code.

    Changing code in benchmarks from "select *" to "select count()" does not make much sense :) The main idea of benchmarks was to simulate a real-life activity, not to show big values :) I tried to avoid possible optimizations on DBMS side, thus benchmarks do real fetching and not only counting.

    About numchild: it would be great if it's possible to make it optional. Looking forward for a clean patch on top of vanila sqlamp. I'll close this issue, feel free to open a new one specifically for numchild.


  4. Log in to comment