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

Add helpText to requires #373

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions openmaptiles/pgutils.py
Original file line number Diff line number Diff line change
Expand Up @@ -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("'", "''"))
44 changes: 42 additions & 2 deletions openmaptiles/sql.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

from sys import stderr

from openmaptiles.pgutils import quote_literal
from openmaptiles.tileset import Tileset, Layer


Expand Down Expand Up @@ -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')
Expand Down
8 changes: 7 additions & 1 deletion openmaptiles/tileset.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.')
Expand All @@ -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()))}')
Expand Down
23 changes: 19 additions & 4 deletions tests/expected/parallel_sql/parallel/mountain_peak.sql
Original file line number Diff line number Diff line change
@@ -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$$;
23 changes: 19 additions & 4 deletions tests/expected/parallel_sql2/parallel/mountain_peak.sql
Original file line number Diff line number Diff line change
@@ -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$$;
25 changes: 20 additions & 5 deletions tests/expected/sql.sql
Original file line number Diff line number Diff line change
Expand Up @@ -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$$;

Expand Down
15 changes: 12 additions & 3 deletions tests/python/test_sql.py
Original file line number Diff line number Diff line change
Expand Up @@ -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


Expand All @@ -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

Expand Down Expand Up @@ -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))
Expand All @@ -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)
Expand Down