fix: Making introspection more efficient #36
Conversation
nidhi-akkio
changed the title
nidhi-akkio
changed the title
| if schema_name.lower() != "information_schema" | ||
| ] | ||
|
|
||
| if self.config.get("schema"): |
There was a problem hiding this comment.
@edgarrmondragon I'm curious what you think about this. I know the schema setting was documented to be the schema used to connect to the database but not necessarily meant to be a filter for only data in that database. It seems like this PR would implement it to only consider data in the configured schema.
There was a problem hiding this comment.
Hi, is there an update on this? We would really like to merge this in asap.
There was a problem hiding this comment.
@edgarrmondragon ping on the previous message in case you have thoughts but we mentioned it in office hours and it looks like this is implementation specific and not related to the SDK as I originally thought.
@nidhi-akkio sorry about the delay. A few more comments but I think we can get this merged soon.
This is a of breaking change for some users who assigned a schema to use for connecting but are expecting the status quo where its not using it for filtering. If they upgrade they'll silently stop syncing tables outside this schema. We can either:
- accept the breaking change and document it well in release notes and update the tap setting description which will also get pulled into the hub docs.
- we can add this as a new config like
schema_filter. Avoiding breaking changes but likely making the difference between the 2 options confusing for users, althoughschemain its current state is already confusing to me 😅 .
@nidhi-akkio @edgarrmondragon what do you think?
There was a problem hiding this comment.
I think, like you say @pnadolny13, that the current usage and description of the schema setting, i.e. The initial schema for the Snowflake session. is rather confusing.
My preference would be to not introduce a breaking change, add a new setting schema_filter and think about what to do with the existing schema setting.
cc @nidhi-akkio
Previously, we were unnecessarily making calls to get available schemas and iterating through them to find relevant tables. However, now, if a user passes in a schema in the configs, we only introspect on that one schema