Do not remove LEFT JOIN if there is table hint.

[sdanyliv]

Wrong JOIN optimization with Temporal Tables.

[sdanyliv]

/azp run test-all

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[jods4]

@sdanyliv out of curiosity, what's the underlying issue?
I don't see why it would be wrong to remove the LEFT JOIN in the test case you created?

Is it a cardinality problem with temporal ranges (your test case uses AS OF, so only it should return only 1 row)?

[sdanyliv]

@jods4. with temporal tables here can be duplicates of Primary Key. Removing joins is not an option in that case. We can simplify restriction for TemporalTableAsOf, but not for TemporalTableAll. Decided to disable optimization for any hints right now.

[jods4]

Ok so this is indeed a cardinality issue?
Join on table PK assumes cardinality of 1 but this could be many.

This fix feels like a hack. There might be other optimisations or transformations that look at cardinality and they would all be incorrect as well.
It'd be better if we could modify the cardinality to say "potentially many" when applying temporal modifiers (except AS OF).

If you merge this PR, it'd be really nice to leave a comment on the new condition to explain why it's there so we can review it in the future.
It would also be nice to create a test case that would actually fail: using something other than AS OF and with data that would return the wrong number of results.

[sdanyliv]

It is question to current hint implementation. I've just noticed that we have issue with this one and in production it may lead in problems. Since I have no time to work with this right now - disabled optimization for tables with hints. Will add comment about that when tests are passed.

[jods4]

@sdanyliv I see this was merged, please don't forget to add a comment so we remember why this was added and could potentially refactor it in the future.

[sdanyliv]

Yeah, I even tried to include Cardinality and give up. Current Query Hint realization do not give this possibility.

[jods4]

@sdanyliv Understood, no problem if you need a fix for a bug in production.

Just for long-term maintenance I think we need to remember to add a test that would fail if this is removed and a comment to explain for what specific case we keep LEFT JOIN with hints.