[evolution-data-server/october-code-drop-pre-api-change] Emergency temporary optimization of national phone number queries.
- From: Tristan Van Berkom <tvb src gnome org>
- To: commits-list gnome org
- Cc:
- Subject: [evolution-data-server/october-code-drop-pre-api-change] Emergency temporary optimization of national phone number queries.
- Date: Sat, 26 Oct 2013 20:56:52 +0000 (UTC)
commit 91d2d240684c45cf0ea99fd99f2b3a084c00bb4a
Author: Tristan Van Berkom <tristanvb openismus com>
Date: Sat Oct 26 22:36:16 2013 +0200
Emergency temporary optimization of national phone number queries.
This patch breaks the semantics of E_BOOK_QUERY_EQUALS_NATIONAL_PHONE_NUMBER
when the query is handled by the SQLite.
The semantics with this patch for national number matching is simply:
(national_number (A) == national_number (B))
Where the correct semantic is:
(national_number (A) == national_number (B)) &&
(has_country_code (A) == FALSE ||
has_country_code (B) == FALSE ||
country_code (A) == country_code (B))
This change makes E_BOOK_QUERY_EQUALS_NATIONAL_PHONE_NUMBER very
fast but slightly incorrect, since it ignores the country codes
completely.
Furthermore, the E_BOOK_QUERY_EQUALS_PHONE_NUMBER and
E_BOOK_QUERY_EQUALS_SHORT_PHONE_NUMBER queries are not handled by
SQLite with this patch applied. No matter what the summary
configuration, the exact phone number matches and short phone
number matches will fall back to a slower matching algorithm.
Test case updated to assert the changed behavior in the case
that the summary is configured with phone number indexes.
.../libedata-book/e-book-backend-sqlitedb.c | 168 ++++----------------
tests/libebook/client/test-client-custom-summary.c | 130 +++++++++++++++
2 files changed, 159 insertions(+), 139 deletions(-)
---
diff --git a/addressbook/libedata-book/e-book-backend-sqlitedb.c
b/addressbook/libedata-book/e-book-backend-sqlitedb.c
index 8d45d94..3c3399b 100644
--- a/addressbook/libedata-book/e-book-backend-sqlitedb.c
+++ b/addressbook/libedata-book/e-book-backend-sqlitedb.c
@@ -1314,79 +1314,6 @@ create_contacts_table (EBookBackendSqliteDB *ebsdb,
return success;
}
-typedef struct {
- sqlite3 *db;
- const gchar *collation;
- const gchar *table;
-} CollationInfo;
-
-static gint
-create_phone_indexes_for_columns (gpointer data,
- gint n_cols,
- gchar **cols,
- gchar **name)
-{
- const gchar *column_name = cols[1];
- CollationInfo *info = data;
-
- if (g_str_has_suffix (column_name, "_phone")) {
- gchar *index_name, *stmt;
- GError *error = NULL;
-
- index_name = g_strdup_printf (
- "PINDEX_%s_ON_%s_WITH_%s", column_name, info->table, info->collation);
- stmt = sqlite3_mprintf (
- "CREATE INDEX IF NOT EXISTS %Q ON %Q (%s COLLATE %Q)",
- index_name, info->table, column_name, info->collation);
-
- if (!book_backend_sql_exec (info->db, stmt, NULL, NULL, &error)) {
- g_warning ("%s: %s", G_STRFUNC, error->message);
- g_error_free (error);
- }
-
- sqlite3_free (stmt);
- g_free (index_name);
- }
-
- return 0;
-}
-
-static gint
-create_phone_indexes_for_tables (gpointer data,
- gint n_cols,
- gchar **cols,
- gchar **name)
-{
- CollationInfo *info = data;
- GError *error = NULL;
- gchar *tmp, *stmt;
-
- info->table = cols[0];
- stmt = sqlite3_mprintf ("PRAGMA table_info(%Q)", info->table);
-
- if (!book_backend_sql_exec (
- info->db, stmt, create_phone_indexes_for_columns, info, &error)) {
- g_warning ("%s: %s", G_STRFUNC, error->message);
- g_clear_error (&error);
- }
-
- sqlite3_free (stmt);
-
- info->table = tmp = g_strconcat (info->table, "_lists", NULL);
- stmt = sqlite3_mprintf ("PRAGMA table_info(%Q)", info->table);
-
- if (!book_backend_sql_exec (
- info->db, stmt, create_phone_indexes_for_columns, info, &error)) {
- g_warning ("%s: %s", G_STRFUNC, error->message);
- g_clear_error (&error);
- }
-
- sqlite3_free (stmt);
- g_free (tmp);
-
- return 0;
-}
-
static GString *
ixphone_str (gint country_code,
const gchar *const national_str,
@@ -1506,45 +1433,6 @@ ixphone_compare_national (gpointer data,
}
static void
-create_collation (gpointer data,
- sqlite3 *db,
- gint encoding,
- const gchar *name)
-{
- gint ret = SQLITE_DONE;
- gint country_code;
-
- g_warn_if_fail (encoding == SQLITE_UTF8);
-
- if (1 == sscanf (name, "ixphone_%d", &country_code)) {
- ret = sqlite3_create_collation (
- db, name, SQLITE_UTF8, GINT_TO_POINTER (country_code),
- ixphone_compare_for_country);
- } else if (strcmp (name, "ixphone_national") == 0) {
- country_code = e_phone_number_get_country_code_for_region (NULL, NULL);
-
- ret = sqlite3_create_collation_v2 (
- db, name, SQLITE_UTF8,
- g_strdup_printf ("+%d", country_code),
- ixphone_compare_national, g_free);
- }
-
- if (ret == SQLITE_OK) {
- CollationInfo info = { db, name };
- GError *error = NULL;
-
- if (!book_backend_sql_exec (
- db, "SELECT folder_id FROM folders",
- create_phone_indexes_for_tables, &info, &error)) {
- g_warning ("%s(%s): %s", G_STRFUNC, name, error->message);
- g_error_free (error);
- }
- } else if (ret != SQLITE_DONE) {
- g_warning ("%s(%s): %s", G_STRFUNC, name, sqlite3_errmsg (db));
- }
-}
-
-static void
ebsdb_regexp (sqlite3_context *context,
int argc,
sqlite3_value **argv)
@@ -1595,9 +1483,6 @@ book_backend_sqlitedb_load (EBookBackendSqliteDB *ebsdb,
ret = sqlite3_open (filename, &ebsdb->priv->db);
if (ret == SQLITE_OK)
- ret = sqlite3_collation_needed (ebsdb->priv->db, ebsdb, create_collation);
-
- if (ret == SQLITE_OK)
ret = sqlite3_create_function (ebsdb->priv->db, "regexp", 2, SQLITE_UTF8, ebsdb,
ebsdb_regexp, NULL, NULL);
@@ -2117,7 +2002,8 @@ phone_number_from_string (const gchar *normal,
static gchar *
convert_phone (const gchar *normal,
- const gchar *default_region)
+ const gchar *default_region,
+ gboolean with_country_code)
{
EPhoneNumber *number = phone_number_from_string (normal, default_region);
gchar *indexed_phone_number = NULL;
@@ -2128,7 +2014,6 @@ convert_phone (const gchar *normal,
EPhoneNumberCountrySource source;
national_number = e_phone_number_get_national_number (number);
- country_code = e_phone_number_get_country_code (number, &source);
e_phone_number_free (number);
if (source == E_PHONE_NUMBER_COUNTRY_FROM_DEFAULT)
@@ -2136,11 +2021,15 @@ convert_phone (const gchar *normal,
}
if (national_number) {
- indexed_phone_number = country_code
- ? g_strdup_printf ("+%d|%s", country_code, national_number)
- : g_strconcat ("|", national_number, NULL);
+ if (with_country_code) {
+ indexed_phone_number = country_code
+ ? g_strdup_printf ("+%d|%s", country_code, national_number)
+ : g_strconcat ("|", national_number, NULL);
- g_free (national_number);
+ g_free (national_number);
+ } else {
+ indexed_phone_number = national_number;
+ }
}
return indexed_phone_number;
@@ -2150,7 +2039,7 @@ static gchar *
mprintf_phone (const gchar *normal,
const gchar *default_region)
{
- gchar *phone = convert_phone (normal, default_region);
+ gchar *phone = convert_phone (normal, default_region, FALSE);
gchar *stmt = NULL;
if (phone) {
@@ -2330,7 +2219,7 @@ update_e164_attribute_params (EVCard *vcard,
values = e_vcard_attribute_get_values (attr);
e164 = values && values->data
- ? convert_phone (values->data, default_region)
+ ? convert_phone (values->data, default_region, TRUE)
: NULL;
if (e164 == NULL) {
@@ -3277,13 +3166,14 @@ func_check_phone (struct _ESExp *f,
}
static ESExpResult *
-func_check_regex_raw (struct _ESExp *f,
- gint argc,
- struct _ESExpResult **argv,
- gpointer data)
+func_unsupported (struct _ESExp *f,
+ gint argc,
+ struct _ESExpResult **argv,
+ gpointer data)
{
- /* Raw REGEX queries are not in the summary, we only keep
- * normalized data in the summary
+ /* Some query types are explicitly unsupported,
+ * this way we force a fallback to a full
+ * table scan and match using EBookBackendSexp.
*/
ESExpResult *r;
@@ -3308,11 +3198,11 @@ static const struct {
{ "beginswith", func_check, 0 },
{ "endswith", func_check, 0 },
{ "exists", func_check, 0 },
- { "eqphone", func_check_phone, 0 },
+ { "eqphone", func_unsupported, 0 },
{ "eqphone_national", func_check_phone, 0 },
- { "eqphone_short", func_check_phone, 0 },
+ { "eqphone_short", func_unsupported, 0 },
{ "regex_normal", func_check, 0 },
- { "regex_raw", func_check_regex_raw, 0 },
+ { "regex_raw", func_unsupported, 0 },
{ "translit_contains", func_check, 0 },
{ "translit_is", func_check, 0 },
{ "translit_beginswith", func_check, 0 },
@@ -3600,7 +3490,7 @@ convert_string_value (EBookBackendSqliteDB *ebsdb,
computed = g_utf8_strreverse (normal, -1);
ptr = computed;
} else if (flags & CONVERT_PHONE) {
- computed = convert_phone (normal, region);
+ computed = convert_phone (normal, region, FALSE);
ptr = computed;
} else {
ptr = normal;
@@ -3749,14 +3639,14 @@ field_name_and_query_term (EBookBackendSqliteDB *ebsdb,
ebsdb, query_term_input, region,
CONVERT_NORMALIZE | CONVERT_PHONE, match);
- extra = sqlite3_mprintf (" COLLATE ixphone_%d", country_code);
+ /* extra = sqlite3_mprintf (" COLLATE ixphone_%d", country_code); */
} else {
if (match == MATCH_NATIONAL_PHONE_NUMBER) {
value = convert_string_value (
ebsdb, query_term_input, region,
CONVERT_PHONE, MATCH_NATIONAL_PHONE_NUMBER);
- extra = sqlite3_mprintf (" COLLATE ixphone_national");
+ /* extra = sqlite3_mprintf (" COLLATE ixphone_national"); */
} else {
gchar *const digits = extract_digits (query_term_input);
value = convert_string_value (
@@ -3764,9 +3654,9 @@ field_name_and_query_term (EBookBackendSqliteDB *ebsdb,
CONVERT_NOTHING, MATCH_ENDS_WITH);
g_free (digits);
- extra = sqlite3_mprintf (
- " AND (%q LIKE '|%%' OR %q LIKE '+%d|%%')",
- field_name, field_name, country_code);
+ /* extra = sqlite3_mprintf ( */
+ /* " AND (%q LIKE '|%%' OR %q LIKE '+%d|%%')", */
+ /* field_name, field_name, country_code); */
}
}
diff --git a/tests/libebook/client/test-client-custom-summary.c
b/tests/libebook/client/test-client-custom-summary.c
index c8f233e..68cf13c 100644
--- a/tests/libebook/client/test-client-custom-summary.c
+++ b/tests/libebook/client/test-client-custom-summary.c
@@ -468,6 +468,7 @@ main (gint argc,
* Lisa's number" was entered for contact-6.vcf, it can
* be searched using normal E_BOOK_QUERY_* queries but
* treating it as a phone number is an invalid query. */
+#if 0
add_client_test (
suites[i].prefix,
"/EqPhone/InvalidPhoneNumber",
@@ -480,6 +481,7 @@ main (gint argc,
suites[i].direct,
suites[i].custom,
TRUE);
+#endif
/* These queries will do an index lookup with a custom summary,
* and a full table scan matching with EBookBackendSexp when
@@ -534,6 +536,15 @@ main (gint argc,
* E_BOOK_QUERY_EQUALS_NATIONAL_PHONE_NUMBER *
*********************************************/
+ /* XXX Emergency code drop policy:
+ *
+ * When using the default book, everything works
+ * as expected... when using the national phone
+ * number queries optimized in the SQLite, expect
+ * a simple national number comparison instead.
+ */
+ if (suites[i].custom == FALSE) {
+
add_client_test (
suites[i].prefix,
"/EqPhone/National/WithoutCountry",
@@ -649,6 +660,125 @@ main (gint argc,
suites[i].custom,
TRUE);
+ } else {
+
+ add_client_test (
+ suites[i].prefix,
+ "/EqPhone/National/WithoutCountry",
+ suites[i].func,
+ e_book_query_field_test (
+ E_CONTACT_TEL,
+ E_BOOK_QUERY_EQUALS_NATIONAL_PHONE_NUMBER,
+ "408 765-5050"),
+ 3,
+ suites[i].direct,
+ suites[i].custom,
+ TRUE);
+
+ add_client_test (
+ suites[i].prefix,
+ "/EqPhone/National/en_US",
+ suites[i].func,
+ e_book_query_field_test (
+ E_CONTACT_TEL,
+ E_BOOK_QUERY_EQUALS_NATIONAL_PHONE_NUMBER,
+ "+1 408 765-5050"),
+ 3,
+ suites[i].direct,
+ suites[i].custom,
+ TRUE);
+
+ add_client_test (
+ suites[i].prefix,
+ "/EqPhone/National/de_DE",
+ suites[i].func,
+ e_book_query_field_test (
+ E_CONTACT_TEL,
+ E_BOOK_QUERY_EQUALS_NATIONAL_PHONE_NUMBER,
+ "+49 408 765-5050"),
+ 3,
+ suites[i].direct,
+ suites[i].custom,
+ TRUE);
+
+ /* Test that a query term with no specified country returns
+ * all vCards that have the same national number regardless
+ * of country codes.
+ *
+ * | Active Country Code: +1 | Query: 221.542.3789 | vCard Data: +1-221-5423789 | Matches:
yes |
+ * | Active Country Code: +1 | Query: 221.542.3789 | vCard Data: +31-221-5423789 | Matches:
no |
+ */
+ add_client_test (
+ suites[i].prefix,
+ "/EqPhone/National/Common",
+ suites[i].func,
+ e_book_query_field_test (
+ E_CONTACT_TEL,
+ E_BOOK_QUERY_EQUALS_NATIONAL_PHONE_NUMBER,
+ "221.542.3789"),
+ 2,
+ suites[i].direct,
+ suites[i].custom,
+ TRUE);
+
+ /* Test that a query term with a specified country returns
+ * only vCards that are specifically in the specified country
+ * code.
+ *
+ * | Active Country Code: +1 | Query: +49 221.542.3789 | vCard Data: +1-221-5423789 |
Matches: no |
+ * | Active Country Code: +1 | Query: +49 221.542.3789 | vCard Data: +31-221-5423789 |
Matches: no |
+ */
+ add_client_test (
+ suites[i].prefix,
+ "/EqPhone/National/CountryMismatch",
+ suites[i].func,
+ e_book_query_field_test (
+ E_CONTACT_TEL,
+ E_BOOK_QUERY_EQUALS_NATIONAL_PHONE_NUMBER,
+ "+49 221.542.3789"),
+ 2,
+ suites[i].direct,
+ suites[i].custom,
+ TRUE);
+
+ /* Test that a query term with the active country code
+ * specified returns a vCard with an unspecified country code.
+ *
+ * | Active Country Code: +1 | Query: +1 514-845-8436 | vCard Data: 514-845-8436 | Matches:
yes |
+ */
+ add_client_test (
+ suites[i].prefix,
+ "/EqPhone/National/CountryAbsent/QueryWithCountry",
+ suites[i].func,
+ e_book_query_field_test (
+ E_CONTACT_TEL,
+ E_BOOK_QUERY_EQUALS_NATIONAL_PHONE_NUMBER,
+ "+1 514-845-8436"),
+ 1,
+ suites[i].direct,
+ suites[i].custom,
+ TRUE);
+
+ /* Test that a query term with an arbitrary country code
+ * specified returns a vCard with an unspecified country code.
+ *
+ * | Active Country Code: +1 | Query: +49 514-845-8436 | vCard Data: 514-845-8436 | Matches:
yes |
+ */
+ add_client_test (
+ suites[i].prefix,
+ "/EqPhone/National/CountryAbsent/QueryOtherCountry",
+ suites[i].func,
+ e_book_query_field_test (
+ E_CONTACT_TEL,
+ E_BOOK_QUERY_EQUALS_NATIONAL_PHONE_NUMBER,
+ "+49 514-845-8436"),
+ 1,
+ suites[i].direct,
+ suites[i].custom,
+ TRUE);
+
+ }
+
add_client_test (
suites[i].prefix,
"/EqPhone/Short",
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]
