drachels / moodle-mod_mootyper

4 stars 18 forks source link

Sql in var $finalexercisecompletesql fails because its missing group by #121

Closed al-sha-el closed 7 months ago

al-sha-el commented 8 months ago

Hello,

I've encountered a problem where the sql failed because it was missing the group by.

ERROR: Column "mtg.timetaken" must appear in the GROUP-BY clause or be used in an aggregate function
LINE 3: mtg.timetaken,
^
SELECT COUNT(mtg.userid),
AVG(mtg.grade),
mtg.timetaken,
mtg.mistakedetails,
mtg.exercise,
AVG(mtg.precisionfield),
AVG(mtg.wpm),
AVG(mtg.pass)
FROM mdl_mootyper m
JOIN mdl_mootyper_grades mtg ON mtg.mootyper = m.id
WHERE m.id = $1
AND mtg.userid = $2
AND mtg.grade >= m.grade_mootyper
AND mtg.precisionfield >= m.requiredgoal
AND mtg.wpm >= m.requiredwpm
AND mtg.pass = 1
[array (
0 => '59',
1 => 512,
)]
Error code: dmlreadexceptio

In my research, I identified that in the file _classes/completion/customcompletion.php , within the function _getstate($rule) , the variable $finalexercisecompletesql without the "GROUP BY" clause, and its never added.

classes/completion/custom_completion.php line 83:

$finalexercisecompletesql = "SELECT COUNT(mtg.userid),
                                            AVG(mtg.grade),
                                            mtg.timetaken,
                                            mtg.mistakedetails,
                                            mtg.exercise,
                                            AVG(mtg.precisionfield),
                                            AVG(mtg.wpm),
                                            AVG(mtg.pass)
                                       FROM {mootyper} m
                                       JOIN {mootyper_grades} mtg ON mtg.mootyper = m.id
                                      WHERE m.id = :mootyper
                                        AND mtg.userid = :userid
                                        AND mtg.grade >= m.grade_mootyper
                                        AND mtg.precisionfield >= m.requiredgoal
                                        AND mtg.wpm >= m.requiredwpm
                                        AND mtg.pass = 1";

pluginversion 4.2.6 Moodleversion 4.1.8 php 8.1 Postgresql 13

Best regards Alex

drachels commented 8 months ago

Thanks for the information, al-sha-el. I am currently working on the file as I have found some other completion problems with MooTyper. I will include the GROUPT BY clause in the next release. I will also check the other SQL's in other parts of the code.

9D3F commented 8 months ago

Hello @drachels,

unfortunately we are also very strongly affected by the problem.

We coordinate several thousand Moodle instances for schools. They can no longer use the activity properly, although it has been very well received so far. Can you please let us know when we can expect an adjustment of the plugin on your part and when the new version will be available?

Many thanks in advance 9D3F

drachels commented 8 months ago

Since I have had major work in progress with other plugins and MooTyper, it will take me a little while to upgrade my only "neglected" server running PostgreSQL. I will see if I can get a MooTyper v4.2.6+ version out over the weekend. Or, if things go well, maybe a new MooTyper v4.2.7.

drachels commented 8 months ago

Wow, I just remembered why I have never liked PostgreSQL...it is extremely picky and did NOT like that my MooTyper did not have any grades in the mdl_mootyper_grades table, when I started working on this. But, finally, if you have a test site here is what needs to be in that sql: // 20240112 Modified for Postgresql. $finalexercisecompletesql = "SELECT COALESCE(COUNT(mtg.userid), 0) AS count, AVG(mtg.grade), mtg.timetaken, mtg.mistakedetails, mtg.exercise, AVG(mtg.precisionfield), AVG(mtg.wpm), AVG(mtg.pass) FROM {mootyper} m LEFT JOIN {mootyper_grades} mtg ON mtg.mootyper = m.id AND mtg.userid = :userid AND mtg.grade >= m.grade_mootyper AND mtg.precisionfield >= m.requiredgoal AND mtg.wpm >= m.requiredwpm AND mtg.pass = 1 WHERE m.id = :mootyper GROUP BY mtg.timetaken, mtg.mistakedetails, mtg.exercise ORDER BY mtg.timetaken ASC";

Note that this gets rid of the error, but does seem to break the MooTyper extra completions, but not the built in Moodle completions, so I am still working on it.

9D3F commented 7 months ago

Hey @drachels ,

first of all, thank you very much for your quick feedback on the error - can you tell us more specifically when we can expect the new version of this great plugin?

we and our users would really appreciate it!

Thank you very much 9D3F

drachels commented 7 months ago

@9D3F , Sorry for the late reply as I was out-of-town yesterday for a retired teachers association luncheon. As to when I might be able to publish a new version of MooTyper, all I can say is that I am working on it. However due to age and health related issues, as well as server issues, I cannot pin it down.

Please note that even when I came up with the new SQL listed above, I cannot just publish a new version with it in the code. I have to run a whole bunch of tests to make sure I do not break MooTyper for all the MariaDB and MySQL database users.

Also I note that you have not mentioned any details regarding your setup, so I can only work on making sure new code will work for the original posting.

drachels commented 7 months ago

Additional info: One frustrating thing slowing me down is that I had discovered that all the new MooTyper completion settings were not being updated correctly. Seems they were likely only working with, Purge caches, which I do on a fairly regular basis, probably just enough to make me think the completions were okay. I just got them to work, but I am 99% sure the code is in the wrong place, plus I have a ton of debug code in the files that needs to be removed, plus I reverted to an earlier version of the code and need to check that other fixes and changes from the removed code, get placed back into the current code.

Anyway, right now my eyes feel like sandpaper rubbing on them every time I blink. Been looking at a monitor too long for the day, so I need to quit. Hopefully, tomorrow I might have some code worth pushing to github, so you guys might try it out, if you have a development server.

9D3F commented 7 months ago

Hi @drachels,

thank you for the informations about the Mootyper! Please take care of your health and set yourself limits.

It's great to see how committed you are. If you can create a new version, we would really appreciate it BUT NO MUST!!!

All the best for you @drachels

drachels commented 7 months ago

Well, good news, and bad news. My eye doctor gave me a new antibiotic eye drop prescription, that seems to be working pretty good, so far. At least I am coming up on seven hours straight, without a noticeable problem. That's one good news item.

Good news number two is that I seem to have completions working correctly for MariaDB and MySQL. The bad news is that I do not have it working without spitting out a bunch of debug messages, when I am testing with PostgreSQL, on a Moodle 4.3 server.

I actually wasted 30 minutes thinking I had a problem with my SQL, when it actually turned out to be bugs in the Ad-hoc database queries plugin, as it is list as being good only up to Moodle 4.0 and I was testing in Moodle 4.3.

So, I am done for today and have to go work on something for my personal use. I will see what I can do tomorrow.

9D3F commented 7 months ago

Hey @drachels,

thank you very much for the news - all in all, this is positive evidence to build on! I'm sure that you'll be able to fix the other little aches and bugs with time.

Best Regards Dominic

drachels commented 7 months ago

Hmm, my favorable impression for the MS Chat AI went down a LOT. After working all this time, I have come up with a MUCH simpler solution, that works not only for PostgreSQL, but my other databases that I use. AND it does not require the GROUP By. I still have some more testing to do, but will be out tomorrow for a doctors appointment, but thought I would let you guys have access to my current code in the master branch and MOODLE_427_STABLE branch.

NOTE: I thought of some cases where I might need to add/change code base on the MooTyper mode that the activity is set up to use. Will just have to wait a bit, though.

9D3F commented 7 months ago

Hey @drachels,

but that's good news!!! We're already looking forward to the new version so that the plugin is fully available to users again 🙏

All the best Dominic

9D3F commented 7 months ago

Hey @drachels,

very cool that a new version of the plugin is available:

4.2.7 (Build: 2024013100) (2024013100) Moodle 3.11, 4.0, 4.1, 4.2, 4.3 Released: Freitag, 2. Februar 2024, 01:11

is the bug now obsolete due to the new version?

Kind Regards Dominic

drachels commented 7 months ago

Yes, that is the latest version and works with PostgreSQL.

9D3F commented 7 months ago

Hey @drachels,

wonderful thank you very much for your commitment a great contribution to the community!