orm icon indicating copy to clipboard operation
orm copied to clipboard

`INSTANCE OF` support in Criteria API Expressions

Open skornev opened this issue 9 years ago • 28 comments

You can you use "Instance" of in DQL and QB. Maybe I missed something but it looks like there is no way to use it in Expression?

skornev avatar Jun 30 '16 13:06 skornev

@skornev ->andWhere('a INSTANCEOF B')

Ocramius avatar Jun 30 '16 13:06 Ocramius

It does not work.

An exception has been thrown during the rendering of a template ("Catchable Fatal Error: Argument 1 passed to Doctrine\Common\Collections\Criteria::andWhere() must implement interface Doctrine\Common\Collections\Expr\Expression, string given

skornev avatar Jun 30 '16 14:06 skornev

I want to use Criteria::create() ->where(Criteria::expr()->eq("isFolder", "1"))->andWhere('a INSTANCEOF OF B)

skornev avatar Jun 30 '16 14:06 skornev

Ah, I thought this was about DQL only (and the DQL expression builder), not about the criteria API, sorry. Re-opening and renaming issue.

Ocramius avatar Jul 01 '16 01:07 Ocramius

+1 for an instanceOf() Expression in the Criteria API.

SInCE avatar Jul 06 '16 08:07 SInCE

Yes please!

JeromeGill avatar Jul 07 '16 17:07 JeromeGill

it would be awesome!

massimilianobraglia avatar Jul 08 '16 16:07 massimilianobraglia

:+1: Yes please.

greydnls avatar Sep 07 '16 16:09 greydnls

+1

imunhatep avatar Oct 27 '16 14:10 imunhatep

+1

jonbev avatar Dec 08 '16 16:12 jonbev

+1

martinsifra avatar Jan 08 '17 19:01 martinsifra

+1

dc-dstepanov avatar Jan 18 '17 14:01 dc-dstepanov

+1 Definitely! 👍

trickeyone avatar Mar 02 '17 21:03 trickeyone

+1 (and nothing since june 2016 ???)

tvequaud avatar Mar 07 '17 08:03 tvequaud

Yep, nothing since then.

On 7 Mar 2017 3:47 a.m., "Thomas Véquaud" [email protected] wrote:

+1 (and nothing since june 2016 ???)

— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub https://github.com/doctrine/doctrine2/issues/5908#issuecomment-284658480, or mute the thread https://github.com/notifications/unsubscribe-auth/AAJakI22Iz3iMJ8l89l47GUewz8OAC47ks5rjRmYgaJpZM4JCJiN .

Ocramius avatar Mar 07 '17 18:03 Ocramius

So sad....

pixels-farm avatar May 23 '17 12:05 pixels-farm

+1

courtney-miles avatar Jun 02 '17 02:06 courtney-miles

The interested has been clearly shown already so stop +1 and do a PR if you're interested...

theofidry avatar Jun 02 '17 10:06 theofidry

so stop +1

Apologies -- my +1 was not intended to aggravate anyone.

Some projects embrace +1 as a way to gauge interest in features in the absence of voting.

courtney-miles avatar Jun 02 '17 10:06 courtney-miles

@theofidry I made a brief research on this issue. "INSTANCE OF" expression is based on Entity annotations and QueryBuilder, which resolves expressions to DQL. But Criteria expressions resolved to raw SQL. As I understand, Criteria expression resolver (BAsicEntityPersister) should be rewritten to use QueryBuilder. That would be a huge PR and I doubt that it will pass CR by Doctrine team.

imunhatep avatar Jun 02 '17 13:06 imunhatep

@Imunhatep thanks for looking into it. I guess then it's up to the core team to determine if this can be done in 3.x in which case it can be added to the 3.0 milestone or if this feature won't be implemented

theofidry avatar Jun 02 '17 13:06 theofidry

I were thinking a little bit on this problem and probably i was wrong. There is no need to rewite Context expression resolver to use QueryBuilder it would be overkill. A simplier approach would be just add a similar logic for resolving INSTANCE OF expression from QueryBuilder to Context expression resolver. Or to not duplicate business logic.. Move INSTANCE OF expression resolving logic to a separate handler. And add filtering for collections. It's still not an easy task.. But at least possible.

imunhatep avatar Jun 02 '17 18:06 imunhatep

Hey, two years have gone since the last post. Any news about this issue?

I stumbled upon the same problem as described in https://github.com/doctrine/collections/issues/113 and didn't find another smart solution: Cannot use the discriminator column directly ("Unrecognized field"), cannot use the specific properties of the subclasses ("Unrecognized field" as they are ManyToOne-relations which isn't supported by the Criteria), cannot use instanceOf, cannot hydrate the whole Collection to filter it (it can be very huge in my case as it is an activity log).

How did others solve that? For now I avoid the Collection and have a repository method to get the count of log entries of a specific type. This is suboptimal, because all my other entities can handle their isDeletable() state on their own counting their restricting relations.

spackmat avatar Jun 11 '19 09:06 spackmat

@spackmat Unfortunately, v1.5 implemented strict checking which makes my solution from #113 not work any longer. I submitted #168 which has been added to 2.0 milestone. Hopefully we'll get a fix soon. For my projects, I've had to lock them at 1.4 to keep this functionality.

trickeyone avatar Jun 11 '19 10:06 trickeyone

I did a dirty workaround in my case, since I only wanted to enable a user to be deleted, if it only has activities editing a User entity (itself in most cases) and preventing deletion if it has activities of any other type. So when there are less than 20 activities, I hydrate them all with a filter() and when there are 20 or more, I assume that there will be at least one of another type and the user cannot be deleted. So if one creates a new user, logs in as this user and edits this user 20 times, it is incorrectly considered undeletable and thus can only be hidden. I can live with that.

spackmat avatar Jun 11 '19 10:06 spackmat

Of course Pull Requests are always welcome and your time and effort is appreciated.

SenseException avatar Jun 11 '19 21:06 SenseException

any news there?

shmaltorhbooks avatar Nov 25 '24 00:11 shmaltorhbooks

any news there?

@shmaltorhbooks , if you wish, you could check out the PR and rebase the branch over 2.3.x, as it has already been confirmed that using visit method would be more appropriate than instanceof checks

rela589n avatar Dec 29 '24 19:12 rela589n