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

Use an explicit list of "placeholder" tables instead of a magical substring #46

Open
crisptrutski opened this issue May 30, 2024 · 1 comment
Labels
bug Something isn't working good first issue Good for newcomers

Comments

@crisptrutski
Copy link
Collaborator

When analyzing Metabase queries with references to logical views (aka models), Macaw is passed a query with "sentinel" names in place of actual tables. In order for find-replace to work correctly, we treat such qualifiers as if they were missing, so that we can cascade down to the entry qualified by the actual table.

This trick currently relies on Metabase's naming convention, and could have false positives, or break if this convention changes.

A more robust solution would be to take a set of placeholder names to be treated like this. Checking membership in this set should also be faster than searching for substrings too.

@crisptrutski crisptrutski added bug Something isn't working good first issue Good for newcomers labels May 30, 2024
@crisptrutski
Copy link
Collaborator Author

We can "red green" this by adding a test with what would currently be a false positive.

We should be able to pass this set via the opts map, perhaps under a key called something like :placeholder-tables.

In future we may want to replace this set with a mapping to the fully qualified columns (and their aliases) that are exposed by these placeholders. Or we could just do that from the start. Note that even regular tables could populate this list, which could fix some previously ambiguous cases.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

1 participant