[GitHub] carbondata pull request #2807: [CARBONDATA-2997] Support read schema from in...

[qiuchenjian-2]

GitHub user xubo245 opened a pull request:

    https://github.com/apache/carbondata/pull/2807

    [CARBONDATA-2997] Support read schema from index file and data file in CSDK

    [CARBONDATA-2997] Support read schema from index file and data file in CSDK
   
    1.support readSchemaInIndexFile
    2.support readSchemaInDataFile
    3.support get field name and data type name
    4.suppport get array child element data type name
    5.can read schema when carbonreader has set ak,sk,endpoint
    6.TODO: need support read scehma from S3 in the future
   
    Be sure to do all of the following checklist to help us incorporate
    your contribution quickly and easily:
   
     - [ ] Any interfaces changed?
     add interface
     - [ ] Any backward compatibility impacted?
     No
     - [ ] Document update required?
    Yes
     - [ ] Testing done
        add test case
     - [ ] For large changes, please consider breaking it into sub-tasks under an umbrella JIRA.
    https://issues.apache.org/jira/browse/CARBONDATA-2951


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

    $ git pull https://github.com/xubo245/carbondata CARBONDATA-2997_supportReadSchema

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

    https://github.com/apache/carbondata/pull/2807.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 #2807
   
----
commit 380077653c7697855cb09f4541442560593d6e29
Author: xubo245 <xubo29@...>
Date:   2018-10-10T03:49:01Z

    [CARBONDATA-2997] Support read schema from index file and data file in CSDK
    1.support readSchemaInIndexFile
    2.support readSchemaInDataFile
    3.support get field name and data type name
    4.suppport get array child element data type name
    5.can read schema when carbonreader has set ak,sk,endpoint
    6.TODO: need support read scehma from S3 in the future

----


---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2807
 
    Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder2.1/759/



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2807
 
    Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder2.1/761/



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2807
 
    Build Success with Spark 2.2.1, Please check CI http://95.216.28.178:8080/job/ApacheCarbonPRBuilder1/959/



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2807
 
    Build Success with Spark 2.3.1, Please check CI http://136.243.101.176:8080/job/carbondataprbuilder2.3/9027/



---

[qiuchenjian-2]

Github user xubo245 commented on the issue:

    https://github.com/apache/carbondata/pull/2807
 
    @KanakaKumar @kunal642 @jackylk Please review it.


---

[qiuchenjian-2]

Github user ajantha-bhat commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/2807#discussion_r228406676
 
    --- Diff: docs/CSDK-guide.md ---
    @@ -126,12 +126,39 @@ bool readFromS3(JNIEnv *env, char *argv[]) {
         reader.close();
     }
     
    -// 3. destory JVM
    +// 3. read schema
    +/**
    --- End diff --
   
    This can be removed.
   
    As after #2792.  we just put link to main.cpp. No need to duplicate the code


---

[qiuchenjian-2]

Github user ajantha-bhat commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/2807#discussion_r228406995
 
    --- Diff: store/CSDK/CarbonSchemaReader.cpp ---
    @@ -0,0 +1,41 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements.  See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License.  You may obtain a copy of the License at
    + *
    + *    http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +#include "CarbonSchemaReader.h"
    +
    +CarbonSchemaReader::CarbonSchemaReader(JNIEnv *env) {
    +    this->carbonSchemaReaderClass = env->FindClass("org/apache/carbondata/sdk/file/CarbonSchemaReader");
    --- End diff --
   
    Add validation, env->FindClass cana return null.


---

[qiuchenjian-2]

Github user ajantha-bhat commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/2807#discussion_r228407125
 
    --- Diff: store/CSDK/CarbonSchemaReader.cpp ---
    @@ -0,0 +1,41 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements.  See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License.  You may obtain a copy of the License at
    + *
    + *    http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +#include "CarbonSchemaReader.h"
    +
    +CarbonSchemaReader::CarbonSchemaReader(JNIEnv *env) {
    +    this->carbonSchemaReaderClass = env->FindClass("org/apache/carbondata/sdk/file/CarbonSchemaReader");
    +    this->jniEnv = env;
    +}
    +
    +jobject CarbonSchemaReader::readSchemaInDataFile(char *path) {
    +    jmethodID buildID = jniEnv->GetStaticMethodID(carbonSchemaReaderClass, "readSchemaInDataFile",
    --- End diff --
   
    Add validation and for below get the exception from java using jni.
    Do this for all the newly added API


---

[qiuchenjian-2]

Github user ajantha-bhat commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/2807#discussion_r228409221
 
    --- Diff: store/CSDK/main.cpp ---
    @@ -55,6 +57,56 @@ JNIEnv *initJVM() {
         return env;
     }
     
    +/**
    + * test read Schema from Index File
    + *
    + * @param env jni env
    + * @return whether it is success
    + */
    +bool readSchemaInIndexFile(JNIEnv *env) {
    +    printf("\nread Schema from Index File:\n");
    +    CarbonSchemaReader carbonSchemaReader(env);
    +    jobject schema = carbonSchemaReader.readSchemaInIndexFile(
    +            "../resources/carbondata/510199997055746_batchno0-0-null-510199277323454.carbonindex");
    +    Schema carbonSchema(env, schema);
    +    int length = carbonSchema.getFieldsLength();
    +    printf("schema length is:%d\n", length);
    +    for (int i = 0; i < length; i++) {
    +        printf("%d\t", i);
    +        printf("%s\t", carbonSchema.getFieldName(i));
    +        printf("%s\n", carbonSchema.getFieldDataTypeName(i));
    +        if (strcmp(carbonSchema.getFieldDataTypeName(i), "ARRAY") == 0) {
    +            printf("Array Element Type Name is:%s\n", carbonSchema.getArrayElementTypeName(i));
    +        }
    +    }
    +    return true;
    +}
    +
    +/**
    + * test read Schema from Data File
    + *
    + * @param env jni env
    + * @return whether it is success
    + */
    +bool readSchemaInDataFile(JNIEnv *env) {
    +    printf("\nread Schema from Data File:\n");
    +    CarbonSchemaReader carbonSchemaReader(env);
    +    jobject schema = carbonSchemaReader.readSchemaInDataFile(
    --- End diff --
   
    do we have these binaries part-0-510199997055746_batchno0-0-null-510199277323454.carbondata? I think we should not keep any binaries in repo.
   



---

[qiuchenjian-2]

Github user ajantha-bhat commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/2807#discussion_r228409246
 
    --- Diff: store/CSDK/main.cpp ---
    @@ -168,9 +220,51 @@ bool readFromS3(JNIEnv *env, char *argv[]) {
             printf("\n");
         }
     
    +    printf("\nread Schema from Index File from S3:\n");
    +    CarbonSchemaReader carbonSchemaReader(env);
    +    jobject schema = carbonSchemaReader.readSchemaInIndexFile(
    +            "s3a://sdk/WriterOutput/carbondata/510199997055746_batchno0-0-null-510199277323454.carbonindex");
    --- End diff --
   
    do we have these binaries part-0-510199997055746_batchno0-0-null-510199277323454.carbondata? I think we should not keep any binaries in repo.


---

[qiuchenjian-2]

Github user ajantha-bhat commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/2807#discussion_r228409346
 
    --- Diff: store/CSDK/main.cpp ---
    @@ -168,9 +220,51 @@ bool readFromS3(JNIEnv *env, char *argv[]) {
             printf("\n");
         }
     
    +    printf("\nread Schema from Index File from S3:\n");
    +    CarbonSchemaReader carbonSchemaReader(env);
    +    jobject schema = carbonSchemaReader.readSchemaInIndexFile(
    +            "s3a://sdk/WriterOutput/carbondata/510199997055746_batchno0-0-null-510199277323454.carbonindex");
    +    Schema carbonSchema(env, schema);
    +    int length = carbonSchema.getFieldsLength();
    +    printf("schema length is:%d\n", length);
    +    for (int i = 0; i < length; i++) {
    +        printf("%d\t", i);
    +        printf("%s\t", carbonSchema.getFieldName(i));
    +        printf("%s\n", carbonSchema.getFieldDataTypeName(i));
    +        if (strcmp(carbonSchema.getFieldDataTypeName(i), "ARRAY") == 0) {
    +            printf("%s\n", carbonSchema.getArrayElementTypeName(i));
    +        }
    +    }
    +
         reader.close();
     }
     
    +/**
    + * test read Schema from Index File from S3
    + * TODO: need support in the future
    + *
    + * @param env jni env
    + * @return whether it is success
    + */
    +bool readSchemaInIndexFileFromS3(JNIEnv *env) {
    +    printf("\nread Schema from Index File:\n");
    +    CarbonSchemaReader carbonSchemaReader(env);
    +    jobject schema = carbonSchemaReader.readSchemaInIndexFile(
    +            "s3a://sdk/WriterOutput/carbondata/510199997055746_batchno0-0-null-510199277323454.carbonindex");
    --- End diff --
   
    same as above.
   



---

[qiuchenjian-2]

Github user ajantha-bhat commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/2807#discussion_r228409446
 
    --- Diff: store/CSDK/main.cpp ---
    @@ -168,9 +220,51 @@ bool readFromS3(JNIEnv *env, char *argv[]) {
             printf("\n");
         }
     
    +    printf("\nread Schema from Index File from S3:\n");
    +    CarbonSchemaReader carbonSchemaReader(env);
    +    jobject schema = carbonSchemaReader.readSchemaInIndexFile(
    +            "s3a://sdk/WriterOutput/carbondata/510199997055746_batchno0-0-null-510199277323454.carbonindex");
    +    Schema carbonSchema(env, schema);
    +    int length = carbonSchema.getFieldsLength();
    +    printf("schema length is:%d\n", length);
    +    for (int i = 0; i < length; i++) {
    +        printf("%d\t", i);
    +        printf("%s\t", carbonSchema.getFieldName(i));
    +        printf("%s\n", carbonSchema.getFieldDataTypeName(i));
    +        if (strcmp(carbonSchema.getFieldDataTypeName(i), "ARRAY") == 0) {
    +            printf("%s\n", carbonSchema.getArrayElementTypeName(i));
    +        }
    +    }
    +
         reader.close();
     }
     
    +/**
    + * test read Schema from Index File from S3
    + * TODO: need support in the future
    + *
    + * @param env jni env
    + * @return whether it is success
    + */
    +bool readSchemaInIndexFileFromS3(JNIEnv *env) {
    --- End diff --
   
    in main.cpp this testcase is commented. so remove here also


---

[qiuchenjian-2]

Github user ajantha-bhat commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/2807#discussion_r228410478
 
    --- Diff: store/sdk/src/test/java/org/apache/carbondata/sdk/file/CarbonSchemaReaderTest.java ---
    @@ -0,0 +1,233 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    --- End diff --
   
    a. No need to generate data for each testcase, can generate in beforeAll and all test case use same data.
    b. CarbonReaderTest has already covered scenarios mentioned in the below test. (both data and index file read)
   
    **so, this test case is not required. Please remove it.**



---

[qiuchenjian-2]

Github user ajantha-bhat commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/2807#discussion_r228410579
 
    --- Diff: store/CSDK/Schema.cpp ---
    @@ -0,0 +1,58 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements.  See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License.  You may obtain a copy of the License at
    + *
    + *    http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +#include <jni.h>
    +#include "Schema.h"
    +
    +Schema::Schema(JNIEnv *env, jobject schema) {
    --- End diff --
   
    same as other file.
   
    Add validation and for below get the exception from java using jni.
    Do this for all the newly added API


---

[qiuchenjian-2]

Github user ajantha-bhat commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/2807#discussion_r228411015
 
    --- Diff: store/CSDK/main.cpp ---
    @@ -55,6 +57,56 @@ JNIEnv *initJVM() {
         return env;
     }
     
    +/**
    + * test read Schema from Index File
    + *
    + * @param env jni env
    + * @return whether it is success
    + */
    +bool readSchemaInIndexFile(JNIEnv *env) {
    +    printf("\nread Schema from Index File:\n");
    +    CarbonSchemaReader carbonSchemaReader(env);
    +    jobject schema = carbonSchemaReader.readSchemaInIndexFile(
    +            "../resources/carbondata/510199997055746_batchno0-0-null-510199277323454.carbonindex");
    --- End diff --
   
    should not have binary in the repo. (index file)



---

[qiuchenjian-2]

Github user xubo245 commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/2807#discussion_r228562895
 
    --- Diff: docs/CSDK-guide.md ---
    @@ -126,12 +126,39 @@ bool readFromS3(JNIEnv *env, char *argv[]) {
         reader.close();
     }
     
    -// 3. destory JVM
    +// 3. read schema
    +/**
    --- End diff --
   
    yes, removed


---

[qiuchenjian-2]

Github user xubo245 commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/2807#discussion_r228570065
 
    --- Diff: store/CSDK/CarbonSchemaReader.cpp ---
    @@ -0,0 +1,41 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements.  See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License.  You may obtain a copy of the License at
    + *
    + *    http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +#include "CarbonSchemaReader.h"
    +
    +CarbonSchemaReader::CarbonSchemaReader(JNIEnv *env) {
    +    this->carbonSchemaReaderClass = env->FindClass("org/apache/carbondata/sdk/file/CarbonSchemaReader");
    --- End diff --
   
    ok, added.


---

[qiuchenjian-2]

Github user xubo245 commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/2807#discussion_r228570143
 
    --- Diff: store/CSDK/CarbonSchemaReader.cpp ---
    @@ -0,0 +1,41 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements.  See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License.  You may obtain a copy of the License at
    + *
    + *    http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +#include "CarbonSchemaReader.h"
    +
    +CarbonSchemaReader::CarbonSchemaReader(JNIEnv *env) {
    +    this->carbonSchemaReaderClass = env->FindClass("org/apache/carbondata/sdk/file/CarbonSchemaReader");
    +    this->jniEnv = env;
    +}
    +
    +jobject CarbonSchemaReader::readSchemaInDataFile(char *path) {
    +    jmethodID buildID = jniEnv->GetStaticMethodID(carbonSchemaReaderClass, "readSchemaInDataFile",
    --- End diff --
   
    ok, done


---

[qiuchenjian-2]

Github user xubo245 commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/2807#discussion_r228570298
 
    --- Diff: store/CSDK/main.cpp ---
    @@ -55,6 +57,56 @@ JNIEnv *initJVM() {
         return env;
     }
     
    +/**
    + * test read Schema from Index File
    + *
    + * @param env jni env
    + * @return whether it is success
    + */
    +bool readSchemaInIndexFile(JNIEnv *env) {
    +    printf("\nread Schema from Index File:\n");
    +    CarbonSchemaReader carbonSchemaReader(env);
    +    jobject schema = carbonSchemaReader.readSchemaInIndexFile(
    +            "../resources/carbondata/510199997055746_batchno0-0-null-510199277323454.carbonindex");
    +    Schema carbonSchema(env, schema);
    +    int length = carbonSchema.getFieldsLength();
    +    printf("schema length is:%d\n", length);
    +    for (int i = 0; i < length; i++) {
    +        printf("%d\t", i);
    +        printf("%s\t", carbonSchema.getFieldName(i));
    +        printf("%s\n", carbonSchema.getFieldDataTypeName(i));
    +        if (strcmp(carbonSchema.getFieldDataTypeName(i), "ARRAY") == 0) {
    +            printf("Array Element Type Name is:%s\n", carbonSchema.getArrayElementTypeName(i));
    +        }
    +    }
    +    return true;
    +}
    +
    +/**
    + * test read Schema from Data File
    + *
    + * @param env jni env
    + * @return whether it is success
    + */
    +bool readSchemaInDataFile(JNIEnv *env) {
    +    printf("\nread Schema from Data File:\n");
    +    CarbonSchemaReader carbonSchemaReader(env);
    +    jobject schema = carbonSchemaReader.readSchemaInDataFile(
    --- End diff --
   
    no binaries files in repo, only in my local


---