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

Support dimension links with CROSS JOIN #1179

Merged
merged 3 commits into from
Oct 26, 2024

Conversation

shangyian
Copy link
Contributor

@shangyian shangyian commented Sep 26, 2024

Summary

This change supports using CROSS JOIN as the join type when creating a dimension join link. While we allow the CROSS join type to be passed in when creating dimension links, we don't actually support this join type because we don't allow for the join ON clause to be empty (typical in cross joins).

Test Plan

Deployment Plan

Copy link

netlify bot commented Sep 26, 2024

Deploy Preview for thriving-cassata-78ae72 canceled.

Name Link
🔨 Latest commit 880103c
🔍 Latest deploy log https://app.netlify.com/sites/thriving-cassata-78ae72/deploys/671c5b16d8d29700081e4499

@shangyian shangyian requested a review from anhqle October 26, 2024 03:52
@shangyian shangyian marked this pull request as ready for review October 26, 2024 03:52
@shangyian shangyian merged commit 64356e8 into DataJunction:main Oct 26, 2024
16 checks passed
@anhqle
Copy link
Contributor

anhqle commented Oct 26, 2024

While we allow the CROSS join type to be passed in when creating dimension links, we don't actually support this join type because we don't allow for the join ON clause to be empty (typical in cross joins).

hmm, so what new functionality are we gaining here?

@shangyian
Copy link
Contributor Author

@anhqle this is making it so that dimension links with join type CROSS actually work when generating SQL (so the generated SQL will have the cross join). This setup could mean that things like cross join to some window could be done only optionally when requested through dimension links.

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.

2 participants