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

[SQL] Join queries can fail if column PK names are the same across tables #2002

Closed
seanstory opened this issue Dec 20, 2023 · 5 comments
Closed

Comments

@seanstory
Copy link
Member

Bug Description

An advanced sync rules query like:

SELECT TOP 10 Bird.Name, Chicken.Food FROM Bird INNER JOIN Chicken ON Chicken.TagId = Bird.Id;

Should work, and we expect Joins to be one of the most common use cases for Advanced Sync Rules on SQL connectors. However, this type of Advanced Sync Rule leads to warnings in the logs like:

[FMWK][06:31:05][WARNING] [Connector id: u****************L, index name: search-m********, Sync job id: **************] Skipping custom query for tables Bird, Chicken as there are multiple tables with same primary key column name.

Blocks like

if has_duplicates(primary_key_columns):
self._logger.warning(
f"Skipping custom query for tables {format_list(tables)} as there are multiple primary key columns with the same name. Consider using 'AS' to uniquely identify primary key columns from different tables."
)
return

are in all our SQL connectors, but it's unclear to me what the purpose is, and why we're checking uniqueness for table PKs.

Both Paul and the discuss user have validated that using AS in the query to rename selected fields does not help.

To Reproduce

  1. create a SQL db with two tables that both have a PK on an ID column
  2. create a connector for that database
  3. setup advanced sync rules to join your two tables on ID
  4. note that your sync results in no data

Expected behavior

You should be able to use advanced sync rules for joins, even if the PK columns have the same name.

Environment

8.11-8.13

Additional context

Slack thread: https://elastic.slack.com/archives/C01795T48LQ/p1702570480692019

@seanstory
Copy link
Member Author

Related issue: #467

@seanstory
Copy link
Member Author

@timgrein is this still on your radar? Looks like MSSQL and Oracle are still incomplete?

@timgrein
Copy link
Contributor

timgrein commented Feb 22, 2024

PostgreSQL fix: #2007
MySQL fix: #2005

@timgrein
Copy link
Contributor

MSSQL fix: #2209

@timgrein
Copy link
Contributor

timgrein commented Mar 7, 2024

Closing as all PRs for PostgreSQL, MySQL and MSSQL were merged

@timgrein timgrein closed this as completed Mar 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants