You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@drill.apache.org by "ASF GitHub Bot (JIRA)" <ji...@apache.org> on 2017/10/11 04:28:01 UTC

[jira] [Commented] (DRILL-5783) Make code generation in the TopN operator more modular and test it

    [ https://issues.apache.org/jira/browse/DRILL-5783?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16199790#comment-16199790 ] 

ASF GitHub Bot commented on DRILL-5783:
---------------------------------------

GitHub user ilooner opened a pull request:

    https://github.com/apache/drill/pull/984

    DRILL-5783 Made a unit test for generated Priority Queue. DRILL-5841 Fix tmp folder errors.

    ## DRILL-5783
    
    - A unit test is created for the priority queue in the TopN operator
    - The code generation classes passed around a completely unused function registry reference in some places so I removed it.
    - The priority queue had unused parameters for some of its methods so I removed them.
    - I added an add method to value vectors so that value vectors can easily be generated in unit tests.
    
    ## DRILL-5841
    
    - There were many many ways in which temporary folders were created in unit tests. I have unified the way these folders are created with the DirTestWatcher, SubDirTestWatcher, and BaseDirTestWatcher. All the unit tests have been updated to use these. The test watchers create temp directories in ./target/<fully qualified test class name>/<method name>. So all the files generated and used in the context of a test can easily be found in the same consistent location.
    - This change should fix the sporadic hashagg test failures, as well as failures caused by stray files in /tmp
    
    Misc
    
    - Added a TableBuilder so we don't have to becom human json parsers to generate test data.
    - A Pstore test could fail sporadically
    - General code cleanup. 
    - There are many places where String.format is used unnecessarily. The test builder methods already use String.format for you when you pass them args. I cleaned some of these up.

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/ilooner/drill DRILL-5783

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/drill/pull/984.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #984
    
----
commit c9c554340a4a2ca8bab257a478d8201cd5f4c92a
Author: Timothy Farkas <ti...@apache.org>
Date:   2017-09-18T23:21:44Z

     - Added property for codegen dump.
     - Removed unnecessary FunctionRegistry argument to the code generator.

commit 68f77a2e9a904bcd5a808fefd824f2f108134aec
Author: Timothy Farkas <ti...@apache.org>
Date:   2017-09-18T23:31:38Z

     - Removed unused imports

commit eff908b4b18fe5ccb67480f6e0c36a61e27d12e2
Author: Timothy Farkas <ti...@apache.org>
Date:   2017-09-18T23:34:33Z

     - Removed unnecessary modifiers from the priority queue interface.

commit 319fae58faaf78f807d8d6f9c0928f801ec7e352
Author: Timothy Farkas <ti...@apache.org>
Date:   2017-09-19T00:28:58Z

     - Made creating a priority queue more modular by refactoring the createNewPriorityQueue method

commit e82e316a628e3c2f3a1f9ef7005299065eb4379d
Author: Timothy Farkas <ti...@apache.org>
Date:   2017-09-19T21:08:19Z

     - Have some codegen dump test code done

commit e2057d6c9abc301889ab46a12848cee7506f45d5
Author: Timothy Farkas <ti...@apache.org>
Date:   2017-09-21T22:07:52Z

     - Made fix

commit 6289cff376f37c8d4e2984b309d7f6eb13703f2f
Author: Timothy Farkas <ti...@apache.org>
Date:   2017-09-28T23:24:38Z

     - Removed unused imports
     - Removed commented out code
     - Formatting fixes

commit cb80f885c86a00cda82e114b3aec212890ce6d90
Author: Timothy Farkas <ti...@apache.org>
Date:   2017-09-29T23:03:49Z

     - Added the option to the DirTestWatcher to not delete the tmp dir at the end of the test
     - Added a TableFileBuilder to be used in testing
     - Changed the External sort test to use the TableFileBuilder

commit bb58b21ad16d6adbeb11c66d5a38d11c20a0b05d
Author: Timothy Farkas <ti...@apache.org>
Date:   2017-09-29T23:06:15Z

     - Delete the directory at the beginning of a test in DirTestWatcher

commit f0d89abf2dc3422081bcd49183b8bf0ae3742996
Author: Timothy Farkas <ti...@apache.org>
Date:   2017-09-30T01:51:13Z

     - Used existing test framework for the TopNBatchTest

commit 7f8d978c12d0e2b2efd3b3c64f5450ca76f0ef0e
Author: Timothy Farkas <ti...@apache.org>
Date:   2017-10-05T06:27:55Z

     - Removed passing around contexts unnecessarily in the TopNBatch operator.
     - Fixed some javadocs

commit c539ebd20c421d8672440ad3bb863c446f39e775
Author: Timothy Farkas <ti...@apache.org>
Date:   2017-10-06T03:22:04Z

    - Added a record batch builder, and added an add method to value vector mutators.

commit 89a5f8576bd2f50ce540b633a28ace55b31f08e6
Author: Timothy Farkas <ti...@apache.org>
Date:   2017-10-06T05:46:48Z

     - Added Javadoc to the PriorityQueue.
     - Removed an unnecessary method
     - Got the unit test to work

commit 540715d250601590367d7bc71614ab52e891c6ec
Author: Timothy Farkas <ti...@apache.org>
Date:   2017-10-06T10:27:46Z

     - Added utilities for comparing vector batch results
     - Removed redundant unit tests

commit 9b0c557ec0c6030969dae620c5954e5dfb2119e9
Author: Timothy Farkas <ti...@apache.org>
Date:   2017-10-06T10:30:40Z

     - Reduced the amount of data generated for the topN unit test.

commit 7ed3dcf7bf0ce807b81e72a17d91d909ac788415
Author: Timothy Farkas <ti...@apache.org>
Date:   2017-10-06T16:14:21Z

     - Moved comparator function in DrillTestWrapper into a comparator class
     - Made the TopN unit test work

commit 3e9d8905699a56c36cbdbf253daf374ae0983e65
Author: Timothy Farkas <ti...@apache.org>
Date:   2017-10-06T16:20:03Z

     - Fixed missing license

commit 55e35ab8a84b9eb32fdf1b674a23a3f45e1e9e55
Author: Timothy Farkas <ti...@apache.org>
Date:   2017-10-06T16:31:07Z

     - Fixed checkstyle errors

commit 0c310225d47a78a35c9cafd70869b7520edfa17c
Author: Timothy Farkas <ti...@apache.org>
Date:   2017-10-07T04:00:00Z

     - Fixed javadoc errors in FixtureBuilder

commit 133a44b88355de031695cf8d9c3503618219e4fe
Author: Timothy Farkas <ti...@apache.org>
Date:   2017-10-07T04:10:32Z

     - Cleaning up unused variables and methods in FixtureBuilder

commit a685b5718f2c25d1797d378bdf35be9a88559048
Author: Timothy Farkas <ti...@apache.org>
Date:   2017-10-07T04:34:33Z

     - Fixed errors after rebase

commit b920eb3ed4e29cf72a74ceaf43bc4c418f4dcefa
Author: Timothy Farkas <ti...@apache.org>
Date:   2017-10-07T09:55:10Z

     - Used DirTestWatcher to create temporary directories in the ClusterFixture
     - Fixed broken unit tests

commit 96defcfcee6b602df0051e7ec8516ef627a44388
Author: Timothy Farkas <ti...@apache.org>
Date:   2017-10-08T00:57:32Z

     - Made the RecordBatchBuilder and BatchUtils use the same table format as the DrillTestWrapper. Also added a more detailed comparison function for tables.

commit 8812043bfed74c6f0f6db1a23c0d029b4fd29222
Author: Timothy Farkas <ti...@apache.org>
Date:   2017-10-08T00:59:40Z

     - Moved files in testutils package to test package

commit 1b1dada9c712c9283b4765cb7142af967ba5dd24
Author: Timothy Farkas <ti...@apache.org>
Date:   2017-10-08T01:02:10Z

     - Moved more testing utilities into the org.apache.drill.test package

commit 5878073cf0ebb4d28a6da6fb4594ea52576d5b1d
Author: Timothy Farkas <ti...@apache.org>
Date:   2017-10-08T01:05:35Z

     - Fixed broken javadoc links

commit d9ac7d485b6e580c7becf6caa8dc96ebe20465ed
Author: Timothy Farkas <ti...@apache.org>
Date:   2017-10-08T01:08:27Z

     - Moved unit tests out of org.apache.drill.test since that package should only contain testing utilities

commit fe33936818d1b170651e6ea36b560d06af0033e7
Author: Timothy Farkas <ti...@apache.org>
Date:   2017-10-10T09:19:24Z

     - Made all the tests use a uniform tmp directory system
     - Fixed sporadic failures due to stale files being present on build machines
     - Use an isolated workspace for each test not global /tmp folder
     - Fixed test failures due to stray files in /tmp

commit dd8a2c230008739f20432389cb61515c6459eb05
Author: Timothy Farkas <ti...@apache.org>
Date:   2017-10-10T11:07:41Z

     - Fixed broken tests

commit 99115492fd621403d0862e2ff8b650479a5935e5
Author: Timothy Farkas <ti...@apache.org>
Date:   2017-10-10T13:07:03Z

     - Fixed more testing bugs

----


> Make code generation in the TopN operator more modular and test it
> ------------------------------------------------------------------
>
>                 Key: DRILL-5783
>                 URL: https://issues.apache.org/jira/browse/DRILL-5783
>             Project: Apache Drill
>          Issue Type: Improvement
>            Reporter: Timothy Farkas
>            Assignee: Timothy Farkas
>




--
This message was sent by Atlassian JIRA
(v6.4.14#64029)