Constructing an SQL expression from three parameters

Question

I've been on stackoverflow looking for an alternative to the ugly if-elif-else structure shown below. The if-else structure takes three values as input and returns a pre-formatted string used for an SQL query. The code works right now, but it is difficult to explain, ugly, and I don't know a better way to do re-format it.

I need a solution that is more readable. I was thinking a dictionary object, but I can't wrap my mind how to implement one with the complex if/else structure I have below. An ideal solution will be more readable.

def _getQuery(activity,pollutant,feedstock):
 if feedstock.startswith('CG') and pollutant.startswith('VOC'):
 pass
 if feedstock == 'FR' and activity == 'Harvest':
 return 'FR'
 elif feedstock == 'FR':
 return 'No Activity'
 elif(activity == 'Fertilizer' and pollutant != 'NOx' and pollutant != 'NH3'):
 return 'No Activity'
 elif(activity == 'Chemical' and pollutant != 'VOC'):
 return 'No Activity' 
 elif( (feedstock == 'CS' or feedstock == 'WS')
 and 
 (activity == 'Non-Harvest' or 
 activity == 'Chemical')): 
 return 'No Activity'
 elif( ( ( pollutant.startswith('CO') or pollutant.startswith('SO') ) 
 and 
 ( activity == 'Non-Harvest' or 
 activity == 'Harvest' or 
 activity == 'Transport' ) 
 )
 or 
 ( pollutant.startswith('VOC') 
 and not
 ( feedstock.startswith('CG') or feedstock.startswith('SG') )
 )
 ): 
 rawTable = feedstock + '_raw'
 return """
 with
 activitySum as (select distinct fips, sum(%s) as x 
 from %s where description ilike '%s' group by fips),
 totalSum as (select distinct r.fips, sum(r.%s) as x 
 from %s r group by r.fips),
 ratios as (select t.fips, (a.x/t.x) as x from activitySum a, totalSum t 
 where a.fips = t.fips),
 maxRatio as (select r.fips, r.x as x from ratios r 
 group by r.fips, r.x order by x desc limit 1),
 minRatio as (select r.fips, r.x as x from ratios r 
 group by r.fips, r.x order by x asc limit 1)
 select mx.x, mn.x from maxRatio mx, minRatio mn;
 """ % (pollutant, rawTable, '%'+activity+'%',
 pollutant, rawTable)
 elif( (pollutant[0:2] == 'PM')
 and 
 (activity == 'Non-Harvest' or 
 activity == 'Harvest' or 
 activity == 'Transport')): 
 rawTable = feedstock + '_raw'
 return """
 with
 activitySum as (select distinct fips, (sum(%s) + sum(fug_%s)) as x 
 from %s where description ilike '%s' group by fips),
 totalSum as (select distinct r.fips, (sum(r.%s) + sum(r.fug_%s)) as x 
 from %s r group by r.fips),
 ratios as (select t.fips, (a.x/t.x) as x from activitySum a, totalSum t 
 where a.fips = t.fips),
 maxRatio as (select r.fips, r.x as x from ratios r 
 group by r.fips, r.x order by x desc limit 1),
 minRatio as (select r.fips, r.x as x from ratios r 
 group by r.fips, r.x order by x asc limit 1)
 select mx.x, mn.x from maxRatio mx, minRatio mn;
 """ % (pollutant, pollutant, rawTable, '%'+activity+'%',
 pollutant, pollutant, rawTable)
.
.
.
Lots more complex elif statements

• Stuart: I'm not sure it's possible to answer this without knowing more about the range of possible parameters and return values. It might be worth creating more variables with meaningful names that would help make clear why certain combinations of parameter values result in a particular output.
• nfisher: @Stuart, should I include the full if/elif structure in the question?
• Stuart: It might help if it's not terribly long. To start with, though, I would note that there is a lot of repetition in the long query string """with activitySum etc. which appears twice with only minor changes. Maybe see if you can use the if/elif structure to set values of variables with meaningful names which are then sanitised and fed into a single 'template' query string at the end.
• nfisher: @Stuart, thanks for pointing out the code duplicate, by removing the duplicates, the code is looking more reasonable. Thanks!
• Gareth Rees: It can't be right to return a SQL query in some cases, and the string 'No Activity' in others.

Answer 1

My thought would be to create a collection of objects that can be iterated over. Each object would have an interface like the following.

def _getQuery(activity,pollutant,feedstock):
 for matcher in MATCHERS:
 if matcher.matches(activity,pollutant,feedstock):
 return matcher.get_result(activity,pollutant,feedstock)
 raise Exception('Not matched')

Then you build up the set of matcher classes for the various cases and ad an instance of each to MATCHERS. Just remember that earlier elements in the list take precedence to later ones. So the most specific cases should be first in the list ans the most general cases should be at the end.

NOTE: Your string building code is vulnerable to SQL-injection. If you do go forward with manually building your selection strings, your input needs to be more thoroughly sanitized.

• nfisher: thanks for your comments, especially the part about SQL-injection! I was hoping for a more object-oriented approach and this seems quite reasonable!