-
Notifications
You must be signed in to change notification settings - Fork 46
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 new hive _push
method that supports cli operation and table properties
#43
Conversation
omniduct/databases/hiveserver2.py
Outdated
temp_dir = tempfile.mkdtemp(prefix='omniduct_hiveserver2') | ||
tmp_fname = os.path.join(temp_dir, 'data_{}.csv'.format(time.time())) | ||
logger.info('Saving dataframe to file... {}'.format(tmp_fname)) | ||
df.fillna(r'\N').to_csv(tmp_fname, index=False, header=False, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
does r'\N' get parsed as NULL by hive later?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. r'\N' is the character sequence interpreted as NULL (by default).
omniduct/databases/hiveserver2.py
Outdated
; | ||
""").render(**locals()) | ||
|
||
print(cmd) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
print statement
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed.
omniduct/databases/hiveserver2.py
Outdated
'U': 'STRING', # Unicode | ||
'V': 'STRING' # void | ||
} | ||
sep = sep or chr(1) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you comment on why this character? probably should be moved up to the other defaults (above the DTYPE map)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
chr(1)
is the CTRL-A character, which is the default separator used by Hive when tables are stored as text files. It's a way better choice than r'\t'
, since the tab character is not at all infrequent in strings that one might want to store.
Overall LGTM. FYI I'm working on getting the pandas to_sql faster and that should be in soonish |
@danfrankj Does your upstream work in pandas affect Hive, or just Presto? I mean, old versions of Hive don't even have the INSERT statement ;). |
@matthewwardrop df.to_sql will (after the upstream change) check the sqlalchemy dialect for |
@danfrankj That part I understood... just wondering whether to make the CLI approach the default for Hive or not. Does Hive via |
661cfda
to
fc6588f
Compare
fc6588f
to
1730995
Compare
I think it's time we added the CLI operation for hive in upstream Omniduct, given that it is by far the fastest (and only, for older versions of Hive) method for directly getting data from pandas DataFrame objects into Hive. There's a few edge-cases I haven't covered here; such as removing the columns associated with partition keys when exporting the DataFrame to CSV. Is there anything else I missed?
@danfrankj