fix: Avoid WriteKind duplication between connector and logical plan

Acutally I think it will be better to logical plan depends on connector, but it is as it is

Author: MBkkt

axiom/common/CMakeLists.txt

@@ -12,6 +12,6 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-add_library(axiom_common Session.cpp)
+add_library(axiom_common Session.cpp WriteKind.cpp)
 
 target_link_libraries(axiom_common axiom_connectors)

axiom/common/WriteKind.cpp

@@ -0,0 +1,20 @@
+#include "axiom/common/WriteKind.h"
+
+namespace facebook::axiom {
+namespace {
+
+const auto& writeKindNames() {
+  static const folly::F14FastMap<WriteKind, std::string_view> kNames = {
+      {WriteKind::kCreate, "CREATE"},
+      {WriteKind::kInsert, "INSERT"},
+      {WriteKind::kUpdate, "UPDATE"},
+      {WriteKind::kDelete, "DELETE"},
+  };
+  return kNames;
+}
+
+} // namespace
+
+AXIOM_DEFINE_ENUM_NAME(WriteKind, writeKindNames);
+
+} // namespace facebook::axiom

axiom/common/WriteKind.h

@@ -0,0 +1,51 @@
+/*
+ * Copyright (c) Meta Platforms, Inc. and its affiliates.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+#pragma once
+
+#include "axiom/common/Enums.h"
+
+namespace facebook::axiom {
+
+/// Specifies what type of write is intended when initiating or concluding a
+/// write operation.
+enum class WriteKind {
+  /// A write operation to a new table which does not yet exist in the
+  /// connector. Covers both creation of an empty table and create as select
+  /// operations.
+  kCreate = 1,
+
+  /// Rows are added and all columns must be specified for the TableWriter.
+  /// Covers insert, Hive partition replacement or any other operation which
+  /// adds whole rows.
+  kInsert = 2,
+
+  /// Individual rows are deleted. Only row ids as per
+  /// ConnectorMetadata::rowIdHandles() are passed to the TableWriter.
+  kDelete = 3,
+
+  /// Column values in individual rows are changed. The TableWriter
+  /// gets first the row ids as per ConnectorMetadata::rowIdHandles()
+  /// and then new values for the columns being changed. The new values
+  /// may overlap with row ids if the row id is a set of primary key
+  /// columns.
+  kUpdate = 4,
+};
+
+AXIOM_DECLARE_ENUM_NAME(WriteKind);
+
+} // namespace facebook::axiom
+
+AXIOM_ENUM_FORMATTER(facebook::axiom::WriteKind);

axiom/connectors/CMakeLists.txt

@@ -26,4 +26,4 @@ endif()
 
 add_library(axiom_connectors ConnectorMetadata.cpp SchemaResolver.cpp SchemaUtils.cpp)
 
-target_link_libraries(axiom_connectors velox_common_base velox_memory velox_connector)
+target_link_libraries(axiom_connectors axiom_common velox_common_base velox_memory velox_connector)

axiom/connectors/ConnectorMetadata.cpp

@@ -17,8 +17,8 @@
 #include "axiom/connectors/ConnectorMetadata.h"
 
 namespace facebook::axiom::connector {
-
 namespace {
+
 const auto& tableKindNames() {
   static const folly::F14FastMap<TableKind, std::string_view> kNames = {
       {TableKind::kTable, "TABLE"},
@@ -27,22 +27,10 @@ const auto& tableKindNames() {
   return kNames;
 }
 
-const auto& writeKindNames() {
-  static const folly::F14FastMap<WriteKind, std::string_view> kNames = {
-      {WriteKind::kCreate, "CREATE"},
-      {WriteKind::kInsert, "INSERT"},
-      {WriteKind::kUpdate, "UPDATE"},
-      {WriteKind::kDelete, "DELETE"},
-  };
-  return kNames;
-}
-
 } // namespace
 
 AXIOM_DEFINE_ENUM_NAME(TableKind, tableKindNames);
 
-AXIOM_DEFINE_ENUM_NAME(WriteKind, writeKindNames);
-
 namespace {
 velox::RowTypePtr makeRowType(const std::vector<const Column*>& columns) {
   folly::F14FastSet<std::string> uniqueNames;

axiom/connectors/ConnectorMetadata.h

@@ -16,6 +16,7 @@
 #pragma once
 
 #include "axiom/common/Enums.h"
+#include "axiom/common/WriteKind.h"
 #include "axiom/connectors/ConnectorSession.h"
 #include "axiom/connectors/ConnectorSplitManager.h"
 #include "velox/common/memory/HashStringAllocator.h"
@@ -480,33 +481,6 @@ class ConnectorWriteHandle {
 
 using ConnectorWriteHandlePtr = std::shared_ptr<ConnectorWriteHandle>;
 
-/// Specifies what type of write is intended when initiating or concluding a
-/// write operation.
-enum class WriteKind {
-  /// A write operation to a new table which does not yet exist in the
-  /// connector. Covers both creation of an empty table and create as select
-  /// operations.
-  kCreate = 1,
-
-  /// Rows are added and all columns must be specified for the TableWriter.
-  /// Covers insert, Hive partition replacement or any other operation which
-  /// adds whole rows.
-  kInsert = 2,
-
-  /// Individual rows are deleted. Only row ids as per
-  /// ConnectorMetadata::rowIdHandles() are passed to the TableWriter.
-  kDelete = 3,
-
-  /// Column values in individual rows are changed. The TableWriter
-  /// gets first the row ids as per ConnectorMetadata::rowIdHandles()
-  /// and then new values for the columns being changed. The new values
-  /// may overlap with row ids if the row id is a set of primary key
-  /// columns.
-  kUpdate = 4,
-};
-
-AXIOM_DECLARE_ENUM_NAME(WriteKind);
-
 using RowsFuture = folly::SemiFuture<int64_t>;
 
 class ConnectorMetadata {
@@ -668,5 +642,3 @@ class ConnectorMetadata {
 } // namespace facebook::axiom::connector
 
 AXIOM_ENUM_FORMATTER(facebook::axiom::connector::TableKind);
-
-AXIOM_ENUM_FORMATTER(facebook::axiom::connector::WriteKind);

axiom/logical_plan/LogicalPlanNode.cpp

@@ -390,21 +390,4 @@ void TableWriteNode::accept(
   visitor.visit(*this, context);
 }
 
-namespace {
-
-folly::F14FastMap<WriteKind, std::string_view> writeKindNames() {
-  static const folly::F14FastMap<WriteKind, std::string_view> kNames = {
-      {WriteKind::kCreate, "CREATE"},
-      {WriteKind::kInsert, "INSERT"},
-      {WriteKind::kUpdate, "UPDATE"},
-      {WriteKind::kDelete, "DELETE"},
-  };
-
-  return kNames;
-}
-
-} // namespace
-
-VELOX_DEFINE_ENUM_NAME(WriteKind, writeKindNames);
-
 } // namespace facebook::axiom::logical_plan

axiom/logical_plan/LogicalPlanNode.h

@@ -16,6 +16,7 @@
 #pragma once
 
 #include "axiom/common/Enums.h"
+#include "axiom/common/WriteKind.h"
 #include "axiom/logical_plan/Expr.h"
 #include "velox/type/Variant.h"
 #include "velox/vector/ComplexVector.h"
@@ -659,32 +660,6 @@ class UnnestNode : public LogicalPlanNode {
 
 using UnnestNodePtr = std::shared_ptr<const UnnestNode>;
 
-/// Specifies what type of write is intended when initiating or concluding a
-/// write operation.
-enum class WriteKind {
-  // A write operation to a new table which does not yet exist in the connector.
-  // Covers both creation of an empty table and create as select operations.
-  kCreate = 1,
-
-  // Rows are added and all columns must be specified for the TableWriter.
-  // Covers insert, Hive partition replacement or any other operation which adds
-  // whole rows.
-  kInsert = 2,
-
-  // Individual rows are deleted. Only row ids as per
-  // ConnectorMetadata::rowIdHandles() are passed to the TableWriter.
-  kDelete = 3,
-
-  // Column values in individual rows are changed. The TableWriter
-  // gets first the row ids as per ConnectorMetadata::rowIdHandles()
-  // and then new values for the columns being changed. The new values
-  // may overlap with row ids if the row id is a set of primary key
-  // columns.
-  kUpdate = 4,
-};
-
-AXIOM_DECLARE_ENUM_NAME(WriteKind);
-
 /// Implements create/insert/delete/update as per 'writeKind'.
 ///
 /// The output schema contains a single BIGINT column named 'rows'. The value of
@@ -756,5 +731,3 @@ class TableWriteNode : public LogicalPlanNode {
 using TableWriteNodePtr = std::shared_ptr<const TableWriteNode>;
 
 } // namespace facebook::axiom::logical_plan
-
-AXIOM_ENUM_FORMATTER(facebook::axiom::logical_plan::WriteKind);

axiom/optimizer/QueryGraph.h

@@ -939,7 +939,7 @@ class WritePlan : public PlanObject {
   /// @param columnExprs Expressions producing the values to write.
   WritePlan(
       const connector::Table& table,
-      connector::WriteKind kind,
+      WriteKind kind,
       ExprVector columnExprs)
       : PlanObject{PlanType::kWriteNode},
         table_{table},
@@ -952,7 +952,7 @@ class WritePlan : public PlanObject {
     return table_;
   }
 
-  connector::WriteKind kind() const {
+  WriteKind kind() const {
     return kind_;
   }
 
@@ -962,7 +962,7 @@ class WritePlan : public PlanObject {
 
  private:
   const connector::Table& table_;
-  const connector::WriteKind kind_;
+  const WriteKind kind_;
   const ExprVector columnExprs_;
 };
 

axiom/optimizer/ToGraph.cpp

@@ -1496,10 +1496,8 @@ PlanObjectP ToGraph::addLimit(const lp::LimitNode& limitNode) {
 }
 
 PlanObjectP ToGraph::addWrite(const lp::TableWriteNode& tableWrite) {
-  const auto writeKind =
-      static_cast<connector::WriteKind>(tableWrite.writeKind());
-  if (writeKind != connector::WriteKind::kInsert &&
-      writeKind != connector::WriteKind::kCreate) {
+  const auto writeKind = tableWrite.writeKind();
+  if (writeKind != WriteKind::kInsert && writeKind != WriteKind::kCreate) {
     VELOX_NYI("Only INSERT supported for TableWrite");
   }
   VELOX_CHECK_NULL(