-
Notifications
You must be signed in to change notification settings - Fork 151
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
add PARTITION BY option for CopyInto #431
Conversation
CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅ |
I have read the CLA Document and I hereby sign the CLA |
recheck |
99e4eae
to
b36cff7
Compare
3783fdf
to
b36cff7
Compare
can anyone please take a look at this? |
what's the process for this repo considering and accepting PR's? should we assume we should maintain our own forks long-term? |
annual bump =/ |
@sfc-gh-dszmolka @sfc-gh-astus @sfc-gh-mraba Any chance we could get a review on this? We're still maintaining our own fork of this library so that we can use this feature. |
hi @afavaro and everyone; first of all, i regret it takes such a long time for the team to review this PR. I don't have any explanation which can be shared publicly. However what I think could work, if you please reach out to your Snowflake account teams and express how this PR is important to you. This could help getting more attention on this PR, and hopefully, this product in general. Thank you in advance and I'm really sorry for this. |
Please answer these questions before submitting your pull requests. Thanks!
What GitHub issue is this PR addressing? Make sure that there is an accompanying issue to your PR.
Fixes SNOW-878116:
CopyInto
does not support thePARTITION BY
clause #430Fill out the following pre-review checklist:
Please describe how your code solves the related issue.
This adds an argument to
CopyInto
which can be used to specify a string value forPARTITION BY
. I have a few questions about implementation:CopyInto.copy_options
? If so, is there any particular validation to make sense?