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

Fixed bug with matching default table with alias #594

Merged
merged 4 commits into from
Oct 28, 2022

Conversation

Zhaars
Copy link
Contributor

@Zhaars Zhaars commented Oct 25, 2022

Currently, Acra expects that columns in SELECT statement haven't been aliased if FROM has a non-aliased table or they should have proper alias. But databases allow specific alias for table but use columns without an alias.

So if the query contains Columns without aliases and the table is aliased, Acra should consider this table as default. Added checking for containing aliases in tables expressions and switch between extracting tables functions depending on queries.

Checklist

Added checking if tables expression contains aliases to match with config settings
@Zhaars Zhaars requested a review from Lagovas October 25, 2022 12:41
@Zhaars Zhaars changed the title zhars/fixed_bug_with_default_table_with_alias Fixed bug with default table with alias Oct 25, 2022
dialect dialect.Dialect
}{
{
query: `SELECT "id", "email", "mobile_number" AS "mobileNumber" FROM "users" AS "User" where "User"."is_active"`,
Copy link
Collaborator

Choose a reason for hiding this comment

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

lets use a bit more complicated queries with several aliased tables in the FROM statement to be sure that we use first from the list

>
> Added more complicated tests/extend mapColumnsToAliases func with mapColumnsToAliases
>
> Added more complicated tests/extend mapColumnsToAliases func with mapColumnsToAliases
@Zhaars Zhaars changed the title Fixed bug with default table with alias Fixed bug with matching default table with alias Oct 26, 2022
Added integration test to cover matching tables with same fields
@Zhaars Zhaars merged commit d210d0c into master Oct 28, 2022
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