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

Make view.columns() functionality optional when creating a view #4827

Closed
bakasmarius opened this issue Nov 15, 2021 · 6 comments · Fixed by #4829
Closed

Make view.columns() functionality optional when creating a view #4827

bakasmarius opened this issue Nov 15, 2021 · 6 comments · Fixed by #4829

Comments

@bakasmarius
Copy link
Contributor

Environment

Knex version: 0.95.14
Database + version: PostgreSQL 13.4
OS: Windows10

Feature discussion / request

As a PostgreSQL user, I would like to be able to create such queries for views:
create view "users_view" as select "users".*, "user_roles"."role_code" from "users" inner join "user_roles" on "users"."user_id" = "user_roles"."user_id"

I could achieve it this with this code:

knex.schema.createView('users_view', (view) => {
    view.as(knex('users').join('user_roles', 'users.user_id', 'user_roles.user_id').select('users.*', 'user_roles.role_code'))
  })

But currently this doesn't work because I didn't use view.columns() functionality, so knex throws columns is not iterable error.

I edited the knex\lib\schema\viewcompiler.js file, changed part of createQuery code like this and everything worked as I wanted:

    const formatColumnsArr = [];
    for (const c of columns || []) {
      formatColumnsArr.push(
        columnize_(c, this.viewBuilder, this.client, this.bindingsHolder)
      );
    }
    const formatColumns = formatColumnsArr.length ? ' (' + formatColumnsArr.join(', ') + ')' : ''
    let sql = createStatement + this.viewName() + formatColumns;

I would be happy to provide a PR if this is something you would like.

@OlivierCavadenti
Copy link
Collaborator

OlivierCavadenti commented Nov 15, 2021

I would be happy to provide a PR if this is something you would like.

Please do, views without columns are standard (it will be optional in the code like you write).

@thebrownfox
Copy link
Contributor

@OlivierCavadenti when is this going to be released?

@kibertoad
Copy link
Collaborator

@thefoxie Would RC release work for you?

@thebrownfox
Copy link
Contributor

@kibertoad yeah sure :)

@kibertoad
Copy link
Collaborator

I'll publish 1.0.0-next1 tonight then.

@kibertoad
Copy link
Collaborator

Released in 1.0.1.

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

Successfully merging a pull request may close this issue.

4 participants