Re: [PR] Minor: Csv Options Clean-up [datafusion]

2024-05-27 Thread via GitHub
berkaysynnada commented on PR #10650: URL: https://github.com/apache/datafusion/pull/10650#issuecomment-2133239672 After talking with @ozankabak, we decided to undo the changes related to header options. There's also another place where `SessionState` is used in the `FileFormat` API

Re: [PR] Minor: Csv Options Clean-up [datafusion]

2024-05-27 Thread via GitHub
berkaysynnada commented on PR #10650: URL: https://github.com/apache/datafusion/pull/10650#issuecomment-2132854613 > The upside of the approach in this PR is that it simplifies the code across multiple files (e.g. proto), but the fallback behavior no longer becomes automatic for all custom

Re: [PR] Minor: Csv Options Clean-up [datafusion]

2024-05-26 Thread via GitHub
berkaysynnada commented on code in PR #10650: URL: https://github.com/apache/datafusion/pull/10650#discussion_r1615535832 ## datafusion/core/src/datasource/file_format/csv.rs: ## @@ -301,13 +296,7 @@ impl CsvFormat { while let Some(chunk) = stream.next().await.transpo

Re: [PR] Minor: Csv Options Clean-up [datafusion]

2024-05-25 Thread via GitHub
alamb commented on PR #10650: URL: https://github.com/apache/datafusion/pull/10650#issuecomment-2131235103 > Maybe we can do the following: Instead of making every TableProviderFactory responsible for implementing this behavior in create, we can add some general mechanism at the trait level

Re: [PR] Minor: Csv Options Clean-up [datafusion]

2024-05-25 Thread via GitHub
ozankabak commented on PR #10650: URL: https://github.com/apache/datafusion/pull/10650#issuecomment-2131118238 Maybe I can help clarify things a little bit. The purpose of the PR is not to change the current fallback behavior, just simplify its implementation. AFAICT @berkaysynnada is movin

Re: [PR] Minor: Csv Options Clean-up [datafusion]

2024-05-25 Thread via GitHub
ozankabak commented on code in PR #10650: URL: https://github.com/apache/datafusion/pull/10650#discussion_r1614446423 ## datafusion/physical-expr/src/expressions/cast.rs: ## @@ -170,7 +170,8 @@ impl PhysicalExpr for CastExpr { let target_type = &self.cast_type;

Re: [PR] Minor: Csv Options Clean-up [datafusion]

2024-05-24 Thread via GitHub
alamb commented on code in PR #10650: URL: https://github.com/apache/datafusion/pull/10650#discussion_r1614042615 ## datafusion/functions/Cargo.toml: ## @@ -80,7 +80,7 @@ itertools = { workspace = true } log = { workspace = true } md-5 = { version = "^0.10.0", optional = true

Re: [PR] Minor: Csv Options Clean-up [datafusion]

2024-05-24 Thread via GitHub
metegenez commented on code in PR #10650: URL: https://github.com/apache/datafusion/pull/10650#discussion_r1613754503 ## datafusion/core/src/datasource/file_format/csv.rs: ## @@ -301,13 +296,7 @@ impl CsvFormat { while let Some(chunk) = stream.next().await.transpose()

[PR] Minor: Csv Options Clean-up [datafusion]

2024-05-24 Thread via GitHub
berkaysynnada opened a new pull request, #10650: URL: https://github.com/apache/datafusion/pull/10650 ## Which issue does this PR close? Closes #. ## Rationale for this change When CSV header option is not specified from the options clause, it is set from