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

Support TypeORM VirtualColumn #67

Closed
YeomansIII opened this issue Dec 13, 2022 · 11 comments
Closed

Support TypeORM VirtualColumn #67

YeomansIII opened this issue Dec 13, 2022 · 11 comments
Labels
enhancement New feature or request

Comments

@YeomansIII
Copy link

YeomansIII commented Dec 13, 2022

Is your feature request related to a problem? Please describe.
A new @VirtualColumn decorator was recently added to TypeORM as a read-only virtual column that is generated with each find. It supports WHERE and ORDER BY filtering and sorting, however, nestjs-query does not seem to be generating the proper query code (straight SQL instead of TypeORM repository query)?

I end up with the following GraphQL error:

"errors": [
    {
      "message": "column EventEntity.is_active does not exist",
. . .

I am able to successfully query the Field of the VirtualColumn, but as soon as I try to filter or sort, it says the column doesn't exist (it really doesn't, but the repository should handle that).

TypeORM PR for VirtualColumn feature:
typeorm/typeorm#9339

Have you read the Contributing Guidelines?

Yes

Describe the solution you'd like
Filtering and Sorting on a Virtual Column should work.

Describe alternatives you've considered
Getter/generated field, which does not allow filtering or sorting.

@YeomansIII YeomansIII added the enhancement New feature or request label Dec 13, 2022
@TriPSs
Copy link
Owner

TriPSs commented Dec 13, 2022

Interesting, could it be because TypORM version is not the same? Do you use yarn by any change? If so could you try to add the following:

  "resolutions": {
    "typeorm": "0.3.11"
   }

Then do yarn again to reinstall deps and see if it works.

This is actually a quite awesome feature!

@YeomansIII
Copy link
Author

YeomansIII commented Dec 13, 2022

Agreed, it's an awesome feature and also, I appreciate you taking over as maintainer of this project.

I use npm, but did something similar to pin the typeorm version

"overrides": {
    "@ptc-org/nestjs-query-graphql": {
      "typeorm": "0.3.11"
    }
}

But no luck, still getting the same error on filter and sort.

I edited the main post with this, just FYI:

I am able to successfully query the Field of the VirtualColumn, but as soon as I try to filter or sort, it says the column doesn't exist (it really doesn't, but the repository should handle that).

So selecting the VirtualColumn field does return the appropriate calculated value, it just doesn't work on filter and sort.

@TriPSs
Copy link
Owner

TriPSs commented Dec 14, 2022

Okay thanks for the explanation, will try to check this out somewhere in the coming days.

@TriPSs
Copy link
Owner

TriPSs commented Dec 22, 2022

After some digging it's because the query services tries to filter / order on it as a normal database field, like tabel.field but since it's not a existing field it does not work. When querying it it does work since TypeORM selects all fields as table.field as table_field.

We could make a simple fix by checking if the column we are filtering/ordering on has the property isVirtualProperty and if true add the order/filter with the _ instead of a ..

In the case of sorting it would look like:

  public applySorting<T extends Sortable<Entity>>(qb: T, sorts?: SortField<Entity>[], alias?: string): T {
    if (!sorts) {
      return qb
    }

    return sorts.reduce((prevQb, { field, direction, nulls }) => {
      const columnMetadata = this.repo.metadata.columns.find((col) => col.propertyName === field)

      let col = alias ? `${alias}.${field as string}` : `${field as string}`
      // If the column is virtual we need to use the actual selected field as it does not exist in the database
      if (columnMetadata?.isVirtualProperty) {
        col = `"${alias ? `${alias}_${field as string}` : `${field as string}`}"`
      }

      return prevQb.addOrderBy(col, direction, nulls)
    }, qb)
  }

@jbeck018
Copy link

jbeck018 commented Jun 8, 2023

Any update on this? Just bumped versions in Typeorm and would love to take advantage of nestjs-query functionality on the VirtualColumns.

@MrSquaare
Copy link

After some digging it's because the query services tries to filter / order on it as a normal database field, like tabel.field but since it's not a existing field it does not work. When querying it it does work since TypeORM selects all fields as table.field as table_field.

We could make a simple fix by checking if the column we are filtering/ordering on has the property isVirtualProperty and if true add the order/filter with the _ instead of a ..

In the case of sorting it would look like:

  public applySorting<T extends Sortable<Entity>>(qb: T, sorts?: SortField<Entity>[], alias?: string): T {
    if (!sorts) {
      return qb
    }

    return sorts.reduce((prevQb, { field, direction, nulls }) => {
      const columnMetadata = this.repo.metadata.columns.find((col) => col.propertyName === field)

      let col = alias ? `${alias}.${field as string}` : `${field as string}`
      // If the column is virtual we need to use the actual selected field as it does not exist in the database
      if (columnMetadata?.isVirtualProperty) {
        col = `"${alias ? `${alias}_${field as string}` : `${field as string}`}"`
      }

      return prevQb.addOrderBy(col, direction, nulls)
    }, qb)
  }

Just patched via patch-package, this seems to work great! Is there a reason why this isn't already supported?

@YeomansIII
Copy link
Author

@MrSquaare, would you be able to make a PR?

@MrSquaare
Copy link

@MrSquaare, would you be able to make a PR?

@YeomansIII Unfortunately, after further testing, it doesn't seem to work with some databases (such as Postgres).
Also, I couldn't get it to work with the WHERE clause. Did you see somewhere that VirtualColumn should support the WHERE clause? (I've seen a few people mention that HAVING works with aliases, but I haven't tested it yet).
It seems that the only reliable way to get VirtualColumn to work with ORDER BY, WHERE, ... is via a sub query, which can have performance disadvantages. This remains a viable option.

@TriPSs
Copy link
Owner

TriPSs commented Sep 18, 2023

Just patched via patch-package, this seems to work great! Is there a reason why this isn't already supported?

Partly because what you are saying above, this solution is not correctly tested at all so can have unexpected behaviour with other drivers, also there was no response on the ticket for a while 😅

@DevBrasil
Copy link

DevBrasil commented Oct 13, 2023

Any news about thaat issue ?

Having the same issue when using where contidional, that code fix the sort.

ERROR [ExceptionsHandler] Unknown column 'Quote.daysUntilDue' in 'where clause'
QueryFailedError: Unknown column 'Quote.daysUntilDue' in 'where clause'

@YeomansIII
Copy link
Author

Thanks @TriPSs !

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

No branches or pull requests

5 participants