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.

Sign up for GitHub

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

Jump to bottom

Feature/planop parameter serialization #341

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Feature/planop parameter serialization #341

wants to merge 6 commits into from

Conversation

lanice
Copy link

@lanice lanice commented Apr 3, 2014

Adds serializable parameter structs for PlanOperations.

@lanice
Add custom cereal archive for hyrise
6ed1d56 
* use develop branch of cereal to support name-value pairs for loading
* add Json::Value and std::optional as serializable types (load only)
* add a serialization macro
@lanice
Add cereal_construct to Queryparser, add parameter struct and seriali…
7414940 
…zation to TableLoad and SimpleTableScan.
@lanice
merge master into planop_parameter_serialization
d734577
@hyrise-bot
Copy link

Can one of the admins verify this patch?

@bastih
Copy link
Contributor

bastih commented Apr 3, 2014

add to whitelist

@hyrise-bot
Copy link

Test failed.
Refer to this link for build results: http://devdojo.de:5555/job/hyrise-multiconf-pull/742/

Build Log
last 20 lines

[...truncated 9 lines...]
 * [new ref]         refs/pull/341/head -> refs/pull/341/head
 * [new ref]         refs/pull/341/merge -> refs/pull/341/merge
   0b5f7a2..1ff6523  master     -> github/master
 * [new ref]         refs/pull/341/head -> github/pr/341
Success build forhudson.tasks.Shell@5c27910a
Fetching changes from the remote Git repository
Fetching upstream changes from /home/jenkins/hyrise_mirror
Checking out Revision 1d1d14405b4ba8960e1a6e7e6dd5d2d02409cc11 (detached)
First time build. Skipping changelog.
Cleaning workspace
Resetting working tree
Triggering debug,clang++34
Triggering debug,g++48
Triggering release,clang++34
Triggering release,g++48
debug,clang++34 completed with result FAILURE
debug,g++48 completed with result FAILURE
release,clang++34 completed with result FAILURE
release,g++48 completed with result FAILURE

@hyrise-bot
Copy link

Test PASSED!
Refer to this link for build results: http://devdojo.de:5555/job/hyrise-multiconf-pull/744/

bastih
bastih reviewed Apr 3, 2014
View reviewed changes
src/lib/access/storage/TableLoad.h
bool _binary;
bool _unsafe;
bool _raw;
std::optional<std::string> _header_file_name;
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we need to list the members of the parameter struct twice?

Copy link
Author

Choose a reason for hiding this comment

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

We wouldn't necessarily need them twice, we could just (as you suggested below) use the Parameters struct as member.

Copy link
Author

Choose a reason for hiding this comment

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

Done in fe50fff.

@hyrise-bot
Copy link

Test PASSED!
Refer to this link for build results: http://devdojo.de:5555/job/hyrise-multiconf-pull/753/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@lanice @hyrise-bot @bastih