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

Repository update does not respect readonly flag on CreateDateColumn or UpdateDateColumn #3365

Closed
jdforsythe opened this issue Jan 4, 2019 · 4 comments

Comments

@jdforsythe
Copy link
Contributor

jdforsythe commented Jan 4, 2019

Issue type:

[ ] question
[x] bug report
[ ] feature request
[ ] documentation issue

Database system/driver:

[ ] cordova
[ ] mongodb
[ ] mssql
[ ] mysql / mariadb
[ ] oracle
[x] postgres
[ ] sqlite
[ ] sqljs
[ ] react-native
[ ] expo

TypeORM version:

[ ] latest
[ ] @next
[x] 0.2.11 (or put your version here)

Related to:

#3271

Steps to reproduce or a small repository showing the problem:

I recently switched our update method to use repository.update() instead of repository.save() to prevent the needless extra SELECT before UPDATE.

pizza-route.ts

@JsonController('/pizzas')
export class PizzaController {
  private readonly pizzaRepository: Repository<Pizza>;

  constructor() {
    const connection: Connection = getConnectionManager().get();
    this.pizzaRepository = connection.getRepository<Pizza>(Pizza);
  }

  @Put('/:id')
  async put(@Param('id') id: number, @EntityFromBody({ required: true }) pizza: Pizza) {
    /* before */
    // return this.pizzaRepository.save(pizza);

    /* after */
    return this.pizzaRepository.update(id, pizza).then((res: UpdateResult) => {
      console.log(res); // always empty { generatedMaps: [], raw: [] }
      return pizza; // can't return updated DB object
   });
  }
}

I was surprised by two things:

  1. It started throwing this same error from @CreateDateColumn and @UpdateDateColumn should use value if provided. #3271:

multiple assignments to same column \"updatedAt\".

I thought adding the readonly flag:

@UpdateDateColumn({ readonly: true }) updatedAt: Date;

would fix the problem, but the repository.update() method appears to ignore the readonly flag for the UpdateDateColumn() and CreateDateColumn().

  1. The repository.update() method does not give access to the updated DB value like repository.save() does. I expected to see the query like:
UPDATE "pizza" SET "name" = $2, "updatedAt" = CURRENT_TIMESTAMP WHERE "id" = $1 RETURNING *

but the query was

UPDATE "pizza" SET "id" = $2, "name" = $3, "updatedAt" = $4, "updatedAt" = CURRENT_TIMESTAMP WHERE "id" = $1

with no RETURNING *. The UpdateResult doesn't contain the updated record from the DB and in my testing it's always completely empty:

{
  generatedMaps: [],
  raw: []
}

so even if the error wasn't occurring, I wouldn't be able to send a return object with the new updatedAt value or any other DB-calculated values back to the client without performing a subsequent SELECT

@pleerock
Copy link
Member

pleerock commented Jan 7, 2019

by using update instead of save you disable lot of orm functionality, not just disabling additional select.

when you use @UpdateDateColumn typeorm suppose to manage its date on its own. If you don't need it - just create a regular column and update its date when you need it.

@pleerock
Copy link
Member

pleerock commented Jan 7, 2019

I expected to see the query like: UPDATE "pizza" SET "name" = $2, "updatedAt" = CURRENT_TIMESTAMP WHERE "id" = $1 RETURNING *

some people can expect returning, some not. But update doesn't tell anything about returning - it just tells that it updates. That's why you don't have anything in returning result.

@jesslombera
Copy link

Even when I save the entity, it still does not update the @UpdateDateColumn

@pleerock
Copy link
Member

@lomberas it can't be so, make sure you do everything right or file a new issue.

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

4 participants