#844186: Clarify merge queries introduced two regressions to the base MergeQuery implementation:

  • The merge query key is used (needlessly) in the fields of the UPDATE query; this is a problem because some databases (like SQL Server) doesn't allow update of identity fields
  • In a corner case, an exception message can be echoed directly to the browser
CommentFileSizeAuthor
#3 943042-merge-query-regressions.patch2.59 KBdamien tournoud
#2 943042-merge-query-regressions.patch2.59 KBdamien tournoud
#1 943042-merge-query-regressions.patch898 bytesdamien tournoud

Comments

damien tournoud’s picture

damien tournoud commented
Status: Active » Needs review
StatusFileSize
new 943042-merge-query-regressions.patch898 bytes
damien tournoud’s picture

damien tournoud commented
StatusFileSize
new 943042-merge-query-regressions.patch2.59 KB

Clarify the documentation via @chx.

damien tournoud’s picture

damien tournoud commented
StatusFileSize
new 943042-merge-query-regressions.patch2.59 KB

Even more clarifications via @chx.

chx’s picture

chx commented
Status: Needs review » Reviewed & tested by the community

Yes. This makes sense. Even I got confused by the old code. Good that Damien catched this one.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 943042-merge-query-regressions.patch, failed testing.

Crell’s picture

Crell commented
Status: Needs work » Needs review

#3: 943042-merge-query-regressions.patch queued for re-testing.

chx’s picture

chx commented
Status: Needs review » Reviewed & tested by the community

bot fluke.

dries’s picture

dries commented
Status: Reviewed & tested by the community » Fixed

Committed to CVS HEAD. Thanks.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.