init

[SerheyDolgushev]

Avoiding JOIN in extended filter to fix MySQL Query Error 1054 (http://stackoverflow.com/questions/10556505/mysql-query-error-1054) on ezp installations without https://github.com/ezsystems/ezpublish-legacy/commit/24bd6d8b3992236a4b5f47852cf335b28b8d50f0

[emodric]

Hi!

I'm afraid I can't accept this. Inner join was added by pull request #68 for a reason: to fix https://jira.ez.no/browse/EZP-19158.

[SerheyDolgushev]

Ah... i see. Probably in the extended filter we should check ezp version, and if it contains https://jira.ez.no/browse/EZP-19158 use join, otherwise join should not be used. What do you think about it?

[emodric]

Hm... Maybe, if you can correctly check for eZ Publish Enterprise as well as Community Edition.

[crevillo]

@emodric has more to say here, but seems weird to me doing a check of the version inside a deep function like this...

[emodric]

Yep, it does. That's why i'm still skeptical.

[SerheyDolgushev]

Probably ini setting (enabled by default). JOIN will be used in extended filter, If it this setting is enabled. Otherwise JOIN will be not used in it. It will be possible to disable it on old installations. What do you think about this approach?

[emodric]

@L0rdJ That could work. eztags.ini/[BackwardsCompatibilitySettings]/UseJoinsInExtendedAttributeFilter true/false for example

[emodric]

@L0rdJ Go for it :)

[SerheyDolgushev]

Done. Please check it.

[SerheyDolgushev]

Please, check one more time.

[crevillo]

+1. even @emodric is your man here.

[emodric]

I can't check at the moment if this works as is should.

If you guys checked and verified both variants, I'll merge it.

[SerheyDolgushev]

I checked only UseJoinsInExtendedAttributeFilter=disabled case. And it works pretty well on old ezp installations. On old ezp installations i`m getting the same error as before for UseJoinsInExtendedAttributeFilter=enabled.

[andrerom]

Isn't this a bit BC settings overkill? Wouldn't it be better to say versions containing the join requirers a patched / recent version of eZ Publish?

[SerheyDolgushev]

Sorry, but there are some old ezp installations with lot of custom extensions. And it is not always possible to implement kernel patch without any pain (even if patch is more correct solution). This BC setting supposed to be used only in such cases.

[SerheyDolgushev]

BTW, tested on 2012 with UseJoinsInExtendedAttributeFilter=enabled (which is default) and it works fine.

[SerheyDolgushev]

Yep, sure, but there some projects where composer is not used. Also, probably there will be some new features in the upcoming releases of this extension. In both cases it will be easier just to clone the latest release and change this BC setting. Also this pull request shouldn't have any serious performance impacts.

[emodric]

I agree with @L0rdJ. Why not allow older installations which do not use composer to use newer eZ Tags versions?

[emodric]

BTW, tested on 2012 with UseJoinsInExtendedAttributeFilter=enabled (which is default) and it works fine.

Great, thanks! I'll merge it as soon as @andrerom agrees :)

[crevillo]

But are we Sure that all functionalities of ez tags work with older ezp versions? If so, from which versión?

[andrerom]

Great, thanks! I'll merge it as soon as @andrerom agrees :)

Not the approach we take in newer code, but sure go ahead.

[emodric]

Not the approach we take in newer code, but sure go ahead.

True, but I would argue there is a difference between developing an application and developing a plugin for application which can support various versions of the application. We had this same discussion in pull request about changing the name of the admin design ;)

[emodric]

PR merged.

Thanks everyone! (especially @crevillo for helping with code review)

[crevillo]

Ah. You're very welcome! I Have some time now i'm Holidays :)