Skip to content

Conversation

@epgif
Copy link
Collaborator

@epgif epgif commented Jun 20, 2025

This is a backport of

  • feat: add SchemaProvider::table_type(table_name: &str)

InformationSchemaConfig::make_tables only needs the TableType not the
whole TableProvider, and the former may require an expensive catalog
operation to construct and the latter may not.

This allows avoiding SELECT * FROM information_schema.tables having to
make 1 of those potentially expensive operations per table.

  • test: new InformationSchemaConfig::make_tables behavior

  • Move tests to same file to fix CI


Co-authored-by: Andrew Lamb [email protected]

* feat:  add SchemaProvider::table_type(table_name: &str)

InformationSchemaConfig::make_tables only needs the TableType not the
whole TableProvider, and the former may require an expensive catalog
operation to construct and the latter may not.

This allows avoiding `SELECT * FROM information_schema.tables` having to
make 1 of those potentially expensive operations per table.

* test:  new InformationSchemaConfig::make_tables behavior

* Move tests to same file to fix CI

---------

Co-authored-by: Andrew Lamb <[email protected]>
@epgif epgif requested a review from alamb June 23, 2025 14:51
@alamb
Copy link
Collaborator

alamb commented Jun 23, 2025

FYI @crepererum who is in the process of upgrading DataFusion

I think we need to target this PR into a branch other than main (I think to the one in #64) so it is picked up by iinfluxdb_iox

@epgif
Copy link
Collaborator Author

epgif commented Jun 23, 2025

FYI @crepererum who is in the process of upgrading DataFusion

I think we need to target this PR into a branch other than main (I think to the one in #64) so it is picked up by iinfluxdb_iox

Sorry, I'm not familiar with the process. I can target another branch, or someone else can cherry-pick (it's clean) if that's easier.

@alamb
Copy link
Collaborator

alamb commented Jun 23, 2025

No worries -- let's wait for @crepererum to respond tomorrow -- we have another inflight upgrade of DataFusion here

@crepererum
Copy link

Given how slow the DF CI is, I'm probably gonna need another day or so.

@epgif
Copy link
Collaborator Author

epgif commented Jun 25, 2025

Given how slow the DF CI is, I'm probably gonna need another day or so.

OK. I'm ready when you are. Do I need to target a different branch?

Thanks!

@epgif
Copy link
Collaborator Author

epgif commented Jun 26, 2025

@alamb @crepererum any update? Thanks!

@crepererum
Copy link

We could include this in our fork. That would require:

  1. create a "take 3" PR with a new branch that is identical to Patched DF 46.0.1 (take 2) #66
  2. cherry-pick the changes from this PR here on top of that
  3. update IOx to point to the new PR from step 1 (incl. the change from step 2)

@epgif
Copy link
Collaborator Author

epgif commented Jul 2, 2025

We could include this in our fork. That would require:

  1. create a "take 3" PR with a new branch that is identical to Patched DF 46.0.1 (take 2) #66

Hi @crepererum,

I don't know what you mean there. That PR is a draft; where is it going? Who would create this "take 3"?

I don't mind rolling up my sleeves here, but I need a little more guidance.

Thanks!

@crepererum
Copy link

We could include this in our fork. That would require:

  1. create a "take 3" PR with a new branch that is identical to Patched DF 46.0.1 (take 2) #66

Hi @crepererum,

I don't know what you mean there. That PR is a draft; where is it going? Who would create this "take 3"?

I don't mind rolling up my sleeves here, but I need a little more guidance.

Thanks!

If you can wait a few days more, I'll try to do that myself after the DF 47 upgrade went through. I think having two people working on DF version changes / pins is gonna be a mess.

@epgif
Copy link
Collaborator Author

epgif commented Jul 3, 2025

@crepererum I agree having 2 working on that would be a mess. I'll wait.

Thanks!

@crepererum
Copy link

This is included in our pinned DF version as of #68.

@crepererum crepererum closed this Jul 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants