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

custom JOINs are being added above core JOINS in elgg_get_entities_from_annotation_calculation? #8580

Closed
propertunist opened this Issue Jun 24, 2015 · 19 comments

Comments

Projects
None yet
4 participants
@propertunist

propertunist commented Jun 24, 2015

as is documented in this community thread, when i provide a JOIN parameter to elgg_get_entities_from_annotation_calculation that references an alias that is created in the core code, an SQL error is thrown:

https://community.elgg.org/discussion/view/2223883/changing-the-sequence-of-where-clauses-in-elgg-get-entities-from-annotation-calculation

this appears to be because my join is being used in the final query above/before the core JOIN that contains the alias that i am referencing. as matt stated, the custom JOINs would be more appropriately added after the core JOINs.

@hypeJunction

This comment has been minimized.

Show comment
Hide comment
@hypeJunction

hypeJunction Jun 24, 2015

Contributor

Can confirm running into this
On Jun 24, 2015 8:57 PM, "ura soul" notifications@github.com wrote:

as is documented in this community thread, when i provide a JOIN parameter
to elgg_get_entities_from_annotation_calculation that references an alias
that is created in the core code, an SQL error is thrown:

https://community.elgg.org/discussion/view/2223883/changing-the-sequence-of-where-clauses-in-elgg-get-entities-from-annotation-calculation

this appears to be because my join is being used in the final query
above/before the core JOIN that contains the alias that i am referencing.
as matt stated, the custom JOINs would be more appropriately added after
the core JOINs.


Reply to this email directly or view it on GitHub
#8580.

Contributor

hypeJunction commented Jun 24, 2015

Can confirm running into this
On Jun 24, 2015 8:57 PM, "ura soul" notifications@github.com wrote:

as is documented in this community thread, when i provide a JOIN parameter
to elgg_get_entities_from_annotation_calculation that references an alias
that is created in the core code, an SQL error is thrown:

https://community.elgg.org/discussion/view/2223883/changing-the-sequence-of-where-clauses-in-elgg-get-entities-from-annotation-calculation

this appears to be because my join is being used in the final query
above/before the core JOIN that contains the alias that i am referencing.
as matt stated, the custom JOINs would be more appropriately added after
the core JOINs.


Reply to this email directly or view it on GitHub
#8580.

@propertunist

This comment has been minimized.

Show comment
Hide comment
@propertunist

propertunist Jun 24, 2015

what did you use as a workaround?

propertunist commented Jun 24, 2015

what did you use as a workaround?

@hypeJunction

This comment has been minimized.

Show comment
Hide comment
@hypeJunction

hypeJunction Jun 24, 2015

Contributor

Writing a custom query was inefficient, I just created a metadata that is
updated when annotations are created/updated/deleted.
On Jun 24, 2015 9:34 PM, "ura soul" notifications@github.com wrote:

what did you use as a workaround?


Reply to this email directly or view it on GitHub
#8580 (comment).

Contributor

hypeJunction commented Jun 24, 2015

Writing a custom query was inefficient, I just created a metadata that is
updated when annotations are created/updated/deleted.
On Jun 24, 2015 9:34 PM, "ura soul" notifications@github.com wrote:

what did you use as a workaround?


Reply to this email directly or view it on GitHub
#8580 (comment).

@hypeJunction

This comment has been minimized.

Show comment
Hide comment
@hypeJunction

hypeJunction Jun 24, 2015

Contributor

I mean I didn't find one. I needed to also list entities with no
annotations, and I couldn't build a query with custom joins/selects using
ege*, partially because of that bug
On Jun 24, 2015 10:00 PM, "Ismayil Khayredinov" info@hypejunction.com
wrote:

Writing a custom query was inefficient, I just created a metadata that is
updated when annotations are created/updated/deleted.
On Jun 24, 2015 9:34 PM, "ura soul" notifications@github.com wrote:

what did you use as a workaround?


Reply to this email directly or view it on GitHub
#8580 (comment).

Contributor

hypeJunction commented Jun 24, 2015

I mean I didn't find one. I needed to also list entities with no
annotations, and I couldn't build a query with custom joins/selects using
ege*, partially because of that bug
On Jun 24, 2015 10:00 PM, "Ismayil Khayredinov" info@hypejunction.com
wrote:

Writing a custom query was inefficient, I just created a metadata that is
updated when annotations are created/updated/deleted.
On Jun 24, 2015 9:34 PM, "ura soul" notifications@github.com wrote:

what did you use as a workaround?


Reply to this email directly or view it on GitHub
#8580 (comment).

@propertunist

This comment has been minimized.

Show comment
Hide comment
@propertunist

propertunist Jun 24, 2015

aha ok. i think in my case i could use a custom query, but i think i will just disable the feature that uses this lookup until the elgg core is fixed as it is non essential. thanks

propertunist commented Jun 24, 2015

aha ok. i think in my case i could use a custom query, but i think i will just disable the feature that uses this lookup until the elgg core is fixed as it is non essential. thanks

@propertunist

This comment has been minimized.

Show comment
Hide comment
@propertunist

propertunist Dec 7, 2015

so i have come up against this same bug in a different way now. any chance of a fix for this in core?

propertunist commented Dec 7, 2015

so i have come up against this same bug in a different way now. any chance of a fix for this in core?

@mrclay

This comment has been minimized.

Show comment
Hide comment
@mrclay

mrclay Dec 8, 2015

Member

Why not try to dig in and fix it?

Member

mrclay commented Dec 8, 2015

Why not try to dig in and fix it?

@propertunist

This comment has been minimized.

Show comment
Hide comment
@propertunist

propertunist Dec 8, 2015

mainly because:
a) i am not familiar enough with elgg core to feel that i am not wasting my time ;)
b) i am locked out of github due to a problem with ssh / netbeans on my dev machine (so i haven't uploaded anything for months).. neither github support or netbeans forums could figure out the problem.
c) i've got a long list of non core things to fix.

none the less, i will look and see what i can find.

propertunist commented Dec 8, 2015

mainly because:
a) i am not familiar enough with elgg core to feel that i am not wasting my time ;)
b) i am locked out of github due to a problem with ssh / netbeans on my dev machine (so i haven't uploaded anything for months).. neither github support or netbeans forums could figure out the problem.
c) i've got a long list of non core things to fix.

none the less, i will look and see what i can find.

@mrclay

This comment has been minimized.

Show comment
Hide comment
@mrclay
Member

mrclay commented Dec 8, 2015

@propertunist

This comment has been minimized.

Show comment
Hide comment
@propertunist

propertunist Dec 11, 2015

so one of the fixes is needed here: https://github.com/Elgg/Elgg/blob/1.12/engine/lib/metastrings.php#L219

i ammended that line to become:

array_unshift($joins, "JOIN {$db_prefix}entities e ON n_table.entity_guid = e.guid");

and that fixed one of my problems.
now i need to find the other place where a similar change is needed to allow my other query to run correctly. i think it is in an annotation file, but i haven't found it yet.

propertunist commented Dec 11, 2015

so one of the fixes is needed here: https://github.com/Elgg/Elgg/blob/1.12/engine/lib/metastrings.php#L219

i ammended that line to become:

array_unshift($joins, "JOIN {$db_prefix}entities e ON n_table.entity_guid = e.guid");

and that fixed one of my problems.
now i need to find the other place where a similar change is needed to allow my other query to run correctly. i think it is in an annotation file, but i haven't found it yet.

@propertunist

This comment has been minimized.

Show comment
Hide comment
@propertunist

propertunist Dec 11, 2015

seems i have been able to resolve my two issues here with just one change to elgg core. :)
so i will move on to figuring out how to upload to github again!

propertunist commented Dec 11, 2015

seems i have been able to resolve my two issues here with just one change to elgg core. :)
so i will move on to figuring out how to upload to github again!

@propertunist

This comment has been minimized.

Show comment
Hide comment
@propertunist

propertunist May 30, 2016

i now have github working locally (after reinstalling the OS) and have also found another correction that is needed in relation to this original issue. the same/similar fix is needed for the river file too:

$joins[] = "JOIN {$dbprefix}entities oe ON rv.object_guid = oe.guid";

becomes

array_unshift($joins, "JOIN {$db_prefix}entities oe ON rv.object_guid = oe.guid");

in engine/lib/river.php

propertunist commented May 30, 2016

i now have github working locally (after reinstalling the OS) and have also found another correction that is needed in relation to this original issue. the same/similar fix is needed for the river file too:

$joins[] = "JOIN {$dbprefix}entities oe ON rv.object_guid = oe.guid";

becomes

array_unshift($joins, "JOIN {$db_prefix}entities oe ON rv.object_guid = oe.guid");

in engine/lib/river.php

@propertunist

This comment has been minimized.

Show comment
Hide comment
@propertunist

propertunist May 30, 2016

if i create a pull request for this does it need to go into 2.x ? i ask because i am using 12.x and would prefer the fix to go into 12.x and above and am not sure of the protocol here.

propertunist commented May 30, 2016

if i create a pull request for this does it need to go into 2.x ? i ask because i am using 12.x and would prefer the fix to go into 12.x and above and am not sure of the protocol here.

@mrclay

This comment has been minimized.

Show comment
Hide comment
@mrclay

mrclay May 31, 2016

Member

Fix 1.12 if the problem exists there.

Member

mrclay commented May 31, 2016

Fix 1.12 if the problem exists there.

propertunist pushed a commit to propertunist/Elgg that referenced this issue May 31, 2016

ura soul
fix(query handling): optional joins now added to river and metastring…
… queries after default joins to allow default joins to be referenced by optional joins.

    When building more complex queries there may be a need to reference table aliases introduced by the default joins in _elgg_get_metastring_based_objects and elgg_get_river. This fix ensures that the default join table aliases are available when defining custom joins - which is a requirement for building effective and efficient queries.

    Fixes #8580
@propertunist

This comment has been minimized.

Show comment
Hide comment
@propertunist

propertunist May 31, 2016

had some issues with git / github and ended up somehow with two commits and i am unsure how to delete the first one.

propertunist commented May 31, 2016

had some issues with git / github and ended up somehow with two commits and i am unsure how to delete the first one.

@propertunist

This comment has been minimized.

Show comment
Hide comment
@propertunist

propertunist May 31, 2016

hmm.. travis failed the commit due to the commit message, except as far as i know the commit message is in the required format.

propertunist commented May 31, 2016

hmm.. travis failed the commit due to the commit message, except as far as i know the commit message is in the required format.

@beck24

This comment has been minimized.

Show comment
Hide comment
@beck24

beck24 May 31, 2016

Member

I think the parameter in the brackets should only be one word, and there may be a length limit to the message title.

try:

fix(river): custom joins added after default joins

Member

beck24 commented May 31, 2016

I think the parameter in the brackets should only be one word, and there may be a length limit to the message title.

try:

fix(river): custom joins added after default joins

@propertunist

This comment has been minimized.

Show comment
Hide comment
@propertunist

propertunist May 31, 2016

ok, that was my only thought on it too. unfortunately, i am not up to the level of super magician with github and have no idea how to properly revoke the two commits i have already made. the first commit was made from a fork that i then deleted because it over-wrote my local elgg file structure (which was a nightmare in itself!) and so now i have no local version of that fork from which to act against github's reference to it.
is there a command along the lines of: git undo noob errors ?

propertunist commented May 31, 2016

ok, that was my only thought on it too. unfortunately, i am not up to the level of super magician with github and have no idea how to properly revoke the two commits i have already made. the first commit was made from a fork that i then deleted because it over-wrote my local elgg file structure (which was a nightmare in itself!) and so now i have no local version of that fork from which to act against github's reference to it.
is there a command along the lines of: git undo noob errors ?

propertunist pushed a commit to propertunist/Elgg that referenced this issue May 31, 2016

ura soul
fix(query-handling): optional joins now added to river and metastring…
… queries after default joins to allow default joins to be referenced by optional joins.

    When building more complex queries there may be a need to reference table aliases introduced by the default joins in _elgg_get_metastring_based_objects and elgg_get_river. This fix ensures that the default join table aliases are available when defining custom joins - which is a requirement for building effective and efficient queries.

    Fixes #8580

propertunist pushed a commit to propertunist/Elgg that referenced this issue May 31, 2016

ura soul
fix(river): optional joins now added to river and metastring queries …
…after default joins to allow default joins to be referenced by optional joins.

    When building more complex queries there may be a need to reference table aliases introduced by the default joins in _elgg_get_metastring_based_objects and elgg_get_river. This fix ensures that the default join table aliases are available when defining custom joins - which is a requirement for building effective and efficient queries.

    Fixes #8580

propertunist pushed a commit to propertunist/Elgg that referenced this issue May 31, 2016

ura soul
fix(river): optional joins now added to river and metastring queries …
…after default joins to allow default joins to be referenced by optional joins.

When building more complex queries there may be a need to reference table aliases introduced by the default joins in _elgg_get_metastring_based_objects
 and elgg_get_river. This fix ensures that the default join table aliases are available when defining custom joins
- which is a requirement for building effective and efficient queries.

    Fixes #8580

propertunist pushed a commit to propertunist/Elgg that referenced this issue May 31, 2016

ura soul
fix(river): optional joins now added to river and metastring queries …
…after default joins to allow

default joins to be referenced by optional joins.

When building more complex queries there may be a need to reference table aliases introduced
by the default joins in _elgg_get_metastring_based_objects and elgg_get_river. This fix ensures
that the default join table aliases are available when defining custom joins
- which is a requirement for building effective and efficient queries.

    Fixes #8580

propertunist added a commit to propertunist/Elgg that referenced this issue Jun 1, 2016

fix(river): optional joins now added to river and metastring queries …
…after default joins to allow

default joins to be referenced by optional joins.

When building more complex queries there may be a need to reference table aliases introduced
by the default joins in _elgg_get_metastring_based_objects and elgg_get_river. This fix ensures
that the default join table aliases are available when defining custom joins
- which is a requirement for building effective and efficient queries.

Fixes #8580

propertunist pushed a commit to propertunist/Elgg that referenced this issue Jun 1, 2016

ura soul
fix(river): custom joins can now reference default joined tables.
When building more complex queries there may be a need to reference table aliases introduced
by the default joins in _elgg_get_metastring_based_objects and elgg_get_river. This fix ensures
that the default join table aliases are available when defining custom joins
- which is a requirement for building effective and efficient queries.

Fixes #8580
@propertunist

This comment has been minimized.

Show comment
Hide comment
@propertunist

propertunist Jun 15, 2016

this has now been resolved

propertunist commented Jun 15, 2016

this has now been resolved

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