822179 - MySQL-compatible rendering of sqlalchemy query expressions

Status: RESOLVED FIXED

(Cloud Services Graveyard :: Server: Sync, defect)

Description

[Ryan Kelly [:rfkelly]]

Attachment: patch to use a backend-specific dialect when rendering queries

The sync2.0 codebase does some fiddly things with sqlalchemy query expressions, including rendering them into a string so that we can add a /* queryName=BLAH */ comment to each query.  I uncovered a problem in the current rendering logic, whereby it can sometimes render SQL syntax that is not valid for MySQL.

To avoid this we need to use a backend-specific Dialect object when rendering the expression to a string.  But this in turn can break other parts of the code, by mucking with the formatting of bind parameter names.

The attached patch implements the best compromise I could find - use a backend-specific Dialect object, but force it to render bindparams in "named" format as expected by other parts of the code.  It's pretty hacky...but that puts it in good company with the rest of the sqlalchemy-related code.

Comment 1

[Toby Elliott [:telliott]]

Comment on attachment 692861 [details] [diff] [review]
patch to use a backend-specific dialect when rendering queries

This is horrible. I mean, it's fine, but the level of indirection we're hitting for relatively simple SQL just ensures we're going to have maintenance and comprehension issues in the long run :P

Comment 2

[Ryan Kelly [:rfkelly]]

https://github.com/mozilla-services/server-syncstorage/commit/e4108784d7ca51252703c2e465b9b5f6b120af0d