Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Filtering collection over multiple conditions in one relation #526

Closed
tomaskavalek opened this issue Jul 28, 2021 · 15 comments · Fixed by #542
Closed

Filtering collection over multiple conditions in one relation #526

tomaskavalek opened this issue Jul 28, 2021 · 15 comments · Fixed by #542

Comments

@tomaskavalek
Copy link

Please help, is it possible to filter over multiple conditions in one relation? How to do that?

The target is to have all the records which have atribute with ID 1 and values 35 or 37, and attribute with ID 2 and values 36 or 28.

Code bellow doesn't solve the problem – in final query is only one LEFT JOIN, so condition is always false.

$this->findBy([
	ICollection:: AND,
	[
		'prices->currency' => $crcy,
	],
	[
		ICollection:: AND,
		'values->attribute->id' => 1,
		'values->id' => [35, 37],
	],
	[
		ICollection:: AND,
		'values->attribute->name' => 2,
		'values->id' => [36, 28],
	]
]);

Is there any way to do that?

@stpnkcrk
Copy link
Contributor

stpnkcrk commented Jul 28, 2021

AND is the default when finding, so you can skip these. Also, multiple findBy() methods might be chained, again, AND would be used. Lastly, you talk about ID in cases of both attributes, but in the second one, you're looking for values->attribute->name, might that be a problem?

If I were to write such a query, I'd try something like this. Maybe that would help?

$this->findBy([
    'prices->currency' => $crcy,
])->findBy([
    'values->attribute->id' => 1,
    'values->id' => [35, 37],
])->findBy([
    'values->attribute->id' => 2,
    'values->id' => [36, 28],
]);

There might also be an issue with your model definition, but that's hard to guess without seeing it.

@tomaskavalek
Copy link
Author

Thanks – values->attribute->name it was just a typo, because I modified the code to not be with czech names.

Unfortunately, your solution doesn`t work – I have already tried this.

My model (Product don't have all properties) – if it should help:

/**
 * Product
 *
 * @property int $id {primary}
 * @property ManyHasMany|AttributeValue[] $values {m:m AttributeValue::$products, isMain=true}
 */
class Product extends Entity
{
}
/**
 * AttributeValue
 *
 * @property int $id {primary}
 * @property string $value
 * @property Attribute $attribute {m:1 Attribute::$values}
 * @property ManyHasMany|Product[] $products {m:m Product::$values}
 */
class AttributeValue extends Entity
{
}
/**
 * Attribute
 *
 * @property int $id {primary}
 * @property string $name
 * @property OneHasMany|AttributeValue[] $values {1:m AttributeValue::$attribute}
 */
class Attribute extends Entity
{
}

@tomaskavalek
Copy link
Author

It generates query like this:

SELECT `products`.*
FROM `products` AS `products`
LEFT JOIN `prices` AS `prices` ON (`products`.`id` = `prices`.`product_id`)
LEFT JOIN `products_x_attributes_values` AS `products_x_attributes_values` ON (`products`.`id` = `products_x_attributes_values`.`product_id`)
LEFT JOIN `attributes_values` AS `values` ON (`products_x_attributes_values`.`attribute_value_id` = `values`.`id`)
LEFT JOIN `attributes` AS `values_attribute` ON (`values`.`attribute_id` = `values_attribute`.`id`)
WHERE ((`prices`.`currency` = 'CZK'))
  AND ((`values_attribute`.`name` = 'Kategorie')
       AND (`values`.`id` IN (35)))
  AND ((`values_attribute`.`name` = 'Značka')
       AND (`values`.`id` IN (36)))
  AND (1=1)
GROUP BY `products`.`id`
;

But i expected:

SELECT `products`.*
FROM `products` AS `products`
LEFT JOIN `prices` AS `prices` ON (`products`.`id` = `prices`.`product_id`)
LEFT JOIN `products_x_attributes_values` AS `products_x_attributes_values` ON (`products`.`id` = `products_x_attributes_values`.`product_id`)
LEFT JOIN `attributes_values` AS `values` ON (`products_x_attributes_values`.`attribute_value_id` = `values`.`id`)
LEFT JOIN `attributes` AS `values_attribute` ON (`values`.`attribute_id` = `values_attribute`.`id`)
LEFT JOIN `products_x_attributes_values` AS `products_x_attributes_values2` ON (`products`.`id` = `products_x_attributes_values2`.`product_id`)
LEFT JOIN `attributes_values` AS `values2` ON (`products_x_attributes_values2`.`attribute_value_id` = `values2`.`id`)
LEFT JOIN `attributes` AS `values_attribute2` ON (`values2`.`attribute_id` = `values_attribute2`.`id`)
WHERE ((`prices`.`currency` = 'CZK'))
  AND ((`values_attribute`.`name` = 'Kategorie')
       AND (`values`.`id` IN (35)))
  AND ((`values_attribute2`.`name` = 'Značka')
       AND (`values2`.`id` IN (36)))
  AND (1=1)
GROUP BY `products`.`id`
;

@mabar
Copy link
Contributor

mabar commented Jul 28, 2021

I think your problem is related to fact that currently is supported only any filtering in to many relationships, not all nor none. It is extremely complicated to implement an universal solution and for now it's imho easier to just write sql the way you want it.

From what @hrach wrote to me, potential future implementation may look like this (currently is only implicit any supported, none of explicit):

  1. find authors with ANY books with ANY tag.name = 'Tag 1'
    • authors with some books about 'Tag 1'
    • $authors->findBy(['any(books)->any(tags)->name' => 'Tag 1'])
  2. find authors with ANY books with NONE tag.name = 'Tag 1'
    • authors with some books not about 'Tag 1'
    • $authors->findBy(['any(books)->none(tags)->name' => 'Tag 1'])
  3. find authors with ALL books with ANY tag.name = 'Tag 1'
    • authors with all books about 'Tag 1'
    • $authors->findBy(['all(books)->any(tags)->name' => 'Tag 1'])
    • $authors->findBy(['none(books)->any(tags)->name!=' => 'Tag 1'])
  4. find authors with ALL books with ALL tag.name = 'Tag 1'
    • authors with all books about only 'Tag 1'
    • $authors->findBy(['all(books)->all(tags)->name' => 'Tag 1'])
    • $authors->findBy(['none(books)->none(tags)->name!=' => 'Tag 1'])
  5. find authors with NONE books with ANY tag.name = 'Tag 1'
    • authors with no books about 'Tag 1'
    • $authors->findBy(['none(books)->any(tags)->name' => 'Tag 1'])
  6. find authors with NONE books with ALL tag.name = 'Tag 1'
    • authors with no books about only 'Tag 1'
    • $authors->findBy(['none(books)->all(tags)->name' => 'Tag 1'])

e.g. number 5 may generate following SQL, but it may even be impossible to generate an universal SQL or require postgre/mysql/sqlite-specific code

SELECT authors.*
FROM authors
LEFT JOIN (
    SELECT books.* 
    FROM books
    LEFT JOIN (
        SELECT books_x_tags.*
        FROM books_x_tags
        LEFT JOIN tags ON (books_x_tags.tag_id = tags.id)
        WHERE tags.name = 'Tag 1'
    ) books_x_tags ON (books.id = books_x_tags.book_id)
    GROUP BY books.id
    HAVING count(books_x_tags.*) > 0
) books ON (authors.id = books.author_id)
GROUP BY authors.id
HAVING COUNT(books.*) = 0

And that's NOT FUNNY.

@hrach
Copy link
Member

hrach commented Jul 28, 2021

Thanks for reporting this and Mabar for posting the related discussion we had on Slack. This is definitely something we wanna support but it is the most difficult thing I was ever designing.

@hrach hrach added this to the v5.0 milestone Jul 28, 2021
@tomaskavalek
Copy link
Author

OK, I will solve it with extra method in Mapper for now.

@hrach
Copy link
Member

hrach commented Dec 11, 2021

Ok, this got solved along with #541 (see more description of implemented changes).

The implementation of this rather experimental for now, but allow you to explicitly define an aggregation key.

By default, the joins are using AnyAggregation. To change the aggregation key, you have to pass the aggregation explicitly with custom aggregation key:

public function testIndependentSelects(): void
{
	$authors = $this->orm->authors->findBy([
		ICollection::AND,
		[
			ICollection::AND,
			new AnyAggregator('any1'),
			'books->title' => 'Book 1',
			'books->price->cents' => 50,
		],
		[
			ICollection::AND,
			new AnyAggregator('any2'),
			'books->title' => 'Book 2',
			'books->price->cents' => 150,
		],
	]);

	Assert::same(1, $authors->count());
	Assert::same(1, $authors->countStored());
}

If you can, I'd welcome feedback if it works for you and if I haven't broken anything else.

@stepapo
Copy link

stepapo commented Dec 14, 2021

Hi, I tried the dev version and the following now doesn't work (in both mysql and postgres) while in 4.0 it worked fine.

$books = $this->orm->books->findBy([
    'tags->id' => 1,
    'publisher->name' => 'Nextras publisher A'
]);
Assert::same($books->count(), 1);

MySQL: Unknown column 'publisher.name' in 'having clause'
Postgres: ERROR: column "publisher.name" must appear in the GROUP BY clause or be used in an aggregate function

It's probably solvable by using simple JOIN for publisher table and including publisher.name in ON clause.

@hrach
Copy link
Member

hrach commented Mar 6, 2022

Finally, Orm in main branch should support everything supported in v4.0. I'd really appreciate any testing if we haven't broken too much. cc @stepapo thx a lot.

@stepapo
Copy link

stepapo commented Mar 12, 2022

Hi @hrach thank you for the fix. In Postgres, it works great. Unfortunately in MySQL, it stops working as soon as you pick two columns with the same name from different tables.

$books = $this->orm->books->findBy([
	'tags->id' => 1,
	'author->name' => 'Writer 1',
	'publisher->name' => 'Nextras publisher A'
]);
Assert::same($books->count(), 1);

MySQL: Unknown column 'author.name' in 'having clause'

@hrach
Copy link
Member

hrach commented Mar 12, 2022

😢 Obviously, we had a pretty small test suite. I'll have to test on my project :/

@hrach
Copy link
Member

hrach commented Mar 12, 2022

Before implementing a fix I'm trying to understand the issue: https://dba.stackexchange.com/questions/308665/fqn-columns-clash-in-having-clause-unknown-column-error

@stepapo
Copy link

stepapo commented Mar 14, 2022

Despite the MySQL issue, I tried new aggregation features and it seems to be working great. With a custom helper I can use simple commands like this and they give desired results. I am looking forward to using this in my projects.

$this->orm->books->findBy(Has::any('tags->id', [1, 2])); // has any of tags 1 or 2
$this->orm->books->findBy(Has::any('tags->id', [1, 2, null])); // has any of tags 1 or 2 or has no tags
$this->orm->books->findBy(Has::all('tags->id', [1, 2])); // has all of tags 1 and 2
$this->orm->books->findBy(Has::exact('tags->id', [1, 2])); // has exactly tags 1 and 2 and no other
$this->orm->books->findBy(Has::exact('tags->id', null)); // has no tags
$this->orm->books->findBy(Has::none('tags->id', [1, 2])); // does not have tags 1 and 2
$this->orm->books->findBy(Has::none('tags->id', [1, null])); // does not have tag 1 and does not have no tags (has at least one tag)

The helper looks like this (surely could be rewritten in a better way, but works for me so far):

class Has
{
	public static function any(string $expression, $value): array
	{
		$value = $value === null ? [null] : (array) $value;
		if (in_array(null, $value, true)) {
			$value = array_filter($value, fn($v) => $v !== null);
			return count($value) > 0
				? [
					ICollection::OR,
					[$expression => $value],
					[CompareEqualsFunction::class, [CountAggregateFunction::class, $expression], 0],
				] : [CompareEqualsFunction::class, [CountAggregateFunction::class, $expression], 0];
		}
		return [$expression => $value];
	}

	public static function all(string $expression, $value): array
	{
		$result = [ICollection::AND];
		$value = $value === null ? [null] : (array) $value;
		foreach ($value as $v) {
			$result[] = $v === null
				? [CompareEqualsFunction::class, [CountAggregateFunction::class, $expression], 0]
				: [ICollection::AND, new AnyAggregator(Random::generate()), $expression => $v];
		}
		return $result;
	}

	public static function exact(string $expression, $value): array
	{
		$result = self::all($expression, $value);
		$value = $value === null ? [null] : (array) $value;
		$value = array_filter($value, fn($v) => $v !== null);
		$count = count($value);
		if ($count > 0) {
			$result[] = [CompareEqualsFunction::class, [CountAggregateFunction::class, $expression], $count];
		}
		return $result;
	}

	public static function none(string $expression, $value): array
	{
		$value = $value === null ? [null] : (array) $value;
		if (in_array(null, $value, true)) {
			$value = array_filter($value, fn($v) => $v !== null);
			return count($value) > 0
				? [
					ICollection::AND,
					[ICollection::AND, new NoneAggregator(), $expression => $value],
					[CompareGreaterThanFunction::class, [CountAggregateFunction::class, $expression], 0],
				] : [CompareGreaterThanFunction::class, [CountAggregateFunction::class, $expression], 0];
		}
		return [ICollection::AND, new NoneAggregator(), $expression => $value];
	}
}

@radimvaculik
Copy link
Contributor

@stepapo Could you share NoneAggregator and AnyAggregator classes as well? Thanks.

@stepapo
Copy link

stepapo commented Feb 12, 2023

@stepapo Could you share NoneAggregator and AnyAggregator classes as well? Thanks.

@radimvaculik Those are part of Nextras ORM under the namespace Nextras\Orm\Collection\Aggregations (in main branch)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants