| Saturday, August 20th, 2011, 00:22 UTC | ||
| [00:22:36] | Unhelpful_ is now known as Unhelpful | |
| [00:40:47] | Mousey (Mousey!~wtfisme@ross154.net) has quit (Ping timeout: 268 seconds) | |
| [01:02:29] | zombor (zombor!~zombor_@kohana/developer/zombor) has quit (Remote host closed the connection) | |
| [01:05:01] | gigem_ (gigem_!~david@mythtv/developer/gigem) has quit (Remote host closed the connection) | |
| [01:05:30] | gigem_ (gigem_!~david@mythtv/developer/gigem) has joined #mythtv | |
| [01:57:49] | zombor (zombor!~zombor_@kohana/developer/zombor) has joined #mythtv | |
| [02:40:17] | taylorr: | I noticed that we used to have a keyboard binding for the HD subchannel seperator called SUBCHANNELSEP – does anyone know what happened to that? |
| [02:40:46] | taylorr: | it would be nice to not have to use '_' since it requires a shift on a keyboard |
| [02:41:09] | taylorr: | being able to use a '.' would be nice |
| [02:41:58] | iamlindoro: | taylorr: suspect it went away when the option to set the subchannel separator did |
| [02:42:09] | iamlindoro: | ATSC channels are now all _, all other digital channels are all - |
| [02:43:12] | taylorr: | not cool at all |
| [02:43:52] | taylorr: | a useful setting removed because of whiny users |
| [02:44:47] | iamlindoro: | I am not sure we can blame the users on this one, I think it went with the rewrite of the scanner, but I suspect it is probably intended to return |
| [02:45:11] | iamlindoro: | and even if not intended, as the scanner gets moved to the web service it's an ideal time to allow the formatting options to return |
| [02:45:17] | taylorr: | would it be acceptable to have some code take the entered channum and replace '.' with '_' since '.' is a pretty standard syntax |
| [02:45:45] | iamlindoro: | What might be nicer, though, than a channel sep binding, would be channel autocomplete |
| [02:46:03] | iamlindoro: | ie you see a buttonlist of items that match as you enter, then you can select using up/down/select |
| [02:46:23] | taylorr: | that'd be cool |
| [02:46:31] | iamlindoro: | so "17–1" and you start typing "1...7..." and get a list of 17, 17–1, 17–2, 17–3, etc. |
| [03:07:00] | sphery: | taylorr: you don't have to actually type the separator... for 17_1, you type in 171--only time it won't work is if you have both a 17_1 and a 171 channel (in which case, fix your channel numbers to be unique :) |
| [03:08:09] | sphery: | and, really, I'd have assumed that the smart channel change would already treat any non-numeric character as a separator in a channel number |
| [03:08:50] | iamlindoro: | I long ago swapped things to their virtual channel numbers, so I had sort of forgotten about dealing with the sep |
| [03:09:02] | sphery: | technically, it might not work if you have a 1712, too, but how many channel numbers do people need? |
| [03:09:37] | sphery: | (i.e. with 1712, 171 is not yet a unique channel number--could be 17_1 or 1712, so..) |
| [03:11:33] | zombor (zombor!~zombor_@kohana/developer/zombor) has quit (Remote host closed the connection) | |
| [03:12:52] | sphery: | Oh, and, really, 171 /will/ work with 17_1 even if you have 1712--you just might have to hit SELECT to actually change |
| [03:17:14] | Bombo_ (Bombo_!~bombo@dslb-084-060-195-179.pools.arcor-ip.net) has joined #mythtv | |
| [03:18:39] | Bombo (Bombo!~bombo@dslb-084-060-241-000.pools.arcor-ip.net) has quit (Ping timeout: 268 seconds) | |
| [03:19:19] | taylorr: | sphery: it's easy to say fix your channel numbers but I have ATSC and satellite and I don't want to do that |
| [03:20:08] | taylorr: | and 17.1 doesn't work |
| [03:21:44] | sphery: | well, if you want to dedicate a remote button to a separator char and want to type it, feel free to extend the channel changer to allow any non-numeric char to equal # (or, eventually if someone brings back user control of the separator--which I think was the plan--whatever char they select) |
| [03:22:25] | zombor (zombor!~zombor_@kohana/developer/zombor) has joined #mythtv | |
| [03:23:14] | taylorr: | I'll wait for the user control of the separator to be added back |
| [03:23:20] | sphery: | it would make things better than they are now--and I don't see anyone in a hurry to bring back user control over the separator, so likely a much easier change than reworking the dead Qt scanner interface + the command-line scanner + the in-the-works HTTP scanner interface :) |
| [03:27:08] | taylorr: | I think allowing any special character be used as a separator might be dangerous depending on how it's implemented |
| [03:27:40] | taylorr: | mainly because someone could customize their channum in such a fashion that it might break |
| [03:27:40] | iamlindoro: | My separators is "DROP DATABASE mythconverg;" |
| [03:27:48] | sphery: | right, just saying that in the live tv channel changer, take any non-numeric value and replace it with # |
| [03:27:57] | sphery: | i.e. should be a single line of code |
| [03:28:15] | taylorr: | what does # do? |
| [03:28:20] | sphery: | (change the user input to be numbers and # only, basically, rather than matching exactly what they type) |
| [03:28:30] | sphery: | oh, then _ or whatever the current separator is |
| [03:28:31] | iamlindoro: | or perhaps, '\";DROP DATABASE mythconverg;\"' |
| [03:28:38] | iamlindoro: | </scheming> |
| [03:30:12] | taylorr: | but then you might have someone change their channum to be x.y instead of x_y |
| [03:30:59] | sphery: | well, if our UI doesn't allow x.y, they shouldn't be editing the DB directly... When we add back user-control of separator (giving them a list of _ or . or # , then we just replace with the specified sepator char) |
| [03:31:16] | sphery: | oh, and it seems you never need select to switch channels--it will change channels at timeout |
| [03:31:22] | taylorr: | mythweb allows anything you want |
| [03:31:56] | sphery: | looks like TV::ProcessSmartChannel() is where we'd make the change to replace non-numeric with a valid channel separator |
| [03:32:22] | sphery: | mythweb is raw database editing, so it allows a lot of things that won't work :) |
| [03:48:06] | zombor (zombor!~zombor_@kohana/developer/zombor) has quit (Remote host closed the connection) | |
| [03:48:22] | sphery: | actually, it looks like we're already slapping all of the once-valid spacers ("", "_", "-", "#", ".") in automatically in TVRec::CheckChannelPrefix() in a loop, so we could easily do the same for the "replace non-numeric with channel separator" logic |
| [03:49:43] | sphery: | try what they type, then go through and change the non-numeric chars in the input to _, -, #, and . and see if any are valid |
| [03:59:59] | iamlindoro: | j-rod|afk: You leaving us for EyeTV? :) |
| [04:21:06] | JEDIDIAH___ (JEDIDIAH___!~jedi@cpe-76-185-75-39.tx.res.rr.com) has quit (Remote host closed the connection) | |
| [04:53:28] | JEDIDIAH__ (JEDIDIAH__!~jedi@cpe-76-185-75-39.tx.res.rr.com) has joined #mythtv | |
| [05:35:19] | aloril (aloril!~aloril@dsl-tkubrasgw3-fe7ef900-153.dhcp.inet.fi) has quit (Ping timeout: 246 seconds) | |
| [05:48:29] | aloril (aloril!~aloril@84.249.126.153) has joined #mythtv | |
| [06:00:11] | Captain_Murdoch (Captain_Murdoch!~cpinkham@mythtv/developer/CaptainMurdoch) has quit (Ping timeout: 264 seconds) | |
| [06:10:24] | len (len!~quassel@174-30-215-151.mpls.qwest.net) has quit (Remote host closed the connection) | |
| [06:13:01] | len (len!~quassel@174-30-215-151.mpls.qwest.net) has joined #mythtv | |
| [06:13:14] | Captain_Murdoch (Captain_Murdoch!~cpinkham@ip72-218-58-187.hr.hr.cox.net) has joined #mythtv | |
| [06:13:14] | Captain_Murdoch (Captain_Murdoch!~cpinkham@ip72-218-58-187.hr.hr.cox.net) has quit (Changing host) | |
| [06:13:14] | Captain_Murdoch (Captain_Murdoch!~cpinkham@mythtv/developer/CaptainMurdoch) has joined #mythtv | |
| [06:13:20] | len (len!~quassel@174-30-215-151.mpls.qwest.net) has quit (Remote host closed the connection) | |
| [06:15:56] | len (len!~quassel@174-30-215-151.mpls.qwest.net) has joined #mythtv | |
| [06:33:08] | stoffel (stoffel!~quassel@p57B4D109.dip.t-dialin.net) has joined #mythtv | |
| [06:33:08] | stoffel (stoffel!~quassel@p57B4D109.dip.t-dialin.net) has quit (Remote host closed the connection) | |
| [06:39:57] | stoffel (stoffel!~quassel@p57B4D109.dip.t-dialin.net) has joined #mythtv | |
| [06:59:06] | len (len!~quassel@174-30-215-151.mpls.qwest.net) has quit (Remote host closed the connection) | |
| [07:08:38] | natanojl (natanojl!~jonatan@c83-252-237-63.bredband.comhem.se) has joined #mythtv | |
| [07:51:55] | Kunalagon (Kunalagon!~Kunalagon@212.200.243.60) has joined #mythtv | |
| [08:09:59] | Kunalagon (Kunalagon!~Kunalagon@212.200.243.60) has quit (Quit: Leaving.) | |
| [10:01:40] | stoffel (stoffel!~quassel@p57B4D109.dip.t-dialin.net) has quit (Ping timeout: 260 seconds) | |
| [10:03:54] | stuartm: | danielk22: iirc you wrote mythdeque, so can you just sanity check the following before I commit – http://pastebin.com/XGZdMFdt |
| [10:04:51] | stuartm: | as I'm reading it, mythdeque is just a wrapper around deque and it's still zero-indexed, so i = size() would be out of bounds? |
| [10:05:02] | mike|2 (mike|2!~mike@c-24-21-63-118.hsd1.or.comcast.net) has quit (Remote host closed the connection) | |
| [10:05:55] | 77CAA2I9P (77CAA2I9P!~mike@c-24-21-63-118.hsd1.or.comcast.net) has joined #mythtv | |
| [10:07:08] | stuartm: | hmm, unless ... I might be misunderstanding the intent in that code |
| [10:17:09] | stuartm: | could be re-written as http://pastebin.com/5Wp7s3Gn which is easier to read |
| [10:21:00] | xris (xris!~xris@mythtv/developer/xris) has quit (Ping timeout: 258 seconds) | |
| [10:22:37] | xris (xris!~xris@xris.forevermore.net) has joined #mythtv | |
| [10:24:14] | stoffel (stoffel!~quassel@p57B4D109.dip.t-dialin.net) has joined #mythtv | |
| [10:27:11] | jya_ (jya_!~jyavenard@60-242-40-141.static.tpgi.com.au) has joined #mythtv | |
| [10:27:11] | jya_ (jya_!~jyavenard@60-242-40-141.static.tpgi.com.au) has quit (Changing host) | |
| [10:27:11] | jya_ (jya_!~jyavenard@mythtv/developer/jya) has joined #mythtv | |
| [10:27:55] | jya (jya!~jyavenard@mythtv/developer/jya) has quit (Ping timeout: 260 seconds) | |
| [10:27:56] | jya_ is now known as jya | |
| [11:12:52] | jcarlos (jcarlos!~quassel@85.137.99.76.dyn.user.ono.com) has quit (Read error: Connection reset by peer) | |
| [11:13:27] | pheld (pheld!~heldal@cl-5.osl-01.no.sixxs.net) has quit (Quit: Leaving.) | |
| [11:14:46] | jcarlos (jcarlos!~quassel@85.137.99.76.dyn.user.ono.com) has joined #mythtv | |
| [12:02:56] | zombor (zombor!~zombor_@kohana/developer/zombor) has joined #mythtv | |
| [12:03:53] | gigem_ (gigem_!~david@mythtv/developer/gigem) has quit (Remote host closed the connection) | |
| [12:04:19] | gigem_ (gigem_!~david@host103.16.intrusion.com) has joined #mythtv | |
| [12:04:20] | gigem_ (gigem_!~david@host103.16.intrusion.com) has quit (Changing host) | |
| [12:04:20] | gigem_ (gigem_!~david@mythtv/developer/gigem) has joined #mythtv | |
| [12:18:06] | zombor (zombor!~zombor_@kohana/developer/zombor) has quit (Remote host closed the connection) | |
| [13:05:37] | danielk22: | stuartm: Yes MythDeque is just a wrapper around deque, I wouldn't have written if we had been using Qt4 at the time, the Qt4 QList provides all the same functionality. |
| [13:07:45] | danielk22: | I haven't had my coffee yet, but to my eyes that code looks equivalent. |
| [13:31:57] | danielk22: | stuartm: Heh, there is one thing that MythDeque provides that QList doesn't, if you do a dequeue on an empty list you get a null initialized return value. The code you wrote would work with both containers, while the original doesn't work with QList. |
| [13:38:19] | stuartm: | ok, I'll commit the re-written version since it's easier to see what's happening and as a bonus it's one step closer to using QList there instead |
| [13:38:51] | jya (jya!~jyavenard@mythtv/developer/jya) has quit (Quit: jya) | |
| [13:40:17] | stuartm: | I'm still not sure whether that size() comparison is incorrect in the original code, even with caffeine in my system I find that I'm not getting my head around it, the re-written code eliminates the whole question |
| [13:50:53] | danielk22: | stuartm: it's correct, the code needs to look at the whole list and we've popped one item already... |
| [13:51:36] | danielk22: | You could event make is <= size() * 2 it would just go through the list twice :) |
| [13:52:53] | stuartm: | danielk22: yeah, I'd had that realisation about 5 minutes ago :) |
| [13:58:22] | Bombo (Bombo!~bombo@dslb-084-062-228-009.pools.arcor-ip.net) has joined #mythtv | |
| [14:01:13] | Bombo_ (Bombo_!~bombo@dslb-084-060-195-179.pools.arcor-ip.net) has quit (Ping timeout: 268 seconds) | |
| [14:20:14] | foobum_ is now known as foobum | |
| [14:50:09] | danielk22: | sphery: I've pushed the UTC branch to github.. I forgot to add warnings about the fact that it doesn't convert dates in the DB yet and that some recording rules and manual schedule will not work properly. |
| [15:18:31] | danielk22: | gigem: gigem_: In the UTC branch one thing I didn't deal with is the custom recording rules.. the DAYOFWEEK() and HOUR() mysql functions won't work very well as is.. I think the best way to deal with this is to automagically insert a functioning CONVERT_TZ() around starttime and endtime in custom rules, what's your take on it? |
| [15:28:52] | stuartm: | Beirdo: ok, that's made a tiny dent on those warnings, we should disable -Wunused-but-set-variable for libav* |
| [15:29:04] | stuartm: | maybe we can figure out a way of disabling warnings for qt code too |
| [15:39:24] | stoffel (stoffel!~quassel@p57B4D109.dip.t-dialin.net) has quit (Ping timeout: 246 seconds) | |
| [15:52:29] | andreax (andreax!~andreaz@p57B92D3D.dip.t-dialin.net) has joined #mythtv | |
| [16:05:27] | stuartm: | danielk22: it starts getting a little unwieldy which the need to remember the conversion to the local timezone each time – granted the places where we might need to do that are limited e.g. Manual Schedule, but ... |
| [16:05:50] | stuartm: | s/which/with/ |
| [16:09:56] | stuartm: | maybe I'm worrying for no good reason, it's just that when you end up with "m_startDateTime.toLocalTime().time().hour()" there is a nagging feeling that it's becoming messy |
| [16:16:41] | andreax1 (andreax1!~andreaz@p57B94903.dip.t-dialin.net) has joined #mythtv | |
| [16:16:45] | andreax (andreax!~andreaz@p57B92D3D.dip.t-dialin.net) has quit (Ping timeout: 258 seconds) | |
| [16:24:05] | stoffel (stoffel!~quassel@p57B4B03D.dip.t-dialin.net) has joined #mythtv | |
| [16:59:23] | Bombo (Bombo!~bombo@dslb-084-062-228-009.pools.arcor-ip.net) has quit (Quit: Lost terminal) | |
| [17:04:48] | gigem_ (gigem_!~david@mythtv/developer/gigem) has quit (Remote host closed the connection) | |
| [17:05:14] | gigem_ (gigem_!~david@host103.16.intrusion.com) has joined #mythtv | |
| [17:05:15] | gigem_ (gigem_!~david@host103.16.intrusion.com) has quit (Changing host) | |
| [17:05:15] | gigem_ (gigem_!~david@mythtv/developer/gigem) has joined #mythtv | |
| [17:17:57] | highzeth (highzeth!~hz@hoiseth.no) has quit (Read error: Connection reset by peer) | |
| [17:18:34] | highzeth (highzeth!~hz@hoiseth.no) has joined #mythtv | |
| [17:28:32] | stoffel (stoffel!~quassel@p57B4B03D.dip.t-dialin.net) has quit (Ping timeout: 240 seconds) | |
| [17:42:46] | okolsi (okolsi!~mythtv@a88-115-32-206.elisa-laajakaista.fi) has quit (Quit: leaving) | |
| [17:44:30] | kwmonroe` (kwmonroe`!~kwmonroe@129.42.208.179) has joined #mythtv | |
| [17:47:27] | kwmonroe (kwmonroe!~kwmonroe@32.97.110.58) has quit (Ping timeout: 276 seconds) | |
| [18:01:00] | stoffel (stoffel!~quassel@p57B4B03D.dip.t-dialin.net) has joined #mythtv | |
| [18:08:45] | GreyFoxx (GreyFoxx!~greg@mythtv/developer/GreyFoxx) has quit (Ping timeout: 260 seconds) | |
| [18:19:13] | kormoc_afk is now known as kormoc | |
| [18:24:50] | xris (xris!~xris@xris.forevermore.net) has quit (Changing host) | |
| [18:24:50] | xris (xris!~xris@mythtv/developer/xris) has joined #mythtv | |
| [18:25:36] | stoffel (stoffel!~quassel@p57B4B03D.dip.t-dialin.net) has quit (Remote host closed the connection) | |
| [18:45:02] | kth (kth!~kth@unaffiliated/kth) has joined #mythtv | |
| [18:46:27] | kth (kth!~kth@unaffiliated/kth) has quit (Client Quit) | |
| [18:47:19] | gigem: | danielk22: at this point, i don't know enough about mysql's time handling to have a take other than it's probably going to cause a problem. i assume CONVERT_TZ() converts UTC to local time. if that's correct, then it might work. |
| [19:06:13] | danielk22: | gigem: CONVERT_TZ(DATETIME,'+00:00','EST') would convert from UTC to EST. The way I was thinking of implementing it is to have a function provide the second param. That way if the user doesn't have timezone tables loaded in the DB we can fall back to '-05:00' if they do we can use 'EST' (in this case). |
| [19:07:07] | danielk22: | Using EST is will work across daylight savings time, but I don't want to require them to have these tables loaded if it's easily avoidable. |
| [19:12:48] | danielk22: | stuartm: Thankfully places where you actually need to manually convert to local time are pretty limited. I spotted than a dozen places out of three thousand or so QDateTime uses.. MythDate::toString() takes care of the conversion for the UI 99% of the time. |
| [19:14:46] | danielk22: | stuartm: There are 71 instances of toLocalTime, most of them pretty tightly clustered around places that format time for the UI. |
| [19:33:29] | gigem: | danielk22: just to be clear, would the function to provide the time offset be in c++ or a prepared function in the db? |
| [19:37:30] | danielk22: | Something like this would be run on the custom rules querystr.replace("program.starttime", QString("CONVERT_TZ(program.starttime,'+00:00','%1')").arg(MythDate::G etMySqlTimeZone()"); where GetMySqlTimeZone() returns either "EST" or the current offset "-05:00" if the DB doesn't support "EST". |
| [19:38:27] | danielk22: | So it would be using the mysql CONVERT_TZ function, but we'd bake in the to timezone value in C++. |
| [19:43:05] | gigem: | okay. there's still one other area of concern, though. the new filters are another form of custom rule and would need to be handled similary. |
| [19:44:21] | danielk22: | gigem: if you can point me to the place where the QString::replace should occur for custom rules and filters I'll come up with some PoC code for both.. |
| [19:54:01] | gigem: | the filters handled in Scheduler::UpdateMatches() around line 3296 and the custom rules in Scheduler::BuildNewRecordsQueries() around line 3170. |
| [20:00:25] | zombor (zombor!~zombor_@kohana/developer/zombor) has joined #mythtv | |
| [20:03:39] | GreyFoxx (GreyFoxx!~greg@mythtv/developer/GreyFoxx) has joined #mythtv | |
| [20:05:33] | danielk22: | gigem: thanks |
| [20:07:40] | zombor (zombor!~zombor_@kohana/developer/zombor) has quit (Remote host closed the connection) | |
| [20:13:17] | len (len!~quassel@184-97-158-26.mpls.qwest.net) has joined #mythtv | |
| [20:14:08] | stuartm: | danielk22: there's probably a proportion of those places where we should be using toString() anyway but that's another job for another time |
| [20:38:30] | danielk22: | stuartm: yeah, there is also MythDate::kDatabase and MythDate::ISODate/Qt::kISODate they appear to be used interchangeably for inserting DATETIME entries into the DB. The only difference is the first one doesn't have the "T" between the date and time. |
| [20:39:27] | wagner_ is now known as wagnerrp | |
| [20:39:29] | wagnerrp (wagnerrp!~Wagner@nr-ft1-66-42-241-137.fuse.net) has quit (Changing host) | |
| [20:39:29] | wagnerrp (wagnerrp!~Wagner@mythtv/developer/wagnerrp) has joined #mythtv | |
| [20:40:09] | danielk22: | stuartm: AFACIT mysql doesn't care whether you use ISO 8601 separate date and time format or ISO 8601 combined date and time format; but there may be some argument for being consistent. |
| [20:40:43] | stuartm: | less liable to cause confusion for one |
| [20:41:47] | kth (kth!~kth@unaffiliated/kth) has joined #mythtv | |
| [20:42:29] | kth (kth!~kth@unaffiliated/kth) has quit (Client Quit) | |
| [20:45:11] | sphery: | danielk22: doesn't sound like you're planning to change how we send dates to the DB, but if we use string dates (versus doing a proper prepared query and passing a QDateTime), we can't put the T between date and time--MySQL 5.5 will error on dates with invalid formats (previous versions were more permissive) |
| [20:46:00] | sphery: | that was the issue for http://code.mythtv.org/trac/ticket/8585 |
| [20:46:10] | stuartm: | I knew that there were still fringe places which did their own thing wrt to date formatting for the UI, especially plugins, some of which I'm afraid to touch for fear of upsetting people over a plugin I don't even use |
| [20:46:40] | stuartm: | sphery: well that settles it IMHO |
| [20:47:37] | stuartm: | sphery: that said, we should be using prepared queries and bound values everywhere so either way works ... |
| [20:48:53] | iamlindoro: | stuartm: No objection from me if any plugin I maintain has screwy date formatting |
| [20:49:16] | sphery: | yeah, there are only a few cases that still use string date/time representations in queries--mainly because they're using hard-coded literal values (though we could actually change them to use prepared queries, too, and let the drivers make sure it's always understood by the DB) |
| [20:49:31] | iamlindoro: | stuartm: In those I have worked on I've tried to use example code from elsewhere for consistency, but that can be a dual-edged sword |
| [20:49:35] | stuartm: | mytharchive comes to mind |
| [20:49:51] | stuartm: | iamlindoro: right, if the example you're copying from is wrong ... :) |
| [20:49:56] | iamlindoro: | exactly |
| [20:53:30] | stuartm: | it's a little clearer in master, we have some date formatting helpers, you passing in what you want (date, time, date + time etc) and it will return a string that conforms to both the user's preferences and is formatted identically to everywhere else |
| [20:54:03] | danielk22: | sphery: Good to know. This means we really need to check on all the locations using ISODate.. all 191 occurances :| I think most of them are just using it for debug messages.. We should also look at the places using kDatabase.. even if it works we really should be just binding the QDateTime.. |
| [20:54:53] | stuartm: | has anyone given serious thought to systematically replacing all non-prepared queries with properly prepared ones? |
| [20:55:29] | danielk22: | stuartm: isn't that less efficient if you're just going to do the query once? |
| [20:56:45] | stuartm: | I can't honestly say that the rewards are equal to the effort but DB security and providing an example for others to follow would be my reasons for doing it |
| [20:57:55] | stuartm: | danielk22: I cannot say, but if that's the case we're already doing it in any number of places where the query is executed just once or infrequently so ... |
| [20:59:14] | danielk22: | stuartm: if your looking for a project to do when too tired for serious coding... we also do a lot of query.isActive() and query.size() calls that are unnecessary and there are places that could do with some DBError() reporting on query failure.. :) |
| [20:59:22] | stuartm: | certainly prepared queries have been strongly recommended over non-prepared queries for a number of years now, mostly on the grounds of security and integrity – it prevents unsanitised crap screwing up the database |
| [21:00:07] | stuartm: | danielk22: I'm not looking at it myself right now, I was just curious whether anyone had been thinking of doing it |
| [21:00:36] | danielk22: | stuartm: I think using prepare with bindings is cleaner and in many cases readability trumps speed. |
| [21:02:21] | danielk22: | I haven't been thinking of doing it.. but I often do it when I run across code that doesn't using bindings and the easy transform is trivially correct. |
| [21:08:21] | kth (kth!~kth@unaffiliated/kth) has joined #mythtv | |
| [21:09:06] | sphery: | danielk22: yeah, I didn't find kDatabase when looking at #8585 (likely because it actually worked :), but I did fix all the literal dates in SQL in https://github.com/MythTV/mythtv/commit/7e33788e and changed all the locations with bindValue(.*toString that were string-ifying dates to just bind the QDateTime in https://github.com/MythTV/mythtv/commit/cd073b6e9 |
| [21:11:27] | sphery: | Yeah, I think we should use prepared queries any time we have "dynamic" SQL--never use string concatenation to create dynamic SQL strings. For the "static" SQL, though, I don't have a strong preference. |
| [21:12:11] | sphery: | i.e. no query.arg(value).arg(value) stuff... Just use a prepared query and bindValue() |
| [21:13:29] | kth (kth!~kth@unaffiliated/kth) has quit (Quit: Leaving.) | |
| [21:14:07] | danielk22: | sphery: But I can see an exception with something like the CONVERT_TZ stuff I just talked about this morning.. it's something we're inserting into someone else's query. |
| [21:14:11] | stuartm: | sphery: kDatabase has only existed for a few hours |
| [21:15:27] | danielk22: | kDatabase just replaces places where people where using toString() with an ISO 8601 separated date and time format. |
| [21:15:42] | danielk22: | And it's only in the utc branch :) |
| [21:18:41] | danielk22: | stuartm: and if we're going to talk about SQL security, we do allow people to insert their own SQL into the scheduler... |
| [21:22:13] | stuartm: | danielk22: well that's a sore point for me, I don't like it for a host of reasons – impossible to provide a decent remote friendly UI, requires users to be fluent in SQL and allows users to screw up their databases/schedules without even knowing it etc |
| [21:22:31] | stuartm: | but it's not going anywhere, so I choose to ignore it :) |
| [21:24:13] | sphery: | ah, that's why I didn't notice it, before. :) and, yeah, as I understood the CONVERT_TZ stuff, it seemed good--it's just used to replace the strings that would be strings, anyway, because we're allowing users to insert sql strings into the scheduler query and all, right? |
| [21:24:17] | stuartm: | oh and it's currently the only place to use a multiline textedit, a widget I've never managed to get working as intended and which remains a thorn in my side |
| [21:25:19] | iamlindoro: | stuartm: I still want to do the "refrigerator magnet" approach to that some day |
| [21:26:22] | iamlindoro: | present a list of possible "nouns" (columns), then when one is selected, present a list of verbs (=, <, !=, etc), then either allow "finishing" the phrase or adding an "AND" |
| [21:26:57] | iamlindoro: | FWIW the plot editor widgets for Recordings and Videos are users of a multiline text edit |
| [21:27:18] | danielk22: | sphery: yeah, basically it's so that the user can insert raw SQL without needing to worry about timezone conversions. |
| [21:28:30] | sphery: | yeah, seems like a good way to handle it--and the scheduler query does other .arg() stuff, anyway (and I wouldn't even consider trying to prepare-ify that :) |
| [21:29:36] | stuartm: | iamlindoro: on some level I think the custom scheduler was a bit of a cop-out, having done a fantastic job to offer a powerful and versatile scheduler which does the job for 99.9%, a tiny minority still wanted more and instead of coming up with a clever approach or drawing a line we ended up with a direct sql editor ... |
| [21:31:32] | danielk22: | sphery: I looked at all the MythDate::kDatabase uses, and only one was an instance of converting a date to a string before binding it to a DATETIME. The others were saving dates into the settings table. |
| [21:31:55] | sphery: | iamlindoro: I like that idea a lot--but would love it if we did an even more simplified approach using only our example clauses as "functional" magnets (with appropriate questions to allow them to fill in title or whatever) where the user doesn't have to know about or see any DB-related info. Then, maybe, do another "Use at your own risk" UI that does the lower-level stuff where they pick tables and columns and comparisons or sql functions or ... |
| [21:32:23] | sphery: | I think sticking together our example clauses to form a custom rule would account for 99.9% of users' needs |
| [21:32:40] | jya (jya!~jyavenard@60-242-40-141.static.tpgi.com.au) has joined #mythtv | |
| [21:32:40] | jya (jya!~jyavenard@60-242-40-141.static.tpgi.com.au) has quit (Changing host) | |
| [21:32:40] | jya (jya!~jyavenard@mythtv/developer/jya) has joined #mythtv | |
| [21:33:21] | stuartm: | iamlindoro: we're here now and so we have to make the best of it I guess, the approach you suggest is the best one I've heard |
| [21:33:22] | danielk22: | sphery: if you want to fix it in master right now it's m_searchTime in proglist.h |
| [21:35:22] | jya (jya!~jyavenard@mythtv/developer/jya) has quit (Client Quit) | |
| [21:38:45] | sphery: | danielk22: ah, cool--I hadn't noticed that one because it didn't use the bindValue() function in the same line. I'll change it |
| [21:40:15] | sphery: | I found what I changed through a grep--but I'm actually impressed that was the only location that used a different construct for binding the date string--figured there would be several that may have split the bindValue() and toString() onto different lines or something. |
| [21:41:54] | danielk22: | Well maybe they used ISODate.. and those would actually be the ones the DB would reject.. I haven't look at those yet.. 191 occurrences.. I just did kDatabase because it had much fewer occurrences. |
| [21:47:50] | sphery: | ah, you fixed it in utc... should I cherry pick from there, I suppose? |
| [21:48:35] | danielk22: | it probably won't apply cleanly... |
| [21:50:08] | sphery: | ah, yeah, because of the MythDate change you had already made... |
| [21:50:44] | sphery: | maybe it's better to just leave it as is in master, then, so it doesn't cause you a conflict on merge (since it's at least using the right string format, it works even with MySQL 5.5) |
| [22:07:14] | danielk22: | sphery: Check the latest utc commit, I found some instances where we were using ISO 8601 date/time format for DB inserts when I looked at ISODate usages. |
| [22:09:00] | danielk22: | Don't worry about the conflicts, git doesn't handle them as well as svn, but it handles them as well as cvs did ; I can deal with them almost as quickly. |
| [22:12:23] | sphery: | ah, yeah... I never grep'ed for the bindings[.*toString( pattern... I'll go ahead and fix those in master, then backport that one to -fixes, then I'll convert the 2 proglist toString() bindings to bind the QDateTime directly in master only |
| [22:16:17] | okolsi (okolsi!~mythtv@unaffiliated/okolsi) has joined #mythtv | |
| [23:03:47] | sailerboy (sailerboy!sailerboy@ipv61.sailerboy.net) has quit (Ping timeout: 264 seconds) | |
| [23:21:02] | sailerboy (sailerboy!sailerboy@ipv61.sailerboy.net) has joined #mythtv | |
| [23:29:50] | Cougar (Cougar!~cougar@kkk.version6.net) has quit (Read error: Operation timed out) | |
| [23:36:51] | Cougar (Cougar!~cougar@kkk.version6.net) has joined #mythtv | |
| [23:38:27] | PointyPumper (PointyPumper!~pintlezz@190.244.73.13) has quit (Ping timeout: 252 seconds) | |
| [23:40:06] | dblain (dblain!~dblain@mythtv/developer/dblain) has quit () | |
| [23:46:18] | PointyPumper (PointyPumper!~pintlezz@190.244.73.13) has joined #mythtv | |
| [23:51:01] | dblain (dblain!~dblain@c-76-127-227-175.hsd1.ma.comcast.net) has joined #mythtv | |
| [23:51:03] | dblain (dblain!~dblain@mythtv/developer/dblain) has joined #mythtv | |
| [23:51:03] | dblain (dblain!~dblain@c-76-127-227-175.hsd1.ma.comcast.net) has quit (Changing host) | |
IRC Logs collected by
BeirdoBot.
Please use the above link to report any bugs.