From 535c7030a019672ce72d2db8efe532cfab492125 Mon Sep 17 00:00:00 2001 From: Fabio Ambauen Date: Thu, 15 Dec 2022 12:29:08 +0100 Subject: [PATCH] fix: properly quote aliases - small additional fixes --- caluma/caluma_analytics/simple_table.py | 6 +++- caluma/caluma_analytics/sql.py | 27 +++++++++++------- .../test_run_analytics_cmdline.ambr | 28 +++++++++---------- 3 files changed, 36 insertions(+), 25 deletions(-) diff --git a/caluma/caluma_analytics/simple_table.py b/caluma/caluma_analytics/simple_table.py index 56664babe..ff921b0d5 100644 --- a/caluma/caluma_analytics/simple_table.py +++ b/caluma/caluma_analytics/simple_table.py @@ -1155,7 +1155,11 @@ def _fields(self): @cached_property def field_ordering(self): - return list(self.table.fields.all().values_list("alias", flat=True)) + return list( + self.table.fields.all() + .filter(show_output=True) + .values_list("alias", flat=True) + ) def get_sql_and_params(self): """Return a list of records as specified in the given table config.""" diff --git a/caluma/caluma_analytics/sql.py b/caluma/caluma_analytics/sql.py index 399e10bf9..61c837e1c 100644 --- a/caluma/caluma_analytics/sql.py +++ b/caluma/caluma_analytics/sql.py @@ -7,11 +7,14 @@ from typing import Dict, List, Optional, Tuple, Union from uuid import uuid4 +import psycopg2.sql from django.db import connection from django.utils.text import slugify def _make_name(value, name_hint=None): + if name_hint: + name_hint = re.sub(r"[^a-zA-Z_]", "_", name_hint) prefix = name_hint if name_hint else "p" length = 5 if name_hint else 12 @@ -19,6 +22,11 @@ def _make_name(value, name_hint=None): return f"{prefix}_{suffix[:length]}" +def quote_identifier(name): + name = name.replace("%", "%%") + return psycopg2.sql.Identifier(name).as_string(connection.connection) + + @dataclass class Query: """Represent an SQL query (might be a subquery). @@ -109,7 +117,7 @@ def add_field_filter(self, alias, filter_values): filters_sql = ", ".join( [self.makeparam(val, f"flt_{alias}") for val in filter_values] ) - self.filters.append(f"{alias} IN ({filters_sql})") + self.filters.append(f"{quote_identifier(alias)} IN ({filters_sql})") @classmethod def from_queryset(cls, queryset): @@ -123,12 +131,11 @@ def from_queryset(cls, queryset): # we're not interested in, as we're JOINing our stuff by ourselves. # To avoid ambiguity, we select ONLY what's in the main model's # field list - q = connection.ops.quote_name field_list = ", \n".join( [ - q(queryset.model._meta.db_table) + quote_identifier(queryset.model._meta.db_table) + "." - + q(field.db_column or field.attname) + + quote_identifier(field.db_column or field.attname) for field in queryset.model._meta.concrete_fields ] ) @@ -217,7 +224,7 @@ class DateExprField(AttrField): extract_part: Optional[str] = field(default=None) def expr(self, query): - q_id = connection.ops.quote_name(self.extract) + q_id = quote_identifier(self.extract) return f"EXTRACT({self.extract_part} FROM {q_id})" @@ -227,7 +234,7 @@ class JSONExtractorField(AttrField): def expr(self, query): key_param = query.makeparam(self.json_key) - q_id = connection.ops.quote_name(self.extract) + q_id = quote_identifier(self.extract) self_alias = query.self_alias() # Extract text from JSON field, so that it comes from the DB # as actual text @@ -241,7 +248,7 @@ class HStoreExtractorField(AttrField): def expr(self, query): key_param = query.makeparam(self.hstore_key) - q_id = connection.ops.quote_name(self.extract) + q_id = quote_identifier(self.extract) self_alias = query.self_alias() # Extract text from HStore field, so that it comes from the DB # as actual text @@ -402,18 +409,18 @@ def _field_list(self): # we always need to select everything, even from subqueries fields = [ # First, the direct fields - f'''{expr} AS "{alias}"''' + f"""{expr} AS {quote_identifier(alias)}""" for expr, alias in self.query.select ] if self.query.outer_ref: _, inner = self.query.outer_ref - fields.append(f'"{inner}"') + fields.append(f"{quote_identifier(inner)}") # The subquery fields are already aliased, so no need # to re-alias them def _collect(q): for _, alias in q.select: - yield f'"{alias}"' + yield f"{quote_identifier(alias)}" for sub_q, _ in q.joins: yield from _collect(sub_q) diff --git a/caluma/caluma_analytics/tests/__snapshots__/test_run_analytics_cmdline.ambr b/caluma/caluma_analytics/tests/__snapshots__/test_run_analytics_cmdline.ambr index b602da1e5..df127dfa0 100644 --- a/caluma/caluma_analytics/tests/__snapshots__/test_run_analytics_cmdline.ambr +++ b/caluma/caluma_analytics/tests/__snapshots__/test_run_analytics_cmdline.ambr @@ -98,15 +98,15 @@ ORDER BY document_id - ) AS "answer_af542_caea5" ON (document_2a07e.id = "answer_af542_caea5".document_id) + ) AS "answer_af____caea5" ON (document_2a07e.id = "answer_af____caea5".document_id) WHERE form_id = 'top_form' ORDER BY id - ) AS "document_2a07e_27f15" ON (case_ac50e.document_id = "document_2a07e_27f15".id) + ) AS "document__a__e_27f15" ON (case_ac50e.document_id = "document__a__e_27f15".id) - ) AS analytics_2a8de + ) AS analytics_5e2b5 -- PARAMS: ''', @@ -188,16 +188,16 @@ ORDER BY document_id - ) AS "answer_af542_caea5" ON (document_2a07e.id = "answer_af542_caea5".document_id) + ) AS "answer_af____caea5" ON (document_2a07e.id = "answer_af____caea5".document_id) WHERE form_id = 'top_form' ORDER BY id - ) AS "document_2a07e_27f15" ON (case_ac50e.document_id = "document_2a07e_27f15".id) + ) AS "document__a__e_27f15" ON (case_ac50e.document_id = "document__a__e_27f15".id) - ) AS analytics_2a8de - WHERE analytics_result_blablub IN (%(flt_analytics_result_blablub_8201d)s, %(flt_analytics_result_blablub_8e5e6)s) + ) AS analytics_5e2b5 + WHERE "analytics_result_blablub" IN (%(flt_analytics_result_blablub_8201d)s, %(flt_analytics_result_blablub_8e5e6)s) -- PARAMS: -- flt_analytics_result_blablub_8201d: Shelly Watson -- flt_analytics_result_blablub_8e5e6: Daniel Stewart @@ -277,15 +277,15 @@ ORDER BY document_id - ) AS "answer_af542_caea5" ON (document_2a07e.id = "answer_af542_caea5".document_id) + ) AS "answer_af____caea5" ON (document_2a07e.id = "answer_af____caea5".document_id) WHERE form_id = 'top_form' ORDER BY id - ) AS "document_2a07e_27f15" ON (case_ac50e.document_id = "document_2a07e_27f15".id) + ) AS "document__a__e_27f15" ON (case_ac50e.document_id = "document__a__e_27f15".id) - ) AS analytics_2a8de + ) AS analytics_5e2b5 -- PARAMS: ''', @@ -363,16 +363,16 @@ ORDER BY document_id - ) AS "answer_af542_caea5" ON (document_2a07e.id = "answer_af542_caea5".document_id) + ) AS "answer_af____caea5" ON (document_2a07e.id = "answer_af____caea5".document_id) WHERE form_id = 'top_form' ORDER BY id - ) AS "document_2a07e_27f15" ON (case_ac50e.document_id = "document_2a07e_27f15".id) + ) AS "document__a__e_27f15" ON (case_ac50e.document_id = "document__a__e_27f15".id) - ) AS analytics_2a8de - WHERE analytics_result_blablub IN (%(flt_analytics_result_blablub_8201d)s, %(flt_analytics_result_blablub_8e5e6)s) + ) AS analytics_5e2b5 + WHERE "analytics_result_blablub" IN (%(flt_analytics_result_blablub_8201d)s, %(flt_analytics_result_blablub_8e5e6)s) -- PARAMS: -- flt_analytics_result_blablub_8201d: Shelly Watson -- flt_analytics_result_blablub_8e5e6: Daniel Stewart