use const ref and unique_ptr where possible

Author: wgtmac

src/iceberg/catalog.h

@@ -22,6 +22,7 @@
 #include <map>
 #include <memory>
 #include <string>
+#include <string_view>
 #include <vector>
 
 #include "iceberg/expected.h"
@@ -36,14 +37,14 @@ class ICEBERG_EXPORT Catalog {
   virtual ~Catalog() = default;
 
   /// \brief Return the name for this catalog
-  virtual std::string name() const = 0;
+  virtual std::string_view name() const = 0;
 
   /// \brief Return all the identifiers under this namespace
   ///
   /// \param ns a namespace
   /// \return a list of identifiers for tables or Error::kNoSuchNamespace
   /// if the namespace does not exist
-  virtual expected<std::vector<TableIdentifier>, Error> ListTables(
+  virtual expected<std::vector<TableIdentifier>, ErrorKind> ListTables(
       const Namespace& ns) const = 0;
 
   /// \brief Create a table
@@ -54,7 +55,7 @@ class ICEBERG_EXPORT Catalog {
   /// \param location a location for the table; leave empty if unspecified
   /// \param properties a string map of table properties
   /// \return a Table instance or Error::kAlreadyExists if the table already exists
-  virtual expected<std::unique_ptr<Table>, Error> CreateTable(
+  virtual expected<std::unique_ptr<Table>, ErrorKind> CreateTable(
       const TableIdentifier& identifier, const Schema& schema, const PartitionSpec& spec,
       const std::string& location,
       const std::map<std::string, std::string>& properties) = 0;
@@ -68,7 +69,7 @@ class ICEBERG_EXPORT Catalog {
   /// \param properties a string map of table properties
   /// \return a Transaction to create the table or Error::kAlreadyExists if the table
   /// already exists
-  virtual expected<std::shared_ptr<Transaction>, Error> NewCreateTableTransaction(
+  virtual expected<std::shared_ptr<Transaction>, ErrorKind> NewCreateTableTransaction(
       const TableIdentifier& identifier, const Schema& schema, const PartitionSpec& spec,
       const std::string& location,
       const std::map<std::string, std::string>& properties) = 0;
@@ -94,15 +95,15 @@ class ICEBERG_EXPORT Catalog {
   /// \param identifier a table identifier
   /// \return instance of Table implementation referred to by identifier or
   /// Error::kNoSuchTable if the table does not exist
-  virtual expected<std::shared_ptr<Table>, Error> LoadTable(
+  virtual expected<std::shared_ptr<Table>, ErrorKind> LoadTable(
       const TableIdentifier& identifier) const = 0;
 
   /// \brief Register a table with the catalog if it does not exist
   ///
   /// \param identifier a table identifier
   /// \param metadata_file_location the location of a metadata file
   /// \return a Table instance or Error::kAlreadyExists if the table already exists
-  virtual expected<std::shared_ptr<Table>, Error> RegisterTable(
+  virtual expected<std::shared_ptr<Table>, ErrorKind> RegisterTable(
       const TableIdentifier& identifier, const std::string& metadata_file_location) = 0;
 
   /// \brief Initialize a catalog given a custom name and a map of catalog properties

src/iceberg/table.h

@@ -20,6 +20,7 @@
 #pragma once
 
 #include <map>
+#include <memory>
 #include <string>
 #include <vector>
 
@@ -35,10 +36,10 @@ class ICEBERG_EXPORT Table {
   virtual ~Table() = default;
 
   /// \brief Return the full name for this table
-  virtual std::string name() const = 0;
+  virtual const std::string& name() const = 0;
 
   /// \brief Returns the UUID of the table
-  virtual std::string uuid() const = 0;
+  virtual const std::string& uuid() const = 0;
 
   /// \brief Refresh the current table metadata
   virtual void Refresh() = 0;
@@ -47,25 +48,25 @@ class ICEBERG_EXPORT Table {
   virtual const std::shared_ptr<Schema>& schema() const = 0;
 
   /// \brief Return a map of schema for this table
-  virtual std::map<int32_t, std::shared_ptr<Schema>> schemas() const = 0;
+  virtual const std::map<int32_t, std::shared_ptr<Schema>>& schemas() const = 0;
 
   /// \brief Return the partition spec for this table
   virtual const std::shared_ptr<PartitionSpec>& spec() const = 0;
 
   /// \brief Return a map of partition specs for this table
-  virtual std::map<int32_t, std::shared_ptr<PartitionSpec>> specs() const = 0;
+  virtual const std::map<int32_t, std::shared_ptr<PartitionSpec>>& specs() const = 0;
 
   /// \brief Return the sort order for this table
   virtual const std::shared_ptr<SortOrder>& sort_order() const = 0;
 
   /// \brief Return a map of sort order IDs to sort orders for this table
-  virtual std::map<int32_t, std::shared_ptr<SortOrder>> sort_orders() const = 0;
+  virtual const std::map<int32_t, std::shared_ptr<SortOrder>>& sort_orders() const = 0;
 
   /// \brief Return a map of string properties for this table
-  virtual std::map<std::string, std::string> properties() const = 0;
+  virtual const std::map<std::string, std::string>& properties() const = 0;
 
   /// \brief Return the table's base location
-  virtual std::string location() const = 0;
+  virtual const std::string& location() const = 0;
 
   /// \brief Return the table's current snapshot
   virtual const std::shared_ptr<Snapshot>& current_snapshot() const = 0;
@@ -75,21 +76,21 @@ class ICEBERG_EXPORT Table {
   ///
   /// \param snapshot_id the ID of the snapshot to get
   /// \return the Snapshot with the given id
-  virtual expected<std::shared_ptr<Snapshot>, Error> snapshot(
+  virtual expected<std::shared_ptr<Snapshot>, ErrorKind> snapshot(
       int64_t snapshot_id) const = 0;
 
   /// \brief Get the snapshots of this table
-  virtual std::vector<std::shared_ptr<Snapshot>> snapshots() const = 0;
+  virtual const std::vector<std::shared_ptr<Snapshot>>& snapshots() const = 0;
 
   /// \brief Get the snapshot history of this table
   ///
   /// \return a vector of history entries
-  virtual std::vector<std::shared_ptr<HistoryEntry>> history() const = 0;
+  virtual const std::vector<std::shared_ptr<HistoryEntry>>& history() const = 0;
 
   /// \brief Create a new table scan for this table
   ///
   /// Once a table scan is created, it can be refined to project columns and filter data.
-  virtual std::shared_ptr<TableScan> NewScan() const = 0;
+  virtual std::unique_ptr<TableScan> NewScan() const = 0;
 
   /// \brief Create a new append API to add files to this table and commit
   virtual std::shared_ptr<AppendFiles> NewAppend() = 0;

src/iceberg/table_identifier.h

@@ -36,7 +36,7 @@ struct ICEBERG_EXPORT Namespace {
 
 /// \brief Identifies a table in iceberg catalog.
 struct ICEBERG_EXPORT TableIdentifier {
-  Namespace name_space;
+  Namespace ns;
   std::string name;
 };
 

src/iceberg/table_operations.h

@@ -19,6 +19,7 @@
 
 #pragma once
 
+#include <memory>
 #include <string>
 
 #include "iceberg/expected.h"
@@ -33,10 +34,10 @@ class ICEBERG_EXPORT TableOperations {
   virtual ~TableOperations() = default;
 
   /// \brief Return the currently loaded table metadata, without checking for updates.
-  virtual std::shared_ptr<TableMetadata> Current() = 0;
+  virtual const std::shared_ptr<TableMetadata>& Current() = 0;
 
   /// \brief Return the current table metadata after checking for updates.
-  virtual std::shared_ptr<TableMetadata> Refresh() = 0;
+  virtual const std::shared_ptr<TableMetadata>& Refresh() = 0;
 
   /// \brief Replace the base table metadata with a new version.
   ///
@@ -57,8 +58,8 @@ class ICEBERG_EXPORT TableOperations {
   ///
   /// \param base table metadata on which changes were based
   /// \param metadata new table metadata with updates
-  virtual expected<void, Error> Commit(const TableMetadata& base,
-                                       const TableMetadata& metadata) = 0;
+  virtual expected<void, ErrorKind> Commit(const TableMetadata& base,
+                                           const TableMetadata& metadata) = 0;
 
   /// \brief Returns an FileIO to read and write table data and metadata files.
   // virtual std::shared_ptr<FileIO> io() const = 0;
@@ -70,7 +71,7 @@ class ICEBERG_EXPORT TableOperations {
   /// \brief Returns a LocationProvider that supplies locations for new new data files.
   ///
   /// \return a location provider configured for the current table state
-  virtual std::shared_ptr<LocationProvider> location_provider() const = 0;
+  virtual std::unique_ptr<LocationProvider> location_provider() const = 0;
 
   /// \brief Return a temporary TableOperations instance that uses configuration from
   /// uncommitted metadata.

src/iceberg/type_fwd.h

@@ -83,7 +83,7 @@ class UuidType;
 
 /// \brief Error types for iceberg.
 /// TODO: add more and sort them based on some rules.
-enum class Error {
+enum class ErrorKind {
   kNoSuchNamespace,
   kAlreadyExists,
   kNoSuchTable,