From ff5369d5b6e1f7627ad2bc0b12933593ff4e7f74 Mon Sep 17 00:00:00 2001 From: Jochen Topf Date: Sat, 10 Dec 2022 19:11:45 +0100 Subject: [PATCH 1/2] Add testing capability to pgsql capability code And use it to test flex index setup. --- src/pgsql-capabilities-int.hpp | 36 ++++ src/pgsql-capabilities.cpp | 24 +-- tests/CMakeLists.txt | 1 + tests/test-flex-indexes.cpp | 345 +++++++++++++++++++++++++++++++++ 4 files changed, 388 insertions(+), 18 deletions(-) create mode 100644 src/pgsql-capabilities-int.hpp create mode 100644 tests/test-flex-indexes.cpp diff --git a/src/pgsql-capabilities-int.hpp b/src/pgsql-capabilities-int.hpp new file mode 100644 index 000000000..43052d6c1 --- /dev/null +++ b/src/pgsql-capabilities-int.hpp @@ -0,0 +1,36 @@ +#ifndef OSM2PGSQL_PGSQL_CAPABILITIES_INT_HPP +#define OSM2PGSQL_PGSQL_CAPABILITIES_INT_HPP + +/** + * SPDX-License-Identifier: GPL-2.0-or-later + * + * This file is part of osm2pgsql (https://osm2pgsql.org/). + * + * Copyright (C) 2006-2022 by the osm2pgsql developer community. + * For a full list of authors see the git log. + */ + +#include "pgsql-capabilities.hpp" + +#include +#include +#include + +struct database_capabilities_t +{ + std::map settings; + + std::set extensions; + std::set schemas; + std::set tablespaces; + std::set index_methods; + + std::string database_name; + + uint32_t database_version = 0; + postgis_version postgis{}; +}; + +database_capabilities_t &database_capabilities_for_testing() noexcept; + +#endif // OSM2PGSQL_PGSQL_CAPABILITIES_INT_HPP diff --git a/src/pgsql-capabilities.cpp b/src/pgsql-capabilities.cpp index 7e48cbe76..37948fbe4 100644 --- a/src/pgsql-capabilities.cpp +++ b/src/pgsql-capabilities.cpp @@ -10,28 +10,11 @@ #include "format.hpp" #include "logging.hpp" #include "pgsql-capabilities.hpp" +#include "pgsql-capabilities-int.hpp" #include "pgsql.hpp" #include "version.hpp" -#include -#include #include -#include - -struct database_capabilities_t -{ - std::map settings; - - std::set extensions; - std::set schemas; - std::set tablespaces; - std::set index_methods; - - std::string database_name; - - uint32_t database_version = 0; - postgis_version postgis{}; -}; static database_capabilities_t &capabilities() noexcept { @@ -39,6 +22,11 @@ static database_capabilities_t &capabilities() noexcept return c; } +database_capabilities_t &database_capabilities_for_testing() noexcept +{ + return capabilities(); +} + static void init_set_from_query(std::set *set, pg_conn_t const &db_connection, char const *table, char const *column, diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 11bfc74c7..d0dfcd461 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -84,6 +84,7 @@ set_test(test-wkb LABELS NoDB) # these tests require LUA support if (WITH_LUA) + set_test(test-flex-indexes) set_test(test-output-flex) set_test(test-output-flex-multi-input) set_test(test-output-flex-nodes) diff --git a/tests/test-flex-indexes.cpp b/tests/test-flex-indexes.cpp new file mode 100644 index 000000000..d2715c418 --- /dev/null +++ b/tests/test-flex-indexes.cpp @@ -0,0 +1,345 @@ +/** + * SPDX-License-Identifier: GPL-2.0-or-later + * + * This file is part of osm2pgsql (https://osm2pgsql.org/). + * + * Copyright (C) 2006-2022 by the osm2pgsql developer community. + * For a full list of authors see the git log. + */ + +#include + +#include "flex-lua-index.hpp" +#include "flex-table.hpp" +#include "lua.hpp" +#include "pgsql-capabilities-int.hpp" + +class test_framework +{ +public: + test_framework() + : m_lua_state(luaL_newstate(), [](lua_State *state) { lua_close(state); }) + { + auto &c = database_capabilities_for_testing(); + + c.settings = {}; + + c.extensions = {"postgis"}; + c.schemas = {"testschema"}; + c.tablespaces = {"somewhereelse"}; + c.index_methods = {"gist", "btree"}; + + c.database_version = 110000; + } + + lua_State *lua_state() const noexcept { return m_lua_state.get(); } + + bool run_lua(char const *code) + { + return luaL_dostring(lua_state(), code) == 0; + } + +private: + std::shared_ptr m_lua_state; +}; + +TEST_CASE("check index with single column", "[NoDB]") +{ + test_framework tf; + + flex_table_t table{"test_table"}; + table.add_column("geom", "geometry", ""); + + REQUIRE(table.indexes().empty()); + + REQUIRE(tf.run_lua("return { method = 'gist', column = 'geom' }")); + flex_lua_setup_index(tf.lua_state(), &table); + + REQUIRE(table.indexes().size() == 1); + flex_index_t const &idx = table.indexes().front(); + REQUIRE(idx.method() == "gist"); + REQUIRE(idx.columns() == R"(("geom"))"); + REQUIRE_FALSE(idx.is_unique()); + REQUIRE(idx.tablespace().empty()); + REQUIRE(idx.expression().empty()); + REQUIRE(idx.where_condition().empty()); + REQUIRE(idx.include_columns().empty()); +} + +TEST_CASE("check index with multiple columns", "[NoDB]") +{ + test_framework tf; + + flex_table_t table{"test_table"}; + table.add_column("a", "int", ""); + table.add_column("b", "int", ""); + + REQUIRE(tf.run_lua("return { method = 'btree', column = {'a', 'b'} }")); + flex_lua_setup_index(tf.lua_state(), &table); + + REQUIRE(table.indexes().size() == 1); + auto const &idx = table.indexes()[0]; + REQUIRE(idx.method() == "btree"); + REQUIRE(idx.columns() == R"(("a","b"))"); + REQUIRE_FALSE(idx.is_unique()); + REQUIRE(idx.tablespace().empty()); + REQUIRE(idx.expression().empty()); + REQUIRE(idx.where_condition().empty()); + REQUIRE(idx.include_columns().empty()); +} + +TEST_CASE("check unique index", "[NoDB]") +{ + test_framework tf; + + flex_table_t table{"test_table"}; + table.add_column("col", "int", ""); + + REQUIRE(tf.run_lua( + "return { method = 'btree', column = 'col', unique = true }")); + flex_lua_setup_index(tf.lua_state(), &table); + + REQUIRE(table.indexes().size() == 1); + auto const &idx = table.indexes()[0]; + REQUIRE(idx.method() == "btree"); + REQUIRE(idx.columns() == R"(("col"))"); + REQUIRE(idx.is_unique()); + REQUIRE(idx.tablespace().empty()); + REQUIRE(idx.expression().empty()); + REQUIRE(idx.where_condition().empty()); + REQUIRE(idx.include_columns().empty()); +} + +TEST_CASE("check index with tablespace from table", "[NoDB]") +{ + test_framework tf; + + flex_table_t table{"test_table"}; + table.set_index_tablespace("foo"); + table.add_column("col", "int", ""); + + REQUIRE(tf.run_lua("return { method = 'btree', column = 'col' }")); + flex_lua_setup_index(tf.lua_state(), &table); + + REQUIRE(table.indexes().size() == 1); + auto const &idx = table.indexes()[0]; + REQUIRE(idx.method() == "btree"); + REQUIRE(idx.columns() == R"(("col"))"); + REQUIRE_FALSE(idx.is_unique()); + REQUIRE(idx.tablespace() == "foo"); + REQUIRE(idx.expression().empty()); + REQUIRE(idx.where_condition().empty()); + REQUIRE(idx.include_columns().empty()); +} + +TEST_CASE("check index with tablespace", "[NoDB]") +{ + test_framework tf; + + flex_table_t table{"test_table"}; + table.add_column("col", "int", ""); + + REQUIRE(tf.run_lua("return { method = 'btree', column = 'col', tablespace " + "= 'somewhereelse' }")); + flex_lua_setup_index(tf.lua_state(), &table); + + REQUIRE(table.indexes().size() == 1); + auto const &idx = table.indexes()[0]; + REQUIRE(idx.method() == "btree"); + REQUIRE(idx.columns() == R"(("col"))"); + REQUIRE_FALSE(idx.is_unique()); + REQUIRE(idx.tablespace() == "somewhereelse"); + REQUIRE(idx.expression().empty()); + REQUIRE(idx.where_condition().empty()); + REQUIRE(idx.include_columns().empty()); +} + +TEST_CASE("check index with expression and where clause", "[NoDB]") +{ + test_framework tf; + + flex_table_t table{"test_table"}; + table.add_column("col", "text", ""); + + REQUIRE(tf.run_lua("return { method = 'btree', expression = 'lower(col)'," + " where = 'length(col) > 1' }")); + flex_lua_setup_index(tf.lua_state(), &table); + + REQUIRE(table.indexes().size() == 1); + auto const &idx = table.indexes()[0]; + REQUIRE(idx.method() == "btree"); + REQUIRE(idx.columns() == ""); + REQUIRE_FALSE(idx.is_unique()); + REQUIRE(idx.tablespace().empty()); + REQUIRE(idx.expression() == "lower(col)"); + REQUIRE(idx.where_condition() == "length(col) > 1"); + REQUIRE(idx.include_columns().empty()); +} + +TEST_CASE("check index with include", "[NoDB]") +{ + test_framework tf; + + flex_table_t table{"test_table"}; + table.add_column("col", "int", ""); + table.add_column("extra", "int", ""); + + REQUIRE(tf.run_lua( + "return { method = 'btree', column = 'col', include = 'extra' }")); + flex_lua_setup_index(tf.lua_state(), &table); + + REQUIRE(table.indexes().size() == 1); + auto const &idx = table.indexes()[0]; + REQUIRE(idx.method() == "btree"); + REQUIRE(idx.columns() == R"(("col"))"); + REQUIRE_FALSE(idx.is_unique()); + REQUIRE(idx.tablespace().empty()); + REQUIRE(idx.expression().empty()); + REQUIRE(idx.where_condition().empty()); + REQUIRE(idx.include_columns() == R"(("extra"))"); +} + +TEST_CASE("check index with include as array", "[NoDB]") +{ + test_framework tf; + + flex_table_t table{"test_table"}; + table.add_column("col", "int", ""); + table.add_column("extra", "int", ""); + + REQUIRE(tf.run_lua( + "return { method = 'btree', column = 'col', include = { 'extra' } }")); + flex_lua_setup_index(tf.lua_state(), &table); + + REQUIRE(table.indexes().size() == 1); + auto const &idx = table.indexes()[0]; + REQUIRE(idx.method() == "btree"); + REQUIRE(idx.columns() == R"(("col"))"); + REQUIRE_FALSE(idx.is_unique()); + REQUIRE(idx.tablespace().empty()); + REQUIRE(idx.expression().empty()); + REQUIRE(idx.where_condition().empty()); + REQUIRE(idx.include_columns() == R"(("extra"))"); +} + +TEST_CASE("check index with empty include array", "[NoDB]") +{ + test_framework tf; + + flex_table_t table{"test_table"}; + table.add_column("col", "int", ""); + table.add_column("extra", "int", ""); + + REQUIRE(tf.run_lua( + "return { method = 'btree', column = 'col', include = {} }")); + flex_lua_setup_index(tf.lua_state(), &table); + + REQUIRE(table.indexes().size() == 1); + auto const &idx = table.indexes()[0]; + REQUIRE(idx.method() == "btree"); + REQUIRE(idx.columns() == R"(("col"))"); + REQUIRE_FALSE(idx.is_unique()); + REQUIRE(idx.tablespace().empty()); + REQUIRE(idx.expression().empty()); + REQUIRE(idx.where_condition().empty()); + REQUIRE(idx.include_columns().empty()); +} + +TEST_CASE("check multiple indexes", "[NoDB]") +{ + test_framework tf; + + flex_table_t table{"test_table"}; + table.add_column("a", "int", ""); + table.add_column("b", "int", ""); + + REQUIRE(tf.run_lua("return { method = 'btree', column = {'a'} }")); + flex_lua_setup_index(tf.lua_state(), &table); + REQUIRE(tf.run_lua("return { method = 'gist', column = 'b' }")); + flex_lua_setup_index(tf.lua_state(), &table); + + REQUIRE(table.indexes().size() == 2); + auto const &idx0 = table.indexes()[0]; + REQUIRE(idx0.method() == "btree"); + REQUIRE(idx0.columns() == R"(("a"))"); + + auto const &idx1 = table.indexes()[1]; + REQUIRE(idx1.method() == "gist"); + REQUIRE(idx1.columns() == R"(("b"))"); +} + +TEST_CASE("check various broken index configs", "[NoDB]") +{ + test_framework tf; + + flex_table_t table{"test_table"}; + table.add_column("col", "text", ""); + + SECTION("empty index description") { REQUIRE(tf.run_lua("return {}")); } + + SECTION("missing method") + { + REQUIRE(tf.run_lua("return { column = 'col' }")); + } + SECTION("non-existent method") + { + REQUIRE(tf.run_lua("return { method = 'abc', column = 'col' }")); + } + SECTION("wrong type for method") + { + REQUIRE(tf.run_lua("return { method = 123, column = 'col' }")); + } + SECTION("non-existent column") + { + REQUIRE(tf.run_lua("return { method = 'btree', column = 'x' }")); + } + SECTION("wrong type for column") + { + REQUIRE(tf.run_lua("return { method = 'btree', column = true }")); + } + SECTION("empty array for column") + { + REQUIRE(tf.run_lua("return { method = 'btree', column = {} }")); + } + SECTION("wrong type for expression") + { + REQUIRE(tf.run_lua("return { method = 'btree', expression = true }")); + } + SECTION("column and expression") + { + REQUIRE(tf.run_lua("return { method = 'btree', column = 'col', " + "expression = 'lower(col)' }")); + } + SECTION("non-existent tablespace") + { + REQUIRE(tf.run_lua( + "return { method = 'btree', column = 'col', tablespace = 'not' }")); + } + SECTION("wrong type for tablespace") + { + REQUIRE(tf.run_lua( + "return { method = 'btree', column = 'col', tablespace = 1.3 }")); + } + SECTION("wrong type for unique") + { + REQUIRE(tf.run_lua( + "return { method = 'btree', column = 'col', unique = 1 }")); + } + SECTION("wrong type for where condition") + { + REQUIRE(tf.run_lua( + "return { method = 'btree', column = 'col', where = {} }")); + } + SECTION("wrong type for include") + { + REQUIRE(tf.run_lua( + "return { btree = 'btree', column = 'col', include = 1.2 }")); + } + SECTION("unknown column for include") + { + REQUIRE(tf.run_lua( + "return { btree = 'btree', column = 'col', include = 'foo' }")); + } + + REQUIRE_THROWS(flex_lua_setup_index(tf.lua_state(), &table)); +} From e862650f17c379fc5a356b0d3662658f1acac891 Mon Sep 17 00:00:00 2001 From: Jochen Topf Date: Mon, 12 Dec 2022 14:19:33 +0100 Subject: [PATCH 2/2] Refactor flex_lua_setup_index() code To avoid having to count numbers of values on the Lua stack. --- src/flex-lua-index.cpp | 35 +++++++++++++++++++++-------------- 1 file changed, 21 insertions(+), 14 deletions(-) diff --git a/src/flex-lua-index.cpp b/src/flex-lua-index.cpp index f20b0053f..f6481ae84 100644 --- a/src/flex-lua-index.cpp +++ b/src/flex-lua-index.cpp @@ -49,16 +49,18 @@ static void check_and_add_columns(flex_table_t const &table, void flex_lua_setup_index(lua_State *lua_state, flex_table_t *table) { + // get method char const *const method = luaX_get_table_string(lua_state, "method", -1, "Index definition"); if (!has_index_method(method)) { throw std::runtime_error{"Unknown index method '{}'."_format(method)}; } - + lua_pop(lua_state, 1); auto &index = table->add_index(method); + // get columns std::vector columns; - lua_getfield(lua_state, -2, "column"); + lua_getfield(lua_state, -1, "column"); if (lua_isstring(lua_state, -1)) { check_and_add_column(*table, &columns, lua_tostring(lua_state, -1)); index.set_columns(columns); @@ -75,19 +77,21 @@ void flex_lua_setup_index(lua_State *lua_state, flex_table_t *table) "The 'column' field in an index definition must contain a " "string or an array."}; } + lua_pop(lua_state, 1); + // get expression std::string const expression = luaX_get_table_string( - lua_state, "expression", -3, "Index definition", ""); - - index.set_expression(expression); - + lua_state, "expression", -1, "Index definition", ""); + lua_pop(lua_state, 1); if (expression.empty() == columns.empty()) { throw std::runtime_error{"You must set either the 'column' or the " "'expression' field in index definition."}; } + index.set_expression(expression); + // get include columns std::vector include_columns; - lua_getfield(lua_state, -4, "include"); + lua_getfield(lua_state, -1, "include"); if (get_database_version() >= 110000) { if (lua_isstring(lua_state, -1)) { check_and_add_column(*table, &include_columns, @@ -105,9 +109,12 @@ void flex_lua_setup_index(lua_State *lua_state, flex_table_t *table) "Database version ({}) doesn't support" " include columns in indexes."_format(get_database_version())}; } + lua_pop(lua_state, 1); + // get tablespace std::string const tablespace = luaX_get_table_string( - lua_state, "tablespace", -5, "Index definition", ""); + lua_state, "tablespace", -1, "Index definition", ""); + lua_pop(lua_state, 1); check_identifier(tablespace, "tablespace"); if (!has_tablespace(tablespace)) { throw std::runtime_error{"Unknown tablespace '{}'."_format(tablespace)}; @@ -115,13 +122,13 @@ void flex_lua_setup_index(lua_State *lua_state, flex_table_t *table) index.set_tablespace(tablespace.empty() ? table->index_tablespace() : tablespace); - index.set_is_unique(luaX_get_table_bool(lua_state, "unique", -6, + // get unique + index.set_is_unique(luaX_get_table_bool(lua_state, "unique", -1, "Index definition", false)); + lua_pop(lua_state, 1); + // get where condition index.set_where_condition( - luaX_get_table_string(lua_state, "where", -7, "Index definition", "")); - - // stack has: "where", "unique", "tablespace", "includes", "expression", - // "column", "method" - lua_pop(lua_state, 7); + luaX_get_table_string(lua_state, "where", -1, "Index definition", "")); + lua_pop(lua_state, 1); }