-
Notifications
You must be signed in to change notification settings - Fork 304
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
HPCC-31454 Enable Parquet interface in client tools #18562
HPCC-31454 Enable Parquet interface in client tools #18562
Conversation
Jira Issue: https://hpccsystems.atlassian.net//browse/HPCC-31454 Jirabot Action Result: |
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.
Looks good, I just have one question.
plugins/parquet/CMakeLists.txt
Outdated
@@ -1,6 +1,6 @@ | |||
############################################################################## | |||
|
|||
# HPCC SYSTEMS software Copyright (C) 2022 HPCC Systems®. | |||
# HPCC SYSTEMS software Copyright (C) 2022 HPCC Systems�. |
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.
Is this change correct?
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.
I didn't make that change myself, and I assumed that some editor was fixing it. On my system, it now looks like:
HPCC SYSTEMS software Copyright (C) 2022 HPCC Systems®.
That is Unicode character 00AE.
I just compared it with the same character in plugins/kafka/CMakeLists.txt and it is the same (00AE).
I think it's correct, just not rendered correctly here.
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.
Ok, that makes sense.
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.
I just realized that, in my previous comment, I was comparing characters in a difference git branch. After switching to the right branch I see that this character is wrong.
I will revert this change in this file.
Shared library is not available in client tools, but it can at least be referenced in ECL code.
dc3bba5
to
97b0560
Compare
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.
Looks good.
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.
Changes look ok - does this mean that parquet will be recognised in the clienttools, while not being included?
Exactly. It only affects generic disk read code, and specifically whether a file type of 'parquet' is recognized. With this PR, that will happen even though the Parquet shared libraries are not linked. |
@ghalliday Please merge. |
The Parquet shared library is not available in client tools, but parquet (the file type) can at least be referenced in ECL code when using the new generic disk reader code.
Type of change:
Checklist:
Smoketest:
Testing: