Skip to content

Conversation

aschackmull
Copy link
Contributor

Semantically this is a noop, but making AccessPath more obviously equal to TAccessPath means that the optimiser is able to eliminate a bunch of type checks that otherwise interferes badly with the join order in the standard order in pathStep (they cause the usual prev-before-delta mess).

@aschackmull aschackmull added the no-change-note-required This PR does not need a change note label May 12, 2022
@aschackmull aschackmull requested review from a team as code owners May 12, 2022 12:35
* tracked object. The final type indicates the type of the tracked object.
*/
abstract private class AccessPath extends TAccessPath {
private class AccessPath extends TAccessPath {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can I conclude from this that when I'm creating a QL class that wraps an IPA type, it's better (from the compiler's point of view) to make the type non-abstract? Or is there something special about the AccessPath type that means I can't apply this in general?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, you can indeed conclude that.

Copy link
Contributor

Choose a reason for hiding this comment

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

A candidate for a QL4QL query.

Copy link
Contributor

@hvitved hvitved left a comment

Choose a reason for hiding this comment

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

LGTM. I have started a DCA run.

@aschackmull aschackmull merged commit 83f817c into github:main May 16, 2022
@aschackmull aschackmull deleted the dataflow/perf-std-order branch May 16, 2022 08:05
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.

4 participants