fix: properly quote aliases - small additional fixes

projectcaluma · Dec 15, 2022 · 535c703 · 535c703
1 parent 43c7ffc
commit 535c703
Show file tree

Hide file tree

Showing 3 changed files with 36 additions and 25 deletions.
diff --git 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
@@ -7,18 +7,26 @@
 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
 
     suffix = hashlib.md5(str(value).encode("utf-8")).hexdigest()
     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
@@ -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