Add helpText to requires (#373)

Add `helpText` to `layer.requires` like discussed in openmaptiles/openmaptiles#1220 I had to change from `SELECT 'osm_ocean_polygon'::regclass;` to `PERFORM 'osm_ocean_polygon'::regclass;` because plpgsql needs a return value if `SELECT` is used. Also added an error if a function has no arguments: ``` when invalid_text_representation then RAISE EXCEPTION '%! The arguments of the required function "osmFunc" of the layer "water" are missing. Example: "osmFunc(text)"', SQLERRM; ``` Example: ``` layer: id: "water" requires: helpText: 'This is a text with "quotes"' functions: "osmFunc" tables: "osm_ocean_polygon" ``` SQL: ``` -- Assert osm_ocean_polygon exists do $$ begin PERFORM 'osm_ocean_polygon'::regclass; exception when undefined_table then RAISE EXCEPTION '%! This is error text with "quotes"', SQLERRM; end; $$ language 'plpgsql'; -- Assert osmFunc exists do $$ begin PERFORM 'osmFunc'::regprocedure; exception when undefined_function then RAISE EXCEPTION '%! This is a text with "quotes"', SQLERRM; when invalid_text_representation then RAISE EXCEPTION '%! The arguments of the required function "osmFunc" of the layer "water" are missing. Example: "osmFunc(text)"', SQLERRM; end; $$ language 'plpgsql'; ``` Result of `make import-sql`: > psql:/sql/parallel/water__waterway.sql:10: ERROR: relation "osm_ocean_polygon2" does not exist! This is a text with "quotes" Co-authored-by: Yuri Astrakhan <[email protected]>
openmaptiles · Oct 8, 2021 · 208dd6a · 208dd6a
1 parent 0493b77
commit 208dd6a
Show file tree

Hide file tree

Showing 7 changed files with 124 additions and 19 deletions.
diff --git a/openmaptiles/pgutils.py b/openmaptiles/pgutils.py
@@ -164,3 +164,8 @@ def print_query_error(error_msg, err, pg_warnings, verbose, query, layer_sql=Non
             query_msg += f'\n\n== MVT SQL\n{layer_sql}'
         print(query_msg)
     print(f'{line}\n')
+
+
+def quote_literal(string):
+    """Adapted from asyncpg.utils"""
+    return "'{}'".format(string.replace("'", "''"))
diff --git a/openmaptiles/sql.py b/openmaptiles/sql.py
@@ -3,6 +3,7 @@
 
 from sys import stderr
 
+from openmaptiles.pgutils import quote_literal
 from openmaptiles.tileset import Tileset, Layer
 
 
@@ -74,17 +75,56 @@ def collect_sql(tileset_filename, parallel=False, nodata=False
 
 def layer_to_sql(layer: Layer, nodata: bool):
     sql = f"DO $$ BEGIN RAISE NOTICE 'Processing layer {layer.id}'; END$$;\n\n"
+
     for table in layer.requires_tables:
-        sql += f"-- Assert {table} exists\nSELECT '{table}'::regclass;\n\n"
+        sql += sql_assert_table(table, layer.requires_helpText, layer.id)
+
     for func in layer.requires_functions:
-        sql += f"-- Assert {func} exists\nSELECT '{func}'::regprocedure;\n\n"
+        sql += sql_assert_func(func, layer.requires_helpText, layer.id)
+
     for schema in layer.schemas:
         sql += to_sql(schema, layer, nodata) + '\n\n'
     sql += f"DO $$ BEGIN RAISE NOTICE 'Finished layer {layer.id}'; END$$;\n"
 
     return sql
 
 
+def _sql_hint_clause(hint):
+    if hint:
+        return f',\n                    HINT = {quote_literal(hint)}'
+    else:
+        return ''
+
+
+def sql_assert_table(table, hint, layer_id):
+    return f"""\
+DO $$ BEGIN
+    PERFORM {quote_literal(table)}::regclass;
+EXCEPTION
+    WHEN undefined_table THEN
+        RAISE EXCEPTION '%', SQLERRM
+            USING DETAIL = 'this table or view is required for layer "{layer_id}"'{_sql_hint_clause(hint)};
+END;
+$$ LANGUAGE 'plpgsql';\n
+"""
+
+
+def sql_assert_func(func, hint, layer_id):
+    return f"""\
+DO $$ BEGIN
+    PERFORM {quote_literal(func)}::regprocedure;
+EXCEPTION
+    WHEN undefined_function THEN
+        RAISE EXCEPTION '%', SQLERRM
+            USING DETAIL = 'this function is required for layer "{layer_id}"'{_sql_hint_clause(hint)};
+    WHEN invalid_text_representation THEN
+        RAISE EXCEPTION '%', SQLERRM
+            USING DETAIL = 'Required function "{func}" in layer "{layer_id}" is incorrectly declared. Use full function signature with parameter types, e.g. "my_magic_func(TEXT, TEXT)"';
+END;
+$$ LANGUAGE 'plpgsql';\n
+"""
+
+
 def get_slice_language_tags(tileset):
     include_tags = list(map(lambda l: 'name:' + l, tileset.languages))
     include_tags.append('int_name')

diff --git a/openmaptiles/tileset.py b/openmaptiles/tileset.py
@@ -115,7 +115,8 @@ def __init__(self,
         else:
             requires = requires.copy()  # dict will be modified to detect unrecognized properties
 
-        err = 'If set, "requires" parameter must be a map with optional "layers", "tables", and "functions" sub-elements. Each sub-element must be a string or a list of strings. If "requires" is a list or a string itself, it is treated as a list of layers. '
+        err = 'If set, "requires" parameter must be a map with optional "layers", "tables", and "functions" sub-elements. Each sub-element must be a string or a list of strings. If "requires" is a list or a string itself, it is treated as a list of layers. ' + \
+            'Optionally add "helpText" sub-element string to help the user with generating missing tables and functions.'
         self.requires_layers = get_requires_prop(
             requires, 'layers',
             err + '"requires.layers" must be an ID of another layer, or a list of layer IDs.')
@@ -126,6 +127,11 @@ def __init__(self,
             requires, 'functions',
             err + '"requires.functions" must be a PostgreSQL function name with parameters or a list of functions. Example: "sql_func(TEXT, TEXT)"')
 
+        self.requires_helpText = None
+        if requires.get('helpText'):
+            self.requires_helpText = requires.get('helpText')
+            requires.pop('helpText', [])
+
         if requires:
             # get_requires_prop will delete the key it handled. Remaining keys are errors.
             raise ValueError(f'Unrecognized sub-elements in the \"requires\" parameter: {str(list(requires.keys()))}')

diff --git a/tests/expected/parallel_sql/parallel/mountain_peak.sql b/tests/expected/parallel_sql/parallel/mountain_peak.sql
@@ -1,9 +1,24 @@
 DO $$ BEGIN RAISE NOTICE 'Processing layer mountain_peak'; END$$;
 
--- Assert my_magic_table exists
-SELECT 'my_magic_table'::regclass;
+DO $$ BEGIN
+    PERFORM 'my_magic_table'::regclass;
+EXCEPTION
+    WHEN undefined_table THEN
+        RAISE EXCEPTION '%', SQLERRM
+            USING DETAIL = 'this table or view is required for layer "mountain_peak"';
+END;
+$$ LANGUAGE 'plpgsql';
 
--- Assert my_magic_func(TEXT, TEXT) exists
-SELECT 'my_magic_func(TEXT, TEXT)'::regprocedure;
+DO $$ BEGIN
+    PERFORM 'my_magic_func(TEXT, TEXT)'::regprocedure;
+EXCEPTION
+    WHEN undefined_function THEN
+        RAISE EXCEPTION '%', SQLERRM
+            USING DETAIL = 'this function is required for layer "mountain_peak"';
+    WHEN invalid_text_representation THEN
+        RAISE EXCEPTION '%', SQLERRM
+            USING DETAIL = 'Required function "my_magic_func(TEXT, TEXT)" in layer "mountain_peak" is incorrectly declared. Use full function signature with parameter types, e.g. "my_magic_func(TEXT, TEXT)"';
+END;
+$$ LANGUAGE 'plpgsql';
 
 DO $$ BEGIN RAISE NOTICE 'Finished layer mountain_peak'; END$$;
diff --git a/tests/expected/parallel_sql2/parallel/mountain_peak.sql b/tests/expected/parallel_sql2/parallel/mountain_peak.sql
@@ -1,9 +1,24 @@
 DO $$ BEGIN RAISE NOTICE 'Processing layer mountain_peak'; END$$;
 
--- Assert my_magic_table exists
-SELECT 'my_magic_table'::regclass;
+DO $$ BEGIN
+    PERFORM 'my_magic_table'::regclass;
+EXCEPTION
+    WHEN undefined_table THEN
+        RAISE EXCEPTION '%', SQLERRM
+            USING DETAIL = 'this table or view is required for layer "mountain_peak"';
+END;
+$$ LANGUAGE 'plpgsql';
 
--- Assert my_magic_func(TEXT, TEXT) exists
-SELECT 'my_magic_func(TEXT, TEXT)'::regprocedure;
+DO $$ BEGIN
+    PERFORM 'my_magic_func(TEXT, TEXT)'::regprocedure;
+EXCEPTION
+    WHEN undefined_function THEN
+        RAISE EXCEPTION '%', SQLERRM
+            USING DETAIL = 'this function is required for layer "mountain_peak"';
+    WHEN invalid_text_representation THEN
+        RAISE EXCEPTION '%', SQLERRM
+            USING DETAIL = 'Required function "my_magic_func(TEXT, TEXT)" in layer "mountain_peak" is incorrectly declared. Use full function signature with parameter types, e.g. "my_magic_func(TEXT, TEXT)"';
+END;
+$$ LANGUAGE 'plpgsql';
 
 DO $$ BEGIN RAISE NOTICE 'Finished layer mountain_peak'; END$$;
diff --git a/tests/expected/sql.sql b/tests/expected/sql.sql
@@ -70,11 +70,26 @@ DO $$ BEGIN RAISE NOTICE 'Finished layer enumfield'; END$$;
 
 DO $$ BEGIN RAISE NOTICE 'Processing layer mountain_peak'; END$$;
 
--- Assert my_magic_table exists
-SELECT 'my_magic_table'::regclass;
-
--- Assert my_magic_func(TEXT, TEXT) exists
-SELECT 'my_magic_func(TEXT, TEXT)'::regprocedure;
+DO $$ BEGIN
+    PERFORM 'my_magic_table'::regclass;
+EXCEPTION
+    WHEN undefined_table THEN
+        RAISE EXCEPTION '%', SQLERRM
+            USING DETAIL = 'this table or view is required for layer "mountain_peak"';
+END;
+$$ LANGUAGE 'plpgsql';
+
+DO $$ BEGIN
+    PERFORM 'my_magic_func(TEXT, TEXT)'::regprocedure;
+EXCEPTION
+    WHEN undefined_function THEN
+        RAISE EXCEPTION '%', SQLERRM
+            USING DETAIL = 'this function is required for layer "mountain_peak"';
+    WHEN invalid_text_representation THEN
+        RAISE EXCEPTION '%', SQLERRM
+            USING DETAIL = 'Required function "my_magic_func(TEXT, TEXT)" in layer "mountain_peak" is incorrectly declared. Use full function signature with parameter types, e.g. "my_magic_func(TEXT, TEXT)"';
+END;
+$$ LANGUAGE 'plpgsql';
 
 DO $$ BEGIN RAISE NOTICE 'Finished layer mountain_peak'; END$$;
 

diff --git a/tests/python/test_sql.py b/tests/python/test_sql.py
@@ -4,7 +4,7 @@
 from typing import List, Union, Dict
 from unittest import main, TestCase
 
-from openmaptiles.sql import collect_sql
+from openmaptiles.sql import collect_sql, sql_assert_table, sql_assert_func
 from openmaptiles.tileset import ParsedData, Tileset
 
 
@@ -18,10 +18,12 @@ class Case:
 def expected_sql(case: Case):
     result = f"DO $$ BEGIN RAISE NOTICE 'Processing layer {case.id}'; END$$;\n\n"
     if isinstance(case.reqs, dict):
+        # Use helper functions for SQL generation. Actual SQL is tested by integration tests
         for table in case.reqs.get('tables', []):
-            result += f"-- Assert {table} exists\nSELECT '{table}'::regclass;\n\n"
+            result += sql_assert_table(table, case.reqs.get('helpText'), case.id)
         for func in case.reqs.get('functions', []):
-            result += f"-- Assert {func} exists\nSELECT '{func}'::regprocedure;\n\n"
+            result += sql_assert_func(func, case.reqs.get('helpText'), case.id)
+
     result += f"""\
 -- Layer {case.id} - {case.id}_s.yaml
 
@@ -105,7 +107,12 @@ def test_require(self):
         c10 = Case('c10', 'SELECT 10;', reqs=dict(tables=['tbl1', 'tbl2']))
         c11 = Case('c11', 'SELECT 11;', reqs=dict(functions=['fnc1']))
         c12 = Case('c12', 'SELECT 12;', reqs=dict(functions=['fnc1', 'fnc2']))
+        c13 = Case('c13', 'SELECT 13;', reqs=dict(functions=['fnc1', 'fnc2'],
+                                                  helpText="Custom 'ERROR MESSAGE' for missing function - single quote"))
+        c14 = Case('c14', 'SELECT 14;',
+                   reqs=dict(tables=['tbl1'], helpText='Custom "ERROR MESSAGE" for missing table - double quote'))
 
+        self._test('a18', [c12], dict(c12=[c12]))
         self._test('a01', [], {})
         self._test('a02', [c1], dict(c1=c1))
         self._test('a03', [c1, c2], dict(c1=c1, c2=c2))
@@ -127,6 +134,8 @@ def test_require(self):
         self._test('a16', [c10], dict(c10=[c10]))
         self._test('a17', [c11], dict(c11=[c11]))
         self._test('a18', [c12], dict(c12=[c12]))
+        self._test('a19', [c13], dict(c13=[c13]))
+        self._test('a20', [c14], dict(c14=[c14]))
 
     def _ts_parse(self, reqs, expected_layers, expected_tables, expected_funcs, extra_cases=None):
         cases = [] if not extra_cases else list(extra_cases)