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

ex fb ads: support ad_id in primary key #79

Merged
merged 2 commits into from
Oct 9, 2019
Merged

Conversation

kacurez
Copy link
Member

@kacurez kacurez commented Oct 3, 2019

If query path(aka endpoint) is empty and there is ad_id in the columns then ad_id will be also contained in the primary key of the resulting table. It is needed for queries such as:

path: "" (empty string)
fields:

insights.time_range({"since":"2019-09-01","until":"2019-09-30"}).level(ad).time_increment(1){account_id,account_name, ad_name, ad_id,impressions,clicks,spend}

detais here https://keboola.slack.com/archives/C1B7SH7T2/p1569917984001200

@kacurez kacurez requested a review from pivnicek October 3, 2019 21:33
Copy link

@pivnicek pivnicek left a comment

Choose a reason for hiding this comment

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

Lisp blows my mind, but I get what's going on I think:)

lgtm

@kacurez
Copy link
Member Author

kacurez commented Oct 7, 2019

Yes your are right about the ad_id is only add to PK if the path is root. I will merge after I notify owners of those couple of configs that will be affected. thanks!

@kacurez kacurez merged commit 9f60c3d into master Oct 9, 2019
@kacurez kacurez deleted the kacurez-ad-id-pk branch October 9, 2019 14:45
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