MSqlQuery.size() removal for libmythtv

[anonymous]

this overrides #3060.

can someone close #3060?

[usleepless@…]

i forgot to fill in "Reported by"

[david@…]

The QT manual (3.3) says that query.exec() "Returns TRUE and sets the query state to active if the query was successful".

So couldn't things like "query.exec() && query.isActive() &&..." be simplified to "query.exec() && ..." while you're at it?

[usleepless@…]

David,

thanks for thinking along. i already removed some of the isActive() calls, but i rather wait and see if this ticket is commited. i don't want to change too much on the first run.

if it commited, i will press on and submit ticket(s) for the rest of the code-base.

regards,

usleep

[luitjens]

I have been using this patch for a day without problems. Also I looked over the patch and it appears to be correct. However, I think the patch should be updated to reflect David's comments before being committed as it is a simple change. Usleep could you update the patch to remove the unnecessary isActive() calls please?

Thanks, Justin

[luitjens]

updated patch to revision 12767

[luitjens]

updated to r12767 and replaced a few .size() calls with .numRowsAffected(). usleep does this work for you?

[luitjens]

This patch has worked just fine. Tonight I recorded 9 programs in 2 hours while watching another recorded program with zero problems. I have also updated the patch to the latest head and removed a couple more .size() calls and replaced some of them with .numRowsAffected(). usleep is this function compatible for you?

The patch can be found here: ​http://svn.mythtv.org/trac/attachment/ticket/3061/patch.r12767.2.diff

[david@…]

Replying to luitjens:

This patch has worked just fine. Tonight I recorded 9 programs in 2 hours while watching another recorded program with zero problems. I have also updated the patch to the latest head and removed a couple more .size() calls and replaced some of them with .numRowsAffected(). usleep is this function compatible for you?

Ok, I did a quick review of the patch:

• I couldn't find any numRowsAffected calls in the patch.
• There are still a lot of redundant isActive() calls left
• The .size() removal has been taken too far. For instance, code like this is just plain ugly:

-        int qsize = query.size();
+        while(query.next())
+            qsize++

MySQL and PostgreSQL (and any other DB I've used with QT) support the .size() call, so I see no point in not using it where appropriate.

In order to not come across as a nag...I'll provide an updated patch later today or tomorrow.

[usleepless@…]

David,

i will update it myself ( according to your remarks ).

how do i update the patch to revision revision 12767?

regards,

usleep

[david@…]

Replying to usleepless@gmail.com:

how do i update the patch to revision revision 12767?

You should not have to do anything special, just make sure that you have an up-to-date SVN repo (use "svn update") before you create the patch.

[usleepless@…]

i did "svn update", had one weird conflict.

i am unable to upload to trac ( rejecting spam: led )

how come?

regards,

usleep

[luitjens]

Sorry,

I must have attached the wrong patch. Also I agree that the qsize part is ugly. If numRowsAffected solves the size issue then we should use that.

[david@…]

I've reviewed the latest patch, and first of all...beautiful. It removes around 700 unnecessary function calls. The diffstat says a lot about how much work it is:

 cardutil.cpp           |   37 ++++-----
 channel.cpp            |    2 
 channelbase.cpp        |   26 +++---
 channeleditor.cpp      |   17 +---
 channelsettings.cpp    |   16 ++-
 channelsettings.h      |   15 +--
 channelutil.cpp        |  100 +++++++++++++++---------
 customedit.cpp         |    8 -
 datadirect.cpp         |   15 +--
 dbcheck.cpp            |   14 +--
 dbox2channel.cpp       |    6 -
 dbox2epg.cpp           |    4 
 diseqc.cpp             |   20 ++--
 dtvchannel.cpp         |    6 -
 dtvmultiplex.cpp       |    2 
 dvbchannel.cpp         |    5 -
 eit.cpp                |    2 
 eitcache.cpp           |    4 
 eithelper.cpp          |    4 
 eitscanner.cpp         |    2 
 iptvchannel.cpp        |    2 
 jobqueue.cpp           |  104 ++++++++-----------------
 livetvchain.cpp        |    6 -
 mhi.cpp                |    4 
 mpegrecorder.cpp       |    2 
 profilegroup.cpp       |   16 +--
 progfind.cpp           |   24 +++--
 proglist.cpp           |   12 --
 programdata.cpp        |   34 +++-----
 programinfo.cpp        |  201 +++++++++++++++++++------------------------------
 recordingprofile.cpp   |   26 +++---
 remoteencoder.cpp      |    2 
 scanwizardhelpers.cpp  |    4 
 scheduledrecording.cpp |    9 --
 siscan.cpp             |   16 +--
 sourceutil.cpp         |   16 +--
 sr_items.cpp           |    2 
 sr_items.h             |    6 -
 storagegroup.cpp       |   12 +-
 transporteditor.cpp    |    8 -
 tv_play.cpp            |   14 +--
 tv_rec.cpp             |   42 ++++------
 videodev_myth.h        |   12 +-
 videoout_xv.cpp        |    3 
 videosource.cpp        |   31 +++----
 yuv2rgb.cpp            |   11 ++
 46 files changed, 436 insertions(+), 488 deletions(-)

It looks almost perfect, some things to fix though:

• You introduce a spelling error in dbcheck.cpp (see the last line):

  -        if (!recordids.exec())
  -            return false;
   
  -        if (recordids.isActive() && recordids.size() > 0)
  +        if (recordids.ecec())
  

• The changes to videodev_myth.h are completely unrelated to the SQL changes
• Same thing goes for videoout_xv.cpp
• Same thing goes for mpegrecorder.cpp
• Same thing goes for yuv2rgb.cpp
• You haven't resolved the conflict in channelutil.cpp (search that file for "<<<<" and you'll see it)

[Isaac Richards]

Patches which obviously haven't even been compiled or tested by the submittor should NOT be getting posted to trac.

[usleepless@…]

fixed typo and actually compiled ( friday i was due to leave for the weekend, sorry ... ) removed the patches for videodev_myth.h, videoout_xv.cpp, mpegrecorder.cpp and yuv2rgb.cpp ( which were fbsd patches, sorry again )

[david@…]

I've reviewed the new patch and it looks good, all the complaints I had have been resolved.

I hope it can get committed soon since a patch that touches this many files will bitrot quite quickly.

[greg]

long dead patch which is now way way out of date. Closing