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

JSON Keys break RawResult to EntityTransformer #6833

Closed
Maddimax opened this issue Oct 2, 2020 · 5 comments
Closed

JSON Keys break RawResult to EntityTransformer #6833

Maddimax opened this issue Oct 2, 2020 · 5 comments

Comments

@Maddimax
Copy link
Contributor

Maddimax commented Oct 2, 2020

( I've provided a PR for this issue here: #6834 )
For the following entity:

class MyId {
   first: number;
   second: number;
}

@Entity({ name: 'tests' })
export class Test {
  @PrimaryColumn('jsonb')
  id: MyId;
}

With the following data:

id
{"first": 1, "second": 2}
{"first": 1, "second": 3}

The following code will only return one entity:

getRepository('tests').createQueryBuilder()
        .select()
        .where('id IN (:...ids)', {
          ids: [
            { first: 1, second: 2 },
            { first: 1, second: 3 },
          ],
        })
        .getMany();

This is due to:

Adding

if (typeof keyValue === 'object') {
  return JSON.stringify(keyValue)
}

solves the issue

Maddimax pushed a commit to AshampooSystems/typeorm that referenced this issue Oct 2, 2020
Maddimax pushed a commit to AshampooSystems/typeorm that referenced this issue Oct 2, 2020
@imnotjames
Copy link
Contributor

I think we can improve this solution... Somehow. Not sure how exactly... JSONB and JSON aren't the same - least of which is the on-disk serialization format.

Duplicate keys, unsorted keys, etc, will cause differences between the two as well, which this will not handle.

@Maddimax
Copy link
Contributor Author

Maddimax commented Oct 3, 2020

Could you elaborate what you mean by Duplicate / unsorted keys and how this would not be handled? I'm totally open to improve the solution.

@Maddimax
Copy link
Contributor Author

Maddimax commented Oct 3, 2020

I've added tests for this issue in #6833 as well

@imnotjames
Copy link
Contributor

Sure - if you flip the keys around it should fail to find them but technically it's the same thing. Both to JSONB and as parsed JSON.

const payload = { second: 2, first: 1 }
manager.save({ id: payload })
manager.find ({ where: { id: payload } })

Because JSONB won't store it as you wrote it - it should sort the keys - this should fail if it were a test.

@Maddimax
Copy link
Contributor Author

Maddimax commented Oct 3, 2020

Understood. I've added a test for this and it seems as if the order does not matter: https://github.com/typeorm/typeorm/pull/6834/files#diff-09760a400d18a7e291718cb6c8f624b9R69

Maddimax pushed a commit to AshampooSystems/typeorm that referenced this issue Oct 5, 2020
When using PrimaryColumn with a 'jsonb' type column
( on a postgres database ) the RawSqlResultsToEntityTransformer
would simply convert the key to '[object Object]' instead of a
unique value based on the key value. It would then wrongly consider each
entity as having the same key.

To allow for complex keys the transformer now uses JSON.stringify()
to convert the keys value if they are of type obejct to a unique
string that can be compared
to other entities.

fixes typeorm#6833
zaro pushed a commit to zaro/typeorm that referenced this issue Jan 12, 2021
* fix: allow for complex jsonb primary key columns

When using PrimaryColumn with a 'jsonb' type column
( on a postgres database ) the RawSqlResultsToEntityTransformer
would simply convert the key to '[object Object]' instead of a
unique value based on the key value. It would then wrongly consider each
entity as having the same key.

To allow for complex keys the transformer now uses JSON.stringify()
to convert the keys value if they are of type obejct to a unique
string that can be compared
to other entities.

fixes typeorm#6833

* test: add tests for jsonb primarykey columns

These tests verify that Entities with jsonb primary columns are correctly
converted to Entites
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

2 participants