Change order of parameters in adaptive table factory

Summary:
This is minor, but if we put the writing talbe factory as the third parameter, when we add a new table format, we'll have a situation:
1) block based factory
2) plain table factory
3) output factory
4) new format factory

I think it makes more sense to have output as the first parameter.

Also, fixed a NewAdaptiveTableFactory() call in unit test

Test Plan: unit test

Reviewers: sdong

Reviewed By: sdong

Subscribers: leveldb

Differential Revision: https://reviews.facebook.net/D19119

diff --git a/db/plain_table_db_test.cc b/db/plain_table_db_test.cc
index 2e1e8be6e922be1057251caf6471f11f736b32f9..0df57a41b38009303c488dba643a432c27b7da61 100644 (file)
--- a/db/plain_table_db_test.cc
+++ b/db/plain_table_db_test.cc
@@ -862,8 +862,10 @@ TEST(PlainTableDBTest, AdaptiveTable) {
 
   options.create_if_missing = false;
   std::shared_ptr<TableFactory> dummy_factory;
-  options.table_factory.reset(
-      NewAdaptiveTableFactory(dummy_factory, dummy_factory, false));
+  std::shared_ptr<TableFactory> block_based_factory(
+      NewBlockBasedTableFactory());
+  options.table_factory.reset(NewAdaptiveTableFactory(
+      block_based_factory, dummy_factory, dummy_factory));
   Reopen(&options);
   ASSERT_EQ("v3", Get("1000000000000foo"));
   ASSERT_EQ("v2", Get("0000000000000bar"));

diff --git a/include/rocksdb/table.h b/include/rocksdb/table.h
index fb169fdf29e280aa61b6b4dc5e64ee825da02e2b..01bfae4316ef8bc8d7eea4d3c02b4c2d6cc02d8b 100644 (file)
--- a/include/rocksdb/table.h
+++ b/include/rocksdb/table.h
@@ -207,14 +207,14 @@ class TableFactory {
 // Create a special table factory that can open both of block based table format
 // and plain table, based on setting inside the SST files. It should be used to
 // convert a DB from one table format to another.
+// @table_factory_to_write: the table factory used when writing to new files.
 // @block_based_table_factory:  block based table factory to use. If NULL, use
 //                              a default one.
 // @plain_table_factory: plain table factory to use. If NULL, use a default one.
-// @table_factory_to_write: the table factory used when writing to new files.
 extern TableFactory* NewAdaptiveTableFactory(
+    std::shared_ptr<TableFactory> table_factory_to_write = nullptr,
     std::shared_ptr<TableFactory> block_based_table_factory = nullptr,
-    std::shared_ptr<TableFactory> plain_table_factory = nullptr,
-    std::shared_ptr<TableFactory> table_factory_to_write = nullptr);
+    std::shared_ptr<TableFactory> plain_table_factory = nullptr);
 
 #endif  // ROCKSDB_LITE
 

diff --git a/table/adaptive_table_factory.cc b/table/adaptive_table_factory.cc
index 9d05be7c73e1b2170ac71209b199e0c6fe37cc8d..2a7d1725182cbdbf59c830f0c933fe91d7f24625 100644 (file)
--- a/table/adaptive_table_factory.cc
+++ b/table/adaptive_table_factory.cc
@@ -10,21 +10,21 @@
 namespace rocksdb {
 
 AdaptiveTableFactory::AdaptiveTableFactory(
+    std::shared_ptr<TableFactory> table_factory_to_write,
     std::shared_ptr<TableFactory> block_based_table_factory,
-    std::shared_ptr<TableFactory> plain_table_factory,
-    std::shared_ptr<TableFactory> table_factory_to_write)
-    : block_based_table_factory_(block_based_table_factory),
-      plain_table_factory_(plain_table_factory),
-      table_factory_to_write_(table_factory_to_write) {
+    std::shared_ptr<TableFactory> plain_table_factory)
+    : table_factory_to_write_(table_factory_to_write),
+      block_based_table_factory_(block_based_table_factory),
+      plain_table_factory_(plain_table_factory) {
+  if (!table_factory_to_write_) {
+    table_factory_to_write_ = block_based_table_factory_;
+  }
   if (!plain_table_factory_) {
     plain_table_factory_.reset(NewPlainTableFactory());
   }
   if (!block_based_table_factory_) {
     block_based_table_factory_.reset(NewBlockBasedTableFactory());
   }
-  if (!table_factory_to_write_) {
-    table_factory_to_write_ = block_based_table_factory_;
-  }
 }
 
 extern const uint64_t kPlainTableMagicNumber;
@@ -62,11 +62,11 @@ TableBuilder* AdaptiveTableFactory::NewTableBuilder(
 }
 
 extern TableFactory* NewAdaptiveTableFactory(
+    std::shared_ptr<TableFactory> table_factory_to_write,
     std::shared_ptr<TableFactory> block_based_table_factory,
-    std::shared_ptr<TableFactory> plain_table_factory,
-    std::shared_ptr<TableFactory> table_factory_to_write) {
+    std::shared_ptr<TableFactory> plain_table_factory) {
   return new AdaptiveTableFactory(
-      block_based_table_factory, plain_table_factory, table_factory_to_write);
+      table_factory_to_write, block_based_table_factory, plain_table_factory);
 }
 
 }  // namespace rocksdb

diff --git a/table/adaptive_table_factory.h b/table/adaptive_table_factory.h
index 4bf2fb2c3cd133d748c54b5eb7c9b06e24333f05..e5632f786bfc918544d14dc1f145f5d99094cfc4 100644 (file)
--- a/table/adaptive_table_factory.h
+++ b/table/adaptive_table_factory.h
@@ -26,9 +26,9 @@ class AdaptiveTableFactory : public TableFactory {
   ~AdaptiveTableFactory() {}
 
   explicit AdaptiveTableFactory(
+      std::shared_ptr<TableFactory> table_factory_to_write,
       std::shared_ptr<TableFactory> block_based_table_factory,
-      std::shared_ptr<TableFactory> plain_table_factory,
-      std::shared_ptr<TableFactory> table_factory_to_write);
+      std::shared_ptr<TableFactory> plain_table_factory);
   const char* Name() const override { return "AdaptiveTableFactory"; }
   Status NewTableReader(const Options& options, const EnvOptions& soptions,
                         const InternalKeyComparator& internal_comparator,
@@ -41,9 +41,9 @@ class AdaptiveTableFactory : public TableFactory {
       override;
 
  private:
+  std::shared_ptr<TableFactory> table_factory_to_write_;
   std::shared_ptr<TableFactory> block_based_table_factory_;
   std::shared_ptr<TableFactory> plain_table_factory_;
-  std::shared_ptr<TableFactory> table_factory_to_write_;
 };
 
 }  // namespace rocksdb