diff --git a/cumulus_library/.sqlfluff b/cumulus_library/.sqlfluff index 5288bf9c..460d1cc2 100644 --- a/cumulus_library/.sqlfluff +++ b/cumulus_library/.sqlfluff @@ -17,7 +17,7 @@ capitalisation_policy = upper [sqlfluff:templater:jinja] -load_macros_from_path = cumulus_library/studies/core/core_templates +load_macros_from_path = cumulus_library/template_sql/shared_macros,cumulus_library/studies/core/core_templates [sqlfluff:templater:jinja:context] code_systems = ["http://snomed.info/sct", "http://hl7.org/fhir/sid/icd-10-cm"] diff --git a/cumulus_library/statistics/counts.py b/cumulus_library/statistics/counts.py index 0d0707f9..1b7f3a81 100644 --- a/cumulus_library/statistics/counts.py +++ b/cumulus_library/statistics/counts.py @@ -5,8 +5,8 @@ from cumulus_library.base_table_builder import BaseTableBuilder from cumulus_library.errors import CountsBuilderError +from cumulus_library.statistics.statistics_templates import counts_templates from cumulus_library.study_parser import StudyManifestParser -from cumulus_library.template_sql.statistics import counts_templates class CountsBuilder(BaseTableBuilder): @@ -71,7 +71,7 @@ def get_count_query( :keyword min_subject: An integer setting the minimum bin size for inclusion (default: 10) :keyword fhir_resource: The type of FHIR resource to count (see - template_sql/templates.statistics.CountableFhirResource) + statistics/statistics_templates/count_templates.CountableFhirResource) """ if not table_name or not source_table or not table_cols: raise CountsBuilderError( diff --git a/cumulus_library/statistics/psm.py b/cumulus_library/statistics/psm.py index 0e05b223..689f1d5b 100644 --- a/cumulus_library/statistics/psm.py +++ b/cumulus_library/statistics/psm.py @@ -17,8 +17,8 @@ from psmpy.functions import cohenD from cumulus_library import base_table_builder, databases +from cumulus_library.statistics.statistics_templates import psm_templates from cumulus_library.template_sql import base_templates -from cumulus_library.template_sql.statistics import psm_templates @dataclass diff --git a/cumulus_library/template_sql/statistics/count.sql.jinja b/cumulus_library/statistics/statistics_templates/count.sql.jinja similarity index 96% rename from cumulus_library/template_sql/statistics/count.sql.jinja rename to cumulus_library/statistics/statistics_templates/count.sql.jinja index 72b98877..8be45de2 100644 --- a/cumulus_library/template_sql/statistics/count.sql.jinja +++ b/cumulus_library/statistics/statistics_templates/count.sql.jinja @@ -1,9 +1,4 @@ -{%- macro comma_delineate(loop) -%} -{%- if not loop.last -%} -, -{%- endif -%} -{%- endmacro -%} - +{%- import 'syntax.sql.jinja' as syntax -%} {#- TODO: move secondary/tertiary resource declaration upstream to statistics/counts.py level -#} {%- macro secondary_resource(fhir_resource) -%} @@ -30,7 +25,7 @@ logic at the statistics/counts.py level -#} {%- macro cols_delineated_list(table_cols, fhir_resource)-%} {% for col in table_cols %} "{{ column_or_alias(col) }}" - {{- comma_delineate(loop) }} + {{- syntax.comma_delineate(loop) }} {%- endfor %} {%- if fhir_resource in ('documentreference', 'observation' ) -%}, enc_class_display @@ -42,7 +37,7 @@ which will drop nulls, resulting in potentially non-unique keys -#} {%- macro cols_coalesced_list(table_cols, fhir_resource)-%} {% for col in table_cols %} COALESCE("{{ column_or_alias(col) }}",'') - {{- comma_delineate(loop) }} + {{- syntax.comma_delineate(loop) }} {%- endfor %} {%- if fhir_resource in ('documentreference', 'observation' ) -%}, COALESCE(enc_class_display,'') @@ -90,7 +85,7 @@ CREATE TABLE {{ table_name }} AS ( {%- else %} s."{{ col.name }}" AS {{ col.alias }} {%- endif -%} - {{- comma_delineate(loop) -}} + {{- syntax.comma_delineate(loop) -}} {% endfor %} --noqa: enable=RF03, AL02 FROM {{ source_table }} AS s @@ -137,7 +132,7 @@ CREATE TABLE {{ table_name }} AS ( cast({{ column_or_alias(col) }} AS varchar), '{{ missing_null }}' ) AS {{ column_or_alias(col) }} - {{- comma_delineate(loop) }} + {{- syntax.comma_delineate(loop) }} {%- endfor -%} {%- if filter_resource %} FROM filtered_table @@ -205,7 +200,7 @@ CREATE TABLE {{ table_name }} AS ( {%- endif %} {%- for col in table_cols %} p."{{ column_or_alias(col) }}" - {{- comma_delineate(loop) }} + {{- syntax.comma_delineate(loop) }} {%- endfor %} {%- if fhir_resource in ('documentreference', 'observation' ) -%}, p.enc_class_display diff --git a/cumulus_library/template_sql/statistics/counts_templates.py b/cumulus_library/statistics/statistics_templates/counts_templates.py similarity index 77% rename from cumulus_library/template_sql/statistics/counts_templates.py rename to cumulus_library/statistics/statistics_templates/counts_templates.py index c32200d5..e743b029 100644 --- a/cumulus_library/template_sql/statistics/counts_templates.py +++ b/cumulus_library/statistics/statistics_templates/counts_templates.py @@ -2,9 +2,8 @@ from enum import Enum from pathlib import Path -from jinja2 import Template - from cumulus_library.errors import CountsBuilderError +from cumulus_library.template_sql import base_templates class CountableFhirResource(Enum): @@ -62,15 +61,16 @@ def get_count_query( ) table_cols = table_col_classed - with open(f"{path}/count.sql.jinja") as count_query: - query = Template(count_query.read()).render( - table_name=table_name, - source_table=source_table, - table_cols=table_cols, - min_subject=min_subject, - where_clauses=where_clauses, - fhir_resource=fhir_resource, - filter_resource=filter_resource, - ) - # workaround for conflicting sqlfluff enforcement - return query.replace("-- noqa: disable=LT02\n", "") + query = base_templates.get_base_template( + "count", + path, + table_name=table_name, + source_table=source_table, + table_cols=table_cols, + min_subject=min_subject, + where_clauses=where_clauses, + fhir_resource=fhir_resource, + filter_resource=filter_resource, + ) + # workaround for conflicting sqlfluff enforcement + return query.replace("-- noqa: disable=LT02\n", "") diff --git a/cumulus_library/template_sql/statistics/psm_create_covariate_table.sql.jinja b/cumulus_library/statistics/statistics_templates/psm_create_covariate_table.sql.jinja similarity index 100% rename from cumulus_library/template_sql/statistics/psm_create_covariate_table.sql.jinja rename to cumulus_library/statistics/statistics_templates/psm_create_covariate_table.sql.jinja diff --git a/cumulus_library/template_sql/statistics/psm_distinct_ids.sql.jinja b/cumulus_library/statistics/statistics_templates/psm_distinct_ids.sql.jinja similarity index 100% rename from cumulus_library/template_sql/statistics/psm_distinct_ids.sql.jinja rename to cumulus_library/statistics/statistics_templates/psm_distinct_ids.sql.jinja diff --git a/cumulus_library/template_sql/statistics/psm_templates.py b/cumulus_library/statistics/statistics_templates/psm_templates.py similarity index 73% rename from cumulus_library/template_sql/statistics/psm_templates.py rename to cumulus_library/statistics/statistics_templates/psm_templates.py index a91f5bb9..3dae3ff0 100644 --- a/cumulus_library/template_sql/statistics/psm_templates.py +++ b/cumulus_library/statistics/statistics_templates/psm_templates.py @@ -2,9 +2,8 @@ from pathlib import Path -from jinja2 import Template - from cumulus_library.errors import CumulusLibraryError +from cumulus_library.template_sql import base_templates def get_distinct_ids( @@ -33,14 +32,14 @@ def get_distinct_ids( "to be defined if supplied", ) - path = Path(__file__).parent - with open(f"{path}/psm_distinct_ids.sql.jinja") as distinct_ids: - return Template(distinct_ids.read()).render( - columns=columns, - source_table=source_table, - join_id=join_id, - filter_table=filter_table, - ) + return base_templates.get_base_template( + "psm_distinct_ids", + Path(__file__).parent, + columns=columns, + source_table=source_table, + join_id=join_id, + filter_table=filter_table, + ) def get_create_covariate_table( @@ -74,16 +73,16 @@ def get_create_covariate_table( "to be defined if supplied", ) - path = Path(__file__).parent - with open(f"{path}/psm_create_covariate_table.sql.jinja") as create_covariate_table: - return Template(create_covariate_table.read()).render( - target_table=target_table, - pos_source_table=pos_source_table, - neg_source_table=neg_source_table, - table_suffix=table_suffix, - primary_ref=primary_ref, - dependent_variable=dependent_variable, - count_ref=count_ref, - count_table=count_table, - join_cols_by_table=join_cols_by_table, - ) + return base_templates.get_base_template( + "psm_create_covariate_table", + Path(__file__).parent, + target_table=target_table, + pos_source_table=pos_source_table, + neg_source_table=neg_source_table, + table_suffix=table_suffix, + primary_ref=primary_ref, + dependent_variable=dependent_variable, + count_ref=count_ref, + count_table=count_table, + join_cols_by_table=join_cols_by_table, + ) diff --git a/cumulus_library/studies/core/core_templates/core_templates.py b/cumulus_library/studies/core/core_templates/core_templates.py index 0c0e7562..c946a960 100644 --- a/cumulus_library/studies/core/core_templates/core_templates.py +++ b/cumulus_library/studies/core/core_templates/core_templates.py @@ -1,7 +1,5 @@ import pathlib -import jinja2 - from cumulus_library.template_sql import base_templates PATH = pathlib.Path(__file__).parent @@ -13,11 +11,9 @@ def get_core_template( config: dict | None = None, ) -> str: """Extracts code system details as a standalone table""" - with open(f"{PATH}/{target_table}.sql.jinja") as file: - core_template = file.read() - loader = jinja2.FileSystemLoader(PATH) - env = jinja2.Environment(loader=loader).from_string(core_template) - return env.render(schema=schema, config=config) + return base_templates.get_base_template( + target_table, path=pathlib.Path(__file__).parent, schema=schema, config=config + ) def validate_schema(cursor: object, schema: str, expected_table_cols, parser): diff --git a/cumulus_library/studies/core/core_templates/core_utils.jinja b/cumulus_library/studies/core/core_templates/core_utils.jinja index 5b711286..d312e852 100644 --- a/cumulus_library/studies/core/core_templates/core_utils.jinja +++ b/cumulus_library/studies/core/core_templates/core_utils.jinja @@ -1,3 +1,4 @@ +{% import 'syntax.sql.jinja' as syntax %} {#- A collection of macros for constructing common safe column selects For layout, is is generally assumed that these are being invoked from within @@ -9,7 +10,7 @@ {% macro basic_cols(table, alias, targets) %} {%- for col in targets %} {{ alias }}.{{ col }} - {{- comma_delineate(loop) }} + {{- syntax.comma_delineate(loop) }} {%- endfor %} {%- endmacro %} @@ -43,7 +44,7 @@ targets is an array expecting a data type of the following: {%- else %} NULL AS {{ col }} {%- endif %} - {{- comma_delineate(loop) }} + {{- syntax.comma_delineate(loop) }} {%- endfor %} {%- endmacro %} @@ -75,7 +76,7 @@ targets is an array expecting a data type of the following: {%- else %} cast(NULL AS date) AS {{ col.lower() }} {%- endif %} - {{- comma_delineate(loop) }} + {{- syntax.comma_delineate(loop) }} {%- endfor %} {%- endmacro %} @@ -110,16 +111,7 @@ targets is assumed to be a list of tuples of one of the following format: {%- else %} cast(NULL AS date) AS {{ col[0] }}_{{ col[1] }} {%- endif %} - {{- comma_delineate(loop) }} + {{- syntax.comma_delineate(loop) }} {%- endfor %} {%- endmacro %} -{#- convenience function for looping over items - -TODO: this should move to some kind of global jinja utils location -#} -{%- macro comma_delineate(loop) -%} -{%- if not loop.last -%} -, -{%- endif -%} -{%- endmacro -%} - diff --git a/cumulus_library/template_sql/base_templates.py b/cumulus_library/template_sql/base_templates.py index 8d22fd86..7314e890 100644 --- a/cumulus_library/template_sql/base_templates.py +++ b/cumulus_library/template_sql/base_templates.py @@ -1,40 +1,62 @@ """ Collection of jinja template getters for common SQL queries """ -from enum import Enum -from pathlib import Path +import enum +import pathlib -from jinja2 import Template -from pandas import DataFrame +import jinja2 +import pandas from cumulus_library.template_sql import sql_utils -PATH = Path(__file__).parent - -class TableView(Enum): +class TableView(enum.Enum): """Convenience enum for building drop queries""" TABLE = "TABLE" VIEW = "VIEW" -# TODO: Consolidate to a generic template reader +def get_base_template( + filename_stem: str, path: pathlib.Path | None = None, **kwargs: dict +) -> str: + """Abstract renderer for jinja templates + + You can use this renderer directly, but if you are designing a function + that you expect to be commonly used, it's recommended to create a context + specific loader function instead. + + This function will autoload macros in cumulus_library/template_sql/shared_macros, + as well as any macros in a folder provided by path + """ + base_path = pathlib.Path(__file__).parent + with open(f"{path or base_path}/{filename_stem}.sql.jinja") as file: + template = file.read() + macro_paths = [base_path / "shared_macros"] + if path: + macro_paths.append(path) + loader = jinja2.FileSystemLoader(macro_paths) + env = jinja2.Environment(loader=loader).from_string(template) + return env.render(**kwargs) + + +# All remaining functions are context-specific calls aimed at providing +# guidance around table creation for commonly used SQL functions def get_alias_table_query(source_table: str, target_table: str): - """Creates a 1-1 alias of a given table""" - with open(f"{PATH}/alias_table.sql.jinja") as alias_table: - return Template(alias_table.read()).render( - source_table=source_table, target_table=target_table - ) + """Creates a view of source_table named target_table""" + return get_base_template( + "alias_table", source_table=source_table, target_table=target_table + ) def get_code_system_pairs(output_table_name: str, code_system_tables: list) -> str: """Extracts code system details as a standalone table""" - with open(f"{PATH}/code_system_pairs.sql.jinja") as code_system_pairs: - return Template(code_system_pairs.read()).render( - output_table_name=output_table_name, code_system_tables=code_system_tables - ) + return get_base_template( + "code_system_pairs", + output_table_name=output_table_name, + code_system_tables=code_system_tables, + ) def get_codeable_concept_denormalize_query( @@ -56,26 +78,26 @@ def get_codeable_concept_denormalize_query( # for loop will do a single pass. This implicitly means that we're not # filtering, so this parameter will be otherwise ignored config.code_systems = config.code_systems or ["all"] - - with open(f"{PATH}/codeable_concept_denormalize.sql.jinja") as codable_concept: - return Template(codable_concept.read()).render( - source_table=config.source_table, - source_id=config.source_id, - column_name=config.column_name, - is_array=config.is_array, - target_table=config.target_table, - filter_priority=config.filter_priority, - code_systems=config.code_systems, - ) + return get_base_template( + "codeable_concept_denormalize", + source_table=config.source_table, + source_id=config.source_id, + column_name=config.column_name, + is_array=config.is_array, + target_table=config.target_table, + filter_priority=config.filter_priority, + code_systems=config.code_systems, + ) def get_column_datatype_query(schema_name: str, table_name: str, column_names: list): - with open(f"{PATH}/column_datatype.sql.jinja") as column_datatype: - return Template(column_datatype.read()).render( - schema_name=schema_name, - table_name=table_name, - column_names=column_names, - ) + """Gets the in-database data representation of a given column""" + return get_base_template( + "column_datatype", + schema_name=schema_name, + table_name=table_name, + column_names=column_names, + ) def get_create_view_query( @@ -87,12 +109,12 @@ def get_create_view_query( :param dataset: Array of data arrays to insert, i.e. [['1','3'],['2','4']] :param table_cols: Comma deleniated column names, i.e. ['first,second'] """ - with open(f"{PATH}/create_view_as.sql.jinja") as cvas: - return Template(cvas.read()).render( - view_name=view_name, - dataset=dataset, - view_cols=view_cols, - ) + return get_base_template( + "create_view_as", + view_name=view_name, + dataset=dataset, + view_cols=view_cols, + ) def get_ctas_query( @@ -110,27 +132,31 @@ def get_ctas_query( :param dataset: Array of data arrays to insert, i.e. [['1','3'],['2','4']] :param table_cols: Comma deleniated column names, i.e. ['first,second'] """ - with open(f"{PATH}/ctas.sql.jinja") as ctas: - return Template(ctas.read()).render( - schema_name=schema_name, - table_name=table_name, - dataset=dataset, - table_cols=table_cols, - ) + return get_base_template( + "ctas", + schema_name=schema_name, + table_name=table_name, + dataset=dataset, + table_cols=table_cols, + ) -def get_ctas_query_from_df(schema_name: str, table_name: str, df: DataFrame) -> str: +def get_ctas_query_from_df( + schema_name: str, table_name: str, df: pandas.DataFrame +) -> str: """Generates a create table as query from a dataframe - This is a convenience wrapper for get_ctas_query. - :param schema_name: The athena schema to create the table in :param table_name: The name of the athena table to create :param df: A pandas dataframe """ split_dict = df.to_dict(orient="split") - return get_ctas_query( - schema_name, table_name, split_dict["data"], split_dict["columns"] + return get_base_template( + "ctas", + schema_name=schema_name, + table_name=table_name, + dataset=split_dict["data"], + table_cols=split_dict["columns"], ) @@ -155,22 +181,21 @@ def get_ctas_empty_query( """ if not table_cols_types: table_cols_types = ["varchar"] * len(table_cols) - with open(f"{PATH}/ctas_empty.sql.jinja") as ctas_empty: - return Template(ctas_empty.read()).render( - schema_name=schema_name, - table_name=table_name, - table_cols=table_cols, - table_cols_types=table_cols_types, - ) + return get_base_template( + "ctas_empty", + schema_name=schema_name, + table_name=table_name, + table_cols=table_cols, + table_cols_types=table_cols_types, + ) def get_drop_view_table(name: str, view_or_table: str) -> str: """Generates a drop table if exists query""" if view_or_table in [e.value for e in TableView]: - with open(f"{PATH}/drop_view_table.sql.jinja") as drop_view_table: - return Template(drop_view_table.read()).render( - view_or_table_name=name, view_or_table=view_or_table - ) + return get_base_template( + "drop_view_table", view_or_table_name=name, view_or_table=view_or_table + ) def get_extension_denormalize_query(config: sql_utils.ExtensionConfig) -> str: @@ -186,16 +211,16 @@ def get_extension_denormalize_query(config: sql_utils.ExtensionConfig) -> str: :param config: An instance of ExtensionConfig. """ - with open(f"{PATH}/extension_denormalize.sql.jinja") as extension_denormalize: - return Template(extension_denormalize.read()).render( - source_table=config.source_table, - source_id=config.source_id, - target_table=config.target_table, - target_col_prefix=config.target_col_prefix, - fhir_extension=config.fhir_extension, - ext_systems=config.ext_systems, - is_array=config.is_array, - ) + return get_base_template( + "extension_denormalize", + source_table=config.source_table, + source_id=config.source_id, + target_table=config.target_table, + target_col_prefix=config.target_col_prefix, + fhir_extension=config.fhir_extension, + ext_systems=config.ext_systems, + is_array=config.is_array, + ) def get_insert_into_query( @@ -212,13 +237,13 @@ def get_insert_into_query( :param dataset: Array of data arrays to insert, i.e. [['1','3'],['2','4']] """ type_casts = type_casts or {} - with open(f"{PATH}/insert_into.sql.jinja") as insert_into: - return Template(insert_into.read()).render( - table_name=table_name, - table_cols=table_cols, - dataset=dataset, - type_casts=type_casts, - ) + return get_base_template( + "insert_into", + table_name=table_name, + table_cols=table_cols, + dataset=dataset, + type_casts=type_casts, + ) def get_is_table_not_empty_query( @@ -229,18 +254,20 @@ def get_is_table_not_empty_query( ): unnests = unnests or [] conditions = conditions or [] - with open(f"{PATH}/is_table_not_empty.sql.jinja") as is_table_not_empty: - return Template(is_table_not_empty.read()).render( - source_table=source_table, - field=field, - unnests=unnests, - conditions=conditions, - ) + return get_base_template( + "is_table_not_empty", + source_table=source_table, + field=field, + unnests=unnests, + conditions=conditions, + ) def get_select_all_query(source_table: str): - with open(f"{PATH}/select_all.sql.jinja") as select_all: - return Template(select_all.read()).render(source_table=source_table) + return get_base_template( + "select_all", + source_table=source_table, + ) def get_show_tables(schema_name: str, prefix: str) -> str: @@ -252,10 +279,7 @@ def get_show_tables(schema_name: str, prefix: str) -> str: :param schema_name: The athena schema to query :param table_name: The prefix to filter by. Jinja template auto adds '__'. """ - with open(f"{PATH}/show_tables.sql.jinja") as show_tables: - return Template(show_tables.read()).render( - schema_name=schema_name, prefix=prefix - ) + return get_base_template("show_tables", schema_name=schema_name, prefix=prefix) def get_show_views(schema_name: str, prefix: str) -> str: @@ -267,7 +291,4 @@ def get_show_views(schema_name: str, prefix: str) -> str: :param schema_name: The athena schema to query :param table_name: The prefix to filter by. Jinja template auto adds '__'. """ - with open(f"{PATH}/show_views.sql.jinja") as show_tables: - return Template(show_tables.read()).render( - schema_name=schema_name, prefix=prefix - ) + return get_base_template("show_views", schema_name=schema_name, prefix=prefix) diff --git a/cumulus_library/template_sql/codeable_concept_denormalize.sql.jinja b/cumulus_library/template_sql/codeable_concept_denormalize.sql.jinja index e954cd57..c56af74d 100644 --- a/cumulus_library/template_sql/codeable_concept_denormalize.sql.jinja +++ b/cumulus_library/template_sql/codeable_concept_denormalize.sql.jinja @@ -1,3 +1,4 @@ +{%- import 'syntax.sql.jinja' as syntax -%} CREATE TABLE {{ target_table }} AS ( WITH {%- for system in code_systems %} @@ -37,9 +38,7 @@ CREATE TABLE {{ target_table }} AS ( code, display FROM system_{{ column_name }}_{{ loop.index0 }} - {%- if not loop.last %} - UNION - {%- endif -%} + {{ syntax.union_delineate(loop) }} {%- endfor %} ) {%- if filter_priority -%}, diff --git a/cumulus_library/template_sql/create_view_as.sql.jinja b/cumulus_library/template_sql/create_view_as.sql.jinja index 9300c909..640eccaf 100644 --- a/cumulus_library/template_sql/create_view_as.sql.jinja +++ b/cumulus_library/template_sql/create_view_as.sql.jinja @@ -1,3 +1,4 @@ +{%- import 'syntax.sql.jinja' as syntax -%} CREATE OR REPLACE VIEW {{ view_name }} AS ( SELECT * FROM ( VALUES @@ -5,23 +6,17 @@ CREATE OR REPLACE VIEW {{ view_name }} AS ( ( {%- for field in row -%} '{{ field }}' - {%- if not loop.last -%} - , - {%- endif -%} + {{- syntax.comma_delineate(loop) }} {%- endfor -%} ) - {%- if not loop.last -%} - , - {%- endif -%} + {{- syntax.comma_delineate(loop) }} {%- endfor %} ) AS t ( {%- for col in view_cols -%} "{{ col }}" - {%- if not loop.last -%} - , - {%- endif -%} + {{- syntax.comma_delineate(loop) }} {%- endfor -%} ) ); diff --git a/cumulus_library/template_sql/ctas.sql.jinja b/cumulus_library/template_sql/ctas.sql.jinja index 96d58c19..ab6db1e3 100644 --- a/cumulus_library/template_sql/ctas.sql.jinja +++ b/cumulus_library/template_sql/ctas.sql.jinja @@ -1,3 +1,4 @@ +{%- import 'syntax.sql.jinja' as syntax -%} CREATE TABLE "{{ schema_name }}"."{{ table_name }}" AS ( SELECT * FROM ( VALUES @@ -5,22 +6,16 @@ CREATE TABLE "{{ schema_name }}"."{{ table_name }}" AS ( ( {%- for field in row -%} cast('{{ field }}' AS varchar) - {%- if not loop.last -%} - , - {%- endif -%} + {{- syntax.comma_delineate(loop) }} {%- endfor -%} ) - {%- if not loop.last -%} - , - {%- endif -%} + {{- syntax.comma_delineate(loop) }} {%- endfor %} ) AS t ( {%- for col in table_cols -%} "{{ col }}" - {%- if not loop.last -%} - , - {%- endif -%} + {{- syntax.comma_delineate(loop) }} {%- endfor -%} ) ); diff --git a/cumulus_library/template_sql/ctas_empty.sql.jinja b/cumulus_library/template_sql/ctas_empty.sql.jinja index 980d4f22..2b7163d3 100644 --- a/cumulus_library/template_sql/ctas_empty.sql.jinja +++ b/cumulus_library/template_sql/ctas_empty.sql.jinja @@ -1,3 +1,4 @@ +{%- import 'syntax.sql.jinja' as syntax -%} CREATE TABLE IF NOT EXISTS "{{ schema_name }}"."{{ table_name }}" AS ( SELECT * FROM ( @@ -5,18 +6,14 @@ AS ( ( {%- for type in table_cols_types -%} cast(NULL AS {{ type }}) - {%- if not loop.last -%} - , - {%- endif -%} + {{- syntax.comma_delineate(loop) }} {%- endfor -%} ) ) AS t ( {%- for col in table_cols -%} "{{ col }}" - {%- if not loop.last -%} - , - {%- endif -%} + {{- syntax.comma_delineate(loop) }} {%- endfor -%} ) ); diff --git a/cumulus_library/template_sql/extension_denormalize.sql.jinja b/cumulus_library/template_sql/extension_denormalize.sql.jinja index 3595aa83..e1689652 100644 --- a/cumulus_library/template_sql/extension_denormalize.sql.jinja +++ b/cumulus_library/template_sql/extension_denormalize.sql.jinja @@ -1,3 +1,4 @@ +{%- import 'syntax.sql.jinja' as syntax -%} CREATE TABLE {{ target_table }} AS ( WITH {%- for system in ext_systems %} @@ -29,9 +30,7 @@ CREATE TABLE {{ target_table }} AS ( {{ target_col_prefix }}_code, {{ target_col_prefix }}_display FROM system_{{ system }} - {%- if not loop.last %} - UNION - {%- endif -%} + {{ syntax.union_delineate(loop) }} {%- endfor %} ORDER BY id, priority ) diff --git a/cumulus_library/template_sql/insert_into.sql.jinja b/cumulus_library/template_sql/insert_into.sql.jinja index e643d886..9807243c 100644 --- a/cumulus_library/template_sql/insert_into.sql.jinja +++ b/cumulus_library/template_sql/insert_into.sql.jinja @@ -1,10 +1,9 @@ +{%- import 'syntax.sql.jinja' as syntax -%} INSERT INTO {{ table_name }} ( {%- for col in table_cols -%} "{{ col }}" - {%- if not loop.last -%} - , - {%- endif -%} + {{- syntax.comma_delineate(loop) }} {%- endfor -%} ) VALUES @@ -16,12 +15,8 @@ VALUES {%- else -%} '{{ field }}' {%- endif -%} - {%- if not loop.last -%} - , - {%- endif -%} + {{- syntax.comma_delineate(loop) }} {%- endfor -%} ) -{%- if not loop.last -%} -, -{%- endif -%} +{{- syntax.comma_delineate(loop) }} {%- endfor %}; diff --git a/cumulus_library/template_sql/shared_macros/syntax.sql.jinja b/cumulus_library/template_sql/shared_macros/syntax.sql.jinja new file mode 100644 index 00000000..853af262 --- /dev/null +++ b/cumulus_library/template_sql/shared_macros/syntax.sql.jinja @@ -0,0 +1,14 @@ +{# Commonly used macros related to basic SQL syntax formatting #} + +{%- macro comma_delineate(loop) -%} +{%- if not loop.last -%} +, +{%- endif -%} +{%- endmacro -%} + +{%- macro union_delineate(loop) -%} +{%- if not loop.last -%} +UNION +{%- endif -%} +{%- endmacro -%} +--noqa: LT12 diff --git a/tests/test_counts_builder.py b/tests/test_counts_builder.py index 7b70ac9c..b37f5f1a 100644 --- a/tests/test_counts_builder.py +++ b/tests/test_counts_builder.py @@ -75,7 +75,9 @@ def test_get_where_clauses(clause, min_subject, expected, raises): ("table", "source", None, {}, pytest.raises(CountsBuilderError)), ], ) -@mock.patch("cumulus_library.template_sql.statistics.counts_templates.get_count_query") +@mock.patch( + "cumulus_library.statistics.statistics_templates.counts_templates.get_count_query" +) def test_get_count_query( mock_count, table_name, source_table, table_cols, kwargs, raises ): @@ -145,7 +147,9 @@ def test_get_count_query( ), ], ) -@mock.patch("cumulus_library.template_sql.statistics.counts_templates.get_count_query") +@mock.patch( + "cumulus_library.statistics.statistics_templates.counts_templates.get_count_query" +) def test_count_wrappers( mock_count, table_name, diff --git a/tests/test_counts_templates.py b/tests/test_counts_templates.py index 55a211a0..df701273 100644 --- a/tests/test_counts_templates.py +++ b/tests/test_counts_templates.py @@ -2,7 +2,7 @@ import pytest -from cumulus_library.template_sql.statistics.counts_templates import get_count_query +from cumulus_library.statistics.statistics_templates import counts_templates @pytest.mark.parametrize( @@ -200,7 +200,9 @@ ], ) def test_count_query(expected, kwargs): - query = get_count_query("test_table", "test_source", ["age", "sex"], **kwargs) + query = counts_templates.get_count_query( + "test_table", "test_source", ["age", "sex"], **kwargs + ) # Snippet for getting updated template output # with open("output.sql", "w") as f: # f.write(query) diff --git a/tests/test_psm_templates.py b/tests/test_psm_templates.py index 7a1aeab8..ac29cec2 100644 --- a/tests/test_psm_templates.py +++ b/tests/test_psm_templates.py @@ -4,11 +4,8 @@ import pytest -from cumulus_library.errors import CumulusLibraryError -from cumulus_library.template_sql.statistics.psm_templates import ( - get_create_covariate_table, - get_distinct_ids, -) +from cumulus_library import errors +from cumulus_library.statistics.statistics_templates import psm_templates @pytest.mark.parametrize( @@ -41,14 +38,23 @@ FROM source""", does_not_raise(), ), - (["a", "b"], "source", "ref_id", None, "", pytest.raises(CumulusLibraryError)), + ( + ["a", "b"], + "source", + "ref_id", + None, + "", + pytest.raises(errors.CumulusLibraryError), + ), ], ) def test_get_distinct_ids( columns, source_table, join_id, filter_table, expected, raises ): with raises: - query = get_distinct_ids(columns, source_table, join_id, filter_table) + query = psm_templates.get_distinct_ids( + columns, source_table, join_id, filter_table + ) assert query == expected @@ -128,7 +134,7 @@ def test_get_distinct_ids( "join_table", None, "", - pytest.raises(CumulusLibraryError), + pytest.raises(errors.CumulusLibraryError), ), ], ) @@ -146,7 +152,7 @@ def test_create_covariate_table( raises, ): with raises: - query = get_create_covariate_table( + query = psm_templates.get_create_covariate_table( target, pos_source, neg_source, diff --git a/tests/test_templates.py b/tests/test_templates.py index 8c5d85bb..59bc940c 100644 --- a/tests/test_templates.py +++ b/tests/test_templates.py @@ -42,6 +42,7 @@ def test_codeable_concept_denormalize_all_creation(): code, display FROM system_code_col_0 + ) SELECT id, @@ -110,6 +111,7 @@ def test_codeable_concept_denormalize_filter_creation(): code, display FROM system_code_col_1 + ), partitioned_table AS ( @@ -152,7 +154,6 @@ def test_codeable_concept_denormalize_filter_creation(): ], ) query = base_templates.get_codeable_concept_denormalize_query(config) - assert query == expected @@ -310,6 +311,7 @@ def test_extension_denormalize_creation(): prefix_code, prefix_display FROM system_text + ORDER BY id, priority )