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

NH-3777: Replaced NHibernate.Loader.TopologicalSorter by a more performant impl... #419

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

TAkkerman
Copy link

...ementation.

  • Because of a slight change in the (internal) Interface of TopologicalSorter, JoinWalker had to be adjusted (note that the removed parameter i was identical to the return value for the current use of the sorter)

@TAkkerman TAkkerman changed the title - Replaced NHibernate.Loader.TopologicalSorter by a more performant impl... NH-3777: Replaced NHibernate.Loader.TopologicalSorter by a more performant impl... Apr 16, 2015
@TAkkerman
Copy link
Author

Well, it seems that the topological sort itself was alright, but there must be more restrictions on the order than those that are exposed to the sort algorithm. Will try to update to a variant that matches to the order the old algorithm produces when there are multiple valid solutions.

@hazzik
Copy link
Member

hazzik commented Feb 12, 2017

Hi @TAkkerman, can you please enable Allow edits from maintainers feature?

@TAkkerman
Copy link
Author

'allow edits from maintainers' should be active now.

Akkerman added 2 commits February 13, 2017 23:29
…rmant implementation.

- Because of a slight change in the (internal) Interface of TopologicalSorter, JoinWalker had to be adjusted (note that the removed parameter i was identical to the return value for the current use of the sorter)
…ological sort.

Before, the only constraints exposed to the topological sort where constraints that were found in the explicit "On"-SqlString of the underlying OuterJoinableAssociation and not the implicit dependency of the join itself (LHS depends on RHS)
@hazzik
Copy link
Member

hazzik commented Feb 13, 2017

@TAkkerman I've force pushed with rebase on top of master. I've cleaned up indents & namings.

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

Successfully merging this pull request may close these issues.

None yet

2 participants