Allow rename and deletion of subfields

rui-mo · rui-mo · commit a3638d6c511d · 2025-12-25T12:35:49.000+08:00
diff --git a/velox/connectors/hive/HiveConnectorUtil.cpp b/velox/connectors/hive/HiveConnectorUtil.cpp
@@ -569,7 +569,7 @@ void configureReaderOptions(
       break;
     }
     default:
-      useColumnNamesForColumnMapping = false;
+      break;
   }
 
   readerOptions.setUseColumnNamesForColumnMapping(
diff --git a/velox/connectors/hive/SplitReader.cpp b/velox/connectors/hive/SplitReader.cpp
@@ -440,10 +440,11 @@ std::vector<TypePtr> SplitReader::adaptColumns(
       auto fileTypeIdx = fileType->getChildIdxIfExists(fieldName);
       if (!fileTypeIdx.has_value()) {
         // Column is missing. Most likely due to schema evolution.
-        VELOX_CHECK(tableSchema, "Unable to resolve column '{}'", fieldName);
+        auto schema = tableSchema ? tableSchema : readerOutputType_;
+        VELOX_CHECK(schema, "Unable to resolve column '{}'", fieldName);
         childSpec->setConstantValue(
             BaseVector::createNullConstant(
-                tableSchema->findChild(fieldName),
+                schema->findChild(fieldName),
                 1,
                 connectorQueryCtx_->memoryPool()));
       } else {
diff --git a/velox/dwio/common/SelectiveFlatMapColumnReader.h b/velox/dwio/common/SelectiveFlatMapColumnReader.h
@@ -24,11 +24,13 @@ namespace facebook::velox::dwio::common {
 class SelectiveFlatMapColumnReader : public SelectiveStructColumnReaderBase {
  protected:
   SelectiveFlatMapColumnReader(
+      const dwio::common::ColumnReaderOptions& columnReaderOptions,
       const TypePtr& requestedType,
       const std::shared_ptr<const dwio::common::TypeWithId>& fileType,
       FormatParams& params,
       velox::common::ScanSpec& scanSpec)
       : SelectiveStructColumnReaderBase(
+            columnReaderOptions,
             requestedType,
             fileType,
             params,
diff --git a/velox/dwio/common/SelectiveStructColumnReader.cpp b/velox/dwio/common/SelectiveStructColumnReader.cpp
@@ -425,7 +425,9 @@ void SelectiveStructColumnReaderBase::read(
   }
 
   const auto& childSpecs = scanSpec_->children();
-  VELOX_CHECK(!childSpecs.empty());
+  if (!columnReaderOptions_.useColumnNamesForColumnMapping_) {
+    VELOX_CHECK(!childSpecs.empty());
+  }
   for (size_t i = 0; i < childSpecs.size(); ++i) {
     const auto& childSpec = childSpecs[i];
 
@@ -524,9 +526,12 @@ bool SelectiveStructColumnReaderBase::isChildMissing(
                                                   // row type that doesn't exist
                                                   // in the output.
        fileType_->type()->kind() !=
-           TypeKind::MAP && // If this is the case it means this is a flat map,
-                            // so it can't have "missing" fields.
-       childSpec.channel() >= fileType_->size());
+           TypeKind::MAP // If this is the case it means this is a flat map,
+                         // so it can't have "missing" fields.
+       ) &&
+      (columnReaderOptions_.useColumnNamesForColumnMapping_
+           ? !asRowType(fileType_->type())->containsChild(childSpec.fieldName())
+           : childSpec.channel() >= fileType_->size());
 }
 
 std::unique_ptr<velox::dwio::common::ColumnLoader>
@@ -538,7 +543,9 @@ SelectiveStructColumnReaderBase::makeColumnLoader(vector_size_t index) {
 void SelectiveStructColumnReaderBase::getValues(
     const RowSet& rows,
     VectorPtr* result) {
-  VELOX_CHECK(!scanSpec_->children().empty());
+  if (!columnReaderOptions_.useColumnNamesForColumnMapping_) {
+    VELOX_CHECK(!scanSpec_->children().empty());
+  }
   VELOX_CHECK_NOT_NULL(
       *result, "SelectiveStructColumnReaderBase expects a non-null result");
   VELOX_CHECK(
diff --git a/velox/dwio/common/SelectiveStructColumnReader.h b/velox/dwio/common/SelectiveStructColumnReader.h
@@ -16,6 +16,7 @@
 
 #pragma once
 
+#include "velox/dwio/common/Options.h"
 #include "velox/dwio/common/SelectiveColumnReaderInternal.h"
 
 namespace facebook::velox::dwio::common {
@@ -113,13 +114,15 @@ class SelectiveStructColumnReaderBase : public SelectiveColumnReader {
   static constexpr int32_t kConstantChildSpecSubscript{-1};
 
   SelectiveStructColumnReaderBase(
+      const dwio::common::ColumnReaderOptions& columnReaderOptions,
       const TypePtr& requestedType,
       const std::shared_ptr<const dwio::common::TypeWithId>& fileType,
       FormatParams& params,
       velox::common::ScanSpec& scanSpec,
       bool isRoot = false,
       bool generateLazyChildren = true)
       : SelectiveColumnReader(requestedType, fileType, params, scanSpec),
+        columnReaderOptions_(columnReaderOptions),
         debugString_(
             getExceptionContext().message(VeloxException::Type::kSystem)),
         isRoot_(isRoot),
@@ -180,6 +183,8 @@ class SelectiveStructColumnReaderBase : public SelectiveColumnReader {
     }
   }
 
+  const dwio::common::ColumnReaderOptions& columnReaderOptions_;
+
   // Context information obtained from ExceptionContext. Stored here
   // so that LazyVector readers under this can add this to their
   // ExceptionContext. Allows contextualizing reader errors to split
diff --git a/velox/dwio/dwrf/reader/DwrfReader.cpp b/velox/dwio/dwrf/reader/DwrfReader.cpp
@@ -320,13 +320,14 @@ DwrfRowReader::DwrfRowReader(
     makeProjectedNodes(*getReader().schemaWithId(), *projectedNodes_);
   }
 
+  // Reader options must be configured before calling 'getUnitLoader()',
+  // which triggers 'SelectiveDwrfReader::build'.
+  columnReaderOptions_ = dwio::common::makeColumnReaderOptions(
+      readerBaseShared()->readerOptions());
   unitLoader_ = getUnitLoader();
   if (!emptyFile()) {
     getReader().loadCache();
   }
-
-  columnReaderOptions_ = dwio::common::makeColumnReaderOptions(
-      readerBaseShared()->readerOptions());
 }
 
 std::unique_ptr<ColumnReader>& DwrfRowReader::getColumnReader() {
diff --git a/velox/dwio/dwrf/reader/SelectiveFlatMapColumnReader.cpp b/velox/dwio/dwrf/reader/SelectiveFlatMapColumnReader.cpp
@@ -203,6 +203,7 @@ class SelectiveFlatMapAsStructReader : public SelectiveStructColumnReaderBase {
       DwrfParams& params,
       common::ScanSpec& scanSpec)
       : SelectiveStructColumnReaderBase(
+            columnReaderOptions,
             requestedType,
             fileType,
             params,
@@ -242,6 +243,7 @@ class SelectiveFlatMapAsMapReader : public SelectiveStructColumnReaderBase {
       DwrfParams& params,
       common::ScanSpec& scanSpec)
       : SelectiveStructColumnReaderBase(
+            columnReaderOptions,
             requestedType,
             fileType,
             params,
@@ -286,6 +288,7 @@ class SelectiveFlatMapReader
       DwrfParams& params,
       common::ScanSpec& scanSpec)
       : dwio::common::SelectiveFlatMapColumnReader(
+            columnReaderOptions,
             requestedType,
             fileType,
             params,
diff --git a/velox/dwio/dwrf/reader/SelectiveStructColumnReader.cpp b/velox/dwio/dwrf/reader/SelectiveStructColumnReader.cpp
@@ -31,6 +31,7 @@ SelectiveStructColumnReader::SelectiveStructColumnReader(
     common::ScanSpec& scanSpec,
     bool isRoot)
     : SelectiveStructColumnReaderBase(
+          columnReaderOptions,
           requestedType,
           fileType,
           params,
diff --git a/velox/dwio/dwrf/reader/SelectiveStructColumnReader.h b/velox/dwio/dwrf/reader/SelectiveStructColumnReader.h
@@ -25,13 +25,15 @@ class SelectiveStructColumnReaderBase
     : public dwio::common::SelectiveStructColumnReaderBase {
  public:
   SelectiveStructColumnReaderBase(
+      const dwio::common::ColumnReaderOptions& columnReaderOptions,
       const TypePtr& requestedType,
       const std::shared_ptr<const dwio::common::TypeWithId>& fileType,
       DwrfParams& params,
       common::ScanSpec& scanSpec,
       bool isRoot = false,
       bool generateLazyChildren = true)
       : dwio::common::SelectiveStructColumnReaderBase(
+            columnReaderOptions,
             requestedType,
             fileType,
             params,
diff --git a/velox/dwio/parquet/reader/StructColumnReader.cpp b/velox/dwio/parquet/reader/StructColumnReader.cpp
@@ -32,7 +32,12 @@ StructColumnReader::StructColumnReader(
     const std::shared_ptr<const dwio::common::TypeWithId>& fileType,
     ParquetParams& params,
     common::ScanSpec& scanSpec)
-    : SelectiveStructColumnReader(requestedType, fileType, params, scanSpec) {
+    : SelectiveStructColumnReader(
+          columnReaderOptions,
+          requestedType,
+          fileType,
+          params,
+          scanSpec) {
   auto& childSpecs = scanSpec_->stableChildren();
   for (auto i = 0; i < childSpecs.size(); ++i) {
     auto childSpec = childSpecs[i];
diff --git a/velox/exec/tests/TableScanTest.cpp b/velox/exec/tests/TableScanTest.cpp
@@ -1196,6 +1196,111 @@ TEST_F(TableScanTest, missingColumnsInRepeatedColumns) {
       .assertResults(expected);
 }
 
+TEST_F(TableScanTest, structMatchByName) {
+  const auto assertSelectUseColumnNames =
+      [this](
+          const RowTypePtr& outputType,
+          const std::string& sql,
+          const std::string& filePath,
+          const std::string& remainingFilter = "") {
+        const auto plan =
+            PlanBuilder().tableScan(outputType, {}, remainingFilter).planNode();
+        AssertQueryBuilder(plan, duckDbQueryRunner_)
+            .connectorSessionProperty(
+                kHiveConnectorId,
+                connector::hive::HiveConfig::kOrcUseColumnNamesSession,
+                "true")
+            .split(makeHiveConnectorSplit(filePath))
+            .assertResults(sql);
+      };
+
+  std::vector<int64_t> values = {2};
+  const auto id = makeFlatVector<int64_t>(values);
+  const auto name = makeRowVector(
+      {"first", "last"},
+      {
+          makeFlatVector<std::string>({"Janet"}),
+          makeFlatVector<std::string>({"Jones"}),
+      });
+  const auto address = makeFlatVector<std::string>({"567 Maple Drive"});
+  auto vector = makeRowVector({"id", "name", "address"}, {id, name, address});
+
+  auto file = TempFilePath::create();
+  writeToFile(file->getPath(), {vector});
+  createDuckDbTable({vector});
+
+  assertSelectUseColumnNames(
+      asRowType(vector->type()),
+      "SELECT id, name, address from tmp",
+      file->getPath());
+
+  // Add one non-existing subfield 'middle' to the 'name' field and rename field
+  // 'address'.
+  auto rowType =
+      ROW({"id", "name", "email"},
+          {BIGINT(),
+           ROW({"first", "middle", "last"}, {VARCHAR(), VARCHAR(), VARCHAR()}),
+           VARCHAR()});
+  assertSelectUseColumnNames(
+      rowType, "SELECT 2, ('Janet', null, 'Jones'), null", file->getPath());
+
+  // Filter pushdown on the non-existing field.
+  assertSelectUseColumnNames(
+      rowType,
+      "SELECT * from tmp where false",
+      file->getPath(),
+      "not(is_null(name.middle))");
+
+  // Rename subfields of the 'name' field.
+  rowType =
+      ROW({"id", "name", "address"},
+          {BIGINT(), ROW({"a", "b"}, {VARCHAR(), VARCHAR()}), VARCHAR()});
+  assertSelectUseColumnNames(
+      rowType, "SELECT 2, row(null, null), '567 Maple Drive'", file->getPath());
+
+  // Filter pushdown on the NULL subfield.
+  assertSelectUseColumnNames(
+      rowType,
+      "SELECT * from tmp where false",
+      file->getPath(),
+      "not(is_null(name.a))");
+
+  // Deletion of one subfield from the 'name' field.
+  rowType =
+      ROW({"id", "name", "address"},
+          {BIGINT(), ROW({"full"}, {VARCHAR()}), VARCHAR()});
+  assertSelectUseColumnNames(
+      rowType, "SELECT 2, row(null), '567 Maple Drive'", file->getPath());
+
+  // Filter pushdown on the non-existing subfield.
+  assertSelectUseColumnNames(
+      rowType,
+      "SELECT * from tmp where false",
+      file->getPath(),
+      "not(is_null(name.full))");
+
+  // No subfield in the 'name' field.
+  rowType = ROW({"id", "name", "address"}, {BIGINT(), ROW({}, {}), VARCHAR()});
+  const auto op = PlanBuilder()
+                      .startTableScan()
+                      .outputType(rowType)
+                      .dataColumns(rowType)
+                      .endTableScan()
+                      .planNode();
+  const auto split = makeHiveConnectorSplit(file->getPath());
+  const auto result =
+      AssertQueryBuilder(op)
+          .connectorSessionProperty(
+              kHiveConnectorId,
+              connector::hive::HiveConfig::kOrcUseColumnNamesSession,
+              "true")
+          .split(split)
+          .copyResults(pool());
+  const auto rows = result->as<RowVector>();
+  const auto expected = makeRowVector(ROW({}, {}), 1);
+  facebook::velox::test::assertEqualVectors(expected, rows->childAt(1));
+}
+
 // Tests queries that use Lazy vectors with multiple layers of wrapping.
 TEST_F(TableScanTest, constDictLazy) {
   vector_size_t size = 1'000;

Original file line number	Diff line number	Diff line change
`@@ -569,7 +569,7 @@ void configureReaderOptions(`
`569`	`569`	`break;`
`570`	`570`	`}`
`571`	`571`	`default:`
`572`		`- useColumnNamesForColumnMapping = false;`
	`572`	`+ break;`
`573`	`573`	`}`
`574`	`574`
`575`	`575`	`readerOptions.setUseColumnNamesForColumnMapping(`
Original file line number	Diff line number	Diff line change
`@@ -320,13 +320,14 @@ DwrfRowReader::DwrfRowReader(`
`320`	`320`	`makeProjectedNodes(getReader().schemaWithId(), projectedNodes_);`
`321`	`321`	`}`
`322`	`322`
	`323`	`+ // Reader options must be configured before calling 'getUnitLoader()',`
	`324`	`+ // which triggers 'SelectiveDwrfReader::build'.`
	`325`	`+ columnReaderOptions_ = dwio::common::makeColumnReaderOptions(`
	`326`	`+ readerBaseShared()->readerOptions());`
`323`	`327`	`unitLoader_ = getUnitLoader();`
`324`	`328`	`if (!emptyFile()) {`
`325`	`329`	`getReader().loadCache();`
`326`	`330`	`}`
`327`		`-`
`328`		`- columnReaderOptions_ = dwio::common::makeColumnReaderOptions(`
`329`		`- readerBaseShared()->readerOptions());`
`330`	`331`	`}`
`331`	`332`
`332`	`333`	`std::unique_ptr<ColumnReader>& DwrfRowReader::getColumnReader() {`