From e15595df64f96b6dd0dc6e9b38f9cdbdc0adf802 Mon Sep 17 00:00:00 2001 From: Alexey Shchepin Date: Thu, 15 Mar 2018 17:55:05 +0300 Subject: [PATCH] Add 'new_sql_schema' config option, --enable-new-sql-schema now sets its default value to true (#2239) --- ejabberd.yml.example | 5 ++ src/ejabberd_sql.erl | 14 ++- src/ejabberd_sql_pt.erl | 195 +++++++++++++++++++++------------------- src/mod_mam_sql.erl | 10 +-- src/mod_roster.erl | 12 +-- src/mod_vcard_sql.erl | 8 +- 6 files changed, 132 insertions(+), 112 deletions(-) diff --git a/ejabberd.yml.example b/ejabberd.yml.example index 28b238057..00f272733 100644 --- a/ejabberd.yml.example +++ b/ejabberd.yml.example @@ -445,6 +445,11 @@ auth_method: internal ## ## sql_keepalive_interval: undefined +## +## Use the new SQL schema +## +## new_sql_schema: true + ###. =============== ###' TRAFFIC SHAPERS diff --git a/src/ejabberd_sql.erl b/src/ejabberd_sql.erl index 1db61b70d..675941aea 100644 --- a/src/ejabberd_sql.erl +++ b/src/ejabberd_sql.erl @@ -39,6 +39,7 @@ sql_bloc/2, abort/1, restart/1, + use_new_schema/0, sql_query_to_iolist/1, escape/1, standard_escape/1, @@ -95,6 +96,12 @@ -define(PREPARE_KEY, ejabberd_sql_prepare). +-ifdef(NEW_SQL_SCHEMA). +-define(USE_NEW_SCHEMA_DEFAULT, true). +-else. +-define(USE_NEW_SCHEMA_DEFAULT, false). +-endif. + %%-define(DBGFSM, true). -ifdef(DBGFSM). @@ -272,6 +279,9 @@ sqlite_file(Host) -> binary_to_list(File) end. +use_new_schema() -> + ejabberd_config:get_option(new_sql_schema, ?USE_NEW_SCHEMA_DEFAULT). + %%%---------------------------------------------------------------------- %%% Callback functions from gen_fsm %%%---------------------------------------------------------------------- @@ -1133,9 +1143,11 @@ opt_type(sql_connect_timeout) -> fun (I) when is_integer(I), I > 0 -> I end; opt_type(sql_queue_type) -> fun(ram) -> ram; (file) -> file end; +opt_type(new_sql_schema) -> fun(B) when is_boolean(B) -> B end; opt_type(_) -> [sql_database, sql_keepalive_interval, sql_password, sql_port, sql_server, sql_username, sql_ssl, sql_ssl_verify, sql_ssl_certfile, sql_ssl_cafile, sql_queue_type, sql_query_timeout, - sql_connect_timeout]. + sql_connect_timeout, + new_sql_schema]. diff --git a/src/ejabberd_sql_pt.erl b/src/ejabberd_sql_pt.erl index 10697f692..eb7905bf0 100644 --- a/src/ejabberd_sql_pt.erl +++ b/src/ejabberd_sql_pt.erl @@ -28,7 +28,7 @@ %% API -export([parse_transform/2, format_error/1]). --export([parse/2]). +%-export([parse/2]). -include("ejabberd_sql_pt.hrl"). @@ -41,7 +41,8 @@ res_vars = [], res_pos = 0, server_host_used = false, - used_vars = []}). + used_vars = [], + use_new_schema}). -define(QUERY_RECORD, "sql_query"). @@ -88,26 +89,7 @@ transform(Form) -> [Arg] -> case erl_syntax:type(Arg) of string -> - S = erl_syntax:string_value(Arg), - Pos = erl_syntax:get_pos(Arg), - ParseRes = parse(S, Pos), - UnusedVars = - case ParseRes#state.server_host_used of - {true, SHVar} -> - case ?USE_NEW_SCHEMA of - true -> []; - false -> [SHVar] - end; - false -> - add_warning( - Pos, no_server_host), - [] - end, - set_pos( - add_unused_vars( - make_sql_query(ParseRes), - UnusedVars), - Pos); + transform_sql(Arg); _ -> throw({error, erl_syntax:get_pos(Form), "?SQL argument must be " @@ -123,26 +105,7 @@ transform(Form) -> case {erl_syntax:type(TableArg), erl_syntax:is_proper_list(FieldsArg)}of {string, true} -> - Table = erl_syntax:string_value(TableArg), - ParseRes = - parse_upsert( - erl_syntax:list_elements(FieldsArg)), - Pos = erl_syntax:get_pos(Form), - case lists:keymember( - "server_host", 1, ParseRes) of - true -> - ok; - false -> - add_warning(Pos, no_server_host) - end, - {ParseRes2, UnusedVars} = - filter_upsert_sh(Table, ParseRes), - set_pos( - add_unused_vars( - make_sql_upsert(Table, ParseRes2, Pos), - UnusedVars - ), - Pos); + transform_upsert(Form, TableArg, FieldsArg); _ -> throw({error, erl_syntax:get_pos(Form), "?SQL_UPSERT arguments must be " @@ -158,26 +121,7 @@ transform(Form) -> case {erl_syntax:type(TableArg), erl_syntax:is_proper_list(FieldsArg)}of {string, true} -> - Table = erl_syntax:string_value(TableArg), - ParseRes = - parse_insert( - erl_syntax:list_elements(FieldsArg)), - Pos = erl_syntax:get_pos(Form), - case lists:keymember( - "server_host", 1, ParseRes) of - true -> - ok; - false -> - add_warning(Pos, no_server_host) - end, - {ParseRes2, UnusedVars} = - filter_upsert_sh(Table, ParseRes), - set_pos( - add_unused_vars( - make_sql_insert(Table, ParseRes2), - UnusedVars - ), - Pos); + transform_insert(Form, TableArg, FieldsArg); _ -> throw({error, erl_syntax:get_pos(Form), "?SQL_INSERT arguments must be " @@ -226,11 +170,81 @@ top_transform(Forms) when is_list(Forms) -> end end, Forms). -parse(S, Loc) -> - parse1(S, [], #state{loc = Loc}). +transform_sql(Arg) -> + S = erl_syntax:string_value(Arg), + Pos = erl_syntax:get_pos(Arg), + ParseRes = parse(S, Pos, true), + ParseResOld = parse(S, Pos, false), + case ParseRes#state.server_host_used of + {true, _SHVar} -> + ok; + false -> + add_warning( + Pos, no_server_host), + [] + end, + set_pos( + make_schema_check( + make_sql_query(ParseRes), + make_sql_query(ParseResOld) + ), + Pos). -parse(S, ParamPos, Loc) -> - parse1(S, [], #state{loc = Loc, param_pos = ParamPos}). +transform_upsert(Form, TableArg, FieldsArg) -> + Table = erl_syntax:string_value(TableArg), + ParseRes = + parse_upsert( + erl_syntax:list_elements(FieldsArg)), + Pos = erl_syntax:get_pos(Form), + case lists:keymember( + "server_host", 1, ParseRes) of + true -> + ok; + false -> + add_warning(Pos, no_server_host) + end, + ParseResOld = + filter_upsert_sh(Table, ParseRes), + set_pos( + make_schema_check( + make_sql_upsert(Table, ParseRes, Pos), + make_sql_upsert(Table, ParseResOld, Pos) + ), + Pos). + +transform_insert(Form, TableArg, FieldsArg) -> + Table = erl_syntax:string_value(TableArg), + ParseRes = + parse_insert( + erl_syntax:list_elements(FieldsArg)), + Pos = erl_syntax:get_pos(Form), + case lists:keymember( + "server_host", 1, ParseRes) of + true -> + ok; + false -> + add_warning(Pos, no_server_host) + end, + ParseResOld = + filter_upsert_sh(Table, ParseRes), + set_pos( + make_schema_check( + make_sql_insert(Table, ParseRes), + make_sql_insert(Table, ParseResOld) + ), + Pos). + + +parse(S, Loc, UseNewSchema) -> + parse1(S, [], + #state{loc = Loc, + use_new_schema = UseNewSchema}). + +parse(S, ParamPos, Loc, UseNewSchema) -> + parse1(S, [], + #state{loc = Loc, + param_pos = ParamPos, + use_new_schema = UseNewSchema}). parse1([], Acc, State) -> State1 = append_string(lists:reverse(Acc), State), @@ -274,7 +288,7 @@ parse1([$%, $( | S], Acc, State) -> State3 = State2#state{server_host_used = {true, Name}, used_vars = [Name | State2#state.used_vars]}, - case ?USE_NEW_SCHEMA of + case State#state.use_new_schema of true -> Convert = erl_syntax:application( @@ -350,7 +364,7 @@ make_var(V) -> make_sql_query(State) -> - Hash = erlang:phash2(State#state{loc = undefined}), + Hash = erlang:phash2(State#state{loc = undefined, use_new_schema = true}), SHash = <<"Q", (integer_to_binary(Hash))/binary>>, Query = pack_query(State#state.'query'), EQuery = @@ -442,7 +456,7 @@ parse_upsert_field1([], _Acc, _ParamPos, Loc) -> "?SQL_UPSERT fields must have the " "following form: \"[!-]name=value\""}); parse_upsert_field1([$= | S], Acc, ParamPos, Loc) -> - {lists:reverse(Acc), parse(S, ParamPos, Loc)}; + {lists:reverse(Acc), parse(S, ParamPos, Loc, true)}; parse_upsert_field1([C | S], Acc, ParamPos, Loc) -> parse_upsert_field1(S, [C | Acc], ParamPos, Loc). @@ -632,7 +646,7 @@ parse_insert_field1([], _Acc, _ParamPos, Loc) -> "?SQL_INSERT fields must have the " "following form: \"name=value\""}); parse_insert_field1([$= | S], Acc, ParamPos, Loc) -> - {lists:reverse(Acc), parse(S, ParamPos, Loc)}; + {lists:reverse(Acc), parse(S, ParamPos, Loc, true)}; parse_insert_field1([C | S], Acc, ParamPos, Loc) -> parse_insert_field1(S, [C | Acc], ParamPos, Loc). @@ -640,6 +654,23 @@ parse_insert_field1([C | S], Acc, ParamPos, Loc) -> make_sql_insert(Table, ParseRes) -> make_sql_query(make_sql_upsert_insert(Table, ParseRes)). +make_schema_check(Tree, Tree) -> + Tree; +make_schema_check(New, Old) -> + erl_syntax:case_expr( + erl_syntax:application( + erl_syntax:atom(ejabberd_sql), + erl_syntax:atom(use_new_schema), + []), + [erl_syntax:clause( + [erl_syntax:abstract(true)], + none, + [New]), + erl_syntax:clause( + [erl_syntax:abstract(false)], + none, + [Old])]). + concat_states(States) -> lists:foldr( @@ -711,26 +742,10 @@ set_pos(Tree, Pos) -> end, Tree). filter_upsert_sh(Table, ParseRes) -> - case ?USE_NEW_SCHEMA of - true -> - {ParseRes, []}; - false -> - lists:foldr( - fun({Field, _Match, ST} = P, {Acc, Vars}) -> - if - Field /= "server_host" orelse Table == "route" -> - {[P | Acc], Vars}; - true -> - {Acc, ST#state.used_vars ++ Vars} - end - end, {[], []}, ParseRes) - end. - -add_unused_vars(Tree, []) -> - Tree; -add_unused_vars(Tree, Vars) -> - erl_syntax:block_expr( - lists:map(fun erl_syntax:variable/1, Vars) ++ [Tree]). + lists:filter( + fun({Field, _Match, _ST}) -> + Field /= "server_host" orelse Table == "route" + end, ParseRes). -ifdef(ENABLE_PT_WARNINGS). diff --git a/src/mod_mam_sql.erl b/src/mod_mam_sql.erl index 174cc8de1..1609ff79e 100644 --- a/src/mod_mam_sql.erl +++ b/src/mod_mam_sql.erl @@ -38,12 +38,6 @@ -include("logger.hrl"). -include("ejabberd_sql_pt.hrl"). --ifdef(NEW_SQL_SCHEMA). --define(USE_NEW_SCHEMA, true). --else. --define(USE_NEW_SCHEMA, false). --endif. - %%%=================================================================== %%% API %%%=================================================================== @@ -332,7 +326,7 @@ make_sql_query(User, LServer, MAMQuery, RSM) -> SServer = Escape(LServer), Query = - case ?USE_NEW_SCHEMA of + case ejabberd_sql:use_new_schema() of true -> [<<"SELECT ">>, TopClause, <<" timestamp, xml, peer, kind, nick" @@ -361,7 +355,7 @@ make_sql_query(User, LServer, MAMQuery, RSM) -> [Query, <<" ORDER BY timestamp ASC ">>, LimitClause, <<";">>] end, - case ?USE_NEW_SCHEMA of + case ejabberd_sql:use_new_schema() of true -> {QueryPage, [<<"SELECT COUNT(*) FROM archive WHERE username='">>, diff --git a/src/mod_roster.erl b/src/mod_roster.erl index 3b893d21d..f1c4453db 100644 --- a/src/mod_roster.erl +++ b/src/mod_roster.erl @@ -1174,18 +1174,18 @@ import_stop(_LServer, _DBType) -> ets:delete(rostergroups_tmp), ok. --ifdef(NEW_SQL_SCHEMA). --define(ROW_LENGTH, 10). --else. --define(ROW_LENGTH, 9). --endif. +row_length() -> + case ejabberd_sql:use_new_schema() of + true -> 10; + false -> 9 + end. import(LServer, {sql, _}, _DBType, <<"rostergroups">>, [LUser, SJID, Group]) -> LJID = jid:tolower(jid:decode(SJID)), ets:insert(rostergroups_tmp, {{LUser, LServer, LJID}, Group}), ok; import(LServer, {sql, _}, DBType, <<"rosterusers">>, Row) -> - I = mod_roster_sql:raw_to_record(LServer, lists:sublist(Row, ?ROW_LENGTH)), + I = mod_roster_sql:raw_to_record(LServer, lists:sublist(Row, row_length())), Groups = [G || {_, G} <- ets:lookup(rostergroups_tmp, I#roster.usj)], RosterItem = I#roster{groups = Groups}, Mod = gen_mod:db_mod(DBType, ?MODULE), diff --git a/src/mod_vcard_sql.erl b/src/mod_vcard_sql.erl index ce7506b6b..57d2052a0 100644 --- a/src/mod_vcard_sql.erl +++ b/src/mod_vcard_sql.erl @@ -39,12 +39,6 @@ -include("ejabberd_sql_pt.hrl"). -include("translate.hrl"). --ifdef(NEW_SQL_SCHEMA). --define(USE_NEW_SCHEMA, true). --else. --define(USE_NEW_SCHEMA, false). --endif. - %%%=================================================================== %%% API %%%=================================================================== @@ -268,7 +262,7 @@ make_matchspec(LServer, Data) -> filter_fields(Data, <<"">>, LServer). filter_fields([], Match, LServer) -> - case ?USE_NEW_SCHEMA of + case ejabberd_sql:use_new_schema() of true -> SServer = ejabberd_sql:escape(LServer), case Match of