Skip to content
This repository has been archived by the owner on Jan 28, 2021. It is now read-only.

Resolve tables with database qualifier #402

Closed
erizocosmico opened this issue Oct 5, 2018 · 2 comments
Closed

Resolve tables with database qualifier #402

erizocosmico opened this issue Oct 5, 2018 · 2 comments
Assignees
Labels
enhancement New feature or request hacktoberfest Ready to pick for Hacktoberfest

Comments

@erizocosmico
Copy link
Contributor

Right now, tables can't have database qualifiers. We should be able to resolve them with database qualifiers.

We should check if columns can also be qualified with the database. If they can, they should also be handled.

See: src-d/gitbase#361 (comment)

@erizocosmico erizocosmico added enhancement New feature or request hacktoberfest Ready to pick for Hacktoberfest labels Oct 5, 2018
@erizocosmico erizocosmico self-assigned this Oct 8, 2018
@erizocosmico
Copy link
Contributor Author

erizocosmico commented Oct 8, 2018

I've been working on this for a while and I realised something that was not obvious from the outside before I was refactoring deep inside the analyzer. There is a huge blocker for this task: schema does not have database information. So, after the QualifyColumns step, you lose that info and if there is any kind of ambiguity between tables of different databases, there's no way to choose between them.

Because we cannot for sure put the database into the sql.Column as it is right now (just like we should not put Source), I suggest we take a step back from this and rethink how we want to pass around the schema so this is fixable in the future.

For example, we could change the schema not to be a []sql.Column and instead be a struct that contains the columns []sql.Column, which would not have the Source anymore. Then, it can also contain column metadata, such as the table and database info.
Tables don't need to output the table name anymore. They would just have to return immutable schemas every time Schema is called. Then ResolvedTable adds that information and passes the schema along.
That way, we can add methods such as func (s Schema) Table(colIndex int) and func (s Schema) Database(colIndex int) to get info about the columns.

This, of course, is a huge refactor across both go-mysql-server and gitbase, which is not very aligned with the OKRs, so maybe we can just focus on this task on making it work for mysqldump, which is just resolve the table itself, and not columns, etc and assume there is no ambiguity. Then, when we have time we can tackle this.

WDYT @ajnavarro?

@ajnavarro
Copy link
Contributor

I think is a good idea. On SparkSQL they have UnresolvedAttribute that becomes AttributeReference on ResolveReferences rule (just if you want to have a look).

We can take advantage of this refactor and try to improve column and aliases resolution, that they are causing some problems.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request hacktoberfest Ready to pick for Hacktoberfest
Projects
None yet
Development

No branches or pull requests

2 participants