Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Columns from views are not filtered out #48

Open
Oreilles opened this issue May 16, 2021 · 8 comments
Open

Columns from views are not filtered out #48

Oreilles opened this issue May 16, 2021 · 8 comments

Comments

@Oreilles
Copy link
Contributor

Oreilles commented May 16, 2021

tableInfo() filters the views out:

.andWhere({ table_type: 'BASE TABLE' })

But columns() doesn't, therefore returning columns with tables we can't get info on.

Maybe it would be preferable to add a is_view attribute to the Table type and not filter them out silently.

@rijkvanzanten
Copy link
Collaborator

Should we include views under regular columns, or have a dedicated .views() method? 🤔

@Oreilles
Copy link
Contributor Author

Should we include views under regular columns, or have a dedicated .views() method? 🤔

Then you'd have to set a filter for columns in order not to encounter the problem I raised.
It's probably easier to just add a view attribute 🙂

@rijkvanzanten
Copy link
Collaborator

True, we'd have to make a change to fix this issue regardless, just trying to figure out what the ideal long-term solution is 🙂

We either treat "views" as regular tables/columns, but with a flag is_view, or we treat them as a separate entity and return them under .views.

I guess it's most common to see/interact with them in the same bucket as regular tables, so lets go with that is_view flag 👍🏻

@rijkvanzanten
Copy link
Collaborator

Ref directus/directus#5815

@rijkvanzanten
Copy link
Collaborator

@Oreilles For the time being, I'll filter out the views, so it matches the tableInfo method 👍🏻 Then we'll revisit and add proper views support both in here and knex itself (ref knex/knex#1626)

@azrikahar
Copy link

and knex itself (ref knex/knex#1626)

Just as an update: this was recently resolved by knex/knex#4748

@rijkvanzanten
Copy link
Collaborator

@azrikahar Knex itself implemented support for views, but that doesn't mean the knex-schema-inspector correctly handles views when querying columnInfo 🙂

@azrikahar
Copy link

Yea I think what I mean is that the Knex half is there, so you guys can look into the knex-schema-inspector half (since it seems like you're waiting for this), which is why I only quoted the last few letters of your sentence involving Knex only 😅 Apologies if that wasn't clear enough (or if I even misunderstand this latest message 😵)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants