Bug 1168606 - Reduced duplication for handling sort key comparisons in SQL queries and improved type-safety. r=ttung,asuth
authorSimon Giesecke <sgiesecke@mozilla.com>
Tue, 05 Nov 2019 16:45:41 +0000
changeset 500620 cf53f4e5731358d2d0fd8cfc8e86292e917e1c76
parent 500619 b525ad6838f091c2ab7cfc52cda593eca7941fa8
child 500621 1abd3e7f9df46911581ba653270bc8a8864374d8
push id36768
push usershindli@mozilla.com
push dateTue, 05 Nov 2019 22:07:34 +0000
treeherdermozilla-central@e96c1ca93d25 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersttung, asuth
bugs1168606
milestone72.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1168606 - Reduced duplication for handling sort key comparisons in SQL queries and improved type-safety. r=ttung,asuth Depends on D44009 Differential Revision: https://phabricator.services.mozilla.com/D44377
dom/indexedDB/ActorsParent.cpp
--- a/dom/indexedDB/ActorsParent.cpp
+++ b/dom/indexedDB/ActorsParent.cpp
@@ -293,16 +293,17 @@ constexpr auto kStmtParamNameFileIds = N
 constexpr auto kStmtParamNameValueLocale = NS_LITERAL_CSTRING("value_locale");
 constexpr auto kStmtParamNameLimit = NS_LITERAL_CSTRING("limit");
 
 // The following constants define some names of columns in tables, which are
 // referred to in remote locations, e.g. in calls to
 // GetBindingClauseForKeyRange.
 constexpr auto kColumnNameKey = NS_LITERAL_CSTRING("key");
 constexpr auto kColumnNameValue = NS_LITERAL_CSTRING("value");
+constexpr auto kColumnNameAliasSortKey = NS_LITERAL_CSTRING("sort_column");
 
 // SQL fragments used at multiple locations.
 constexpr auto kOpenLimit = NS_LITERAL_CSTRING(" LIMIT ");
 
 // The deletion marker file is created before RemoveDatabaseFilesAndDirectory
 // begins deleting a database. It is removed as the last step of deletion. If a
 // deletion marker file is found when initializing the origin, the deletion
 // routine is run again to ensure that the database and all of its related files
@@ -9357,16 +9358,59 @@ nsresult LocalizeKey(const Key& aBaseKey
   return NS_OK;
 }
 
 nsAutoCString ToAutoCString(const uint32_t aInt) {
   nsAutoCString result;
   result.AppendInt(aInt);
   return result;
 }
+
+enum struct ComparisonOperator {
+  LessThan,
+  LessOrEquals,
+  Equals,
+  GreaterThan,
+  GreaterOrEquals,
+};
+
+constexpr nsLiteralCString GetComparisonOperatorString(
+    const ComparisonOperator aComparisonOperator) {
+  switch (aComparisonOperator) {
+    case ComparisonOperator::LessThan:
+      return NS_LITERAL_CSTRING("<");
+    case ComparisonOperator::LessOrEquals:
+      return NS_LITERAL_CSTRING("<=");
+    case ComparisonOperator::Equals:
+      return NS_LITERAL_CSTRING("==");
+    case ComparisonOperator::GreaterThan:
+      return NS_LITERAL_CSTRING(">");
+    case ComparisonOperator::GreaterOrEquals:
+      return NS_LITERAL_CSTRING(">=");
+  }
+
+  // TODO: This is just to silence the "control reaches end of non-void
+  // function" warning. Cannot use MOZ_CRASH in a constexpr function,
+  // unfortunately.
+  return NS_LITERAL_CSTRING("");
+}
+
+nsAutoCString GetKeyClause(const nsCString& aColumnName,
+                           const ComparisonOperator aComparisonOperator,
+                           const nsLiteralCString& aStmtParamName) {
+  return aColumnName + NS_LITERAL_CSTRING(" ") +
+         GetComparisonOperatorString(aComparisonOperator) +
+         NS_LITERAL_CSTRING(" :") + aStmtParamName;
+}
+
+nsAutoCString GetSortKeyClause(const ComparisonOperator aComparisonOperator,
+                               const nsLiteralCString& aStmtParamName) {
+  return GetKeyClause(kColumnNameAliasSortKey, aComparisonOperator,
+                      aStmtParamName);
+}
 }  // namespace
 
 /*******************************************************************************
  * Exported functions
  ******************************************************************************/
 
 PBackgroundIDBFactoryParent* AllocPBackgroundIDBFactoryParent(
     const LoggingInfo& aLoggingInfo) {
@@ -18930,19 +18974,20 @@ nsresult DatabaseOperationBase::DeleteOb
     if (NS_WARN_IF(NS_FAILED(rv))) {
       return rv;
     }
   } else {
     const auto keyRangeClause =
         MaybeGetBindingClauseForKeyRange(aKeyRange, kColumnNameKey);
 
     rv = aConnection->GetCachedStatement(
-        NS_LITERAL_CSTRING("SELECT index_data_values, key "
-                           "FROM object_data "
-                           "WHERE object_store_id = :") +
+        NS_LITERAL_CSTRING("SELECT index_data_values, ") + kColumnNameKey +
+            NS_LITERAL_CSTRING(" "
+                               "FROM object_data "
+                               "WHERE object_store_id = :") +
             kStmtParamNameObjectStoreId + keyRangeClause +
             NS_LITERAL_CSTRING(";"),
         &selectStmt);
     if (NS_WARN_IF(NS_FAILED(rv))) {
       return rv;
     }
 
     if (aKeyRange.isSome()) {
@@ -25994,57 +26039,70 @@ void Cursor::OpenOp::PrepareIndexKeyCond
     mCursor->SetOptionalKeyRange(mOptionalKeyRange, &open);
     if (mOptionalKeyRange.isSome() &&
         !mCursor->mLocaleAwareRangeBound.IsUnset()) {
       AppendConditionClause(aSortColumn, kStmtParamNameRangeBound,
                             isIncreasingOrder, !open, aQueryStart);
     }
   }
 
-  const auto& comparisonChar =
-      isIncreasingOrder ? NS_LITERAL_CSTRING(">") : NS_LITERAL_CSTRING("<");
-
   mCursor->mContinueToQuery =
-      aQueryStart + NS_LITERAL_CSTRING(" AND sort_column ") + comparisonChar +
-      NS_LITERAL_CSTRING("= :") + kStmtParamNameCurrentKey;
+      aQueryStart + NS_LITERAL_CSTRING(" AND ") +
+      GetSortKeyClause(isIncreasingOrder ? ComparisonOperator::GreaterOrEquals
+                                         : ComparisonOperator::LessOrEquals,
+                       kStmtParamNameCurrentKey);
 
   switch (mCursor->mDirection) {
     case IDBCursor::NEXT:
     case IDBCursor::PREV:
       mCursor->mContinueQuery =
-          aQueryStart + NS_LITERAL_CSTRING(" AND sort_column ") +
-          comparisonChar + NS_LITERAL_CSTRING("= :") +
-          kStmtParamNameCurrentKey + NS_LITERAL_CSTRING(" AND ( sort_column ") +
-          comparisonChar + NS_LITERAL_CSTRING(" :") + kStmtParamNameCurrentKey +
-          NS_LITERAL_CSTRING(" OR ") + aObjectDataKeyPrefix +
-          NS_LITERAL_CSTRING("object_data_key ") + comparisonChar +
-          NS_LITERAL_CSTRING(" :") + kStmtParamNameObjectStorePosition +
+          aQueryStart + NS_LITERAL_CSTRING(" AND ") +
+          GetSortKeyClause(isIncreasingOrder
+                               ? ComparisonOperator::GreaterOrEquals
+                               : ComparisonOperator::LessOrEquals,
+                           kStmtParamNameCurrentKey) +
+          NS_LITERAL_CSTRING(" AND ( ") +
+          GetSortKeyClause(isIncreasingOrder ? ComparisonOperator::GreaterThan
+                                             : ComparisonOperator::LessThan,
+                           kStmtParamNameCurrentKey) +
+          NS_LITERAL_CSTRING(" OR ") +
+          GetKeyClause(
+              aObjectDataKeyPrefix + NS_LITERAL_CSTRING("object_data_key"),
+              isIncreasingOrder ? ComparisonOperator::GreaterThan
+                                : ComparisonOperator::LessThan,
+              kStmtParamNameObjectStorePosition) +
           NS_LITERAL_CSTRING(" ) ");
 
       mCursor->mContinuePrimaryKeyQuery =
           aQueryStart +
           NS_LITERAL_CSTRING(
               " AND ("
-              "(sort_column == :") +
-          kStmtParamNameCurrentKey + NS_LITERAL_CSTRING(" AND ") +
-          aObjectDataKeyPrefix + NS_LITERAL_CSTRING("object_data_key ") +
-          comparisonChar + NS_LITERAL_CSTRING("= :") +
-          kStmtParamNameObjectStorePosition +
-          NS_LITERAL_CSTRING(
-              ") OR "
-              "sort_column ") +
-          comparisonChar + NS_LITERAL_CSTRING(" :") + kStmtParamNameCurrentKey +
+              "(") +
+          GetSortKeyClause(ComparisonOperator::Equals,
+                           kStmtParamNameCurrentKey) +
+          NS_LITERAL_CSTRING(" AND ") +
+          GetKeyClause(
+              aObjectDataKeyPrefix + NS_LITERAL_CSTRING("object_data_key"),
+              isIncreasingOrder ? ComparisonOperator::GreaterOrEquals
+                                : ComparisonOperator::LessOrEquals,
+              kStmtParamNameObjectStorePosition) +
+          NS_LITERAL_CSTRING(") OR ") +
+          GetSortKeyClause(isIncreasingOrder ? ComparisonOperator::GreaterThan
+                                             : ComparisonOperator::LessThan,
+                           kStmtParamNameCurrentKey) +
           NS_LITERAL_CSTRING(")");
       break;
 
     case IDBCursor::NEXT_UNIQUE:
     case IDBCursor::PREV_UNIQUE:
       mCursor->mContinueQuery =
-          aQueryStart + NS_LITERAL_CSTRING(" AND sort_column ") +
-          comparisonChar + NS_LITERAL_CSTRING(" :") + kStmtParamNameCurrentKey;
+          aQueryStart + NS_LITERAL_CSTRING(" AND ") +
+          GetSortKeyClause(isIncreasingOrder ? ComparisonOperator::GreaterThan
+                                             : ComparisonOperator::LessThan,
+                           kStmtParamNameCurrentKey);
       break;
 
     default:
       MOZ_CRASH("Should never get here!");
   }
 
   const nsAutoCString suffix = aDirectionClause + kOpenLimit +
                                NS_LITERAL_CSTRING(":") + kStmtParamNameLimit;
@@ -26217,24 +26275,22 @@ nsresult Cursor::OpenOp::DoIndexDatabase
   AUTO_PROFILER_LABEL("Cursor::OpenOp::DoIndexDatabaseWork", DOM);
 
   const bool usingKeyRange = mOptionalKeyRange.isSome();
 
   const auto& indexTable = mCursor->mUniqueIndex
                                ? NS_LITERAL_CSTRING("unique_index_data")
                                : NS_LITERAL_CSTRING("index_data");
 
-  NS_NAMED_LITERAL_CSTRING(sortColumn, "sort_column");
-
   // The result of MakeColumnPairSelectionList is stored in a local variable,
   // since inlining it into the next statement causes a crash on some Mac OS X
   // builds (see https://bugzilla.mozilla.org/show_bug.cgi?id=1168606#c110).
   const auto columnPairSelectionList = MakeColumnPairSelectionList(
       NS_LITERAL_CSTRING("index_table.value"),
-      NS_LITERAL_CSTRING("index_table.value_locale"), sortColumn,
+      NS_LITERAL_CSTRING("index_table.value_locale"), kColumnNameAliasSortKey,
       mCursor->IsLocaleAware());
   const nsCString sortColumnAlias = NS_LITERAL_CSTRING("SELECT ") +
                                     columnPairSelectionList +
                                     NS_LITERAL_CSTRING(", ");
 
   nsAutoCString queryStart = sortColumnAlias +
                              NS_LITERAL_CSTRING(
                                  "index_table.object_data_key, "
@@ -26247,20 +26303,21 @@ nsresult Cursor::OpenOp::DoIndexDatabase
                                  "JOIN object_data "
                                  "ON index_table.object_store_id = "
                                  "object_data.object_store_id "
                                  "AND index_table.object_data_key = "
                                  "object_data.key "
                                  "WHERE index_table.index_id = :") +
                              kStmtParamNameId;
 
-  const auto keyRangeClause =
-      MaybeGetBindingClauseForKeyRange(mOptionalKeyRange, sortColumn);
-
-  nsAutoCString directionClause = NS_LITERAL_CSTRING(" ORDER BY ") + sortColumn;
+  const auto keyRangeClause = MaybeGetBindingClauseForKeyRange(
+      mOptionalKeyRange, kColumnNameAliasSortKey);
+
+  nsAutoCString directionClause =
+      NS_LITERAL_CSTRING(" ORDER BY ") + kColumnNameAliasSortKey;
 
   switch (mCursor->mDirection) {
     case IDBCursor::NEXT:
     case IDBCursor::NEXT_UNIQUE:
       directionClause.AppendLiteral(" ASC, index_table.object_data_key ASC");
       break;
 
     case IDBCursor::PREV:
@@ -26316,17 +26373,17 @@ nsresult Cursor::OpenOp::DoIndexDatabase
   }
 
   rv = PopulateResponseFromStatement(stmt, true);
   if (NS_WARN_IF(NS_FAILED(rv))) {
     return rv;
   }
 
   // Now we need to make the query to get the next match.
-  PrepareIndexKeyConditionClause(sortColumn, directionClause,
+  PrepareIndexKeyConditionClause(kColumnNameAliasSortKey, directionClause,
                                  NS_LITERAL_CSTRING("index_table."),
                                  std::move(queryStart));
 
   // The degree to which extra responses on OpenOp can actually be used depends
   // on the parameters of subsequent ContinueOp operations, see also comment in
   // ContinueOp::DoDatabaseWork.
 
   return PopulateExtraResponses(stmt, mCursor->mMaxExtraCount,
@@ -26345,39 +26402,38 @@ nsresult Cursor::OpenOp::DoIndexKeyDatab
   AUTO_PROFILER_LABEL("Cursor::OpenOp::DoIndexKeyDatabaseWork", DOM);
 
   const bool usingKeyRange = mOptionalKeyRange.isSome();
 
   const auto& table = mCursor->mUniqueIndex
                           ? NS_LITERAL_CSTRING("unique_index_data")
                           : NS_LITERAL_CSTRING("index_data");
 
-  NS_NAMED_LITERAL_CSTRING(sortColumn, "sort_column");
-
   // The result of MakeColumnPairSelectionList is stored in a local variable,
   // since inlining it into the next statement causes a crash on some Mac OS X
   // builds (see https://bugzilla.mozilla.org/show_bug.cgi?id=1168606#c110).
   const auto columnPairSelectionList = MakeColumnPairSelectionList(
       NS_LITERAL_CSTRING("value"), NS_LITERAL_CSTRING("value_locale"),
-      sortColumn, mCursor->IsLocaleAware());
+      kColumnNameAliasSortKey, mCursor->IsLocaleAware());
   const nsCString sortColumnAlias = NS_LITERAL_CSTRING("SELECT ") +
                                     columnPairSelectionList +
                                     NS_LITERAL_CSTRING(", ");
 
   nsAutoCString queryStart = sortColumnAlias +
                              NS_LITERAL_CSTRING(
                                  "object_data_key "
                                  " FROM ") +
                              table + NS_LITERAL_CSTRING(" WHERE index_id = :") +
                              kStmtParamNameId;
 
-  const auto keyRangeClause =
-      MaybeGetBindingClauseForKeyRange(mOptionalKeyRange, sortColumn);
-
-  nsAutoCString directionClause = NS_LITERAL_CSTRING(" ORDER BY ") + sortColumn;
+  const auto keyRangeClause = MaybeGetBindingClauseForKeyRange(
+      mOptionalKeyRange, kColumnNameAliasSortKey);
+
+  nsAutoCString directionClause =
+      NS_LITERAL_CSTRING(" ORDER BY ") + kColumnNameAliasSortKey;
 
   switch (mCursor->mDirection) {
     case IDBCursor::NEXT:
     case IDBCursor::NEXT_UNIQUE:
       directionClause.AppendLiteral(" ASC, object_data_key ASC");
       break;
 
     case IDBCursor::PREV:
@@ -26432,17 +26488,17 @@ nsresult Cursor::OpenOp::DoIndexKeyDatab
   }
 
   rv = PopulateResponseFromStatement(stmt, true);
   if (NS_WARN_IF(NS_FAILED(rv))) {
     return rv;
   }
 
   // Now we need to make the query to get the next match.
-  PrepareIndexKeyConditionClause(sortColumn, directionClause,
+  PrepareIndexKeyConditionClause(kColumnNameAliasSortKey, directionClause,
                                  NS_LITERAL_CSTRING(""), std::move(queryStart));
 
   return NS_OK;
 }
 
 nsresult Cursor::OpenOp::DoDatabaseWork(DatabaseConnection* aConnection) {
   MOZ_ASSERT(aConnection);
   aConnection->AssertIsOnConnectionThread();