Database Engine: escaping table field names

[gibbonweb]

I just ran into an issue where my queries on one table would always fail, although they were managed through an SQLBean. Turns out I had a field in my table called 'as' which produced SQL whenever mentioned. This could be fixed by using backticks around the field names like so:

SELECT `id`, `as` FROM mytable WHERE `id` = 2;

Instead of:

SELECT id, as FROM mytable WHERE id = 2;

...which would produce an error due to incorrect use of the keyword AS.

While you could argue that naming a field 'AS' is bad practice due to exactly this kind of possible confusion, 'AS' in this case stood for a german word with 15 letters in a table with 10 similar fields, which would be horrible to write out or display as a whole table... What do you think? Would this have to be implemented in the MysqlPDOEngine, or even in the PDOEngine? I guess it could be solved by just wrapping field names with backticks anywhere they appear, but I'm not sure wether all supported database engines handle field name escaping the same way?

[TomFrost]

Excellent find! This addition should go in StandardSQLFormatter.php -- very very simple fix. I'll check it out later today.

[gibbonweb]

Ok, now that I know WHERE to put it, I'll look into it in the next few days - if you haven't already fixed it by then ;-)

[gibbonweb]

I have fixed this problem locally and would like to post a pull request; since somehow our fork histories have diverged a bit, this is kinda ugly... All my random rebasing attempts have failed - how do I create a branch which is based on YOUR HEAD, into which I could write the fix and send it back to you? Currently, any pull request I could post would contain a bunch of my idiotic commits I'd like to disappear ;) cheers, Git Noob.

[gibbonweb]

OK as you can see, I somehow managed to do it myself ;)

[hamador]

This solution actually ends up breaking some of the SQLBeans functionality (i.e. mapping) and limiting what can be done with query. Might I suggest removing the escapes from the Query class, as anyone that is using query directly should be capable escaping their own possibly offending fields, and rethink a solution to be added to SQLBeans.

[gibbonweb]

Wouldn't it be better to just add proper escaping support to SQLBeans?

[hamador]

At the moment all fields sent to Query are escaped as a whole meaning that table.field syntax would not work. The only way to join/map tables with ambiguous field names is to use that syntax. Now there are a few options that come to mind:

Tell Query not to escape a field, either through an argument or separate function Tell Query when you are using table.field syntax so it can escape accordingly Always use table.field syntax in Query, removing that burden from SQLBeans, and thus allowing you to code the escape accordingly

There are other things to take into account, before the escaping I could use Query to execute this without any special considerations :

SELECT COUNT(DISTINCT source_id) FROM charges WHERE CONVERT(VARCHAR,create_timestamp,105) = CONVERT(VARCHAR,getdate(),105)

When you escape fields at the Query level it becomes impossible to execute that through the Query class and would require a standard PDO Statement. Now we could build in COUNT,CONVERT, ect, functions into Query and the formatters but that brings to light a whole new problem of Nesting these functions to run queries like :

SELECT COALESCE(CONVERT(VARCHAR,modify_timestamp,105),CONVERT(VARCHAR,create_timestamp,105))

In my opinion leaving the freedom in Query to do more complex statements would be more ideal. Anyone with an offending field name in their tables could simply escape when calling $query->field() or in the fields array of the SQLBean (ie $fields = array( 'as','distinct'); ) if a solution outside of Query cannot be found, like say adding the escaping ot all of the members of the $fields array in the loop inside the select function of SQLBeans

[hamador]

Removed the escape from SQLFormater, and added it to SQLBeans here: https://github.com/hamador/Hydrogen/tree/DatabaseChanges If you care to take a look at how that would work, I feel that may be a more acceptable solution.