From e732af8756e0e35f5ead1f9310380fc0312654e8 Mon Sep 17 00:00:00 2001
From: Your Name <yuchenlai87@gmail.com>
Date: Mon, 2 Mar 2026 11:44:02 +0800
Subject: [PATCH] Support SecondOfTime expression via version-specific shims

---
 .../org/apache/comet/serde/strings.scala      | 46 +++++++++++++++++++
 .../apache/comet/shims/CometExprShim.scala    |  5 ++
 .../apache/comet/shims/CometExprShim.scala    |  3 ++
 .../apache/comet/shims/CometExprShim.scala    |  3 ++
 .../apache/comet/CometExpressionSuite.scala   | 17 +++++++
 5 files changed, 74 insertions(+)

diff --git a/spark/src/main/scala/org/apache/comet/serde/strings.scala b/spark/src/main/scala/org/apache/comet/serde/strings.scala
index 64ba644048..b7648d8023 100644
--- a/spark/src/main/scala/org/apache/comet/serde/strings.scala
+++ b/spark/src/main/scala/org/apache/comet/serde/strings.scala
@@ -408,4 +408,50 @@ trait CommonStringExprs {
         None
     }
   }
+
+  def secondOfTimeToProto(
+      expr: Expression,
+      inputs: Seq[Attribute],
+      binding: Boolean): Option[Expr] = {
+    val childOpt = expr.children.headOption.orElse {
+      withInfo(expr, "SecondOfTime has no child expression")
+      None
+    }
+
+    childOpt.flatMap { child =>
+      val timeZoneId = {
+        val exprClass = expr.getClass
+        try {
+          val timeZoneIdMethod = exprClass.getMethod("timeZoneId")
+          timeZoneIdMethod.invoke(expr).asInstanceOf[Option[String]]
+        } catch {
+          case _: NoSuchMethodException =>
+            try {
+              val timeZoneIdField = exprClass.getField("timeZoneId")
+              timeZoneIdField.get(expr).asInstanceOf[Option[String]]
+            } catch {
+              case _: NoSuchFieldException | _: SecurityException => None
+            }
+        }
+      }
+
+      exprToProtoInternal(child, inputs, binding)
+        .map { childExpr =>
+          val builder = ExprOuterClass.Second.newBuilder()
+          builder.setChild(childExpr)
+
+          val timeZone = timeZoneId.getOrElse("UTC")
+          builder.setTimezone(timeZone)
+
+          ExprOuterClass.Expr
+            .newBuilder()
+            .setSecond(builder)
+            .build()
+        }
+        .orElse {
+          withInfo(expr, child)
+          None
+        }
+    }
+  }
 }
diff --git a/spark/src/main/spark-3.4/org/apache/comet/shims/CometExprShim.scala b/spark/src/main/spark-3.4/org/apache/comet/shims/CometExprShim.scala
index 600931c346..8a1a399ca2 100644
--- a/spark/src/main/spark-3.4/org/apache/comet/shims/CometExprShim.scala
+++ b/spark/src/main/spark-3.4/org/apache/comet/shims/CometExprShim.scala
@@ -21,9 +21,11 @@ package org.apache.comet.shims
 
 import org.apache.spark.sql.catalyst.expressions._
 
+import org.apache.comet.CometSparkSessionExtensions.withInfo
 import org.apache.comet.expressions.CometEvalMode
 import org.apache.comet.serde.CommonStringExprs
 import org.apache.comet.serde.ExprOuterClass.{BinaryOutputStyle, Expr}
+import org.apache.comet.serde.QueryPlanSerde.exprToProtoInternal
 
 /**
  * `CometExprShim` acts as a shim for parsing expressions from different Spark versions.
@@ -43,6 +45,9 @@ trait CometExprShim extends CommonStringExprs {
         // Right child is the encoding expression.
         stringDecode(expr, s.charset, s.bin, inputs, binding)
 
+      case _ if expr.getClass.getSimpleName == "SecondOfTime" =>
+        secondOfTimeToProto(expr, inputs, binding)
+
       case _ => None
     }
   }
diff --git a/spark/src/main/spark-3.5/org/apache/comet/shims/CometExprShim.scala b/spark/src/main/spark-3.5/org/apache/comet/shims/CometExprShim.scala
index 8e9cb1c07b..7f97f61606 100644
--- a/spark/src/main/spark-3.5/org/apache/comet/shims/CometExprShim.scala
+++ b/spark/src/main/spark-3.5/org/apache/comet/shims/CometExprShim.scala
@@ -91,6 +91,9 @@ trait CometExprShim extends CommonStringExprs {
 //        val optExpr = scalarFunctionExprToProto("width_bucket", childExprs: _*)
 //        optExprWithInfo(optExpr, wb, wb.children: _*)
 
+      case _ if expr.getClass.getSimpleName == "SecondOfTime" =>
+        secondOfTimeToProto(expr, inputs, binding)
+
       case _ => None
     }
   }
diff --git a/spark/src/main/spark-4.0/org/apache/comet/shims/CometExprShim.scala b/spark/src/main/spark-4.0/org/apache/comet/shims/CometExprShim.scala
index 2c5cebd166..53f3b9e0bb 100644
--- a/spark/src/main/spark-4.0/org/apache/comet/shims/CometExprShim.scala
+++ b/spark/src/main/spark-4.0/org/apache/comet/shims/CometExprShim.scala
@@ -113,6 +113,9 @@ trait CometExprShim extends CommonStringExprs {
 //        val optExpr = scalarFunctionExprToProto("width_bucket", childExprs: _*)
 //        optExprWithInfo(optExpr, wb, wb.children: _*)
 
+      case _ if expr.getClass.getSimpleName == "SecondOfTime" =>
+        secondOfTimeToProto(expr, inputs, binding)
+
       case _ => None
     }
   }
diff --git a/spark/src/test/scala/org/apache/comet/CometExpressionSuite.scala b/spark/src/test/scala/org/apache/comet/CometExpressionSuite.scala
index f0f022868f..fb7b096b84 100644
--- a/spark/src/test/scala/org/apache/comet/CometExpressionSuite.scala
+++ b/spark/src/test/scala/org/apache/comet/CometExpressionSuite.scala
@@ -572,6 +572,23 @@ class CometExpressionSuite extends CometTestBase with AdaptiveSparkPlanHelper {
     }
   }
 
+  test("seconds_of_time expression") {
+    // This test verifies that seconds() function works correctly with timestamp columns.
+    // If Spark generates SecondOfTime expression (a RuntimeReplaceable expression),
+    // it will be handled by the version-specific shim and converted to Second proto.
+    Seq(true, false).foreach { dictionaryEnabled =>
+      withTempDir { dir =>
+        val path = new Path(dir.toURI.toString, "part-r-0.parquet")
+        makeRawTimeParquetFile(path, dictionaryEnabled = dictionaryEnabled, 10000)
+        readParquetFile(path.toString) { df =>
+          val query = df.select(expr("second(_1)"))
+
+          checkSparkAnswerAndOperator(query)
+        }
+      }
+    }
+  }
+
   test("hour on int96 timestamp column") {
     import testImplicits._