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

[gather_columns] Traversing function calls #14

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

john-bodley
Copy link

This PR augments the print_column_resolution_order method to also examine columns which are function calls, so queries of the form

SELECT SUM(foo) FROM a

associate the column foo with table a, i.e., produces the output,

Checking query:
select sum(foo) from a
[SingleColumn(expression=FunctionCall(name=QualifiedName(sum), distinct=False, arguments=[QualifiedNameReference(name=QualifiedName(foo))]))]
[Table(name=QualifiedName(a))]

Table Column Resolution:
QualifiedNameReference(name=QualifiedName(foo)): [Table(name=QualifiedName(a))]

Note that the column_name variable was unused and thus I deleted it.

@john-bodley john-bodley force-pushed the johnbodley-gather_columns-functional-call branch 4 times, most recently from 6f1ce7f to b22815a Compare March 26, 2017 07:02
@john-bodley john-bodley force-pushed the johnbodley-gather_columns-functional-call branch from b22815a to 90062f6 Compare March 26, 2017 07:04
@brianv0
Copy link
Member

brianv0 commented Mar 27, 2017

I'm happy to merge this as-is, but for the purposes of limited scope on this example, what do you think of checking the function call name to see that it's in one of pre-defined ones in SQL, e.g.: AVG, MAX, MIN, SUM,COUNT? I only say this because I know that we miss expressions of the form select sum(10*a) from foo. Those require a slightly more complicated visitor (or an additional one) I think, but we could work on that.

@john-bodley
Copy link
Author

@brianv0 I'm happy to augment the PR to include a whitelist of function names.

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