From 7106e33be276f5cfaf72b2d5e8fc77e792cc10ce Mon Sep 17 00:00:00 2001
From: a-hirota <a-hirota@nri.co.jp>
Date: Sun, 21 Sep 2025 03:19:36 +0000
Subject: [PATCH] Add defensive checks for Parquet statistics handling and
 improve library path resolution

This commit addresses potential crashes when reading Parquet files with statistics,
particularly when there are ABI incompatibilities between compile-time and runtime
Arrow/Parquet library versions.

Changes:
- arrow_meta.cpp: Enhanced null pointer checks and exception handling for statistics access
  * Removed redundant shared_ptr.get() checks
  * Added specific exception catching with debug logging
  * Separated try-catch blocks for null count and min/max statistics access
  * Improved granular error handling to isolate different types of statistics failures
- Makefile: Added automatic rpath configuration for both Parquet and Arrow libraries
  * Ensures runtime library path matches compile-time library selection
  * Covers both HAS_PARQUET=yes and Arrow-only scenarios

The defensive programming approach prevents segmentation faults when virtual method
calls fail due to vtable corruption from library version mismatches, while providing
better debugging information in debug builds. Each statistics method call is now
individually protected to maximize data recovery in case of partial failures.
---
 src/Makefile       | 10 +++++++++
 src/arrow_meta.cpp | 52 +++++++++++++++++++++++++++++++++++++++++-----
 2 files changed, 57 insertions(+), 5 deletions(-)

diff --git a/src/Makefile b/src/Makefile
index a0e7df78..c88bd5a2 100644
--- a/src/Makefile
+++ b/src/Makefile
@@ -78,8 +78,18 @@ ifeq ($(HAS_PARQUET),yes)
 PGSTROM_FLAGS += -DHAS_PARQUET=1
 PG_CXXFLAGS += $(shell pkgconf --cflags parquet)
 SHLIB_LINK += $(shell pkgconf --libs parquet)
+# Add runtime library path
+PARQUET_LIBDIR = $(shell pkgconf --variable=libdir parquet)
+ifneq ($(PARQUET_LIBDIR),)
+SHLIB_LINK += -Wl,-rpath,$(PARQUET_LIBDIR)
+endif
 else
 SHLIB_LINK += $(shell pkgconf --libs arrow)
+# Add runtime library path for Arrow
+ARROW_LIBDIR = $(shell pkgconf --variable=libdir arrow)
+ifneq ($(ARROW_LIBDIR),)
+SHLIB_LINK += -Wl,-rpath,$(ARROW_LIBDIR)
+endif
 endif
 endif
 
diff --git a/src/arrow_meta.cpp b/src/arrow_meta.cpp
index f1e9c562..e6840f8d 100644
--- a/src/arrow_meta.cpp
+++ b/src/arrow_meta.cpp
@@ -3223,7 +3223,26 @@ __readParquetMinMaxStats(ArrowFieldNode *field,
 	std::string	min_datum;
 	std::string	max_datum;
 
-	switch (stats->physical_type())
+	if (!stats)
+		return;
+
+	// Safely attempt to get physical_type
+	parquet::Type::type phys_type;
+	try {
+		phys_type = stats->physical_type();
+	} catch (const std::exception& e) {
+#ifdef PGSTROM_DEBUG
+		elog(DEBUG1, "Failed to get physical_type from Parquet statistics: %s", e.what());
+#endif
+		return;
+	} catch (...) {
+#ifdef PGSTROM_DEBUG
+		elog(DEBUG1, "Unknown error getting physical_type from Parquet statistics");
+#endif
+		return;
+	}
+
+	switch (phys_type)
 	{
 		case parquet::Type::BOOLEAN: {
 			auto __stat = std::dynamic_pointer_cast<const parquet::BoolStatistics>(stats);
@@ -3359,10 +3378,33 @@ __readParquetRowGroupMetadata(ArrowMessage *rbatch_message,
 		field->length = col_meta->num_values();
 		if (stats)
 		{
-			if (stats->HasNullCount())
-				field->null_count = stats->null_count();
-			if (stats->HasMinMax())
-				__readParquetMinMaxStats(field, stats);
+			// Handle null count statistics
+			try {
+				if (stats->HasNullCount())
+					field->null_count = stats->null_count();
+			} catch (const std::exception& e) {
+#ifdef PGSTROM_DEBUG
+				elog(DEBUG1, "Failed to access null count statistics: %s", e.what());
+#endif
+			} catch (...) {
+#ifdef PGSTROM_DEBUG
+				elog(DEBUG1, "Unknown error accessing null count statistics");
+#endif
+			}
+
+			// Handle min/max statistics separately
+			try {
+				if (stats->HasMinMax())
+					__readParquetMinMaxStats(field, stats);
+			} catch (const std::exception& e) {
+#ifdef PGSTROM_DEBUG
+				elog(DEBUG1, "Failed to access min/max statistics: %s", e.what());
+#endif
+			} catch (...) {
+#ifdef PGSTROM_DEBUG
+				elog(DEBUG1, "Unknown error accessing min/max statistics");
+#endif
+			}
 		}
 		/*
 		 * Some additional Parquet specific attrobutes for dump only