Launchpad itself

Merge ~twom/launchpad:stormy-questions-about-tests into launchpad:master

  1. Git
  2. lp:~twom/launchpad
  3. stormy-questions-about-tests
  4. Merge into master
Proposed by Tom Wardill
Status: Merged
Approved by: Tom Wardill
Approved revision: 3030b4e56ee47e009f49973c08045f9af64e8d55
Merge reported by: Otto Co-Pilot
Merged at revision: not available
Proposed branch: ~twom/launchpad:stormy-questions-about-tests
Merge into: launchpad:master
Diff against target: 14 lines (+2/-1)
1 file modified
lib/lp/answers/model/question.py (+2/-1)
Reviewer Review Type Date Requested Status
Colin Watson (community) Approve
Ioana Lasc (community) Approve
Review via email: mp+394553@code.launchpad.net

Commit message

Different ReferenceSet implementation

To post a comment you must log in.
Revision history for this message
Ioana Lasc (ilasc) :
review: Approve
Revision history for this message
Ioana Lasc (ilasc) wrote :

Not sure though if that order_by clause needs to be:
     order_by='QuestionReopening.datecreated'

Revision history for this message
Colin Watson (cjwatson) wrote :

The problem here can be seen by running lib/lp/answers/doc/workflow.txt with LP_DEBUG_SQL=1 and comparing the query for QuestionReopening. Before:

  SELECT QuestionReopening.answerer, QuestionReopening.date_solved, QuestionReopening.datecreated, QuestionReopening.id, QuestionReopening.priorstate, QuestionReopening.question, QuestionReopening.reopener FROM QuestionReopening WHERE QuestionReopening.question = 22 ORDER BY QuestionReopening.datecreated LIMIT 1

After:

  SELECT QuestionReopening.answerer, QuestionReopening.date_solved, QuestionReopening.datecreated, QuestionReopening.id, QuestionReopening.priorstate, QuestionReopening.question, QuestionReopening.reopener FROM Question, QuestionReopening WHERE QuestionReopening.question = 22 ORDER BY Question.datecreated LIMIT 1

So you should revert the change in this MP and take Ioana's suggestion instead, because the problem is that ReferenceSet resolves table-free column names relative to the class of the local key, while SQLMultipleJoin does so relative to the class of the remote key. The right fix is to declare a fully-qualified order_by so that there's no ambiguity.

review: Needs Fixing
Revision history for this message
Colin Watson (cjwatson) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/lib/lp/answers/model/question.py b/lib/lp/answers/model/question.py
2index 8015c6f..9c51e88 100644
3--- a/lib/lp/answers/model/question.py
4+++ b/lib/lp/answers/model/question.py
5@@ -235,7 +235,8 @@ class Question(SQLBase, BugLinkTargetMixin):
6 _messages = ReferenceSet(
7 'id', 'QuestionMessage.question_id', order_by='QuestionMessage.id')
8 reopenings = ReferenceSet(
9- 'id', 'QuestionReopening.question_id', order_by='datecreated')
10+ 'id', 'QuestionReopening.question_id',
11+ order_by='QuestionReopening.datecreated')
12
13 @property
14 def messages(self):

Subscribers

People subscribed via source and target branches

to status/vote changes: