Skip to content
This repository has been archived by the owner on Aug 2, 2022. It is now read-only.

Support JOIN without table alias #274

Merged

Conversation

abbashus
Copy link
Contributor

@abbashus abbashus commented Nov 7, 2019

*Issue #232 *

Description of changes:
This pull request adds the ability to have JOIN queries without table alias given all the fields belong to either table but not common to both.

Testing:

  • Passing Gradle build and existing unit and integration test
  • Added new integration tests

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

public void rewrite(SQLQueryExpr root) {

final Multimap<String, Table> tableByFieldName = ArrayListMultimap.create();
final Set<String> tableNameAndAlias = new HashSet<>();
Copy link
Contributor

Choose a reason for hiding this comment

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

How is tableNameAndAlias used? I don't see what it's used for other than just storing the table name and alias which doesn't seem like it's necessary to create the Set for

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right, good catch. I used it previously for a special case, and removed that after refactoring, Will remove it.

String tableName = tableExpr.getExpr().toString().replaceAll(" ", "").split("/")[0];

if (tableExpr.getAlias() == null) {
// Could we not directly use table name as alias?
Copy link
Contributor

Choose a reason for hiding this comment

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

If the table join with itself, the alias will be duplicate. not sure wheter the syntax or semantic check this before reach here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For self join, the alias will not be duplicate as we generate different alias for each table without alias. I have added more checks to handle joins with same tables on either side. Added more integration tests as well.

Copy link
Contributor

@penghuo penghuo left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@davidcui1225 davidcui1225 left a comment

Choose a reason for hiding this comment

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

LGTM

@abbashus abbashus merged commit 517362f into opendistro-for-elasticsearch:master Nov 8, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants