[GitHub] [carbondata] nihal0107 opened a new pull request #3921: [CARBONDATA-3928] Removed records from exception message.

classic Classic list List threaded Threaded
10 messages Options
Reply | Threaded
Open this post in threaded view
|

[GitHub] [carbondata] nihal0107 opened a new pull request #3921: [CARBONDATA-3928] Removed records from exception message.

GitBox

nihal0107 opened a new pull request #3921:
URL: https://github.com/apache/carbondata/pull/3921


    ### Why is this PR needed?
   Currently, when the string length exceeds 32000 and bad record action as default then
   records are thrown in exception message which violates security concern.
   
    ### What changes were proposed in this PR?
    Removed the records from the exception message and only printed in logger file and redirected csv file.
       
    ### Does this PR introduce any user interface change?
    - No
   
    ### Is any new testcase added?
    - Yes
   
       
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [carbondata] CarbonDataQA1 commented on pull request #3921: [CARBONDATA-3928] Removed records from exception message.

GitBox

CarbonDataQA1 commented on pull request #3921:
URL: https://github.com/apache/carbondata/pull/3921#issuecomment-690984549


   Build Success with Spark 2.3.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/4046/
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [carbondata] CarbonDataQA1 commented on pull request #3921: [CARBONDATA-3928] Removed records from exception message.

GitBox
In reply to this post by GitBox

CarbonDataQA1 commented on pull request #3921:
URL: https://github.com/apache/carbondata/pull/3921#issuecomment-690984968


   Build Success with Spark 2.4.5, Please check CI http://121.244.95.60:12545/job/ApacheCarbon_PR_Builder_2.4.5/2308/
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [carbondata] CarbonDataQA1 commented on pull request #3921: [CARBONDATA-3928] Removed records from exception message.

GitBox
In reply to this post by GitBox

CarbonDataQA1 commented on pull request #3921:
URL: https://github.com/apache/carbondata/pull/3921#issuecomment-690984549






----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [carbondata] CarbonDataQA1 commented on pull request #3921: [CARBONDATA-3928] Removed records from exception message.

GitBox
In reply to this post by GitBox

CarbonDataQA1 commented on pull request #3921:
URL: https://github.com/apache/carbondata/pull/3921#issuecomment-690984549






----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [carbondata] ajantha-bhat commented on a change in pull request #3921: [CARBONDATA-3928] Removed records from exception message.

GitBox
In reply to this post by GitBox

ajantha-bhat commented on a change in pull request #3921:
URL: https://github.com/apache/carbondata/pull/3921#discussion_r488454458



##########
File path: processing/src/main/java/org/apache/carbondata/processing/loading/converter/impl/RowConverterImpl.java
##########
@@ -119,10 +118,6 @@ public CarbonRow convert(CarbonRow row) throws CarbonDataLoadingException {
             .getTableProperties();
     String spatialProperty = properties.get(CarbonCommonConstants.SPATIAL_INDEX);
     boolean isSpatialColumn = false;
-    Object[] rawData = row.getRawData();
-    if (rawData == null) {
-      rawData = row.getData() == null ? null : row.getData().clone();

Review comment:
       Please revert back this change, somecases rawData will not be set, hence they are setting here before converting the row.
   Now your current code will use null instead of row.getData().clone() in this scenario.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [carbondata] nihal0107 commented on a change in pull request #3921: [CARBONDATA-3928] Removed records from exception message.

GitBox
In reply to this post by GitBox

nihal0107 commented on a change in pull request #3921:
URL: https://github.com/apache/carbondata/pull/3921#discussion_r488461592



##########
File path: processing/src/main/java/org/apache/carbondata/processing/loading/converter/impl/RowConverterImpl.java
##########
@@ -119,10 +118,6 @@ public CarbonRow convert(CarbonRow row) throws CarbonDataLoadingException {
             .getTableProperties();
     String spatialProperty = properties.get(CarbonCommonConstants.SPATIAL_INDEX);
     boolean isSpatialColumn = false;
-    Object[] rawData = row.getRawData();
-    if (rawData == null) {
-      rawData = row.getData() == null ? null : row.getData().clone();

Review comment:
       I had only added this scenario last time because at that time we wanted rawdata for every bad record action. But now we need this only in case of bad record logger is enable or action is redirect. And in this case, rawdata will be always available.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [carbondata] ajantha-bhat commented on a change in pull request #3921: [CARBONDATA-3928] Removed records from exception message.

GitBox
In reply to this post by GitBox

ajantha-bhat commented on a change in pull request #3921:
URL: https://github.com/apache/carbondata/pull/3921#discussion_r488463308



##########
File path: processing/src/main/java/org/apache/carbondata/processing/loading/converter/impl/RowConverterImpl.java
##########
@@ -119,10 +118,6 @@ public CarbonRow convert(CarbonRow row) throws CarbonDataLoadingException {
             .getTableProperties();
     String spatialProperty = properties.get(CarbonCommonConstants.SPATIAL_INDEX);
     boolean isSpatialColumn = false;
-    Object[] rawData = row.getRawData();
-    if (rawData == null) {
-      rawData = row.getData() == null ? null : row.getData().clone();

Review comment:
       Ok then.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [carbondata] ajantha-bhat commented on pull request #3921: [CARBONDATA-3928] Removed records from exception message.

GitBox
In reply to this post by GitBox

ajantha-bhat commented on pull request #3921:
URL: https://github.com/apache/carbondata/pull/3921#issuecomment-692538365


   LGTM


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [carbondata] asfgit closed pull request #3921: [CARBONDATA-3928] Removed records from exception message.

GitBox
In reply to this post by GitBox

asfgit closed pull request #3921:
URL: https://github.com/apache/carbondata/pull/3921


   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]