erenavsarogullari opened a new issue, #20371:
URL: https://github.com/apache/datafusion/issues/20371

   ### Is your feature request related to a problem or challenge?
   
   Currently, `datafusion.runtime.max_temp_directory_size` is a disk based 
config but when it is set as `invalid limit` or `invalid unit`, `memory limit` 
is being mentioned in error message. This seems to introduce inconsistency 
between `disk` vs `memory` `limit`. This error message can be updated more 
generic by setting config name and covering both memory and disk based 
capacity/size settings.
   
   **Current:**
   ```
   statement error DataFusion error: Error during planning: Failed to parse 
number from memory limit 'invalid_size'
   SET datafusion.runtime.max_temp_directory_size = 'invalid_size'
   
   statement error DataFusion error: Error during planning: Unsupported unit 
'B' in memory limit '1000B'
   SET datafusion.runtime.max_temp_directory_size = '1000B'
   ```
   
   **New:**
   ```
   statement error DataFusion error: Error during planning: Failed to parse 
number from 'datafusion.runtime.max_temp_directory_size', limit 'invalid_size'
   SET datafusion.runtime.max_temp_directory_size = 'invalid_size'
   
   statement error DataFusion error: Error during planning: Unsupported unit 
'B' in 'datafusion.runtime.max_temp_directory_size', limit '1024B'. Unit must 
be one of: 'K', 'M', 'G'
   SET datafusion.runtime.max_temp_directory_size = '1024B'
   ```
   
   ### Describe the solution you'd like
   
   Following improvements can be applied:
   1. Setting problematic config name in error message to cover all use-cases 
(e.g: both memory and disk based capacity/size settings),
   2. Allowed units are also added in error message,
   3. `SessionContext.parse_memory_limit()` => 
`SessionContext.parse_capacity_limit()` function signature is also being 
renamed to cover both memory and disk based capacity/size settings. This is 
applied to both `SessionContext` and `Benchmark Utils`,
   4. Being added new UT cases to cover these negative use-cases in terms of 
above changes.
   
   ### Describe alternatives you've considered
   
   _No response_
   
   ### Additional context
   
   _No response_


-- 
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]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to