Add 'new_sql_schema' config option, --enable-new-sql-schema now sets its default value to true (#2239)

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
 

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].

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),
+                                    transform_sql(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);
                                 _ ->
                                     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),
+                                    transform_upsert(Form, TableArg, FieldsArg);
-                                    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);
                                 _ ->
                                     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),
+                                    transform_insert(Form, TableArg, FieldsArg);
-                                    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);
                                 _ ->
                                     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) ->
+transform_sql(Arg) ->
-    parse1(S, [], #state{loc = Loc}).
+    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) ->
+transform_upsert(Form, TableArg, FieldsArg) ->
-    parse1(S, [], #state{loc = Loc, param_pos = ParamPos}).
+    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
+    lists:filter(
-        true ->
+      fun({Field, _Match, _ST}) ->
-            {ParseRes, []};
+              Field /= "server_host" orelse Table == "route"
-        false ->
+      end, ParseRes).
-            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]).
 
 -ifdef(ENABLE_PT_WARNINGS).
 

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='">>,

src/mod_roster.erl

@@ -1174,18 +1174,18 @@ import_stop(_LServer, _DBType) ->
     ets:delete(rostergroups_tmp),
     ok.
 
--ifdef(NEW_SQL_SCHEMA).
+row_length() ->
--define(ROW_LENGTH, 10).
+    case ejabberd_sql:use_new_schema() of
--else.
+        true -> 10;
--define(ROW_LENGTH, 9).
+        false -> 9
--endif.
+    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),

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