colinmarc commented on PR #16750:
URL: https://github.com/apache/datafusion/pull/16750#issuecomment-3089114038
The doc tests were fiddly, sorry about that. I got them to pass locally now.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on
colinmarc commented on PR #16750:
URL: https://github.com/apache/datafusion/pull/16750#issuecomment-3073352066
> Otherwise since this PR changes the API and will require existing users to
upgrade, I think the only other thing missing in this PR is a note in the
upgrade guide. Basically it c
colinmarc commented on code in PR #16750:
URL: https://github.com/apache/datafusion/pull/16750#discussion_r2207305842
##
datafusion/proto/src/logical_plan/mod.rs:
##
@@ -117,18 +117,18 @@ pub trait LogicalExtensionCodec: Debug + Send + Sync {
fn try_encode(&self, node: &E
colinmarc commented on code in PR #16750:
URL: https://github.com/apache/datafusion/pull/16750#discussion_r2207303997
##
datafusion/catalog/src/default_table_source.rs:
##
@@ -87,7 +107,7 @@ impl TableSource for DefaultTableSource {
}
}
-/// Wrap TableProvider in TableSo
colinmarc commented on code in PR #16750:
URL: https://github.com/apache/datafusion/pull/16750#discussion_r2207303459
##
datafusion/catalog/src/default_table_source.rs:
##
@@ -45,6 +45,26 @@ impl DefaultTableSource {
pub fn new(table_provider: Arc) -> Self {
Self {
alamb commented on PR #16750:
URL: https://github.com/apache/datafusion/pull/16750#issuecomment-3070675944
> Somewhat unrelated, but maybe something like the following would be
cleaner (untested)?
>
> ```rust
> trait TableProvider: TableSource {
> // only scan and get_table
alamb commented on code in PR #16750:
URL: https://github.com/apache/datafusion/pull/16750#discussion_r2205627323
##
datafusion/catalog/src/default_table_source.rs:
##
@@ -45,6 +45,26 @@ impl DefaultTableSource {
pub fn new(table_provider: Arc) -> Self {
Self { tab
colinmarc commented on PR #16750:
URL: https://github.com/apache/datafusion/pull/16750#issuecomment-3066014621
Somewhat unrelated, but maybe something like the following would be cleaner
(untested)?
```rust
trait TableProvider: TableSource {
// only scan and get_table_defin
colinmarc commented on code in PR #16750:
URL: https://github.com/apache/datafusion/pull/16750#discussion_r2202889418
##
datafusion/catalog/src/default_table_source.rs:
##
@@ -45,6 +45,26 @@ impl DefaultTableSource {
pub fn new(table_provider: Arc) -> Self {
Self {
colinmarc commented on code in PR #16750:
URL: https://github.com/apache/datafusion/pull/16750#discussion_r2202889418
##
datafusion/catalog/src/default_table_source.rs:
##
@@ -45,6 +45,26 @@ impl DefaultTableSource {
pub fn new(table_provider: Arc) -> Self {
Self {
alamb commented on code in PR #16750:
URL: https://github.com/apache/datafusion/pull/16750#discussion_r2202549466
##
datafusion/catalog/src/default_table_source.rs:
##
@@ -45,6 +45,26 @@ impl DefaultTableSource {
pub fn new(table_provider: Arc) -> Self {
Self { tab
colinmarc commented on PR #16750:
URL: https://github.com/apache/datafusion/pull/16750#issuecomment-3063952865
I added a test! Let me know if that seems like enough.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
colinmarc opened a new pull request, #16750:
URL: https://github.com/apache/datafusion/pull/16750
Currently, only instances of `TableProvider` are considered by
`LogicalExtensionCodec`, and are automatically wrapped in a
`DefaultTableSource` when deserializing. That doesn't work with custom
13 matches
Mail list logo
