What 's wrong with this query?

[Miss_B]

Hello,

Can someone please tell me what is wrong with the following query? It is about a mod that I submitted, but I heard that it is a nightmare... duplicate data logging, really bad queries because of the missing limits and because the table is literally never truncated. But unfortunately no details were provided as to what part is horrible, what limit etc.

What I am trying to do is query a custom table and get the duplicate entries. I have made a join with the members table to get the quick search working. Any input/advice is greatly appreciated. 

		$arrayCheck = array();
		
        foreach( \IPS\Db::i()->select( 'customtable_log.device, MAX(customtable_log.user_id) AS user_id, GROUP_CONCAT(DISTINCT core_members.name SEPARATOR ", ") AS shared_by', 'customtable_log', NULL, NULL, NULL, 'customtable_log.device', array( 'COUNT(customtable_log.user_id) > 1' ) )->join('core_members', 'customtable_log.user_id = core_members.member_id') as $row )
        {
	       $arrayCheck[] = $row;
        }
		
		$table = new \IPS\Helpers\Table\Custom($arrayCheck, \IPS\Http\Url::internal( 'app=customapp&module=settings&controller=check' ) );

 

Then for the quick search I am using this:

$table->quickSearch = 'shared_by';

 

And for the limit:

$table->limit = 20;

 

[Daniel F]

I mentioned it already in the ticket => there's no limit in the query. 
You're literally fetching ALL the records from the table which can become really huge since you're not truncating it.

foreach( \IPS\Db::i()->select( 'customtable_log.device, MAX(customtable_log.user_id) AS user_id, GROUP_CONCAT(DISTINCT core_members.name SEPARATOR ", ") AS shared_by', 'customtable_log', NULL, NULL, NULL, 'customtable_log.device', array( 'COUNT(customtable_log.user_id) > 1' ) )->join('core_members', 'customtable_log.user_id = core_members.member_id') as $row )
        {

It's a custom TableHelper and you're iterating over the DB Query results before the data get passed to the DB Helper.

[Miss_B]

Thank you for your reply @Daniel F, is the limit not added by $table->limit? Can you please tell me what you mean by truncating it? If the table is truncated, then there will be no data to display? 

[Adriano Faria]

He’s talking about limiting the query and not use $table->limit, which by the way has no effect here as custom table has no pagination; you will have to make your manually if you want to paginate records.

[Miss_B]

16 minutes ago, Adriano Faria said:

He’s talking about limiting the query and not use $table->limit, which by the way has no effect here as custom table has no pagination;

Ah I see, thank you. I was not aware that the custom table has no pagination. Is there any guide where I can read up on this? I have searched but could find no info on the matter. If I change the query to the following, will it help?

[quote] foreach( \IPS\Db::i()->select( 'customtable_log.device, MAX(customtable_log.user_id) AS user_id, GROUP_CONCAT(DISTINCT core_members.name SEPARATOR ", ") AS shared_by', 'customtable_log', NULL, NULL, NULL, 'customtable_log.device', array( 'COUNT(customtable_log.user_id) > 1' ), array( 0, 10 ) )->join('core_members', 'customtable_log.user_id = core_members.member_id') as $row )
[/quote]

What about the truncating of the table mentioned above? Any clues as to where/what?

 

 

Edited May 30, 2022 by Miss_B

[Adriano Faria]

3 minutes ago, Miss_B said:

What about the truncating of the table mentioned above? Any clues as to where/what?

From what I understand, you don’t have a prune routine to delete old entries from your table so it can get really huge, that’s why he suggested to use LIMIT in the query. The way it is it’s fetching all records from your table.

Regarding pagination in custom tables, take a look in the public controller of the messenger.

[Miss_B]

23 minutes ago, Adriano Faria said:

From what I understand, you don’t have a prune routine to delete old entries from your table so it can get really huge, that’s why he suggested to use LIMIT in the query. The way it is it’s fetching all records from your table.

So, if I add an automated prunning task, that would fix the issue?

23 minutes ago, Adriano Faria said:

Regarding pagination in custom tables, take a look in the public controller of the messenger.

Many thanks. I will look into it.

[Adriano Faria]

25 minutes ago, Miss_B said:

So, if I add an automated prunning task, that would fix the issue?

Yes. Create a setting to the admin choose the number of days and a task to delete records older than that number of days. There a lot that in the suite. See the log errors pruning setting and task, for example.

[Miss_B]

2 minutes ago, Adriano Faria said:

Yes. Create a setting to the admin choose the number of days and a task to delete records older than that number of days. There a lot that in the suite. See the log errors pruning setting and task, for example.

Thank you again. I have used the prunning task on my other apps. I will add it here too. 

[Martin A.]

What's your reason for adding all the entries to an array and then use that in a custom table, instead of 

$table = new \IPS\Helpers\Table\Db( 'customtable_log', $url, $where );
$table->selects = [ 'customtable_log.device', 'MAX(customtable_log.user_id) AS user_id'];
$table->joins = [ [ 'select' =>  'GROUP_CONCAT(DISTINCT core_members.name SEPARATOR ", ") AS shared_by', 'from' => 'core_members', 'where' => 'customtable_log.user_id = core_members.member_id' ] ];
$table->groupBy = 'COUNT(customtable_log.user_id) > 1';

 ?

I haven't tested this, but you should be able to use the Db class even if you need to use a query out of the ordinary.

[Miss_B]

2 hours ago, Martin A. said:

What's your reason for adding all the entries to an array and then use that in a custom table, instead of 

$table = new \IPS\Helpers\Table\Db( 'customtable_log', $url, $where );
$table->selects = [ 'customtable_log.device', 'MAX(customtable_log.user_id) AS user_id'];
$table->joins = [ [ 'select' =>  'GROUP_CONCAT(DISTINCT core_members.name SEPARATOR ", ") AS shared_by', 'from' => 'core_members', 'where' => 'customtable_log.user_id = core_members.member_id' ] ];
$table->groupBy = 'COUNT(customtable_log.user_id) > 1';

 ?

I haven't tested this, but you should be able to use the Db class even if you need to use a query out of the ordinary.

Many thanks for your reply. Using the tables like this seems even better. The reason that I added all entries to an array was becaue of my other topic below. And it had worked until now.

https://invisioncommunity.com/forums/topic/456844-how-to-use-this-query-with-ipsdbi-select/#comment-2819689

When I use the tables in your example I get this error:

Quote

IPS\Db\Exception: Unknown column 'COUNT(customtable.user_id) > 1' in 'group statement' (1054)

I think that is because the COUNT(sharedloginsdetectors_log.user_id) > 1 is part of the HAVING query. In raw sql qould be:

HAVING COUNT(customtable.user_id) > 1

I am grouping the results by device. Can you please tell me how can I add the HAVING part with tables? That would solve the issue.

[Martin A.]

Sorry about that, didn't realize that was your HAVING clause. 

I don't see an easy way for us to add HAVING to that query. For @Daniel F & co on the other hand... 🙂

[Miss_B]

1 hour ago, Martin A. said:

Sorry about that, didn't realize that was your HAVING clause. 

No problem at all. 

 

1 hour ago, Martin A. said:

I don't see an easy way for us to add HAVING to that query. For @Daniel F & co on the other hand... 🙂

I figured it out myself. The HAVING part should be added to the $table->selects. Many many thanks for your help on this @Martin A..  Using the tables is indeed much easier than adding all the entries to the array as I did. The only drawback is that the search is not working with this method.

Anyways, this can be considered solved. Thank you also to everyone else who replied.