Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Nibrs agency api #521

Merged
merged 6 commits into from
May 22, 2017
Merged

Nibrs agency api #521

merged 6 commits into from
May 22, 2017

Conversation

cacraig
Copy link
Contributor

@cacraig cacraig commented May 22, 2017

This fixes some major performance issues in production with the NIBRS API's. The indexes and such are already live.

@cacraig
Copy link
Contributor Author

cacraig commented May 22, 2017

@harrisj - I can't seem to tag you as the reviewer on this for some reason. When you get a chance, can you take a look at this? Thanks!

Copy link
Contributor

@harrisj harrisj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have a few questions so I can understand what is going on, but looking good. Given the performance issues, I think I will merge it, but I'd like to understand why the 2012 tables are being used.

@@ -1015,7 +1015,7 @@ def get_field_table(self, field):
return ('ref_race','race_code')

if field in ['sex_code']:
return ('nibrs_victim_denorm_2012','sex_code')
return ('victim_counts_2012','sex_code')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason why this is pegged to a single year of materialized data?

@@ -1083,9 +1083,9 @@ def base_query(self, field):
join_table,join_field = self.get_field_table(field)
if join_field:
if self.year:
query_gap_fill = ' RIGHT JOIN (SELECT DISTINCT ' + join_table + '.' + join_field + ' AS :field, c.year from ' + join_table + ' RIGHT JOIN (SELECT year::text, :view_name.:field from :view_name ' + inner_where_query + ' GROUP BY year,:view_name.:field ORDER BY year DESC) c ON (c.:field = ' + join_table +'.' + join_field + ')) b ON (a.:field = b.:field)'
query_gap_fill = ' RIGHT JOIN (SELECT DISTINCT ' + join_table + '.' + join_field + ' AS :field, c.year from ' + join_table + ' CROSS JOIN (SELECT year::text from nibrs_years WHERE year::int = :year) c) b ON (a.:field = b.:field)'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sing to the tune of Trogdor: "CROSS JOIN! CROSS JOIN! Combinating the tables! Combinating the fields!"

Seriously, this is interesting, but I think I would like if you could use SqlAlchemy's interpolation (I think there are some cases where you can say "Unescape this field") since I worry this would trip off SQL injection alerts?

@harrisj harrisj merged commit a1dec9e into master May 22, 2017
@harrisj harrisj deleted the nibrs-agency-api branch May 22, 2017 21:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants