Uploaded image for project: 'Calcite'
  1. Calcite
  2. CALCITE-469

README.md instruction for compile in example for csv doesn't work

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 1.0.0-incubating
    • Component/s: None
    • Labels:
      None

      Description

      mvn compile doesn't put the required files in the expected place for the tutorial. Change to 'mvn clean install'.

      I will provide the oneliner patch.

      1. CALCITE-469.patch
        2 kB
        Larry McCay
      2. OPTIQ-469.patch
        0.4 kB
        Larry McCay

        Activity

        Hide
        vladimirsitnikov Vladimir Sitnikov added a comment -

        I see more problems with CSV readme:
        1) It points to outdated git://github.com/julianhyde/optiq-csv.git
        2) Current https://github.com/apache/incubator-calcite depends on the SNAPSHOT version, thus to test the CVS one needs to build full Calcite.

        Taking 1&2 into account I think the proper way to test CSV adapter is as follows:
        1) Download Calcite from the github mirror
        2) mvn install -DskipTests -Dcheckstyle.skip=true
        3) cd examples/csv, etc.

        And I would like to ensure all queries are copy-pasteable. E.g. "sqlline>" does not appear in the middle of the query.

        Any objections/comments?

        Show
        vladimirsitnikov Vladimir Sitnikov added a comment - I see more problems with CSV readme: 1) It points to outdated git://github.com/julianhyde/optiq-csv.git 2) Current https://github.com/apache/incubator-calcite depends on the SNAPSHOT version, thus to test the CVS one needs to build full Calcite. Taking 1&2 into account I think the proper way to test CSV adapter is as follows: 1) Download Calcite from the github mirror 2) mvn install -DskipTests -Dcheckstyle.skip=true 3) cd examples/csv , etc. And I would like to ensure all queries are copy-pasteable. E.g. "sqlline>" does not appear in the middle of the query. Any objections/comments?
        Hide
        julianhyde Julian Hyde added a comment -

        Vladimir Sitnikov +1

        In (2), mvn install -DskipTests will suffice. Checkstyle doesn't take long. Fewer characters for someone to mis-type.

        Show
        julianhyde Julian Hyde added a comment - Vladimir Sitnikov +1 In (2), mvn install -DskipTests will suffice. Checkstyle doesn't take long. Fewer characters for someone to mis-type.
        Hide
        vladimirsitnikov Vladimir Sitnikov added a comment -

        Here's mvn clean install -DskipTests benchmark (MB 15", 2,6 GHz Intel Core i7, java 1.8):

        $ mvn clean install -DskipTests
        real	0m48.776s
        user	2m35.216s
        sys	0m8.338s
        
        $ mvn clean install -DskipTests -Dcheckstyle.skip=true
        real	0m33.752s
        user	1m43.380s
        sys	0m7.200s
        
        $ mvn clean install -DskipTests
        real	0m42.889s
        user	2m19.373s
        sys	0m7.837s
        
        $ mvn clean install -DskipTests -Dcheckstyle.skip=true
        real	0m30.275s
        user	1m37.801s
        sys	0m6.508s
        

        Probably for fast machines checkstyle is not a big deal.

        Larry McCay, Can you please prepare a step by step that will include usage of new github mirror?

        Show
        vladimirsitnikov Vladimir Sitnikov added a comment - Here's mvn clean install -DskipTests benchmark (MB 15", 2,6 GHz Intel Core i7, java 1.8): $ mvn clean install -DskipTests real 0m48.776s user 2m35.216s sys 0m8.338s $ mvn clean install -DskipTests -Dcheckstyle.skip=true real 0m33.752s user 1m43.380s sys 0m7.200s $ mvn clean install -DskipTests real 0m42.889s user 2m19.373s sys 0m7.837s $ mvn clean install -DskipTests -Dcheckstyle.skip=true real 0m30.275s user 1m37.801s sys 0m6.508s Probably for fast machines checkstyle is not a big deal. Larry McCay , Can you please prepare a step by step that will include usage of new github mirror?
        Hide
        lmccay Larry McCay added a comment -

        Sure.

        Show
        lmccay Larry McCay added a comment - Sure.
        Hide
        lmccay Larry McCay added a comment -

        Vladimir Sitnikov - I didn't find the copy and paste confusing or anything - so I'd like to make sure that I understand what you want.

        Multiple line copy that doesn't pick up the sqlline> prompt between lines - something like:

        Simple Model

        !connect jdbc:calcite:model=target/test-classes/model.json admin admin
        !tables
        !describe emps
        SELECT * FROM emps;
        EXPLAIN PLAN FOR SELECT * FROM emps;
        

        More Complex Model

        !connect jdbc:calcite:model=target/test-classes/smart.json admin admin
        EXPLAIN PLAN FOR SELECT * FROM emps;
        SELECT depts.name, count(*)
           FROM emps JOIN depts USING (deptno)
           GROUP BY depts.name;
        VALUES char_length('hello, ' || 'world!');
        

        Each of the above blocks can be copied and executed together from a paste.

        Show
        lmccay Larry McCay added a comment - Vladimir Sitnikov - I didn't find the copy and paste confusing or anything - so I'd like to make sure that I understand what you want. Multiple line copy that doesn't pick up the sqlline> prompt between lines - something like: Simple Model !connect jdbc:calcite:model=target/test-classes/model.json admin admin !tables !describe emps SELECT * FROM emps; EXPLAIN PLAN FOR SELECT * FROM emps; More Complex Model !connect jdbc:calcite:model=target/test-classes/smart.json admin admin EXPLAIN PLAN FOR SELECT * FROM emps; SELECT depts.name, count(*) FROM emps JOIN depts USING (deptno) GROUP BY depts.name; VALUES char_length('hello, ' || 'world!'); Each of the above blocks can be copied and executed together from a paste.
        Hide
        lmccay Larry McCay added a comment -

        Added CALCITE-469.patch for correcting URLs for github, for project pages at the bottom and removed the sqlline> prompt from the SQL examples.

        Show
        lmccay Larry McCay added a comment - Added CALCITE-469 .patch for correcting URLs for github, for project pages at the bottom and removed the sqlline> prompt from the SQL examples.
        Hide
        vladimirsitnikov Vladimir Sitnikov added a comment -

        While I reviewing the patch I came across travis-ci link in the top of examples/csv/readme.

        I'll remove the link for now, however we need some kind of CI to move forward.

        Show
        vladimirsitnikov Vladimir Sitnikov added a comment - While I reviewing the patch I came across travis-ci link in the top of examples/csv/readme. I'll remove the link for now, however we need some kind of CI to move forward.
        Hide
        vladimirsitnikov Vladimir Sitnikov added a comment -

        Here are the comments:
        1) cd examples/csv does not work. example should be used instead
        2) git clone was missing somehow

        Committed as https://git-wip-us.apache.org/repos/asf?p=incubator-calcite.git;a=commitdiff;h=539253562067dcea9609c766fa7b6f413e1ac001

        Show
        vladimirsitnikov Vladimir Sitnikov added a comment - Here are the comments: 1) cd examples/csv does not work. example should be used instead 2) git clone was missing somehow Committed as https://git-wip-us.apache.org/repos/asf?p=incubator-calcite.git;a=commitdiff;h=539253562067dcea9609c766fa7b6f413e1ac001
        Hide
        julianhyde Julian Hyde added a comment -

        Closing now that 1.0.0-incubating has been released.

        Show
        julianhyde Julian Hyde added a comment - Closing now that 1.0.0-incubating has been released.

          People

          • Assignee:
            vladimirsitnikov Vladimir Sitnikov
            Reporter:
            lmccay Larry McCay
          • Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development