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

Fix relation column expression processing #3390

Merged
merged 1 commit into from
Feb 4, 2025

Conversation

gs-rpant1729
Copy link
Contributor

@gs-rpant1729 gs-rpant1729 commented Feb 3, 2025

What type of PR is this?

Bug Fix

What does this PR do / why is it needed ?

Filter post joins on Relations led to invalid SQL being generated.

This change also updates isolateTdsSelect to return a subselect with TableAliasColumnNames instead of just ColumnNames which led to loss of column origin info. The generated SQLs using this function still remain semantically identical. However, lots of tests assert on the SQL, so lots of test updates :(

Which issue(s) this PR fixes:

Fixes #

Other notes for reviewers:

Does this PR introduce a user-facing change?

@gs-rpant1729 gs-rpant1729 requested a review from a team as a code owner February 3, 2025 15:23
Copy link

github-actions bot commented Feb 3, 2025

Test Results

     775 files   -    302       775 suites   - 302   46m 42s ⏱️ - 47m 21s
  8 037 tests  - 4 799    7 932 ✔️  - 4 750  104 💤  - 50  1 +1 
12 580 runs   - 6 332  12 475 ✔️  - 6 283  104 💤  - 50  1 +1 

For more details on these failures, see this check.

Results for commit 61a244f. ± Comparison against base commit e48edf2.

@gs-ssh16 gs-ssh16 changed the title fix relation filter bug and isolateTdsSelect Fix relation column expression processing Feb 4, 2025
@gs-ssh16 gs-ssh16 merged commit e5aaa67 into finos:master Feb 4, 2025
5 of 6 checks passed
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.

3 participants