[GitHub] drill issue #788: DRILL-5318: Sub-operator test fixture

2017-04-25 Thread jinfengni
Github user jinfengni commented on the issue:

https://github.com/apache/drill/pull/788
  
It's fine to have example in a follow-up PR.

I was trying to see if I can use new operator/sub-operator test framework 
for the schema change task I'm doing. That requires to work with one or 
multiple RecordBatch (both data and operator).  


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] drill issue #788: DRILL-5318: Sub-operator test fixture

2017-04-25 Thread paul-rogers
Github user paul-rogers commented on the issue:

https://github.com/apache/drill/pull/788
  
One of the key reasons for creating tests is that they show how to use an 
API. In this case, the associated tests show how to use the row set 
abstractions to create a schema, populate a row set and so on. Please also see 
the package-info.java file for Javadoc that provides an overview.

Please let me know if you want to use the test framework. If so, I'd like 
to work with you to figure out what additional documentation and/or examples 
would be useful.

This PR is a dependency on the "main show": a PR with refactored sort code 
and complete sort unit tests. That PR will be submitted this week now that all 
the prerequisites are in master. That will demonstrate now to use the framework 
to create a suite of unit tests. I can point to my personal repo if if it would 
be helpful to preview that code.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] drill issue #788: DRILL-5318: Sub-operator test fixture

2017-04-20 Thread paul-rogers
Github user paul-rogers commented on the issue:

https://github.com/apache/drill/pull/788
  
Revised the commit header. Note that the commit messages also identifies 
that this PR is dependent on DRILL-5323, and so this PR must be committed after 
DRILL-5323.

Sorry for the confusion. The work was broken into multiple commits to ease 
the burden on code reviewers. Unfortunately, there is no free lunch, and the 
multiple commits puts a large burden on the submitter to keep all branches in 
sync, and on the committer to pull the PRs in the correct order.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] drill issue #788: DRILL-5318: Sub-operator test fixture

2017-04-20 Thread paul-rogers
Github user paul-rogers commented on the issue:

https://github.com/apache/drill/pull/788
  
Please see the initial comment which identifies that this PR depends on 
several others. Two have been done, only DRILL-5323 remains. Please commit 
DRILL-5323 first, then this one.

Note that, per your request, this PR is rebased onto DRILL-5323. The 
history here includes all changes from DRILL-5323 plus additional changes 
specific to DRILL-5318. As I understand it, if you first commit DRILL-5323 the 
common changes will reside in master, and will be ignored when you then (as a 
second step) commit this PR.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] drill issue #788: DRILL-5318: Sub-operator test fixture

2017-04-20 Thread sudheeshkatkam
Github user sudheeshkatkam commented on the issue:

https://github.com/apache/drill/pull/788
  
I am confused by your comment. Which should be committed first, 5318 or 
5323? Your comments says 5318, but the commit message for 5318 and the fact 
that 5318 is based on 5323 suggests 5323 should be committed first?

Please format the commit message in 70fdc88 so it starts with "DRILL-: 
..."


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] drill issue #788: DRILL-5318: Sub-operator test fixture

2017-04-20 Thread paul-rogers
Github user paul-rogers commented on the issue:

https://github.com/apache/drill/pull/788
  
Rebased onto DRILL-5318 so that this branch includes the DRILL-5318 code 
that this PR depends upon, as well as the additional code required for this PR. 
Fixed the merge errors that caused the compilation issue.

Please commit DRILL-5318 first. Then, commit this PR. Given that this PR is 
already rebased on 5318, this PR should merge cleanly into master.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] drill issue #788: DRILL-5318: Sub-operator test fixture

2017-04-19 Thread sudheeshkatkam
Github user sudheeshkatkam commented on the issue:

https://github.com/apache/drill/pull/788
  
This does not compile. Please fix the errors and squash the commits into 
one.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] drill issue #788: DRILL-5318: Sub-operator test fixture

2017-04-19 Thread sudheeshkatkam
Github user sudheeshkatkam commented on the issue:

https://github.com/apache/drill/pull/788
  
+1


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] drill issue #788: DRILL-5318: Sub-operator test fixture

2017-04-03 Thread sohami
Github user sohami commented on the issue:

https://github.com/apache/drill/pull/788
  
Thanks for the changes. LGTM. +1


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---