alamb commented on issue #4517:
URL:
https://github.com/apache/arrow-datafusion/issues/4517#issuecomment-1338130834
So I started bashing this out, with the incremental way like:
```rust
/// DataFusion specific configuration names.
pub enum ConfigName
{
/// Configuration option "datafusion.execution.target_partitions"
TargetPartitions,
/// Configuration option
"datafusion.catalog.create_default_catalog_and_schema"
CreateDefaultCatalogAndSchema,
/// Configuration option "datafusion.catalog.information_schema"
InformationSchema,
/// Configuration option "datafusion.optimizer.repartition_joins"
RepartitionJoins,
/// Configuration option "datafusion.optimizer.repartition_aggregations"
RepartitionAggregates,
/// Configuration option "datafusion.optimizer.repartition_windows"
RepartitionWindows,
...}
```
But then I was thinking, if we are going to change the API anyways, why not
go all the way with something fully statically typed and removing
`ConfigDefinition` entirely:
```rust
/// DataFusion specific configuration names.
pub enum ConfigValue
{
/// Configuration option "datafusion.execution.target_partitions"
TargetPartitions(usize),
/// Configuration option for arbitrary user defined data
UserDefined {
name: String,
value: Option<ScalarValue>
},
...}
impl ConfigValue {
/// Return the name of this configuration value
fn name(&self) -> &str {
match self {
Self::TargetPartitions(_) => "datafusion.execution.target_partitions",
...
}
/// Return the human readable description for this configuration value
fn description(&self) -> &str {
match self {
Self::TargetPartitions(_) =>
"Number of partitions for query execution. Increasing
partitions can increase \
concurrency. Defaults to the number of cpu cores on the
system.",
...
}
/// set the value of the configuration value
fn set_value(&mut self, new_value: ScalarValue) ->Result<()>{
match (self, value) {
(Self::TargetPartitions(v), ScalarValue::UInt64(Some(new_value))) =>
*v = new_value,
(Self::TargetPartitions(v), _) => return Err("Expected uint64 for {}
but got {:?}", self.name(), new_value)).
...
}
```
But before I go crank that through the process I wanted to get some feedback
if that was a desirable way to go. I would retain the ability to store
arbitrary name/value pairs in the metadata.
What do you think @thinkharderdev @yahoNanJing @andygrove @avantgardnerio ?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
