erenavsarogullari opened a new pull request, #20372: URL: https://github.com/apache/datafusion/pull/20372
## Which issue does this PR close? - Closes #20371. ## Rationale for this change 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 mentioned in error message. This seems to introduce `inconsistency` between `disk` vs `memory` limit. This error message can be updated more generic as capacity limit by setting problematic 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 '1024B' SET datafusion.runtime.max_temp_directory_size = '1024B' ``` **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' ``` ## What changes are included in this PR? Following improvements are being offered: 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. ## Are these changes tested? Yes and adding new UT cases ## Are there any user-facing changes? Yes, new more detailed and generic error messages are exposed to end-users. -- 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]
