appDB: Nice URLs live demo and code review

[John Klehm]

On Mon, Apr 7, 2008 at 4:41 PM, Chris Morgan <chmorgan at gmail.com> wrote:
> Hey John.
>
>  Looks like you've gotten the nice urls working, great job man. It will
>  be great to have this before 1.0 is shipped since it will make our
>  urls a lot nicer looking.
>
>  I had a few comments about the code changes. You don't follow the
>  variable naming convention of the appdb. We have some prefixes for
>  variables that we use to keep the types of variables straight, you can
>  find these in the coding standards file in the root of the appdb tree.
>

Right, oops.

>  The code in 0005-matchByVerbAndTerm.php-view-that-calls-into-the.patch.txt
>  looks like it is using query_appdb() to perform a direct query. The
>  indexing code also appears to do this. We prefer using the query
>  wrapper function query_parameters() as this performs escaping of input
>  parameters and helps to reduce the possibility of sql injection
>  vulnerabilities.
>

Well I can certainly use that.  For the params I do call
appdb_escape_query => calls mysql_real_escape_string which I had
thought was the "your injects stop here" escaping func =P

>  I noticed the code to perform the grouped inserts. What is the
>  performance difference between perform the inserts individually vs.
>  grouped together? If we run the indexer daily and its relatively quick
>  we may be able to drop the complexity of worrying about how many
>  inserts we can group together.
>
Well the runIndexer.php is just a one time thing to create the intial
index. If it were run twice there would be doubles of every previous
entry.  Once we have it for the existing data any new data can be
indexed when its state becomes 'accepted' or however the add new app
flow goes.  I have not implemented the "add index values on data
creation" functionality yet though.

Regarding multiple inserts I think this is what was in my head when I
wrote that:

http://dev.mysql.com/doc/refman/5.0/en/insert-speed.html
Right where it reads "You can use the following methods to speed up inserts"

If the multiple inserts are a concern its easy to run them one at a
time just set the config switch APPDB_MYSQL_MAX_INSERTS_PER_QUERY to 1
in the config.php file.

>  0005-matchByVerbAndTerm.php-view-that-calls-into-the.patch.txt appears
>  to be generating output for display. It would be useful to have some
>  comments at the head of the file to describe what the file is used for
>  and why it would need to conditionally output lists when don't find a
>  object page match.
>

Sure Ill write out the code paths in comments at the top, that file
does do a bit.

Thanks for the feedback! =))

John