Use functions to map column indexes to IDs.

Signed-off-by: ienkovich <[email protected]>

Author: ienkovich

omniscidb/ArrowStorage/ArrowStorage.cpp

@@ -67,6 +67,20 @@ size_t computeTotalStringsLength(std::shared_ptr<arrow::ChunkedArray> arr,
   return total_bytes;
 }
 
+/**
+ * Get column ID by its 0-based index (position) in the table.
+ */
+int columnId(size_t col_idx) {
+  return static_cast<int>(col_idx + 1000);
+}
+
+/**
+ * Translate column ID to 0-based index (position) in the table.
+ */
+size_t columnIndex(int col_id) {
+  return static_cast<size_t>(col_id - 1000);
+}
+
 }  // anonymous namespace
 
 void ArrowStorage::fetchBuffer(const ChunkKey& key,
@@ -79,7 +93,7 @@ void ArrowStorage::fetchBuffer(const ChunkKey& key,
   mapd_shared_lock<mapd_shared_mutex> table_lock(table.mutex);
   data_lock.unlock();
 
-  size_t col_idx = static_cast<size_t>(key[CHUNK_KEY_COLUMN_IDX] - 1);
+  size_t col_idx = columnIndex(key[CHUNK_KEY_COLUMN_IDX]);
   size_t frag_idx = static_cast<size_t>(key[CHUNK_KEY_FRAGMENT_IDX] - 1);
   CHECK_LT(frag_idx, table.fragments.size());
   CHECK_LT(col_idx, table.col_data.size());
@@ -134,7 +148,7 @@ std::unique_ptr<AbstractDataToken> ArrowStorage::getZeroCopyBufferMemory(
           ->type;
 
   if (!col_type->isVarLen()) {
-    size_t col_idx = static_cast<size_t>(key[CHUNK_KEY_COLUMN_IDX] - 1);
+    size_t col_idx = columnIndex(key[CHUNK_KEY_COLUMN_IDX]);
     size_t frag_idx = static_cast<size_t>(key[CHUNK_KEY_FRAGMENT_IDX] - 1);
     CHECK_EQ(key.size(), (size_t)4);
     size_t elem_size = col_type->size();
@@ -293,7 +307,7 @@ TableFragmentsInfo ArrowStorage::getTableMetadata(int db_id, int table_id) const
     frag_info.deviceIds.push_back(0);  // Data_Namespace::CPU_LEVEL
     frag_info.deviceIds.push_back(0);  // Data_Namespace::GPU_LEVEL
     for (size_t col_idx = 0; col_idx < frag.metadata.size(); ++col_idx) {
-      frag_info.setChunkMetadata(static_cast<int>(col_idx + 1), frag.metadata[col_idx]);
+      frag_info.setChunkMetadata(columnId(col_idx), frag.metadata[col_idx]);
     }
   }
   return res;
@@ -331,14 +345,14 @@ TableInfoPtr ArrowStorage::createTable(const std::string& table_name,
   TableInfoPtr res;
   int table_id;
   mapd_unique_lock<mapd_shared_mutex> data_lock(data_mutex_);
+  size_t next_col_idx = 0;
   {
     mapd_unique_lock<mapd_shared_mutex> dict_lock(dict_mutex_);
     mapd_unique_lock<mapd_shared_mutex> schema_lock(schema_mutex_);
     table_id = next_table_id_++;
     checkNewTableParams(table_name, columns, options);
     res = addTableInfo(
         db_id_, table_id, table_name, false, Data_Namespace::MemoryLevel::CPU_LEVEL, 0);
-    int next_col_id = 1;
     std::unordered_map<int, int> dict_ids;
     for (auto& col : columns) {
       auto type = col.type;
@@ -382,10 +396,10 @@ TableInfoPtr ArrowStorage::createTable(const std::string& table_name,
           type = elem_type;
         }
       }
-      auto col_info =
-          addColumnInfo(db_id_, table_id, next_col_id++, col.name, type, false);
+      auto col_info = addColumnInfo(
+          db_id_, table_id, columnId(next_col_idx++), col.name, type, false);
     }
-    addRowidColumn(db_id_, table_id);
+    addRowidColumn(db_id_, table_id, columnId(next_col_idx++));
   }
 
   std::vector<std::shared_ptr<arrow::Field>> fields;
@@ -479,7 +493,7 @@ void ArrowStorage::appendArrowTable(std::shared_ptr<arrow::Table> at, int table_
   threading::parallel_for(
       threading::blocked_range(0, (int)at->columns().size()), [&](auto range) {
         for (auto col_idx = range.begin(); col_idx != range.end(); col_idx++) {
-          auto col_info = getColumnInfo(db_id_, table_id, col_idx + 1);
+          auto col_info = getColumnInfo(db_id_, table_id, columnId(col_idx));
           auto col_type = col_info->type;
           auto col_arr = at->column(col_idx);
 
@@ -605,7 +619,7 @@ void ArrowStorage::appendArrowTable(std::shared_ptr<arrow::Table> at, int table_
       auto& first_frag = fragments.front();
       last_frag.row_count += first_frag.row_count;
       for (size_t col_idx = 0; col_idx < last_frag.metadata.size(); ++col_idx) {
-        auto col_type = getColumnInfo(db_id_, table_id, col_idx + 1)->type;
+        auto col_type = getColumnInfo(db_id_, table_id, columnId(col_idx))->type;
         size_t num_elems = last_frag.metadata[col_idx]->numElements() +
                            first_frag.metadata[col_idx]->numElements();
         size_t num_bytes = last_frag.metadata[col_idx]->numBytes() +

omniscidb/QueryBuilder/QueryBuilder.cpp

@@ -2269,9 +2269,10 @@ BuilderNode BuilderNode::sort(const std::vector<BuilderSortField>& fields,
   auto base = node_;
   if (node_->is<Scan>()) {
     // Filter out rowid column if it's not used in the sort.
+    auto scan = node_->as<Scan>();
     bool uses_rowid =
         std::any_of(collation.begin(), collation.end(), [&](const SortField& field) {
-          return field.getField() == node_->size() - 1;
+          return scan->isVirtualCol(field.getField());
         });
     int cols_to_proj = uses_rowid ? node_->size() : node_->size() - 1;
     std::vector<int> col_indices(cols_to_proj);

omniscidb/QueryEngine/WorkUnitBuilder.cpp

@@ -349,7 +349,7 @@ void WorkUnitBuilder::process(const ir::Node* node) {
         col_var = ir::makeExpr<ir::ColumnVar>(node->getOutputMetainfo()[i].type(),
                                               token->dbId(),
                                               token->tableId(),
-                                              i + 1,
+                                              token->columnId(i),
                                               rte_idx,
                                               false);
       }

omniscidb/ResultSetRegistry/ResultSetMetadata.cpp

@@ -6,6 +6,7 @@
  */
 
 #include "ResultSetMetadata.h"
+#include "ResultSetTableToken.h"
 
 #include "Shared/thread_count.h"
 
@@ -80,7 +81,8 @@ ChunkMetadataMap synthesizeMetadata(const ResultSet* rows) {
     for (size_t i = 0; i < rows->colCount(); ++i) {
       decoders.emplace_back(Encoder::Create(nullptr, rows->colType(i)));
       const auto it_ok =
-          metadata_map.emplace(i + 1, decoders.back()->getMetadata(rows->colType(i)));
+          metadata_map.emplace(ResultSetTableToken::columnId(i),
+                               decoders.back()->getMetadata(rows->colType(i)));
       CHECK(it_ok.second);
     }
     return metadata_map;
@@ -212,7 +214,7 @@ ChunkMetadataMap synthesizeMetadata(const ResultSet* rows) {
         num_bytes,
         rows->rowCount(),
         dummy_encoders[0][i]->getMetadata(elem_type)->chunkStats());
-    const auto it_ok = metadata_map.emplace(i + 1, meta);
+    const auto it_ok = metadata_map.emplace(ResultSetTableToken::columnId(i), meta);
     CHECK(it_ok.second);
   }
   return metadata_map;

omniscidb/ResultSetRegistry/ResultSetRegistry.cpp

@@ -51,6 +51,14 @@ TableFragmentsInfo getEmptyTableMetadata(int table_id) {
   return res;
 }
 
+int columnId(size_t col_idx) {
+  return ResultSetTableToken::columnId(col_idx);
+}
+
+size_t columnIndex(int col_id) {
+  return ResultSetTableToken::columnIndex(col_id);
+}
+
 }  // namespace
 
 ResultSetRegistry::ResultSetRegistry(ConfigPtr config)
@@ -111,14 +119,14 @@ ResultSetTableTokenPtr ResultSetRegistry::put(ResultSetTable table) {
   for (size_t col_idx = 0; col_idx < first_rs->colCount(); ++col_idx) {
     addColumnInfo(db_id_,
                   table_id,
-                  static_cast<int>(col_idx + 1),
+                  columnId(col_idx),
                   first_rs->colName(col_idx),
                   first_rs->colType(col_idx),
                   false);
     has_varlen = has_varlen || first_rs->colType(col_idx)->isVarLen();
     has_array = has_array || first_rs->colType(col_idx)->isArray();
   }
-  addRowidColumn(db_id_, table_id);
+  addRowidColumn(db_id_, table_id, columnId(first_rs->colCount()));
 
   // TODO: lazily compute row count and try to avoid global write
   // locks for that
@@ -190,8 +198,8 @@ ChunkStats ResultSetRegistry::getChunkStats(int table_id,
       frag.meta = synthesizeMetadata(frag.rs.get());
     }
   }
-  CHECK(frag.meta.count(col_idx + 1));
-  return frag.meta.at(col_idx + 1)->chunkStats();
+  CHECK(frag.meta.count(columnId(col_idx)));
+  return frag.meta.at(columnId(col_idx))->chunkStats();
 }
 
 void ResultSetRegistry::fetchBuffer(const ChunkKey& key,
@@ -204,7 +212,7 @@ void ResultSetRegistry::fetchBuffer(const ChunkKey& key,
   mapd_shared_lock<mapd_shared_mutex> table_lock(table.mutex);
   data_lock.unlock();
 
-  size_t col_idx = static_cast<size_t>(key[CHUNK_KEY_COLUMN_IDX] - 1);
+  size_t col_idx = columnIndex(key[CHUNK_KEY_COLUMN_IDX]);
   size_t frag_idx = static_cast<size_t>(key[CHUNK_KEY_FRAGMENT_IDX] - 1);
   CHECK_LT(frag_idx, table.fragments.size());
   auto& rs = table.fragments[frag_idx].rs;
@@ -223,7 +231,7 @@ ResultSetRegistry::getZeroCopyBufferMemory(const ChunkKey& key, size_t num_bytes
   mapd_shared_lock<mapd_shared_mutex> table_lock(table.mutex);
   data_lock.unlock();
 
-  size_t col_idx = static_cast<size_t>(key[CHUNK_KEY_COLUMN_IDX] - 1);
+  size_t col_idx = columnIndex(key[CHUNK_KEY_COLUMN_IDX]);
   size_t frag_idx = static_cast<size_t>(key[CHUNK_KEY_FRAGMENT_IDX] - 1);
   CHECK_LT(frag_idx, table.fragments.size());
   auto& frag = table.fragments[frag_idx];
@@ -317,7 +325,7 @@ TableFragmentsInfo ResultSetRegistry::getTableMetadata(int db_id, int table_id)
               [this, table_id, frag_idx, col_idx](ChunkStats& stats) {
                 stats = this->getChunkStats(table_id, frag_idx, col_idx);
               });
-          frag_info.setChunkMetadata(static_cast<int>(col_idx + 1), meta);
+          frag_info.setChunkMetadata(columnId(col_idx), meta);
         }
       }
     } else {

omniscidb/ResultSetRegistry/ResultSetTableToken.h

@@ -37,6 +37,11 @@ class ResultSetTableToken {
   int dbId() const { return tinfo_->db_id; }
   int tableId() const { return tinfo_->table_id; }
 
+  // Column ID <-> Index mapping. Use a significant offset to early catch
+  // cases of indexes used as IDs and vice versa.
+  static int columnId(size_t col_idx) { return static_cast<int>(col_idx + 2000); }
+  static size_t columnIndex(int col_id) { return static_cast<size_t>(col_id - 2000); }
+
   size_t rowCount() const { return row_count_; }
 
   size_t resultSetCount() const { return tinfo_->fragments; }

omniscidb/SchemaMgr/SimpleSchemaProvider.h

@@ -157,9 +157,8 @@ class SimpleSchemaProvider : public SchemaProvider {
     return addColumnInfo(std::make_shared<ColumnInfo>(args...));
   }
 
-  ColumnInfoPtr addRowidColumn(int db_id, int table_id) {
+  ColumnInfoPtr addRowidColumn(int db_id, int table_id, int col_id) {
     CHECK_EQ(column_index_by_name_.count({db_id, table_id}), (size_t)1);
-    int col_id = static_cast<int>(column_index_by_name_[{db_id, table_id}].size() + 1);
     return addColumnInfo(db_id, table_id, col_id, "rowid", ctx_.int64(), true);
   }