From 38363f359342b418e919eb242fa4da84d4c8c503 Mon Sep 17 00:00:00 2001
From: eitsupi <ts1s1andn@gmail.com>
Date: Sun, 9 Nov 2025 04:44:11 +0000
Subject: [PATCH 1/2] perf: Validate attributes of S7 objects on the Rust side

---
 R/functions-lazy.R             |  1 -
 src/rust/src/conversion/mod.rs |  6 ++++++
 src/rust/src/conversion/s7.rs  | 16 +++++++---------
 3 files changed, 13 insertions(+), 10 deletions(-)

diff --git a/R/functions-lazy.R b/R/functions-lazy.R
index e1d60b460..6b0533c0b 100644
--- a/R/functions-lazy.R
+++ b/R/functions-lazy.R
@@ -157,7 +157,6 @@ pl__collect_all <- function(
     # TODO: add support for argument `optimizations`
     optflags <- QueryOptFlags()
     check_is_S7(optflags, QueryOptFlags)
-    validate(optflags)
 
     lfs <- lapply(lazy_frames, \(x) x$`_ldf`)
 
diff --git a/src/rust/src/conversion/mod.rs b/src/rust/src/conversion/mod.rs
index 53a80c138..b198da3c9 100644
--- a/src/rust/src/conversion/mod.rs
+++ b/src/rust/src/conversion/mod.rs
@@ -40,6 +40,12 @@ impl<T> From<T> for Wrap<T> {
     }
 }
 
+// TODO: Move this to upstream?
+pub(crate) fn try_extract_attribute(obj: &Sexp, attr_name: &str) -> savvy::Result<Sexp> {
+    obj.get_attrib(attr_name)?
+        .ok_or_else(|| savvy::Error::from(format!("Attribute '{}' does not exist.", attr_name)))
+}
+
 impl TryFrom<&str> for PlRDataType {
     type Error = String;
 
diff --git a/src/rust/src/conversion/s7.rs b/src/rust/src/conversion/s7.rs
index e56a2fee9..c6f0e9ab1 100644
--- a/src/rust/src/conversion/s7.rs
+++ b/src/rust/src/conversion/s7.rs
@@ -1,5 +1,6 @@
+use super::try_extract_attribute;
 use crate::lazyframe::PlROptFlags;
-use savvy::{Sexp, TypedSexp};
+use savvy::Sexp;
 
 impl TryFrom<Sexp> for PlROptFlags {
     type Error = savvy::Error;
@@ -23,13 +24,10 @@ impl TryFrom<Sexp> for PlROptFlags {
             "streaming",
         ];
 
-        ATTR_NAMES.iter().for_each(|attr_name| {
-            // Safety: validated on the R side
-            let attr_value = match obj.get_attrib(attr_name).unwrap().unwrap().into_typed() {
-                TypedSexp::Logical(l) => l.iter().next().unwrap(),
-                _ => unreachable!(),
-            };
-            match *attr_name {
+        for &attr_name in ATTR_NAMES {
+            let attr_value: bool = try_extract_attribute(&obj, attr_name)?.try_into()?;
+
+            match attr_name {
                 "type_coercion" => opts.set_type_coercion(attr_value),
                 "type_check" => opts.set_type_check(attr_value),
                 "predicate_pushdown" => opts.set_predicate_pushdown(attr_value),
@@ -45,7 +43,7 @@ impl TryFrom<Sexp> for PlROptFlags {
                 "streaming" => opts.set_streaming(attr_value),
                 _ => unreachable!(),
             }
-        });
+        }
         Ok(opts)
     }
 }

From b9fc0e9b0e6aee3869b6722a1584767b9b9d10ee Mon Sep 17 00:00:00 2001
From: eitsupi <ts1s1andn@gmail.com>
Date: Sun, 9 Nov 2025 05:08:45 +0000
Subject: [PATCH 2/2] refactor: simplify

---
 src/rust/src/conversion/mod.rs | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/rust/src/conversion/mod.rs b/src/rust/src/conversion/mod.rs
index b198da3c9..0df57cdea 100644
--- a/src/rust/src/conversion/mod.rs
+++ b/src/rust/src/conversion/mod.rs
@@ -6,7 +6,7 @@ pub use categorical::PlRCategories;
 use polars::series::ops::NullBehavior;
 use savvy::{
     ListSexp, NotAvailableValue, NumericScalar, NumericSexp, NumericTypedSexp, Sexp, StringSexp,
-    TypedSexp,
+    TypedSexp, savvy_err,
 };
 use search_sorted::SearchSortedSide;
 use std::{num::NonZeroUsize, str::FromStr};
@@ -43,7 +43,7 @@ impl<T> From<T> for Wrap<T> {
 // TODO: Move this to upstream?
 pub(crate) fn try_extract_attribute(obj: &Sexp, attr_name: &str) -> savvy::Result<Sexp> {
     obj.get_attrib(attr_name)?
-        .ok_or_else(|| savvy::Error::from(format!("Attribute '{}' does not exist.", attr_name)))
+        .ok_or(savvy_err!("Attribute '{attr_name}' does not exist."))
 }
 
 impl TryFrom<&str> for PlRDataType {