alamb commented on code in PR #16970:
URL: https://github.com/apache/datafusion/pull/16970#discussion_r2246374299
##########
datafusion/core/src/execution/session_state.rs:
##########
@@ -738,6 +738,17 @@ impl SessionState {
self.config.options()
}
+ /// return the configuration options
+ pub fn config_options_arc(&self) -> Arc<ConfigOptions> {
+ self.config.options_arc()
+ }
+
+ /// Mark the start of the execution
Review Comment:
I agree, `mark_start_execution` would be a better name
##########
datafusion/core/tests/user_defined/user_defined_scalar_functions.rs:
##########
@@ -1786,3 +1786,58 @@ async fn test_extension_based_udf() -> Result<()> {
ctx.deregister_table("t")?;
Ok(())
}
+
+#[tokio::test]
+async fn test_config_options_work_for_scalar_func() -> Result<()> {
+ #[derive(Debug)]
+ struct TestScalarUDF {
+ signature: Signature,
+ }
+
+ impl ScalarUDFImpl for TestScalarUDF {
+ fn as_any(&self) -> &dyn Any {
+ self
+ }
+ fn name(&self) -> &str {
+ "TestScalarUDF"
+ }
+
+ fn signature(&self) -> &Signature {
+ &self.signature
+ }
+
+ fn return_type(&self, _arg_types: &[DataType]) -> Result<DataType> {
+ Ok(DataType::Utf8)
+ }
+
+ fn invoke_with_args(&self, args: ScalarFunctionArgs) ->
Result<ColumnarValue> {
+ let tz = args.config_options.execution.time_zone.clone();
+ Ok(ColumnarValue::Scalar(ScalarValue::from(tz)))
+ }
+ }
+
+ let udf = ScalarUDF::from(TestScalarUDF {
+ signature: Signature::uniform(1, vec![DataType::Utf8],
Volatility::Stable),
+ });
+
+ let mut config = SessionConfig::new();
+ config.options_mut().execution.time_zone = "AEST".into();
+
+ let ctx = SessionContext::new_with_config(config);
+
+ ctx.register_udf(udf.clone());
+
+ let df = ctx.read_empty()?;
+ let df = df.select(vec![udf.call(vec![lit("a")]).alias("a")])?;
+ let actual = df.collect().await?;
+
+ let expected_schema = Schema::new(vec![Field::new("a", DataType::Utf8,
false)]);
+ let expected = RecordBatch::try_new(
+ SchemaRef::from(expected_schema),
+ vec![create_array!(Utf8, ["AEST"])],
Review Comment:
nice
##########
datafusion/core/src/execution/session_state.rs:
##########
@@ -1887,8 +1898,8 @@ impl OptimizerConfig for SessionState {
&self.execution_props.alias_generator
}
- fn options(&self) -> &ConfigOptions {
- self.config_options()
+ fn options(&self) -> Arc<ConfigOptions> {
Review Comment:
If we are going to change this I suggest changing to `&Arc<ConfigOptions>`,
that way the caller can choose if they want to clone the Arc or not, and normal
borrowing rules work nicely
##########
datafusion/expr/src/execution_props.rs:
##########
@@ -35,6 +36,8 @@ pub struct ExecutionProps {
pub query_execution_start_time: DateTime<Utc>,
/// Alias generator used by subquery optimizer rules
pub alias_generator: Arc<AliasGenerator>,
+ /// Snapshot of config options
Review Comment:
```suggestion
/// Snapshot of config options when the query started
```
##########
datafusion/execution/src/config.rs:
##########
@@ -140,6 +140,11 @@ impl SessionConfig {
&self.options
}
+ /// Returns the config options as an Arc
+ pub fn options_arc(&self) -> Arc<ConfigOptions> {
+ Arc::clone(&self.options)
Review Comment:
I think we should change this to just be an &Arc
It might also be worth changing `options()` directly to return `&Arc` 🤔
```suggestion
/// Returns the config options as an Arc
pub fn options_arc(&self) -> &Arc<ConfigOptions> {
&self.options
```
##########
datafusion/core/src/execution/session_state.rs:
##########
@@ -738,6 +738,17 @@ impl SessionState {
self.config.options()
}
+ /// return the configuration options
+ pub fn config_options_arc(&self) -> Arc<ConfigOptions> {
Review Comment:
We could potentially change `config_options()` to return
`&Arc<ConfigOptions>` which would also be an API change but perhaps not that
disruptive as most code would not need to be changed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org
For additional commands, e-mail: github-h...@datafusion.apache.org
