Zorba

Merge lp:~zorba-coders/zorba/updrevalidate into lp:zorba

  1. updrevalidate
  2. Merge into trunk
Proposed by Markos Zaharioudakis
Status: Merged
Approved by: Federico Cavalieri
Approved revision: 10521
Merged at revision: 10516
Proposed branch: lp:~zorba-coders/zorba/updrevalidate
Merge into: lp:zorba
Diff against target: 389 lines (+78/-73)
4 files modified
src/runtime/schema/schema_impl.cpp (+5/-6)
src/store/naive/pul_primitives.cpp (+29/-17)
src/store/naive/pul_primitives.h (+6/-4)
src/store/naive/simple_pul.cpp (+38/-46)
To merge this branch: bzr merge lp:~zorba-coders/zorba/updrevalidate
Reviewer Review Type Date Requested Status
Federico Cavalieri Approve
Markos Zaharioudakis Approve
Review via email: mp+79331@code.launchpad.net

Commit message

Cosmetic changes + changing some ulong types to csize

To post a comment you must log in.
Revision history for this message
Markos Zaharioudakis (markos-za) wrote :

Federico, I forgot to submit some cosmetic changes to this branch before pushing the "approve" button. So, now I have created this new merge proposal. Can you please review? Thanks!.

Revision history for this message
Markos Zaharioudakis (markos-za) :
review: Approve
Revision history for this message
Federico Cavalieri (fcavalieri) wrote :

Sure, but to me it looks like no change has been made at all (Preview Diff contains only the word Empty) and no "In a few minutes the diff will be updated". Maybe I just have to wait...

Revision history for this message
Federico Cavalieri (fcavalieri) wrote :

> Sure, but to me it looks like no change has been made at all (Preview Diff
> contains only the word Empty) and no "In a few minutes the diff will be
> updated". Maybe I just have to wait...

Yes I just had to wait

Revision history for this message
Federico Cavalieri (fcavalieri) wrote :

Why variable numUpdates in PULImpl::mergeUpdateList is declared, assigned but never read?

Revision history for this message
Markos Zaharioudakis (markos-za) wrote :

> Why variable numUpdates in PULImpl::mergeUpdateList is declared, assigned but
> never read?

Good question :) The easy answer is to just remove it. But I think it can actually be used to replace the myList.size() in the 2 for loops at lines 1289 and 1308. Even though new entries can be added in myList during each iteration of the outer for-loop, I think that during those 2 inner for-loops it is ok to search myList only up to numUpdates. Do you agree?

Revision history for this message
Federico Cavalieri (fcavalieri) wrote :

> > Why variable numUpdates in PULImpl::mergeUpdateList is declared, assigned
> but
> > never read?
>
> Good question :) The easy answer is to just remove it. But I think it can
> actually be used to replace the myList.size() in the 2 for loops at lines 1289
> and 1308. Even though new entries can be added in myList during each iteration
> of the outer for-loop, I think that during those 2 inner for-loops it is ok to
> search myList only up to numUpdates. Do you agree?

Yes I agree, the ops in the other PUL cannot conflict with each other. A conflict must be
between one of the operation of the first pul and one of the second pul.

Revision history for this message
Zorba Build Bot (zorba-buildbot) wrote :

There are additional revisions which have not been approved in review. Please seek review and approval of these new revisions.

Revision history for this message
Federico Cavalieri (fcavalieri) :
review: Approve
Revision history for this message
Zorba Build Bot (zorba-buildbot) wrote :

Validation queue starting for merge proposal.
Log at: http://zorbatest.lambda.nu:8080/remotequeue/updrevalidate-2011-10-14T08-10-00.964Z/log.html

Revision history for this message
Zorba Build Bot (zorba-buildbot) wrote :

Validation queue job updrevalidate-2011-10-14T08-10-00.964Z is finished. The final status was:

All tests succeeded!

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/runtime/schema/schema_impl.cpp'
2--- src/runtime/schema/schema_impl.cpp 2011-10-12 21:30:46 +0000
3+++ src/runtime/schema/schema_impl.cpp 2011-10-14 08:04:27 +0000
4@@ -86,23 +86,22 @@
5 {
6 store::Item_t node;
7
8- PlanIteratorState *state;
9- std::auto_ptr<store::PUL> pul;
10+ PlanIteratorState* state;
11+ store::PUL_t pul;
12
13 DEFAULT_STACK_INIT(PlanIteratorState, state, planState);
14
15 if (consumeNext(node, theChild.getp(), planState))
16 {
17- pul.reset(GENV_ITEMFACTORY->createPendingUpdateList());
18+ pul = GENV_ITEMFACTORY->createPendingUpdateList();
19
20 pul->addRevalidate(&loc,node);
21
22- result = pul.release();
23+ result.transfer(pul);
24 STACK_PUSH(true, state);
25 }
26
27- STACK_END (state);
28-
29+ STACK_END(state);
30 }
31
32
33
34=== modified file 'src/store/naive/pul_primitives.cpp'
35--- src/store/naive/pul_primitives.cpp 2011-10-12 21:41:55 +0000
36+++ src/store/naive/pul_primitives.cpp 2011-10-14 08:04:27 +0000
37@@ -466,21 +466,24 @@
38 void UpdSetElementType::apply()
39 {
40 ElementNode* target = ELEM_NODE(theTarget);
41-
42- theOldTypeName=target->getType();
43- theOldHaveTypedValue=target->haveTypedValue();
44+ TextNode* textChild;
45+
46+ theOldTypeName = target->getType();
47+ theOldHaveTypedValue = target->haveTypedValue();
48+ theOldHaveTypedTypedValue = target->haveTypedTypedValue(textChild);
49+
50 if (theOldHaveTypedValue)
51- theOldHaveEmptyTypedValue=target->haveEmptyTypedValue();
52- theOldIsInSubstitutionGroup=target->isInSubstitutionGroup();
53+ theOldHaveEmptyTypedValue = target->haveEmptyTypedValue();
54+
55+ theOldIsInSubstitutionGroup = target->isInSubstitutionGroup();
56
57 target->setType(theTypeName);
58
59- TextNode* textChild;
60- theOldHaveTypedTypedValue=target->haveTypedTypedValue(textChild);
61 if (theOldHaveTypedTypedValue)
62 {
63- theOldHaveListTypedValue= textChild->haveListValue();
64- theOldTypedValue=textChild->getValue();
65+ theOldHaveListTypedValue = textChild->haveListValue();
66+ theOldTypedValue = textChild->getValue();
67+
68 zstring textValue;
69 textChild->getStringValue2(textValue);
70
71@@ -504,6 +507,7 @@
72 textChild = target->getUniqueTextChild();
73
74 textChild->setTypedValue(theTypedValue);
75+
76 if (theHaveListTypedValue)
77 textChild->setHaveListValue();
78 else
79@@ -520,9 +524,10 @@
80 else
81 target->resetInSubstGroup();
82
83- theIsApplied=true;
84+ theIsApplied = true;
85 }
86
87+
88 void UpdSetElementType::undo()
89 {
90 if (theIsApplied)
91@@ -551,6 +556,7 @@
92 TextNode* textChild = target->getUniqueTextChild();
93
94 textChild->setTypedValue(theOldTypedValue);
95+
96 if (theOldHaveListTypedValue)
97 textChild->setHaveListValue();
98 else
99@@ -567,7 +573,7 @@
100 else
101 target->resetInSubstGroup();
102
103- theIsApplied=false;
104+ theIsApplied = false;
105 }
106 }
107
108@@ -629,9 +635,9 @@
109 {
110 AttributeNode* target = ATTR_NODE(theTarget);
111
112- theOldTypeName=target->getType();
113+ theOldTypeName = target->getType();
114 theOldTypedValue.transfer(target->theTypedValue);
115- theOldHaveListValue=target->haveListValue();
116+ theOldHaveListValue = target->haveListValue();
117
118 target->setType(theTypeName);
119 target->theTypedValue.transfer(theTypedValue);
120@@ -641,9 +647,10 @@
121 else
122 target->resetHaveListValue();
123
124- theIsApplied=true;
125+ theIsApplied = true;
126 }
127
128+
129 void UpdSetAttributeType::undo()
130 {
131 if (theIsApplied)
132@@ -657,10 +664,11 @@
133 else
134 target->resetHaveListValue();
135
136- theIsApplied=false;
137+ theIsApplied = false;
138 }
139 }
140
141+
142 /*******************************************************************************
143
144 ********************************************************************************/
145@@ -669,11 +677,15 @@
146 #ifndef ZORBA_NO_XMLSCHEMA
147 std::set<store::Item*> nodes;
148
149- theRevalidationPul=GET_STORE().getItemFactory()->createPendingUpdateList();
150+ theRevalidationPul = GET_STORE().getItemFactory()->createPendingUpdateList();
151+
152 nodes.insert(theTarget.getp());
153+
154 if (!thePul->theValidator)
155 return;
156- thePul->theValidator->validate(nodes,*theRevalidationPul.getp());
157+
158+ thePul->theValidator->validate(nodes, *theRevalidationPul.getp());
159+
160 try
161 {
162 theRevalidationPul->applyUpdates(false);
163
164=== modified file 'src/store/naive/pul_primitives.h'
165--- src/store/naive/pul_primitives.h 2011-10-12 20:59:49 +0000
166+++ src/store/naive/pul_primitives.h 2011-10-14 08:04:27 +0000
167@@ -808,7 +808,8 @@
168
169
170 /*******************************************************************************
171-
172+ This primitive is generated by the validate-in-place function (in module
173+ http://www.zorba-xquery.com/modules/schema)
174 ********************************************************************************/
175 class UpdRevalidate : public UpdatePrimitive
176 {
177@@ -818,13 +819,14 @@
178 protected:
179 store::PUL_t theRevalidationPul;
180
181+protected:
182 UpdRevalidate(PULImpl* pul, const QueryLoc* aLoc, store::Item_t& target)
183 :
184- UpdatePrimitive(pul, aLoc, target){}
185-
186+ UpdatePrimitive(pul, aLoc, target)
187+ {
188+ }
189
190 public:
191-
192 store::UpdateConsts::UpdPrimKind getKind() const
193 {
194 return store::UpdateConsts::UP_REVALIDATE;
195
196=== modified file 'src/store/naive/simple_pul.cpp'
197--- src/store/naive/simple_pul.cpp 2011-10-13 08:12:41 +0000
198+++ src/store/naive/simple_pul.cpp 2011-10-14 08:04:27 +0000
199@@ -798,6 +798,7 @@
200 theValidationList.push_back(upd);
201 }
202
203+
204 void PULImpl::addRevalidate(
205 const QueryLoc* aQueryLoc,
206 store::Item_t& target)
207@@ -810,8 +811,7 @@
208 bool found = pul->theNodeToUpdatesMap.get(n, updates);
209
210 UpdRevalidate* upd = GET_STORE().getPULFactory().
211- createUpdRevalidate(this, aQueryLoc, target);
212-
213+ createUpdRevalidate(this, aQueryLoc, target);
214
215 pul->theRevalidateList.push_back(upd);
216
217@@ -1133,6 +1133,7 @@
218 thisPul->theDeleteList,
219 otherPul->theDeleteList,
220 UP_LIST_DELETE);
221+
222 // Merge revalidation primitives
223 mergeUpdateList(thisPul,
224 thisPul->theRevalidateList,
225@@ -1185,22 +1186,21 @@
226 }
227
228 // Merge fn:put primitives
229- ulong numPuts = (ulong)thePutList.size();
230- ulong numOtherPuts = (ulong)otherp->thePutList.size();
231+ csize numPuts = thePutList.size();
232+ csize numOtherPuts = otherp->thePutList.size();
233
234- for (ulong i = 0; i < numOtherPuts; ++i)
235+ for (csize i = 0; i < numOtherPuts; ++i)
236 {
237 UpdPut* otherUpd = static_cast<UpdPut*>(otherp->thePutList[i]);
238
239- for (ulong j = 0; j < numPuts; ++j)
240+ for (csize j = 0; j < numPuts; ++j)
241 {
242 UpdPut* upd = static_cast<UpdPut*>(thePutList[j]);
243
244 if (upd->theTargetUri->equals(otherUpd->theTargetUri))
245 {
246- throw XQUERY_EXCEPTION(
247- err::XUDY0031, ERROR_PARAMS( upd->theTargetUri->getStringValue() )
248- );
249+ throw XQUERY_EXCEPTION(err::XUDY0031,
250+ ERROR_PARAMS(upd->theTargetUri->getStringValue()));
251 }
252 }
253
254@@ -1275,18 +1275,18 @@
255 std::vector<UpdatePrimitive*>& otherList,
256 UpdListKind listKind)
257 {
258- ulong numUpdates;
259- ulong numOtherUpdates;
260-
261- numUpdates = (ulong)myList.size();
262- numOtherUpdates = (ulong)otherList.size();
263-
264- for (ulong i = 0; i < numOtherUpdates; ++i)
265+ csize numUpdates;
266+ csize numOtherUpdates;
267+
268+ numUpdates = myList.size();
269+ numOtherUpdates = otherList.size();
270+
271+ for (csize i = 0; i < numOtherUpdates; ++i)
272 {
273 if (listKind == UP_LIST_CREATE_COLLECTION)
274 {
275 UpdCreateCollection* otherUpd = static_cast<UpdCreateCollection*>(otherList[i]);
276- for (ulong j = 0; j < myList.size(); ++j)
277+ for (csize j = 0; j < numUpdates; ++j)
278 {
279 if (myList[j]->getKind() == store::UpdateConsts::UP_CREATE_COLLECTION)
280 {
281@@ -1305,7 +1305,7 @@
282 else if (listKind == UP_LIST_CREATE_INDEX)
283 {
284 UpdCreateIndex* otherUpd = static_cast<UpdCreateIndex*>(otherList[i]);
285- for (ulong j = 0; j < myList.size(); ++j)
286+ for (csize j = 0; j < numUpdates; ++j)
287 {
288 if (myList[j]->getKind() == store::UpdateConsts::UP_CREATE_INDEX)
289 {
290@@ -1367,22 +1367,18 @@
291 for (ulong j = 0; j < numTargetUpdates; j++)
292 {
293 if (store::UpdateConsts::isRename((*targetUpdates)[j]->getKind()))
294- throw XQUERY_EXCEPTION(
295- err::XUDY0015,
296- ERROR_LOC( (*targetUpdates)[j]->theLoc )
297- );
298+ throw XQUERY_EXCEPTION(err::XUDY0015,
299+ ERROR_LOC((*targetUpdates)[j]->theLoc));
300 }
301 }
302 else if (store::UpdateConsts::isReplaceValue(updKind))
303 {
304- ulong numTargetUpdates = (ulong)targetUpdates->size();
305- for (ulong j = 0; j < numTargetUpdates; j++)
306+ csize numTargetUpdates = targetUpdates->size();
307+ for (csize j = 0; j < numTargetUpdates; j++)
308 {
309 if (store::UpdateConsts::isReplaceValue((*targetUpdates)[j]->getKind()))
310- throw XQUERY_EXCEPTION(
311- err::XUDY0017,
312- ERROR_LOC( (*targetUpdates)[j]->theLoc )
313- );
314+ throw XQUERY_EXCEPTION(err::XUDY0017,
315+ ERROR_LOC((*targetUpdates)[j]->theLoc));
316 }
317 }
318 break;
319@@ -1391,14 +1387,12 @@
320 {
321 if (store::UpdateConsts::isReplaceNode(updKind))
322 {
323- ulong numTargetUpdates = (ulong)targetUpdates->size();
324- for (ulong j = 0; j < numTargetUpdates; ++j)
325+ csize numTargetUpdates = (ulong)targetUpdates->size();
326+ for (csize j = 0; j < numTargetUpdates; ++j)
327 {
328 if (store::UpdateConsts::isReplaceNode((*targetUpdates)[j]->getKind()))
329- throw XQUERY_EXCEPTION(
330- err::XUDY0016,
331- ERROR_LOC( (*targetUpdates)[j]->theLoc )
332- );
333+ throw XQUERY_EXCEPTION(err::XUDY0016,
334+ ERROR_LOC((*targetUpdates)[j]->theLoc));
335 }
336 }
337 break;
338@@ -1407,14 +1401,12 @@
339 {
340 if (updKind == store::UpdateConsts::UP_REPLACE_CONTENT)
341 {
342- ulong numTargetUpdates = (ulong)targetUpdates->size();
343- for (ulong j = 0; j < numTargetUpdates; ++j)
344+ csize numTargetUpdates = targetUpdates->size();
345+ for (csize j = 0; j < numTargetUpdates; ++j)
346 {
347 if ((*targetUpdates)[j]->getKind() == store::UpdateConsts::UP_REPLACE_CONTENT)
348- throw XQUERY_EXCEPTION(
349- err::XUDY0017,
350- ERROR_LOC( (*targetUpdates)[j]->theLoc )
351- );
352+ throw XQUERY_EXCEPTION(err::XUDY0017,
353+ ERROR_LOC((*targetUpdates)[j]->theLoc));
354 }
355 }
356 break;
357@@ -1423,8 +1415,8 @@
358 {
359 if (updKind == store::UpdateConsts::UP_DELETE)
360 {
361- ulong numTargetUpdates = (ulong)targetUpdates->size();
362- ulong j;
363+ csize numTargetUpdates = targetUpdates->size();
364+ csize j;
365 for (j = 0; j < numTargetUpdates; ++j)
366 {
367 if ((*targetUpdates)[j]->getKind() == store::UpdateConsts::UP_DELETE)
368@@ -1551,8 +1543,8 @@
369 UpdCollection* upd = static_cast<UpdCollection*>
370 (pul->theInsertIntoCollectionList[i]);
371
372- ulong numDocs = upd->numNodes();
373- for (ulong j = 0; j < numDocs; ++j)
374+ csize numDocs = upd->numNodes();
375+ for (csize j = 0; j < numDocs; ++j)
376 pul->theInsertedDocs.push_back(static_cast<XmlNode*>(upd->getNode(j)));
377 }
378
379@@ -1563,8 +1555,8 @@
380 UpdCollection* upd = static_cast<UpdCollection*>
381 (pul->theDeleteFromCollectionList[i]);
382
383- ulong numDocs = upd->numNodes();
384- for (ulong j = 0; j < numDocs; ++j)
385+ csize numDocs = upd->numNodes();
386+ for (csize j = 0; j < numDocs; ++j)
387 pul->theDeletedDocs.push_back(static_cast<XmlNode*>(upd->getNode(j)));
388 }
389 }

Subscribers

People subscribed via source and target branches