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

SQL: fix function definition of ticket_dependee_missing #200

Merged
merged 2 commits into from
Feb 11, 2019

Conversation

a-tze
Copy link
Collaborator

@a-tze a-tze commented Jan 4, 2019

SQL: fix function definition of ticket_dependee_missing, fixes #199

@a-tze a-tze requested a review from pegro January 4, 2019 19:01
Copy link
Member

@pegro pegro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess the patch fixes the issue, but somehow I feel it would be more obvious to have "tbl_ticket dependee" as the FROM clause (and all the joins in reverse order), since dependee is used in SELECT, not depender (which is the now the FROM clause). If I'm not mistaken this should result in fewer LEFT JOINS, or am I missing something?

@a-tze
Copy link
Collaborator Author

a-tze commented Jan 15, 2019

Interesting suggesstion... I'll try to refactor the function and EXPLAIN it.

@a-tze
Copy link
Collaborator Author

a-tze commented Jan 15, 2019

Maybe you missed that "ep" and "epv" are also SELECTed, even before "tbl_ticket dependee" (of course this is only the view of an imperative programmer with the concept of lazy evaluation in mind and doesn't actually apply here).
However, if the order of joins is reversed, then "ep" and "epv" are at the end of the JOIN chain, nothing gained. The execution plan is almost identical for both versions, and the estimated costs are completely equal on my system (4.43..24.75).
I'm not sure if PostgreSQL would use some shortcuts when doing the actual query.

For me it was more obvious to "crawl" from the given depending ticket to the unknown dependee ticket.

Another approach to get rid of all those JOINS (in multiple places) would be to keep track of the dependee via a dedicated column in tbl_ticket. It is seldomly changed but read extremely often (view_servicable_tickets etc.)

@jjeising
Copy link
Member

Another approach to get rid of all those JOINS (in multiple places) would be to keep track of the dependee via a dedicated column in tbl_ticket.

If we do that we should combine the effort with other planed cached values and columns, see #167.

@a-tze a-tze force-pushed the 199-dependee-ticket branch from 219d2c5 to b25072d Compare January 15, 2019 22:24
@a-tze
Copy link
Collaborator Author

a-tze commented Jan 15, 2019

If we do that we should combine the effort with other planed cached values and columns, see #167.

Of course, but the intention of this PR was bugfixing.

@a-tze a-tze force-pushed the 199-dependee-ticket branch from b25072d to a7fe54a Compare February 10, 2019 14:16
@a-tze
Copy link
Collaborator Author

a-tze commented Feb 10, 2019

Turns out, the actual bugfix comes from not returning null, but changing the default return value to true was indeed wrong.

@a-tze a-tze requested a review from pegro February 10, 2019 14:17
Copy link
Member

@pegro pegro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be fine

@a-tze a-tze merged commit 80f11cf into master Feb 11, 2019
@a-tze a-tze deleted the 199-dependee-ticket branch February 11, 2019 22:10
@a-tze a-tze restored the 199-dependee-ticket branch May 24, 2019 10:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

The _status component shows "dependee ticket missing", but it's not.
3 participants